From 496aaa947ec563bd02b3148f220ff0afe1b32abb Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 26 Jul 2019 20:40:18 +0000 Subject: net/cmdline: split interfaces_by_mac and init network config determination Previously "cmdline" network configuration could be either user-specified network-config=... configuration data, or initramfs-provided configuration data. Before data sources could modify the order in which network config sources were considered, this conflation didn't matter (and, indeed, in the default data source configuration it will continue to not matter). However, it _is_ desirable for a data source to be able to specify that its network configuration should be preferred over the initramfs-provided network configuration but still allow explicit network-config=... configuration passed to the kernel cmdline to continue to override both of those sources. (This also modifies the Oracle data source to use read_initramfs_config directly, which is effectively what it was using read_kernel_cmdline_config for previously.) --- cloudinit/sources/DataSourceOracle.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'cloudinit/sources/DataSourceOracle.py') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 70b9c58a..76cfa38c 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -48,7 +48,7 @@ class DataSourceOracle(sources.DataSource): return False # network may be configured if iscsi root. If that is the case - # then read_kernel_cmdline_config will return non-None. + # then read_initramfs_config will return non-None. if _is_iscsi_root(): data = self.crawl_metadata() else: @@ -118,10 +118,8 @@ class DataSourceOracle(sources.DataSource): We nonetheless return cmdline provided config if present and fallback to generate fallback.""" if self._network_config == sources.UNSET: - cmdline_cfg = cmdline.read_kernel_cmdline_config() - if cmdline_cfg: - self._network_config = cmdline_cfg - else: + self._network_config = cmdline.read_initramfs_config() + if not self._network_config: self._network_config = self.distro.generate_fallback_config() return self._network_config @@ -137,7 +135,7 @@ def _is_platform_viable(): def _is_iscsi_root(): - return bool(cmdline.read_kernel_cmdline_config()) + return bool(cmdline.read_initramfs_config()) def _load_index(content): -- cgit v1.2.3 From 0e79a1b89287358a77fe31fb82c4bcd83ff48894 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Wed, 14 Aug 2019 20:44:50 +0000 Subject: DataSourceOracle: configure secondary NICs on Virtual Machines Oracle Cloud Infrastructure's Instance Metadata Service provides network configuration information for non-primary NICs. This commit introduces support, on Virtual Machines[0], for fetching that network metadata, converting it to v1 network-config[1] and combining it into the network configuration generated for the primary interface. By default, this behaviour is not enabled. Configuring the Oracle datasource to `configure_secondary_nics` enables it: datasource: Oracle: configure_secondary_nics: true Failures to fetch and generate secondary NIC configuration will log a warning, but otherwise will not affect boot. [0] The expected use of the IMDS-provided network configuration is substantially different on Bare Metal Machines, so support for that will be addressed separately. [1] This is v1 config, because cloudinit.net.cmdline generates v1 config and we need to integrate the secondary NICs into that configuration. --- cloudinit/sources/DataSourceOracle.py | 89 +++++++++++++- cloudinit/sources/tests/test_oracle.py | 214 ++++++++++++++++++++++++++++++++- doc/rtd/topics/datasources/oracle.rst | 25 +++- 3 files changed, 324 insertions(+), 4 deletions(-) (limited to 'cloudinit/sources/DataSourceOracle.py') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 76cfa38c..086af799 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -16,7 +16,7 @@ Notes: """ from cloudinit.url_helper import combine_url, readurl, UrlError -from cloudinit.net import dhcp +from cloudinit.net import dhcp, get_interfaces_by_mac from cloudinit import net from cloudinit import sources from cloudinit import util @@ -28,8 +28,80 @@ import re LOG = logging.getLogger(__name__) +BUILTIN_DS_CONFIG = { + # Don't use IMDS to configure secondary NICs by default + 'configure_secondary_nics': False, +} CHASSIS_ASSET_TAG = "OracleCloud.com" METADATA_ENDPOINT = "http://169.254.169.254/openstack/" +VNIC_METADATA_URL = 'http://169.254.169.254/opc/v1/vnics/' +# https://docs.cloud.oracle.com/iaas/Content/Network/Troubleshoot/connectionhang.htm#Overview, +# indicates that an MTU of 9000 is used within OCI +MTU = 9000 + + +def _add_network_config_from_opc_imds(network_config): + """ + Fetch data from Oracle's IMDS, generate secondary NIC config, merge it. + + The primary NIC configuration should not be modified based on the IMDS + values, as it should continue to be configured for DHCP. As such, this + takes an existing network_config dict which is expected to have the primary + NIC configuration already present. It will mutate the given dict to + include the secondary VNICs. + + :param network_config: + A v1 network config dict with the primary NIC already configured. This + dict will be mutated. + + :raises: + Exceptions are not handled within this function. Likely exceptions are + those raised by url_helper.readurl (if communicating with the IMDS + fails), ValueError/JSONDecodeError (if the IMDS returns invalid JSON), + and KeyError/IndexError (if the IMDS returns valid JSON with unexpected + contents). + """ + resp = readurl(VNIC_METADATA_URL) + vnics = json.loads(str(resp)) + + if 'nicIndex' in vnics[0]: + # TODO: Once configure_secondary_nics defaults to True, lower the level + # of this log message. (Currently, if we're running this code at all, + # someone has explicitly opted-in to secondary VNIC configuration, so + # we should warn them that it didn't happen. Once it's default, this + # would be emitted on every Bare Metal Machine launch, which means INFO + # or DEBUG would be more appropriate.) + LOG.warning( + 'VNIC metadata indicates this is a bare metal machine; skipping' + ' secondary VNIC configuration.' + ) + return + + interfaces_by_mac = get_interfaces_by_mac() + + for vnic_dict in vnics[1:]: + # We skip the first entry in the response because the primary interface + # is already configured by iSCSI boot; applying configuration from the + # IMDS is not required. + mac_address = vnic_dict['macAddr'].lower() + if mac_address not in interfaces_by_mac: + LOG.debug('Interface with MAC %s not found; skipping', mac_address) + continue + name = interfaces_by_mac[mac_address] + subnet = { + 'type': 'static', + 'address': vnic_dict['privateIp'], + 'netmask': vnic_dict['subnetCidrBlock'].split('/')[1], + 'gateway': vnic_dict['virtualRouterIp'], + 'control': 'manual', + } + network_config['config'].append({ + 'name': name, + 'type': 'physical', + 'mac_address': mac_address, + 'mtu': MTU, + 'subnets': [subnet], + }) class DataSourceOracle(sources.DataSource): @@ -39,6 +111,13 @@ class DataSourceOracle(sources.DataSource): vendordata_pure = None _network_config = sources.UNSET + def __init__(self, sys_cfg, *args, **kwargs): + super(DataSourceOracle, self).__init__(sys_cfg, *args, **kwargs) + + self.ds_cfg = util.mergemanydict([ + util.get_cfg_by_path(sys_cfg, ['datasource', self.dsname], {}), + BUILTIN_DS_CONFIG]) + def _is_platform_viable(self): """Check platform environment to report if this datasource may run.""" return _is_platform_viable() @@ -121,6 +200,14 @@ class DataSourceOracle(sources.DataSource): self._network_config = cmdline.read_initramfs_config() if not self._network_config: self._network_config = self.distro.generate_fallback_config() + if self.ds_cfg.get('configure_secondary_nics'): + try: + # Mutate self._network_config to include secondary VNICs + _add_network_config_from_opc_imds(self._network_config) + except Exception: + util.logexc( + LOG, + "Failed to fetch secondary network configuration!") return self._network_config diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 282382c5..3e146776 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -18,10 +18,52 @@ import uuid DS_PATH = "cloudinit.sources.DataSourceOracle" MD_VER = "2013-10-17" +# `curl -L http://169.254.169.254/opc/v1/vnics/` on a Oracle Bare Metal Machine +# with a secondary VNIC attached (vnicId truncated for Python line length) +OPC_BM_SECONDARY_VNIC_RESPONSE = """\ +[ { + "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtyvcucqkhdqmgjszebxe4hrb!!TRUNCATED||", + "privateIp" : "10.0.0.8", + "vlanTag" : 0, + "macAddr" : "90:e2:ba:d4:f1:68", + "virtualRouterIp" : "10.0.0.1", + "subnetCidrBlock" : "10.0.0.0/24", + "nicIndex" : 0 +}, { + "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtfmkxjdy2sqidndiwrsg63zf!!TRUNCATED||", + "privateIp" : "10.0.4.5", + "vlanTag" : 1, + "macAddr" : "02:00:17:05:CF:51", + "virtualRouterIp" : "10.0.4.1", + "subnetCidrBlock" : "10.0.4.0/24", + "nicIndex" : 0 +} ]""" + +# `curl -L http://169.254.169.254/opc/v1/vnics/` on a Oracle Virtual Machine +# with a secondary VNIC attached +OPC_VM_SECONDARY_VNIC_RESPONSE = """\ +[ { + "vnicId" : "ocid1.vnic.oc1.phx.abyhqljtch72z5pd76cc2636qeqh7z_truncated", + "privateIp" : "10.0.0.230", + "vlanTag" : 1039, + "macAddr" : "02:00:17:05:D1:DB", + "virtualRouterIp" : "10.0.0.1", + "subnetCidrBlock" : "10.0.0.0/24" +}, { + "vnicId" : "ocid1.vnic.oc1.phx.abyhqljt4iew3gwmvrwrhhf3bp5drj_truncated", + "privateIp" : "10.0.0.231", + "vlanTag" : 1041, + "macAddr" : "00:00:17:02:2B:B1", + "virtualRouterIp" : "10.0.0.1", + "subnetCidrBlock" : "10.0.0.0/24" +} ]""" + class TestDataSourceOracle(test_helpers.CiTestCase): """Test datasource DataSourceOracle.""" + with_logs = True + ds_class = oracle.DataSourceOracle my_uuid = str(uuid.uuid4()) @@ -79,6 +121,16 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertEqual( 'metadata (http://169.254.169.254/openstack/)', ds.subplatform) + def test_sys_cfg_can_enable_configure_secondary_nics(self): + # Confirm that behaviour is toggled by sys_cfg + ds, _mocks = self._get_ds() + self.assertFalse(ds.ds_cfg['configure_secondary_nics']) + + sys_cfg = { + 'datasource': {'Oracle': {'configure_secondary_nics': True}}} + ds, _mocks = self._get_ds(sys_cfg=sys_cfg) + self.assertTrue(ds.ds_cfg['configure_secondary_nics']) + @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) def test_without_userdata(self, m_is_iscsi_root): """If no user-data is provided, it should not be in return dict.""" @@ -133,9 +185,12 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertEqual(self.my_md['uuid'], ds.get_instance_id()) self.assertEqual(my_userdata, ds.userdata_raw) + @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", + side_effect=lambda network_config: network_config) @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config): + def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config, + _m_add_network_config_from_opc_imds): """network_config should read kernel cmdline.""" distro = mock.MagicMock() ds, _ = self._get_ds(distro=distro, patches={ @@ -151,9 +206,12 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertEqual([mock.call()], m_initramfs_config.call_args_list) self.assertFalse(distro.generate_fallback_config.called) + @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", + side_effect=lambda network_config: network_config) @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config): + def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config, + _m_add_network_config_from_opc_imds): """test that fallback network is generated if no kernel cmdline.""" distro = mock.MagicMock() ds, _ = self._get_ds(distro=distro, patches={ @@ -175,6 +233,76 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertEqual(ncfg, ds.network_config) self.assertEqual(1, m_initramfs_config.call_count) + @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") + @mock.patch(DS_PATH + ".cmdline.read_initramfs_config", + return_value={'some': 'config'}) + @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) + def test_secondary_nics_added_to_network_config_if_enabled( + self, _m_is_iscsi_root, _m_initramfs_config, + m_add_network_config_from_opc_imds): + + needle = object() + + def network_config_side_effect(network_config): + network_config['secondary_added'] = needle + + m_add_network_config_from_opc_imds.side_effect = ( + network_config_side_effect) + + distro = mock.MagicMock() + ds, _ = self._get_ds(distro=distro, patches={ + '_is_platform_viable': {'return_value': True}, + 'crawl_metadata': { + 'return_value': { + MD_VER: {'system_uuid': self.my_uuid, + 'meta_data': self.my_md}}}}) + ds.ds_cfg['configure_secondary_nics'] = True + self.assertEqual(needle, ds.network_config['secondary_added']) + + @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") + @mock.patch(DS_PATH + ".cmdline.read_initramfs_config", + return_value={'some': 'config'}) + @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) + def test_secondary_nics_not_added_to_network_config_by_default( + self, _m_is_iscsi_root, _m_initramfs_config, + m_add_network_config_from_opc_imds): + + def network_config_side_effect(network_config): + network_config['secondary_added'] = True + + m_add_network_config_from_opc_imds.side_effect = ( + network_config_side_effect) + + distro = mock.MagicMock() + ds, _ = self._get_ds(distro=distro, patches={ + '_is_platform_viable': {'return_value': True}, + 'crawl_metadata': { + 'return_value': { + MD_VER: {'system_uuid': self.my_uuid, + 'meta_data': self.my_md}}}}) + self.assertNotIn('secondary_added', ds.network_config) + + @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") + @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") + @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) + def test_secondary_nic_failure_isnt_blocking( + self, _m_is_iscsi_root, m_initramfs_config, + m_add_network_config_from_opc_imds): + + m_add_network_config_from_opc_imds.side_effect = Exception() + + distro = mock.MagicMock() + ds, _ = self._get_ds(distro=distro, patches={ + '_is_platform_viable': {'return_value': True}, + 'crawl_metadata': { + 'return_value': { + MD_VER: {'system_uuid': self.my_uuid, + 'meta_data': self.my_md}}}}) + ds.ds_cfg['configure_secondary_nics'] = True + self.assertEqual(ds.network_config, m_initramfs_config.return_value) + self.assertIn('Failed to fetch secondary network configuration', + self.logs.getvalue()) + @mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4())) class TestReadMetaData(test_helpers.HttprettyTestCase): @@ -335,4 +463,86 @@ class TestLoadIndex(test_helpers.CiTestCase): oracle._load_index("\n".join(["meta_data.json", "user_data"]))) +class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): + + with_logs = True + + def setUp(self): + super(TestNetworkConfigFromOpcImds, self).setUp() + self.add_patch(DS_PATH + '.readurl', 'm_readurl') + self.add_patch(DS_PATH + '.get_interfaces_by_mac', + 'm_get_interfaces_by_mac') + + def test_failure_to_readurl(self): + # readurl failures should just bubble out to the caller + self.m_readurl.side_effect = Exception('oh no') + with self.assertRaises(Exception) as excinfo: + oracle._add_network_config_from_opc_imds({}) + self.assertEqual(str(excinfo.exception), 'oh no') + + def test_empty_response(self): + # empty response error should just bubble out to the caller + self.m_readurl.return_value = '' + with self.assertRaises(Exception): + oracle._add_network_config_from_opc_imds([]) + + def test_invalid_json(self): + # invalid JSON error should just bubble out to the caller + self.m_readurl.return_value = '{' + with self.assertRaises(Exception): + oracle._add_network_config_from_opc_imds([]) + + def test_no_secondary_nics_does_not_mutate_input(self): + self.m_readurl.return_value = json.dumps([{}]) + # We test this by passing in a non-dict to ensure that no dict + # operations are used; failure would be seen as exceptions + oracle._add_network_config_from_opc_imds(object()) + + def test_bare_metal_machine_skipped(self): + # nicIndex in the first entry indicates a bare metal machine + self.m_readurl.return_value = OPC_BM_SECONDARY_VNIC_RESPONSE + # We test this by passing in a non-dict to ensure that no dict + # operations are used + self.assertFalse(oracle._add_network_config_from_opc_imds(object())) + self.assertIn('bare metal machine', self.logs.getvalue()) + + def test_missing_mac_skipped(self): + self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE + self.m_get_interfaces_by_mac.return_value = {} + + network_config = {'version': 1, 'config': [{'primary': 'nic'}]} + oracle._add_network_config_from_opc_imds(network_config) + + self.assertEqual(1, len(network_config['config'])) + self.assertIn( + 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping', + self.logs.getvalue()) + + def test_secondary_nic(self): + self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE + mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' + self.m_get_interfaces_by_mac.return_value = { + mac_addr: nic_name, + } + + network_config = {'version': 1, 'config': [{'primary': 'nic'}]} + oracle._add_network_config_from_opc_imds(network_config) + + # The input is mutated + self.assertEqual(2, len(network_config['config'])) + + secondary_nic_cfg = network_config['config'][1] + self.assertEqual(nic_name, secondary_nic_cfg['name']) + self.assertEqual('physical', secondary_nic_cfg['type']) + self.assertEqual(mac_addr, secondary_nic_cfg['mac_address']) + self.assertEqual(9000, secondary_nic_cfg['mtu']) + + self.assertEqual(1, len(secondary_nic_cfg['subnets'])) + subnet_cfg = secondary_nic_cfg['subnets'][0] + # These values are hard-coded in OPC_VM_SECONDARY_VNIC_RESPONSE + self.assertEqual('10.0.0.231', subnet_cfg['address']) + self.assertEqual('24', subnet_cfg['netmask']) + self.assertEqual('10.0.0.1', subnet_cfg['gateway']) + self.assertEqual('manual', subnet_cfg['control']) + # vi: ts=4 expandtab diff --git a/doc/rtd/topics/datasources/oracle.rst b/doc/rtd/topics/datasources/oracle.rst index f2383cee..98c4657c 100644 --- a/doc/rtd/topics/datasources/oracle.rst +++ b/doc/rtd/topics/datasources/oracle.rst @@ -8,7 +8,7 @@ This datasource reads metadata, vendor-data and user-data from Oracle Platform --------------- -OCI provides bare metal and virtual machines. In both cases, +OCI provides bare metal and virtual machines. In both cases, the platform identifies itself via DMI data in the chassis asset tag with the string 'OracleCloud.com'. @@ -22,5 +22,28 @@ Cloud-init has a specific datasource for Oracle in order to: implementation. +Configuration +------------- + +The following configuration can be set for the datasource in system +configuration (in ``/etc/cloud/cloud.cfg`` or ``/etc/cloud/cloud.cfg.d/``). + +The settings that may be configured are: + +* **configure_secondary_nics**: A boolean, defaulting to False. If set + to True on an OCI Virtual Machine, cloud-init will fetch networking + metadata from Oracle's IMDS and use it to configure the non-primary + network interface controllers in the system. If set to True on an + OCI Bare Metal Machine, it will have no effect (though this may + change in the future). + +An example configuration with the default values is provided below: + +.. sourcecode:: yaml + + datasource: + Oracle: + configure_secondary_nics: false + .. _Oracle Compute Infrastructure: https://cloud.oracle.com/ .. vi: textwidth=78 -- cgit v1.2.3 From 2c52e6e88b19f5db8d55eb7280ee27703e05d75f Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Mon, 19 Aug 2019 20:52:54 +0000 Subject: DataSourceOracle: prefer DS network config over initramfs The Oracle platform provides networking configuration from two sources: * the primary interface configuration comes from the initramfs, because Oracle instance all iSCSI boot * secondary interface configuration comes from an IMDS accessed over HTTP As we need to combine these two sources of network configuration, the default "prefer initramfs config over data source config" behaviour isn't appropriate; we would never get the IMDS interfaces via that route. Instead, the Oracle data source has code to combine these two sources, so we prefer its network configuration over the initramfs configuration. (This is not appropriate default behaviour, because _in general_ data sources won't know how to merge initramfs-provided configuration into their provided configuration, so switching this order for all data sources would result in initramfs configuration being discarded on any data source that implements network_config.) --- cloudinit/sources/DataSourceOracle.py | 7 +++++++ cloudinit/sources/tests/test_oracle.py | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) (limited to 'cloudinit/sources/DataSourceOracle.py') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 086af799..6e73f568 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -109,6 +109,13 @@ class DataSourceOracle(sources.DataSource): dsname = 'Oracle' system_uuid = None vendordata_pure = None + network_config_sources = ( + sources.NetworkConfigSource.cmdline, + sources.NetworkConfigSource.ds, + sources.NetworkConfigSource.initramfs, + sources.NetworkConfigSource.system_cfg, + ) + _network_config = sources.UNSET def __init__(self, sys_cfg, *args, **kwargs): diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 3e146776..3ddf7dfd 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -1,7 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. from cloudinit.sources import DataSourceOracle as oracle -from cloudinit.sources import BrokenMetadata +from cloudinit.sources import BrokenMetadata, NetworkConfigSource from cloudinit import helpers from cloudinit.tests import helpers as test_helpers @@ -303,6 +303,14 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertIn('Failed to fetch secondary network configuration', self.logs.getvalue()) + def test_ds_network_cfg_preferred_over_initramfs(self): + """Ensure that DS net config is preferred over initramfs config""" + network_config_sources = oracle.DataSourceOracle.network_config_sources + self.assertLess( + network_config_sources.index(NetworkConfigSource.ds), + network_config_sources.index(NetworkConfigSource.initramfs) + ) + @mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4())) class TestReadMetaData(test_helpers.HttprettyTestCase): -- cgit v1.2.3 From e7881d5c62e68e4962e2fcd3d12089e5a3b94dc9 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 28 Aug 2019 16:10:35 +0000 Subject: Oracle: Render secondary vnic IP and MTU values only When rendering secondary vnic configuration from IMDS, only emit configuration for the IP and MTU values only. Add support to mutate either a v1 or a v2 network_config input. --- cloudinit/sources/DataSourceOracle.py | 36 +++++++++++++++++-------------- cloudinit/sources/tests/test_oracle.py | 39 +++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 19 deletions(-) (limited to 'cloudinit/sources/DataSourceOracle.py') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 6e73f568..1cb0636c 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -51,8 +51,8 @@ def _add_network_config_from_opc_imds(network_config): include the secondary VNICs. :param network_config: - A v1 network config dict with the primary NIC already configured. This - dict will be mutated. + A v1 or v2 network config dict with the primary NIC already configured. + This dict will be mutated. :raises: Exceptions are not handled within this function. Likely exceptions are @@ -88,20 +88,24 @@ def _add_network_config_from_opc_imds(network_config): LOG.debug('Interface with MAC %s not found; skipping', mac_address) continue name = interfaces_by_mac[mac_address] - subnet = { - 'type': 'static', - 'address': vnic_dict['privateIp'], - 'netmask': vnic_dict['subnetCidrBlock'].split('/')[1], - 'gateway': vnic_dict['virtualRouterIp'], - 'control': 'manual', - } - network_config['config'].append({ - 'name': name, - 'type': 'physical', - 'mac_address': mac_address, - 'mtu': MTU, - 'subnets': [subnet], - }) + + if network_config['version'] == 1: + subnet = { + 'type': 'static', + 'address': vnic_dict['privateIp'], + } + network_config['config'].append({ + 'name': name, + 'type': 'physical', + 'mac_address': mac_address, + 'mtu': MTU, + 'subnets': [subnet], + }) + elif network_config['version'] == 2: + network_config['ethernets'][name] = { + 'addresses': [vnic_dict['privateIp']], + 'mtu': MTU, 'dhcp4': False, 'dhcp6': False, + 'match': {'macaddress': mac_address}} class DataSourceOracle(sources.DataSource): diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 3ddf7dfd..2a70bbc9 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -526,6 +526,18 @@ class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping', self.logs.getvalue()) + def test_missing_mac_skipped_v2(self): + self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE + self.m_get_interfaces_by_mac.return_value = {} + + network_config = {'version': 2, 'ethernets': {'primary': {'nic': {}}}} + oracle._add_network_config_from_opc_imds(network_config) + + self.assertEqual(1, len(network_config['ethernets'])) + self.assertIn( + 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping', + self.logs.getvalue()) + def test_secondary_nic(self): self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' @@ -549,8 +561,29 @@ class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): subnet_cfg = secondary_nic_cfg['subnets'][0] # These values are hard-coded in OPC_VM_SECONDARY_VNIC_RESPONSE self.assertEqual('10.0.0.231', subnet_cfg['address']) - self.assertEqual('24', subnet_cfg['netmask']) - self.assertEqual('10.0.0.1', subnet_cfg['gateway']) - self.assertEqual('manual', subnet_cfg['control']) + + def test_secondary_nic_v2(self): + self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE + mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' + self.m_get_interfaces_by_mac.return_value = { + mac_addr: nic_name, + } + + network_config = {'version': 2, 'ethernets': {'primary': {'nic': {}}}} + oracle._add_network_config_from_opc_imds(network_config) + + # The input is mutated + self.assertEqual(2, len(network_config['ethernets'])) + + secondary_nic_cfg = network_config['ethernets']['ens3'] + self.assertFalse(secondary_nic_cfg['dhcp4']) + self.assertFalse(secondary_nic_cfg['dhcp6']) + self.assertEqual(mac_addr, secondary_nic_cfg['match']['macaddress']) + self.assertEqual(9000, secondary_nic_cfg['mtu']) + + self.assertEqual(1, len(secondary_nic_cfg['addresses'])) + # These values are hard-coded in OPC_VM_SECONDARY_VNIC_RESPONSE + self.assertEqual('10.0.0.231', secondary_nic_cfg['addresses'][0]) + # vi: ts=4 expandtab -- cgit v1.2.3 From fa47d527a03a00319936323f0a857fbecafceaf7 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Mon, 9 Sep 2019 21:13:01 +0000 Subject: net,Oracle: Add support for netfailover detection Add support for detecting netfailover[1] device 3-tuple in networking layer. In the Oracle datasource ensure that if a provided network config, either fallback or provided config includes a netfailover master to remove any MAC address value as this can break under 3-netdev as the other two devices have the same MAC. 1. https://www.kernel.org/doc/html/latest/networking/net_failover.html --- cloudinit/net/__init__.py | 133 +++++++++++++- cloudinit/net/tests/test_init.py | 310 +++++++++++++++++++++++++++++++++ cloudinit/sources/DataSourceOracle.py | 62 ++++++- cloudinit/sources/tests/test_oracle.py | 147 ++++++++++++++++ cloudinit/tests/helpers.py | 8 + 5 files changed, 656 insertions(+), 4 deletions(-) (limited to 'cloudinit/sources/DataSourceOracle.py') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index ea707c09..0eb952fe 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -109,6 +109,123 @@ def is_bond(devname): return os.path.exists(sys_dev_path(devname, "bonding")) +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 + as these devices are to be ignored. + """ + if driver is None: + driver = device_driver(devname) + if is_netfail_primary(devname, driver) or is_netfail_standby(devname, + driver): + return True + return False + + +def get_dev_features(devname): + """ Returns a str from reading /sys/class/net//device/features.""" + features = '' + try: + features = read_sys_net(devname, 'device/features') + except Exception: + pass + return features + + +def has_netfail_standby_feature(devname): + """ Return True if VIRTIO_NET_F_STANDBY bit (62) is set. + + https://github.com/torvalds/linux/blob/ \ + 089cf7f6ecb266b6a4164919a2e69bd2f938374a/ \ + include/uapi/linux/virtio_net.h#L60 + """ + features = get_dev_features(devname) + if not features or len(features) < 64: + return False + return features[62] == "1" + + +def is_netfail_master(devname, driver=None): + """ A device is a "netfail master" device if: + + - The device does NOT have the 'master' sysfs attribute + - The device driver is 'virtio_net' + - The device has the standby feature bit set + + Return True if all of the above is True. + """ + if os.path.exists(sys_dev_path(devname, path='master')): + return False + + if driver is None: + driver = device_driver(devname) + + if driver != "virtio_net": + return False + + if not has_netfail_standby_feature(devname): + return False + + return True + + +def is_netfail_primary(devname, driver=None): + """ A device is a "netfail primary" device if: + + - the device has a 'master' sysfs file + - the device driver is not 'virtio_net' + - the 'master' sysfs file points to device with virtio_net driver + - the 'master' device has the 'standby' feature bit set + + Return True if all of the above is True. + """ + # /sys/class/net//master -> ../../ + master_sysfs_path = sys_dev_path(devname, path='master') + if not os.path.exists(master_sysfs_path): + return False + + if driver is None: + driver = device_driver(devname) + + if driver == "virtio_net": + return False + + master_devname = os.path.basename(os.path.realpath(master_sysfs_path)) + master_driver = device_driver(master_devname) + if master_driver != "virtio_net": + return False + + master_has_standby = has_netfail_standby_feature(master_devname) + if not master_has_standby: + return False + + return True + + +def is_netfail_standby(devname, driver=None): + """ A device is a "netfail standby" device if: + + - The device has a 'master' sysfs attribute + - The device driver is 'virtio_net' + - The device has the standby feature bit set + + Return True if all of the above is True. + """ + if not os.path.exists(sys_dev_path(devname, path='master')): + return False + + if driver is None: + driver = device_driver(devname) + + if driver != "virtio_net": + return False + + if not has_netfail_standby_feature(devname): + return False + + return True + + def is_renamed(devname): """ /* interface name assignment types (sysfs name_assign_type attribute) */ @@ -227,6 +344,9 @@ def find_fallback_nic(blacklist_drivers=None): if is_bond(interface): # skip any bonds continue + if is_netfailover(interface): + # ignore netfailover primary/standby interfaces + continue carrier = read_sys_net_int(interface, 'carrier') if carrier: connected.append(interface) @@ -273,9 +393,14 @@ def generate_fallback_config(blacklist_drivers=None, config_driver=None): if not target_name: # can't read any interfaces addresses (or there are none); give up return None - target_mac = read_sys_net_safe(target_name, 'address') - cfg = {'dhcp4': True, 'set-name': target_name, - 'match': {'macaddress': target_mac.lower()}} + + # netfail cannot use mac for matching, they have duplicate macs + if is_netfail_master(target_name): + match = {'name': target_name} + else: + match = { + 'macaddress': read_sys_net_safe(target_name, 'address').lower()} + cfg = {'dhcp4': True, 'set-name': target_name, 'match': match} if config_driver: driver = device_driver(target_name) if driver: @@ -661,6 +786,8 @@ def get_interfaces(): continue if is_bond(name): continue + if is_netfailover(name): + continue mac = get_interface_mac(name) # some devices may not have a mac (tun0) if not mac: diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index d2e38f00..7259dbe3 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -204,6 +204,10 @@ class TestGenerateFallbackConfig(CiTestCase): self.add_patch('cloudinit.net.util.is_container', 'm_is_container', return_value=False) self.add_patch('cloudinit.net.util.udevadm_settle', 'm_settle') + self.add_patch('cloudinit.net.is_netfailover', 'm_netfail', + return_value=False) + self.add_patch('cloudinit.net.is_netfail_master', 'm_netfail_master', + return_value=False) def test_generate_fallback_finds_connected_eth_with_mac(self): """generate_fallback_config finds any connected device with a mac.""" @@ -268,6 +272,61 @@ class TestGenerateFallbackConfig(CiTestCase): ensure_file(os.path.join(self.sysdir, 'eth0', 'bonding')) self.assertIsNone(net.generate_fallback_config()) + def test_generate_fallback_config_skips_netfail_devs(self): + """gen_fallback_config ignores netfail primary,sby no mac on master.""" + mac = 'aa:bb:cc:aa:bb:cc' # netfailover devs share the same mac + for iface in ['ens3', 'ens3sby', 'enP0s1f3']: + write_file(os.path.join(self.sysdir, iface, 'carrier'), '1') + write_file( + os.path.join(self.sysdir, iface, 'addr_assign_type'), '0') + write_file( + os.path.join(self.sysdir, iface, 'address'), mac) + + def is_netfail(iface, _driver=None): + # ens3 is the master + if iface == 'ens3': + return False + return True + self.m_netfail.side_effect = is_netfail + + def is_netfail_master(iface, _driver=None): + # ens3 is the master + if iface == 'ens3': + return True + return False + self.m_netfail_master.side_effect = is_netfail_master + expected = { + 'ethernets': { + 'ens3': {'dhcp4': True, 'match': {'name': 'ens3'}, + 'set-name': 'ens3'}}, + 'version': 2} + result = net.generate_fallback_config() + self.assertEqual(expected, result) + + +class TestNetFindFallBackNic(CiTestCase): + + with_logs = True + + def setUp(self): + super(TestNetFindFallBackNic, self).setUp() + sys_mock = mock.patch('cloudinit.net.get_sys_class_path') + self.m_sys_path = sys_mock.start() + self.sysdir = self.tmp_dir() + '/' + self.m_sys_path.return_value = self.sysdir + self.addCleanup(sys_mock.stop) + self.add_patch('cloudinit.net.util.is_container', 'm_is_container', + return_value=False) + self.add_patch('cloudinit.net.util.udevadm_settle', 'm_settle') + + def test_generate_fallback_finds_first_connected_eth_with_mac(self): + """find_fallback_nic finds any connected device with a mac.""" + write_file(os.path.join(self.sysdir, 'eth0', 'carrier'), '1') + write_file(os.path.join(self.sysdir, 'eth1', 'carrier'), '1') + mac = 'aa:bb:cc:aa:bb:cc' + write_file(os.path.join(self.sysdir, 'eth1', 'address'), mac) + self.assertEqual('eth1', net.find_fallback_nic()) + class TestGetDeviceList(CiTestCase): @@ -365,6 +424,26 @@ class TestGetInterfaceMAC(CiTestCase): expected = [('eth2', 'aa:bb:cc:aa:bb:cc', None, None)] self.assertEqual(expected, net.get_interfaces()) + @mock.patch('cloudinit.net.is_netfailover') + def test_get_interfaces_by_mac_skips_netfailvoer(self, m_netfail): + """Ignore interfaces if netfailover primary or standby.""" + mac = 'aa:bb:cc:aa:bb:cc' # netfailover devs share the same mac + for iface in ['ens3', 'ens3sby', 'enP0s1f3']: + write_file( + os.path.join(self.sysdir, iface, 'addr_assign_type'), '0') + write_file( + os.path.join(self.sysdir, iface, 'address'), mac) + + def is_netfail(iface, _driver=None): + # ens3 is the master + if iface == 'ens3': + return False + else: + return True + m_netfail.side_effect = is_netfail + expected = [('ens3', mac, None, None)] + self.assertEqual(expected, net.get_interfaces()) + class TestInterfaceHasOwnMAC(CiTestCase): @@ -922,3 +1001,234 @@ class TestWaitForPhysdevs(CiTestCase): self.m_get_iface_mac.return_value = {} net.wait_for_physdevs(netcfg, strict=False) self.assertEqual(5 * len(physdevs), self.m_udev_settle.call_count) + + +class TestNetFailOver(CiTestCase): + + with_logs = True + + def setUp(self): + super(TestNetFailOver, self).setUp() + self.add_patch('cloudinit.net.util', 'm_util') + self.add_patch('cloudinit.net.read_sys_net', 'm_read_sys_net') + self.add_patch('cloudinit.net.device_driver', 'm_device_driver') + + def test_get_dev_features(self): + devname = self.random_string() + features = self.random_string() + self.m_read_sys_net.return_value = features + + self.assertEqual(features, net.get_dev_features(devname)) + self.assertEqual(1, self.m_read_sys_net.call_count) + self.assertEqual(mock.call(devname, 'device/features'), + self.m_read_sys_net.call_args_list[0]) + + def test_get_dev_features_none_returns_empty_string(self): + devname = self.random_string() + self.m_read_sys_net.side_effect = Exception('error') + self.assertEqual('', net.get_dev_features(devname)) + self.assertEqual(1, self.m_read_sys_net.call_count) + self.assertEqual(mock.call(devname, 'device/features'), + self.m_read_sys_net.call_args_list[0]) + + @mock.patch('cloudinit.net.get_dev_features') + def test_has_netfail_standby_feature(self, m_dev_features): + devname = self.random_string() + standby_features = ('0' * 62) + '1' + '0' + m_dev_features.return_value = standby_features + self.assertTrue(net.has_netfail_standby_feature(devname)) + + @mock.patch('cloudinit.net.get_dev_features') + def test_has_netfail_standby_feature_short_is_false(self, m_dev_features): + devname = self.random_string() + standby_features = self.random_string() + m_dev_features.return_value = standby_features + self.assertFalse(net.has_netfail_standby_feature(devname)) + + @mock.patch('cloudinit.net.get_dev_features') + def test_has_netfail_standby_feature_not_present_is_false(self, + m_dev_features): + devname = self.random_string() + standby_features = '0' * 64 + m_dev_features.return_value = standby_features + self.assertFalse(net.has_netfail_standby_feature(devname)) + + @mock.patch('cloudinit.net.get_dev_features') + def test_has_netfail_standby_feature_no_features_is_false(self, + m_dev_features): + devname = self.random_string() + standby_features = None + m_dev_features.return_value = standby_features + self.assertFalse(net.has_netfail_standby_feature(devname)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_master(self, m_exists, m_standby): + devname = self.random_string() + driver = 'virtio_net' + m_exists.return_value = False # no master sysfs attr + m_standby.return_value = True # has standby feature flag + self.assertTrue(net.is_netfail_master(devname, driver)) + + @mock.patch('cloudinit.net.sys_dev_path') + def test_is_netfail_master_checks_master_attr(self, m_sysdev): + devname = self.random_string() + driver = 'virtio_net' + m_sysdev.return_value = self.random_string() + self.assertFalse(net.is_netfail_master(devname, driver)) + self.assertEqual(1, m_sysdev.call_count) + self.assertEqual(mock.call(devname, path='master'), + m_sysdev.call_args_list[0]) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_master_wrong_driver(self, m_exists, m_standby): + devname = self.random_string() + driver = self.random_string() + self.assertFalse(net.is_netfail_master(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_master_has_master_attr(self, m_exists, m_standby): + devname = self.random_string() + driver = 'virtio_net' + m_exists.return_value = True # has master sysfs attr + self.assertFalse(net.is_netfail_master(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_master_no_standby_feat(self, m_exists, m_standby): + devname = self.random_string() + driver = 'virtio_net' + m_exists.return_value = False # no master sysfs attr + m_standby.return_value = False # no standby feature flag + self.assertFalse(net.is_netfail_master(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + @mock.patch('cloudinit.net.sys_dev_path') + def test_is_netfail_primary(self, m_sysdev, m_exists, m_standby): + devname = self.random_string() + driver = self.random_string() # device not virtio_net + master_devname = self.random_string() + m_sysdev.return_value = "%s/%s" % (self.random_string(), + master_devname) + m_exists.return_value = True # has master sysfs attr + self.m_device_driver.return_value = 'virtio_net' # master virtio_net + m_standby.return_value = True # has standby feature flag + self.assertTrue(net.is_netfail_primary(devname, driver)) + self.assertEqual(1, self.m_device_driver.call_count) + self.assertEqual(mock.call(master_devname), + self.m_device_driver.call_args_list[0]) + self.assertEqual(1, m_standby.call_count) + self.assertEqual(mock.call(master_devname), + m_standby.call_args_list[0]) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + @mock.patch('cloudinit.net.sys_dev_path') + def test_is_netfail_primary_wrong_driver(self, m_sysdev, m_exists, + m_standby): + devname = self.random_string() + driver = 'virtio_net' + self.assertFalse(net.is_netfail_primary(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + @mock.patch('cloudinit.net.sys_dev_path') + def test_is_netfail_primary_no_master(self, m_sysdev, m_exists, m_standby): + devname = self.random_string() + driver = self.random_string() # device not virtio_net + m_exists.return_value = False # no master sysfs attr + self.assertFalse(net.is_netfail_primary(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + @mock.patch('cloudinit.net.sys_dev_path') + def test_is_netfail_primary_bad_master(self, m_sysdev, m_exists, + m_standby): + devname = self.random_string() + driver = self.random_string() # device not virtio_net + master_devname = self.random_string() + m_sysdev.return_value = "%s/%s" % (self.random_string(), + master_devname) + m_exists.return_value = True # has master sysfs attr + self.m_device_driver.return_value = 'XXXX' # master not virtio_net + self.assertFalse(net.is_netfail_primary(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + @mock.patch('cloudinit.net.sys_dev_path') + def test_is_netfail_primary_no_standby(self, m_sysdev, m_exists, + m_standby): + devname = self.random_string() + driver = self.random_string() # device not virtio_net + master_devname = self.random_string() + m_sysdev.return_value = "%s/%s" % (self.random_string(), + master_devname) + m_exists.return_value = True # has master sysfs attr + self.m_device_driver.return_value = 'virtio_net' # master virtio_net + m_standby.return_value = False # master has no standby feature flag + self.assertFalse(net.is_netfail_primary(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_standby(self, m_exists, m_standby): + devname = self.random_string() + driver = 'virtio_net' + m_exists.return_value = True # has master sysfs attr + m_standby.return_value = True # has standby feature flag + self.assertTrue(net.is_netfail_standby(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_standby_wrong_driver(self, m_exists, m_standby): + devname = self.random_string() + driver = self.random_string() + self.assertFalse(net.is_netfail_standby(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_standby_no_master(self, m_exists, m_standby): + devname = self.random_string() + driver = 'virtio_net' + m_exists.return_value = False # has master sysfs attr + self.assertFalse(net.is_netfail_standby(devname, driver)) + + @mock.patch('cloudinit.net.has_netfail_standby_feature') + @mock.patch('cloudinit.net.os.path.exists') + def test_is_netfail_standby_no_standby_feature(self, m_exists, m_standby): + devname = self.random_string() + driver = 'virtio_net' + m_exists.return_value = True # has master sysfs attr + m_standby.return_value = False # has standby feature flag + self.assertFalse(net.is_netfail_standby(devname, driver)) + + @mock.patch('cloudinit.net.is_netfail_standby') + @mock.patch('cloudinit.net.is_netfail_primary') + def test_is_netfailover_primary(self, m_primary, m_standby): + devname = self.random_string() + driver = self.random_string() + m_primary.return_value = True + m_standby.return_value = False + self.assertTrue(net.is_netfailover(devname, driver)) + + @mock.patch('cloudinit.net.is_netfail_standby') + @mock.patch('cloudinit.net.is_netfail_primary') + def test_is_netfailover_standby(self, m_primary, m_standby): + devname = self.random_string() + driver = self.random_string() + m_primary.return_value = False + m_standby.return_value = True + self.assertTrue(net.is_netfailover(devname, driver)) + + @mock.patch('cloudinit.net.is_netfail_standby') + @mock.patch('cloudinit.net.is_netfail_primary') + def test_is_netfailover_returns_false(self, m_primary, m_standby): + devname = self.random_string() + driver = self.random_string() + m_primary.return_value = False + m_standby.return_value = False + self.assertFalse(net.is_netfailover(devname, driver)) + +# vi: ts=4 expandtab diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 1cb0636c..eec87403 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -16,7 +16,7 @@ Notes: """ from cloudinit.url_helper import combine_url, readurl, UrlError -from cloudinit.net import dhcp, get_interfaces_by_mac +from cloudinit.net import dhcp, get_interfaces_by_mac, is_netfail_master from cloudinit import net from cloudinit import sources from cloudinit import util @@ -108,6 +108,56 @@ def _add_network_config_from_opc_imds(network_config): 'match': {'macaddress': mac_address}} +def _ensure_netfailover_safe(network_config): + """ + Search network config physical interfaces to see if any of them are + a netfailover master. If found, we prevent matching by MAC as the other + failover devices have the same MAC but need to be ignored. + + Note: we rely on cloudinit.net changes which prevent netfailover devices + from being present in the provided network config. For more details about + netfailover devices, refer to cloudinit.net module. + + :param network_config + A v1 or v2 network config dict with the primary NIC, and possibly + secondary nic configured. This dict will be mutated. + + """ + # ignore anything that's not an actual network-config + if 'version' not in network_config: + return + + if network_config['version'] not in [1, 2]: + LOG.debug('Ignoring unknown network config version: %s', + network_config['version']) + return + + mac_to_name = get_interfaces_by_mac() + if network_config['version'] == 1: + for cfg in [c for c in network_config['config'] if 'type' in c]: + if cfg['type'] == 'physical': + if 'mac_address' in cfg: + mac = cfg['mac_address'] + cur_name = mac_to_name.get(mac) + if not cur_name: + continue + elif is_netfail_master(cur_name): + del cfg['mac_address'] + + elif network_config['version'] == 2: + for _, cfg in network_config.get('ethernets', {}).items(): + if 'match' in cfg: + macaddr = cfg.get('match', {}).get('macaddress') + if macaddr: + cur_name = mac_to_name.get(macaddr) + if not cur_name: + continue + elif is_netfail_master(cur_name): + del cfg['match']['macaddress'] + del cfg['set-name'] + cfg['match']['name'] = cur_name + + class DataSourceOracle(sources.DataSource): dsname = 'Oracle' @@ -208,9 +258,13 @@ class DataSourceOracle(sources.DataSource): We nonetheless return cmdline provided config if present and fallback to generate fallback.""" if self._network_config == sources.UNSET: + # this is v1 self._network_config = cmdline.read_initramfs_config() + if not self._network_config: + # this is now v2 self._network_config = self.distro.generate_fallback_config() + if self.ds_cfg.get('configure_secondary_nics'): try: # Mutate self._network_config to include secondary VNICs @@ -219,6 +273,12 @@ class DataSourceOracle(sources.DataSource): util.logexc( LOG, "Failed to fetch secondary network configuration!") + + # we need to verify that the nic selected is not a netfail over + # device and, if it is a netfail master, then we need to avoid + # emitting any match by mac + _ensure_netfailover_safe(self._network_config) + return self._network_config diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 2a70bbc9..85b6db97 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -8,6 +8,7 @@ from cloudinit.tests import helpers as test_helpers from textwrap import dedent import argparse +import copy import httpretty import json import mock @@ -586,4 +587,150 @@ class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): self.assertEqual('10.0.0.231', secondary_nic_cfg['addresses'][0]) +class TestNetworkConfigFiltersNetFailover(test_helpers.CiTestCase): + + with_logs = True + + def setUp(self): + super(TestNetworkConfigFiltersNetFailover, self).setUp() + self.add_patch(DS_PATH + '.get_interfaces_by_mac', + 'm_get_interfaces_by_mac') + self.add_patch(DS_PATH + '.is_netfail_master', 'm_netfail_master') + + def test_ignore_bogus_network_config(self): + netcfg = {'something': 'here'} + passed_netcfg = copy.copy(netcfg) + oracle._ensure_netfailover_safe(passed_netcfg) + self.assertEqual(netcfg, passed_netcfg) + + def test_ignore_network_config_unknown_versions(self): + netcfg = {'something': 'here', 'version': 3} + passed_netcfg = copy.copy(netcfg) + oracle._ensure_netfailover_safe(passed_netcfg) + self.assertEqual(netcfg, passed_netcfg) + + def test_checks_v1_type_physical_interfaces(self): + mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' + self.m_get_interfaces_by_mac.return_value = { + mac_addr: nic_name, + } + netcfg = {'version': 1, 'config': [ + {'type': 'physical', 'name': nic_name, 'mac_address': mac_addr, + 'subnets': [{'type': 'dhcp4'}]}]} + passed_netcfg = copy.copy(netcfg) + self.m_netfail_master.return_value = False + oracle._ensure_netfailover_safe(passed_netcfg) + self.assertEqual(netcfg, passed_netcfg) + self.assertEqual([mock.call(nic_name)], + self.m_netfail_master.call_args_list) + + def test_checks_v1_skips_non_phys_interfaces(self): + mac_addr, nic_name = '00:00:17:02:2b:b1', 'bond0' + self.m_get_interfaces_by_mac.return_value = { + mac_addr: nic_name, + } + netcfg = {'version': 1, 'config': [ + {'type': 'bond', 'name': nic_name, 'mac_address': mac_addr, + 'subnets': [{'type': 'dhcp4'}]}]} + passed_netcfg = copy.copy(netcfg) + oracle._ensure_netfailover_safe(passed_netcfg) + self.assertEqual(netcfg, passed_netcfg) + self.assertEqual(0, self.m_netfail_master.call_count) + + def test_removes_master_mac_property_v1(self): + nic_master, mac_master = 'ens3', self.random_string() + nic_other, mac_other = 'ens7', self.random_string() + nic_extra, mac_extra = 'enp0s1f2', self.random_string() + self.m_get_interfaces_by_mac.return_value = { + mac_master: nic_master, + mac_other: nic_other, + mac_extra: nic_extra, + } + netcfg = {'version': 1, 'config': [ + {'type': 'physical', 'name': nic_master, + 'mac_address': mac_master}, + {'type': 'physical', 'name': nic_other, 'mac_address': mac_other}, + {'type': 'physical', 'name': nic_extra, 'mac_address': mac_extra}, + ]} + + def _is_netfail_master(iface): + if iface == 'ens3': + return True + return False + self.m_netfail_master.side_effect = _is_netfail_master + expected_cfg = {'version': 1, 'config': [ + {'type': 'physical', 'name': nic_master}, + {'type': 'physical', 'name': nic_other, 'mac_address': mac_other}, + {'type': 'physical', 'name': nic_extra, 'mac_address': mac_extra}, + ]} + oracle._ensure_netfailover_safe(netcfg) + self.assertEqual(expected_cfg, netcfg) + + def test_checks_v2_type_ethernet_interfaces(self): + mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' + self.m_get_interfaces_by_mac.return_value = { + mac_addr: nic_name, + } + netcfg = {'version': 2, 'ethernets': { + nic_name: {'dhcp4': True, 'critical': True, 'set-name': nic_name, + 'match': {'macaddress': mac_addr}}}} + passed_netcfg = copy.copy(netcfg) + self.m_netfail_master.return_value = False + oracle._ensure_netfailover_safe(passed_netcfg) + self.assertEqual(netcfg, passed_netcfg) + self.assertEqual([mock.call(nic_name)], + self.m_netfail_master.call_args_list) + + def test_skips_v2_non_ethernet_interfaces(self): + mac_addr, nic_name = '00:00:17:02:2b:b1', 'wlps0' + self.m_get_interfaces_by_mac.return_value = { + mac_addr: nic_name, + } + netcfg = {'version': 2, 'wifis': { + nic_name: {'dhcp4': True, 'critical': True, 'set-name': nic_name, + 'match': {'macaddress': mac_addr}}}} + passed_netcfg = copy.copy(netcfg) + oracle._ensure_netfailover_safe(passed_netcfg) + self.assertEqual(netcfg, passed_netcfg) + self.assertEqual(0, self.m_netfail_master.call_count) + + def test_removes_master_mac_property_v2(self): + nic_master, mac_master = 'ens3', self.random_string() + nic_other, mac_other = 'ens7', self.random_string() + nic_extra, mac_extra = 'enp0s1f2', self.random_string() + self.m_get_interfaces_by_mac.return_value = { + mac_master: nic_master, + mac_other: nic_other, + mac_extra: nic_extra, + } + netcfg = {'version': 2, 'ethernets': { + nic_extra: {'dhcp4': True, 'set-name': nic_extra, + 'match': {'macaddress': mac_extra}}, + nic_other: {'dhcp4': True, 'set-name': nic_other, + 'match': {'macaddress': mac_other}}, + nic_master: {'dhcp4': True, 'set-name': nic_master, + 'match': {'macaddress': mac_master}}, + }} + + def _is_netfail_master(iface): + if iface == 'ens3': + return True + return False + self.m_netfail_master.side_effect = _is_netfail_master + + expected_cfg = {'version': 2, 'ethernets': { + nic_master: {'dhcp4': True, 'match': {'name': nic_master}}, + nic_extra: {'dhcp4': True, 'set-name': nic_extra, + 'match': {'macaddress': mac_extra}}, + nic_other: {'dhcp4': True, 'set-name': nic_other, + 'match': {'macaddress': mac_other}}, + }} + oracle._ensure_netfailover_safe(netcfg) + import pprint + pprint.pprint(netcfg) + print('---- ^^ modified ^^ ---- vv original vv ----') + pprint.pprint(expected_cfg) + self.assertEqual(expected_cfg, netcfg) + + # vi: ts=4 expandtab diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 23fddd07..4dad2afd 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -6,7 +6,9 @@ import functools import httpretty import logging import os +import random import shutil +import string import sys import tempfile import time @@ -243,6 +245,12 @@ class CiTestCase(TestCase): myds.metadata.update(metadata) return cloud.Cloud(myds, self.paths, sys_cfg, mydist, None) + @classmethod + def random_string(cls, length=8): + """ return a random lowercase string with default length of 8""" + return ''.join( + random.choice(string.ascii_lowercase) for _ in range(length)) + class ResourceUsingTestCase(CiTestCase): -- cgit v1.2.3