wrong HWADDR on interface when ethernet_mac_address is given on the bond

[frigo]

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.

[Dan Watkins (oddbloke)]

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.)

[Dan Watkins (oddbloke)]

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:
            - 2.30.162.119/23
            interfaces:
            - eno50
            - eno53
            macaddress: b2:35:4a:42:42:ed
            nameservers:
                addresses:
                - 2.23.186.183
                - 2.23.186.184
                search: []
            parameters:
                mode: active-backup
            routes:
            - to: 0.0.0.0/0
                via: 2.30.162.1
    vlans:
        bond0.591:
            id: 591
            link: bond0
            macaddress: b2:35:4a:42:42:ed

[Dan Watkins (oddbloke)]

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.

[Dan Watkins (oddbloke)]

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

[Dan Watkins (oddbloke)]

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.

[Ryan Harper (raharper)]

> 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.

[James Falcon (falcojr)]

Tracked in Github Issues as https://github.com/canonical/cloud-init/issues/3856