From 94a64529dccebd8fe8c7969370b8696e46023fbd Mon Sep 17 00:00:00 2001 From: Paride Legovini Date: Wed, 30 Jan 2019 15:38:56 +0000 Subject: Resolve flake8 comparison and pycodestyle over-ident issues Fixes: - flake8: use ==/!= to compare str, bytes, and int literals - pycodestyle: E117 over-indented --- cloudinit/stages.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'cloudinit/stages.py') diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 8a064124..da7d349a 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -548,11 +548,11 @@ class Init(object): with events.ReportEventStack("consume-user-data", "reading and applying user-data", parent=self.reporter): - self._consume_userdata(frequency) + self._consume_userdata(frequency) with events.ReportEventStack("consume-vendor-data", "reading and applying vendor-data", parent=self.reporter): - self._consume_vendordata(frequency) + self._consume_vendordata(frequency) # Perform post-consumption adjustments so that # modules that run during the init stage reflect -- 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/stages.py') 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/stages.py') 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/stages.py') 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 26e1f063677d974ab3b1a48e010e732a538b9a8d Mon Sep 17 00:00:00 2001 From: Dominic Schlegel Date: Wed, 16 Oct 2019 13:49:43 +0000 Subject: Small typo fixes in code comments. --- cloudinit/stages.py | 2 +- cloudinit/templater.py | 4 ++-- cloudinit/url_helper.py | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'cloudinit/stages.py') diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 50129884..77c21de0 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -501,7 +501,7 @@ class Init(object): # Init the handlers first for (_ctype, mod) in c_handlers.items(): if mod in c_handlers.initialized: - # Avoid initing the same module twice (if said module + # Avoid initiating the same module twice (if said module # is registered to more than one content-type). continue handlers.call_begin(mod, data, frequency) diff --git a/cloudinit/templater.py b/cloudinit/templater.py index b668674b..e47cdeda 100644 --- a/cloudinit/templater.py +++ b/cloudinit/templater.py @@ -44,7 +44,7 @@ MISSING_JINJA_PREFIX = u'CI_MISSING_JINJA_VAR/' @implements_to_string # Needed for python2.7. Otherwise cached super.__str__ class UndefinedJinjaVariable(JUndefined): - """Class used to represent any undefined jinja template varible.""" + """Class used to represent any undefined jinja template variable.""" def __str__(self): return u'%s%s' % (MISSING_JINJA_PREFIX, self._undefined_name) @@ -58,7 +58,7 @@ class UndefinedJinjaVariable(JUndefined): def basic_render(content, params): - """This does sumple replacement of bash variable like templates. + """This does simple replacement of bash variable like templates. It identifies patterns like ${a} or $a and can also identify patterns like ${a.b} or $a.b which will look for a key 'b' in the dictionary rooted diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py index 44ee61d4..0f4c36f7 100644 --- a/cloudinit/url_helper.py +++ b/cloudinit/url_helper.py @@ -326,10 +326,10 @@ def wait_for_url(urls, max_wait=None, timeout=None, sleep_time_cb: call method with 2 arguments (response, loop_n) that generates the next sleep time. - the idea of this routine is to wait for the EC2 metdata service to + the idea of this routine is to wait for the EC2 metadata service to come up. On both Eucalyptus and EC2 we have seen the case where the instance hit the MD before the MD service was up. EC2 seems - to have permenantely fixed this, though. + to have permanently fixed this, though. In openstack, the metadata service might be painfully slow, and unable to avoid hitting a timeout of even up to 10 seconds or more @@ -338,7 +338,7 @@ def wait_for_url(urls, max_wait=None, timeout=None, Offset those needs with the need to not hang forever (and block boot) on a system where cloud-init is configured to look for EC2 Metadata service but is not going to find one. It is possible that the instance - data host (169.254.169.254) may be firewalled off Entirely for a sytem, + data host (169.254.169.254) may be firewalled off Entirely for a system, meaning that the connection will block forever unless a timeout is set. A value of None for max_wait will retry indefinitely. -- cgit v1.2.3 From 45ea695f9b4fce180c662ab4211575d64912634e Mon Sep 17 00:00:00 2001 From: Pavel Zakharov Date: Thu, 31 Oct 2019 16:26:54 +0000 Subject: Add config for ssh-key import and consuming user-data This patch enables control over SSH public-key import and discarding supplied user-data (both disabled by default). allow-userdata: false ssh: allow_public_ssh_keys: false This feature enables closed appliances to prevent customers from unintentionally breaking the appliance which were not designed for user interaction. The downstream change for this is here: https://github.com/delphix/cloud-init/pull/4 --- cloudinit/config/cc_ssh.py | 15 +++++++++++-- cloudinit/config/tests/test_ssh.py | 46 ++++++++++++++++++++++++++++++-------- cloudinit/stages.py | 6 ++++- doc/rtd/topics/format.rst | 8 +++++++ tests/unittests/test_data.py | 40 +++++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 12 deletions(-) (limited to 'cloudinit/stages.py') diff --git a/cloudinit/config/cc_ssh.py b/cloudinit/config/cc_ssh.py index fdd8f4d3..050285a8 100755 --- a/cloudinit/config/cc_ssh.py +++ b/cloudinit/config/cc_ssh.py @@ -56,9 +56,13 @@ root login is disabled, and root login opts are set to:: no-port-forwarding,no-agent-forwarding,no-X11-forwarding Authorized keys for the default user/first user defined in ``users`` can be -specified using `ssh_authorized_keys``. Keys should be specified as a list of +specified using ``ssh_authorized_keys``. Keys should be specified as a list of public keys. +Importing ssh public keys for the default user (defined in ``users``)) is +enabled by default. This feature may be disabled by setting +``allow_publish_ssh_keys: false``. + .. note:: see the ``cc_set_passwords`` module documentation to enable/disable ssh password authentication @@ -91,6 +95,7 @@ public keys. ssh_authorized_keys: - ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAGEA3FSyQwBI6Z+nCSjUU ... - ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEA3I7VUf2l5gSn5uavROsc5HRDpZ ... + allow_public_ssh_keys: ssh_publish_hostkeys: enabled: (Defaults to true) blacklist: (Defaults to [dsa]) @@ -207,7 +212,13 @@ def handle(_name, cfg, cloud, log, _args): disable_root_opts = util.get_cfg_option_str(cfg, "disable_root_opts", ssh_util.DISABLE_USER_OPTS) - keys = cloud.get_public_ssh_keys() or [] + keys = [] + if util.get_cfg_option_bool(cfg, 'allow_public_ssh_keys', True): + keys = cloud.get_public_ssh_keys() or [] + else: + log.debug('Skipping import of publish ssh keys per ' + 'config setting: allow_public_ssh_keys=False') + if "ssh_authorized_keys" in cfg: cfgkeys = cfg["ssh_authorized_keys"] keys.extend(cfgkeys) diff --git a/cloudinit/config/tests/test_ssh.py b/cloudinit/config/tests/test_ssh.py index e7789842..0c554414 100644 --- a/cloudinit/config/tests/test_ssh.py +++ b/cloudinit/config/tests/test_ssh.py @@ -5,6 +5,9 @@ import os.path from cloudinit.config import cc_ssh from cloudinit import ssh_util from cloudinit.tests.helpers import CiTestCase, mock +import logging + +LOG = logging.getLogger(__name__) MODPATH = "cloudinit.config.cc_ssh." @@ -87,7 +90,7 @@ class TestHandleSsh(CiTestCase): cc_ssh.PUBLISH_HOST_KEYS = False cloud = self.tmp_cloud( distro='ubuntu', metadata={'public-keys': keys}) - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) options = ssh_util.DISABLE_USER_OPTS.replace("$USER", "NONE") options = options.replace("$DISABLE_USER", "root") m_glob.assert_called_once_with('/etc/ssh/ssh_host_*key*') @@ -100,6 +103,31 @@ class TestHandleSsh(CiTestCase): self.assertEqual([mock.call(set(keys), "root", options=options)], m_setup_keys.call_args_list) + @mock.patch(MODPATH + "glob.glob") + @mock.patch(MODPATH + "ug_util.normalize_users_groups") + @mock.patch(MODPATH + "os.path.exists") + def test_dont_allow_public_ssh_keys(self, m_path_exists, m_nug, + m_glob, m_setup_keys): + """Test allow_public_ssh_keys=False ignores ssh public keys from + platform. + """ + cfg = {"allow_public_ssh_keys": False} + keys = ["key1"] + user = "clouduser" + m_glob.return_value = [] # Return no matching keys to prevent removal + # Mock os.path.exits to True to short-circuit the key writing logic + m_path_exists.return_value = True + m_nug.return_value = ({user: {"default": user}}, {}) + cloud = self.tmp_cloud( + distro='ubuntu', metadata={'public-keys': keys}) + cc_ssh.handle("name", cfg, cloud, LOG, None) + + options = ssh_util.DISABLE_USER_OPTS.replace("$USER", user) + options = options.replace("$DISABLE_USER", "root") + self.assertEqual([mock.call(set(), user), + mock.call(set(), "root", options=options)], + m_setup_keys.call_args_list) + @mock.patch(MODPATH + "glob.glob") @mock.patch(MODPATH + "ug_util.normalize_users_groups") @mock.patch(MODPATH + "os.path.exists") @@ -115,7 +143,7 @@ class TestHandleSsh(CiTestCase): m_nug.return_value = ({user: {"default": user}}, {}) cloud = self.tmp_cloud( distro='ubuntu', metadata={'public-keys': keys}) - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) options = ssh_util.DISABLE_USER_OPTS.replace("$USER", user) options = options.replace("$DISABLE_USER", "root") @@ -140,7 +168,7 @@ class TestHandleSsh(CiTestCase): m_nug.return_value = ({user: {"default": user}}, {}) cloud = self.tmp_cloud( distro='ubuntu', metadata={'public-keys': keys}) - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) options = ssh_util.DISABLE_USER_OPTS.replace("$USER", user) options = options.replace("$DISABLE_USER", "root") @@ -165,7 +193,7 @@ class TestHandleSsh(CiTestCase): cloud = self.tmp_cloud( distro='ubuntu', metadata={'public-keys': keys}) cloud.get_public_ssh_keys = mock.Mock(return_value=keys) - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) self.assertEqual([mock.call(set(keys), user), mock.call(set(keys), "root", options="")], @@ -196,7 +224,7 @@ class TestHandleSsh(CiTestCase): cfg = {} expected_call = [self.test_hostkeys[key_type] for key_type in ['ecdsa', 'ed25519', 'rsa']] - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) self.assertEqual([mock.call(expected_call)], cloud.datasource.publish_host_keys.call_args_list) @@ -225,7 +253,7 @@ class TestHandleSsh(CiTestCase): cfg = {'ssh_publish_hostkeys': {'enabled': True}} expected_call = [self.test_hostkeys[key_type] for key_type in ['ecdsa', 'ed25519', 'rsa']] - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) self.assertEqual([mock.call(expected_call)], cloud.datasource.publish_host_keys.call_args_list) @@ -252,7 +280,7 @@ class TestHandleSsh(CiTestCase): cloud.datasource.publish_host_keys = mock.Mock() cfg = {'ssh_publish_hostkeys': {'enabled': False}} - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) self.assertFalse(cloud.datasource.publish_host_keys.call_args_list) cloud.datasource.publish_host_keys.assert_not_called() @@ -282,7 +310,7 @@ class TestHandleSsh(CiTestCase): 'blacklist': ['dsa', 'rsa']}} expected_call = [self.test_hostkeys[key_type] for key_type in ['ecdsa', 'ed25519']] - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) self.assertEqual([mock.call(expected_call)], cloud.datasource.publish_host_keys.call_args_list) @@ -312,6 +340,6 @@ class TestHandleSsh(CiTestCase): 'blacklist': []}} expected_call = [self.test_hostkeys[key_type] for key_type in ['dsa', 'ecdsa', 'ed25519', 'rsa']] - cc_ssh.handle("name", cfg, cloud, None, None) + cc_ssh.handle("name", cfg, cloud, LOG, None) self.assertEqual([mock.call(expected_call)], cloud.datasource.publish_host_keys.call_args_list) diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 77c21de0..71f3a49e 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -549,7 +549,11 @@ class Init(object): with events.ReportEventStack("consume-user-data", "reading and applying user-data", parent=self.reporter): - self._consume_userdata(frequency) + if util.get_cfg_option_bool(self.cfg, 'allow_userdata', True): + self._consume_userdata(frequency) + else: + LOG.debug('allow_userdata = False: discarding user-data') + with events.ReportEventStack("consume-vendor-data", "reading and applying vendor-data", parent=self.reporter): diff --git a/doc/rtd/topics/format.rst b/doc/rtd/topics/format.rst index 76050402..f9f4ba6c 100644 --- a/doc/rtd/topics/format.rst +++ b/doc/rtd/topics/format.rst @@ -196,6 +196,14 @@ Example Also this `blog`_ post offers another example for more advanced usage. +Disabling User-Data +=================== + +Cloud-init can be configured to ignore any user-data provided to instance. +This allows custom images to prevent users from accidentally breaking closed +appliances. Setting ``allow_userdata: false`` in the configuration will disable +cloud-init from processing user-data. + .. [#] See your cloud provider for applicable user-data size limitations... .. _blog: http://foss-boss.blogspot.com/2011/01/advanced-cloud-init-custom-handlers.html .. vi: textwidth=78 diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py index 22cf8f28..e55feb22 100644 --- a/tests/unittests/test_data.py +++ b/tests/unittests/test_data.py @@ -525,6 +525,46 @@ c: 4 self.assertEqual(cfg.get('password'), 'gocubs') self.assertEqual(cfg.get('locale'), 'chicago') + @mock.patch('cloudinit.util.read_conf_with_confd') + def test_dont_allow_user_data(self, mock_cfg): + mock_cfg.return_value = {"allow_userdata": False} + + # test that user-data is ignored but vendor-data is kept + user_blob = ''' +#cloud-config-jsonp +[ + { "op": "add", "path": "/baz", "value": "qux" }, + { "op": "add", "path": "/bar", "value": "qux2" } +] +''' + vendor_blob = ''' +#cloud-config-jsonp +[ + { "op": "add", "path": "/baz", "value": "quxA" }, + { "op": "add", "path": "/bar", "value": "quxB" }, + { "op": "add", "path": "/foo", "value": "quxC" } +] +''' + self.reRoot() + initer = stages.Init() + initer.datasource = FakeDataSource(user_blob, vendordata=vendor_blob) + initer.read_cfg() + initer.initialize() + initer.fetch() + initer.instancify() + initer.update() + initer.cloudify().run('consume_data', + initer.consume_data, + args=[PER_INSTANCE], + freq=PER_INSTANCE) + mods = stages.Modules(initer) + (_which_ran, _failures) = mods.run_section('cloud_init_modules') + cfg = mods.cfg + self.assertIn('vendor_data', cfg) + self.assertEqual('quxA', cfg['baz']) + self.assertEqual('quxB', cfg['bar']) + self.assertEqual('quxC', cfg['foo']) + class TestConsumeUserDataHttp(TestConsumeUserData, helpers.HttprettyTestCase): -- 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/stages.py') diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index 0ad6b7f1..01d61fa1 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -79,8 +79,6 @@ from cloudinit import templater from cloudinit import url_helper from cloudinit import util -import six - RUBY_VERSION_DEFAULT = "1.8" CHEF_DIRS = tuple([ @@ -273,7 +271,7 @@ def run_chef(chef_cfg, log): cmd_args = chef_cfg['exec_arguments'] if isinstance(cmd_args, (list, tuple)): cmd.extend(cmd_args) - elif isinstance(cmd_args, six.string_types): + elif isinstance(cmd_args, str): cmd.append(cmd_args) else: log.warning("Unknown type %s provided for chef" diff --git a/cloudinit/config/cc_mcollective.py b/cloudinit/config/cc_mcollective.py index d5f63f5f..351183f1 100644 --- a/cloudinit/config/cc_mcollective.py +++ b/cloudinit/config/cc_mcollective.py @@ -49,9 +49,7 @@ private certificates for mcollective. Their values will be written to """ import errno - -import six -from six import BytesIO +import io # Used since this can maintain comments # and doesn't need a top level section @@ -73,7 +71,7 @@ def configure(config, server_cfg=SERVER_CFG, # original file in order to be able to mix the rest up. try: old_contents = util.load_file(server_cfg, quiet=False, decode=False) - mcollective_config = ConfigObj(BytesIO(old_contents)) + mcollective_config = ConfigObj(io.BytesIO(old_contents)) except IOError as e: if e.errno != errno.ENOENT: raise @@ -93,7 +91,7 @@ def configure(config, server_cfg=SERVER_CFG, 'plugin.ssl_server_private'] = pricert_file mcollective_config['securityprovider'] = 'ssl' else: - if isinstance(cfg, six.string_types): + if isinstance(cfg, str): # Just set it in the 'main' section mcollective_config[cfg_name] = cfg elif isinstance(cfg, (dict)): @@ -119,7 +117,7 @@ def configure(config, server_cfg=SERVER_CFG, raise # Now we got the whole (new) file, write to disk... - contents = BytesIO() + contents = io.BytesIO() mcollective_config.write(contents) util.write_file(server_cfg, contents.getvalue(), mode=0o644) diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index 9e074bda..5498bbaa 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -6,19 +6,17 @@ """NTP: enable and configure ntp""" -from cloudinit.config.schema import ( - get_schema_doc, validate_cloudconfig_schema) +import copy +import os +from textwrap import dedent + from cloudinit import log as logging -from cloudinit.settings import PER_INSTANCE from cloudinit import temp_utils from cloudinit import templater from cloudinit import type_utils from cloudinit import util - -import copy -import os -import six -from textwrap import dedent +from cloudinit.config.schema import get_schema_doc, validate_cloudconfig_schema +from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) @@ -460,7 +458,7 @@ def supplemental_schema_validation(ntp_config): for key, value in sorted(ntp_config.items()): keypath = 'ntp:config:' + key if key == 'confpath': - if not all([value, isinstance(value, six.string_types)]): + if not all([value, isinstance(value, str)]): errors.append( 'Expected a config file path {keypath}.' ' Found ({value})'.format(keypath=keypath, value=value)) @@ -472,11 +470,11 @@ def supplemental_schema_validation(ntp_config): elif key in ('template', 'template_name'): if value is None: # Either template or template_name can be none continue - if not isinstance(value, six.string_types): + if not isinstance(value, str): errors.append( 'Expected a string type for {keypath}.' ' Found ({value})'.format(keypath=keypath, value=value)) - elif not isinstance(value, six.string_types): + elif not isinstance(value, str): errors.append( 'Expected a string type for {keypath}.' ' Found ({value})'.format(keypath=keypath, value=value)) diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py index 43a479cf..3e81a3c7 100644 --- a/cloudinit/config/cc_power_state_change.py +++ b/cloudinit/config/cc_power_state_change.py @@ -49,16 +49,15 @@ key returns 0. condition: """ -from cloudinit.settings import PER_INSTANCE -from cloudinit import util - import errno import os import re -import six import subprocess import time +from cloudinit.settings import PER_INSTANCE +from cloudinit import util + frequency = PER_INSTANCE EXIT_FAIL = 254 @@ -183,7 +182,7 @@ def load_power_state(cfg): pstate['timeout']) condition = pstate.get("condition", True) - if not isinstance(condition, six.string_types + (list, bool)): + if not isinstance(condition, (str, list, bool)): raise TypeError("condition type %s invalid. must be list, bool, str") return (args, timeout, condition) diff --git a/cloudinit/config/cc_rsyslog.py b/cloudinit/config/cc_rsyslog.py index ff211f65..5df0137d 100644 --- a/cloudinit/config/cc_rsyslog.py +++ b/cloudinit/config/cc_rsyslog.py @@ -180,7 +180,6 @@ config entries. Legacy to new mappings are as follows: import os import re -import six from cloudinit import log as logging from cloudinit import util @@ -233,9 +232,9 @@ def load_config(cfg): fillup = ( (KEYNAME_CONFIGS, [], list), - (KEYNAME_DIR, DEF_DIR, six.string_types), - (KEYNAME_FILENAME, DEF_FILENAME, six.string_types), - (KEYNAME_RELOAD, DEF_RELOAD, six.string_types + (list,)), + (KEYNAME_DIR, DEF_DIR, str), + (KEYNAME_FILENAME, DEF_FILENAME, str), + (KEYNAME_RELOAD, DEF_RELOAD, (str, list)), (KEYNAME_REMOTES, DEF_REMOTES, dict)) for key, default, vtypes in fillup: diff --git a/cloudinit/config/cc_ubuntu_advantage.py b/cloudinit/config/cc_ubuntu_advantage.py index f846e9a5..8b6d2a1a 100644 --- a/cloudinit/config/cc_ubuntu_advantage.py +++ b/cloudinit/config/cc_ubuntu_advantage.py @@ -4,8 +4,6 @@ from textwrap import dedent -import six - from cloudinit.config.schema import ( get_schema_doc, validate_cloudconfig_schema) from cloudinit import log as logging @@ -98,7 +96,7 @@ def configure_ua(token=None, enable=None): if enable is None: enable = [] - elif isinstance(enable, six.string_types): + elif isinstance(enable, str): LOG.warning('ubuntu_advantage: enable should be a list, not' ' a string; treating as a single enable') enable = [enable] diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py index 0b6546e2..bd87e9e5 100644 --- a/cloudinit/config/cc_write_files.py +++ b/cloudinit/config/cc_write_files.py @@ -57,7 +57,6 @@ binary gzip data can be specified and will be decoded before being written. import base64 import os -import six from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE @@ -126,7 +125,7 @@ def decode_perms(perm, default): if perm is None: return default try: - if isinstance(perm, six.integer_types + (float,)): + if isinstance(perm, (int, float)): # Just 'downcast' it (if a float) return int(perm) else: diff --git a/cloudinit/config/cc_yum_add_repo.py b/cloudinit/config/cc_yum_add_repo.py index 3b354a7d..3673166a 100644 --- a/cloudinit/config/cc_yum_add_repo.py +++ b/cloudinit/config/cc_yum_add_repo.py @@ -30,13 +30,9 @@ entry, the config entry will be skipped. # any repository configuration options (see man yum.conf) """ +import io import os - -try: - from configparser import ConfigParser -except ImportError: - from ConfigParser import ConfigParser -import six +from configparser import ConfigParser from cloudinit import util @@ -57,7 +53,7 @@ def _format_repo_value(val): # Can handle 'lists' in certain cases # See: https://linux.die.net/man/5/yum.conf return "\n".join([_format_repo_value(v) for v in val]) - if not isinstance(val, six.string_types): + if not isinstance(val, str): return str(val) return val @@ -72,7 +68,7 @@ def _format_repository_config(repo_id, repo_config): # For now assume that people using this know # the format of yum and don't verify keys/values further to_be.set(repo_id, k, _format_repo_value(v)) - to_be_stream = six.StringIO() + to_be_stream = io.StringIO() to_be.write(to_be_stream) to_be_stream.seek(0) lines = to_be_stream.readlines() diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index cdce26f2..92598a2d 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -9,13 +9,11 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import six -from six import StringIO - import abc import os import re import stat +from io import StringIO from cloudinit import importer from cloudinit import log as logging @@ -53,8 +51,7 @@ _EC2_AZ_RE = re.compile('^[a-z][a-z]-(?:[a-z]+-)+[0-9][a-z]$') PREFERRED_NTP_CLIENTS = ['chrony', 'systemd-timesyncd', 'ntp', 'ntpdate'] -@six.add_metaclass(abc.ABCMeta) -class Distro(object): +class Distro(metaclass=abc.ABCMeta): usr_lib_exec = "/usr/lib" hosts_fn = "/etc/hosts" @@ -429,7 +426,7 @@ class Distro(object): # support kwargs having groups=[list] or groups="g1,g2" groups = kwargs.get('groups') if groups: - if isinstance(groups, six.string_types): + if isinstance(groups, str): groups = groups.split(",") # remove any white spaces in group names, most likely @@ -544,7 +541,7 @@ class Distro(object): if 'ssh_authorized_keys' in kwargs: # Try to handle this in a smart manner. keys = kwargs['ssh_authorized_keys'] - if isinstance(keys, six.string_types): + if isinstance(keys, str): keys = [keys] elif isinstance(keys, dict): keys = list(keys.values()) @@ -668,7 +665,7 @@ class Distro(object): if isinstance(rules, (list, tuple)): for rule in rules: lines.append("%s %s" % (user, rule)) - elif isinstance(rules, six.string_types): + elif isinstance(rules, str): lines.append("%s %s" % (user, rules)) else: msg = "Can not create sudoers rule addition with type %r" diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py index 40e435e7..026d1142 100644 --- a/cloudinit/distros/freebsd.py +++ b/cloudinit/distros/freebsd.py @@ -5,10 +5,8 @@ # This file is part of cloud-init. See LICENSE file for license information. import os -import six -from six import StringIO - import re +from io import StringIO from cloudinit import distros from cloudinit import helpers @@ -108,8 +106,7 @@ class Distro(distros.Distro): } for key, val in kwargs.items(): - if (key in pw_useradd_opts and val and - isinstance(val, six.string_types)): + if key in pw_useradd_opts and val and isinstance(val, str): pw_useradd_cmd.extend([pw_useradd_opts[key], val]) elif key in pw_useradd_flags and val: diff --git a/cloudinit/distros/parsers/sys_conf.py b/cloudinit/distros/parsers/sys_conf.py index 44df17de..dee4c551 100644 --- a/cloudinit/distros/parsers/sys_conf.py +++ b/cloudinit/distros/parsers/sys_conf.py @@ -4,11 +4,9 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import six -from six import StringIO - import pipes import re +from io import StringIO # This library is used to parse/write # out the various sysconfig files edited (best attempt effort) @@ -65,7 +63,7 @@ class SysConf(configobj.ConfigObj): return out_contents.getvalue() def _quote(self, value, multiline=False): - if not isinstance(value, six.string_types): + if not isinstance(value, str): raise ValueError('Value "%s" is not a string' % (value)) if len(value) == 0: return '' diff --git a/cloudinit/distros/ug_util.py b/cloudinit/distros/ug_util.py index 9378dd78..08446a95 100755 --- a/cloudinit/distros/ug_util.py +++ b/cloudinit/distros/ug_util.py @@ -9,8 +9,6 @@ # # This file is part of cloud-init. See LICENSE file for license information. -import six - from cloudinit import log as logging from cloudinit import type_utils from cloudinit import util @@ -29,7 +27,7 @@ LOG = logging.getLogger(__name__) # is the standard form used in the rest # of cloud-init def _normalize_groups(grp_cfg): - if isinstance(grp_cfg, six.string_types): + if isinstance(grp_cfg, str): grp_cfg = grp_cfg.strip().split(",") if isinstance(grp_cfg, list): c_grp_cfg = {} @@ -39,7 +37,7 @@ def _normalize_groups(grp_cfg): if k not in c_grp_cfg: if isinstance(v, list): c_grp_cfg[k] = list(v) - elif isinstance(v, six.string_types): + elif isinstance(v, str): c_grp_cfg[k] = [v] else: raise TypeError("Bad group member type %s" % @@ -47,12 +45,12 @@ def _normalize_groups(grp_cfg): else: if isinstance(v, list): c_grp_cfg[k].extend(v) - elif isinstance(v, six.string_types): + elif isinstance(v, str): c_grp_cfg[k].append(v) else: raise TypeError("Bad group member type %s" % type_utils.obj_name(v)) - elif isinstance(i, six.string_types): + elif isinstance(i, str): if i not in c_grp_cfg: c_grp_cfg[i] = [] else: @@ -89,7 +87,7 @@ def _normalize_users(u_cfg, def_user_cfg=None): if isinstance(u_cfg, dict): ad_ucfg = [] for (k, v) in u_cfg.items(): - if isinstance(v, (bool, int, float) + six.string_types): + if isinstance(v, (bool, int, float, str)): if util.is_true(v): ad_ucfg.append(str(k)) elif isinstance(v, dict): @@ -99,12 +97,12 @@ def _normalize_users(u_cfg, def_user_cfg=None): raise TypeError(("Unmappable user value type %s" " for key %s") % (type_utils.obj_name(v), k)) u_cfg = ad_ucfg - elif isinstance(u_cfg, six.string_types): + elif isinstance(u_cfg, str): u_cfg = util.uniq_merge_sorted(u_cfg) users = {} for user_config in u_cfg: - if isinstance(user_config, (list,) + six.string_types): + if isinstance(user_config, (list, str)): for u in util.uniq_merge(user_config): if u and u not in users: users[u] = {} @@ -209,7 +207,7 @@ def normalize_users_groups(cfg, distro): old_user = cfg['user'] # Translate it into the format that is more useful # going forward - if isinstance(old_user, six.string_types): + if isinstance(old_user, str): old_user = { 'name': old_user, } @@ -238,7 +236,7 @@ def normalize_users_groups(cfg, distro): default_user_config = util.mergemanydict([old_user, distro_user_config]) base_users = cfg.get('users', []) - if not isinstance(base_users, (list, dict) + six.string_types): + if not isinstance(base_users, (list, dict, str)): LOG.warning(("Format for 'users' key must be a comma separated string" " or a dictionary or a list and not %s"), type_utils.obj_name(base_users)) @@ -252,7 +250,7 @@ def normalize_users_groups(cfg, distro): base_users.append({'name': 'default'}) elif isinstance(base_users, dict): base_users['default'] = dict(base_users).get('default', True) - elif isinstance(base_users, six.string_types): + elif isinstance(base_users, str): # Just append it on to be re-parsed later base_users += ",default" diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 9b126100..63d6e291 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -10,8 +10,6 @@ import logging import socket import struct -import six - from cloudinit import safeyaml from cloudinit import util @@ -186,7 +184,7 @@ class NetworkState(object): def iter_interfaces(self, filter_func=None): ifaces = self._network_state.get('interfaces', {}) - for iface in six.itervalues(ifaces): + for iface in ifaces.values(): if filter_func is None: yield iface else: @@ -220,8 +218,7 @@ class NetworkState(object): ) -@six.add_metaclass(CommandHandlerMeta) -class NetworkStateInterpreter(object): +class NetworkStateInterpreter(metaclass=CommandHandlerMeta): initial_network_state = { 'interfaces': {}, @@ -970,7 +967,7 @@ def ipv4_mask_to_net_prefix(mask): """ if isinstance(mask, int): return mask - if isinstance(mask, six.string_types): + if isinstance(mask, str): try: return int(mask) except ValueError: @@ -997,7 +994,7 @@ def ipv6_mask_to_net_prefix(mask): if isinstance(mask, int): return mask - if isinstance(mask, six.string_types): + if isinstance(mask, str): try: return int(mask) except ValueError: diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py index 5f32e90f..2a61a7a8 100644 --- a/cloudinit/net/renderer.py +++ b/cloudinit/net/renderer.py @@ -6,7 +6,7 @@ # This file is part of cloud-init. See LICENSE file for license information. import abc -import six +import io from .network_state import parse_net_config_data from .udev import generate_udev_rule @@ -34,7 +34,7 @@ class Renderer(object): """Given state, emit udev rules to map mac to ifname.""" # TODO(harlowja): this seems shared between eni renderer and # this, so move it to a shared location. - content = six.StringIO() + content = io.StringIO() for iface in network_state.iter_interfaces(filter_by_physical): # for physical interfaces write out a persist net udev rule if 'name' in iface and iface.get('mac_address'): diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index 3e06af01..07668d3e 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -1,16 +1,15 @@ # This file is part of cloud-init. See LICENSE file for license information. +import io import os import re -import six +from configobj import ConfigObj -from cloudinit.distros.parsers import networkmanager_conf -from cloudinit.distros.parsers import resolv_conf from cloudinit import log as logging from cloudinit import util - -from configobj import ConfigObj +from cloudinit.distros.parsers import networkmanager_conf +from cloudinit.distros.parsers import resolv_conf from . import renderer from .network_state import ( @@ -96,7 +95,7 @@ class ConfigMap(object): return len(self._conf) def to_string(self): - buf = six.StringIO() + buf = io.StringIO() buf.write(_make_header()) if self._conf: buf.write("\n") @@ -104,7 +103,7 @@ class ConfigMap(object): value = self._conf[key] if isinstance(value, bool): value = self._bool_map[value] - if not isinstance(value, six.string_types): + if not isinstance(value, str): value = str(value) buf.write("%s=%s\n" % (key, _quote_value(value))) return buf.getvalue() @@ -150,7 +149,7 @@ class Route(ConfigMap): # only accept ipv4 and ipv6 if proto not in ['ipv4', 'ipv6']: raise ValueError("Unknown protocol '%s'" % (str(proto))) - buf = six.StringIO() + buf = io.StringIO() buf.write(_make_header()) if self._conf: buf.write("\n") diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 9698261b..f73b37ed 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -3,7 +3,6 @@ import copy import inspect import os -import six import stat from cloudinit.event import EventType @@ -13,7 +12,7 @@ from cloudinit.sources import ( EXPERIMENTAL_TEXT, INSTANCE_JSON_FILE, INSTANCE_JSON_SENSITIVE_FILE, METADATA_UNKNOWN, REDACT_SENSITIVE_VALUE, UNSET, DataSource, canonical_cloud_id, redact_sensitive_keys) -from cloudinit.tests.helpers import CiTestCase, skipIf, mock +from cloudinit.tests.helpers import CiTestCase, mock from cloudinit.user_data import UserDataProcessor from cloudinit import util @@ -422,7 +421,6 @@ class TestDataSource(CiTestCase): {'network_json': 'is good'}, instance_data['ds']['network_json']) - @skipIf(not six.PY3, "json serialization on <= py2.7 handles bytes") def test_get_data_base64encodes_unserializable_bytes(self): """On py3, get_data base64encodes any unserializable content.""" tmp = self.tmp_dir() @@ -440,35 +438,6 @@ class TestDataSource(CiTestCase): {'key1': 'val1', 'key2': {'key2.1': 'EjM='}}, instance_json['ds']['meta_data']) - @skipIf(not six.PY2, "json serialization on <= py2.7 handles bytes") - def test_get_data_handles_bytes_values(self): - """On py2 get_data handles bytes values without having to b64encode.""" - tmp = self.tmp_dir() - datasource = DataSourceTestSubclassNet( - self.sys_cfg, self.distro, Paths({'run_dir': tmp}), - custom_metadata={'key1': 'val1', 'key2': {'key2.1': b'\x123'}}) - self.assertTrue(datasource.get_data()) - json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) - content = util.load_file(json_file) - instance_json = util.load_json(content) - self.assertEqual([], instance_json['base64_encoded_keys']) - self.assertEqual( - {'key1': 'val1', 'key2': {'key2.1': '\x123'}}, - instance_json['ds']['meta_data']) - - @skipIf(not six.PY2, "Only python2 hits UnicodeDecodeErrors on non-utf8") - def test_non_utf8_encoding_gets_b64encoded(self): - """When non-utf-8 values exist in py2 instance-data is b64encoded.""" - tmp = self.tmp_dir() - datasource = DataSourceTestSubclassNet( - self.sys_cfg, self.distro, Paths({'run_dir': tmp}), - custom_metadata={'key1': 'val1', 'key2': {'key2.1': b'ab\xaadef'}}) - self.assertTrue(datasource.get_data()) - json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) - instance_json = util.load_json(util.load_file(json_file)) - key21_value = instance_json['ds']['meta_data']['key2']['key2.1'] - self.assertEqual('ci-b64:' + util.b64e(b'ab\xaadef'), key21_value) - def test_get_hostname_subclass_support(self): """Validate get_hostname signature on all subclasses of DataSource.""" # Use inspect.getfullargspec when we drop py2.6 and py2.7 diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 85b6db97..6c551fcb 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -13,7 +13,6 @@ import httpretty import json import mock import os -import six import uuid DS_PATH = "cloudinit.sources.DataSourceOracle" @@ -334,7 +333,7 @@ class TestReadMetaData(test_helpers.HttprettyTestCase): for k, v in data.items(): httpretty.register_uri( httpretty.GET, self.mdurl + MD_VER + "/" + k, - v if not isinstance(v, six.text_type) else v.encode('utf-8')) + v if not isinstance(v, str) else v.encode('utf-8')) def test_broken_no_sys_uuid(self, m_read_system_uuid): """Datasource requires ability to read system_uuid and true return.""" diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 71f3a49e..db8ba64c 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -6,11 +6,9 @@ import copy import os +import pickle import sys -import six -from six.moves import cPickle as pickle - from cloudinit.settings import ( FREQUENCIES, CLOUD_CONFIG, PER_INSTANCE, RUN_CLOUD_CONFIG) @@ -758,7 +756,7 @@ class Modules(object): for item in cfg_mods: if not item: continue - if isinstance(item, six.string_types): + if isinstance(item, str): module_list.append({ 'mod': item.strip(), }) diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 4dad2afd..0220648d 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -4,6 +4,7 @@ from __future__ import print_function import functools import httpretty +import io import logging import os import random @@ -14,7 +15,6 @@ import tempfile import time import mock -import six import unittest2 from unittest2.util import strclass @@ -72,7 +72,7 @@ def retarget_many_wrapper(new_base, am, old_func): # Python 3 some of these now accept file-descriptors (integers). # That breaks rebase_path() so in lieu of a better solution, just # don't rebase if we get a fd. - if isinstance(path, six.string_types): + if isinstance(path, str): n_args[i] = rebase_path(path, new_base) return old_func(*n_args, **kwds) return wrapper @@ -149,7 +149,7 @@ class CiTestCase(TestCase): if self.with_logs: # Create a log handler so unit tests can search expected logs. self.logger = logging.getLogger() - self.logs = six.StringIO() + self.logs = io.StringIO() formatter = logging.Formatter('%(levelname)s: %(message)s') handler = logging.StreamHandler(self.logs) handler.setFormatter(formatter) @@ -166,7 +166,7 @@ class CiTestCase(TestCase): else: cmd = args[0] - if not isinstance(cmd, six.string_types): + if not isinstance(cmd, str): cmd = cmd[0] pass_through = False if not isinstance(self.allowed_subp, (list, bool)): @@ -346,8 +346,9 @@ class FilesystemMockingTestCase(ResourceUsingTestCase): def patchOpen(self, new_root): trap_func = retarget_many_wrapper(new_root, 1, open) - name = 'builtins.open' if six.PY3 else '__builtin__.open' - self.patched_funcs.enter_context(mock.patch(name, trap_func)) + self.patched_funcs.enter_context( + mock.patch('builtins.open', trap_func) + ) def patchStdoutAndStderr(self, stdout=None, stderr=None): if stdout is not None: @@ -420,7 +421,7 @@ def populate_dir(path, files): p = os.path.sep.join([path, name]) util.ensure_dir(os.path.dirname(p)) with open(p, "wb") as fp: - if isinstance(content, six.binary_type): + if isinstance(content, bytes): fp.write(content) else: fp.write(content.encode('utf-8')) diff --git a/tests/unittests/test_cli.py b/tests/unittests/test_cli.py index d283f136..e57c15d1 100644 --- a/tests/unittests/test_cli.py +++ b/tests/unittests/test_cli.py @@ -1,8 +1,8 @@ # This file is part of cloud-init. See LICENSE file for license information. -from collections import namedtuple import os -import six +import io +from collections import namedtuple from cloudinit.cmd import main as cli from cloudinit.tests import helpers as test_helpers @@ -18,7 +18,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def setUp(self): super(TestCLI, self).setUp() - self.stderr = six.StringIO() + self.stderr = io.StringIO() self.patchStdoutAndStderr(stderr=self.stderr) def _call_main(self, sysv_args=None): @@ -147,7 +147,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_conditional_subcommands_from_entry_point_sys_argv(self): """Subcommands from entry-point are properly parsed from sys.argv.""" - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) expected_errors = [ @@ -178,7 +178,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_collect_logs_subcommand_parser(self): """The subcommand cloud-init collect-logs calls the subparser.""" # Provide -h param to collect-logs to avoid having to mock behavior. - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'collect-logs', '-h']) self.assertIn('usage: cloud-init collect-log', stdout.getvalue()) @@ -186,7 +186,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_clean_subcommand_parser(self): """The subcommand cloud-init clean calls the subparser.""" # Provide -h param to clean to avoid having to mock behavior. - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'clean', '-h']) self.assertIn('usage: cloud-init clean', stdout.getvalue()) @@ -194,7 +194,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_status_subcommand_parser(self): """The subcommand cloud-init status calls the subparser.""" # Provide -h param to clean to avoid having to mock behavior. - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'status', '-h']) self.assertIn('usage: cloud-init status', stdout.getvalue()) @@ -219,7 +219,7 @@ class TestCLI(test_helpers.FilesystemMockingTestCase): def test_wb_devel_schema_subcommand_doc_content(self): """Validate that doc content is sane from known examples.""" - stdout = six.StringIO() + stdout = io.StringIO() self.patchStdoutAndStderr(stdout=stdout) self._call_main(['cloud-init', 'devel', 'schema', '--doc']) expected_doc_sections = [ diff --git a/tests/unittests/test_datasource/test_smartos.py b/tests/unittests/test_datasource/test_smartos.py index d5b1c29c..62084de5 100644 --- a/tests/unittests/test_datasource/test_smartos.py +++ b/tests/unittests/test_datasource/test_smartos.py @@ -33,8 +33,6 @@ from cloudinit.sources.DataSourceSmartOS import ( identify_file) from cloudinit.event import EventType -import six - from cloudinit import helpers as c_helpers from cloudinit.util import ( b64e, subp, ProcessExecutionError, which, write_file) @@ -798,7 +796,7 @@ class TestJoyentMetadataClient(FilesystemMockingTestCase): return self.serial.write.call_args[0][0] def test_get_metadata_writes_bytes(self): - self.assertIsInstance(self._get_written_line(), six.binary_type) + self.assertIsInstance(self._get_written_line(), bytes) def test_get_metadata_line_starts_with_v2(self): foo = self._get_written_line() diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py index f4311268..2dab3a54 100644 --- a/tests/unittests/test_handler/test_handler_chef.py +++ b/tests/unittests/test_handler/test_handler_chef.py @@ -4,7 +4,6 @@ import httpretty import json import logging import os -import six from cloudinit import cloud from cloudinit.config import cc_chef @@ -178,7 +177,7 @@ class TestChef(FilesystemMockingTestCase): continue # the value from the cfg overrides that in the default val = cfg['chef'].get(k, v) - if isinstance(val, six.string_types): + if isinstance(val, str): self.assertIn(val, c) c = util.load_file(cc_chef.CHEF_FB_PATH) self.assertEqual({}, json.loads(c)) diff --git a/tests/unittests/test_handler/test_handler_write_files.py b/tests/unittests/test_handler/test_handler_write_files.py index bc8756ca..ed0a4da2 100644 --- a/tests/unittests/test_handler/test_handler_write_files.py +++ b/tests/unittests/test_handler/test_handler_write_files.py @@ -1,17 +1,16 @@ # This file is part of cloud-init. See LICENSE file for license information. -from cloudinit.config.cc_write_files import write_files, decode_perms -from cloudinit import log as logging -from cloudinit import util - -from cloudinit.tests.helpers import CiTestCase, FilesystemMockingTestCase - import base64 import gzip +import io import shutil -import six import tempfile +from cloudinit import log as logging +from cloudinit import util +from cloudinit.config.cc_write_files import write_files, decode_perms +from cloudinit.tests.helpers import CiTestCase, FilesystemMockingTestCase + LOG = logging.getLogger(__name__) YAML_TEXT = """ @@ -138,7 +137,7 @@ class TestDecodePerms(CiTestCase): def _gzip_bytes(data): - buf = six.BytesIO() + buf = io.BytesIO() fp = None try: fp = gzip.GzipFile(fileobj=buf, mode="wb") diff --git a/tests/unittests/test_log.py b/tests/unittests/test_log.py index cd6296d6..e069a487 100644 --- a/tests/unittests/test_log.py +++ b/tests/unittests/test_log.py @@ -2,14 +2,15 @@ """Tests for cloudinit.log """ -from cloudinit.analyze.dump import CLOUD_INIT_ASCTIME_FMT -from cloudinit import log as ci_logging -from cloudinit.tests.helpers import CiTestCase import datetime +import io import logging -import six import time +from cloudinit import log as ci_logging +from cloudinit.analyze.dump import CLOUD_INIT_ASCTIME_FMT +from cloudinit.tests.helpers import CiTestCase + class TestCloudInitLogger(CiTestCase): @@ -18,7 +19,7 @@ class TestCloudInitLogger(CiTestCase): # of sys.stderr, we'll plug in a StringIO() object so we can see # what gets logged logging.Formatter.converter = time.gmtime - self.ci_logs = six.StringIO() + self.ci_logs = io.StringIO() self.ci_root = logging.getLogger() console = logging.StreamHandler(self.ci_logs) console.setFormatter(logging.Formatter(ci_logging.DEF_CON_FORMAT)) diff --git a/tests/unittests/test_merging.py b/tests/unittests/test_merging.py index 3a5072c7..10871bcf 100644 --- a/tests/unittests/test_merging.py +++ b/tests/unittests/test_merging.py @@ -13,13 +13,11 @@ import glob import os import random import re -import six import string SOURCE_PAT = "source*.*yaml" EXPECTED_PAT = "expected%s.yaml" -TYPES = [dict, str, list, tuple, None] -TYPES.extend(six.integer_types) +TYPES = [dict, str, list, tuple, None, int] def _old_mergedict(src, cand): @@ -85,7 +83,7 @@ def _make_dict(current_depth, max_depth, rand): pass if t in [tuple]: base = tuple(base) - elif t in six.integer_types: + elif t in [int]: base = rand.randint(0, 2 ** 8) elif t in [str]: base = _random_str(rand) diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 0e71db82..75a3f0b4 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -2,16 +2,15 @@ from __future__ import print_function +import io +import json import logging import os import re import shutil import stat -import tempfile - -import json -import six import sys +import tempfile import yaml from cloudinit import importer, util @@ -320,7 +319,7 @@ class TestLoadYaml(helpers.CiTestCase): def test_python_unicode(self): # complex type of python/unicode is explicitly allowed - myobj = {'1': six.text_type("FOOBAR")} + myobj = {'1': "FOOBAR"} safe_yaml = yaml.dump(myobj) self.assertEqual(util.load_yaml(blob=safe_yaml, default=self.mydefault), @@ -663,8 +662,8 @@ class TestMultiLog(helpers.FilesystemMockingTestCase): self.patchOS(self.root) self.patchUtils(self.root) self.patchOpen(self.root) - self.stdout = six.StringIO() - self.stderr = six.StringIO() + self.stdout = io.StringIO() + self.stderr = io.StringIO() self.patchStdoutAndStderr(self.stdout, self.stderr) def test_stderr_used_by_default(self): @@ -879,8 +878,8 @@ class TestSubp(helpers.CiTestCase): """Raised exc should have stderr, stdout as string if no decode.""" with self.assertRaises(util.ProcessExecutionError) as cm: util.subp([BOGUS_COMMAND], decode=True) - self.assertTrue(isinstance(cm.exception.stdout, six.string_types)) - self.assertTrue(isinstance(cm.exception.stderr, six.string_types)) + self.assertTrue(isinstance(cm.exception.stdout, str)) + self.assertTrue(isinstance(cm.exception.stderr, str)) def test_bunch_of_slashes_in_path(self): self.assertEqual("/target/my/path/", -- cgit v1.2.3