canonical / cloud-init

Official upstream for the cloud-init: cloud instance initialization
https://cloud-init.io/
Other
2.93k stars 871 forks source link

wrong HWADDR on interface when ethernet_mac_address is given on the bond #3856

Open ubuntu-server-builder opened 1 year ago

ubuntu-server-builder commented 1 year ago

This bug was originally filed in Launchpad as LP: #1919045

Launchpad details
affected_projects = []
assignee = None
assignee_name = None
date_closed = None
date_created = 2021-03-13T21:52:47.007288+00:00
date_fix_committed = None
date_fix_released = None
id = 1919045
importance = undecided
is_complete = False
lp_url = https://bugs.launchpad.net/cloud-init/+bug/1919045
milestone = None
owner = rigault-francois
owner_name = frigo
private = False
status = triaged
submitter = rigault-francois
submitter_name = frigo
tags = []
duplicates = []

Launchpad user frigo(rigault-francois) wrote on 2021-03-13T21:52:47.007288+00:00

creating a configuration for an active-backup bond:

cloud-init devel net-convert -p network_data.json -k network_data.json \
-d /tmp -D rhel -O sysconfig \
-m eno50,b2:35:4a:42:42:ed -m eno53,b2:35:4a:42:42:ee

with cloud-init (cloud-init-19.4-7.el7_9.3.x86_64, cloud-init-20.3-10.el8.noarch) assuming there is a ethernet_mac_address defined on the bond:

    {
      "id": "bond0",
      "type": "bond",
      "ethernet_mac_address": "b2:35:4a:42:42:ed",
      "bond_mode": "active-backup"
      ...

we see cloud-init creates a file ifcfg-eno53 wrongly containing a HWADDR with the address of the bond. For a bond, we expect both interfaces to display the same mac when running ip link (default behavior with fail_over_mac=0 bonding module parameter). However, the HWADDR in the ifcfg file must retain the original mac address of the interface, or the interface is not configured correctly. Practically, it is never configured as slave.

Workaround seems to just remove the "ethernet_mac_address" on the bond there (I originally added it because it is required according to https://github.com/openstack/nova/blob/master/doc/api_schemas/network_data.json, which I was using to validate my network_data file).

I would expect the ifcfg-eno53 mac address to not be impacted by the presence of ethernet_mac_address on the bond. For reference I attach my network_data.json if anyone wants to try.

ubuntu-server-builder commented 1 year ago

Launchpad user frigo(rigault-francois) wrote on 2021-03-13T21:52:47.007288+00:00

Launchpad attachments: network_data.json

ubuntu-server-builder commented 1 year ago

Launchpad user Dan Watkins(oddbloke) wrote on 2021-03-17T16:01:08.395216+00:00

Hi frigo, thanks for the bug report! I've reproduced the issue locally, and done some initial investigation.

The OpenStack network_data.json parsing code itself is not responsible for this: if I instrument it to emit the config it generates, I see:

[{'mac_address': 'b2:35:4a:42:42:ed', 'mtu': 1500, 'name': 'eno50', 'subnets': [], 'type': 'physical'}, {'mac_address': 'b2:35:4a:42:42:ee', 'mtu': 1500, 'name': 'eno53', 'subnets': [], 'type': 'physical'}, {'bond_interfaces': ['eno50', 'eno53'], 'name': 'bond0', 'params': {'bond_mode': 'active-backup', 'mac_address': 'b2:35:4a:42:42:ed'}, 'subnets': [{'address': '2.30.162.119', 'ipv4': True, 'netmask': '255.255.254.0', 'routes': [{'gateway': '2.30.162.1', 'netmask': '0.0.0.0', 'network': '0.0.0.0'}], 'type': 'static'}], 'type': 'bond'}, {'mac_address': 'b2:35:4a:42:42:ed', 'name': 'bond0.591', 'subnets': [], 'type': 'vlan', 'vlan_id': 591, 'vlan_link': 'bond0'}, {'address': '2.23.186.183', 'type': 'nameserver'}, {'address': '2.23.186.184', 'type': 'nameserver'}]

As we can see, eno53 correctly has its :ee MAC address.

Digging in further, I believe the issue is in the network_state code (and so, I suspect, does not only affect RHEL/sysconfig). Specifically, https://github.com/canonical/cloud-init/blob/master/cloudinit/net/network_state.py#L456-L458 is doing (earlier line included for context):

    for ifname in command.get('bond_interfaces'):
        ...
        # copy in bond config into slave
        for param, val in command.get('params').items():
            bond_if.update({param: val})

This iterates over each of the ifnames in bond_interfaces, and sets every (param, val) in command["params"]; in a debugger, I can see that command["params"] is:

ipdb> command.get('params')
{'mac_address': 'b2:35:4a:42:42:ed', 'bond_mode': 'active-backup'}

As a result, every bond member will have the bond's configured MAC address set on them. (In the given configuration, eno50's MAC address is also modified, but as it already has the same MAC address as the bond, it has no effect.)

ubuntu-server-builder commented 1 year ago

Launchpad user Dan Watkins(oddbloke) wrote on 2021-03-17T16:02:12.256761+00:00

I've confirmed this issue is present on Ubuntu:

$ cloud-init devel net-convert -k network_data.json -p ~/Downloads/network_data.json -d /tmp -D ubuntu -O netplan -m eno50,b2:35:4a:42:42:ed -m eno53,b2:35:4a:42:42:ee Read input format 'network_data.json' from '/home/daniel/Downloads/network_data.json'. Wrote output format 'netplan' to '/tmp/'

$ cat /tmp/etc/netplan/50-cloud-init.yaml

This file is generated from information provided by the datasource. Changes

to it will not persist across an instance reboot. To disable cloud-init's

network configuration capabilities, write a file

/etc/cloud/cloud.cfg.d/99-disable-network-config.cfg with the following:

network: {config: disabled}

network: version: 2 ethernets: eno50: match: macaddress: b2:35:4a:42:42:ed mtu: 1500 set-name: eno50 eno53: match: macaddress: b2:35:4a:42:42:ed mtu: 1500 set-name: eno53 bonds: bond0: addresses:

ubuntu-server-builder commented 1 year ago

Launchpad user Dan Watkins(oddbloke) wrote on 2021-03-17T16:04:10.417609+00:00

I believe that https://git.launchpad.net/cloud-init/commit/?id=c6cfed7f introduced this issue: prior to that, mac_address was not set in the params dict. We will need to be mindful of not regressing that usecase when addressing this.

ubuntu-server-builder commented 1 year ago

Launchpad user Dan Watkins(oddbloke) wrote on 2021-03-17T16:06:46.112428+00:00

(The bug that commit was fixing is https://bugs.launchpad.net/cloud-init/+bug/1682064)

ubuntu-server-builder commented 1 year ago

Launchpad user Dan Watkins(oddbloke) wrote on 2021-03-17T16:11:10.299516+00:00

This is all consonant with your described workaround: if ethernet_mac_address is not set on the bond in the network_data.json configuration, it will not be present in params and so will not override the MAC addresses set on physical interfaces.

ubuntu-server-builder commented 1 year ago

Launchpad user Ryan Harper(raharper) wrote on 2021-03-17T22:44:50.441109+00:00

As a result, every bond member will have the bond's configured MAC address set on them. (In the given configuration, eno50's MAC address is also modified, but as it already has the same MAC address as the bond, it has no effect.)

Some historical context: when this code was written we were rendering only ENI (etc/network/intefaces ifupdown) and the best practice there was to ensure that each bond member included the bond configuration as ifupdown was invoked in parallel and raced as to which interface would be the first to pull in the configuration.

On sysconfig or netplan this sort of duplicate configuration on members may not be needed.