Skip to content

Commit c46db0c

Browse files
MGMT-18993: When installing a node configured with a bond interface comprising of two or more network interfaces, the mac address of the bond and all its slave interfaces is set to match the mac address of the first slave. This can lead to a mismatch with the bootMACAddress field of the corresponding BareMetalHost, preventing the agent from being correctly matched with it. This PR ensures that each interface retains its actual permanent mac address, rather than the visible address set by the bonding configuration. (#1074)
Co-authored-by: omer-vishlitzky <omer.vishlitzky@gmail.com>
1 parent 586754c commit c46db0c

File tree

4 files changed

+51
-5
lines changed

4 files changed

+51
-5
lines changed

src/inventory/interfaces.go

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/openshift/assisted-installer-agent/src/util"
99
"github.com/openshift/assisted-service/models"
1010
"github.com/sirupsen/logrus"
11+
"github.com/vishvananda/netlink"
1112
)
1213

1314
const ipv4LocalLinkCIDR = "169.254.0.0/16"
@@ -82,6 +83,35 @@ func getFlags(flags net.Flags) []string {
8283
}
8384
}
8485

86+
// getPermMacAddress returns the permanent MAC address regardless of network bonding configuration.
87+
// For bond slaves, it returns the PermHardwareAddr,
88+
// For non-slave interfaces, the HardwareAddr is the permanent MAC address anyway
89+
func (i *interfaces) getPermMacAddress(name string) string {
90+
link, err := i.dependencies.LinkByName(name)
91+
if err != nil {
92+
logrus.WithError(err).Warnf("Could not find netlink for interface %s", name)
93+
return ""
94+
}
95+
96+
if link.Attrs() == nil {
97+
logrus.Warnf("No attributes found for interface %s", name)
98+
return ""
99+
}
100+
101+
if link.Attrs().Slave != nil {
102+
if bondSlave, ok := link.Attrs().Slave.(*netlink.BondSlave); ok && bondSlave.PermHardwareAddr != nil {
103+
return bondSlave.PermHardwareAddr.String()
104+
}
105+
}
106+
107+
if link.Attrs().HardwareAddr != nil {
108+
return link.Attrs().HardwareAddr.String()
109+
}
110+
111+
logrus.Warnf("No hardware address found for interface %s", name)
112+
return ""
113+
}
114+
85115
func (i *interfaces) getInterfaces() []*models.Interface {
86116
ret := make([]*models.Interface, 0)
87117
ins, err := i.dependencies.Interfaces()
@@ -95,7 +125,7 @@ func (i *interfaces) getInterfaces() []*models.Interface {
95125
HasCarrier: i.hasCarrier(in.Name()),
96126
IPV4Addresses: make([]string, 0),
97127
IPV6Addresses: make([]string, 0),
98-
MacAddress: in.HardwareAddr().String(),
128+
MacAddress: i.getPermMacAddress(in.Name()),
99129
Name: in.Name(),
100130
Mtu: int64(in.MTU()),
101131
Biosdevname: i.getBiosDevname(in.Name()),

src/inventory/interfaces_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ var _ = Describe("Interfaces", func() {
4545
dependencies.On("ReadFile", "/sys/class/net/eth0/carrier").Return([]byte("1\n"), nil).Once()
4646
dependencies.On("ReadFile", "/sys/class/net/eth0/device/device").Return([]byte("my-device"), nil).Once()
4747
dependencies.On("ReadFile", "/sys/class/net/eth0/device/vendor").Return([]byte("my-vendor"), nil).Once()
48-
dependencies.On("LinkByName", "eth0").Return(&netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "eth0"}}, nil).Once()
48+
util.SetupNetlinkMocks(dependencies, []util.Interface{interfaceMock})
4949
dependencies.On("RouteList", mock.Anything, mock.Anything).Return([]netlink.Route{
5050
{
5151
Dst: &net.IPNet{IP: net.ParseIP("de90::"), Mask: net.CIDRMask(64, 128)},
@@ -105,7 +105,7 @@ var _ = Describe("Interfaces", func() {
105105
dependencies.On("ReadFile", "/sys/class/net/eth2.10/carrier").Return(nil, errors.New("Blah")).Once()
106106
dependencies.On("ReadFile", "/sys/class/net/eth2.10/device/device").Return(nil, errors.New("Blah")).Once()
107107
dependencies.On("ReadFile", "/sys/class/net/eth2.10/device/vendor").Return([]byte("my-vendor2"), nil).Once()
108-
dependencies.On("LinkByName", mock.Anything).Return(&netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "eth0"}}, nil)
108+
util.SetupNetlinkMocks(dependencies, rets)
109109
dependencies.On("RouteList", mock.Anything, mock.Anything).Return([]netlink.Route{
110110
{
111111
Dst: &net.IPNet{IP: net.ParseIP("de90::"), Mask: net.CIDRMask(62, 128)},

src/scanners/machine_uuid_scanner_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ var _ = Describe("Machine uuid test", func() {
116116
dependencies.On("ReadFile", "/sys/class/net/eth1/carrier").Return(nil, errors.New("Blah")).Once()
117117
dependencies.On("ReadFile", "/sys/class/net/eth1/device/device").Return(nil, errors.New("Blah")).Once()
118118
dependencies.On("ReadFile", "/sys/class/net/eth1/device/vendor").Return([]byte("my-vendor2"), nil).Once()
119-
dependencies.On("LinkByName", mock.Anything).Return(&netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "eth0"}}, nil)
119+
agentutils.SetupNetlinkMocks(dependencies, rets)
120120
dependencies.On("RouteList", mock.Anything, mock.Anything).Return([]netlink.Route{
121121
{
122122
Dst: &net.IPNet{IP: net.ParseIP("de90::"), Mask: net.CIDRMask(64, 128)},

src/util/test_utils.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"net"
55

66
"github.com/stretchr/testify/mock"
7+
"github.com/vishvananda/netlink"
78
)
89

910
func NewFilledMockInterface(mtu int, name string, macAddr string, flags net.Flags, addrs []string, speedMbps int64, interfaceType string) *MockInterface {
@@ -16,7 +17,7 @@ func FillInterfaceMock(mock *mock.Mock, mtu int, name string, macAddr string, fl
1617
mock.On("Name").Return(name)
1718
mock.On("MTU").Return(mtu)
1819
hwAddr, _ := net.ParseMAC(macAddr)
19-
mock.On("HardwareAddr").Return(hwAddr)
20+
mock.On("HardwareAddr").Return(hwAddr).Maybe()
2021
mock.On("Flags").Return(flags)
2122
mock.On("Addrs").Return(parseAddresses(addrs), nil).Once()
2223
mock.On("SpeedMbps").Return(speedMbps)
@@ -42,3 +43,18 @@ func parseAddress(addrStr string) net.Addr {
4243
}
4344
return &net.IPNet{IP: ip, Mask: ipnet.Mask}
4445
}
46+
47+
func SetupNetlinkMocks(dependencies *MockIDependencies, interfaces []Interface) {
48+
for _, iface := range interfaces {
49+
name := iface.Name()
50+
hwAddr := iface.HardwareAddr()
51+
52+
link := &netlink.Dummy{
53+
LinkAttrs: netlink.LinkAttrs{
54+
Name: name,
55+
HardwareAddr: hwAddr,
56+
},
57+
}
58+
dependencies.On("LinkByName", name).Return(link, nil).Maybe()
59+
}
60+
}

0 commit comments

Comments
 (0)