From 8439b191ec2f336d544cab86dba2860f969cd5b8 Mon Sep 17 00:00:00 2001 From: Eduardo Otubo Date: Tue, 15 Sep 2020 18:00:00 +0200 Subject: network: Fix type and respect name when rendering vlan in sysconfig. (#541) Prior to this change, vlans were rendered in sysconfig with 'TYPE=Ethernet', and incorrectly rendered the PHYSDEV based on the name of the vlan device rather than the 'link' provided in the network config. The change here fixes: * rendering of TYPE=Ethernet for a vlan * adds a warning if the configured device name is not supported per the RHEL 7 docs "11.5. Naming Scheme for VLAN Interfaces" LP: #1788915 LP: #1826608 RHBZ: #1861871 --- cloudinit/net/sysconfig.py | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 0a5d481d..e9337b12 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -99,6 +99,10 @@ class ConfigMap(object): def __len__(self): return len(self._conf) + def skip_key_value(self, key, val): + """Skip the pair key, value if it matches a certain rule.""" + return False + def to_string(self): buf = io.StringIO() buf.write(_make_header()) @@ -106,6 +110,8 @@ class ConfigMap(object): buf.write("\n") for key in sorted(self._conf.keys()): value = self._conf[key] + if self.skip_key_value(key, value): + continue if isinstance(value, bool): value = self._bool_map[value] if not isinstance(value, str): @@ -214,6 +220,7 @@ class NetInterface(ConfigMap): 'bond': 'Bond', 'bridge': 'Bridge', 'infiniband': 'InfiniBand', + 'vlan': 'Vlan', } def __init__(self, iface_name, base_sysconf_dir, templates, @@ -267,6 +274,11 @@ class NetInterface(ConfigMap): c.routes = self.routes.copy() return c + def skip_key_value(self, key, val): + if key == 'TYPE' and val == 'Vlan': + return True + return False + class Renderer(renderer.Renderer): """Renders network information in a /etc/sysconfig format.""" @@ -697,7 +709,16 @@ class Renderer(renderer.Renderer): iface_cfg['ETHERDEVICE'] = iface_name[:iface_name.rfind('.')] else: iface_cfg['VLAN'] = True - iface_cfg['PHYSDEV'] = iface_name[:iface_name.rfind('.')] + iface_cfg.kind = 'vlan' + + rdev = iface['vlan-raw-device'] + supported = _supported_vlan_names(rdev, iface['vlan_id']) + if iface_name not in supported: + LOG.info( + "Name '%s' for vlan '%s' is not officially supported" + "by RHEL. Supported: %s", + iface_name, rdev, ' '.join(supported)) + iface_cfg['PHYSDEV'] = rdev iface_subnets = iface.get("subnets", []) route_cfg = iface_cfg.routes @@ -896,6 +917,15 @@ class Renderer(renderer.Renderer): "\n".join(netcfg) + "\n", file_mode) +def _supported_vlan_names(rdev, vid): + """Return list of supported names for vlan devices per RHEL doc + 11.5. Naming Scheme for VLAN Interfaces.""" + return [ + v.format(rdev=rdev, vid=int(vid)) + for v in ("{rdev}{vid:04}", "{rdev}{vid}", + "{rdev}.{vid:04}", "{rdev}.{vid}")] + + def available(target=None): sysconfig = available_sysconfig(target=target) nm = available_nm(target=target) -- cgit v1.2.3 From 8ec8c3fc63a59b85888a0b52356b784314a1d4cc Mon Sep 17 00:00:00 2001 From: Anh Vo Date: Tue, 13 Oct 2020 15:42:54 -0400 Subject: net: add the ability to blacklist network interfaces based on driver during enumeration of physical network devices (#591) --- cloudinit/distros/networking.py | 6 +++- cloudinit/net/__init__.py | 35 ++++++++++++------- cloudinit/sources/DataSourceAzure.py | 36 +++++++++++++------- tests/unittests/test_datasource/test_azure.py | 49 +++++++++++++++++++-------- 4 files changed, 86 insertions(+), 40 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index 10ed249d..e407fa29 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -22,6 +22,9 @@ class Networking(metaclass=abc.ABCMeta): Hierarchy" in HACKING.rst for full details. """ + def __init__(self): + self.blacklist_drivers = None + def _get_current_rename_info(self) -> dict: return net._get_current_rename_info() @@ -68,7 +71,8 @@ class Networking(metaclass=abc.ABCMeta): return net.get_interfaces() def get_interfaces_by_mac(self) -> dict: - return net.get_interfaces_by_mac() + return net.get_interfaces_by_mac( + blacklist_drivers=self.blacklist_drivers) def get_master(self, devname: DeviceName): return net.get_master(devname) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index e233149a..75e79ca8 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -746,18 +746,22 @@ def get_ib_interface_hwaddr(ifname, ethernet_format): return mac -def get_interfaces_by_mac(): +def get_interfaces_by_mac(blacklist_drivers=None) -> dict: if util.is_FreeBSD(): - return get_interfaces_by_mac_on_freebsd() + return get_interfaces_by_mac_on_freebsd( + blacklist_drivers=blacklist_drivers) elif util.is_NetBSD(): - return get_interfaces_by_mac_on_netbsd() + return get_interfaces_by_mac_on_netbsd( + blacklist_drivers=blacklist_drivers) elif util.is_OpenBSD(): - return get_interfaces_by_mac_on_openbsd() + return get_interfaces_by_mac_on_openbsd( + blacklist_drivers=blacklist_drivers) else: - return get_interfaces_by_mac_on_linux() + return get_interfaces_by_mac_on_linux( + blacklist_drivers=blacklist_drivers) -def get_interfaces_by_mac_on_freebsd(): +def get_interfaces_by_mac_on_freebsd(blacklist_drivers=None) -> dict(): (out, _) = subp.subp(['ifconfig', '-a', 'ether']) # flatten each interface block in a single line @@ -784,7 +788,7 @@ def get_interfaces_by_mac_on_freebsd(): return results -def get_interfaces_by_mac_on_netbsd(): +def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict(): ret = {} re_field_match = ( r"(?P\w+).*address:\s" @@ -800,7 +804,7 @@ def get_interfaces_by_mac_on_netbsd(): return ret -def get_interfaces_by_mac_on_openbsd(): +def get_interfaces_by_mac_on_openbsd(blacklist_drivers=None) -> dict(): ret = {} re_field_match = ( r"(?P\w+).*lladdr\s" @@ -815,12 +819,13 @@ def get_interfaces_by_mac_on_openbsd(): return ret -def get_interfaces_by_mac_on_linux(): +def get_interfaces_by_mac_on_linux(blacklist_drivers=None) -> dict: """Build a dictionary of tuples {mac: name}. Bridges and any devices that have a 'stolen' mac are excluded.""" ret = {} - for name, mac, _driver, _devid in get_interfaces(): + for name, mac, _driver, _devid in get_interfaces( + blacklist_drivers=blacklist_drivers): if mac in ret: raise RuntimeError( "duplicate mac found! both '%s' and '%s' have mac '%s'" % @@ -838,11 +843,13 @@ def get_interfaces_by_mac_on_linux(): return ret -def get_interfaces(): +def get_interfaces(blacklist_drivers=None) -> list: """Return list of interface tuples (name, mac, driver, device_id) Bridges and any devices that have a 'stolen' mac are excluded.""" ret = [] + if blacklist_drivers is None: + blacklist_drivers = [] devs = get_devicelist() # 16 somewhat arbitrarily chosen. Normally a mac is 6 '00:' tokens. zero_mac = ':'.join(('00',) * 16) @@ -866,7 +873,11 @@ def get_interfaces(): # skip nics that have no mac (00:00....) if name != 'lo' and mac == zero_mac[:len(mac)]: continue - ret.append((name, mac, device_driver(name), device_devid(name))) + # skip nics that have drivers blacklisted + driver = device_driver(name) + if driver in blacklist_drivers: + continue + ret.append((name, mac, driver, device_devid(name))) return ret diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 773c60d7..fc32f8b1 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -83,6 +83,25 @@ UBUNTU_EXTENDED_NETWORK_SCRIPTS = [ '/run/network/interfaces.ephemeral.d', ] +# This list is used to blacklist devices that will be considered +# for renaming or fallback interfaces. +# +# On Azure network devices using these drivers are automatically +# configured by the platform and should not be configured by +# cloud-init's network configuration. +# +# Note: +# Azure Dv4 and Ev4 series VMs always have mlx5 hardware. +# https://docs.microsoft.com/en-us/azure/virtual-machines/dv4-dsv4-series +# https://docs.microsoft.com/en-us/azure/virtual-machines/ev4-esv4-series +# Earlier D and E series VMs (such as Dv2, Dv3, and Ev3 series VMs) +# can have either mlx4 or mlx5 hardware, with the older series VMs +# having a higher chance of coming with mlx4 hardware. +# https://docs.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series +# https://docs.microsoft.com/en-us/azure/virtual-machines/dv3-dsv3-series +# https://docs.microsoft.com/en-us/azure/virtual-machines/ev3-esv3-series +BLACKLIST_DRIVERS = ['mlx4_core', 'mlx5_core'] + def find_storvscid_from_sysctl_pnpinfo(sysctl_out, deviceid): # extract the 'X' from dev.storvsc.X. if deviceid matches @@ -529,6 +548,8 @@ class DataSourceAzure(sources.DataSource): except Exception as e: LOG.warning("Failed to get system information: %s", e) + self.distro.networking.blacklist_drivers = BLACKLIST_DRIVERS + try: crawled_data = util.log_time( logfunc=LOG.debug, msg='Crawl of metadata service', @@ -1468,23 +1489,12 @@ def _generate_network_config_from_imds_metadata(imds_metadata) -> dict: @azure_ds_telemetry_reporter def _generate_network_config_from_fallback_config() -> dict: - """Generate fallback network config excluding mlx4_core & mlx5_core devices. + """Generate fallback network config excluding blacklisted devices. @return: Dictionary containing network version 2 standard configuration. """ - # Azure Dv4 and Ev4 series VMs always have mlx5 hardware. - # https://docs.microsoft.com/en-us/azure/virtual-machines/dv4-dsv4-series - # https://docs.microsoft.com/en-us/azure/virtual-machines/ev4-esv4-series - # Earlier D and E series VMs (such as Dv2, Dv3, and Ev3 series VMs) - # can have either mlx4 or mlx5 hardware, with the older series VMs - # having a higher chance of coming with mlx4 hardware. - # https://docs.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series - # https://docs.microsoft.com/en-us/azure/virtual-machines/dv3-dsv3-series - # https://docs.microsoft.com/en-us/azure/virtual-machines/ev3-esv3-series - blacklist = ['mlx4_core', 'mlx5_core'] - # generate a network config, blacklist picking mlx4_core and mlx5_core devs return net.generate_fallback_config( - blacklist_drivers=blacklist, config_driver=True) + blacklist_drivers=BLACKLIST_DRIVERS, config_driver=True) @azure_ds_telemetry_reporter diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 2b22a879..3b9456f7 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -526,7 +526,7 @@ scbus-1 on xpt0 bus 0 ]) return dsaz - def _get_ds(self, data, agent_command=None, distro=None, + def _get_ds(self, data, agent_command=None, distro='ubuntu', apply_network=None): def dsdevs(): @@ -576,7 +576,7 @@ scbus-1 on xpt0 bus 0 side_effect=_wait_for_files)), ]) - if distro is not None: + if isinstance(distro, str): distro_cls = distros.fetch(distro) distro = distro_cls(distro, data.get('sys_cfg', {}), self.paths) dsrc = dsaz.DataSourceAzure( @@ -638,7 +638,7 @@ scbus-1 on xpt0 bus 0 # Return a non-matching asset tag value m_is_platform_viable.return_value = False dsrc = dsaz.DataSourceAzure( - {}, distro=None, paths=self.paths) + {}, distro=mock.Mock(), paths=self.paths) self.assertFalse(dsrc.get_data()) m_is_platform_viable.assert_called_with(dsrc.seed_dir) @@ -1311,6 +1311,28 @@ scbus-1 on xpt0 bus 0 blacklist_drivers=['mlx4_core', 'mlx5_core'], config_driver=True) + @mock.patch(MOCKPATH + 'net.get_interfaces') + @mock.patch(MOCKPATH + 'util.is_FreeBSD') + def test_blacklist_through_distro( + self, m_is_freebsd, m_net_get_interfaces): + """Verify Azure DS updates blacklist drivers in the distro's + networking object.""" + odata = {'HostName': "myhost", 'UserName': "myuser"} + data = {'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': {}} + + distro_cls = distros.fetch('ubuntu') + distro = distro_cls('ubuntu', {}, self.paths) + dsrc = self._get_ds(data, distro=distro) + dsrc.get_data() + self.assertEqual(distro.networking.blacklist_drivers, + dsaz.BLACKLIST_DRIVERS) + + m_is_freebsd.return_value = False + distro.networking.get_interfaces_by_mac() + m_net_get_interfaces.assert_called_with( + blacklist_drivers=dsaz.BLACKLIST_DRIVERS) + @mock.patch(MOCKPATH + 'subp.subp') def test_get_hostname_with_no_args(self, m_subp): dsaz.get_hostname() @@ -1421,8 +1443,7 @@ class TestAzureBounce(CiTestCase): if ovfcontent is not None: populate_dir(os.path.join(self.paths.seed_dir, "azure"), {'ovf-env.xml': ovfcontent}) - dsrc = dsaz.DataSourceAzure( - {}, distro=None, paths=self.paths) + dsrc = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) if agent_command is not None: dsrc.ds_cfg['agent_command'] = agent_command return dsrc @@ -1906,7 +1927,7 @@ class TestClearCachedData(CiTestCase): tmp = self.tmp_dir() paths = helpers.Paths( {'cloud_dir': tmp, 'run_dir': tmp}) - dsrc = dsaz.DataSourceAzure({}, distro=None, paths=paths) + dsrc = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=paths) clean_values = [dsrc.metadata, dsrc.userdata, dsrc._metadata_imds] dsrc.metadata = 'md' dsrc.userdata = 'ud' @@ -1970,7 +1991,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase): """The _should_reprovision method should return true with config flag present.""" isfile.return_value = False - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) self.assertTrue(dsa._should_reprovision( (None, None, {'PreprovisionedVm': True}, None))) @@ -1978,7 +1999,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase): """The _should_reprovision method should return True if the sentinal exists.""" isfile.return_value = True - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) self.assertTrue(dsa._should_reprovision( (None, None, {'preprovisionedvm': False}, None))) @@ -1986,7 +2007,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase): """The _should_reprovision method should return False if config and sentinal are not present.""" isfile.return_value = False - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) self.assertFalse(dsa._should_reprovision((None, None, {}, None))) @mock.patch(MOCKPATH + 'DataSourceAzure._poll_imds') @@ -1997,7 +2018,7 @@ class TestPreprovisioningShouldReprovision(CiTestCase): username = "myuser" odata = {'HostName': hostname, 'UserName': username} _poll_imds.return_value = construct_valid_ovf_env(data=odata) - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) dsa._reprovision() _poll_imds.assert_called_with() @@ -2051,7 +2072,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): fake_resp.side_effect = fake_timeout_once - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file): dsa._poll_imds() self.assertEqual(report_ready_func.call_count, 1) @@ -2071,7 +2092,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): 'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0', 'unknown-245': '624c3620'}] m_media_switch.return_value = None - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) with mock.patch(MOCKPATH + 'REPORTED_READY_MARKER_FILE', report_file): dsa._poll_imds() self.assertEqual(report_ready_func.call_count, 0) @@ -2109,7 +2130,7 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): full_url = url.format(host) fake_resp.return_value = mock.MagicMock(status_code=200, text="ovf", content="ovf") - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) self.assertTrue(len(dsa._poll_imds()) > 0) self.assertEqual(fake_resp.call_args_list, [mock.call(allow_redirects=True, @@ -2146,7 +2167,7 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): content = construct_valid_ovf_env(data=odata) fake_resp.return_value = mock.MagicMock(status_code=200, text=content, content=content) - dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) + dsa = dsaz.DataSourceAzure({}, distro=mock.Mock(), paths=self.paths) md, _ud, cfg, _d = dsa._reprovision() self.assertEqual(md['local-hostname'], hostname) self.assertEqual(cfg['system_info']['default_user']['name'], username) -- cgit v1.2.3 From 3c432b32de1bdce2699525201396a8bbc6a41f3e Mon Sep 17 00:00:00 2001 From: Lukas Märdian Date: Thu, 29 Oct 2020 14:38:56 +0100 Subject: get_interfaces: don't exclude Open vSwitch bridge/bond members (#608) If an OVS bridge was used as the only/primary interface, the 'init' stage failed with a "Not all expected physical devices present" error, leaving the system with a broken SSH setup. LP: #1898997 --- cloudinit/net/__init__.py | 15 +++++++++++++-- cloudinit/net/tests/test_init.py | 36 +++++++++++++++++++++++++++++++++++- tools/.github-cla-signers | 1 + 3 files changed, 49 insertions(+), 3 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 75e79ca8..de65e7af 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -124,6 +124,15 @@ def master_is_bridge_or_bond(devname): return (os.path.exists(bonding_path) or os.path.exists(bridge_path)) +def master_is_openvswitch(devname): + """Return a bool indicating if devname's master is openvswitch""" + master_path = get_master(devname) + if master_path is None: + return False + ovs_path = sys_dev_path(devname, path="upper_ovs-system") + return os.path.exists(ovs_path) + + def is_netfailover(devname, driver=None): """ netfailover driver uses 3 nics, master, primary and standby. this returns True if the device is either the primary or standby @@ -862,8 +871,10 @@ def get_interfaces(blacklist_drivers=None) -> list: continue if is_bond(name): continue - if get_master(name) is not None and not master_is_bridge_or_bond(name): - continue + if get_master(name) is not None: + if (not master_is_bridge_or_bond(name) and + not master_is_openvswitch(name)): + continue if is_netfailover(name): continue mac = get_interface_mac(name) diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index 311ab6f8..0535387a 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -190,6 +190,28 @@ class TestReadSysNet(CiTestCase): self.assertTrue(net.master_is_bridge_or_bond('eth1')) self.assertTrue(net.master_is_bridge_or_bond('eth2')) + def test_master_is_openvswitch(self): + ovs_mac = 'bb:cc:aa:bb:cc:aa' + + # No master => False + write_file(os.path.join(self.sysdir, 'eth1', 'address'), ovs_mac) + + self.assertFalse(net.master_is_bridge_or_bond('eth1')) + + # masters without ovs-system => False + write_file(os.path.join(self.sysdir, 'ovs-system', 'address'), ovs_mac) + + os.symlink('../ovs-system', os.path.join(self.sysdir, 'eth1', + 'master')) + + self.assertFalse(net.master_is_openvswitch('eth1')) + + # masters with ovs-system => True + os.symlink('../ovs-system', os.path.join(self.sysdir, 'eth1', + 'upper_ovs-system')) + + self.assertTrue(net.master_is_openvswitch('eth1')) + def test_is_vlan(self): """is_vlan is True when /sys/net/devname/uevent has DEVTYPE=vlan.""" ensure_file(os.path.join(self.sysdir, 'eth0', 'uevent')) @@ -465,20 +487,32 @@ class TestGetInterfaceMAC(CiTestCase): ): bridge_mac = 'aa:bb:cc:aa:bb:cc' bond_mac = 'cc:bb:aa:cc:bb:aa' + ovs_mac = 'bb:cc:aa:bb:cc:aa' + write_file(os.path.join(self.sysdir, 'br0', 'address'), bridge_mac) write_file(os.path.join(self.sysdir, 'br0', 'bridge'), '') write_file(os.path.join(self.sysdir, 'bond0', 'address'), bond_mac) write_file(os.path.join(self.sysdir, 'bond0', 'bonding'), '') + write_file(os.path.join(self.sysdir, 'ovs-system', 'address'), + ovs_mac) + write_file(os.path.join(self.sysdir, 'eth1', 'address'), bridge_mac) os.symlink('../br0', os.path.join(self.sysdir, 'eth1', 'master')) write_file(os.path.join(self.sysdir, 'eth2', 'address'), bond_mac) os.symlink('../bond0', os.path.join(self.sysdir, 'eth2', 'master')) + write_file(os.path.join(self.sysdir, 'eth3', 'address'), ovs_mac) + os.symlink('../ovs-system', os.path.join(self.sysdir, 'eth3', + 'master')) + os.symlink('../ovs-system', os.path.join(self.sysdir, 'eth3', + 'upper_ovs-system')) + interface_names = [interface[0] for interface in net.get_interfaces()] - self.assertEqual(['eth1', 'eth2'], sorted(interface_names)) + self.assertEqual(['eth1', 'eth2', 'eth3', 'ovs-system'], + sorted(interface_names)) class TestInterfaceHasOwnMAC(CiTestCase): diff --git a/tools/.github-cla-signers b/tools/.github-cla-signers index 475f0872..2d81b700 100644 --- a/tools/.github-cla-signers +++ b/tools/.github-cla-signers @@ -20,6 +20,7 @@ matthewruffell nishigori omBratteng onitake +slyon smoser sshedi TheRealFalcon -- cgit v1.2.3 From b46e4a8cff667c8441622089cf7d57aeb88220cd Mon Sep 17 00:00:00 2001 From: Eduardo Otubo Date: Thu, 29 Oct 2020 15:05:42 +0100 Subject: Explicit set IPV6_AUTOCONF and IPV6_FORCE_ACCEPT_RA on static6 (#634) The static and static6 subnet types for network_data.json were being ignored by the Openstack handler, this would cause the code to break and not function properly. As of today, if a static6 configuration is chosen, the interface will still eventually be available to receive router advertisements or be set from NetworkManager to wait for them and cycle the interface in negative case. It is safe to assume that if the interface is manually configured to use static ipv6 address, there's no need to wait for router advertisements. This patch will set automatically IPV6_AUTOCONF and IPV6_FORCE_ACCEPT_RA both to "no" in this case. This patch fixes the specific behavior only for RHEL flavor and sysconfig renderer. It also introduces new unit tests for the specific case as well as adjusts some existent tests to be compatible with the new options. This patch also addresses this problem by assigning the appropriate subnet type for each case on the openstack handler. rhbz: #1889635 rhbz: #1889635 Signed-off-by: Eduardo Otubo otubo@redhat.com --- cloudinit/net/network_state.py | 3 +- cloudinit/net/sysconfig.py | 4 ++ cloudinit/sources/helpers/openstack.py | 8 ++- tests/unittests/test_distros/test_netconfig.py | 2 + tests/unittests/test_net.py | 98 ++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 2 deletions(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index b2f7d31e..d9e7fd58 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -820,7 +820,8 @@ def _normalize_subnet(subnet): if subnet.get('type') in ('static', 'static6'): normal_subnet.update( - _normalize_net_keys(normal_subnet, address_keys=('address',))) + _normalize_net_keys(normal_subnet, address_keys=( + 'address', 'ip_address',))) normal_subnet['routes'] = [_normalize_route(r) for r in subnet.get('routes', [])] diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index e9337b12..b0eecc44 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -463,6 +463,10 @@ class Renderer(renderer.Renderer): iface_cfg[mtu_key] = subnet['mtu'] else: iface_cfg[mtu_key] = subnet['mtu'] + + if subnet_is_ipv6(subnet) and flavor == 'rhel': + iface_cfg['IPV6_FORCE_ACCEPT_RA'] = False + iface_cfg['IPV6_AUTOCONF'] = False elif subnet_type == 'manual': if flavor == 'suse': LOG.debug('Unknown subnet type setting "%s"', subnet_type) diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index 65e020c5..3e6365f1 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -602,11 +602,17 @@ def convert_net_json(network_json=None, known_macs=None): elif network['type'] in ['ipv6_slaac', 'ipv6_dhcpv6-stateless', 'ipv6_dhcpv6-stateful']: subnet.update({'type': network['type']}) - elif network['type'] in ['ipv4', 'ipv6']: + elif network['type'] in ['ipv4', 'static']: subnet.update({ 'type': 'static', 'address': network.get('ip_address'), }) + elif network['type'] in ['ipv6', 'static6']: + cfg.update({'accept-ra': False}) + subnet.update({ + 'type': 'static6', + 'address': network.get('ip_address'), + }) # Enable accept_ra for stateful and legacy ipv6_dhcp types if network['type'] in ['ipv6_dhcpv6-stateful', 'ipv6_dhcp']: diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index 3f3fe3eb..a1df066a 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -514,7 +514,9 @@ class TestNetCfgDistroRedhat(TestNetCfgDistroBase): DEVICE=eth0 IPV6ADDR=2607:f0d0:1002:0011::2/64 IPV6INIT=yes + IPV6_AUTOCONF=no IPV6_DEFAULTGW=2607:f0d0:1002:0011::1 + IPV6_FORCE_ACCEPT_RA=no NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 642e60cc..5af82e20 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -752,7 +752,9 @@ IPADDR=172.19.1.34 IPV6ADDR=2001:DB8::10/64 IPV6ADDR_SECONDARIES="2001:DB9::10/64 2001:DB10::10/64" IPV6INIT=yes +IPV6_AUTOCONF=no IPV6_DEFAULTGW=2001:DB8::1 +IPV6_FORCE_ACCEPT_RA=no NETMASK=255.255.252.0 NM_CONTROLLED=no ONBOOT=yes @@ -1027,6 +1029,8 @@ NETWORK_CONFIGS = { IPADDR=192.168.14.2 IPV6ADDR=2001:1::1/64 IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no NETMASK=255.255.255.0 NM_CONTROLLED=no ONBOOT=yes @@ -1253,6 +1257,33 @@ NETWORK_CONFIGS = { """), }, }, + 'static6': { + 'yaml': textwrap.dedent("""\ + version: 1 + config: + - type: 'physical' + name: 'iface0' + accept-ra: 'no' + subnets: + - type: 'static6' + address: 2001:1::1/64 + """).rstrip(' '), + 'expected_sysconfig_rhel': { + 'ifcfg-iface0': textwrap.dedent("""\ + BOOTPROTO=none + DEVICE=iface0 + IPV6ADDR=2001:1::1/64 + IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no + DEVICE=iface0 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """), + }, + }, 'dhcpv6_stateless': { 'expected_eni': textwrap.dedent("""\ auto lo @@ -1643,6 +1674,8 @@ pre-down route del -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true IPADDR=192.168.14.2 IPV6ADDR=2001:1::1/64 IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no IPV6_DEFAULTGW=2001:4800:78ff:1b::1 MACADDR=bb:bb:bb:bb:bb:aa NETMASK=255.255.255.0 @@ -2172,6 +2205,8 @@ iface bond0 inet6 static IPADDR1=192.168.1.2 IPV6ADDR=2001:1::1/92 IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no MTU=9000 NETMASK=255.255.255.0 NETMASK1=255.255.255.0 @@ -2277,6 +2312,8 @@ iface bond0 inet6 static IPADDR1=192.168.1.2 IPV6ADDR=2001:1::bbbb/96 IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no IPV6_DEFAULTGW=2001:1::1 MTU=2222 NETMASK=255.255.255.0 @@ -2360,6 +2397,8 @@ iface bond0 inet6 static HWADDR=52:54:00:12:34:00 IPV6ADDR=2001:1::100/96 IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet @@ -2372,6 +2411,8 @@ iface bond0 inet6 static HWADDR=52:54:00:12:34:01 IPV6ADDR=2001:1::101/96 IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no NM_CONTROLLED=no ONBOOT=yes TYPE=Ethernet @@ -3178,6 +3219,61 @@ USERCTL=no self._compare_files_to_expected(entry[self.expected_name], found) self._assert_headers(found) + def test_stattic6_from_json(self): + net_json = { + "services": [{"type": "dns", "address": "172.19.0.12"}], + "networks": [{ + "network_id": "dacd568d-5be6-4786-91fe-750c374b78b4", + "type": "ipv4", "netmask": "255.255.252.0", + "link": "tap1a81968a-79", + "routes": [{ + "netmask": "0.0.0.0", + "network": "0.0.0.0", + "gateway": "172.19.3.254", + }, { + "netmask": "0.0.0.0", # A second default gateway + "network": "0.0.0.0", + "gateway": "172.20.3.254", + }], + "ip_address": "172.19.1.34", "id": "network0" + }, { + "network_id": "mgmt", + "netmask": "ffff:ffff:ffff:ffff::", + "link": "interface1", + "mode": "link-local", + "routes": [], + "ip_address": "fe80::c096:67ff:fe5c:6e84", + "type": "static6", + "id": "network1", + "services": [], + "accept-ra": "false" + }], + "links": [ + { + "ethernet_mac_address": "fa:16:3e:ed:9a:59", + "mtu": None, "type": "bridge", "id": + "tap1a81968a-79", + "vif_id": "1a81968a-797a-400f-8a80-567f997eb93f" + }, + ], + } + macs = {'fa:16:3e:ed:9a:59': 'eth0'} + render_dir = self.tmp_dir() + network_cfg = openstack.convert_net_json(net_json, known_macs=macs) + ns = network_state.parse_net_config_data(network_cfg, + skip_broken=False) + renderer = self._get_renderer() + with self.assertRaises(ValueError): + renderer.render_network_state(ns, target=render_dir) + self.assertEqual([], os.listdir(render_dir)) + + def test_static6_from_yaml(self): + entry = NETWORK_CONFIGS['static6'] + found = self._render_and_read(network_config=yaml.load( + entry['yaml'])) + self._compare_files_to_expected(entry[self.expected_name], found) + self._assert_headers(found) + def test_dhcpv6_reject_ra_config_v2(self): entry = NETWORK_CONFIGS['dhcpv6_reject_ra'] found = self._render_and_read(network_config=yaml.load( @@ -3295,6 +3391,8 @@ USERCTL=no IPADDR=192.168.42.100 IPV6ADDR=2001:db8::100/32 IPV6INIT=yes + IPV6_AUTOCONF=no + IPV6_FORCE_ACCEPT_RA=no IPV6_DEFAULTGW=2001:db8::1 NETMASK=255.255.255.0 NM_CONTROLLED=no -- cgit v1.2.3 From 57349eb7df1c422d9e9558e54b201c85778997ae Mon Sep 17 00:00:00 2001 From: dermotbradley Date: Mon, 9 Nov 2020 17:24:55 +0000 Subject: Make wakeonlan Network Config v2 setting actually work (#626) Add code so that specifying "wakeonlan: true" actually results in relevant configuration entry appearing in /etc/network/interfaces, Netplan, and sysconfig for RHEL and OpenSuse. Add testcases for the above. --- cloudinit/net/eni.py | 4 ++ cloudinit/net/network_state.py | 6 +- cloudinit/net/sysconfig.py | 5 ++ tests/unittests/test_net.py | 143 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 157 insertions(+), 1 deletion(-) (limited to 'cloudinit/net') diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py index 13c041f3..0074691b 100644 --- a/cloudinit/net/eni.py +++ b/cloudinit/net/eni.py @@ -401,6 +401,10 @@ class Renderer(renderer.Renderer): sections = [] subnets = iface.get('subnets', {}) accept_ra = iface.pop('accept-ra', None) + ethernet_wol = iface.pop('wakeonlan', None) + if ethernet_wol: + # Specify WOL setting 'g' for using "Magic Packet" + iface['ethernet-wol'] = 'g' if subnets: for index, subnet in enumerate(subnets): ipv4_subnet_mtu = None diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index d9e7fd58..e8bf9e39 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -369,6 +369,9 @@ class NetworkStateInterpreter(metaclass=CommandHandlerMeta): accept_ra = command.get('accept-ra', None) if accept_ra is not None: accept_ra = util.is_true(accept_ra) + wakeonlan = command.get('wakeonlan', None) + if wakeonlan is not None: + wakeonlan = util.is_true(wakeonlan) iface.update({ 'name': command.get('name'), 'type': command.get('type'), @@ -379,7 +382,8 @@ class NetworkStateInterpreter(metaclass=CommandHandlerMeta): 'address': None, 'gateway': None, 'subnets': subnets, - 'accept-ra': accept_ra + 'accept-ra': accept_ra, + 'wakeonlan': wakeonlan, }) self._network_state['interfaces'].update({command.get('name'): iface}) self.dump_network_state() diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index b0eecc44..a930e612 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -367,6 +367,11 @@ class Renderer(renderer.Renderer): if new_key: iface_cfg[new_key] = old_value + # only set WakeOnLan for physical interfaces + if ('wakeonlan' in iface and iface['wakeonlan'] and + iface['type'] == 'physical'): + iface_cfg['ETHTOOL_OPTS'] = 'wol g' + @classmethod def _render_subnets(cls, iface_cfg, subnets, has_default_route, flavor): # setting base values diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 5af82e20..70453683 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -1378,6 +1378,89 @@ NETWORK_CONFIGS = { """), }, }, + 'wakeonlan_disabled': { + 'expected_eni': textwrap.dedent("""\ + auto lo + iface lo inet loopback + + auto iface0 + iface iface0 inet dhcp + """).rstrip(' '), + 'expected_netplan': textwrap.dedent(""" + network: + ethernets: + iface0: + dhcp4: true + wakeonlan: false + version: 2 + """), + 'expected_sysconfig_opensuse': { + 'ifcfg-iface0': textwrap.dedent("""\ + BOOTPROTO=dhcp4 + STARTMODE=auto + """), + }, + 'expected_sysconfig_rhel': { + 'ifcfg-iface0': textwrap.dedent("""\ + BOOTPROTO=dhcp + DEVICE=iface0 + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """), + }, + 'yaml_v2': textwrap.dedent("""\ + version: 2 + ethernets: + iface0: + dhcp4: true + wakeonlan: false + """).rstrip(' '), + }, + 'wakeonlan_enabled': { + 'expected_eni': textwrap.dedent("""\ + auto lo + iface lo inet loopback + + auto iface0 + iface iface0 inet dhcp + ethernet-wol g + """).rstrip(' '), + 'expected_netplan': textwrap.dedent(""" + network: + ethernets: + iface0: + dhcp4: true + wakeonlan: true + version: 2 + """), + 'expected_sysconfig_opensuse': { + 'ifcfg-iface0': textwrap.dedent("""\ + BOOTPROTO=dhcp4 + ETHTOOL_OPTS="wol g" + STARTMODE=auto + """), + }, + 'expected_sysconfig_rhel': { + 'ifcfg-iface0': textwrap.dedent("""\ + BOOTPROTO=dhcp + DEVICE=iface0 + ETHTOOL_OPTS="wol g" + NM_CONTROLLED=no + ONBOOT=yes + TYPE=Ethernet + USERCTL=no + """), + }, + 'yaml_v2': textwrap.dedent("""\ + version: 2 + ethernets: + iface0: + dhcp4: true + wakeonlan: true + """).rstrip(' '), + }, 'all': { 'expected_eni': ("""\ auto lo @@ -3293,6 +3376,20 @@ USERCTL=no self._compare_files_to_expected(entry[self.expected_name], found) self._assert_headers(found) + def test_wakeonlan_disabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_disabled'] + found = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self._compare_files_to_expected(entry[self.expected_name], found) + self._assert_headers(found) + + def test_wakeonlan_enabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_enabled'] + found = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self._compare_files_to_expected(entry[self.expected_name], found) + self._assert_headers(found) + def test_check_ifcfg_rh(self): """ifcfg-rh plugin is added NetworkManager.conf if conf present.""" render_dir = self.tmp_dir() @@ -3829,6 +3926,20 @@ STARTMODE=auto self._compare_files_to_expected(entry[self.expected_name], found) self._assert_headers(found) + def test_wakeonlan_disabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_disabled'] + found = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self._compare_files_to_expected(entry[self.expected_name], found) + self._assert_headers(found) + + def test_wakeonlan_enabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_enabled'] + found = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self._compare_files_to_expected(entry[self.expected_name], found) + self._assert_headers(found) + def test_render_v4_and_v6(self): entry = NETWORK_CONFIGS['v4_and_v6'] found = self._render_and_read(network_config=yaml.load(entry['yaml'])) @@ -4478,6 +4589,22 @@ class TestNetplanRoundTrip(CiTestCase): entry['expected_netplan'].splitlines(), files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + def testsimple_wakeonlan_disabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_disabled'] + files = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self.assertEqual( + entry['expected_netplan'].splitlines(), + files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + + def testsimple_wakeonlan_enabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_enabled'] + files = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self.assertEqual( + entry['expected_netplan'].splitlines(), + files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + def testsimple_render_all(self): entry = NETWORK_CONFIGS['all'] files = self._render_and_read(network_config=yaml.load(entry['yaml'])) @@ -4645,6 +4772,22 @@ class TestEniRoundTrip(CiTestCase): entry['expected_eni'].splitlines(), files['/etc/network/interfaces'].splitlines()) + def testsimple_wakeonlan_disabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_disabled'] + files = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self.assertEqual( + entry['expected_eni'].splitlines(), + files['/etc/network/interfaces'].splitlines()) + + def testsimple_wakeonlan_enabled_config_v2(self): + entry = NETWORK_CONFIGS['wakeonlan_enabled'] + files = self._render_and_read(network_config=yaml.load( + entry['yaml_v2'])) + self.assertEqual( + entry['expected_eni'].splitlines(), + files['/etc/network/interfaces'].splitlines()) + def testsimple_render_manual(self): """Test rendering of 'manual' for 'type' and 'control'. -- cgit v1.2.3