From 642fffe0203a8ffcc2443db529af49f54fb3b94f Mon Sep 17 00:00:00 2001 From: Laszlo Janosi Date: Thu, 22 Jan 2026 15:38:36 +0000 Subject: [PATCH] Fix vlan and bond config generation This change fixes how glean generates the vlan and bond configurations for Red Hat and related distros. - Problem: glean set TYPE=ETHERNET for all interface types in the ifcfg files. Fix: glean sets proper "TYPE=Bond" and "TYPE=Vlan" for bond and vlan interfaces respectively - Problem: glean set type=802-3-ethernet for all interface types in the NetworkManager keyfiles Fix: glean sets proper "type=bond" and "type=vlan" for bond and vlan interfaces respectively - Problem: glean generates netdev file for physical interfaces Fix: glean generates netdev files for virtual interfaces only. Story: 2011658 Change-Id: Iee68d154cd8e31d1a1781d517ad61f000dd5c4cc Signed-off-by: Laszlo Janosi --- glean/cmd.py | 71 +++++++---- .../fixtures/test/liberty.fedora.network.out | 10 +- .../test/liberty.redhat-10.network.out | 27 +--- .../fixtures/test/liberty.redhat.network.out | 10 +- glean/tests/test_glean.py | 119 ++++++++++++++++++ 5 files changed, 183 insertions(+), 54 deletions(-) diff --git a/glean/cmd.py b/glean/cmd.py index 024c329..55d4659 100644 --- a/glean/cmd.py +++ b/glean/cmd.py @@ -116,9 +116,14 @@ def _network_files(distro): return network_files -def _network_config(args): - distro = args.distro +def _network_config(args, interface): + """Generate network configuration preamble. + Args: + args: Command line arguments + interface: Interface dictionary to determine type from + """ + distro = args.distro if _is_suse(distro): preamble = textwrap.dedent("""\ # Automatically generated, do not edit @@ -127,28 +132,50 @@ def _network_config(args): STARTMODE=auto """) elif is_keyfile_format(): + # Map interface type to connection type + if 'bond_mode' in interface: + connection_type = "bond" + elif 'vlan_id' in interface: + connection_type = "vlan" + else: + connection_type = "802-3-ethernet" + preamble = textwrap.dedent("""\ # Automatically generated, do not edit [connection] - id={id} - type=802-3-ethernet + id={{id}} + type={type} [ipv4] - method={method} + method={{method}} + """).format(type=connection_type) - [802-3-ethernet] - mac_address={mac_address} - """) + # Only regular ethernet needs the [802-3-ethernet] section + if connection_type == "802-3-ethernet": + preamble += textwrap.dedent("""\ + + [802-3-ethernet] + mac_address={mac_address} + """) else: + # For Red Hat ifcfg format + nm_controlled = "yes" if args.use_nm else "no" + if 'bond_mode' in interface: + type_line = "Bond" + elif 'vlan_id' in interface: + type_line = "Vlan" + else: + type_line = "Ethernet" + preamble = textwrap.dedent("""\ # Automatically generated, do not edit - DEVICE={name} - BOOTPROTO={bootproto} - HWADDR={hwaddr} + DEVICE={{name}} + BOOTPROTO={{bootproto}} + HWADDR={{hwaddr}} ONBOOT=yes - NM_CONTROLLED=%s - TYPE=Ethernet - """ % ("yes" if args.use_nm else "no")) + NM_CONTROLLED={nm} + TYPE={type} + """).format(nm=nm_controlled, type=type_line) return preamble @@ -178,7 +205,7 @@ def _set_rh_bonding(name, interface, distro, results): elif is_keyfile_format(): if 'bond_slaves' in interface: - results += "type=bond\n" + pass else: results += "master={0}\n".format(interface['bond_master']) @@ -312,7 +339,7 @@ def _write_rh_v6_keyfile_interface(name, interface, args, files): def _write_rh_interface(name, interface, args): distro = args.distro files_to_write = dict() - results = _network_config(args).format( + results = _network_config(args, interface).format( bootproto="static", name=name, hwaddr=interface['mac_address'], @@ -363,7 +390,7 @@ def _write_rh_interface(name, interface, args): def _write_rh_dhcp(name, interface, args): distro = args.distro filename = _network_files(distro)["ifcfg"] + '-{name}'.format(name=name) - results = _network_config(args).format( + results = _network_config(args, interface).format( bootproto="dhcp", name=name, hwaddr=interface['mac_address']) results += _set_rh_vlan(name, interface, distro) # set_rh_bonding takes results as argument so we need to assign @@ -378,7 +405,7 @@ def _write_rh_keyfile_dhcp(name, interface, args): filename = \ '/etc/NetworkManager/system-connections/{name}.nmconnection'.format( name=name) - results = _network_config(args).format( + results = _network_config(args, interface).format( method="auto", id=name, mac_address=interface['mac_address']) results += _set_rh_vlan(name, interface, distro) # set_rh_bonding takes results as argument so we need to assign @@ -394,12 +421,12 @@ def _write_rh_manual(name, interface, args): filename = \ '/etc/NetworkManager/system-connections/{name}.nmconnection' \ .format(name=name) - results = _network_config(args).format( + results = _network_config(args, interface).format( method="manual", id=name, mac_address=interface['mac_address']) else: filename = _network_files(distro)["ifcfg"] + '-{name}'.format( name=name) - results = _network_config(args).format( + results = _network_config(args, interface).format( bootproto="none", name=name, hwaddr=interface['mac_address']) results += _set_rh_vlan(name, interface, distro) # set_rh_bonding takes results as argument so we need to assign @@ -418,7 +445,7 @@ def _write_rh_keyfile_interface(_name, interface, args): filename = \ '/etc/NetworkManager/system-connections/{name}.nmconnection'.format( name=_name) - results = _network_config(args).format( + results = _network_config(args, interface).format( method="manual", id=_name, mac_address=interface['mac_address']) # insert value after specific option using slicing @@ -705,7 +732,7 @@ def _write_networkd_interface(name, interfaces, args, files_struct=dict()): }) # create netdev files - if 'bond_mode' or 'vlan_id' in interface: + if any(key in interface for key in ['bond_mode', 'vlan_id']): netdev_file = \ '/etc/systemd/network/{name}.netdev'.format(name=iname) if netdev_file not in files_struct: diff --git a/glean/tests/fixtures/test/liberty.fedora.network.out b/glean/tests/fixtures/test/liberty.fedora.network.out index c1ecd6f..18c99bd 100644 --- a/glean/tests/fixtures/test/liberty.fedora.network.out +++ b/glean/tests/fixtures/test/liberty.fedora.network.out @@ -38,7 +38,7 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:12:a4:bc ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Vlan VLAN=yes ### Write /etc/sysconfig/network-scripts/ifcfg-eth4.26 # Automatically generated, do not edit @@ -47,7 +47,7 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:12:a4:bd ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Vlan VLAN=yes ### Write /etc/sysconfig/network-scripts/ifcfg-eth5 # Automatically generated, do not edit @@ -76,7 +76,7 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:05:7b:13 ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Bond ### Write /etc/sysconfig/network-scripts/ifcfg-eth7 # Automatically generated, do not edit DEVICE=eth7 @@ -104,7 +104,7 @@ BOOTPROTO=none HWADDR=bc:76:4e:05:7b:15 ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Bond ### Write /etc/sysconfig/network-scripts/ifcfg-bond1.27 # Automatically generated, do not edit DEVICE=bond1.27 @@ -112,7 +112,7 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:12:a4:be ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Vlan VLAN=yes ### Write /etc/sysconfig/network-scripts/route-eth1 ADDRESS0=10.176.0.0 diff --git a/glean/tests/fixtures/test/liberty.redhat-10.network.out b/glean/tests/fixtures/test/liberty.redhat-10.network.out index 68a111a..1d7b3a9 100644 --- a/glean/tests/fixtures/test/liberty.redhat-10.network.out +++ b/glean/tests/fixtures/test/liberty.redhat-10.network.out @@ -43,14 +43,11 @@ mac_address=bc:76:4e:12:a4:bb # Automatically generated, do not edit [connection] id=eth4.25 -type=802-3-ethernet +type=vlan [ipv4] method=auto -[802-3-ethernet] -mac_address=bc:76:4e:12:a4:bc - [vlan] id=25 parent=ethwithvlan @@ -58,14 +55,11 @@ parent=ethwithvlan # Automatically generated, do not edit [connection] id=eth4.26 -type=802-3-ethernet +type=vlan [ipv4] method=auto -[802-3-ethernet] -mac_address=bc:76:4e:12:a4:bd - [vlan] id=26 parent=ethwithvlan @@ -99,14 +93,10 @@ slave-type=bond # Automatically generated, do not edit [connection] id=bond0 -type=802-3-ethernet +type=bond [ipv4] method=auto - -[802-3-ethernet] -mac_address=bc:76:4e:05:7b:13 -type=bond ### Write /etc/NetworkManager/system-connections/eth7.nmconnection # Automatically generated, do not edit [connection] @@ -137,26 +127,19 @@ slave-type=bond # Automatically generated, do not edit [connection] id=bond1 -type=802-3-ethernet +type=bond [ipv4] method=manual - -[802-3-ethernet] -mac_address=bc:76:4e:05:7b:15 -type=bond ### Write /etc/NetworkManager/system-connections/bond1.27.nmconnection # Automatically generated, do not edit [connection] id=bond1.27 -type=802-3-ethernet +type=vlan [ipv4] method=auto -[802-3-ethernet] -mac_address=bc:76:4e:12:a4:be - [vlan] id=27 parent=bond1 diff --git a/glean/tests/fixtures/test/liberty.redhat.network.out b/glean/tests/fixtures/test/liberty.redhat.network.out index 3dcd063..0d37416 100644 --- a/glean/tests/fixtures/test/liberty.redhat.network.out +++ b/glean/tests/fixtures/test/liberty.redhat.network.out @@ -38,7 +38,7 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:12:a4:bc ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Vlan VLAN=yes ### Write /etc/sysconfig/network-scripts/ifcfg-eth4.26 # Automatically generated, do not edit @@ -47,7 +47,7 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:12:a4:bd ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Vlan VLAN=yes ### Write /etc/sysconfig/network-scripts/route-eth1 ADDRESS0=10.176.0.0 @@ -83,7 +83,7 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:05:7b:13 ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Bond ### Write /etc/sysconfig/network-scripts/ifcfg-eth7 # Automatically generated, do not edit DEVICE=eth7 @@ -111,7 +111,7 @@ BOOTPROTO=none HWADDR=bc:76:4e:05:7b:15 ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Bond ### Write /etc/sysconfig/network-scripts/ifcfg-bond1.27 # Automatically generated, do not edit DEVICE=bond1.27 @@ -119,5 +119,5 @@ BOOTPROTO=dhcp HWADDR=bc:76:4e:12:a4:be ONBOOT=yes %NM_CONTROLLED% -TYPE=Ethernet +TYPE=Vlan VLAN=yes diff --git a/glean/tests/test_glean.py b/glean/tests/test_glean.py index 5e741bc..f243a35 100644 --- a/glean/tests/test_glean.py +++ b/glean/tests/test_glean.py @@ -347,3 +347,122 @@ class TestGlean(base.BaseTestCase): with mock.patch('glean.systemlock.Lock'): self._assert_distro_provider( self.distro, self.style, interfaces[self.style], use_nm=True) + + def test_networkd_physical_ethernet_no_netdev(self): + """Test that physical ethernet interfaces do not generate .netdev files + + Only VLAN and bond interfaces should generate .netdev files. + Physical ethernet interfaces should only have .network files. + + Uses naming patterns to identify interface types: + - VLANs: "{parent}-vlan{vlan_id}" + - Bonds: "bondX" where X is digit (from link['id']) + - Physical: "ethX" where X is digit (from sys_interfaces[mac]) + """ + # Only test for networkd distro + if self.distro != 'networkd': + self.skipTest("networkd-specific test") + + # Run the actual code + with mock.patch('glean.systemlock.Lock'): + self._assert_distro_provider(self.distro, self.style, + interfaces[self.style]) + + found = False + # Check what was generated by examining file_handle_mocks + for file_path in self.file_handle_mocks.keys(): + if (file_path.startswith('/etc/systemd/network/') and + file_path.endswith('.network')): + iface_name = file_path.replace( + '/etc/systemd/network/', '').replace('.network', '') + if 'bond' not in iface_name and '-vlan' not in iface_name: + netdev_path = f'/etc/systemd/network/{iface_name}.netdev' + self.assertNotIn(netdev_path, self.file_handle_mocks, + f"Physical ethernet " + f"interface {iface_name} " + f"should NOT generate .netdev") + found = True + if not found: + self.skipTest("No physical interfaces to check for netdev files") + + def test_networkd_vlan_generates_netdev(self): + """Test that VLAN interfaces generate .netdev files + + VLANs should generate .netdev files. + Uses naming pattern: "{parent}-vlan{vlan_id}" + """ + # Only test for networkd distro + if self.distro != 'networkd': + self.skipTest("networkd-specific test") + + # Run the actual code + with mock.patch('glean.systemlock.Lock'): + self._assert_distro_provider(self.distro, self.style, + interfaces[self.style]) + + # Check what was generated by examining file_handle_mocks + found_vlan_with_netdev = False + + for file_path in self.file_handle_mocks.keys(): + if (file_path.startswith('/etc/systemd/network/') and + file_path.endswith('.network')): + iface_name = file_path.replace( + '/etc/systemd/network/', '').replace('.network', '') + + # Identify VLAN by naming pattern + is_vlan = ('-vlan' in iface_name and + iface_name.split('-vlan')[1].isdigit()) + + if is_vlan: + netdev_file = ( + f'/etc/systemd/network/{iface_name}.netdev') + self.assertIn( + netdev_file, self.file_handle_mocks, + f"VLAN interface {iface_name} should " + f"generate .netdev") + found_vlan_with_netdev = True + + # Ensure we actually tested VLANs + if not found_vlan_with_netdev: + self.skipTest("No VLAN interfaces found to validate") + + def test_networkd_bond_generates_netdev(self): + """Test that bond interfaces generate .netdev files + + Bonds should generate .netdev files. + Uses naming pattern: "bondX" where X is digit (from link['id']) + """ + # Only test for networkd distro + if self.distro != 'networkd': + self.skipTest("networkd-specific test") + + # Run the actual code + with mock.patch('glean.systemlock.Lock'): + self._assert_distro_provider(self.distro, self.style, + interfaces[self.style]) + + # Check what was generated by examining file_handle_mocks + found_bond_with_netdev = False + + for file_path in self.file_handle_mocks.keys(): + if (file_path.startswith('/etc/systemd/network/') and + file_path.endswith('.network')): + iface_name = file_path.replace( + '/etc/systemd/network/', '').replace('.network', '') + + # Identify bond by naming pattern + is_bond = (iface_name.startswith('bond') and + iface_name[4:].isdigit()) + + if is_bond: + netdev_file = ( + f'/etc/systemd/network/{iface_name}.netdev') + self.assertIn( + netdev_file, self.file_handle_mocks, + f"Bond interface {iface_name} should " + f"generate .netdev") + found_bond_with_netdev = True + + # Ensure we actually tested bonds + if not found_bond_with_netdev: + self.skipTest("No bond interfaces found to validate")