From fdadcb5fae51f4e6799314ab98e3aec56c79b17c Mon Sep 17 00:00:00 2001 From: Jason Zions Date: Tue, 15 Jan 2019 21:37:17 +0000 Subject: net: Wait for dhclient to daemonize before reading lease file cloud-init uses dhclient to fetch the DHCP lease so it can extract DHCP options. dhclient creates the leasefile, then writes to it; simply waiting for the leasefile to appear creates a race between dhclient and cloud-init. Instead, wait for dhclient to be parented by init. At that point, we know it has written to the leasefile, so it's safe to copy the file and kill the process. cloud-init creates a temporary directory in which to execute dhclient, and deletes that directory after it has killed the process. If cloud-init abandons waiting for dhclient to daemonize, it will still attempt to delete the temporary directory, but will not report an exception should that attempt fail. LP: #1794399 --- cloudinit/tests/test_temp_utils.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/tests/test_temp_utils.py b/cloudinit/tests/test_temp_utils.py index ffbb92cd..4a52ef89 100644 --- a/cloudinit/tests/test_temp_utils.py +++ b/cloudinit/tests/test_temp_utils.py @@ -2,8 +2,9 @@ """Tests for cloudinit.temp_utils""" -from cloudinit.temp_utils import mkdtemp, mkstemp +from cloudinit.temp_utils import mkdtemp, mkstemp, tempdir from cloudinit.tests.helpers import CiTestCase, wrap_and_call +import os class TestTempUtils(CiTestCase): @@ -98,4 +99,19 @@ class TestTempUtils(CiTestCase): self.assertEqual('/fake/return/path', retval) self.assertEqual([{'dir': '/run/cloud-init/tmp'}], calls) + def test_tempdir_error_suppression(self): + """test tempdir suppresses errors during directory removal.""" + + with self.assertRaises(OSError): + with tempdir(prefix='cloud-init-dhcp-') as tdir: + os.rmdir(tdir) + # As a result, the directory is already gone, + # so shutil.rmtree should raise OSError + + with tempdir(rmtree_ignore_errors=True, + prefix='cloud-init-dhcp-') as tdir: + os.rmdir(tdir) + # Since the directory is already gone, shutil.rmtree would raise + # OSError, but we suppress that + # vi: ts=4 expandtab -- cgit v1.2.3 From fff37e7dc6849fd16db504b0d338fae20a7beb39 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Fri, 8 Feb 2019 22:08:47 +0000 Subject: netinfo: Adjust ifconfig output parsing for FreeBSD ipv6 entries FreeBSD ifconfig output for ipv6 addrs doesn't find scopeid values when present in the output and the pformat rendering assumes that an ipv6 address will have a 'scope6' entry in the netdev info dictionary. This patch finds the scopeid value, which is not always inside <>, and in some cases v6 addrs don't have a scopeid value in the output, so when rendering the table, allow scope6 value to be replaced with the empty value. LP: #1779672 --- cloudinit/netinfo.py | 7 +++++-- cloudinit/tests/test_netinfo.py | 14 ++++++++++++++ tests/data/netinfo/freebsd-ifconfig-output | 17 +++++++++++++++++ tests/data/netinfo/freebsd-netdev-formatted-output | 11 +++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/data/netinfo/freebsd-ifconfig-output create mode 100644 tests/data/netinfo/freebsd-netdev-formatted-output (limited to 'cloudinit/tests') diff --git a/cloudinit/netinfo.py b/cloudinit/netinfo.py index 9ff929c2..e91cd263 100644 --- a/cloudinit/netinfo.py +++ b/cloudinit/netinfo.py @@ -141,6 +141,9 @@ def _netdev_info_ifconfig(ifconfig_data): res = re.match(r'.*<(\S+)>', toks[i + 1]) if res: devs[curdev]['ipv6'][-1]['scope6'] = res.group(1) + else: + devs[curdev]['ipv6'][-1]['scope6'] = toks[i + 1] + return devs @@ -389,8 +392,8 @@ def netdev_pformat(): addr.get('scope', empty), data["hwaddr"])) for addr in data.get('ipv6'): tbl.add_row( - (dev, data["up"], addr["ip"], empty, addr["scope6"], - data["hwaddr"])) + (dev, data["up"], addr["ip"], empty, + addr.get("scope6", empty), data["hwaddr"])) if len(data.get('ipv6')) + len(data.get('ipv4')) == 0: tbl.add_row((dev, data["up"], empty, empty, empty, data["hwaddr"])) diff --git a/cloudinit/tests/test_netinfo.py b/cloudinit/tests/test_netinfo.py index d76e768e..1c8a791e 100644 --- a/cloudinit/tests/test_netinfo.py +++ b/cloudinit/tests/test_netinfo.py @@ -11,6 +11,7 @@ from cloudinit.tests.helpers import CiTestCase, mock, readResource # Example ifconfig and route output SAMPLE_OLD_IFCONFIG_OUT = readResource("netinfo/old-ifconfig-output") SAMPLE_NEW_IFCONFIG_OUT = readResource("netinfo/new-ifconfig-output") +SAMPLE_FREEBSD_IFCONFIG_OUT = readResource("netinfo/freebsd-ifconfig-output") SAMPLE_IPADDRSHOW_OUT = readResource("netinfo/sample-ipaddrshow-output") SAMPLE_ROUTE_OUT_V4 = readResource("netinfo/sample-route-output-v4") SAMPLE_ROUTE_OUT_V6 = readResource("netinfo/sample-route-output-v6") @@ -18,6 +19,7 @@ SAMPLE_IPROUTE_OUT_V4 = readResource("netinfo/sample-iproute-output-v4") SAMPLE_IPROUTE_OUT_V6 = readResource("netinfo/sample-iproute-output-v6") NETDEV_FORMATTED_OUT = readResource("netinfo/netdev-formatted-output") ROUTE_FORMATTED_OUT = readResource("netinfo/route-formatted-output") +FREEBSD_NETDEV_OUT = readResource("netinfo/freebsd-netdev-formatted-output") class TestNetInfo(CiTestCase): @@ -43,6 +45,18 @@ class TestNetInfo(CiTestCase): content = netdev_pformat() self.assertEqual(NETDEV_FORMATTED_OUT, content) + @mock.patch('cloudinit.netinfo.util.which') + @mock.patch('cloudinit.netinfo.util.subp') + def test_netdev_freebsd_nettools_pformat(self, m_subp, m_which): + """netdev_pformat properly rendering netdev new nettools info.""" + m_subp.return_value = (SAMPLE_FREEBSD_IFCONFIG_OUT, '') + m_which.side_effect = lambda x: x if x == 'ifconfig' else None + content = netdev_pformat() + print() + print(content) + print() + self.assertEqual(FREEBSD_NETDEV_OUT, content) + @mock.patch('cloudinit.netinfo.util.which') @mock.patch('cloudinit.netinfo.util.subp') def test_netdev_iproute_pformat(self, m_subp, m_which): diff --git a/tests/data/netinfo/freebsd-ifconfig-output b/tests/data/netinfo/freebsd-ifconfig-output new file mode 100644 index 00000000..3de15a5a --- /dev/null +++ b/tests/data/netinfo/freebsd-ifconfig-output @@ -0,0 +1,17 @@ +vtnet0: flags=8843 metric 0 mtu 1500 + options=6c07bb + ether fa:16:3e:14:1f:99 + hwaddr fa:16:3e:14:1f:99 + inet 10.1.80.61 netmask 0xfffff000 broadcast 10.1.95.255 + nd6 options=29 + media: Ethernet 10Gbase-T + status: active +pflog0: flags=0<> metric 0 mtu 33160 +pfsync0: flags=0<> metric 0 mtu 1500 + syncpeer: 0.0.0.0 maxupd: 128 defer: off +lo0: flags=8049 metric 0 mtu 16384 + options=600003 + inet6 ::1 prefixlen 128 + inet6 fe80::1%lo0 prefixlen 64 scopeid 0x4 + inet 127.0.0.1 netmask 0xff000000 + nd6 options=21 diff --git a/tests/data/netinfo/freebsd-netdev-formatted-output b/tests/data/netinfo/freebsd-netdev-formatted-output new file mode 100644 index 00000000..a9d2ac14 --- /dev/null +++ b/tests/data/netinfo/freebsd-netdev-formatted-output @@ -0,0 +1,11 @@ ++++++++++++++++++++++++++++++++Net device info+++++++++++++++++++++++++++++++ ++---------+-------+----------------+------------+-------+-------------------+ +| Device | Up | Address | Mask | Scope | Hw-Address | ++---------+-------+----------------+------------+-------+-------------------+ +| lo0 | True | 127.0.0.1 | 0xff000000 | . | . | +| lo0 | True | ::1/128 | . | . | . | +| lo0 | True | fe80::1%lo0/64 | . | 0x4 | . | +| pflog0 | False | . | . | . | . | +| pfsync0 | False | . | . | . | . | +| vtnet0 | True | 10.1.80.61 | 0xfffff000 | . | fa:16:3e:14:1f:99 | ++---------+-------+----------------+------------+-------+-------------------+ -- cgit v1.2.3 From 1182ad5f9362e1570c622345a3ac996c07eb2eeb Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 26 Feb 2019 15:37:36 +0000 Subject: tests: fix some slow tests and some leaking state In test_ds_identify, don't mutate otherwise-static test data. When running tests in a random order, this was causing failures due to breaking preconditions for other tests. In tests/helpers, reset logging level in tearDown. Some of the CLI tests set the level of the root logger in a way that isn't correctly reset. For test_poll_imds_re_dhcp_on_timeout and test_dhcp_discovery_run_in_sandbox_warns_invalid_pid, mock out time.sleep; this saves ~11 seconds (or ~40% of previous test time!). --- cloudinit/net/tests/test_dhcp.py | 1 + cloudinit/tests/helpers.py | 1 + tests/unittests/test_datasource/test_azure.py | 1 + tests/unittests/test_ds_identify.py | 2 +- 4 files changed, 4 insertions(+), 1 deletion(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index 79e8842f..51390249 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -117,6 +117,7 @@ class TestDHCPDiscoveryClean(CiTestCase): self.assertEqual('eth9', call[0][1]) self.assertIn('/var/tmp/cloud-init/cloud-init-dhcp-', call[0][2]) + @mock.patch('time.sleep', mock.MagicMock()) @mock.patch('cloudinit.net.dhcp.os.kill') @mock.patch('cloudinit.net.dhcp.util.subp') def test_dhcp_discovery_run_in_sandbox_warns_invalid_pid(self, m_subp, diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 2eb7b0cd..46a49416 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -207,6 +207,7 @@ class CiTestCase(TestCase): if self.with_logs: # Remove the handler we setup logging.getLogger().handlers = self.old_handlers + logging.getLogger().level = None util.subp = _real_subp super(CiTestCase, self).tearDown() diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 417d86a9..5edf36e8 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -1692,6 +1692,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): self.paths = helpers.Paths({'cloud_dir': self.tmp}) dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d + @mock.patch('time.sleep', mock.MagicMock()) @mock.patch(MOCKPATH + 'EphemeralDHCPv4') def test_poll_imds_re_dhcp_on_timeout(self, m_dhcpv4, report_ready_func, fake_resp, m_media_switch, m_dhcp, diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index 756b4fb4..d00c1b4b 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -441,7 +441,7 @@ class TestDsIdentify(DsIdentifyBase): nova does not identify itself on platforms other than intel. https://bugs.launchpad.net/cloud-init/+bugs?field.tag=dsid-nova""" - data = VALID_CFG['OpenStack'].copy() + data = copy.deepcopy(VALID_CFG['OpenStack']) del data['files'][P_PRODUCT_NAME] data.update({'policy_dmi': POLICY_FOUND_OR_MAYBE, 'policy_no_dmi': POLICY_FOUND_OR_MAYBE}) -- cgit v1.2.3 From edf052c3196139169ecbfe98049c278f4babc8ca Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Mon, 4 Mar 2019 18:21:59 +0000 Subject: drop Python 2.6 support and our NIH version detection - Remove the last few places that use `if PY26` - Replace our Python version detection logic with six's (which we were already using in most places) --- cloudinit/tests/helpers.py | 22 +--------------------- cloudinit/util.py | 4 ---- tests/unittests/test_datasource/test_azure.py | 4 +--- 3 files changed, 2 insertions(+), 28 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 46a49416..f41180fd 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -41,26 +41,6 @@ _real_subp = util.subp SkipTest = unittest2.SkipTest skipIf = unittest2.skipIf -# Used for detecting different python versions -PY2 = False -PY26 = False -PY27 = False -PY3 = False - -_PY_VER = sys.version_info -_PY_MAJOR, _PY_MINOR, _PY_MICRO = _PY_VER[0:3] -if (_PY_MAJOR, _PY_MINOR) <= (2, 6): - if (_PY_MAJOR, _PY_MINOR) == (2, 6): - PY26 = True - if (_PY_MAJOR, _PY_MINOR) >= (2, 0): - PY2 = True -else: - if (_PY_MAJOR, _PY_MINOR) == (2, 7): - PY27 = True - PY2 = True - if (_PY_MAJOR, _PY_MINOR) >= (3, 0): - PY3 = True - # Makes the old path start # with new base instead of whatever @@ -357,7 +337,7 @@ class FilesystemMockingTestCase(ResourceUsingTestCase): def patchOpen(self, new_root): trap_func = retarget_many_wrapper(new_root, 1, open) - name = 'builtins.open' if PY3 else '__builtin__.open' + name = 'builtins.open' if six.PY3 else '__builtin__.open' self.patched_funcs.enter_context(mock.patch(name, trap_func)) def patchStdoutAndStderr(self, stdout=None, stderr=None): diff --git a/cloudinit/util.py b/cloudinit/util.py index e5403f7d..a192091f 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -72,7 +72,6 @@ CONTAINER_TESTS = (['systemd-detect-virt', '--quiet', '--container'], PROC_CMDLINE = None _LSB_RELEASE = {} -PY26 = sys.version_info[0:2] == (2, 6) def get_architecture(target=None): @@ -2815,9 +2814,6 @@ def load_shell_content(content, add_empty=False, empty_val=None): variables. Set their value to empty_val.""" def _shlex_split(blob): - if PY26 and isinstance(blob, six.text_type): - # Older versions don't support unicode input - blob = blob.encode("utf8") return shlex.split(blob, comments=True) data = {} diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 5edf36e8..6b05b8f1 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -11,7 +11,7 @@ from cloudinit.util import (b64e, decode_binary, load_file, write_file, from cloudinit.version import version_string as vs from cloudinit.tests.helpers import ( HttprettyTestCase, CiTestCase, populate_dir, mock, wrap_and_call, - ExitStack, PY26, SkipTest) + ExitStack) import crypt import httpretty @@ -221,8 +221,6 @@ class TestAzureDataSource(CiTestCase): def setUp(self): super(TestAzureDataSource, self).setUp() - if PY26: - raise SkipTest("Does not work on python 2.6") self.tmp = self.tmp_dir() # patch cloud_dir, so our 'seed_dir' is guaranteed empty -- cgit v1.2.3 From b3a87fc0a2c88585cf77fa9d2756e96183c838f7 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 17 Jul 2019 20:23:42 +0000 Subject: net: update net sequence, include wait on netdevs, opensuse netrules path On systems with many interfaces, processing udev events may take a while. Cloud-init expects devices included in a provided network-configuration to be present when attempting to configure them. This patch adds a step in net configuration where it will check for devices provided in the configuration and if not found, issue udevadm settle commands to wait for them to appear. Additionally, the default path for udev persistent network rules 70-persistent-net.rules may also be written to systems which include the 75-net-generator.rules. During boot, cloud-init and the generator may race and interleave values causing issues. OpenSUSE will now use a newer file, 85-persistent-net-cloud-init.rules which will take precedence over values created by 75-net-generator and avoid collisions on the same file. LP: #1817368 --- cloudinit/distros/opensuse.py | 2 + cloudinit/net/__init__.py | 88 ++++++++++++---- cloudinit/net/tests/test_init.py | 213 +++++++++++++++++++++++++++++++++++++++ cloudinit/stages.py | 27 +++-- cloudinit/tests/test_stages.py | 11 +- tests/unittests/test_net.py | 6 +- 6 files changed, 315 insertions(+), 32 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/distros/opensuse.py b/cloudinit/distros/opensuse.py index 1bfe0478..e41e2f7b 100644 --- a/cloudinit/distros/opensuse.py +++ b/cloudinit/distros/opensuse.py @@ -38,6 +38,8 @@ class Distro(distros.Distro): 'sysconfig': { 'control': 'etc/sysconfig/network/config', 'iface_templates': '%(base)s/network/ifcfg-%(name)s', + 'netrules_path': ( + 'etc/udev/rules.d/85-persistent-net-cloud-init.rules'), 'route_templates': { 'ipv4': '%(base)s/network/ifroute-%(name)s', 'ipv6': '%(base)s/network/ifroute-%(name)s', diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 624c9b42..f3cec794 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -9,6 +9,7 @@ import errno import logging import os import re +from functools import partial from cloudinit.net.network_state import mask_to_net_prefix from cloudinit import util @@ -292,18 +293,10 @@ def generate_fallback_config(blacklist_drivers=None, config_driver=None): return None -def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): - """read the network config and rename devices accordingly. - if strict_present is false, then do not raise exception if no devices - match. if strict_busy is false, then do not raise exception if the - device cannot be renamed because it is currently configured. - - renames are only attempted for interfaces of type 'physical'. It is - expected that the network system will create other devices with the - correct name in place.""" +def extract_physdevs(netcfg): def _version_1(netcfg): - renames = [] + physdevs = [] for ent in netcfg.get('config', {}): if ent.get('type') != 'physical': continue @@ -317,11 +310,11 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): driver = device_driver(name) if not device_id: device_id = device_devid(name) - renames.append([mac, name, driver, device_id]) - return renames + physdevs.append([mac, name, driver, device_id]) + return physdevs def _version_2(netcfg): - renames = [] + physdevs = [] for ent in netcfg.get('ethernets', {}).values(): # only rename if configured to do so name = ent.get('set-name') @@ -337,16 +330,69 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): driver = device_driver(name) if not device_id: device_id = device_devid(name) - renames.append([mac, name, driver, device_id]) - return renames + physdevs.append([mac, name, driver, device_id]) + return physdevs + + version = netcfg.get('version') + if version == 1: + return _version_1(netcfg) + elif version == 2: + return _version_2(netcfg) + + raise RuntimeError('Unknown network config version: %s' % version) + - if netcfg.get('version') == 1: - return _rename_interfaces(_version_1(netcfg)) - elif netcfg.get('version') == 2: - return _rename_interfaces(_version_2(netcfg)) +def wait_for_physdevs(netcfg, strict=True): + physdevs = extract_physdevs(netcfg) + + # set of expected iface names and mac addrs + expected_ifaces = dict([(iface[0], iface[1]) for iface in physdevs]) + expected_macs = set(expected_ifaces.keys()) + + # set of current macs + present_macs = get_interfaces_by_mac().keys() + + # compare the set of expected mac address values to + # the current macs present; we only check MAC as cloud-init + # has not yet renamed interfaces and the netcfg may include + # such renames. + for _ in range(0, 5): + if expected_macs.issubset(present_macs): + LOG.debug('net: all expected physical devices present') + return - raise RuntimeError('Failed to apply network config names. Found bad' - ' network config version: %s' % netcfg.get('version')) + missing = expected_macs.difference(present_macs) + LOG.debug('net: waiting for expected net devices: %s', missing) + for mac in missing: + # trigger a settle, unless this interface exists + syspath = sys_dev_path(expected_ifaces[mac]) + settle = partial(util.udevadm_settle, exists=syspath) + msg = 'Waiting for udev events to settle or %s exists' % syspath + util.log_time(LOG.debug, msg, func=settle) + + # update present_macs after settles + present_macs = get_interfaces_by_mac().keys() + + msg = 'Not all expected physical devices present: %s' % missing + LOG.warning(msg) + if strict: + raise RuntimeError(msg) + + +def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): + """read the network config and rename devices accordingly. + if strict_present is false, then do not raise exception if no devices + match. if strict_busy is false, then do not raise exception if the + device cannot be renamed because it is currently configured. + + renames are only attempted for interfaces of type 'physical'. It is + expected that the network system will create other devices with the + correct name in place.""" + + try: + _rename_interfaces(extract_physdevs(netcfg)) + except RuntimeError as e: + raise RuntimeError('Failed to apply network config names: %s' % e) def interface_has_own_mac(ifname, strict=False): diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index d393e6ad..e6e77d7a 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -708,3 +708,216 @@ class TestHasURLConnectivity(HttprettyTestCase): httpretty.register_uri(httpretty.GET, self.url, body={}, status=404) self.assertFalse( net.has_url_connectivity(self.url), 'Expected False on url fail') + + +def _mk_v1_phys(mac, name, driver, device_id): + v1_cfg = {'type': 'physical', 'name': name, 'mac_address': mac} + params = {} + if driver: + params.update({'driver': driver}) + if device_id: + params.update({'device_id': device_id}) + + if params: + v1_cfg.update({'params': params}) + + return v1_cfg + + +def _mk_v2_phys(mac, name, driver=None, device_id=None): + v2_cfg = {'set-name': name, 'match': {'macaddress': mac}} + if driver: + v2_cfg['match'].update({'driver': driver}) + if device_id: + v2_cfg['match'].update({'device_id': device_id}) + + return v2_cfg + + +class TestExtractPhysdevs(CiTestCase): + + def setUp(self): + super(TestExtractPhysdevs, self).setUp() + self.add_patch('cloudinit.net.device_driver', 'm_driver') + self.add_patch('cloudinit.net.device_devid', 'm_devid') + + def test_extract_physdevs_looks_up_driver_v1(self): + driver = 'virtio' + self.m_driver.return_value = driver + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', None, '0x1000'], + ] + netcfg = { + 'version': 1, + 'config': [_mk_v1_phys(*args) for args in physdevs], + } + # insert the driver value for verification + physdevs[0][2] = driver + self.assertEqual(sorted(physdevs), + sorted(net.extract_physdevs(netcfg))) + self.m_driver.assert_called_with('eth0') + + def test_extract_physdevs_looks_up_driver_v2(self): + driver = 'virtio' + self.m_driver.return_value = driver + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', None, '0x1000'], + ] + netcfg = { + 'version': 2, + 'ethernets': {args[1]: _mk_v2_phys(*args) for args in physdevs}, + } + # insert the driver value for verification + physdevs[0][2] = driver + self.assertEqual(sorted(physdevs), + sorted(net.extract_physdevs(netcfg))) + self.m_driver.assert_called_with('eth0') + + def test_extract_physdevs_looks_up_devid_v1(self): + devid = '0x1000' + self.m_devid.return_value = devid + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', None], + ] + netcfg = { + 'version': 1, + 'config': [_mk_v1_phys(*args) for args in physdevs], + } + # insert the driver value for verification + physdevs[0][3] = devid + self.assertEqual(sorted(physdevs), + sorted(net.extract_physdevs(netcfg))) + self.m_devid.assert_called_with('eth0') + + def test_extract_physdevs_looks_up_devid_v2(self): + devid = '0x1000' + self.m_devid.return_value = devid + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', None], + ] + netcfg = { + 'version': 2, + 'ethernets': {args[1]: _mk_v2_phys(*args) for args in physdevs}, + } + # insert the driver value for verification + physdevs[0][3] = devid + self.assertEqual(sorted(physdevs), + sorted(net.extract_physdevs(netcfg))) + self.m_devid.assert_called_with('eth0') + + def test_get_v1_type_physical(self): + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], + ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], + ['09:87:65:43:21:10', 'ens0p1', 'mlx4_core', '0:0:1000'], + ] + netcfg = { + 'version': 1, + 'config': [_mk_v1_phys(*args) for args in physdevs], + } + self.assertEqual(sorted(physdevs), + sorted(net.extract_physdevs(netcfg))) + + def test_get_v2_type_physical(self): + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], + ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], + ['09:87:65:43:21:10', 'ens0p1', 'mlx4_core', '0:0:1000'], + ] + netcfg = { + 'version': 2, + 'ethernets': {args[1]: _mk_v2_phys(*args) for args in physdevs}, + } + self.assertEqual(sorted(physdevs), + sorted(net.extract_physdevs(netcfg))) + + def test_get_v2_type_physical_skips_if_no_set_name(self): + netcfg = { + 'version': 2, + 'ethernets': { + 'ens3': { + 'match': {'macaddress': '00:11:22:33:44:55'}, + } + } + } + self.assertEqual([], net.extract_physdevs(netcfg)) + + def test_runtime_error_on_unknown_netcfg_version(self): + with self.assertRaises(RuntimeError): + net.extract_physdevs({'version': 3, 'awesome_config': []}) + + +class TestWaitForPhysdevs(CiTestCase): + + with_logs = True + + def setUp(self): + super(TestWaitForPhysdevs, self).setUp() + self.add_patch('cloudinit.net.get_interfaces_by_mac', + 'm_get_iface_mac') + self.add_patch('cloudinit.util.udevadm_settle', 'm_udev_settle') + + def test_wait_for_physdevs_skips_settle_if_all_present(self): + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], + ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], + ] + netcfg = { + 'version': 2, + 'ethernets': {args[1]: _mk_v2_phys(*args) + for args in physdevs}, + } + self.m_get_iface_mac.side_effect = iter([ + {'aa:bb:cc:dd:ee:ff': 'eth0', + '00:11:22:33:44:55': 'ens3'}, + ]) + net.wait_for_physdevs(netcfg) + self.assertEqual(0, self.m_udev_settle.call_count) + + def test_wait_for_physdevs_calls_udev_settle_on_missing(self): + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], + ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], + ] + netcfg = { + 'version': 2, + 'ethernets': {args[1]: _mk_v2_phys(*args) + for args in physdevs}, + } + self.m_get_iface_mac.side_effect = iter([ + {'aa:bb:cc:dd:ee:ff': 'eth0'}, # first call ens3 is missing + {'aa:bb:cc:dd:ee:ff': 'eth0', + '00:11:22:33:44:55': 'ens3'}, # second call has both + ]) + net.wait_for_physdevs(netcfg) + self.m_udev_settle.assert_called_with(exists=net.sys_dev_path('ens3')) + + def test_wait_for_physdevs_raise_runtime_error_if_missing_and_strict(self): + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], + ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], + ] + netcfg = { + 'version': 2, + 'ethernets': {args[1]: _mk_v2_phys(*args) + for args in physdevs}, + } + self.m_get_iface_mac.return_value = {} + with self.assertRaises(RuntimeError): + net.wait_for_physdevs(netcfg) + + self.assertEqual(5 * len(physdevs), self.m_udev_settle.call_count) + + def test_wait_for_physdevs_no_raise_if_not_strict(self): + physdevs = [ + ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], + ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], + ] + netcfg = { + 'version': 2, + 'ethernets': {args[1]: _mk_v2_phys(*args) + for args in physdevs}, + } + 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) diff --git a/cloudinit/stages.py b/cloudinit/stages.py index da7d349a..5f9d47b9 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -644,18 +644,21 @@ class Init(object): return (ncfg, loc) return (self.distro.generate_fallback_config(), "fallback") - def apply_network_config(self, bring_up): - netcfg, src = self._find_networking_config() - if netcfg is None: - LOG.info("network config is disabled by %s", src) - return - + def _apply_netcfg_names(self, netcfg): try: LOG.debug("applying net config names for %s", netcfg) self.distro.apply_network_config_names(netcfg) except Exception as e: LOG.warning("Failed to rename devices: %s", e) + def apply_network_config(self, bring_up): + # get a network config + netcfg, src = self._find_networking_config() + if netcfg is None: + LOG.info("network config is disabled by %s", src) + return + + # request an update if needed/available if self.datasource is not NULL_DATA_SOURCE: if not self.is_new_instance(): if not self.datasource.update_metadata([EventType.BOOT]): @@ -663,8 +666,20 @@ class Init(object): "No network config applied. Neither a new instance" " nor datasource network update on '%s' event", EventType.BOOT) + # nothing new, but ensure proper names + self._apply_netcfg_names(netcfg) return + else: + # refresh netcfg after update + netcfg, src = self._find_networking_config() + + # ensure all physical devices in config are present + net.wait_for_physdevs(netcfg) + + # apply renames from config + self._apply_netcfg_names(netcfg) + # rendering config LOG.info("Applying network configuration from %s bringup=%s: %s", src, bring_up, netcfg) try: diff --git a/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py index 94b6b255..9b483121 100644 --- a/cloudinit/tests/test_stages.py +++ b/cloudinit/tests/test_stages.py @@ -37,6 +37,7 @@ class FakeDataSource(sources.DataSource): class TestInit(CiTestCase): with_logs = True + allowed_subp = False def setUp(self): super(TestInit, self).setUp() @@ -166,8 +167,9 @@ class TestInit(CiTestCase): 'INFO: network config is disabled by %s' % disable_file, self.logs.getvalue()) + @mock.patch('cloudinit.net.get_interfaces_by_mac') @mock.patch('cloudinit.distros.ubuntu.Distro') - def test_apply_network_on_new_instance(self, m_ubuntu): + def test_apply_network_on_new_instance(self, m_ubuntu, m_macs): """Call distro apply_network_config methods on is_new_instance.""" net_cfg = { 'version': 1, 'config': [ @@ -177,6 +179,8 @@ class TestInit(CiTestCase): def fake_network_config(): return net_cfg, 'fallback' + m_macs.return_value = {'42:42:42:42:42:42': 'eth9'} + self.init._find_networking_config = fake_network_config self.init.apply_network_config(True) self.init.distro.apply_network_config_names.assert_called_with(net_cfg) @@ -206,8 +210,9 @@ class TestInit(CiTestCase): " nor datasource network update on '%s' event" % EventType.BOOT, self.logs.getvalue()) + @mock.patch('cloudinit.net.get_interfaces_by_mac') @mock.patch('cloudinit.distros.ubuntu.Distro') - def test_apply_network_on_datasource_allowed_event(self, m_ubuntu): + def test_apply_network_on_datasource_allowed_event(self, m_ubuntu, m_macs): """Apply network if datasource.update_metadata permits BOOT event.""" old_instance_id = os.path.join( self.init.paths.get_cpath('data'), 'instance-id') @@ -220,6 +225,8 @@ class TestInit(CiTestCase): def fake_network_config(): return net_cfg, 'fallback' + m_macs.return_value = {'42:42:42:42:42:42': 'eth9'} + self.init._find_networking_config = fake_network_config self.init.datasource = FakeDataSource(paths=self.init.paths) self.init.datasource.update_events = {'network': [EventType.BOOT]} diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 18efce98..de4e7f4f 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -515,7 +515,7 @@ nameserver 172.19.0.12 [main] dns = none """.lstrip()), - ('etc/udev/rules.d/70-persistent-net.rules', + ('etc/udev/rules.d/85-persistent-net-cloud-init.rules', "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ', 'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))], 'out_sysconfig_rhel': [ @@ -619,7 +619,7 @@ nameserver 172.19.0.12 [main] dns = none """.lstrip()), - ('etc/udev/rules.d/70-persistent-net.rules', + ('etc/udev/rules.d/85-persistent-net-cloud-init.rules', "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ', 'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))], 'out_sysconfig_rhel': [ @@ -750,7 +750,7 @@ nameserver 172.19.0.12 [main] dns = none """.lstrip()), - ('etc/udev/rules.d/70-persistent-net.rules', + ('etc/udev/rules.d/85-persistent-net-cloud-init.rules', "".join(['SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ', 'ATTR{address}=="fa:16:3e:ed:9a:59", NAME="eth0"\n']))], 'out_sysconfig_rhel': [ -- cgit v1.2.3 From 1dbede64dc645b090b4047a105143b5d5090d214 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 23 Jul 2019 22:07:11 +0000 Subject: stages: allow data sources to override network config source order Currently, if a platform provides any network configuration via the "cmdline" method (i.e. network-data=... on the kernel command line, ip=... on the kernel command line, or iBFT config via /run/net-*.conf), the value of the data source's network_config property is completely ignored. This means that on platforms that use iSCSI boot (such as Oracle Compute Infrastructure), there is no way for the data source to configure any network interfaces other than those that have already been configured by the initramfs. This change allows data sources to specify the order in which network configuration sources are considered. Data sources that opt to use this mechanism will be expected to consume the command line network data and integrate it themselves. (The generic merging of network configuration sources was considered, but we concluded that the single use case we have presently (a) didn't warrant the increased complexity, and (b) didn't give us a broad enough view to be sure that our generic implementation would be sufficiently generic. This change in no way precludes a merging strategy in future.) --- cloudinit/sources/__init__.py | 16 ++++++ cloudinit/stages.py | 37 ++++++++++---- cloudinit/tests/test_stages.py | 71 ++++++++++++++++++++++---- tests/unittests/test_datasource/test_common.py | 11 ++++ 4 files changed, 116 insertions(+), 19 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index e6966b31..9d249366 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -66,6 +66,13 @@ CLOUD_ID_REGION_PREFIX_MAP = { 'china': ('azure-china', lambda c: c == 'azure'), # only change azure } +# NetworkConfigSource represents the canonical list of network config sources +# that cloud-init knows about. (Python 2.7 lacks PEP 435, so use a singleton +# namedtuple as an enum; see https://stackoverflow.com/a/6971002) +_NETCFG_SOURCE_NAMES = ('cmdline', 'ds', 'system_cfg', 'fallback') +NetworkConfigSource = namedtuple('NetworkConfigSource', + _NETCFG_SOURCE_NAMES)(*_NETCFG_SOURCE_NAMES) + class DataSourceNotFoundException(Exception): pass @@ -153,6 +160,15 @@ class DataSource(object): # Track the discovered fallback nic for use in configuration generation. _fallback_interface = None + # The network configuration sources that should be considered for this data + # source. (The first source in this list that provides network + # configuration will be used without considering any that follow.) This + # should always be a subset of the members of NetworkConfigSource with no + # duplicate entries. + network_config_sources = (NetworkConfigSource.cmdline, + NetworkConfigSource.system_cfg, + NetworkConfigSource.ds) + # read_url_params url_max_wait = -1 # max_wait < 0 means do not wait url_timeout = 10 # timeout for each metadata url read attempt diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 5f9d47b9..6bcda2d1 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -24,6 +24,7 @@ from cloudinit.handlers.shell_script import ShellScriptPartHandler from cloudinit.handlers.upstart_job import UpstartJobPartHandler from cloudinit.event import EventType +from cloudinit.sources import NetworkConfigSource from cloudinit import cloud from cloudinit import config @@ -630,19 +631,37 @@ class Init(object): if os.path.exists(disable_file): return (None, disable_file) - cmdline_cfg = ('cmdline', cmdline.read_kernel_cmdline_config()) - dscfg = ('ds', None) + available_cfgs = { + NetworkConfigSource.cmdline: cmdline.read_kernel_cmdline_config(), + NetworkConfigSource.ds: None, + NetworkConfigSource.system_cfg: self.cfg.get('network'), + } + if self.datasource and hasattr(self.datasource, 'network_config'): - dscfg = ('ds', self.datasource.network_config) - sys_cfg = ('system_cfg', self.cfg.get('network')) + available_cfgs[NetworkConfigSource.ds] = ( + self.datasource.network_config) - for loc, ncfg in (cmdline_cfg, sys_cfg, dscfg): + if self.datasource: + order = self.datasource.network_config_sources + else: + order = sources.DataSource.network_config_sources + for cfg_source in order: + if not hasattr(NetworkConfigSource, cfg_source): + LOG.warning('data source specifies an invalid network' + ' cfg_source: %s', cfg_source) + continue + if cfg_source not in available_cfgs: + LOG.warning('data source specifies an unavailable network' + ' cfg_source: %s', cfg_source) + continue + ncfg = available_cfgs[cfg_source] if net.is_disabled_cfg(ncfg): - LOG.debug("network config disabled by %s", loc) - return (None, loc) + LOG.debug("network config disabled by %s", cfg_source) + return (None, cfg_source) if ncfg: - return (ncfg, loc) - return (self.distro.generate_fallback_config(), "fallback") + return (ncfg, cfg_source) + return (self.distro.generate_fallback_config(), + NetworkConfigSource.fallback) def _apply_netcfg_names(self, netcfg): try: diff --git a/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py index 9b483121..7e13e29d 100644 --- a/cloudinit/tests/test_stages.py +++ b/cloudinit/tests/test_stages.py @@ -6,6 +6,7 @@ import os from cloudinit import stages from cloudinit import sources +from cloudinit.sources import NetworkConfigSource from cloudinit.event import EventType from cloudinit.util import write_file @@ -63,7 +64,7 @@ class TestInit(CiTestCase): """find_networking_config returns when disabled by kernel cmdline.""" m_cmdline.return_value = {'config': 'disabled'} self.assertEqual( - (None, 'cmdline'), + (None, NetworkConfigSource.cmdline), self.init._find_networking_config()) self.assertEqual('DEBUG: network config disabled by cmdline\n', self.logs.getvalue()) @@ -78,7 +79,7 @@ class TestInit(CiTestCase): self.init.datasource = FakeDataSource( network_config={'config': 'disabled'}) self.assertEqual( - (None, 'ds'), + (None, NetworkConfigSource.ds), self.init._find_networking_config()) self.assertEqual('DEBUG: network config disabled by ds\n', self.logs.getvalue()) @@ -90,11 +91,61 @@ class TestInit(CiTestCase): self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}}, 'network': {'config': 'disabled'}} self.assertEqual( - (None, 'system_cfg'), + (None, NetworkConfigSource.system_cfg), self.init._find_networking_config()) self.assertEqual('DEBUG: network config disabled by system_cfg\n', self.logs.getvalue()) + @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') + def test__find_networking_config_uses_datasrc_order(self, m_cmdline): + """find_networking_config should check sources in DS defined order""" + # cmdline, which would normally be preferred over other sources, + # disables networking; in this case, though, the DS moves cmdline later + # so its own config is preferred + m_cmdline.return_value = {'config': 'disabled'} + + ds_net_cfg = {'config': {'needle': True}} + self.init.datasource = FakeDataSource(network_config=ds_net_cfg) + self.init.datasource.network_config_sources = [ + NetworkConfigSource.ds, NetworkConfigSource.system_cfg, + NetworkConfigSource.cmdline] + + self.assertEqual( + (ds_net_cfg, NetworkConfigSource.ds), + self.init._find_networking_config()) + + @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') + def test__find_networking_config_warns_if_datasrc_uses_invalid_src( + self, m_cmdline): + """find_networking_config should check sources in DS defined order""" + ds_net_cfg = {'config': {'needle': True}} + self.init.datasource = FakeDataSource(network_config=ds_net_cfg) + self.init.datasource.network_config_sources = [ + 'invalid_src', NetworkConfigSource.ds] + + self.assertEqual( + (ds_net_cfg, NetworkConfigSource.ds), + self.init._find_networking_config()) + self.assertIn('WARNING: data source specifies an invalid network' + ' cfg_source: invalid_src', + self.logs.getvalue()) + + @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') + def test__find_networking_config_warns_if_datasrc_uses_unavailable_src( + self, m_cmdline): + """find_networking_config should check sources in DS defined order""" + ds_net_cfg = {'config': {'needle': True}} + self.init.datasource = FakeDataSource(network_config=ds_net_cfg) + self.init.datasource.network_config_sources = [ + NetworkConfigSource.fallback, NetworkConfigSource.ds] + + self.assertEqual( + (ds_net_cfg, NetworkConfigSource.ds), + self.init._find_networking_config()) + self.assertIn('WARNING: data source specifies an unavailable network' + ' cfg_source: fallback', + self.logs.getvalue()) + @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') def test_wb__find_networking_config_returns_kernel(self, m_cmdline): """find_networking_config returns kernel cmdline config if present.""" @@ -105,7 +156,7 @@ class TestInit(CiTestCase): self.init.datasource = FakeDataSource( network_config={'config': ['fakedatasource']}) self.assertEqual( - (expected_cfg, 'cmdline'), + (expected_cfg, NetworkConfigSource.cmdline), self.init._find_networking_config()) @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') @@ -118,7 +169,7 @@ class TestInit(CiTestCase): self.init.datasource = FakeDataSource( network_config={'config': ['fakedatasource']}) self.assertEqual( - (expected_cfg, 'system_cfg'), + (expected_cfg, NetworkConfigSource.system_cfg), self.init._find_networking_config()) @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') @@ -129,7 +180,7 @@ class TestInit(CiTestCase): expected_cfg = {'config': ['fakedatasource']} self.init.datasource = FakeDataSource(network_config=expected_cfg) self.assertEqual( - (expected_cfg, 'ds'), + (expected_cfg, NetworkConfigSource.ds), self.init._find_networking_config()) @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') @@ -148,7 +199,7 @@ class TestInit(CiTestCase): distro = self.init.distro distro.generate_fallback_config = fake_generate_fallback self.assertEqual( - (fake_cfg, 'fallback'), + (fake_cfg, NetworkConfigSource.fallback), self.init._find_networking_config()) self.assertNotIn('network config disabled', self.logs.getvalue()) @@ -177,7 +228,7 @@ class TestInit(CiTestCase): 'name': 'eth9', 'mac_address': '42:42:42:42:42:42'}]} def fake_network_config(): - return net_cfg, 'fallback' + return net_cfg, NetworkConfigSource.fallback m_macs.return_value = {'42:42:42:42:42:42': 'eth9'} @@ -199,7 +250,7 @@ class TestInit(CiTestCase): 'name': 'eth9', 'mac_address': '42:42:42:42:42:42'}]} def fake_network_config(): - return net_cfg, 'fallback' + return net_cfg, NetworkConfigSource.fallback self.init._find_networking_config = fake_network_config self.init.apply_network_config(True) @@ -223,7 +274,7 @@ class TestInit(CiTestCase): 'name': 'eth9', 'mac_address': '42:42:42:42:42:42'}]} def fake_network_config(): - return net_cfg, 'fallback' + return net_cfg, NetworkConfigSource.fallback m_macs.return_value = {'42:42:42:42:42:42': 'eth9'} diff --git a/tests/unittests/test_datasource/test_common.py b/tests/unittests/test_datasource/test_common.py index 6b01a4ea..2a9cfb29 100644 --- a/tests/unittests/test_datasource/test_common.py +++ b/tests/unittests/test_datasource/test_common.py @@ -83,4 +83,15 @@ class ExpectedDataSources(test_helpers.TestCase): self.assertEqual(set([AliYun.DataSourceAliYun]), set(found)) +class TestDataSourceInvariants(test_helpers.TestCase): + + def test_data_sources_have_valid_network_config_sources(self): + for ds in DEFAULT_LOCAL + DEFAULT_NETWORK: + for cfg_src in ds.network_config_sources: + fail_msg = ('{} has an invalid network_config_sources entry:' + ' {}'.format(str(ds), cfg_src)) + self.assertTrue(hasattr(sources.NetworkConfigSource, cfg_src), + fail_msg) + + # vi: ts=4 expandtab -- cgit v1.2.3 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/net/cmdline.py | 25 ++++++---- cloudinit/sources/DataSourceOracle.py | 10 ++-- cloudinit/sources/__init__.py | 3 +- cloudinit/sources/tests/test_oracle.py | 19 ++++---- cloudinit/stages.py | 1 + cloudinit/tests/test_stages.py | 83 ++++++++++++++++++++++++++++------ tests/unittests/test_net.py | 22 ++++----- 7 files changed, 112 insertions(+), 51 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/net/cmdline.py b/cloudinit/net/cmdline.py index f89a0f73..556a10f3 100755 --- a/cloudinit/net/cmdline.py +++ b/cloudinit/net/cmdline.py @@ -177,21 +177,13 @@ def _is_initramfs_netconfig(files, cmdline): return False -def read_kernel_cmdline_config(files=None, mac_addrs=None, cmdline=None): +def read_initramfs_config(files=None, mac_addrs=None, cmdline=None): if cmdline is None: cmdline = util.get_cmdline() if files is None: files = _get_klibc_net_cfg_files() - if 'network-config=' in cmdline: - data64 = None - for tok in cmdline.split(): - if tok.startswith("network-config="): - data64 = tok.split("=", 1)[1] - if data64: - return util.load_yaml(_b64dgz(data64)) - if not _is_initramfs_netconfig(files, cmdline): return None @@ -204,4 +196,19 @@ def read_kernel_cmdline_config(files=None, mac_addrs=None, cmdline=None): return config_from_klibc_net_cfg(files=files, mac_addrs=mac_addrs) + +def read_kernel_cmdline_config(cmdline=None): + if cmdline is None: + cmdline = util.get_cmdline() + + if 'network-config=' in cmdline: + data64 = None + for tok in cmdline.split(): + if tok.startswith("network-config="): + data64 = tok.split("=", 1)[1] + if data64: + return util.load_yaml(_b64dgz(data64)) + + return None + # vi: ts=4 expandtab diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 70b9c58a..76cfa38c 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -48,7 +48,7 @@ class DataSourceOracle(sources.DataSource): return False # network may be configured if iscsi root. If that is the case - # then read_kernel_cmdline_config will return non-None. + # then read_initramfs_config will return non-None. if _is_iscsi_root(): data = self.crawl_metadata() else: @@ -118,10 +118,8 @@ class DataSourceOracle(sources.DataSource): We nonetheless return cmdline provided config if present and fallback to generate fallback.""" if self._network_config == sources.UNSET: - cmdline_cfg = cmdline.read_kernel_cmdline_config() - if cmdline_cfg: - self._network_config = cmdline_cfg - else: + self._network_config = cmdline.read_initramfs_config() + if not self._network_config: self._network_config = self.distro.generate_fallback_config() return self._network_config @@ -137,7 +135,7 @@ def _is_platform_viable(): def _is_iscsi_root(): - return bool(cmdline.read_kernel_cmdline_config()) + return bool(cmdline.read_initramfs_config()) def _load_index(content): diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 9d249366..c2baccd5 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -69,7 +69,7 @@ CLOUD_ID_REGION_PREFIX_MAP = { # NetworkConfigSource represents the canonical list of network config sources # that cloud-init knows about. (Python 2.7 lacks PEP 435, so use a singleton # namedtuple as an enum; see https://stackoverflow.com/a/6971002) -_NETCFG_SOURCE_NAMES = ('cmdline', 'ds', 'system_cfg', 'fallback') +_NETCFG_SOURCE_NAMES = ('cmdline', 'ds', 'system_cfg', 'fallback', 'initramfs') NetworkConfigSource = namedtuple('NetworkConfigSource', _NETCFG_SOURCE_NAMES)(*_NETCFG_SOURCE_NAMES) @@ -166,6 +166,7 @@ class DataSource(object): # should always be a subset of the members of NetworkConfigSource with no # duplicate entries. network_config_sources = (NetworkConfigSource.cmdline, + NetworkConfigSource.initramfs, NetworkConfigSource.system_cfg, NetworkConfigSource.ds) 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())) diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 6bcda2d1..50129884 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -633,6 +633,7 @@ class Init(object): available_cfgs = { NetworkConfigSource.cmdline: cmdline.read_kernel_cmdline_config(), + NetworkConfigSource.initramfs: cmdline.read_initramfs_config(), NetworkConfigSource.ds: None, NetworkConfigSource.system_cfg: self.cfg.get('network'), } diff --git a/cloudinit/tests/test_stages.py b/cloudinit/tests/test_stages.py index 7e13e29d..d5c9c0e4 100644 --- a/cloudinit/tests/test_stages.py +++ b/cloudinit/tests/test_stages.py @@ -59,20 +59,39 @@ class TestInit(CiTestCase): (None, disable_file), self.init._find_networking_config()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test_wb__find_networking_config_disabled_by_kernel(self, m_cmdline): + def test_wb__find_networking_config_disabled_by_kernel( + self, m_cmdline, m_initramfs): """find_networking_config returns when disabled by kernel cmdline.""" m_cmdline.return_value = {'config': 'disabled'} + m_initramfs.return_value = {'config': ['fake_initrd']} self.assertEqual( (None, NetworkConfigSource.cmdline), self.init._find_networking_config()) self.assertEqual('DEBUG: network config disabled by cmdline\n', self.logs.getvalue()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test_wb__find_networking_config_disabled_by_datasrc(self, m_cmdline): + def test_wb__find_networking_config_disabled_by_initrd( + self, m_cmdline, m_initramfs): + """find_networking_config returns when disabled by kernel cmdline.""" + m_cmdline.return_value = {} + m_initramfs.return_value = {'config': 'disabled'} + self.assertEqual( + (None, NetworkConfigSource.initramfs), + self.init._find_networking_config()) + self.assertEqual('DEBUG: network config disabled by initramfs\n', + self.logs.getvalue()) + + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') + @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') + def test_wb__find_networking_config_disabled_by_datasrc( + self, m_cmdline, m_initramfs): """find_networking_config returns when disabled by datasource cfg.""" m_cmdline.return_value = {} # Kernel doesn't disable networking + m_initramfs.return_value = {} # initramfs doesn't disable networking self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}}, 'network': {}} # system config doesn't disable @@ -84,10 +103,13 @@ class TestInit(CiTestCase): self.assertEqual('DEBUG: network config disabled by ds\n', self.logs.getvalue()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test_wb__find_networking_config_disabled_by_sysconfig(self, m_cmdline): + def test_wb__find_networking_config_disabled_by_sysconfig( + self, m_cmdline, m_initramfs): """find_networking_config returns when disabled by system config.""" m_cmdline.return_value = {} # Kernel doesn't disable networking + m_initramfs.return_value = {} # initramfs doesn't disable networking self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}}, 'network': {'config': 'disabled'}} self.assertEqual( @@ -96,27 +118,31 @@ class TestInit(CiTestCase): self.assertEqual('DEBUG: network config disabled by system_cfg\n', self.logs.getvalue()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test__find_networking_config_uses_datasrc_order(self, m_cmdline): + def test__find_networking_config_uses_datasrc_order( + self, m_cmdline, m_initramfs): """find_networking_config should check sources in DS defined order""" - # cmdline, which would normally be preferred over other sources, - # disables networking; in this case, though, the DS moves cmdline later - # so its own config is preferred + # cmdline and initramfs, which would normally be preferred over other + # sources, disable networking; in this case, though, the DS moves them + # later so its own config is preferred m_cmdline.return_value = {'config': 'disabled'} + m_initramfs.return_value = {'config': 'disabled'} ds_net_cfg = {'config': {'needle': True}} self.init.datasource = FakeDataSource(network_config=ds_net_cfg) self.init.datasource.network_config_sources = [ NetworkConfigSource.ds, NetworkConfigSource.system_cfg, - NetworkConfigSource.cmdline] + NetworkConfigSource.cmdline, NetworkConfigSource.initramfs] self.assertEqual( (ds_net_cfg, NetworkConfigSource.ds), self.init._find_networking_config()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') def test__find_networking_config_warns_if_datasrc_uses_invalid_src( - self, m_cmdline): + self, m_cmdline, m_initramfs): """find_networking_config should check sources in DS defined order""" ds_net_cfg = {'config': {'needle': True}} self.init.datasource = FakeDataSource(network_config=ds_net_cfg) @@ -130,9 +156,10 @@ class TestInit(CiTestCase): ' cfg_source: invalid_src', self.logs.getvalue()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') def test__find_networking_config_warns_if_datasrc_uses_unavailable_src( - self, m_cmdline): + self, m_cmdline, m_initramfs): """find_networking_config should check sources in DS defined order""" ds_net_cfg = {'config': {'needle': True}} self.init.datasource = FakeDataSource(network_config=ds_net_cfg) @@ -146,11 +173,14 @@ class TestInit(CiTestCase): ' cfg_source: fallback', self.logs.getvalue()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test_wb__find_networking_config_returns_kernel(self, m_cmdline): + def test_wb__find_networking_config_returns_kernel( + self, m_cmdline, m_initramfs): """find_networking_config returns kernel cmdline config if present.""" expected_cfg = {'config': ['fakekernel']} m_cmdline.return_value = expected_cfg + m_initramfs.return_value = {'config': ['fake_initrd']} self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}}, 'network': {'config': ['fakesys_config']}} self.init.datasource = FakeDataSource( @@ -159,10 +189,29 @@ class TestInit(CiTestCase): (expected_cfg, NetworkConfigSource.cmdline), self.init._find_networking_config()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') + @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') + def test_wb__find_networking_config_returns_initramfs( + self, m_cmdline, m_initramfs): + """find_networking_config returns kernel cmdline config if present.""" + expected_cfg = {'config': ['fake_initrd']} + m_cmdline.return_value = {} + m_initramfs.return_value = expected_cfg + self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}}, + 'network': {'config': ['fakesys_config']}} + self.init.datasource = FakeDataSource( + network_config={'config': ['fakedatasource']}) + self.assertEqual( + (expected_cfg, NetworkConfigSource.initramfs), + self.init._find_networking_config()) + + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test_wb__find_networking_config_returns_system_cfg(self, m_cmdline): + def test_wb__find_networking_config_returns_system_cfg( + self, m_cmdline, m_initramfs): """find_networking_config returns system config when present.""" m_cmdline.return_value = {} # No kernel network config + m_initramfs.return_value = {} # no initramfs network config expected_cfg = {'config': ['fakesys_config']} self.init._cfg = {'system_info': {'paths': {'cloud_dir': self.tmpdir}}, 'network': expected_cfg} @@ -172,10 +221,13 @@ class TestInit(CiTestCase): (expected_cfg, NetworkConfigSource.system_cfg), self.init._find_networking_config()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test_wb__find_networking_config_returns_datasrc_cfg(self, m_cmdline): + def test_wb__find_networking_config_returns_datasrc_cfg( + self, m_cmdline, m_initramfs): """find_networking_config returns datasource net config if present.""" m_cmdline.return_value = {} # No kernel network config + m_initramfs.return_value = {} # no initramfs network config # No system config for network in setUp expected_cfg = {'config': ['fakedatasource']} self.init.datasource = FakeDataSource(network_config=expected_cfg) @@ -183,10 +235,13 @@ class TestInit(CiTestCase): (expected_cfg, NetworkConfigSource.ds), self.init._find_networking_config()) + @mock.patch('cloudinit.stages.cmdline.read_initramfs_config') @mock.patch('cloudinit.stages.cmdline.read_kernel_cmdline_config') - def test_wb__find_networking_config_returns_fallback(self, m_cmdline): + def test_wb__find_networking_config_returns_fallback( + self, m_cmdline, m_initramfs): """find_networking_config returns fallback config if not defined.""" m_cmdline.return_value = {} # Kernel doesn't disable networking + m_initramfs.return_value = {} # no initramfs network config # Neither datasource nor system_info disable or provide network fake_cfg = {'config': [{'type': 'physical', 'name': 'eth9'}], diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index e2bbb847..1840ade0 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -3558,13 +3558,13 @@ class TestCmdlineConfigParsing(CiTestCase): self.assertEqual(found, self.simple_cfg) -class TestCmdlineReadKernelConfig(FilesystemMockingTestCase): +class TestCmdlineReadInitramfsConfig(FilesystemMockingTestCase): macs = { 'eth0': '14:02:ec:42:48:00', 'eno1': '14:02:ec:42:48:01', } - def test_ip_cmdline_without_ip(self): + def test_without_ip(self): content = {'/run/net-eth0.conf': DHCP_CONTENT_1, cmdline._OPEN_ISCSI_INTERFACE_FILE: "eth0\n"} exp1 = copy.deepcopy(DHCP_EXPECTED_1) @@ -3574,12 +3574,12 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase): populate_dir(root, content) self.reRoot(root) - found = cmdline.read_kernel_cmdline_config( + found = cmdline.read_initramfs_config( cmdline='foo root=/root/bar', mac_addrs=self.macs) self.assertEqual(found['version'], 1) self.assertEqual(found['config'], [exp1]) - def test_ip_cmdline_read_kernel_cmdline_ip(self): + def test_with_ip(self): content = {'/run/net-eth0.conf': DHCP_CONTENT_1} exp1 = copy.deepcopy(DHCP_EXPECTED_1) exp1['mac_address'] = self.macs['eth0'] @@ -3588,18 +3588,18 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase): populate_dir(root, content) self.reRoot(root) - found = cmdline.read_kernel_cmdline_config( + found = cmdline.read_initramfs_config( cmdline='foo ip=dhcp', mac_addrs=self.macs) self.assertEqual(found['version'], 1) self.assertEqual(found['config'], [exp1]) - def test_ip_cmdline_read_kernel_cmdline_ip6(self): + def test_with_ip6(self): content = {'/run/net6-eno1.conf': DHCP6_CONTENT_1} root = self.tmp_dir() populate_dir(root, content) self.reRoot(root) - found = cmdline.read_kernel_cmdline_config( + found = cmdline.read_initramfs_config( cmdline='foo ip6=dhcp root=/dev/sda', mac_addrs=self.macs) self.assertEqual( @@ -3611,15 +3611,15 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase): {'dns_nameservers': ['2001:67c:1562:8010::2:1'], 'control': 'manual', 'type': 'dhcp6', 'netmask': '64'}]}]}) - def test_ip_cmdline_read_kernel_cmdline_none(self): + def test_with_no_ip_or_ip6(self): # if there is no ip= or ip6= on cmdline, return value should be None content = {'net6-eno1.conf': DHCP6_CONTENT_1} files = sorted(populate_dir(self.tmp_dir(), content)) - found = cmdline.read_kernel_cmdline_config( + found = cmdline.read_initramfs_config( files=files, cmdline='foo root=/dev/sda', mac_addrs=self.macs) self.assertIsNone(found) - def test_ip_cmdline_both_ip_ip6(self): + def test_with_both_ip_ip6(self): content = { '/run/net-eth0.conf': DHCP_CONTENT_1, '/run/net6-eth0.conf': DHCP6_CONTENT_1.replace('eno1', 'eth0')} @@ -3634,7 +3634,7 @@ class TestCmdlineReadKernelConfig(FilesystemMockingTestCase): populate_dir(root, content) self.reRoot(root) - found = cmdline.read_kernel_cmdline_config( + found = cmdline.read_initramfs_config( cmdline='foo ip=dhcp ip6=dhcp', mac_addrs=self.macs) self.assertEqual(found['version'], 1) -- cgit v1.2.3 From e6383719c1adccf20b5c21cfba1c5a2462d663a2 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 22 Aug 2019 17:09:22 +0000 Subject: ubuntu-drivers: call db_x_loadtemplatefile to accept NVIDIA EULA Emit a script allowing cloud-init to set linux/nvidia/latelink debconf selection to true. This avoids having to call debconf-set-selections and allows cloud-init to pre-confgure linux-restricted-modules to link NVIDIA drivers to the running kernel. Cloud-init loads this debconf template and sets the value to true in the debconf database by sourcing debconf's /usr/share/debconf/confmodule and uses db_x_loadtemplatefile to register cloud-init's setting for linux/nvidia/latelink. LP: #1840080 --- cloudinit/config/cc_ubuntu_drivers.py | 58 +++++++++++--- cloudinit/config/tests/test_ubuntu_drivers.py | 105 ++++++++++++++++++++------ cloudinit/tests/helpers.py | 3 +- 3 files changed, 130 insertions(+), 36 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py index 4da34ee0..297451d6 100644 --- a/cloudinit/config/cc_ubuntu_drivers.py +++ b/cloudinit/config/cc_ubuntu_drivers.py @@ -2,13 +2,14 @@ """Ubuntu Drivers: Interact with third party drivers in Ubuntu.""" +import os from textwrap import dedent -from cloudinit.config import cc_apt_configure from cloudinit.config.schema import ( get_schema_doc, validate_cloudconfig_schema) from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE +from cloudinit import temp_utils from cloudinit import type_utils from cloudinit import util @@ -65,6 +66,33 @@ OLD_UBUNTU_DRIVERS_STDERR_NEEDLE = ( __doc__ = get_schema_doc(schema) # Supplement python help() +# Use a debconf template to configure a global debconf variable +# (linux/nvidia/latelink) setting this to "true" allows the +# 'linux-restricted-modules' deb to accept the NVIDIA EULA and the package +# will automatically link the drivers to the running kernel. + +# EOL_XENIAL: can then drop this script and use python3-debconf which is only +# available in Bionic and later. Can't use python3-debconf currently as it +# isn't in Xenial and doesn't yet support X_LOADTEMPLATEFILE debconf command. + +NVIDIA_DEBCONF_CONTENT = """\ +Template: linux/nvidia/latelink +Type: boolean +Default: true +Description: Late-link NVIDIA kernel modules? + Enable this to link the NVIDIA kernel modules in cloud-init and + make them available for use. +""" + +NVIDIA_DRIVER_LATELINK_DEBCONF_SCRIPT = """\ +#!/bin/sh +# Allow cloud-init to trigger EULA acceptance via registering a debconf +# template to set linux/nvidia/latelink true +. /usr/share/debconf/confmodule +db_x_loadtemplatefile "$1" cloud-init +""" + + def install_drivers(cfg, pkg_install_func): if not isinstance(cfg, dict): raise TypeError( @@ -90,17 +118,27 @@ def install_drivers(cfg, pkg_install_func): if version_cfg: driver_arg += ':{}'.format(version_cfg) - LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)", + LOG.debug("Installing and activating NVIDIA drivers (%s=%s, version=%s)", cfgpath, nv_acc, version_cfg if version_cfg else 'latest') - # Setting NVIDIA latelink confirms acceptance of EULA for the package - # linux-restricted-modules - # Reference code defining debconf variable is here - # https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/ - # linux-restricted-modules/+git/eoan/tree/debian/templates/ - # nvidia.templates.in - selections = b'linux-restricted-modules linux/nvidia/latelink boolean true' - cc_apt_configure.debconf_set_selections(selections) + # Register and set debconf selection linux/nvidia/latelink = true + tdir = temp_utils.mkdtemp(needs_exe=True) + debconf_file = os.path.join(tdir, 'nvidia.template') + debconf_script = os.path.join(tdir, 'nvidia-debconf.sh') + try: + util.write_file(debconf_file, NVIDIA_DEBCONF_CONTENT) + util.write_file( + debconf_script, + util.encode_text(NVIDIA_DRIVER_LATELINK_DEBCONF_SCRIPT), + mode=0o755) + util.subp([debconf_script, debconf_file]) + except Exception as e: + util.logexc( + LOG, "Failed to register NVIDIA debconf template: %s", str(e)) + raise + finally: + if os.path.isdir(tdir): + util.del_dir(tdir) try: util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg]) diff --git a/cloudinit/config/tests/test_ubuntu_drivers.py b/cloudinit/config/tests/test_ubuntu_drivers.py index 6a763bd9..46952692 100644 --- a/cloudinit/config/tests/test_ubuntu_drivers.py +++ b/cloudinit/config/tests/test_ubuntu_drivers.py @@ -1,6 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. import copy +import os from cloudinit.tests.helpers import CiTestCase, skipUnlessJsonSchema, mock from cloudinit.config.schema import ( @@ -9,11 +10,27 @@ from cloudinit.config import cc_ubuntu_drivers as drivers from cloudinit.util import ProcessExecutionError MPATH = "cloudinit.config.cc_ubuntu_drivers." +M_TMP_PATH = MPATH + "temp_utils.mkdtemp" OLD_UBUNTU_DRIVERS_ERROR_STDERR = ( "ubuntu-drivers: error: argument : invalid choice: 'install' " "(choose from 'list', 'autoinstall', 'devices', 'debug')\n") +class AnyTempScriptAndDebconfFile(object): + + def __init__(self, tmp_dir, debconf_file): + self.tmp_dir = tmp_dir + self.debconf_file = debconf_file + + def __eq__(self, cmd): + if not len(cmd) == 2: + return False + script, debconf_file = cmd + if bool(script.startswith(self.tmp_dir) and script.endswith('.sh')): + return debconf_file == self.debconf_file + return False + + class TestUbuntuDrivers(CiTestCase): cfg_accepted = {'drivers': {'nvidia': {'license-accepted': True}}} install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia'] @@ -28,20 +45,23 @@ class TestUbuntuDrivers(CiTestCase): {'drivers': {'nvidia': {'license-accepted': "TRUE"}}}, schema=drivers.schema, strict=True) - @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") + @mock.patch(M_TMP_PATH) @mock.patch(MPATH + "util.subp", return_value=('', '')) @mock.patch(MPATH + "util.which", return_value=False) def _assert_happy_path_taken( - self, config, m_which, m_subp, m_debconf_set_selections): + self, config, m_which, m_subp, m_tmp): """Positive path test through handle. Package should be installed.""" + tdir = self.tmp_dir() + debconf_file = os.path.join(tdir, 'nvidia.template') + m_tmp.return_value = tdir myCloud = mock.MagicMock() drivers.handle('ubuntu_drivers', config, myCloud, None, None) self.assertEqual([mock.call(['ubuntu-drivers-common'])], myCloud.distro.install_packages.call_args_list) - self.assertEqual([mock.call(self.install_gpgpu)], - m_subp.call_args_list) - m_debconf_set_selections.assert_called_with( - b'linux-restricted-modules linux/nvidia/latelink boolean true') + self.assertEqual( + [mock.call(AnyTempScriptAndDebconfFile(tdir, debconf_file)), + mock.call(self.install_gpgpu)], + m_subp.call_args_list) def test_handle_does_package_install(self): self._assert_happy_path_taken(self.cfg_accepted) @@ -52,20 +72,33 @@ class TestUbuntuDrivers(CiTestCase): new_config['drivers']['nvidia']['license-accepted'] = true_value self._assert_happy_path_taken(new_config) - @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") - @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( - stdout='No drivers found for installation.\n', exit_code=1)) + @mock.patch(M_TMP_PATH) + @mock.patch(MPATH + "util.subp") @mock.patch(MPATH + "util.which", return_value=False) - def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp, _): + def test_handle_raises_error_if_no_drivers_found( + self, m_which, m_subp, m_tmp): """If ubuntu-drivers doesn't install any drivers, raise an error.""" + tdir = self.tmp_dir() + debconf_file = os.path.join(tdir, 'nvidia.template') + m_tmp.return_value = tdir myCloud = mock.MagicMock() + + def fake_subp(cmd): + if cmd[0].startswith(tdir): + return + raise ProcessExecutionError( + stdout='No drivers found for installation.\n', exit_code=1) + m_subp.side_effect = fake_subp + with self.assertRaises(Exception): drivers.handle( 'ubuntu_drivers', self.cfg_accepted, myCloud, None, None) self.assertEqual([mock.call(['ubuntu-drivers-common'])], myCloud.distro.install_packages.call_args_list) - self.assertEqual([mock.call(self.install_gpgpu)], - m_subp.call_args_list) + self.assertEqual( + [mock.call(AnyTempScriptAndDebconfFile(tdir, debconf_file)), + mock.call(self.install_gpgpu)], + m_subp.call_args_list) self.assertIn('ubuntu-drivers found no drivers for installation', self.logs.getvalue()) @@ -113,19 +146,25 @@ class TestUbuntuDrivers(CiTestCase): myLog.debug.call_args_list[0][0][0]) self.assertEqual(0, m_install_drivers.call_count) - @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") + @mock.patch(M_TMP_PATH) @mock.patch(MPATH + "util.subp", return_value=('', '')) @mock.patch(MPATH + "util.which", return_value=True) - def test_install_drivers_no_install_if_present(self, m_which, m_subp, _): + def test_install_drivers_no_install_if_present( + self, m_which, m_subp, m_tmp): """If 'ubuntu-drivers' is present, no package install should occur.""" + tdir = self.tmp_dir() + debconf_file = os.path.join(tdir, 'nvidia.template') + m_tmp.return_value = tdir pkg_install = mock.MagicMock() drivers.install_drivers(self.cfg_accepted['drivers'], pkg_install_func=pkg_install) self.assertEqual(0, pkg_install.call_count) self.assertEqual([mock.call('ubuntu-drivers')], m_which.call_args_list) - self.assertEqual([mock.call(self.install_gpgpu)], - m_subp.call_args_list) + self.assertEqual( + [mock.call(AnyTempScriptAndDebconfFile(tdir, debconf_file)), + mock.call(self.install_gpgpu)], + m_subp.call_args_list) def test_install_drivers_rejects_invalid_config(self): """install_drivers should raise TypeError if not given a config dict""" @@ -134,21 +173,33 @@ class TestUbuntuDrivers(CiTestCase): drivers.install_drivers("mystring", pkg_install_func=pkg_install) self.assertEqual(0, pkg_install.call_count) - @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") - @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( - stderr=OLD_UBUNTU_DRIVERS_ERROR_STDERR, exit_code=2)) + @mock.patch(M_TMP_PATH) + @mock.patch(MPATH + "util.subp") @mock.patch(MPATH + "util.which", return_value=False) def test_install_drivers_handles_old_ubuntu_drivers_gracefully( - self, m_which, m_subp, _): + self, m_which, m_subp, m_tmp): """Older ubuntu-drivers versions should emit message and raise error""" + tdir = self.tmp_dir() + debconf_file = os.path.join(tdir, 'nvidia.template') + m_tmp.return_value = tdir myCloud = mock.MagicMock() + + def fake_subp(cmd): + if cmd[0].startswith(tdir): + return + raise ProcessExecutionError( + stderr=OLD_UBUNTU_DRIVERS_ERROR_STDERR, exit_code=2) + m_subp.side_effect = fake_subp + with self.assertRaises(Exception): drivers.handle( 'ubuntu_drivers', self.cfg_accepted, myCloud, None, None) self.assertEqual([mock.call(['ubuntu-drivers-common'])], myCloud.distro.install_packages.call_args_list) - self.assertEqual([mock.call(self.install_gpgpu)], - m_subp.call_args_list) + self.assertEqual( + [mock.call(AnyTempScriptAndDebconfFile(tdir, debconf_file)), + mock.call(self.install_gpgpu)], + m_subp.call_args_list) self.assertIn('WARNING: the available version of ubuntu-drivers is' ' too old to perform requested driver installation', self.logs.getvalue()) @@ -160,17 +211,21 @@ class TestUbuntuDriversWithVersion(TestUbuntuDrivers): 'drivers': {'nvidia': {'license-accepted': True, 'version': '123'}}} install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia:123'] - @mock.patch(MPATH + "cc_apt_configure.debconf_set_selections") + @mock.patch(M_TMP_PATH) @mock.patch(MPATH + "util.subp", return_value=('', '')) @mock.patch(MPATH + "util.which", return_value=False) - def test_version_none_uses_latest(self, m_which, m_subp, _): + def test_version_none_uses_latest(self, m_which, m_subp, m_tmp): + tdir = self.tmp_dir() + debconf_file = os.path.join(tdir, 'nvidia.template') + m_tmp.return_value = tdir myCloud = mock.MagicMock() version_none_cfg = { 'drivers': {'nvidia': {'license-accepted': True, 'version': None}}} drivers.handle( 'ubuntu_drivers', version_none_cfg, myCloud, None, None) self.assertEqual( - [mock.call(['ubuntu-drivers', 'install', '--gpgpu', 'nvidia'])], + [mock.call(AnyTempScriptAndDebconfFile(tdir, debconf_file)), + mock.call(['ubuntu-drivers', 'install', '--gpgpu', 'nvidia'])], m_subp.call_args_list) def test_specifying_a_version_doesnt_override_license_acceptance(self): diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index f41180fd..23fddd07 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -198,7 +198,8 @@ class CiTestCase(TestCase): prefix="ci-%s." % self.__class__.__name__) else: tmpd = tempfile.mkdtemp(dir=dir) - self.addCleanup(functools.partial(shutil.rmtree, tmpd)) + self.addCleanup( + functools.partial(shutil.rmtree, tmpd, ignore_errors=True)) return tmpd def tmp_path(self, path, dir=None): -- 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/tests') 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 067516d7bc917e4921b9f1424b7a64e92cae0ad2 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 27 Sep 2019 20:46:00 +0000 Subject: util: json.dumps on python 2.7 will handle UnicodeDecodeError on binary Since python 2.7 doesn't handle UnicodeDecodeErrors with the default handler LP: #1801364 --- cloudinit/sources/tests/test_init.py | 12 +++++------- cloudinit/tests/test_util.py | 20 ++++++++++++++++++++ cloudinit/util.py | 27 +++++++++++++++++++++++++-- 3 files changed, 50 insertions(+), 9 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 6378e98b..9698261b 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -457,19 +457,17 @@ class TestDataSource(CiTestCase): instance_json['ds']['meta_data']) @skipIf(not six.PY2, "Only python2 hits UnicodeDecodeErrors on non-utf8") - def test_non_utf8_encoding_logs_warning(self): - """When non-utf-8 values exist in py2 instance-data is not written.""" + 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) - self.assertFalse(os.path.exists(json_file)) - self.assertIn( - "WARNING: Error persisting instance-data.json: 'utf8' codec can't" - " decode byte 0xaa in position 2: invalid start byte", - self.logs.getvalue()) + 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.""" diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index e3d2dbaa..f4f95e92 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -2,7 +2,9 @@ """Tests for cloudinit.util""" +import base64 import logging +import json import platform import cloudinit.util as util @@ -528,6 +530,24 @@ class TestGetLinuxDistro(CiTestCase): self.assertEqual(('foo', '1.1', 'aarch64'), dist) +class TestJsonDumps(CiTestCase): + def test_is_str(self): + """json_dumps should return a string.""" + self.assertTrue(isinstance(util.json_dumps({'abc': '123'}), str)) + + def test_utf8(self): + smiley = '\\ud83d\\ude03' + self.assertEqual( + {'smiley': smiley}, + json.loads(util.json_dumps({'smiley': smiley}))) + + def test_non_utf8(self): + blob = b'\xba\x03Qx-#y\xea' + self.assertEqual( + {'blob': 'ci-b64:' + base64.b64encode(blob).decode('utf-8')}, + json.loads(util.json_dumps({'blob': blob}))) + + @mock.patch('os.path.exists') class TestIsLXD(CiTestCase): diff --git a/cloudinit/util.py b/cloudinit/util.py index aa23b3f3..6e8e73b0 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1599,10 +1599,33 @@ def json_serialize_default(_obj): return 'Warning: redacted unserializable type {0}'.format(type(_obj)) +def json_preserialize_binary(data): + """Preserialize any discovered binary values to avoid json.dumps issues. + + Used only on python 2.7 where default type handling is not honored for + failure to encode binary data. LP: #1801364. + TODO(Drop this function when py2.7 support is dropped from cloud-init) + """ + data = obj_copy.deepcopy(data) + for key, value in data.items(): + if isinstance(value, (dict)): + data[key] = json_preserialize_binary(value) + if isinstance(value, bytes): + data[key] = 'ci-b64:{0}'.format(b64e(value)) + return data + + def json_dumps(data): """Return data in nicely formatted json.""" - return json.dumps(data, indent=1, sort_keys=True, - separators=(',', ': '), default=json_serialize_default) + try: + return json.dumps( + data, indent=1, sort_keys=True, separators=(',', ': '), + default=json_serialize_default) + except UnicodeDecodeError: + if sys.version_info[:2] == (2, 7): + data = json_preserialize_binary(data) + return json.dumps(data) + raise def yaml_dumps(obj, explicit_start=True, explicit_end=True, noalias=False): -- cgit v1.2.3 From 2a135c4a421af47f5bd511e89e385a72f62bde33 Mon Sep 17 00:00:00 2001 From: Igor Galić Date: Mon, 25 Nov 2019 23:10:50 +0100 Subject: FreeBSD: fix for get_linux_distro() and lru_cache (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since `is_FreeBSD()` is used a lot, which uses `system_info()`, which uses `get_linux_distro()` we add caching, by decorating the following functions with `@lru_cache`: - get_architecture() - _lsb_release() - is_FreeBSD - get_linux_distro - system_info() - _get_cmdline() Since [functools](https://docs.python.org/3/library/functools.html) only exists in Python 3, only python 3 will benefit from this improvement. For python 2, our shim is just a pass-thru. Too bad, but, also… https://pythonclock.org/ The main motivation here was, at first, to cache more, following the style of _lsb_release. That is now consolidated under this very same roof. LP: #1815030 --- cloudinit/tests/test_util.py | 19 ++++++++++++++++++ cloudinit/util.py | 47 +++++++++++++++++++++++++++----------------- tests/unittests/test_net.py | 19 ++++++++++++------ 3 files changed, 61 insertions(+), 24 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index f4f95e92..64ed82ea 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -387,6 +387,11 @@ class TestUdevadmSettle(CiTestCase): @mock.patch('os.path.exists') class TestGetLinuxDistro(CiTestCase): + def setUp(self): + # python2 has no lru_cache, and therefore, no cache_clear() + if hasattr(util.get_linux_distro, "cache_clear"): + util.get_linux_distro.cache_clear() + @classmethod def os_release_exists(self, path): """Side effect function""" @@ -399,6 +404,12 @@ class TestGetLinuxDistro(CiTestCase): if path == '/etc/redhat-release': return 1 + @classmethod + def freebsd_version_exists(self, path): + """Side effect function """ + if path == '/bin/freebsd-version': + return 1 + @mock.patch('cloudinit.util.load_file') def test_get_linux_distro_quoted_name(self, m_os_release, m_path_exists): """Verify we get the correct name if the os-release file has @@ -417,6 +428,14 @@ class TestGetLinuxDistro(CiTestCase): dist = util.get_linux_distro() self.assertEqual(('ubuntu', '16.04', 'xenial'), dist) + @mock.patch('cloudinit.util.subp') + def test_get_linux_freebsd(self, m_subp, m_path_exists): + """Verify we get the correct name and release name on FreeBSD.""" + m_path_exists.side_effect = TestGetLinuxDistro.freebsd_version_exists + m_subp.return_value = ("12.0-RELEASE-p10\n", '') + dist = util.get_linux_distro() + self.assertEqual(('freebsd', '12.0-RELEASE-p10', ''), dist) + @mock.patch('cloudinit.util.load_file') def test_get_linux_centos6(self, m_os_release, m_path_exists): """Verify we get the correct name and release name on CentOS 6.""" diff --git a/cloudinit/util.py b/cloudinit/util.py index 1f600df4..c498414d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -50,6 +50,16 @@ from cloudinit import version from cloudinit.settings import (CFG_BUILTIN) +try: + from functools import lru_cache +except ImportError: + def lru_cache(): + """pass-thru replace for Python3's lru_cache()""" + def wrapper(f): + return f + return wrapper + + _DNS_REDIRECT_IP = None LOG = logging.getLogger(__name__) @@ -68,17 +78,15 @@ CONTAINER_TESTS = (['systemd-detect-virt', '--quiet', '--container'], ['running-in-container'], ['lxc-is-container']) -PROC_CMDLINE = None - -_LSB_RELEASE = {} - +@lru_cache() def get_architecture(target=None): out, _ = subp(['dpkg', '--print-architecture'], capture=True, target=target) return out.strip() +@lru_cache() def _lsb_release(target=None): fmap = {'Codename': 'codename', 'Description': 'description', 'Distributor ID': 'id', 'Release': 'release'} @@ -107,11 +115,7 @@ def lsb_release(target=None): # do not use or update cache if target is provided return _lsb_release(target) - global _LSB_RELEASE - if not _LSB_RELEASE: - data = _lsb_release() - _LSB_RELEASE.update(data) - return _LSB_RELEASE + return _lsb_release() def target_path(target, path=None): @@ -546,6 +550,7 @@ def is_ipv4(instr): return len(toks) == 4 +@lru_cache() def is_FreeBSD(): return system_info()['variant'] == "freebsd" @@ -595,6 +600,7 @@ def _parse_redhat_release(release_file=None): return {} +@lru_cache() def get_linux_distro(): distro_name = '' distro_version = '' @@ -622,6 +628,10 @@ def get_linux_distro(): flavor = match.groupdict()['codename'] if distro_name == 'rhel': distro_name = 'redhat' + elif os.path.exists('/bin/freebsd-version'): + distro_name = 'freebsd' + distro_version, _ = subp(['uname', '-r']) + distro_version = distro_version.strip() else: dist = ('', '', '') try: @@ -642,6 +652,7 @@ def get_linux_distro(): return (distro_name, distro_version, flavor) +@lru_cache() def system_info(): info = { 'platform': platform.platform(), @@ -1371,14 +1382,8 @@ def load_file(fname, read_cb=None, quiet=False, decode=True): return contents -def get_cmdline(): - if 'DEBUG_PROC_CMDLINE' in os.environ: - return os.environ["DEBUG_PROC_CMDLINE"] - - global PROC_CMDLINE - if PROC_CMDLINE is not None: - return PROC_CMDLINE - +@lru_cache() +def _get_cmdline(): if is_container(): try: contents = load_file("/proc/1/cmdline") @@ -1393,10 +1398,16 @@ def get_cmdline(): except Exception: cmdline = "" - PROC_CMDLINE = cmdline return cmdline +def get_cmdline(): + if 'DEBUG_PROC_CMDLINE' in os.environ: + return os.environ["DEBUG_PROC_CMDLINE"] + + return _get_cmdline() + + def pipe_in_out(in_fh, out_fh, chunk_size=1024, chunk_cb=None): bytes_piped = 0 while True: diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 0f45dc38..01119e0a 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -4576,6 +4576,7 @@ class TestNetRenderers(CiTestCase): priority=['sysconfig', 'eni']) @mock.patch("cloudinit.net.renderers.netplan.available") + @mock.patch("cloudinit.net.renderers.sysconfig.available") @mock.patch("cloudinit.net.renderers.sysconfig.available_sysconfig") @mock.patch("cloudinit.net.renderers.sysconfig.available_nm") @mock.patch("cloudinit.net.renderers.eni.available") @@ -4583,14 +4584,16 @@ class TestNetRenderers(CiTestCase): def test_sysconfig_selected_on_sysconfig_enabled_distros(self, m_distro, m_eni, m_sys_nm, m_sys_scfg, + m_sys_avail, m_netplan): """sysconfig only selected on specific distros (rhel/sles).""" # Ubuntu with Network-Manager installed - m_eni.return_value = False # no ifupdown (ifquery) - m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown - m_sys_nm.return_value = True # network-manager is installed - m_netplan.return_value = True # netplan is installed + m_eni.return_value = False # no ifupdown (ifquery) + m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown + m_sys_nm.return_value = True # network-manager is installed + m_netplan.return_value = True # netplan is installed + m_sys_avail.return_value = False # no sysconfig on Ubuntu m_distro.return_value = ('ubuntu', None, None) self.assertEqual('netplan', renderers.select(priority=None)[0]) @@ -4598,7 +4601,8 @@ class TestNetRenderers(CiTestCase): m_eni.return_value = False # no ifupdown (ifquery) m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown m_sys_nm.return_value = True # network-manager is installed - m_netplan.return_value = False # netplan is not installed + m_netplan.return_value = False # netplan is not installed + m_sys_avail.return_value = True # sysconfig is available on centos m_distro.return_value = ('centos', None, None) self.assertEqual('sysconfig', renderers.select(priority=None)[0]) @@ -4606,7 +4610,8 @@ class TestNetRenderers(CiTestCase): m_eni.return_value = False # no ifupdown (ifquery) m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown m_sys_nm.return_value = True # network-manager is installed - m_netplan.return_value = False # netplan is not installed + m_netplan.return_value = False # netplan is not installed + m_sys_avail.return_value = True # sysconfig is available on opensuse m_distro.return_value = ('opensuse', None, None) self.assertEqual('sysconfig', renderers.select(priority=None)[0]) @@ -4625,6 +4630,8 @@ class TestNetRenderers(CiTestCase): ] for (distro_name, distro_version, flavor) in distro_values: m_distro.return_value = (distro_name, distro_version, flavor) + if hasattr(util.system_info, "cache_clear"): + util.system_info.cache_clear() result = sysconfig.available() self.assertTrue(result) -- cgit v1.2.3 From f69d33a723b805fec3ee70c3a6127c8cadcb02d8 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Mon, 2 Dec 2019 16:24:18 -0700 Subject: url_helper: read_file_or_url should pass headers param into readurl (#66) Headers param was accidentally omitted and no longer passed through to readurl due to a previous commit. To avoid this omission of params in the future, drop positional param definitions from read_file_or_url and pass all kwargs through to readurl when we are not operating on a file. In util:read_seeded, correct the case where invalid positional param file_retries was being passed into read_file_or_url. Also drop duplicated file:// prefix addition from read_seeded because read_file_or_url does that work anyway. LP: #1854084 --- cloudinit/sources/helpers/azure.py | 6 ++- cloudinit/tests/test_url_helper.py | 52 ++++++++++++++++++++++ cloudinit/url_helper.py | 47 +++++++++++++++---- cloudinit/user_data.py | 2 +- cloudinit/util.py | 15 ++----- .../unittests/test_datasource/test_azure_helper.py | 18 +++++--- 6 files changed, 112 insertions(+), 28 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index f1fba175..f5cdb3fd 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -183,14 +183,16 @@ class AzureEndpointHttpClient(object): if secure: headers = self.headers.copy() headers.update(self.extra_secure_headers) - return url_helper.read_file_or_url(url, headers=headers) + return url_helper.read_file_or_url(url, headers=headers, timeout=5, + retries=10) def post(self, url, data=None, extra_headers=None): headers = self.headers if extra_headers is not None: headers = self.headers.copy() headers.update(extra_headers) - return url_helper.read_file_or_url(url, data=data, headers=headers) + return url_helper.read_file_or_url(url, data=data, headers=headers, + timeout=5, retries=10) class GoalState(object): diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py index aa9f3ec1..e883ddc2 100644 --- a/cloudinit/tests/test_url_helper.py +++ b/cloudinit/tests/test_url_helper.py @@ -4,6 +4,7 @@ from cloudinit.url_helper import ( NOT_FOUND, UrlError, oauth_headers, read_file_or_url, retry_on_url_exc) from cloudinit.tests.helpers import CiTestCase, mock, skipIf from cloudinit import util +from cloudinit import version import httpretty import requests @@ -17,6 +18,9 @@ except ImportError: _missing_oauthlib_dep = True +M_PATH = 'cloudinit.url_helper.' + + class TestOAuthHeaders(CiTestCase): def test_oauth_headers_raises_not_implemented_when_oathlib_missing(self): @@ -67,6 +71,54 @@ class TestReadFileOrUrl(CiTestCase): self.assertEqual(result.contents, data) self.assertEqual(str(result), data.decode('utf-8')) + @mock.patch(M_PATH + 'readurl') + def test_read_file_or_url_passes_params_to_readurl(self, m_readurl): + """read_file_or_url passes all params through to readurl.""" + url = 'http://hostname/path' + response = 'This is my url content\n' + m_readurl.return_value = response + params = {'url': url, 'timeout': 1, 'retries': 2, + 'headers': {'somehdr': 'val'}, + 'data': 'data', 'sec_between': 1, + 'ssl_details': {'cert_file': '/path/cert.pem'}, + 'headers_cb': 'headers_cb', 'exception_cb': 'exception_cb'} + self.assertEqual(response, read_file_or_url(**params)) + params.pop('url') # url is passed in as a positional arg + self.assertEqual([mock.call(url, **params)], m_readurl.call_args_list) + + def test_wb_read_url_defaults_honored_by_read_file_or_url_callers(self): + """Readurl param defaults used when unspecified by read_file_or_url + + Param defaults tested are as follows: + retries: 0, additional headers None beyond default, method: GET, + data: None, check_status: True and allow_redirects: True + """ + url = 'http://hostname/path' + + m_response = mock.MagicMock() + + class FakeSession(requests.Session): + def request(cls, **kwargs): + self.assertEqual( + {'url': url, 'allow_redirects': True, 'method': 'GET', + 'headers': { + 'User-Agent': 'Cloud-Init/%s' % ( + version.version_string())}}, + kwargs) + return m_response + + with mock.patch(M_PATH + 'requests.Session') as m_session: + error = requests.exceptions.HTTPError('broke') + m_session.side_effect = [error, FakeSession()] + # assert no retries and check_status == True + with self.assertRaises(UrlError) as context_manager: + response = read_file_or_url(url) + self.assertEqual('broke', str(context_manager.exception)) + # assert default headers, method, url and allow_redirects True + # Success on 2nd call with FakeSession + response = read_file_or_url(url) + self.assertEqual(m_response, response._response) + class TestRetryOnUrlExc(CiTestCase): diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py index 48ddae45..1496a471 100644 --- a/cloudinit/url_helper.py +++ b/cloudinit/url_helper.py @@ -81,14 +81,19 @@ def combine_url(base, *add_ons): return url -def read_file_or_url(url, timeout=5, retries=10, - headers=None, data=None, sec_between=1, ssl_details=None, - headers_cb=None, exception_cb=None): +def read_file_or_url(url, **kwargs): + """Wrapper function around readurl to allow passing a file path as url. + + When url is not a local file path, passthrough any kwargs to readurl. + + In the case of parameter passthrough to readurl, default values for some + parameters. See: call-signature of readurl in this module for param docs. + """ url = url.lstrip() if url.startswith("/"): url = "file://%s" % url if url.lower().startswith("file://"): - if data: + if kwargs.get("data"): LOG.warning("Unable to post data to file resource %s", url) file_path = url[len("file://"):] try: @@ -101,10 +106,7 @@ def read_file_or_url(url, timeout=5, retries=10, raise UrlError(cause=e, code=code, headers=None, url=url) return FileResponse(file_path, contents=contents) else: - return readurl(url, timeout=timeout, retries=retries, - headers_cb=headers_cb, data=data, - sec_between=sec_between, ssl_details=ssl_details, - exception_cb=exception_cb) + return readurl(url, **kwargs) # Made to have same accessors as UrlResponse so that the @@ -201,6 +203,35 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1, check_status=True, allow_redirects=True, exception_cb=None, session=None, infinite=False, log_req_resp=True, request_method=None): + """Wrapper around requests.Session to read the url and retry if necessary + + :param url: Mandatory url to request. + :param data: Optional form data to post the URL. Will set request_method + to 'POST' if present. + :param timeout: Timeout in seconds to wait for a response + :param retries: Number of times to retry on exception if exception_cb is + None or exception_cb returns True for the exception caught. Default is + to fail with 0 retries on exception. + :param sec_between: Default 1: amount of seconds passed to time.sleep + between retries. None or -1 means don't sleep. + :param headers: Optional dict of headers to send during request + :param headers_cb: Optional callable returning a dict of values to send as + headers during request + :param ssl_details: Optional dict providing key_file, ca_certs, and + cert_file keys for use on in ssl connections. + :param check_status: Optional boolean set True to raise when HTTPError + occurs. Default: True. + :param allow_redirects: Optional boolean passed straight to Session.request + as 'allow_redirects'. Default: True. + :param exception_cb: Optional callable which accepts the params + msg and exception and returns a boolean True if retries are permitted. + :param session: Optional exiting requests.Session instance to reuse. + :param infinite: Bool, set True to retry indefinitely. Default: False. + :param log_req_resp: Set False to turn off verbose debug messages. + :param request_method: String passed as 'method' to Session.request. + Typically GET, or POST. Default: POST if data is provided, GET + otherwise. + """ url = _cleanurl(url) req_args = { 'url': url, diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py index ed83d2d8..15af1daf 100644 --- a/cloudinit/user_data.py +++ b/cloudinit/user_data.py @@ -224,7 +224,7 @@ class UserDataProcessor(object): content = util.load_file(include_once_fn) else: try: - resp = read_file_or_url(include_url, + resp = read_file_or_url(include_url, timeout=5, retries=10, ssl_details=self.ssl_details) if include_once_on and resp.ok(): util.write_file(include_once_fn, resp.contents, diff --git a/cloudinit/util.py b/cloudinit/util.py index 78b6a2d0..9d9d5c72 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -986,13 +986,6 @@ def load_yaml(blob, default=None, allowed=(dict,)): def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0): - if base.startswith("/"): - base = "file://%s" % base - - # default retries for file is 0. for network is 10 - if base.startswith("file://"): - retries = file_retries - if base.find("%s") >= 0: ud_url = base % ("user-data" + ext) md_url = base % ("meta-data" + ext) @@ -1000,14 +993,14 @@ def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0): ud_url = "%s%s%s" % (base, "user-data", ext) md_url = "%s%s%s" % (base, "meta-data", ext) - md_resp = url_helper.read_file_or_url(md_url, timeout, retries, - file_retries) + md_resp = url_helper.read_file_or_url(md_url, timeout=timeout, + retries=retries) md = None if md_resp.ok(): md = load_yaml(decode_binary(md_resp.contents), default={}) - ud_resp = url_helper.read_file_or_url(ud_url, timeout, retries, - file_retries) + ud_resp = url_helper.read_file_or_url(ud_url, timeout=timeout, + retries=retries) ud = None if ud_resp.ok(): ud = ud_resp.contents diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index bd006aba..bd17f636 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -212,8 +212,10 @@ class TestAzureEndpointHttpClient(CiTestCase): response = client.get(url, secure=False) self.assertEqual(1, self.read_file_or_url.call_count) self.assertEqual(self.read_file_or_url.return_value, response) - self.assertEqual(mock.call(url, headers=self.regular_headers), - self.read_file_or_url.call_args) + self.assertEqual( + mock.call(url, headers=self.regular_headers, retries=10, + timeout=5), + self.read_file_or_url.call_args) def test_secure_get(self): url = 'MyTestUrl' @@ -227,8 +229,10 @@ class TestAzureEndpointHttpClient(CiTestCase): response = client.get(url, secure=True) self.assertEqual(1, self.read_file_or_url.call_count) self.assertEqual(self.read_file_or_url.return_value, response) - self.assertEqual(mock.call(url, headers=expected_headers), - self.read_file_or_url.call_args) + self.assertEqual( + mock.call(url, headers=expected_headers, retries=10, + timeout=5), + self.read_file_or_url.call_args) def test_post(self): data = mock.MagicMock() @@ -238,7 +242,8 @@ class TestAzureEndpointHttpClient(CiTestCase): self.assertEqual(1, self.read_file_or_url.call_count) self.assertEqual(self.read_file_or_url.return_value, response) self.assertEqual( - mock.call(url, data=data, headers=self.regular_headers), + mock.call(url, data=data, headers=self.regular_headers, retries=10, + timeout=5), self.read_file_or_url.call_args) def test_post_with_extra_headers(self): @@ -250,7 +255,8 @@ class TestAzureEndpointHttpClient(CiTestCase): expected_headers = self.regular_headers.copy() expected_headers.update(extra_headers) self.assertEqual( - mock.call(mock.ANY, data=mock.ANY, headers=expected_headers), + mock.call(mock.ANY, data=mock.ANY, headers=expected_headers, + retries=10, timeout=5), self.read_file_or_url.call_args) -- cgit v1.2.3 From c4735dd37c89f12569093e5d18e13493cae1ee0b Mon Sep 17 00:00:00 2001 From: Eric Lafontaine Date: Tue, 3 Dec 2019 16:45:14 -0500 Subject: Fix linting failure in test_url_helper (#83) --- cloudinit/tests/test_url_helper.py | 1 + 1 file changed, 1 insertion(+) (limited to 'cloudinit/tests') diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py index e883ddc2..1674120f 100644 --- a/cloudinit/tests/test_url_helper.py +++ b/cloudinit/tests/test_url_helper.py @@ -98,6 +98,7 @@ class TestReadFileOrUrl(CiTestCase): m_response = mock.MagicMock() class FakeSession(requests.Session): + @classmethod def request(cls, **kwargs): self.assertEqual( {'url': url, 'allow_redirects': True, 'method': 'GET', -- cgit v1.2.3 From c5a7d7979c036f6dc6823f429c6b6820f7f74241 Mon Sep 17 00:00:00 2001 From: Conrad Hoffmann <1226676+bitfehler@users.noreply.github.com> Date: Wed, 8 Jan 2020 15:18:48 +0100 Subject: Make tests work with Python 3.8 (#139) * Make DistroChecker test work with Python 3.8 In Python 3.8, `platform.linux_distribution` has been removed. This was anticipated, and the cloud-init code uses its own `util.get_linux_distro` instead, which works fine w/o `platform.linux_distribution`. However, these tests still try to mock the platform function, which fails if it doesn't exist (Python 3.8). Instead, mock the new function here, as this is a test for code that depends on it rather than the function itself. * Make GetLinuxDistro tests work with Python 3.8 In Python 3.8, `platform.dist` was removed, so allow mock to create the function by setting `create=True`. * Make linter happy in Python 3.8 Suppress E1101(no-member) as this function was removed. --- cloudinit/analyze/tests/test_boot.py | 8 ++++---- cloudinit/tests/test_util.py | 6 +++--- cloudinit/util.py | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/analyze/tests/test_boot.py b/cloudinit/analyze/tests/test_boot.py index 706e2cc0..f4001c14 100644 --- a/cloudinit/analyze/tests/test_boot.py +++ b/cloudinit/analyze/tests/test_boot.py @@ -12,17 +12,17 @@ class TestDistroChecker(CiTestCase): @mock.patch('cloudinit.util.system_info', return_value={'dist': ('', '', ''), 'system': ''}) - @mock.patch('platform.linux_distribution', return_value=('', '', '')) + @mock.patch('cloudinit.util.get_linux_distro', return_value=('', '', '')) @mock.patch('cloudinit.util.is_FreeBSD', return_value=False) - def test_blank_distro(self, m_sys_info, m_linux_distribution, m_free_bsd): + def test_blank_distro(self, m_sys_info, m_get_linux_distro, m_free_bsd): self.assertEqual(err_code, dist_check_timestamp()) @mock.patch('cloudinit.util.system_info', return_value={'dist': ('', '', '')}) - @mock.patch('platform.linux_distribution', return_value=('', '', '')) + @mock.patch('cloudinit.util.get_linux_distro', return_value=('', '', '')) @mock.patch('cloudinit.util.is_FreeBSD', return_value=True) def test_freebsd_gentoo_cant_find(self, m_sys_info, - m_linux_distribution, m_is_FreeBSD): + m_get_linux_distro, m_is_FreeBSD): self.assertEqual(err_code, dist_check_timestamp()) @mock.patch('cloudinit.util.subp', return_value=(0, 1)) diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index 64ed82ea..be100646 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -523,7 +523,7 @@ class TestGetLinuxDistro(CiTestCase): self.assertEqual( ('opensuse-tumbleweed', '20180920', platform.machine()), dist) - @mock.patch('platform.dist') + @mock.patch('platform.dist', create=True) def test_get_linux_distro_no_data(self, m_platform_dist, m_path_exists): """Verify we get no information if os-release does not exist""" m_platform_dist.return_value = ('', '', '') @@ -531,7 +531,7 @@ class TestGetLinuxDistro(CiTestCase): dist = util.get_linux_distro() self.assertEqual(('', '', ''), dist) - @mock.patch('platform.dist') + @mock.patch('platform.dist', create=True) def test_get_linux_distro_no_impl(self, m_platform_dist, m_path_exists): """Verify we get an empty tuple when no information exists and Exceptions are not propagated""" @@ -540,7 +540,7 @@ class TestGetLinuxDistro(CiTestCase): dist = util.get_linux_distro() self.assertEqual(('', '', ''), dist) - @mock.patch('platform.dist') + @mock.patch('platform.dist', create=True) def test_get_linux_distro_plat_data(self, m_platform_dist, m_path_exists): """Verify we get the correct platform information""" m_platform_dist.return_value = ('foo', '1.1', 'aarch64') diff --git a/cloudinit/util.py b/cloudinit/util.py index 9d9d5c72..830c8e54 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -635,8 +635,8 @@ def get_linux_distro(): else: dist = ('', '', '') try: - # Will be removed in 3.7 - dist = platform.dist() # pylint: disable=W1505 + # Was removed in 3.8 + dist = platform.dist() # pylint: disable=W1505,E1101 except Exception: pass finally: -- cgit v1.2.3 From c911afbfe5a38823971e9cdbd4d1848c6e5c16de Mon Sep 17 00:00:00 2001 From: Igor Galić Date: Fri, 10 Jan 2020 05:06:06 +0100 Subject: util: move uptime's else branch into its own boottime function (#53) Also fix bugs: - pass binary instead of string to sysctlbyname(), and - unpack the "return value" in a struct, rather than in single integer. LP: #1853160 Co-Authored-By: Ryan Harper --- cloudinit/tests/test_util.py | 15 +++++++++++++++ cloudinit/util.py | 39 +++++++++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 10 deletions(-) (limited to 'cloudinit/tests') diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index be100646..11f37000 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -189,6 +189,21 @@ class TestUtil(CiTestCase): self.assertEqual(is_rw, False) +class TestUptime(CiTestCase): + + @mock.patch('cloudinit.util.boottime') + @mock.patch('cloudinit.util.os.path.exists') + @mock.patch('cloudinit.util.time.time') + def test_uptime_non_linux_path(self, m_time, m_exists, m_boottime): + boottime = 1000.0 + uptime = 10.0 + m_boottime.return_value = boottime + m_time.return_value = boottime + uptime + m_exists.return_value = False + result = util.uptime() + self.assertEqual(str(uptime), result) + + class TestShellify(CiTestCase): def test_input_dict_raises_type_error(self): diff --git a/cloudinit/util.py b/cloudinit/util.py index 830c8e54..76d7db78 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -10,7 +10,6 @@ import contextlib import copy as obj_copy -import ctypes import email import glob import grp @@ -1807,6 +1806,33 @@ def time_rfc2822(): return ts +def boottime(): + """Use sysctlbyname(3) via ctypes to find kern.boottime + + kern.boottime is of type struct timeval. Here we create a + private class to easier unpack it. + + @return boottime: float to be compatible with linux + """ + import ctypes + + NULL_BYTES = b"\x00" + + class timeval(ctypes.Structure): + _fields_ = [ + ("tv_sec", ctypes.c_int64), + ("tv_usec", ctypes.c_int64) + ] + libc = ctypes.CDLL('/lib/libc.so.7') + size = ctypes.c_size_t() + size.value = ctypes.sizeof(timeval) + buf = timeval() + if libc.sysctlbyname(b"kern.boottime" + NULL_BYTES, ctypes.byref(buf), + ctypes.byref(size), None, 0) != -1: + return buf.tv_sec + buf.tv_usec / 1000000.0 + raise RuntimeError("Unable to retrieve kern.boottime on this system") + + def uptime(): uptime_str = '??' method = 'unknown' @@ -1818,15 +1844,8 @@ def uptime(): uptime_str = contents.split()[0] else: method = 'ctypes' - libc = ctypes.CDLL('/lib/libc.so.7') - size = ctypes.c_size_t() - buf = ctypes.c_int() - size.value = ctypes.sizeof(buf) - libc.sysctlbyname("kern.boottime", ctypes.byref(buf), - ctypes.byref(size), None, 0) - now = time.time() - bootup = buf.value - uptime_str = now - bootup + # This is the *BSD codepath + uptime_str = str(time.time() - boottime()) except Exception: logexc(LOG, "Unable to read uptime using method: %s" % method) -- 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/tests') 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/tests') 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