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/tests/test_oracle.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'cloudinit/sources/tests/test_oracle.py') diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 97d62947..282382c5 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -133,9 +133,9 @@ 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 + ".cmdline.read_kernel_cmdline_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_cmdline_config): + def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config): """network_config should read kernel cmdline.""" distro = mock.MagicMock() ds, _ = self._get_ds(distro=distro, patches={ @@ -145,15 +145,15 @@ class TestDataSourceOracle(test_helpers.CiTestCase): MD_VER: {'system_uuid': self.my_uuid, 'meta_data': self.my_md}}}}) ncfg = {'version': 1, 'config': [{'a': 'b'}]} - m_cmdline_config.return_value = ncfg + m_initramfs_config.return_value = ncfg self.assertTrue(ds._get_data()) self.assertEqual(ncfg, ds.network_config) - m_cmdline_config.assert_called_once_with() + self.assertEqual([mock.call()], m_initramfs_config.call_args_list) self.assertFalse(distro.generate_fallback_config.called) - @mock.patch(DS_PATH + ".cmdline.read_kernel_cmdline_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_cmdline_config): + def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config): """test that fallback network is generated if no kernel cmdline.""" distro = mock.MagicMock() ds, _ = self._get_ds(distro=distro, patches={ @@ -163,18 +163,17 @@ class TestDataSourceOracle(test_helpers.CiTestCase): MD_VER: {'system_uuid': self.my_uuid, 'meta_data': self.my_md}}}}) ncfg = {'version': 1, 'config': [{'a': 'b'}]} - m_cmdline_config.return_value = None + m_initramfs_config.return_value = None self.assertTrue(ds._get_data()) ncfg = {'version': 1, 'config': [{'distro1': 'value'}]} distro.generate_fallback_config.return_value = ncfg self.assertEqual(ncfg, ds.network_config) - m_cmdline_config.assert_called_once_with() + self.assertEqual([mock.call()], m_initramfs_config.call_args_list) distro.generate_fallback_config.assert_called_once_with() - self.assertEqual(1, m_cmdline_config.call_count) # test that the result got cached, and the methods not re-called. self.assertEqual(ncfg, ds.network_config) - self.assertEqual(1, m_cmdline_config.call_count) + self.assertEqual(1, m_initramfs_config.call_count) @mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4())) -- 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/tests/test_oracle.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/tests/test_oracle.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/tests/test_oracle.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/tests/test_oracle.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 From bb71a9d08d25193836eda91c328760305285574e Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 21 Jan 2020 18:02:42 -0500 Subject: Drop most of the remaining use of six (#179) --- cloudinit/config/cc_chef.py | 4 +-- cloudinit/config/cc_mcollective.py | 10 +++---- cloudinit/config/cc_ntp.py | 20 ++++++------- cloudinit/config/cc_power_state_change.py | 9 +++--- cloudinit/config/cc_rsyslog.py | 7 ++--- cloudinit/config/cc_ubuntu_advantage.py | 4 +-- cloudinit/config/cc_write_files.py | 3 +- cloudinit/config/cc_yum_add_repo.py | 12 +++----- cloudinit/distros/__init__.py | 13 ++++----- cloudinit/distros/freebsd.py | 7 ++--- cloudinit/distros/parsers/sys_conf.py | 6 ++-- cloudinit/distros/ug_util.py | 22 +++++++-------- cloudinit/net/network_state.py | 11 +++----- cloudinit/net/renderer.py | 4 +-- cloudinit/net/sysconfig.py | 15 +++++----- cloudinit/sources/tests/test_init.py | 33 +--------------------- cloudinit/sources/tests/test_oracle.py | 3 +- cloudinit/stages.py | 6 ++-- cloudinit/tests/helpers.py | 15 +++++----- tests/unittests/test_cli.py | 16 +++++------ tests/unittests/test_datasource/test_smartos.py | 4 +-- tests/unittests/test_handler/test_handler_chef.py | 3 +- .../test_handler/test_handler_write_files.py | 15 +++++----- tests/unittests/test_log.py | 11 ++++---- tests/unittests/test_merging.py | 6 ++-- tests/unittests/test_util.py | 17 ++++++----- 26 files changed, 104 insertions(+), 172 deletions(-) (limited to 'cloudinit/sources/tests/test_oracle.py') diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index 0ad6b7f1..01d61fa1 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -79,8 +79,6 @@ from cloudinit import templater from cloudinit import url_helper from cloudinit import util -import six - RUBY_VERSION_DEFAULT = "1.8" CHEF_DIRS = tuple([ @@ -273,7 +271,7 @@ def run_chef(chef_cfg, log): cmd_args = chef_cfg['exec_arguments'] if isinstance(cmd_args, (list, tuple)): cmd.extend(cmd_args) - elif isinstance(cmd_args, six.string_types): + elif isinstance(cmd_args, str): cmd.append(cmd_args) else: log.warning("Unknown type %s provided for chef" diff --git a/cloudinit/config/cc_mcollective.py b/cloudinit/config/cc_mcollective.py index d5f63f5f..351183f1 100644 --- a/cloudinit/config/cc_mcollective.py +++ b/cloudinit/config/cc_mcollective.py @@ -49,9 +49,7 @@ private certificates for mcollective. Their values will be written to """ import errno - -import six -from six import BytesIO +import io # Used since this can maintain comments # and doesn't need a top level section @@ -73,7 +71,7 @@ def configure(config, server_cfg=SERVER_CFG, # original file in order to be able to mix the rest up. try: old_contents = util.load_file(server_cfg, quiet=False, decode=False) - mcollective_config = ConfigObj(BytesIO(old_contents)) + mcollective_config = ConfigObj(io.BytesIO(old_contents)) except IOError as e: if e.errno != errno.ENOENT: raise @@ -93,7 +91,7 @@ def configure(config, server_cfg=SERVER_CFG, 'plugin.ssl_server_private'] = pricert_file mcollective_config['securityprovider'] = 'ssl' else: - if isinstance(cfg, six.string_types): + if isinstance(cfg, str): # Just set it in the 'main' section mcollective_config[cfg_name] = cfg elif isinstance(cfg, (dict)): @@ -119,7 +117,7 @@ def configure(config, server_cfg=SERVER_CFG, raise # Now we got the whole (new) file, write to disk... - contents = BytesIO() + contents = io.BytesIO() mcollective_config.write(contents) util.write_file(server_cfg, contents.getvalue(), mode=0o644) diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index 9e074bda..5498bbaa 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -6,19 +6,17 @@ """NTP: enable and configure ntp""" -from cloudinit.config.schema import ( - get_schema_doc, validate_cloudconfig_schema) +import copy +import os +from textwrap import dedent + from cloudinit import log as logging -from cloudinit.settings import PER_INSTANCE from cloudinit import temp_utils from cloudinit import templater from cloudinit import type_utils from cloudinit import util - -import copy -import os -import six -from textwrap import dedent +from cloudinit.config.schema import get_schema_doc, validate_cloudconfig_schema +from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) @@ -460,7 +458,7 @@ def supplemental_schema_validation(ntp_config): for key, value in sorted(ntp_config.items()): keypath = 'ntp:config:' + key if key == 'confpath': - if not all([value, isinstance(value, six.string_types)]): + if not all([value, isinstance(value, str)]): errors.append( 'Expected a config file path {keypath}.' ' Found ({value})'.format(keypath=keypath, value=value)) @@ -472,11 +470,11 @@ def supplemental_schema_validation(ntp_config): elif key in ('template', 'template_name'): if value is None: # Either template or template_name can be none continue - if not isinstance(value, six.string_types): + if not isinstance(value, str): errors.append( 'Expected a string type for {keypath}.' ' Found ({value})'.format(keypath=keypath, value=value)) - elif not isinstance(value, six.string_types): + elif not isinstance(value, str): errors.append( 'Expected a string type for {keypath}.' ' Found ({value})'.format(keypath=keypath, value=value)) diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py index 43a479cf..3e81a3c7 100644 --- a/cloudinit/config/cc_power_state_change.py +++ b/cloudinit/config/cc_power_state_change.py @@ -49,16 +49,15 @@ key returns 0. condition: """ -from cloudinit.settings import PER_INSTANCE -from cloudinit import util - import errno import os import re -import six import subprocess import time +from cloudinit.settings import PER_INSTANCE +from cloudinit import util + frequency = PER_INSTANCE EXIT_FAIL = 254 @@ -183,7 +182,7 @@ def load_power_state(cfg): pstate['timeout']) condition = pstate.get("condition", True) - if not isinstance(condition, six.string_types + (list, bool)): + if not isinstance(condition, (str, list, bool)): raise TypeError("condition type %s invalid. must be list, bool, str") return (args, timeout, condition) diff --git a/cloudinit/config/cc_rsyslog.py b/cloudinit/config/cc_rsyslog.py index ff211f65..5df0137d 100644 --- a/cloudinit/config/cc_rsyslog.py +++ b/cloudinit/config/cc_rsyslog.py @@ -180,7 +180,6 @@ config entries. Legacy to new mappings are as follows: import os import re -import six from cloudinit import log as logging from cloudinit import util @@ -233,9 +232,9 @@ def load_config(cfg): fillup = ( (KEYNAME_CONFIGS, [], list), - (KEYNAME_DIR, DEF_DIR, six.string_types), - (KEYNAME_FILENAME, DEF_FILENAME, six.string_types), - (KEYNAME_RELOAD, DEF_RELOAD, six.string_types + (list,)), + (KEYNAME_DIR, DEF_DIR, str), + (KEYNAME_FILENAME, DEF_FILENAME, str), + (KEYNAME_RELOAD, DEF_RELOAD, (str, list)), (KEYNAME_REMOTES, DEF_REMOTES, dict)) for key, default, vtypes in fillup: diff --git a/cloudinit/config/cc_ubuntu_advantage.py b/cloudinit/config/cc_ubuntu_advantage.py index f846e9a5..8b6d2a1a 100644 --- a/cloudinit/config/cc_ubuntu_advantage.py +++ b/cloudinit/config/cc_ubuntu_advantage.py @@ -4,8 +4,6 @@ from textwrap import dedent -import six - from cloudinit.config.schema import ( get_schema_doc, validate_cloudconfig_schema) from cloudinit import log as logging @@ -98,7 +96,7 @@ def configure_ua(token=None, enable=None): if enable is None: enable = [] - elif isinstance(enable, six.string_types): + elif isinstance(enable, str): LOG.warning('ubuntu_advantage: enable should be a list, not' ' a string; treating as a single enable') enable = [enable] diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py index 0b6546e2..bd87e9e5 100644 --- a/cloudinit/config/cc_write_files.py +++ b/cloudinit/config/cc_write_files.py @@ -57,7 +57,6 @@ binary gzip data can be specified and will be decoded before being written. import base64 import os -import six from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE @@ -126,7 +125,7 @@ def decode_perms(perm, default): if perm is None: return default try: - if isinstance(perm, six.integer_types + (float,)): + if isinstance(perm, (int, float)): # Just 'downcast' it (if a float) return int(perm) else: diff --git a/cloudinit/config/cc_yum_add_repo.py b/cloudinit/config/cc_yum_add_repo.py index 3b354a7d..3673166a 100644 --- a/cloudinit/config/cc_yum_add_repo.py +++ b/cloudinit/config/cc_yum_add_repo.py @@ -30,13 +30,9 @@ entry, the config entry will be skipped. # any repository configuration options (see man yum.conf) """ +import io import os - -try: - from configparser import ConfigParser -except ImportError: - from ConfigParser import ConfigParser -import six +from configparser import ConfigParser from cloudinit import util @@ -57,7 +53,7 @@ def _format_repo_value(val): # Can handle 'lists' in certain cases # See: https://linux.die.net/man/5/yum.conf return "\n".join([_format_repo_value(v) for v in val]) - if not isinstance(val, six.string_types): + if not isinstance(val, str): return str(val) return val @@ -72,7 +68,7 @@ def _format_repository_config(repo_id, repo_config): # For now assume that people using this know # the format of yum and don't verify keys/values further to_be.set(repo_id, k, _format_repo_value(v)) - to_be_stream = six.StringIO() + to_be_stream = io.StringIO() to_be.write(to_be_stream) to_be_stream.seek(0) lines = to_be_stream.readlines() diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index cdce26f2..92598a2d 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -9,13 +9,11 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import six -from six import StringIO - import abc import os import re import stat +from io import StringIO from cloudinit import importer from cloudinit import log as logging @@ -53,8 +51,7 @@ _EC2_AZ_RE = re.compile('^[a-z][a-z]-(?:[a-z]+-)+[0-9][a-z]$') PREFERRED_NTP_CLIENTS = ['chrony', 'systemd-timesyncd', 'ntp', 'ntpdate'] -@six.add_metaclass(abc.ABCMeta) -class Distro(object): +class Distro(metaclass=abc.ABCMeta): usr_lib_exec = "/usr/lib" hosts_fn = "/etc/hosts" @@ -429,7 +426,7 @@ class Distro(object): # support kwargs having groups=[list] or groups="g1,g2" groups = kwargs.get('groups') if groups: - if isinstance(groups, six.string_types): + if isinstance(groups, str): groups = groups.split(",") # remove any white spaces in group names, most likely @@ -544,7 +541,7 @@ class Distro(object): if 'ssh_authorized_keys' in kwargs: # Try to handle this in a smart manner. keys = kwargs['ssh_authorized_keys'] - if isinstance(keys, six.string_types): + if isinstance(keys, str): keys = [keys] elif isinstance(keys, dict): keys = list(keys.values()) @@ -668,7 +665,7 @@ class Distro(object): if isinstance(rules, (list, tuple)): for rule in rules: lines.append("%s %s" % (user, rule)) - elif isinstance(rules, six.string_types): + elif isinstance(rules, str): lines.append("%s %s" % (user, rules)) else: msg = "Can not create sudoers rule addition with type %r" diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py index 40e435e7..026d1142 100644 --- a/cloudinit/distros/freebsd.py +++ b/cloudinit/distros/freebsd.py @@ -5,10 +5,8 @@ # This file is part of cloud-init. See LICENSE file for license information. import os -import six -from six import StringIO - import re +from io import StringIO from cloudinit import distros from cloudinit import helpers @@ -108,8 +106,7 @@ class Distro(distros.Distro): } for key, val in kwargs.items(): - if (key in pw_useradd_opts and val and - isinstance(val, six.string_types)): + if key in pw_useradd_opts and val and isinstance(val, str): pw_useradd_cmd.extend([pw_useradd_opts[key], val]) elif key in pw_useradd_flags and val: diff --git a/cloudinit/distros/parsers/sys_conf.py b/cloudinit/distros/parsers/sys_conf.py index 44df17de..dee4c551 100644 --- a/cloudinit/distros/parsers/sys_conf.py +++ b/cloudinit/distros/parsers/sys_conf.py @@ -4,11 +4,9 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import six -from six import StringIO - import pipes import re +from io import StringIO # This library is used to parse/write # out the various sysconfig files edited (best attempt effort) @@ -65,7 +63,7 @@ class SysConf(configobj.ConfigObj): return out_contents.getvalue() def _quote(self, value, multiline=False): - if not isinstance(value, six.string_types): + if not isinstance(value, str): raise ValueError('Value "%s" is not a string' % (value)) if len(value) == 0: return '' diff --git a/cloudinit/distros/ug_util.py b/cloudinit/distros/ug_util.py index 9378dd78..08446a95 100755 --- a/cloudinit/distros/ug_util.py +++ b/cloudinit/distros/ug_util.py @@ -9,8 +9,6 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import six - from cloudinit import log as logging from cloudinit import type_utils from cloudinit import util @@ -29,7 +27,7 @@ LOG = logging.getLogger(__name__) # is the standard form used in the rest # of cloud-init def _normalize_groups(grp_cfg): - if isinstance(grp_cfg, six.string_types): + if isinstance(grp_cfg, str): grp_cfg = grp_cfg.strip().split(",") if isinstance(grp_cfg, list): c_grp_cfg = {} @@ -39,7 +37,7 @@ def _normalize_groups(grp_cfg): if k not in c_grp_cfg: if isinstance(v, list): c_grp_cfg[k] = list(v) - elif isinstance(v, six.string_types): + elif isinstance(v, str): c_grp_cfg[k] = [v] else: raise TypeError("Bad group member type %s" % @@ -47,12 +45,12 @@ def _normalize_groups(grp_cfg): else: if isinstance(v, list): c_grp_cfg[k].extend(v) - elif isinstance(v, six.string_types): + elif isinstance(v, str): c_grp_cfg[k].append(v) else: raise TypeError("Bad group member type %s" % type_utils.obj_name(v)) - elif isinstance(i, six.string_types): + elif isinstance(i, str): if i not in c_grp_cfg: c_grp_cfg[i] = [] else: @@ -89,7 +87,7 @@ def _normalize_users(u_cfg, def_user_cfg=None): if isinstance(u_cfg, dict): ad_ucfg = [] for (k, v) in u_cfg.items(): - if isinstance(v, (bool, int, float) + six.string_types): + if isinstance(v, (bool, int, float, str)): if util.is_true(v): ad_ucfg.append(str(k)) elif isinstance(v, dict): @@ -99,12 +97,12 @@ def _normalize_users(u_cfg, def_user_cfg=None): raise TypeError(("Unmappable user value type %s" " for key %s") % (type_utils.obj_name(v), k)) u_cfg = ad_ucfg - elif isinstance(u_cfg, six.string_types): + elif isinstance(u_cfg, str): u_cfg = util.uniq_merge_sorted(u_cfg) users = {} for user_config in u_cfg: - if isinstance(user_config, (list,) + six.string_types): + if isinstance(user_config, (list, str)): for u in util.uniq_merge(user_config): if u and u not in users: users[u] = {} @@ -209,7 +207,7 @@ def normalize_users_groups(cfg, distro): old_user = cfg['user'] # Translate it into the format that is more useful # going forward - if isinstance(old_user, six.string_types): + if isinstance(old_user, str): old_user = { 'name': old_user, } @@ -238,7 +236,7 @@ def normalize_users_groups(cfg, distro): default_user_config = util.mergemanydict([old_user, distro_user_config]) base_users = cfg.get('users', []) - if not isinstance(base_users, (list, dict) + six.string_types): + if not isinstance(base_users, (list, dict, str)): LOG.warning(("Format for 'users' key must be a comma separated string" " or a dictionary or a list and not %s"), type_utils.obj_name(base_users)) @@ -252,7 +250,7 @@ def normalize_users_groups(cfg, distro): base_users.append({'name': 'default'}) elif isinstance(base_users, dict): base_users['default'] = dict(base_users).get('default', True) - elif isinstance(base_users, six.string_types): + elif isinstance(base_users, str): # Just append it on to be re-parsed later base_users += ",default" diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 9b126100..63d6e291 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -10,8 +10,6 @@ import logging import socket import struct -import six - from cloudinit import safeyaml from cloudinit import util @@ -186,7 +184,7 @@ class NetworkState(object): def iter_interfaces(self, filter_func=None): ifaces = self._network_state.get('interfaces', {}) - for iface in six.itervalues(ifaces): + for iface in ifaces.values(): if filter_func is None: yield iface else: @@ -220,8 +218,7 @@ class NetworkState(object): ) -@six.add_metaclass(CommandHandlerMeta) -class NetworkStateInterpreter(object): +class NetworkStateInterpreter(metaclass=CommandHandlerMeta): initial_network_state = { 'interfaces': {}, @@ -970,7 +967,7 @@ def ipv4_mask_to_net_prefix(mask): """ if isinstance(mask, int): return mask - if isinstance(mask, six.string_types): + if isinstance(mask, str): try: return int(mask) except ValueError: @@ -997,7 +994,7 @@ def ipv6_mask_to_net_prefix(mask): if isinstance(mask, int): return mask - if isinstance(mask, six.string_types): + if isinstance(mask, str): try: return int(mask) except ValueError: diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py index 5f32e90f..2a61a7a8 100644 --- a/cloudinit/net/renderer.py +++ b/cloudinit/net/renderer.py @@ -6,7 +6,7 @@ # This file is part of cloud-init. See LICENSE file for license information. import abc -import six +import io from .network_state import parse_net_config_data from .udev import generate_udev_rule @@ -34,7 +34,7 @@ class Renderer(object): """Given state, emit udev rules to map mac to ifname.""" # TODO(harlowja): this seems shared between eni renderer and # this, so move it to a shared location. - content = six.StringIO() + content = io.StringIO() for iface in network_state.iter_interfaces(filter_by_physical): # for physical interfaces write out a persist net udev rule if 'name' in iface and iface.get('mac_address'): diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 3e06af01..07668d3e 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -1,16 +1,15 @@ # This file is part of cloud-init. See LICENSE file for license information. +import io import os import re -import six +from configobj import ConfigObj -from cloudinit.distros.parsers import networkmanager_conf -from cloudinit.distros.parsers import resolv_conf from cloudinit import log as logging from cloudinit import util - -from configobj import ConfigObj +from cloudinit.distros.parsers import networkmanager_conf +from cloudinit.distros.parsers import resolv_conf from . import renderer from .network_state import ( @@ -96,7 +95,7 @@ class ConfigMap(object): return len(self._conf) def to_string(self): - buf = six.StringIO() + buf = io.StringIO() buf.write(_make_header()) if self._conf: buf.write("\n") @@ -104,7 +103,7 @@ class ConfigMap(object): value = self._conf[key] if isinstance(value, bool): value = self._bool_map[value] - if not isinstance(value, six.string_types): + if not isinstance(value, str): value = str(value) buf.write("%s=%s\n" % (key, _quote_value(value))) return buf.getvalue() @@ -150,7 +149,7 @@ class Route(ConfigMap): # only accept ipv4 and ipv6 if proto not in ['ipv4', 'ipv6']: raise ValueError("Unknown protocol '%s'" % (str(proto))) - buf = six.StringIO() + buf = io.StringIO() buf.write(_make_header()) if self._conf: buf.write("\n") diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 9698261b..f73b37ed 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -3,7 +3,6 @@ import copy import inspect import os -import six import stat from cloudinit.event import EventType @@ -13,7 +12,7 @@ from cloudinit.sources import ( EXPERIMENTAL_TEXT, INSTANCE_JSON_FILE, INSTANCE_JSON_SENSITIVE_FILE, METADATA_UNKNOWN, REDACT_SENSITIVE_VALUE, UNSET, DataSource, canonical_cloud_id, redact_sensitive_keys) -from cloudinit.tests.helpers import CiTestCase, skipIf, mock +from cloudinit.tests.helpers import CiTestCase, mock from cloudinit.user_data import UserDataProcessor from cloudinit import util @@ -422,7 +421,6 @@ class TestDataSource(CiTestCase): {'network_json': 'is good'}, instance_data['ds']['network_json']) - @skipIf(not six.PY3, "json serialization on <= py2.7 handles bytes") def test_get_data_base64encodes_unserializable_bytes(self): """On py3, get_data base64encodes any unserializable content.""" tmp = self.tmp_dir() @@ -440,35 +438,6 @@ class TestDataSource(CiTestCase): {'key1': 'val1', 'key2': {'key2.1': 'EjM='}}, instance_json['ds']['meta_data']) - @skipIf(not six.PY2, "json serialization on <= py2.7 handles bytes") - def test_get_data_handles_bytes_values(self): - """On py2 get_data handles bytes values without having to b64encode.""" - tmp = self.tmp_dir() - datasource = DataSourceTestSubclassNet( - self.sys_cfg, self.distro, Paths({'run_dir': tmp}), - custom_metadata={'key1': 'val1', 'key2': {'key2.1': b'\x123'}}) - self.assertTrue(datasource.get_data()) - json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) - content = util.load_file(json_file) - instance_json = util.load_json(content) - self.assertEqual([], instance_json['base64_encoded_keys']) - self.assertEqual( - {'key1': 'val1', 'key2': {'key2.1': '\x123'}}, - instance_json['ds']['meta_data']) - - @skipIf(not six.PY2, "Only python2 hits UnicodeDecodeErrors on non-utf8") - def test_non_utf8_encoding_gets_b64encoded(self): - """When non-utf-8 values exist in py2 instance-data is b64encoded.""" - tmp = self.tmp_dir() - datasource = DataSourceTestSubclassNet( - self.sys_cfg, self.distro, Paths({'run_dir': tmp}), - custom_metadata={'key1': 'val1', 'key2': {'key2.1': b'ab\xaadef'}}) - self.assertTrue(datasource.get_data()) - json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) - instance_json = util.load_json(util.load_file(json_file)) - key21_value = instance_json['ds']['meta_data']['key2']['key2.1'] - self.assertEqual('ci-b64:' + util.b64e(b'ab\xaadef'), key21_value) - def test_get_hostname_subclass_support(self): """Validate get_hostname signature on all subclasses of DataSource.""" # Use inspect.getfullargspec when we drop py2.6 and py2.7 diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 85b6db97..6c551fcb 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -13,7 +13,6 @@ import httpretty import json import mock import os -import six import uuid DS_PATH = "cloudinit.sources.DataSourceOracle" @@ -334,7 +333,7 @@ class TestReadMetaData(test_helpers.HttprettyTestCase): for k, v in data.items(): httpretty.register_uri( httpretty.GET, self.mdurl + MD_VER + "/" + k, - v if not isinstance(v, six.text_type) else v.encode('utf-8')) + v if not isinstance(v, str) else v.encode('utf-8')) def test_broken_no_sys_uuid(self, m_read_system_uuid): """Datasource requires ability to read system_uuid and true return.""" diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 71f3a49e..db8ba64c 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -6,11 +6,9 @@ import copy import os +import pickle import sys -import six -from six.moves import cPickle as pickle - from cloudinit.settings import ( FREQUENCIES, CLOUD_CONFIG, PER_INSTANCE, RUN_CLOUD_CONFIG) @@ -758,7 +756,7 @@ class Modules(object): for item in cfg_mods: if not item: continue - if isinstance(item, six.string_types): + if isinstance(item, str): module_list.append({ 'mod': item.strip(), }) diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 4dad2afd..0220648d 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -4,6 +4,7 @@ from __future__ import print_function import functools import httpretty +import io import logging import os import random @@ -14,7 +15,6 @@ import tempfile import time import mock -import six import unittest2 from unittest2.util import strclass @@ -72,7 +72,7 @@ def retarget_many_wrapper(new_base, am, old_func): # Python 3 some of these now accept file-descriptors (integers). # That breaks rebase_path() so in lieu of a better solution, just # don't rebase if we get a fd. - if isinstance(path, six.string_types): + if isinstance(path, str): n_args[i] = rebase_path(path, new_base) return old_func(*n_args, **kwds) return wrapper @@ -149,7 +149,7 @@ class CiTestCase(TestCase): if self.with_logs: # Create a log handler so unit tests can search expected logs. self.logger = logging.getLogger() - self.logs = six.StringIO() + self.logs = io.StringIO() formatter = logging.Formatter('%(levelname)s: %(message)s') handler = logging.StreamHandler(self.logs) handler.setFormatter(formatter) @@ -166,7 +166,7 @@ class CiTestCase(TestCase): else: cmd = args[0] - if not isinstance(cmd, six.string_types): + if not isinstance(cmd, str): cmd = cmd[0] pass_through = False if not isinstance(self.allowed_subp, (list, bool)): @@ -346,8 +346,9 @@ class FilesystemMockingTestCase(ResourceUsingTestCase): def patchOpen(self, new_root): trap_func = retarget_many_wrapper(new_root, 1, open) - name = 'builtins.open' if six.PY3 else '__builtin__.open' - self.patched_funcs.enter_context(mock.patch(name, trap_func)) + self.patched_funcs.enter_context( + mock.patch('builtins.open', trap_func) + ) def patchStdoutAndStderr(self, stdout=None, stderr=None): if stdout is not None: @@ -420,7 +421,7 @@ def populate_dir(path, files): p = os.path.sep.join([path, name]) util.ensure_dir(os.path.dirname(p)) with open(p, "wb") as fp: - if isinstance(content, six.binary_type): + if isinstance(content, bytes): fp.write(content) else: fp.write(content.encode('utf-8')) diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py index d283f136..e57c15d1 100644 --- a/tests/unittests/test_cli.py +++ b/tests/unittests/test_cli.py @@ -1,8 +1,8 @@ # This file is part of cloud-init. See LICENSE file for license information. -from collections import namedtuple import os -import six +import io +from collections import namedtuple from cloudinit.cmd import main as cli from cloudinit.tests import helpers as test_helpers @@ -18,7 +18,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def setUp(self): super(TestCLI, self).setUp() - self.stderr = six.StringIO() + self.stderr = io.StringIO() self.patchStdoutAndStderr(stderr=self.stderr) def _call_main(self, sysv_args=None): @@ -147,7 +147,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_conditional_subcommands_from_entry_point_sys_argv(self): """Subcommands from entry-point are properly parsed from sys.argv.""" - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) expected_errors = [ @@ -178,7 +178,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_collect_logs_subcommand_parser(self): """The subcommand cloud-init collect-logs calls the subparser.""" # Provide -h param to collect-logs to avoid having to mock behavior. - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'collect-logs', '-h']) self.assertIn('usage: cloud-init collect-log', stdout.getvalue()) @@ -186,7 +186,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_clean_subcommand_parser(self): """The subcommand cloud-init clean calls the subparser.""" # Provide -h param to clean to avoid having to mock behavior. - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'clean', '-h']) self.assertIn('usage: cloud-init clean', stdout.getvalue()) @@ -194,7 +194,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_status_subcommand_parser(self): """The subcommand cloud-init status calls the subparser.""" # Provide -h param to clean to avoid having to mock behavior. - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'status', '-h']) self.assertIn('usage: cloud-init status', stdout.getvalue()) @@ -219,7 +219,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_wb_devel_schema_subcommand_doc_content(self): """Validate that doc content is sane from known examples.""" - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'devel', 'schema', '--doc']) expected_doc_sections = [ diff --git a/tests/unittests/test_datasource/test_smartos.py b/tests/unittests/test_datasource/test_smartos.py index d5b1c29c..62084de5 100644 --- a/tests/unittests/test_datasource/test_smartos.py +++ b/tests/unittests/test_datasource/test_smartos.py @@ -33,8 +33,6 @@ from cloudinit.sources.DataSourceSmartOS import ( identify_file) from cloudinit.event import EventType -import six - from cloudinit import helpers as c_helpers from cloudinit.util import ( b64e, subp, ProcessExecutionError, which, write_file) @@ -798,7 +796,7 @@ class TestJoyentMetadataClient(FilesystemMockingTestCase): return self.serial.write.call_args[0][0] def test_get_metadata_writes_bytes(self): - self.assertIsInstance(self._get_written_line(), six.binary_type) + self.assertIsInstance(self._get_written_line(), bytes) def test_get_metadata_line_starts_with_v2(self): foo = self._get_written_line() diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py index f4311268..2dab3a54 100644 --- a/tests/unittests/test_handler/test_handler_chef.py +++ b/tests/unittests/test_handler/test_handler_chef.py @@ -4,7 +4,6 @@ import httpretty import json import logging import os -import six from cloudinit import cloud from cloudinit.config import cc_chef @@ -178,7 +177,7 @@ class TestChef(FilesystemMockingTestCase): continue # the value from the cfg overrides that in the default val = cfg['chef'].get(k, v) - if isinstance(val, six.string_types): + if isinstance(val, str): self.assertIn(val, c) c = util.load_file(cc_chef.CHEF_FB_PATH) self.assertEqual({}, json.loads(c)) diff --git a/tests/unittests/test_handler/test_handler_write_files.py b/tests/unittests/test_handler/test_handler_write_files.py index bc8756ca..ed0a4da2 100644 --- a/tests/unittests/test_handler/test_handler_write_files.py +++ b/tests/unittests/test_handler/test_handler_write_files.py @@ -1,17 +1,16 @@ # This file is part of cloud-init. See LICENSE file for license information. -from cloudinit.config.cc_write_files import write_files, decode_perms -from cloudinit import log as logging -from cloudinit import util - -from cloudinit.tests.helpers import CiTestCase, FilesystemMockingTestCase - import base64 import gzip +import io import shutil -import six import tempfile +from cloudinit import log as logging +from cloudinit import util +from cloudinit.config.cc_write_files import write_files, decode_perms +from cloudinit.tests.helpers import CiTestCase, FilesystemMockingTestCase + LOG = logging.getLogger(__name__) YAML_TEXT = """ @@ -138,7 +137,7 @@ class TestDecodePerms(CiTestCase): def _gzip_bytes(data): - buf = six.BytesIO() + buf = io.BytesIO() fp = None try: fp = gzip.GzipFile(fileobj=buf, mode="wb") diff --git a/tests/unittests/test_log.py b/tests/unittests/test_log.py index cd6296d6..e069a487 100644 --- a/tests/unittests/test_log.py +++ b/tests/unittests/test_log.py @@ -2,14 +2,15 @@ """Tests for cloudinit.log """ -from cloudinit.analyze.dump import CLOUD_INIT_ASCTIME_FMT -from cloudinit import log as ci_logging -from cloudinit.tests.helpers import CiTestCase import datetime +import io import logging -import six import time +from cloudinit import log as ci_logging +from cloudinit.analyze.dump import CLOUD_INIT_ASCTIME_FMT +from cloudinit.tests.helpers import CiTestCase + class TestCloudInitLogger(CiTestCase): @@ -18,7 +19,7 @@ class TestCloudInitLogger(CiTestCase): # of sys.stderr, we'll plug in a StringIO() object so we can see # what gets logged logging.Formatter.converter = time.gmtime - self.ci_logs = six.StringIO() + self.ci_logs = io.StringIO() self.ci_root = logging.getLogger() console = logging.StreamHandler(self.ci_logs) console.setFormatter(logging.Formatter(ci_logging.DEF_CON_FORMAT)) diff --git a/tests/unittests/test_merging.py b/tests/unittests/test_merging.py index 3a5072c7..10871bcf 100644 --- a/tests/unittests/test_merging.py +++ b/tests/unittests/test_merging.py @@ -13,13 +13,11 @@ import glob import os import random import re -import six import string SOURCE_PAT = "source*.*yaml" EXPECTED_PAT = "expected%s.yaml" -TYPES = [dict, str, list, tuple, None] -TYPES.extend(six.integer_types) +TYPES = [dict, str, list, tuple, None, int] def _old_mergedict(src, cand): @@ -85,7 +83,7 @@ def _make_dict(current_depth, max_depth, rand): pass if t in [tuple]: base = tuple(base) - elif t in six.integer_types: + elif t in [int]: base = rand.randint(0, 2 ** 8) elif t in [str]: base = _random_str(rand) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 0e71db82..75a3f0b4 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -2,16 +2,15 @@ from __future__ import print_function +import io +import json import logging import os import re import shutil import stat -import tempfile - -import json -import six import sys +import tempfile import yaml from cloudinit import importer, util @@ -320,7 +319,7 @@ class TestLoadYaml(helpers.CiTestCase): def test_python_unicode(self): # complex type of python/unicode is explicitly allowed - myobj = {'1': six.text_type("FOOBAR")} + myobj = {'1': "FOOBAR"} safe_yaml = yaml.dump(myobj) self.assertEqual(util.load_yaml(blob=safe_yaml, default=self.mydefault), @@ -663,8 +662,8 @@ class TestMultiLog(helpers.FilesystemMockingTestCase): self.patchOS(self.root) self.patchUtils(self.root) self.patchOpen(self.root) - self.stdout = six.StringIO() - self.stderr = six.StringIO() + self.stdout = io.StringIO() + self.stderr = io.StringIO() self.patchStdoutAndStderr(self.stdout, self.stderr) def test_stderr_used_by_default(self): @@ -879,8 +878,8 @@ class TestSubp(helpers.CiTestCase): """Raised exc should have stderr, stdout as string if no decode.""" with self.assertRaises(util.ProcessExecutionError) as cm: util.subp([BOGUS_COMMAND], decode=True) - self.assertTrue(isinstance(cm.exception.stdout, six.string_types)) - self.assertTrue(isinstance(cm.exception.stderr, six.string_types)) + self.assertTrue(isinstance(cm.exception.stdout, str)) + self.assertTrue(isinstance(cm.exception.stderr, str)) def test_bunch_of_slashes_in_path(self): self.assertEqual("/target/my/path/", -- cgit v1.2.3 From 5f8f85bb38cc972d3d2c705a1ec73db3f690f323 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Wed, 29 Jan 2020 16:55:39 -0500 Subject: Replace mock library with unittest.mock (#186) * cloudinit: replace "import mock" with "from unittest import mock" * test-requirements.txt: drop mock Co-authored-by: Chad Smith --- cloudinit/config/tests/test_set_passwords.py | 2 +- cloudinit/net/tests/test_init.py | 2 +- cloudinit/net/tests/test_network_state.py | 3 ++- cloudinit/sources/tests/test_oracle.py | 2 +- cloudinit/tests/helpers.py | 2 +- cloudinit/tests/test_dhclient_hook.py | 2 +- cloudinit/tests/test_gpg.py | 4 ++-- cloudinit/tests/test_version.py | 4 ++-- test-requirements.txt | 1 - tests/unittests/test_datasource/test_aliyun.py | 2 +- tests/unittests/test_datasource/test_ec2.py | 2 +- tests/unittests/test_datasource/test_gce.py | 2 +- tests/unittests/test_datasource/test_maas.py | 2 +- tests/unittests/test_distros/test_user_data_normalize.py | 3 ++- tests/unittests/test_handler/test_handler_locale.py | 2 +- tests/unittests/test_reporting.py | 4 ++-- tests/unittests/test_reporting_hyperv.py | 2 +- tests/unittests/test_sshutil.py | 2 +- 18 files changed, 22 insertions(+), 21 deletions(-) (limited to 'cloudinit/sources/tests/test_oracle.py') diff --git a/cloudinit/config/tests/test_set_passwords.py b/cloudinit/config/tests/test_set_passwords.py index 85e2f1fe..3b5cdd06 100644 --- a/cloudinit/config/tests/test_set_passwords.py +++ b/cloudinit/config/tests/test_set_passwords.py @@ -1,6 +1,6 @@ # This file is part of cloud-init. See LICENSE file for license information. -import mock +from unittest import mock from cloudinit.config import cc_set_passwords as setpass from cloudinit.tests.helpers import CiTestCase diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index 6db93e26..5081a337 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -3,10 +3,10 @@ import copy import errno import httpretty -import mock import os import requests import textwrap +from unittest import mock import cloudinit.net as net from cloudinit.util import ensure_file, write_file, ProcessExecutionError diff --git a/cloudinit/net/tests/test_network_state.py b/cloudinit/net/tests/test_network_state.py index fcb4a995..55880852 100644 --- a/cloudinit/net/tests/test_network_state.py +++ b/cloudinit/net/tests/test_network_state.py @@ -1,6 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. -import mock +from unittest import mock + from cloudinit.net import network_state from cloudinit.tests.helpers import CiTestCase diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 6c551fcb..abf3d359 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -11,9 +11,9 @@ import argparse import copy import httpretty import json -import mock import os import uuid +from unittest import mock DS_PATH = "cloudinit.sources.DataSourceOracle" MD_VER = "2013-10-17" diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 0220648d..70f6bad7 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -13,8 +13,8 @@ import string import sys import tempfile import time +from unittest import mock -import mock import unittest2 from unittest2.util import strclass diff --git a/cloudinit/tests/test_dhclient_hook.py b/cloudinit/tests/test_dhclient_hook.py index 7aab8dd5..eadae81c 100644 --- a/cloudinit/tests/test_dhclient_hook.py +++ b/cloudinit/tests/test_dhclient_hook.py @@ -7,8 +7,8 @@ from cloudinit.tests.helpers import CiTestCase, dir2dict, populate_dir import argparse import json -import mock import os +from unittest import mock class TestDhclientHook(CiTestCase): diff --git a/cloudinit/tests/test_gpg.py b/cloudinit/tests/test_gpg.py index 0562b966..8dd57137 100644 --- a/cloudinit/tests/test_gpg.py +++ b/cloudinit/tests/test_gpg.py @@ -1,12 +1,12 @@ # This file is part of cloud-init. See LICENSE file for license information. """Test gpg module.""" +from unittest import mock + from cloudinit import gpg from cloudinit import util from cloudinit.tests.helpers import CiTestCase -import mock - @mock.patch("cloudinit.gpg.time.sleep") @mock.patch("cloudinit.gpg.util.subp") diff --git a/cloudinit/tests/test_version.py b/cloudinit/tests/test_version.py index a96c2a47..778a762c 100644 --- a/cloudinit/tests/test_version.py +++ b/cloudinit/tests/test_version.py @@ -1,10 +1,10 @@ # This file is part of cloud-init. See LICENSE file for license information. +from unittest import mock + from cloudinit.tests.helpers import CiTestCase from cloudinit import version -import mock - class TestExportsFeatures(CiTestCase): def test_has_network_config_v1(self): diff --git a/test-requirements.txt b/test-requirements.txt index d9d41b57..6fb22b24 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,6 +1,5 @@ # Needed generally in tests httpretty>=0.7.1 -mock nose unittest2 coverage diff --git a/tests/unittests/test_datasource/test_aliyun.py b/tests/unittests/test_datasource/test_aliyun.py index e9213ca1..1e66fcdb 100644 --- a/tests/unittests/test_datasource/test_aliyun.py +++ b/tests/unittests/test_datasource/test_aliyun.py @@ -2,8 +2,8 @@ import functools import httpretty -import mock import os +from unittest import mock from cloudinit import helpers from cloudinit.sources import DataSourceAliYun as ay diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 34a089f2..19e1af2b 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -3,7 +3,7 @@ import copy import httpretty import json -import mock +from unittest import mock from cloudinit import helpers from cloudinit.sources import DataSourceEc2 as ec2 diff --git a/tests/unittests/test_datasource/test_gce.py b/tests/unittests/test_datasource/test_gce.py index 67744d32..e9dd6e60 100644 --- a/tests/unittests/test_datasource/test_gce.py +++ b/tests/unittests/test_datasource/test_gce.py @@ -7,8 +7,8 @@ import datetime import httpretty import json -import mock import re +from unittest import mock from base64 import b64encode, b64decode from six.moves.urllib_parse import urlparse diff --git a/tests/unittests/test_datasource/test_maas.py b/tests/unittests/test_datasource/test_maas.py index c84d067e..2a81d3f5 100644 --- a/tests/unittests/test_datasource/test_maas.py +++ b/tests/unittests/test_datasource/test_maas.py @@ -1,11 +1,11 @@ # This file is part of cloud-init. See LICENSE file for license information. from copy import copy -import mock import os import shutil import tempfile import yaml +from unittest import mock from cloudinit.sources import DataSourceMAAS from cloudinit import url_helper diff --git a/tests/unittests/test_distros/test_user_data_normalize.py b/tests/unittests/test_distros/test_user_data_normalize.py index fa4b6cfe..a6faf0ef 100644 --- a/tests/unittests/test_distros/test_user_data_normalize.py +++ b/tests/unittests/test_distros/test_user_data_normalize.py @@ -1,12 +1,13 @@ # This file is part of cloud-init. See LICENSE file for license information. +from unittest import mock + from cloudinit import distros from cloudinit.distros import ug_util from cloudinit import helpers from cloudinit import settings from cloudinit.tests.helpers import TestCase -import mock bcfg = { diff --git a/tests/unittests/test_handler/test_handler_locale.py b/tests/unittests/test_handler/test_handler_locale.py index e29a06f9..b3deb250 100644 --- a/tests/unittests/test_handler/test_handler_locale.py +++ b/tests/unittests/test_handler/test_handler_locale.py @@ -20,10 +20,10 @@ from configobj import ConfigObj from six import BytesIO import logging -import mock import os import shutil import tempfile +from unittest import mock LOG = logging.getLogger(__name__) diff --git a/tests/unittests/test_reporting.py b/tests/unittests/test_reporting.py index e15ba6cf..6814030e 100644 --- a/tests/unittests/test_reporting.py +++ b/tests/unittests/test_reporting.py @@ -2,12 +2,12 @@ # # This file is part of cloud-init. See LICENSE file for license information. +from unittest import mock + from cloudinit import reporting from cloudinit.reporting import events from cloudinit.reporting import handlers -import mock - from cloudinit.tests.helpers import TestCase diff --git a/tests/unittests/test_reporting_hyperv.py b/tests/unittests/test_reporting_hyperv.py index 3582cf0b..b3e083c6 100644 --- a/tests/unittests/test_reporting_hyperv.py +++ b/tests/unittests/test_reporting_hyperv.py @@ -8,7 +8,7 @@ import os import struct import time import re -import mock +from unittest import mock from cloudinit import util from cloudinit.tests.helpers import CiTestCase diff --git a/tests/unittests/test_sshutil.py b/tests/unittests/test_sshutil.py index b227c20b..0be41924 100644 --- a/tests/unittests/test_sshutil.py +++ b/tests/unittests/test_sshutil.py @@ -1,7 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. -from mock import patch from collections import namedtuple +from unittest.mock import patch from cloudinit import ssh_util from cloudinit.tests import helpers as test_helpers -- cgit v1.2.3