From fdadcb5fae51f4e6799314ab98e3aec56c79b17c Mon Sep 17 00:00:00 2001 From: Jason Zions Date: Tue, 15 Jan 2019 21:37:17 +0000 Subject: net: Wait for dhclient to daemonize before reading lease file cloud-init uses dhclient to fetch the DHCP lease so it can extract DHCP options. dhclient creates the leasefile, then writes to it; simply waiting for the leasefile to appear creates a race between dhclient and cloud-init. Instead, wait for dhclient to be parented by init. At that point, we know it has written to the leasefile, so it's safe to copy the file and kill the process. cloud-init creates a temporary directory in which to execute dhclient, and deletes that directory after it has killed the process. If cloud-init abandons waiting for dhclient to daemonize, it will still attempt to delete the temporary directory, but will not report an exception should that attempt fail. LP: #1794399 --- cloudinit/util.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index 7800f7bc..a8a232b6 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2876,4 +2876,20 @@ def udevadm_settle(exists=None, timeout=None): return subp(settle_cmd) +def get_proc_ppid(pid): + """ + Return the parent pid of a process. + """ + ppid = 0 + try: + contents = load_file("/proc/%s/stat" % pid, quiet=True) + except IOError as e: + LOG.warning('Failed to load /proc/%s/stat. %s', pid, e) + if contents: + parts = contents.split(" ", 4) + # man proc says + # ppid %d (4) The PID of the parent. + ppid = int(parts[3]) + return ppid + # vi: ts=4 expandtab -- cgit v1.2.3 From cf30836645473c62599e838ab48b2d31677fa584 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Thu, 7 Feb 2019 22:38:41 +0000 Subject: netplan: Don't render yaml aliases when dumping netplan Cloud-init rendered netplan with duplicate aliases if a network config included "global" nameserver/search values. Netplan uses can read yaml files which do use aliaes but cloud-init did not render a single yaml dictionary, instead it combined yaml sections into a single document which sometimes resulted in duplicate aliases being present. This branch introduces a yaml SafeDumper class which can set the 'ignore_aliases' attribute. This is not enabled by default but callers to util.yaml_dumps can pass a boolean to toggle this. The netplan render uses noalias=True and the resulting yaml output does not contain any aliases. LP: #1815051 --- cloudinit/net/netplan.py | 3 +- cloudinit/safeyaml.py | 7 + cloudinit/util.py | 17 ++- tests/unittests/test_net.py | 336 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 355 insertions(+), 8 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py index 21517fda..e54a34e5 100644 --- a/cloudinit/net/netplan.py +++ b/cloudinit/net/netplan.py @@ -361,7 +361,8 @@ class Renderer(renderer.Renderer): if section: dump = util.yaml_dumps({name: section}, explicit_start=False, - explicit_end=False) + explicit_end=False, + noalias=True) txt = util.indent(dump, ' ' * 4) return [txt] return [] diff --git a/cloudinit/safeyaml.py b/cloudinit/safeyaml.py index 7bcf9dd3..3bd5e03d 100644 --- a/cloudinit/safeyaml.py +++ b/cloudinit/safeyaml.py @@ -17,6 +17,13 @@ _CustomSafeLoader.add_constructor( _CustomSafeLoader.construct_python_unicode) +class NoAliasSafeDumper(yaml.dumper.SafeDumper): + """A class which avoids constructing anchors/aliases on yaml dump""" + + def ignore_aliases(self, data): + return True + + def load(blob): return(yaml.load(blob, Loader=_CustomSafeLoader)) diff --git a/cloudinit/util.py b/cloudinit/util.py index a8a232b6..2be528a0 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1596,14 +1596,17 @@ def json_dumps(data): separators=(',', ': '), default=json_serialize_default) -def yaml_dumps(obj, explicit_start=True, explicit_end=True): +def yaml_dumps(obj, explicit_start=True, explicit_end=True, noalias=False): """Return data in nicely formatted yaml.""" - return yaml.safe_dump(obj, - line_break="\n", - indent=4, - explicit_start=explicit_start, - explicit_end=explicit_end, - default_flow_style=False) + + return yaml.dump(obj, + line_break="\n", + indent=4, + explicit_start=explicit_start, + explicit_end=explicit_end, + default_flow_style=False, + Dumper=(safeyaml.NoAliasSafeDumper + if noalias else yaml.dumper.Dumper)) def ensure_dir(path, mode=None): diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index e041e978..f001ae5a 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -19,6 +19,7 @@ import gzip import io import json import os +import re import textwrap import yaml @@ -103,6 +104,309 @@ STATIC_EXPECTED_1 = { 'address': '10.0.0.2'}], } +V1_NAMESERVER_ALIAS = """ +config: +- id: eno1 + mac_address: 08:94:ef:51:ae:e0 + mtu: 1500 + name: eno1 + subnets: + - type: manual + type: physical +- id: eno2 + mac_address: 08:94:ef:51:ae:e1 + mtu: 1500 + name: eno2 + subnets: + - type: manual + type: physical +- id: eno3 + mac_address: 08:94:ef:51:ae:de + mtu: 1500 + name: eno3 + subnets: + - type: manual + type: physical +- bond_interfaces: + - eno1 + - eno3 + id: bondM + mac_address: 08:94:ef:51:ae:e0 + mtu: 1500 + name: bondM + params: + bond-downdelay: 0 + bond-lacp-rate: fast + bond-miimon: 100 + bond-mode: 802.3ad + bond-updelay: 0 + bond-xmit-hash-policy: layer3+4 + subnets: + - address: 10.101.10.47/23 + gateway: 10.101.11.254 + type: static + type: bond +- id: eno4 + mac_address: 08:94:ef:51:ae:df + mtu: 1500 + name: eno4 + subnets: + - type: manual + type: physical +- id: enp0s20f0u1u6 + mac_address: 0a:94:ef:51:a4:b9 + mtu: 1500 + name: enp0s20f0u1u6 + subnets: + - type: manual + type: physical +- id: enp216s0f0 + mac_address: 68:05:ca:81:7c:e8 + mtu: 9000 + name: enp216s0f0 + subnets: + - type: manual + type: physical +- id: enp216s0f1 + mac_address: 68:05:ca:81:7c:e9 + mtu: 9000 + name: enp216s0f1 + subnets: + - type: manual + type: physical +- id: enp47s0f0 + mac_address: 68:05:ca:64:d3:6c + mtu: 9000 + name: enp47s0f0 + subnets: + - type: manual + type: physical +- bond_interfaces: + - enp216s0f0 + - enp47s0f0 + id: bond0 + mac_address: 68:05:ca:64:d3:6c + mtu: 9000 + name: bond0 + params: + bond-downdelay: 0 + bond-lacp-rate: fast + bond-miimon: 100 + bond-mode: 802.3ad + bond-updelay: 0 + bond-xmit-hash-policy: layer3+4 + subnets: + - type: manual + type: bond +- id: bond0.3502 + mtu: 9000 + name: bond0.3502 + subnets: + - address: 172.20.80.4/25 + type: static + type: vlan + vlan_id: 3502 + vlan_link: bond0 +- id: bond0.3503 + mtu: 9000 + name: bond0.3503 + subnets: + - address: 172.20.80.129/25 + type: static + type: vlan + vlan_id: 3503 + vlan_link: bond0 +- id: enp47s0f1 + mac_address: 68:05:ca:64:d3:6d + mtu: 9000 + name: enp47s0f1 + subnets: + - type: manual + type: physical +- bond_interfaces: + - enp216s0f1 + - enp47s0f1 + id: bond1 + mac_address: 68:05:ca:64:d3:6d + mtu: 9000 + name: bond1 + params: + bond-downdelay: 0 + bond-lacp-rate: fast + bond-miimon: 100 + bond-mode: 802.3ad + bond-updelay: 0 + bond-xmit-hash-policy: layer3+4 + subnets: + - address: 10.101.8.65/26 + routes: + - destination: 213.119.192.0/24 + gateway: 10.101.8.126 + metric: 0 + type: static + type: bond +- address: + - 10.101.10.1 + - 10.101.10.2 + - 10.101.10.3 + - 10.101.10.5 + search: + - foo.bar + - maas + type: nameserver +version: 1 +""" + +NETPLAN_NO_ALIAS = """ +network: + version: 2 + ethernets: + eno1: + match: + macaddress: 08:94:ef:51:ae:e0 + mtu: 1500 + set-name: eno1 + eno2: + match: + macaddress: 08:94:ef:51:ae:e1 + mtu: 1500 + set-name: eno2 + eno3: + match: + macaddress: 08:94:ef:51:ae:de + mtu: 1500 + set-name: eno3 + eno4: + match: + macaddress: 08:94:ef:51:ae:df + mtu: 1500 + set-name: eno4 + enp0s20f0u1u6: + match: + macaddress: 0a:94:ef:51:a4:b9 + mtu: 1500 + set-name: enp0s20f0u1u6 + enp216s0f0: + match: + macaddress: 68:05:ca:81:7c:e8 + mtu: 9000 + set-name: enp216s0f0 + enp216s0f1: + match: + macaddress: 68:05:ca:81:7c:e9 + mtu: 9000 + set-name: enp216s0f1 + enp47s0f0: + match: + macaddress: 68:05:ca:64:d3:6c + mtu: 9000 + set-name: enp47s0f0 + enp47s0f1: + match: + macaddress: 68:05:ca:64:d3:6d + mtu: 9000 + set-name: enp47s0f1 + bonds: + bond0: + interfaces: + - enp216s0f0 + - enp47s0f0 + macaddress: 68:05:ca:64:d3:6c + mtu: 9000 + parameters: + down-delay: 0 + lacp-rate: fast + mii-monitor-interval: 100 + mode: 802.3ad + transmit-hash-policy: layer3+4 + up-delay: 0 + bond1: + addresses: + - 10.101.8.65/26 + interfaces: + - enp216s0f1 + - enp47s0f1 + macaddress: 68:05:ca:64:d3:6d + mtu: 9000 + nameservers: + addresses: + - 10.101.10.1 + - 10.101.10.2 + - 10.101.10.3 + - 10.101.10.5 + search: + - foo.bar + - maas + parameters: + down-delay: 0 + lacp-rate: fast + mii-monitor-interval: 100 + mode: 802.3ad + transmit-hash-policy: layer3+4 + up-delay: 0 + routes: + - metric: 0 + to: 213.119.192.0/24 + via: 10.101.8.126 + bondM: + addresses: + - 10.101.10.47/23 + gateway4: 10.101.11.254 + interfaces: + - eno1 + - eno3 + macaddress: 08:94:ef:51:ae:e0 + mtu: 1500 + nameservers: + addresses: + - 10.101.10.1 + - 10.101.10.2 + - 10.101.10.3 + - 10.101.10.5 + search: + - foo.bar + - maas + parameters: + down-delay: 0 + lacp-rate: fast + mii-monitor-interval: 100 + mode: 802.3ad + transmit-hash-policy: layer3+4 + up-delay: 0 + vlans: + bond0.3502: + addresses: + - 172.20.80.4/25 + id: 3502 + link: bond0 + mtu: 9000 + nameservers: + addresses: + - 10.101.10.1 + - 10.101.10.2 + - 10.101.10.3 + - 10.101.10.5 + search: + - foo.bar + - maas + bond0.3503: + addresses: + - 172.20.80.129/25 + id: 3503 + link: bond0 + mtu: 9000 + nameservers: + addresses: + - 10.101.10.1 + - 10.101.10.2 + - 10.101.10.3 + - 10.101.10.5 + search: + - foo.bar + - maas +""" + + # Examples (and expected outputs for various renderers). OS_SAMPLES = [ { @@ -3065,6 +3369,38 @@ class TestNetplanRoundTrip(CiTestCase): entry['expected_netplan'].splitlines(), files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + def test_render_output_has_yaml_no_aliases(self): + entry = { + 'yaml': V1_NAMESERVER_ALIAS, + 'expected_netplan': NETPLAN_NO_ALIAS, + } + network_config = yaml.load(entry['yaml']) + ns = network_state.parse_net_config_data(network_config) + files = self._render_and_read(state=ns) + # check for alias + content = files['/etc/netplan/50-cloud-init.yaml'] + + # test load the yaml to ensure we don't render something not loadable + # this allows single aliases, but not duplicate ones + parsed = yaml.load(files['/etc/netplan/50-cloud-init.yaml']) + self.assertNotEqual(None, parsed) + + # now look for any alias, avoid rendering them entirely + # generate the first anchor string using the template + # as of this writing, looks like "&id001" + anchor = r'&' + yaml.serializer.Serializer.ANCHOR_TEMPLATE % 1 + found_alias = re.search(anchor, content, re.MULTILINE) + if found_alias: + msg = "Error at: %s\nContent:\n%s" % (found_alias, content) + raise ValueError('Found yaml alias in rendered netplan: ' + msg) + + print(entry['expected_netplan']) + print('-- expected ^ | v rendered --') + print(files['/etc/netplan/50-cloud-init.yaml']) + self.assertEqual( + entry['expected_netplan'].splitlines(), + files['/etc/netplan/50-cloud-init.yaml'].splitlines()) + class TestEniRoundTrip(CiTestCase): -- cgit v1.2.3 From f278a8a3dbb1e45e8d83491ad24e41812cb77ddb Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 26 Feb 2019 15:23:58 +0000 Subject: util: don't determine string_types ourselves six already provides this for us, and we're already paying the cost to determine it there; no need to do it twice. --- cloudinit/sources/DataSourceOVF.py | 4 +++- cloudinit/util.py | 7 +------ 2 files changed, 4 insertions(+), 7 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py index 3a3fcdf6..70e7a5c0 100644 --- a/cloudinit/sources/DataSourceOVF.py +++ b/cloudinit/sources/DataSourceOVF.py @@ -15,6 +15,8 @@ import os import re import time +import six + from cloudinit import log as logging from cloudinit import sources from cloudinit import util @@ -434,7 +436,7 @@ def maybe_cdrom_device(devname): """ if not devname: return False - elif not isinstance(devname, util.string_types): + elif not isinstance(devname, six.string_types): raise ValueError("Unexpected input for devname: %s" % devname) # resolve '..' and multi '/' elements diff --git a/cloudinit/util.py b/cloudinit/util.py index 2be528a0..e5403f7d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -51,11 +51,6 @@ from cloudinit import version from cloudinit.settings import (CFG_BUILTIN) -try: - string_types = (basestring,) -except NameError: - string_types = (str,) - _DNS_REDIRECT_IP = None LOG = logging.getLogger(__name__) @@ -125,7 +120,7 @@ def target_path(target, path=None): # return 'path' inside target, accepting target as None if target in (None, ""): target = "/" - elif not isinstance(target, string_types): + elif not isinstance(target, six.string_types): raise ValueError("Unexpected input for target: %s" % target) else: target = os.path.abspath(target) -- cgit v1.2.3 From edf052c3196139169ecbfe98049c278f4babc8ca Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Mon, 4 Mar 2019 18:21:59 +0000 Subject: drop Python 2.6 support and our NIH version detection - Remove the last few places that use `if PY26` - Replace our Python version detection logic with six's (which we were already using in most places) --- cloudinit/tests/helpers.py | 22 +--------------------- cloudinit/util.py | 4 ---- tests/unittests/test_datasource/test_azure.py | 4 +--- 3 files changed, 2 insertions(+), 28 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 46a49416..f41180fd 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -41,26 +41,6 @@ _real_subp = util.subp SkipTest = unittest2.SkipTest skipIf = unittest2.skipIf -# Used for detecting different python versions -PY2 = False -PY26 = False -PY27 = False -PY3 = False - -_PY_VER = sys.version_info -_PY_MAJOR, _PY_MINOR, _PY_MICRO = _PY_VER[0:3] -if (_PY_MAJOR, _PY_MINOR) <= (2, 6): - if (_PY_MAJOR, _PY_MINOR) == (2, 6): - PY26 = True - if (_PY_MAJOR, _PY_MINOR) >= (2, 0): - PY2 = True -else: - if (_PY_MAJOR, _PY_MINOR) == (2, 7): - PY27 = True - PY2 = True - if (_PY_MAJOR, _PY_MINOR) >= (3, 0): - PY3 = True - # Makes the old path start # with new base instead of whatever @@ -357,7 +337,7 @@ class FilesystemMockingTestCase(ResourceUsingTestCase): def patchOpen(self, new_root): trap_func = retarget_many_wrapper(new_root, 1, open) - name = 'builtins.open' if PY3 else '__builtin__.open' + name = 'builtins.open' if six.PY3 else '__builtin__.open' self.patched_funcs.enter_context(mock.patch(name, trap_func)) def patchStdoutAndStderr(self, stdout=None, stderr=None): diff --git a/cloudinit/util.py b/cloudinit/util.py index e5403f7d..a192091f 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -72,7 +72,6 @@ CONTAINER_TESTS = (['systemd-detect-virt', '--quiet', '--container'], PROC_CMDLINE = None _LSB_RELEASE = {} -PY26 = sys.version_info[0:2] == (2, 6) def get_architecture(target=None): @@ -2815,9 +2814,6 @@ def load_shell_content(content, add_empty=False, empty_val=None): variables. Set their value to empty_val.""" def _shlex_split(blob): - if PY26 and isinstance(blob, six.text_type): - # Older versions don't support unicode input - blob = blob.encode("utf8") return shlex.split(blob, comments=True) data = {} diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 5edf36e8..6b05b8f1 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -11,7 +11,7 @@ from cloudinit.util import (b64e, decode_binary, load_file, write_file, from cloudinit.version import version_string as vs from cloudinit.tests.helpers import ( HttprettyTestCase, CiTestCase, populate_dir, mock, wrap_and_call, - ExitStack, PY26, SkipTest) + ExitStack) import crypt import httpretty @@ -221,8 +221,6 @@ class TestAzureDataSource(CiTestCase): def setUp(self): super(TestAzureDataSource, self).setUp() - if PY26: - raise SkipTest("Does not work on python 2.6") self.tmp = self.tmp_dir() # patch cloud_dir, so our 'seed_dir' is guaranteed empty -- cgit v1.2.3 From 5e5894d68d21bf33649aca36973a0ef2fe72f01d Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 19 Mar 2019 14:24:37 +0000 Subject: Add ubuntu_drivers config module The ubuntu_drivers config module enables usage of the 'ubuntu-drivers' command. At this point it only serves as a way of installing NVIDIA drivers for general purpose graphics processing unit (GPGPU) functionality. Also, a small usability improvement to get_cfg_by_path to allow it to take a string for the key path "toplevel/second/mykey" in addition to the original: ("toplevel", "second", "mykey") --- cloudinit/config/cc_ubuntu_drivers.py | 112 +++++++++++++++++ cloudinit/config/tests/test_ubuntu_drivers.py | 174 ++++++++++++++++++++++++++ cloudinit/util.py | 15 +++ config/cloud.cfg.tmpl | 3 + doc/rtd/topics/modules.rst | 1 + tests/unittests/test_handler/test_schema.py | 1 + 6 files changed, 306 insertions(+) create mode 100644 cloudinit/config/cc_ubuntu_drivers.py create mode 100644 cloudinit/config/tests/test_ubuntu_drivers.py (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py new file mode 100644 index 00000000..91feb603 --- /dev/null +++ b/cloudinit/config/cc_ubuntu_drivers.py @@ -0,0 +1,112 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +"""Ubuntu Drivers: Interact with third party drivers in Ubuntu.""" + +from textwrap import dedent + +from cloudinit.config.schema import ( + get_schema_doc, validate_cloudconfig_schema) +from cloudinit import log as logging +from cloudinit.settings import PER_INSTANCE +from cloudinit import type_utils +from cloudinit import util + +LOG = logging.getLogger(__name__) + +frequency = PER_INSTANCE +distros = ['ubuntu'] +schema = { + 'id': 'cc_ubuntu_drivers', + 'name': 'Ubuntu Drivers', + 'title': 'Interact with third party drivers in Ubuntu.', + 'description': dedent("""\ + This module interacts with the 'ubuntu-drivers' command to install + third party driver packages."""), + 'distros': distros, + 'examples': [dedent("""\ + drivers: + nvidia: + license-accepted: true + """)], + 'frequency': frequency, + 'type': 'object', + 'properties': { + 'drivers': { + 'type': 'object', + 'additionalProperties': False, + 'properties': { + 'nvidia': { + 'type': 'object', + 'additionalProperties': False, + 'required': ['license-accepted'], + 'properties': { + 'license-accepted': { + 'type': 'boolean', + 'description': ("Do you accept the NVIDIA driver" + " license?"), + }, + 'version': { + 'type': 'string', + 'description': ( + 'The version of the driver to install (e.g.' + ' "390", "410"). Defaults to the latest' + ' version.'), + }, + }, + }, + }, + }, + }, +} +OLD_UBUNTU_DRIVERS_STDERR_NEEDLE = ( + "ubuntu-drivers: error: argument : invalid choice: 'install'") + +__doc__ = get_schema_doc(schema) # Supplement python help() + + +def install_drivers(cfg, pkg_install_func): + if not isinstance(cfg, dict): + raise TypeError( + "'drivers' config expected dict, found '%s': %s" % + (type_utils.obj_name(cfg), cfg)) + + cfgpath = 'nvidia/license-accepted' + # Call translate_bool to ensure that we treat string values like "yes" as + # acceptance and _don't_ treat string values like "nah" as acceptance + # because they're True-ish + nv_acc = util.translate_bool(util.get_cfg_by_path(cfg, cfgpath)) + if not nv_acc: + LOG.debug("Not installing NVIDIA drivers. %s=%s", cfgpath, nv_acc) + return + + if not util.which('ubuntu-drivers'): + LOG.debug("'ubuntu-drivers' command not available. " + "Installing ubuntu-drivers-common") + pkg_install_func(['ubuntu-drivers-common']) + + driver_arg = 'nvidia' + version_cfg = util.get_cfg_by_path(cfg, 'nvidia/version') + if version_cfg: + driver_arg += ':{}'.format(version_cfg) + + LOG.debug("Installing NVIDIA drivers (%s=%s, version=%s)", + cfgpath, nv_acc, version_cfg if version_cfg else 'latest') + + try: + util.subp(['ubuntu-drivers', 'install', '--gpgpu', driver_arg]) + except util.ProcessExecutionError as exc: + if OLD_UBUNTU_DRIVERS_STDERR_NEEDLE in exc.stderr: + LOG.warning('the available version of ubuntu-drivers is' + ' too old to perform requested driver installation') + elif 'No drivers found for installation.' in exc.stdout: + LOG.warning('ubuntu-drivers found no drivers for installation') + raise + + +def handle(name, cfg, cloud, log, _args): + if "drivers" not in cfg: + log.debug("Skipping module named %s, no 'drivers' key in config", name) + return + + validate_cloudconfig_schema(cfg, schema) + install_drivers(cfg['drivers'], cloud.distro.install_packages) diff --git a/cloudinit/config/tests/test_ubuntu_drivers.py b/cloudinit/config/tests/test_ubuntu_drivers.py new file mode 100644 index 00000000..efba4ce7 --- /dev/null +++ b/cloudinit/config/tests/test_ubuntu_drivers.py @@ -0,0 +1,174 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +import copy + +from cloudinit.tests.helpers import CiTestCase, skipUnlessJsonSchema, mock +from cloudinit.config.schema import ( + SchemaValidationError, validate_cloudconfig_schema) +from cloudinit.config import cc_ubuntu_drivers as drivers +from cloudinit.util import ProcessExecutionError + +MPATH = "cloudinit.config.cc_ubuntu_drivers." +OLD_UBUNTU_DRIVERS_ERROR_STDERR = ( + "ubuntu-drivers: error: argument : invalid choice: 'install' " + "(choose from 'list', 'autoinstall', 'devices', 'debug')\n") + + +class TestUbuntuDrivers(CiTestCase): + cfg_accepted = {'drivers': {'nvidia': {'license-accepted': True}}} + install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia'] + + with_logs = True + + @skipUnlessJsonSchema() + def test_schema_requires_boolean_for_license_accepted(self): + with self.assertRaisesRegex( + SchemaValidationError, ".*license-accepted.*TRUE.*boolean"): + validate_cloudconfig_schema( + {'drivers': {'nvidia': {'license-accepted': "TRUE"}}}, + schema=drivers.schema, strict=True) + + @mock.patch(MPATH + "util.subp", return_value=('', '')) + @mock.patch(MPATH + "util.which", return_value=False) + def _assert_happy_path_taken(self, config, m_which, m_subp): + """Positive path test through handle. Package should be installed.""" + myCloud = mock.MagicMock() + drivers.handle('ubuntu_drivers', config, myCloud, None, None) + self.assertEqual([mock.call(['ubuntu-drivers-common'])], + myCloud.distro.install_packages.call_args_list) + self.assertEqual([mock.call(self.install_gpgpu)], + m_subp.call_args_list) + + def test_handle_does_package_install(self): + self._assert_happy_path_taken(self.cfg_accepted) + + def test_trueish_strings_are_considered_approval(self): + for true_value in ['yes', 'true', 'on', '1']: + new_config = copy.deepcopy(self.cfg_accepted) + new_config['drivers']['nvidia']['license-accepted'] = true_value + self._assert_happy_path_taken(new_config) + + @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( + stdout='No drivers found for installation.\n', exit_code=1)) + @mock.patch(MPATH + "util.which", return_value=False) + def test_handle_raises_error_if_no_drivers_found(self, m_which, m_subp): + """If ubuntu-drivers doesn't install any drivers, raise an error.""" + myCloud = mock.MagicMock() + with self.assertRaises(Exception): + drivers.handle( + 'ubuntu_drivers', self.cfg_accepted, myCloud, None, None) + self.assertEqual([mock.call(['ubuntu-drivers-common'])], + myCloud.distro.install_packages.call_args_list) + self.assertEqual([mock.call(self.install_gpgpu)], + m_subp.call_args_list) + self.assertIn('ubuntu-drivers found no drivers for installation', + self.logs.getvalue()) + + @mock.patch(MPATH + "util.subp", return_value=('', '')) + @mock.patch(MPATH + "util.which", return_value=False) + def _assert_inert_with_config(self, config, m_which, m_subp): + """Helper to reduce repetition when testing negative cases""" + myCloud = mock.MagicMock() + drivers.handle('ubuntu_drivers', config, myCloud, None, None) + self.assertEqual(0, myCloud.distro.install_packages.call_count) + self.assertEqual(0, m_subp.call_count) + + def test_handle_inert_if_license_not_accepted(self): + """Ensure we don't do anything if the license is rejected.""" + self._assert_inert_with_config( + {'drivers': {'nvidia': {'license-accepted': False}}}) + + def test_handle_inert_if_garbage_in_license_field(self): + """Ensure we don't do anything if unknown text is in license field.""" + self._assert_inert_with_config( + {'drivers': {'nvidia': {'license-accepted': 'garbage'}}}) + + def test_handle_inert_if_no_license_key(self): + """Ensure we don't do anything if no license key.""" + self._assert_inert_with_config({'drivers': {'nvidia': {}}}) + + def test_handle_inert_if_no_nvidia_key(self): + """Ensure we don't do anything if other license accepted.""" + self._assert_inert_with_config( + {'drivers': {'acme': {'license-accepted': True}}}) + + def test_handle_inert_if_string_given(self): + """Ensure we don't do anything if string refusal given.""" + for false_value in ['no', 'false', 'off', '0']: + self._assert_inert_with_config( + {'drivers': {'nvidia': {'license-accepted': false_value}}}) + + @mock.patch(MPATH + "install_drivers") + def test_handle_no_drivers_does_nothing(self, m_install_drivers): + """If no 'drivers' key in the config, nothing should be done.""" + myCloud = mock.MagicMock() + myLog = mock.MagicMock() + drivers.handle('ubuntu_drivers', {'foo': 'bzr'}, myCloud, myLog, None) + self.assertIn('Skipping module named', + myLog.debug.call_args_list[0][0][0]) + self.assertEqual(0, m_install_drivers.call_count) + + @mock.patch(MPATH + "util.subp", return_value=('', '')) + @mock.patch(MPATH + "util.which", return_value=True) + def test_install_drivers_no_install_if_present(self, m_which, m_subp): + """If 'ubuntu-drivers' is present, no package install should occur.""" + pkg_install = mock.MagicMock() + drivers.install_drivers(self.cfg_accepted['drivers'], + pkg_install_func=pkg_install) + self.assertEqual(0, pkg_install.call_count) + self.assertEqual([mock.call('ubuntu-drivers')], + m_which.call_args_list) + self.assertEqual([mock.call(self.install_gpgpu)], + m_subp.call_args_list) + + def test_install_drivers_rejects_invalid_config(self): + """install_drivers should raise TypeError if not given a config dict""" + pkg_install = mock.MagicMock() + with self.assertRaisesRegex(TypeError, ".*expected dict.*"): + drivers.install_drivers("mystring", pkg_install_func=pkg_install) + self.assertEqual(0, pkg_install.call_count) + + @mock.patch(MPATH + "util.subp", side_effect=ProcessExecutionError( + stderr=OLD_UBUNTU_DRIVERS_ERROR_STDERR, exit_code=2)) + @mock.patch(MPATH + "util.which", return_value=False) + def test_install_drivers_handles_old_ubuntu_drivers_gracefully( + self, m_which, m_subp): + """Older ubuntu-drivers versions should emit message and raise error""" + myCloud = mock.MagicMock() + with self.assertRaises(Exception): + drivers.handle( + 'ubuntu_drivers', self.cfg_accepted, myCloud, None, None) + self.assertEqual([mock.call(['ubuntu-drivers-common'])], + myCloud.distro.install_packages.call_args_list) + self.assertEqual([mock.call(self.install_gpgpu)], + m_subp.call_args_list) + self.assertIn('WARNING: the available version of ubuntu-drivers is' + ' too old to perform requested driver installation', + self.logs.getvalue()) + + +# Sub-class TestUbuntuDrivers to run the same test cases, but with a version +class TestUbuntuDriversWithVersion(TestUbuntuDrivers): + cfg_accepted = { + 'drivers': {'nvidia': {'license-accepted': True, 'version': '123'}}} + install_gpgpu = ['ubuntu-drivers', 'install', '--gpgpu', 'nvidia:123'] + + @mock.patch(MPATH + "util.subp", return_value=('', '')) + @mock.patch(MPATH + "util.which", return_value=False) + def test_version_none_uses_latest(self, m_which, m_subp): + myCloud = mock.MagicMock() + version_none_cfg = { + 'drivers': {'nvidia': {'license-accepted': True, 'version': None}}} + drivers.handle( + 'ubuntu_drivers', version_none_cfg, myCloud, None, None) + self.assertEqual( + [mock.call(['ubuntu-drivers', 'install', '--gpgpu', 'nvidia'])], + m_subp.call_args_list) + + def test_specifying_a_version_doesnt_override_license_acceptance(self): + self._assert_inert_with_config({ + 'drivers': {'nvidia': {'license-accepted': False, + 'version': '123'}} + }) + +# vi: ts=4 expandtab diff --git a/cloudinit/util.py b/cloudinit/util.py index a192091f..385f231c 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -703,6 +703,21 @@ def get_cfg_option_list(yobj, key, default=None): # get a cfg entry by its path array # for f['a']['b']: get_cfg_by_path(mycfg,('a','b')) def get_cfg_by_path(yobj, keyp, default=None): + """Return the value of the item at path C{keyp} in C{yobj}. + + example: + get_cfg_by_path({'a': {'b': {'num': 4}}}, 'a/b/num') == 4 + get_cfg_by_path({'a': {'b': {'num': 4}}}, 'c/d') == None + + @param yobj: A dictionary. + @param keyp: A path inside yobj. it can be a '/' delimited string, + or an iterable. + @param default: The default to return if the path does not exist. + @return: The value of the item at keyp." + is not found.""" + + if isinstance(keyp, six.string_types): + keyp = keyp.split("/") cur = yobj for tok in keyp: if tok not in cur: diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl index 7513176b..25db43e0 100644 --- a/config/cloud.cfg.tmpl +++ b/config/cloud.cfg.tmpl @@ -112,6 +112,9 @@ cloud_final_modules: - landscape - lxd {% endif %} +{% if variant in ["ubuntu", "unknown"] %} + - ubuntu-drivers +{% endif %} {% if variant not in ["freebsd"] %} - puppet - chef diff --git a/doc/rtd/topics/modules.rst b/doc/rtd/topics/modules.rst index d9720f6a..3dcdd3bc 100644 --- a/doc/rtd/topics/modules.rst +++ b/doc/rtd/topics/modules.rst @@ -54,6 +54,7 @@ Modules .. automodule:: cloudinit.config.cc_ssh_import_id .. automodule:: cloudinit.config.cc_timezone .. automodule:: cloudinit.config.cc_ubuntu_advantage +.. automodule:: cloudinit.config.cc_ubuntu_drivers .. automodule:: cloudinit.config.cc_update_etc_hosts .. automodule:: cloudinit.config.cc_update_hostname .. automodule:: cloudinit.config.cc_users_groups diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py index 1bad07f6..e69a47a9 100644 --- a/tests/unittests/test_handler/test_schema.py +++ b/tests/unittests/test_handler/test_schema.py @@ -28,6 +28,7 @@ class GetSchemaTest(CiTestCase): 'cc_runcmd', 'cc_snap', 'cc_ubuntu_advantage', + 'cc_ubuntu_drivers', 'cc_zypper_add_repo' ], [subschema['id'] for subschema in schema['allOf']]) -- cgit v1.2.3 From 937555fd422edf8235430afab3c0ab69f9e3b3a4 Mon Sep 17 00:00:00 2001 From: Gonéri Le Bouder Date: Thu, 18 Apr 2019 16:08:20 +0000 Subject: mount_cb: do not pass sync and rw options to mount On FreeBSD, mount_cd9660 does not accept the sync option that is enabled by default. In addition, the sync is only useful with the `rw` mode. However the `rw` mode was never used. This patch removes the `rw` and `sync` parameter of `mount_cb` to simplify the code base and resolve the FreeBSD issue. LP: #1645824 --- cloudinit/sources/DataSourceAzure.py | 2 +- cloudinit/sources/DataSourceConfigDrive.py | 7 ++----- cloudinit/util.py | 15 ++------------- 3 files changed, 5 insertions(+), 19 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 76b16616..64165259 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -407,7 +407,7 @@ class DataSourceAzure(sources.DataSource): elif cdev.startswith("/dev/"): if util.is_FreeBSD(): ret = util.mount_cb(cdev, load_azure_ds_dir, - mtype="udf", sync=False) + mtype="udf") else: ret = util.mount_cb(cdev, load_azure_ds_dir) else: diff --git a/cloudinit/sources/DataSourceConfigDrive.py b/cloudinit/sources/DataSourceConfigDrive.py index 564e3eb3..571d30dc 100644 --- a/cloudinit/sources/DataSourceConfigDrive.py +++ b/cloudinit/sources/DataSourceConfigDrive.py @@ -72,15 +72,12 @@ class DataSourceConfigDrive(openstack.SourceMixin, sources.DataSource): dslist = self.sys_cfg.get('datasource_list') for dev in find_candidate_devs(dslist=dslist): try: - # Set mtype if freebsd and turn off sync - if dev.startswith("/dev/cd"): + if util.is_FreeBSD() and dev.startswith("/dev/cd"): mtype = "cd9660" - sync = False else: mtype = None - sync = True results = util.mount_cb(dev, read_config_drive, - mtype=mtype, sync=sync) + mtype=mtype) found = dev except openstack.NonReadable: pass diff --git a/cloudinit/util.py b/cloudinit/util.py index 385f231c..ea4199cd 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1679,7 +1679,7 @@ def mounts(): return mounted -def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True, +def mount_cb(device, callback, data=None, mtype=None, update_env_for_mount=None): """ Mount the device, call method 'callback' passing the directory @@ -1726,18 +1726,7 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True, for mtype in mtypes: mountpoint = None try: - mountcmd = ['mount'] - mountopts = [] - if rw: - mountopts.append('rw') - else: - mountopts.append('ro') - if sync: - # This seems like the safe approach to do - # (ie where this is on by default) - mountopts.append("sync") - if mountopts: - mountcmd.extend(["-o", ",".join(mountopts)]) + mountcmd = ['mount', '-o', 'ro'] if mtype: mountcmd.extend(['-t', mtype]) mountcmd.append(device) -- cgit v1.2.3 From 6197c347c3960254dbcdb28eb73989d062ad9689 Mon Sep 17 00:00:00 2001 From: Gonéri Le Bouder Date: Tue, 28 May 2019 15:39:48 +0000 Subject: freebsd: ability to grow root file system - UFS file system support - GPT partition table support - add support for newfs's -L parameter (label) - move freebsd specific test from Azure to freebsd --- cloudinit/config/cc_growpart.py | 3 +- cloudinit/config/cc_resizefs.py | 6 +-- cloudinit/util.py | 22 ++++++----- tests/unittests/test_datasource/test_azure.py | 24 ------------ tests/unittests/test_distros/test_freebsd.py | 45 ++++++++++++++++++++++ .../test_handler/test_handler_resizefs.py | 2 +- 6 files changed, 64 insertions(+), 38 deletions(-) create mode 100644 tests/unittests/test_distros/test_freebsd.py (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_growpart.py b/cloudinit/config/cc_growpart.py index bafca9d8..564f376f 100644 --- a/cloudinit/config/cc_growpart.py +++ b/cloudinit/config/cc_growpart.py @@ -215,7 +215,8 @@ def device_part_info(devpath): # FreeBSD doesn't know of sysfs so just get everything we need from # the device, like /dev/vtbd0p2. if util.is_FreeBSD(): - m = re.search('^(/dev/.+)p([0-9])$', devpath) + freebsd_part = "/dev/" + util.find_freebsd_part(devpath) + m = re.search('^(/dev/.+)p([0-9])$', freebsd_part) return (m.group(1), m.group(2)) if not os.path.exists(syspath): diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py index 076b9d5a..afd2e060 100644 --- a/cloudinit/config/cc_resizefs.py +++ b/cloudinit/config/cc_resizefs.py @@ -81,7 +81,7 @@ def _resize_xfs(mount_point, devpth): def _resize_ufs(mount_point, devpth): - return ('growfs', '-y', devpth) + return ('growfs', '-y', mount_point) def _resize_zfs(mount_point, devpth): @@ -101,7 +101,7 @@ def _can_skip_resize_ufs(mount_point, devpth): """ # dumpfs -m / # newfs command for / (/dev/label/rootfs) - newfs -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384 + newfs -L rootf -O 2 -U -a 4 -b 32768 -d 32768 -e 4096 -f 4096 -g 16384 -h 64 -i 8192 -j -k 6408 -m 8 -o time -s 58719232 /dev/label/rootf """ cur_fs_sz = None @@ -110,7 +110,7 @@ def _can_skip_resize_ufs(mount_point, devpth): for line in dumpfs_res.splitlines(): if not line.startswith('#'): newfs_cmd = shlex.split(line) - opt_value = 'O:Ua:s:b:d:e:f:g:h:i:jk:m:o:' + opt_value = 'O:Ua:s:b:d:e:f:g:h:i:jk:m:o:L:' optlist, _args = getopt.getopt(newfs_cmd[1:], opt_value) for o, a in optlist: if o == "-s": diff --git a/cloudinit/util.py b/cloudinit/util.py index ea4199cd..aa23b3f3 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2337,17 +2337,21 @@ def parse_mtab(path): return None -def find_freebsd_part(label_part): - if label_part.startswith("/dev/label/"): - target_label = label_part[5:] - (label_part, _err) = subp(['glabel', 'status', '-s']) - for labels in label_part.split("\n"): +def find_freebsd_part(fs): + splitted = fs.split('/') + if len(splitted) == 3: + return splitted[2] + elif splitted[2] in ['label', 'gpt', 'ufs']: + target_label = fs[5:] + (part, _err) = subp(['glabel', 'status', '-s']) + for labels in part.split("\n"): items = labels.split() - if len(items) > 0 and items[0].startswith(target_label): - label_part = items[2] + if len(items) > 0 and items[0] == target_label: + part = items[2] break - label_part = str(label_part) - return label_part + return str(part) + else: + LOG.warning("Unexpected input in find_freebsd_part: %s", fs) def get_path_dev_freebsd(path, mnt_list): diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 427ab7e7..afb614e4 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -6,7 +6,6 @@ from cloudinit import url_helper from cloudinit.sources import ( UNSET, DataSourceAzure as dsaz, InvalidMetaDataException) from cloudinit.util import (b64e, decode_binary, load_file, write_file, - find_freebsd_part, get_path_dev_freebsd, MountFailedError, json_dumps, load_json) from cloudinit.version import version_string as vs from cloudinit.tests.helpers import ( @@ -391,29 +390,6 @@ scbus-1 on xpt0 bus 0 dev = ds.get_resource_disk_on_freebsd(1) self.assertEqual("da1", dev) - @mock.patch('cloudinit.util.subp') - def test_find_freebsd_part_on_Azure(self, mock_subp): - glabel_out = ''' -gptid/fa52d426-c337-11e6-8911-00155d4c5e47 N/A da0p1 - label/rootfs N/A da0p2 - label/swap N/A da0p3 -''' - mock_subp.return_value = (glabel_out, "") - res = find_freebsd_part("/dev/label/rootfs") - self.assertEqual("da0p2", res) - - def test_get_path_dev_freebsd_on_Azure(self): - mnt_list = ''' -/dev/label/rootfs / ufs rw 1 1 -devfs /dev devfs rw,multilabel 0 0 -fdescfs /dev/fd fdescfs rw 0 0 -/dev/da1s1 /mnt/resource ufs rw 2 2 -''' - with mock.patch.object(os.path, 'exists', - return_value=True): - res = get_path_dev_freebsd('/etc', mnt_list) - self.assertIsNotNone(res) - @mock.patch(MOCKPATH + '_is_platform_viable') def test_call_is_platform_viable_seed(self, m_is_platform_viable): """Check seed_dir using _is_platform_viable and return False.""" diff --git a/tests/unittests/test_distros/test_freebsd.py b/tests/unittests/test_distros/test_freebsd.py new file mode 100644 index 00000000..8af253a2 --- /dev/null +++ b/tests/unittests/test_distros/test_freebsd.py @@ -0,0 +1,45 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +from cloudinit.util import (find_freebsd_part, get_path_dev_freebsd) +from cloudinit.tests.helpers import (CiTestCase, mock) + +import os + + +class TestDeviceLookUp(CiTestCase): + + @mock.patch('cloudinit.util.subp') + def test_find_freebsd_part_label(self, mock_subp): + glabel_out = ''' +gptid/fa52d426-c337-11e6-8911-00155d4c5e47 N/A da0p1 + label/rootfs N/A da0p2 + label/swap N/A da0p3 +''' + mock_subp.return_value = (glabel_out, "") + res = find_freebsd_part("/dev/label/rootfs") + self.assertEqual("da0p2", res) + + @mock.patch('cloudinit.util.subp') + def test_find_freebsd_part_gpt(self, mock_subp): + glabel_out = ''' + gpt/bootfs N/A vtbd0p1 +gptid/3f4cbe26-75da-11e8-a8f2-002590ec6166 N/A vtbd0p1 + gpt/swapfs N/A vtbd0p2 + gpt/rootfs N/A vtbd0p3 + iso9660/cidata N/A vtbd2 +''' + mock_subp.return_value = (glabel_out, "") + res = find_freebsd_part("/dev/gpt/rootfs") + self.assertEqual("vtbd0p3", res) + + def test_get_path_dev_freebsd_label(self): + mnt_list = ''' +/dev/label/rootfs / ufs rw 1 1 +devfs /dev devfs rw,multilabel 0 0 +fdescfs /dev/fd fdescfs rw 0 0 +/dev/da1s1 /mnt/resource ufs rw 2 2 +''' + with mock.patch.object(os.path, 'exists', + return_value=True): + res = get_path_dev_freebsd('/etc', mnt_list) + self.assertIsNotNone(res) diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py index 35187847..db9a0414 100644 --- a/tests/unittests/test_handler/test_handler_resizefs.py +++ b/tests/unittests/test_handler/test_handler_resizefs.py @@ -147,7 +147,7 @@ class TestResizefs(CiTestCase): def test_resize_ufs_cmd_return(self): mount_point = '/' devpth = '/dev/sda2' - self.assertEqual(('growfs', '-y', devpth), + self.assertEqual(('growfs', '-y', mount_point), _resize_ufs(mount_point, devpth)) @mock.patch('cloudinit.util.is_container', return_value=False) -- cgit v1.2.3 From 067516d7bc917e4921b9f1424b7a64e92cae0ad2 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Fri, 27 Sep 2019 20:46:00 +0000 Subject: util: json.dumps on python 2.7 will handle UnicodeDecodeError on binary Since python 2.7 doesn't handle UnicodeDecodeErrors with the default handler LP: #1801364 --- cloudinit/sources/tests/test_init.py | 12 +++++------- cloudinit/tests/test_util.py | 20 ++++++++++++++++++++ cloudinit/util.py | 27 +++++++++++++++++++++++++-- 3 files changed, 50 insertions(+), 9 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 6378e98b..9698261b 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -457,19 +457,17 @@ class TestDataSource(CiTestCase): instance_json['ds']['meta_data']) @skipIf(not six.PY2, "Only python2 hits UnicodeDecodeErrors on non-utf8") - def test_non_utf8_encoding_logs_warning(self): - """When non-utf-8 values exist in py2 instance-data is not written.""" + def test_non_utf8_encoding_gets_b64encoded(self): + """When non-utf-8 values exist in py2 instance-data is b64encoded.""" tmp = self.tmp_dir() datasource = DataSourceTestSubclassNet( self.sys_cfg, self.distro, Paths({'run_dir': tmp}), custom_metadata={'key1': 'val1', 'key2': {'key2.1': b'ab\xaadef'}}) self.assertTrue(datasource.get_data()) json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) - self.assertFalse(os.path.exists(json_file)) - self.assertIn( - "WARNING: Error persisting instance-data.json: 'utf8' codec can't" - " decode byte 0xaa in position 2: invalid start byte", - self.logs.getvalue()) + instance_json = util.load_json(util.load_file(json_file)) + key21_value = instance_json['ds']['meta_data']['key2']['key2.1'] + self.assertEqual('ci-b64:' + util.b64e(b'ab\xaadef'), key21_value) def test_get_hostname_subclass_support(self): """Validate get_hostname signature on all subclasses of DataSource.""" diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index e3d2dbaa..f4f95e92 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -2,7 +2,9 @@ """Tests for cloudinit.util""" +import base64 import logging +import json import platform import cloudinit.util as util @@ -528,6 +530,24 @@ class TestGetLinuxDistro(CiTestCase): self.assertEqual(('foo', '1.1', 'aarch64'), dist) +class TestJsonDumps(CiTestCase): + def test_is_str(self): + """json_dumps should return a string.""" + self.assertTrue(isinstance(util.json_dumps({'abc': '123'}), str)) + + def test_utf8(self): + smiley = '\\ud83d\\ude03' + self.assertEqual( + {'smiley': smiley}, + json.loads(util.json_dumps({'smiley': smiley}))) + + def test_non_utf8(self): + blob = b'\xba\x03Qx-#y\xea' + self.assertEqual( + {'blob': 'ci-b64:' + base64.b64encode(blob).decode('utf-8')}, + json.loads(util.json_dumps({'blob': blob}))) + + @mock.patch('os.path.exists') class TestIsLXD(CiTestCase): diff --git a/cloudinit/util.py b/cloudinit/util.py index aa23b3f3..6e8e73b0 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1599,10 +1599,33 @@ def json_serialize_default(_obj): return 'Warning: redacted unserializable type {0}'.format(type(_obj)) +def json_preserialize_binary(data): + """Preserialize any discovered binary values to avoid json.dumps issues. + + Used only on python 2.7 where default type handling is not honored for + failure to encode binary data. LP: #1801364. + TODO(Drop this function when py2.7 support is dropped from cloud-init) + """ + data = obj_copy.deepcopy(data) + for key, value in data.items(): + if isinstance(value, (dict)): + data[key] = json_preserialize_binary(value) + if isinstance(value, bytes): + data[key] = 'ci-b64:{0}'.format(b64e(value)) + return data + + def json_dumps(data): """Return data in nicely formatted json.""" - return json.dumps(data, indent=1, sort_keys=True, - separators=(',', ': '), default=json_serialize_default) + try: + return json.dumps( + data, indent=1, sort_keys=True, separators=(',', ': '), + default=json_serialize_default) + except UnicodeDecodeError: + if sys.version_info[:2] == (2, 7): + data = json_preserialize_binary(data) + return json.dumps(data) + raise def yaml_dumps(obj, explicit_start=True, explicit_end=True, noalias=False): -- cgit v1.2.3 From 5d5a32e039782ce3e1c0843082fe26260fa9273a Mon Sep 17 00:00:00 2001 From: Conrad Hoffmann Date: Tue, 1 Oct 2019 20:43:29 +0000 Subject: Add support for Arch Linux in render-cloudcfg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit  - Detect Arch Linux and set variant accordingly in `system_info()`  - Allow setting render-cloudcfg variant parameter to 'arch'  - Adjust some basic settings for Arch Linux in the cloud.cfg.tmpl The template might need some additional Arch-specific tweaks in the future, but at least for now the generated config works and contains the most relevant modules. Also: - Sort distro variant lists when adding Arch - Add debian to known variants in render-cloudcfg --- cloudinit/util.py | 3 ++- config/cloud.cfg.tmpl | 6 ++++-- tools/render-cloudcfg | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index 6e8e73b0..0d338ca7 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -656,7 +656,8 @@ def system_info(): var = 'unknown' if system == "linux": linux_dist = info['dist'][0].lower() - if linux_dist in ('centos', 'debian', 'fedora', 'rhel', 'suse'): + if linux_dist in ( + 'arch', 'centos', 'debian', 'fedora', 'rhel', 'suse'): var = linux_dist elif linux_dist in ('ubuntu', 'linuxmint', 'mint'): var = 'ubuntu' diff --git a/config/cloud.cfg.tmpl b/config/cloud.cfg.tmpl index 684c7473..87c37ba0 100644 --- a/config/cloud.cfg.tmpl +++ b/config/cloud.cfg.tmpl @@ -137,7 +137,7 @@ cloud_final_modules: # (not accessible to handlers/transforms) system_info: # This will affect which distro class gets used -{% if variant in ["centos", "debian", "fedora", "rhel", "suse", "ubuntu", "freebsd"] %} +{% if variant in ["arch", "centos", "debian", "fedora", "freebsd", "rhel", "suse", "ubuntu"] %} distro: {{ variant }} {% else %} # Unknown/fallback distro. @@ -185,7 +185,7 @@ system_info: primary: http://ports.ubuntu.com/ubuntu-ports security: http://ports.ubuntu.com/ubuntu-ports ssh_svcname: ssh -{% elif variant in ["centos", "rhel", "fedora", "suse"] %} +{% elif variant in ["arch", "centos", "fedora", "rhel", "suse"] %} # Default user name + that default users groups (if added/used) default_user: name: {{ variant }} @@ -193,6 +193,8 @@ system_info: gecos: {{ variant }} Cloud User {% if variant == "suse" %} groups: [cdrom, users] +{% elif variant == "arch" %} + groups: [wheel, users] {% else %} groups: [wheel, adm, systemd-journal] {% endif %} diff --git a/tools/render-cloudcfg b/tools/render-cloudcfg index 0957c324..a441f4ff 100755 --- a/tools/render-cloudcfg +++ b/tools/render-cloudcfg @@ -4,7 +4,8 @@ import argparse import os import sys -VARIANTS = ["freebsd", "centos", "fedora", "rhel", "suse", "ubuntu", "unknown"] +VARIANTS = ["arch", "centos", "debian", "fedora", "freebsd", "rhel", "suse", + "ubuntu", "unknown"] if "avoid-pep8-E402-import-not-top-of-file": _tdir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) -- cgit v1.2.3 From 5bec6b0e2a2ce5fd03bb04f441536fc130e67997 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Thu, 24 Oct 2019 20:02:15 +0000 Subject: Fix usages of yaml, and move yaml_dump to safeyaml.dumps. Here we replace uses of the pyyaml module directly with functions provided by cloudinit.safeyaml. Also, change/move cloudinit.util.yaml_dumps to cloudinit.safeyaml.dumps LP: #1849640 --- cloudinit/cmd/devel/net_convert.py | 13 +++++-------- cloudinit/cmd/tests/test_main.py | 7 ++++--- cloudinit/config/cc_debug.py | 3 ++- cloudinit/config/cc_salt_minion.py | 6 +++--- cloudinit/config/cc_snappy.py | 3 ++- cloudinit/handlers/cloud_config.py | 3 ++- cloudinit/net/netplan.py | 15 ++++++++------- cloudinit/net/network_state.py | 5 +++-- cloudinit/safeyaml.py | 15 +++++++++++++++ cloudinit/util.py | 16 +--------------- tests/unittests/test_data.py | 3 ++- tests/unittests/test_runs/test_merge_run.py | 3 ++- tests/unittests/test_runs/test_simple_run.py | 7 ++++--- 13 files changed, 53 insertions(+), 46 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/cmd/devel/net_convert.py b/cloudinit/cmd/devel/net_convert.py index 1ad7e0bd..9b768304 100755 --- a/cloudinit/cmd/devel/net_convert.py +++ b/cloudinit/cmd/devel/net_convert.py @@ -5,13 +5,12 @@ import argparse import json import os import sys -import yaml from cloudinit.sources.helpers import openstack from cloudinit.sources import DataSourceAzure as azure from cloudinit.sources import DataSourceOVF as ovf -from cloudinit import distros +from cloudinit import distros, safeyaml from cloudinit.net import eni, netplan, network_state, sysconfig from cloudinit import log @@ -78,13 +77,12 @@ def handle_args(name, args): if args.kind == "eni": pre_ns = eni.convert_eni_data(net_data) elif args.kind == "yaml": - pre_ns = yaml.load(net_data) + pre_ns = safeyaml.load(net_data) if 'network' in pre_ns: pre_ns = pre_ns.get('network') if args.debug: sys.stderr.write('\n'.join( - ["Input YAML", - yaml.dump(pre_ns, default_flow_style=False, indent=4), ""])) + ["Input YAML", safeyaml.dumps(pre_ns), ""])) elif args.kind == 'network_data.json': pre_ns = openstack.convert_net_json( json.loads(net_data), known_macs=known_macs) @@ -100,9 +98,8 @@ def handle_args(name, args): "input data") if args.debug: - sys.stderr.write('\n'.join([ - "", "Internal State", - yaml.dump(ns, default_flow_style=False, indent=4), ""])) + sys.stderr.write('\n'.join( + ["", "Internal State", safeyaml.dumps(ns), ""])) distro_cls = distros.fetch(args.distro) distro = distro_cls(args.distro, {}, None) config = {} diff --git a/cloudinit/cmd/tests/test_main.py b/cloudinit/cmd/tests/test_main.py index a1e534fb..57b8fdf5 100644 --- a/cloudinit/cmd/tests/test_main.py +++ b/cloudinit/cmd/tests/test_main.py @@ -6,8 +6,9 @@ import os from six import StringIO from cloudinit.cmd import main +from cloudinit import safeyaml from cloudinit.util import ( - ensure_dir, load_file, write_file, yaml_dumps) + ensure_dir, load_file, write_file) from cloudinit.tests.helpers import ( FilesystemMockingTestCase, wrap_and_call) @@ -39,7 +40,7 @@ class TestMain(FilesystemMockingTestCase): ], 'cloud_init_modules': ['write-files', 'runcmd'], } - cloud_cfg = yaml_dumps(self.cfg) + cloud_cfg = safeyaml.dumps(self.cfg) ensure_dir(os.path.join(self.new_root, 'etc', 'cloud')) self.cloud_cfg_file = os.path.join( self.new_root, 'etc', 'cloud', 'cloud.cfg') @@ -113,7 +114,7 @@ class TestMain(FilesystemMockingTestCase): """When local-hostname metadata is present, call cc_set_hostname.""" self.cfg['datasource'] = { 'None': {'metadata': {'local-hostname': 'md-hostname'}}} - cloud_cfg = yaml_dumps(self.cfg) + cloud_cfg = safeyaml.dumps(self.cfg) write_file(self.cloud_cfg_file, cloud_cfg) cmdargs = myargs( debug=False, files=None, force=False, local=False, reporter=None, diff --git a/cloudinit/config/cc_debug.py b/cloudinit/config/cc_debug.py index 0a039eb3..610dbc8b 100644 --- a/cloudinit/config/cc_debug.py +++ b/cloudinit/config/cc_debug.py @@ -33,6 +33,7 @@ from six import StringIO from cloudinit import type_utils from cloudinit import util +from cloudinit import safeyaml SKIP_KEYS = frozenset(['log_cfgs']) @@ -49,7 +50,7 @@ def _make_header(text): def _dumps(obj): - text = util.yaml_dumps(obj, explicit_start=False, explicit_end=False) + text = safeyaml.dumps(obj, explicit_start=False, explicit_end=False) return text.rstrip() diff --git a/cloudinit/config/cc_salt_minion.py b/cloudinit/config/cc_salt_minion.py index d6a21d72..cd9cb0b0 100644 --- a/cloudinit/config/cc_salt_minion.py +++ b/cloudinit/config/cc_salt_minion.py @@ -45,7 +45,7 @@ specify them with ``pkg_name``, ``service_name`` and ``config_dir``. import os -from cloudinit import util +from cloudinit import safeyaml, util # Note: see https://docs.saltstack.com/en/latest/topics/installation/ # Note: see https://docs.saltstack.com/en/latest/ref/configuration/ @@ -97,13 +97,13 @@ def handle(name, cfg, cloud, log, _args): if 'conf' in s_cfg: # Add all sections from the conf object to minion config file minion_config = os.path.join(const.conf_dir, 'minion') - minion_data = util.yaml_dumps(s_cfg.get('conf')) + minion_data = safeyaml.dumps(s_cfg.get('conf')) util.write_file(minion_config, minion_data) if 'grains' in s_cfg: # add grains to /etc/salt/grains grains_config = os.path.join(const.conf_dir, 'grains') - grains_data = util.yaml_dumps(s_cfg.get('grains')) + grains_data = safeyaml.dumps(s_cfg.get('grains')) util.write_file(grains_config, grains_data) # ... copy the key pair if specified diff --git a/cloudinit/config/cc_snappy.py b/cloudinit/config/cc_snappy.py index 15bee2d3..b94cd04e 100644 --- a/cloudinit/config/cc_snappy.py +++ b/cloudinit/config/cc_snappy.py @@ -68,6 +68,7 @@ is ``auto``. Options are: from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE from cloudinit import temp_utils +from cloudinit import safeyaml from cloudinit import util import glob @@ -188,7 +189,7 @@ def render_snap_op(op, name, path=None, cfgfile=None, config=None): # Note, however, we do not touch config files on disk. nested_cfg = {'config': {shortname: config}} (fd, cfg_tmpf) = temp_utils.mkstemp() - os.write(fd, util.yaml_dumps(nested_cfg).encode()) + os.write(fd, safeyaml.dumps(nested_cfg).encode()) os.close(fd) cfgfile = cfg_tmpf diff --git a/cloudinit/handlers/cloud_config.py b/cloudinit/handlers/cloud_config.py index 99bf0e61..2a307364 100644 --- a/cloudinit/handlers/cloud_config.py +++ b/cloudinit/handlers/cloud_config.py @@ -14,6 +14,7 @@ from cloudinit import handlers from cloudinit import log as logging from cloudinit import mergers from cloudinit import util +from cloudinit import safeyaml from cloudinit.settings import (PER_ALWAYS) @@ -75,7 +76,7 @@ class CloudConfigPartHandler(handlers.Handler): '', ] lines.extend(file_lines) - lines.append(util.yaml_dumps(self.cloud_buf)) + lines.append(safeyaml.dumps(self.cloud_buf)) else: lines = [] util.write_file(self.cloud_fn, "\n".join(lines), 0o600) diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py index e54a34e5..54be1221 100644 --- a/cloudinit/net/netplan.py +++ b/cloudinit/net/netplan.py @@ -8,6 +8,7 @@ from .network_state import subnet_is_ipv6, NET_CONFIG_TO_V2 from cloudinit import log as logging from cloudinit import util +from cloudinit import safeyaml from cloudinit.net import SYS_CLASS_NET, get_devicelist KNOWN_SNAPD_CONFIG = b"""\ @@ -235,9 +236,9 @@ class Renderer(renderer.Renderer): # if content already in netplan format, pass it back if network_state.version == 2: LOG.debug('V2 to V2 passthrough') - return util.yaml_dumps({'network': network_state.config}, - explicit_start=False, - explicit_end=False) + return safeyaml.dumps({'network': network_state.config}, + explicit_start=False, + explicit_end=False) ethernets = {} wifis = {} @@ -359,10 +360,10 @@ class Renderer(renderer.Renderer): # workaround yaml dictionary key sorting when dumping def _render_section(name, section): if section: - dump = util.yaml_dumps({name: section}, - explicit_start=False, - explicit_end=False, - noalias=True) + dump = safeyaml.dumps({name: section}, + explicit_start=False, + explicit_end=False, + noalias=True) txt = util.indent(dump, ' ' * 4) return [txt] return [] diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index c0c415d0..b485f3d9 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -12,6 +12,7 @@ import struct import six +from cloudinit import safeyaml from cloudinit import util LOG = logging.getLogger(__name__) @@ -253,7 +254,7 @@ class NetworkStateInterpreter(object): 'config': self._config, 'network_state': self._network_state, } - return util.yaml_dumps(state) + return safeyaml.dumps(state) def load(self, state): if 'version' not in state: @@ -272,7 +273,7 @@ class NetworkStateInterpreter(object): setattr(self, key, state[key]) def dump_network_state(self): - return util.yaml_dumps(self._network_state) + return safeyaml.dumps(self._network_state) def as_dict(self): return {'version': self._version, 'config': self._config} diff --git a/cloudinit/safeyaml.py b/cloudinit/safeyaml.py index 3bd5e03d..d6f5f95b 100644 --- a/cloudinit/safeyaml.py +++ b/cloudinit/safeyaml.py @@ -6,6 +6,8 @@ import yaml +YAMLError = yaml.YAMLError + class _CustomSafeLoader(yaml.SafeLoader): def construct_python_unicode(self, node): @@ -27,4 +29,17 @@ class NoAliasSafeDumper(yaml.dumper.SafeDumper): def load(blob): return(yaml.load(blob, Loader=_CustomSafeLoader)) + +def dumps(obj, explicit_start=True, explicit_end=True, noalias=False): + """Return data in nicely formatted yaml.""" + + return yaml.dump(obj, + line_break="\n", + indent=4, + explicit_start=explicit_start, + explicit_end=explicit_end, + default_flow_style=False, + Dumper=(NoAliasSafeDumper + if noalias else yaml.dumper.Dumper)) + # vi: ts=4 expandtab diff --git a/cloudinit/util.py b/cloudinit/util.py index 0d338ca7..1f600df4 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -38,7 +38,6 @@ from base64 import b64decode, b64encode from six.moves.urllib import parse as urlparse import six -import yaml from cloudinit import importer from cloudinit import log as logging @@ -958,7 +957,7 @@ def load_yaml(blob, default=None, allowed=(dict,)): " but got %s instead") % (allowed, type_utils.obj_name(converted))) loaded = converted - except (yaml.YAMLError, TypeError, ValueError) as e: + except (safeyaml.YAMLError, TypeError, ValueError) as e: msg = 'Failed loading yaml blob' mark = None if hasattr(e, 'context_mark') and getattr(e, 'context_mark'): @@ -1629,19 +1628,6 @@ def json_dumps(data): raise -def yaml_dumps(obj, explicit_start=True, explicit_end=True, noalias=False): - """Return data in nicely formatted yaml.""" - - return yaml.dump(obj, - line_break="\n", - indent=4, - explicit_start=explicit_start, - explicit_end=explicit_end, - default_flow_style=False, - Dumper=(safeyaml.NoAliasSafeDumper - if noalias else yaml.dumper.Dumper)) - - def ensure_dir(path, mode=None): if not os.path.isdir(path): # Make the dir and adjust the mode diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py index 3efe7adf..22cf8f28 100644 --- a/tests/unittests/test_data.py +++ b/tests/unittests/test_data.py @@ -27,6 +27,7 @@ from cloudinit.settings import (PER_INSTANCE) from cloudinit import sources from cloudinit import stages from cloudinit import user_data as ud +from cloudinit import safeyaml from cloudinit import util from cloudinit.tests import helpers @@ -502,7 +503,7 @@ c: 4 data = [{'content': '#cloud-config\npassword: gocubs\n'}, {'content': '#cloud-config\nlocale: chicago\n'}, {'content': non_decodable}] - message = b'#cloud-config-archive\n' + util.yaml_dumps(data).encode() + message = b'#cloud-config-archive\n' + safeyaml.dumps(data).encode() self.reRoot() ci = stages.Init() diff --git a/tests/unittests/test_runs/test_merge_run.py b/tests/unittests/test_runs/test_merge_run.py index d1ac4942..ff27a280 100644 --- a/tests/unittests/test_runs/test_merge_run.py +++ b/tests/unittests/test_runs/test_merge_run.py @@ -7,6 +7,7 @@ import tempfile from cloudinit.tests import helpers from cloudinit.settings import PER_INSTANCE +from cloudinit import safeyaml from cloudinit import stages from cloudinit import util @@ -26,7 +27,7 @@ class TestMergeRun(helpers.FilesystemMockingTestCase): 'system_info': {'paths': {'run_dir': new_root}} } ud = helpers.readResource('user_data.1.txt') - cloud_cfg = util.yaml_dumps(cfg) + cloud_cfg = safeyaml.dumps(cfg) util.ensure_dir(os.path.join(new_root, 'etc', 'cloud')) util.write_file(os.path.join(new_root, 'etc', 'cloud', 'cloud.cfg'), cloud_cfg) diff --git a/tests/unittests/test_runs/test_simple_run.py b/tests/unittests/test_runs/test_simple_run.py index d67c422c..cb3aae60 100644 --- a/tests/unittests/test_runs/test_simple_run.py +++ b/tests/unittests/test_runs/test_simple_run.py @@ -5,6 +5,7 @@ import os from cloudinit.settings import PER_INSTANCE +from cloudinit import safeyaml from cloudinit import stages from cloudinit.tests import helpers from cloudinit import util @@ -34,7 +35,7 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): ], 'cloud_init_modules': ['write-files', 'spacewalk', 'runcmd'], } - cloud_cfg = util.yaml_dumps(self.cfg) + cloud_cfg = safeyaml.dumps(self.cfg) util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud')) util.write_file(os.path.join(self.new_root, 'etc', 'cloud', 'cloud.cfg'), cloud_cfg) @@ -130,7 +131,7 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): # re-write cloud.cfg with unverified_modules override cfg = copy.deepcopy(self.cfg) cfg['unverified_modules'] = ['spacewalk'] # Would have skipped - cloud_cfg = util.yaml_dumps(cfg) + cloud_cfg = safeyaml.dumps(cfg) util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud')) util.write_file(os.path.join(self.new_root, 'etc', 'cloud', 'cloud.cfg'), cloud_cfg) @@ -159,7 +160,7 @@ class TestSimpleRun(helpers.FilesystemMockingTestCase): cfg = copy.deepcopy(self.cfg) # Represent empty configuration in /etc/cloud/cloud.cfg cfg['cloud_init_modules'] = None - cloud_cfg = util.yaml_dumps(cfg) + cloud_cfg = safeyaml.dumps(cfg) util.ensure_dir(os.path.join(self.new_root, 'etc', 'cloud')) util.write_file(os.path.join(self.new_root, 'etc', 'cloud', 'cloud.cfg'), cloud_cfg) -- cgit v1.2.3 From 2a135c4a421af47f5bd511e89e385a72f62bde33 Mon Sep 17 00:00:00 2001 From: Igor Galić Date: Mon, 25 Nov 2019 23:10:50 +0100 Subject: FreeBSD: fix for get_linux_distro() and lru_cache (#59) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since `is_FreeBSD()` is used a lot, which uses `system_info()`, which uses `get_linux_distro()` we add caching, by decorating the following functions with `@lru_cache`: - get_architecture() - _lsb_release() - is_FreeBSD - get_linux_distro - system_info() - _get_cmdline() Since [functools](https://docs.python.org/3/library/functools.html) only exists in Python 3, only python 3 will benefit from this improvement. For python 2, our shim is just a pass-thru. Too bad, but, also… https://pythonclock.org/ The main motivation here was, at first, to cache more, following the style of _lsb_release. That is now consolidated under this very same roof. LP: #1815030 --- cloudinit/tests/test_util.py | 19 ++++++++++++++++++ cloudinit/util.py | 47 +++++++++++++++++++++++++++----------------- tests/unittests/test_net.py | 19 ++++++++++++------ 3 files changed, 61 insertions(+), 24 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index f4f95e92..64ed82ea 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -387,6 +387,11 @@ class TestUdevadmSettle(CiTestCase): @mock.patch('os.path.exists') class TestGetLinuxDistro(CiTestCase): + def setUp(self): + # python2 has no lru_cache, and therefore, no cache_clear() + if hasattr(util.get_linux_distro, "cache_clear"): + util.get_linux_distro.cache_clear() + @classmethod def os_release_exists(self, path): """Side effect function""" @@ -399,6 +404,12 @@ class TestGetLinuxDistro(CiTestCase): if path == '/etc/redhat-release': return 1 + @classmethod + def freebsd_version_exists(self, path): + """Side effect function """ + if path == '/bin/freebsd-version': + return 1 + @mock.patch('cloudinit.util.load_file') def test_get_linux_distro_quoted_name(self, m_os_release, m_path_exists): """Verify we get the correct name if the os-release file has @@ -417,6 +428,14 @@ class TestGetLinuxDistro(CiTestCase): dist = util.get_linux_distro() self.assertEqual(('ubuntu', '16.04', 'xenial'), dist) + @mock.patch('cloudinit.util.subp') + def test_get_linux_freebsd(self, m_subp, m_path_exists): + """Verify we get the correct name and release name on FreeBSD.""" + m_path_exists.side_effect = TestGetLinuxDistro.freebsd_version_exists + m_subp.return_value = ("12.0-RELEASE-p10\n", '') + dist = util.get_linux_distro() + self.assertEqual(('freebsd', '12.0-RELEASE-p10', ''), dist) + @mock.patch('cloudinit.util.load_file') def test_get_linux_centos6(self, m_os_release, m_path_exists): """Verify we get the correct name and release name on CentOS 6.""" diff --git a/cloudinit/util.py b/cloudinit/util.py index 1f600df4..c498414d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -50,6 +50,16 @@ from cloudinit import version from cloudinit.settings import (CFG_BUILTIN) +try: + from functools import lru_cache +except ImportError: + def lru_cache(): + """pass-thru replace for Python3's lru_cache()""" + def wrapper(f): + return f + return wrapper + + _DNS_REDIRECT_IP = None LOG = logging.getLogger(__name__) @@ -68,17 +78,15 @@ CONTAINER_TESTS = (['systemd-detect-virt', '--quiet', '--container'], ['running-in-container'], ['lxc-is-container']) -PROC_CMDLINE = None - -_LSB_RELEASE = {} - +@lru_cache() def get_architecture(target=None): out, _ = subp(['dpkg', '--print-architecture'], capture=True, target=target) return out.strip() +@lru_cache() def _lsb_release(target=None): fmap = {'Codename': 'codename', 'Description': 'description', 'Distributor ID': 'id', 'Release': 'release'} @@ -107,11 +115,7 @@ def lsb_release(target=None): # do not use or update cache if target is provided return _lsb_release(target) - global _LSB_RELEASE - if not _LSB_RELEASE: - data = _lsb_release() - _LSB_RELEASE.update(data) - return _LSB_RELEASE + return _lsb_release() def target_path(target, path=None): @@ -546,6 +550,7 @@ def is_ipv4(instr): return len(toks) == 4 +@lru_cache() def is_FreeBSD(): return system_info()['variant'] == "freebsd" @@ -595,6 +600,7 @@ def _parse_redhat_release(release_file=None): return {} +@lru_cache() def get_linux_distro(): distro_name = '' distro_version = '' @@ -622,6 +628,10 @@ def get_linux_distro(): flavor = match.groupdict()['codename'] if distro_name == 'rhel': distro_name = 'redhat' + elif os.path.exists('/bin/freebsd-version'): + distro_name = 'freebsd' + distro_version, _ = subp(['uname', '-r']) + distro_version = distro_version.strip() else: dist = ('', '', '') try: @@ -642,6 +652,7 @@ def get_linux_distro(): return (distro_name, distro_version, flavor) +@lru_cache() def system_info(): info = { 'platform': platform.platform(), @@ -1371,14 +1382,8 @@ def load_file(fname, read_cb=None, quiet=False, decode=True): return contents -def get_cmdline(): - if 'DEBUG_PROC_CMDLINE' in os.environ: - return os.environ["DEBUG_PROC_CMDLINE"] - - global PROC_CMDLINE - if PROC_CMDLINE is not None: - return PROC_CMDLINE - +@lru_cache() +def _get_cmdline(): if is_container(): try: contents = load_file("/proc/1/cmdline") @@ -1393,10 +1398,16 @@ def get_cmdline(): except Exception: cmdline = "" - PROC_CMDLINE = cmdline return cmdline +def get_cmdline(): + if 'DEBUG_PROC_CMDLINE' in os.environ: + return os.environ["DEBUG_PROC_CMDLINE"] + + return _get_cmdline() + + def pipe_in_out(in_fh, out_fh, chunk_size=1024, chunk_cb=None): bytes_piped = 0 while True: diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 0f45dc38..01119e0a 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -4576,6 +4576,7 @@ class TestNetRenderers(CiTestCase): priority=['sysconfig', 'eni']) @mock.patch("cloudinit.net.renderers.netplan.available") + @mock.patch("cloudinit.net.renderers.sysconfig.available") @mock.patch("cloudinit.net.renderers.sysconfig.available_sysconfig") @mock.patch("cloudinit.net.renderers.sysconfig.available_nm") @mock.patch("cloudinit.net.renderers.eni.available") @@ -4583,14 +4584,16 @@ class TestNetRenderers(CiTestCase): def test_sysconfig_selected_on_sysconfig_enabled_distros(self, m_distro, m_eni, m_sys_nm, m_sys_scfg, + m_sys_avail, m_netplan): """sysconfig only selected on specific distros (rhel/sles).""" # Ubuntu with Network-Manager installed - m_eni.return_value = False # no ifupdown (ifquery) - m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown - m_sys_nm.return_value = True # network-manager is installed - m_netplan.return_value = True # netplan is installed + m_eni.return_value = False # no ifupdown (ifquery) + m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown + m_sys_nm.return_value = True # network-manager is installed + m_netplan.return_value = True # netplan is installed + m_sys_avail.return_value = False # no sysconfig on Ubuntu m_distro.return_value = ('ubuntu', None, None) self.assertEqual('netplan', renderers.select(priority=None)[0]) @@ -4598,7 +4601,8 @@ class TestNetRenderers(CiTestCase): m_eni.return_value = False # no ifupdown (ifquery) m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown m_sys_nm.return_value = True # network-manager is installed - m_netplan.return_value = False # netplan is not installed + m_netplan.return_value = False # netplan is not installed + m_sys_avail.return_value = True # sysconfig is available on centos m_distro.return_value = ('centos', None, None) self.assertEqual('sysconfig', renderers.select(priority=None)[0]) @@ -4606,7 +4610,8 @@ class TestNetRenderers(CiTestCase): m_eni.return_value = False # no ifupdown (ifquery) m_sys_scfg.return_value = False # no sysconfig/ifup/ifdown m_sys_nm.return_value = True # network-manager is installed - m_netplan.return_value = False # netplan is not installed + m_netplan.return_value = False # netplan is not installed + m_sys_avail.return_value = True # sysconfig is available on opensuse m_distro.return_value = ('opensuse', None, None) self.assertEqual('sysconfig', renderers.select(priority=None)[0]) @@ -4625,6 +4630,8 @@ class TestNetRenderers(CiTestCase): ] for (distro_name, distro_version, flavor) in distro_values: m_distro.return_value = (distro_name, distro_version, flavor) + if hasattr(util.system_info, "cache_clear"): + util.system_info.cache_clear() result = sysconfig.available() self.assertTrue(result) -- cgit v1.2.3 From 10885c57a7e6539f12966d1fcc89dcf528c926ae Mon Sep 17 00:00:00 2001 From: Igor Galić Date: Wed, 27 Nov 2019 22:32:19 +0100 Subject: dmidecode: log result *after* stripping \n This makes for a slightly prettier and less confusing log. --- cloudinit/util.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index c498414d..78b6a2d0 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -2691,8 +2691,8 @@ def _call_dmidecode(key, dmidecode_path): try: cmd = [dmidecode_path, "--string", key] (result, _err) = subp(cmd) - LOG.debug("dmidecode returned '%s' for '%s'", result, key) result = result.strip() + LOG.debug("dmidecode returned '%s' for '%s'", result, key) if result.replace(".", "") == "": return "" return result -- cgit v1.2.3 From f69d33a723b805fec3ee70c3a6127c8cadcb02d8 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Mon, 2 Dec 2019 16:24:18 -0700 Subject: url_helper: read_file_or_url should pass headers param into readurl (#66) Headers param was accidentally omitted and no longer passed through to readurl due to a previous commit. To avoid this omission of params in the future, drop positional param definitions from read_file_or_url and pass all kwargs through to readurl when we are not operating on a file. In util:read_seeded, correct the case where invalid positional param file_retries was being passed into read_file_or_url. Also drop duplicated file:// prefix addition from read_seeded because read_file_or_url does that work anyway. LP: #1854084 --- cloudinit/sources/helpers/azure.py | 6 ++- cloudinit/tests/test_url_helper.py | 52 ++++++++++++++++++++++ cloudinit/url_helper.py | 47 +++++++++++++++---- cloudinit/user_data.py | 2 +- cloudinit/util.py | 15 ++----- .../unittests/test_datasource/test_azure_helper.py | 18 +++++--- 6 files changed, 112 insertions(+), 28 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index f1fba175..f5cdb3fd 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -183,14 +183,16 @@ class AzureEndpointHttpClient(object): if secure: headers = self.headers.copy() headers.update(self.extra_secure_headers) - return url_helper.read_file_or_url(url, headers=headers) + return url_helper.read_file_or_url(url, headers=headers, timeout=5, + retries=10) def post(self, url, data=None, extra_headers=None): headers = self.headers if extra_headers is not None: headers = self.headers.copy() headers.update(extra_headers) - return url_helper.read_file_or_url(url, data=data, headers=headers) + return url_helper.read_file_or_url(url, data=data, headers=headers, + timeout=5, retries=10) class GoalState(object): diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py index aa9f3ec1..e883ddc2 100644 --- a/cloudinit/tests/test_url_helper.py +++ b/cloudinit/tests/test_url_helper.py @@ -4,6 +4,7 @@ from cloudinit.url_helper import ( NOT_FOUND, UrlError, oauth_headers, read_file_or_url, retry_on_url_exc) from cloudinit.tests.helpers import CiTestCase, mock, skipIf from cloudinit import util +from cloudinit import version import httpretty import requests @@ -17,6 +18,9 @@ except ImportError: _missing_oauthlib_dep = True +M_PATH = 'cloudinit.url_helper.' + + class TestOAuthHeaders(CiTestCase): def test_oauth_headers_raises_not_implemented_when_oathlib_missing(self): @@ -67,6 +71,54 @@ class TestReadFileOrUrl(CiTestCase): self.assertEqual(result.contents, data) self.assertEqual(str(result), data.decode('utf-8')) + @mock.patch(M_PATH + 'readurl') + def test_read_file_or_url_passes_params_to_readurl(self, m_readurl): + """read_file_or_url passes all params through to readurl.""" + url = 'http://hostname/path' + response = 'This is my url content\n' + m_readurl.return_value = response + params = {'url': url, 'timeout': 1, 'retries': 2, + 'headers': {'somehdr': 'val'}, + 'data': 'data', 'sec_between': 1, + 'ssl_details': {'cert_file': '/path/cert.pem'}, + 'headers_cb': 'headers_cb', 'exception_cb': 'exception_cb'} + self.assertEqual(response, read_file_or_url(**params)) + params.pop('url') # url is passed in as a positional arg + self.assertEqual([mock.call(url, **params)], m_readurl.call_args_list) + + def test_wb_read_url_defaults_honored_by_read_file_or_url_callers(self): + """Readurl param defaults used when unspecified by read_file_or_url + + Param defaults tested are as follows: + retries: 0, additional headers None beyond default, method: GET, + data: None, check_status: True and allow_redirects: True + """ + url = 'http://hostname/path' + + m_response = mock.MagicMock() + + class FakeSession(requests.Session): + def request(cls, **kwargs): + self.assertEqual( + {'url': url, 'allow_redirects': True, 'method': 'GET', + 'headers': { + 'User-Agent': 'Cloud-Init/%s' % ( + version.version_string())}}, + kwargs) + return m_response + + with mock.patch(M_PATH + 'requests.Session') as m_session: + error = requests.exceptions.HTTPError('broke') + m_session.side_effect = [error, FakeSession()] + # assert no retries and check_status == True + with self.assertRaises(UrlError) as context_manager: + response = read_file_or_url(url) + self.assertEqual('broke', str(context_manager.exception)) + # assert default headers, method, url and allow_redirects True + # Success on 2nd call with FakeSession + response = read_file_or_url(url) + self.assertEqual(m_response, response._response) + class TestRetryOnUrlExc(CiTestCase): diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py index 48ddae45..1496a471 100644 --- a/cloudinit/url_helper.py +++ b/cloudinit/url_helper.py @@ -81,14 +81,19 @@ def combine_url(base, *add_ons): return url -def read_file_or_url(url, timeout=5, retries=10, - headers=None, data=None, sec_between=1, ssl_details=None, - headers_cb=None, exception_cb=None): +def read_file_or_url(url, **kwargs): + """Wrapper function around readurl to allow passing a file path as url. + + When url is not a local file path, passthrough any kwargs to readurl. + + In the case of parameter passthrough to readurl, default values for some + parameters. See: call-signature of readurl in this module for param docs. + """ url = url.lstrip() if url.startswith("/"): url = "file://%s" % url if url.lower().startswith("file://"): - if data: + if kwargs.get("data"): LOG.warning("Unable to post data to file resource %s", url) file_path = url[len("file://"):] try: @@ -101,10 +106,7 @@ def read_file_or_url(url, timeout=5, retries=10, raise UrlError(cause=e, code=code, headers=None, url=url) return FileResponse(file_path, contents=contents) else: - return readurl(url, timeout=timeout, retries=retries, - headers_cb=headers_cb, data=data, - sec_between=sec_between, ssl_details=ssl_details, - exception_cb=exception_cb) + return readurl(url, **kwargs) # Made to have same accessors as UrlResponse so that the @@ -201,6 +203,35 @@ def readurl(url, data=None, timeout=None, retries=0, sec_between=1, check_status=True, allow_redirects=True, exception_cb=None, session=None, infinite=False, log_req_resp=True, request_method=None): + """Wrapper around requests.Session to read the url and retry if necessary + + :param url: Mandatory url to request. + :param data: Optional form data to post the URL. Will set request_method + to 'POST' if present. + :param timeout: Timeout in seconds to wait for a response + :param retries: Number of times to retry on exception if exception_cb is + None or exception_cb returns True for the exception caught. Default is + to fail with 0 retries on exception. + :param sec_between: Default 1: amount of seconds passed to time.sleep + between retries. None or -1 means don't sleep. + :param headers: Optional dict of headers to send during request + :param headers_cb: Optional callable returning a dict of values to send as + headers during request + :param ssl_details: Optional dict providing key_file, ca_certs, and + cert_file keys for use on in ssl connections. + :param check_status: Optional boolean set True to raise when HTTPError + occurs. Default: True. + :param allow_redirects: Optional boolean passed straight to Session.request + as 'allow_redirects'. Default: True. + :param exception_cb: Optional callable which accepts the params + msg and exception and returns a boolean True if retries are permitted. + :param session: Optional exiting requests.Session instance to reuse. + :param infinite: Bool, set True to retry indefinitely. Default: False. + :param log_req_resp: Set False to turn off verbose debug messages. + :param request_method: String passed as 'method' to Session.request. + Typically GET, or POST. Default: POST if data is provided, GET + otherwise. + """ url = _cleanurl(url) req_args = { 'url': url, diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py index ed83d2d8..15af1daf 100644 --- a/cloudinit/user_data.py +++ b/cloudinit/user_data.py @@ -224,7 +224,7 @@ class UserDataProcessor(object): content = util.load_file(include_once_fn) else: try: - resp = read_file_or_url(include_url, + resp = read_file_or_url(include_url, timeout=5, retries=10, ssl_details=self.ssl_details) if include_once_on and resp.ok(): util.write_file(include_once_fn, resp.contents, diff --git a/cloudinit/util.py b/cloudinit/util.py index 78b6a2d0..9d9d5c72 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -986,13 +986,6 @@ def load_yaml(blob, default=None, allowed=(dict,)): def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0): - if base.startswith("/"): - base = "file://%s" % base - - # default retries for file is 0. for network is 10 - if base.startswith("file://"): - retries = file_retries - if base.find("%s") >= 0: ud_url = base % ("user-data" + ext) md_url = base % ("meta-data" + ext) @@ -1000,14 +993,14 @@ def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0): ud_url = "%s%s%s" % (base, "user-data", ext) md_url = "%s%s%s" % (base, "meta-data", ext) - md_resp = url_helper.read_file_or_url(md_url, timeout, retries, - file_retries) + md_resp = url_helper.read_file_or_url(md_url, timeout=timeout, + retries=retries) md = None if md_resp.ok(): md = load_yaml(decode_binary(md_resp.contents), default={}) - ud_resp = url_helper.read_file_or_url(ud_url, timeout, retries, - file_retries) + ud_resp = url_helper.read_file_or_url(ud_url, timeout=timeout, + retries=retries) ud = None if ud_resp.ok(): ud = ud_resp.contents diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index bd006aba..bd17f636 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -212,8 +212,10 @@ class TestAzureEndpointHttpClient(CiTestCase): response = client.get(url, secure=False) self.assertEqual(1, self.read_file_or_url.call_count) self.assertEqual(self.read_file_or_url.return_value, response) - self.assertEqual(mock.call(url, headers=self.regular_headers), - self.read_file_or_url.call_args) + self.assertEqual( + mock.call(url, headers=self.regular_headers, retries=10, + timeout=5), + self.read_file_or_url.call_args) def test_secure_get(self): url = 'MyTestUrl' @@ -227,8 +229,10 @@ class TestAzureEndpointHttpClient(CiTestCase): response = client.get(url, secure=True) self.assertEqual(1, self.read_file_or_url.call_count) self.assertEqual(self.read_file_or_url.return_value, response) - self.assertEqual(mock.call(url, headers=expected_headers), - self.read_file_or_url.call_args) + self.assertEqual( + mock.call(url, headers=expected_headers, retries=10, + timeout=5), + self.read_file_or_url.call_args) def test_post(self): data = mock.MagicMock() @@ -238,7 +242,8 @@ class TestAzureEndpointHttpClient(CiTestCase): self.assertEqual(1, self.read_file_or_url.call_count) self.assertEqual(self.read_file_or_url.return_value, response) self.assertEqual( - mock.call(url, data=data, headers=self.regular_headers), + mock.call(url, data=data, headers=self.regular_headers, retries=10, + timeout=5), self.read_file_or_url.call_args) def test_post_with_extra_headers(self): @@ -250,7 +255,8 @@ class TestAzureEndpointHttpClient(CiTestCase): expected_headers = self.regular_headers.copy() expected_headers.update(extra_headers) self.assertEqual( - mock.call(mock.ANY, data=mock.ANY, headers=expected_headers), + mock.call(mock.ANY, data=mock.ANY, headers=expected_headers, + retries=10, timeout=5), self.read_file_or_url.call_args) -- cgit v1.2.3 From c5a7d7979c036f6dc6823f429c6b6820f7f74241 Mon Sep 17 00:00:00 2001 From: Conrad Hoffmann <1226676+bitfehler@users.noreply.github.com> Date: Wed, 8 Jan 2020 15:18:48 +0100 Subject: Make tests work with Python 3.8 (#139) * Make DistroChecker test work with Python 3.8 In Python 3.8, `platform.linux_distribution` has been removed. This was anticipated, and the cloud-init code uses its own `util.get_linux_distro` instead, which works fine w/o `platform.linux_distribution`. However, these tests still try to mock the platform function, which fails if it doesn't exist (Python 3.8). Instead, mock the new function here, as this is a test for code that depends on it rather than the function itself. * Make GetLinuxDistro tests work with Python 3.8 In Python 3.8, `platform.dist` was removed, so allow mock to create the function by setting `create=True`. * Make linter happy in Python 3.8 Suppress E1101(no-member) as this function was removed. --- cloudinit/analyze/tests/test_boot.py | 8 ++++---- cloudinit/tests/test_util.py | 6 +++--- cloudinit/util.py | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/analyze/tests/test_boot.py b/cloudinit/analyze/tests/test_boot.py index 706e2cc0..f4001c14 100644 --- a/cloudinit/analyze/tests/test_boot.py +++ b/cloudinit/analyze/tests/test_boot.py @@ -12,17 +12,17 @@ class TestDistroChecker(CiTestCase): @mock.patch('cloudinit.util.system_info', return_value={'dist': ('', '', ''), 'system': ''}) - @mock.patch('platform.linux_distribution', return_value=('', '', '')) + @mock.patch('cloudinit.util.get_linux_distro', return_value=('', '', '')) @mock.patch('cloudinit.util.is_FreeBSD', return_value=False) - def test_blank_distro(self, m_sys_info, m_linux_distribution, m_free_bsd): + def test_blank_distro(self, m_sys_info, m_get_linux_distro, m_free_bsd): self.assertEqual(err_code, dist_check_timestamp()) @mock.patch('cloudinit.util.system_info', return_value={'dist': ('', '', '')}) - @mock.patch('platform.linux_distribution', return_value=('', '', '')) + @mock.patch('cloudinit.util.get_linux_distro', return_value=('', '', '')) @mock.patch('cloudinit.util.is_FreeBSD', return_value=True) def test_freebsd_gentoo_cant_find(self, m_sys_info, - m_linux_distribution, m_is_FreeBSD): + m_get_linux_distro, m_is_FreeBSD): self.assertEqual(err_code, dist_check_timestamp()) @mock.patch('cloudinit.util.subp', return_value=(0, 1)) diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index 64ed82ea..be100646 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -523,7 +523,7 @@ class TestGetLinuxDistro(CiTestCase): self.assertEqual( ('opensuse-tumbleweed', '20180920', platform.machine()), dist) - @mock.patch('platform.dist') + @mock.patch('platform.dist', create=True) def test_get_linux_distro_no_data(self, m_platform_dist, m_path_exists): """Verify we get no information if os-release does not exist""" m_platform_dist.return_value = ('', '', '') @@ -531,7 +531,7 @@ class TestGetLinuxDistro(CiTestCase): dist = util.get_linux_distro() self.assertEqual(('', '', ''), dist) - @mock.patch('platform.dist') + @mock.patch('platform.dist', create=True) def test_get_linux_distro_no_impl(self, m_platform_dist, m_path_exists): """Verify we get an empty tuple when no information exists and Exceptions are not propagated""" @@ -540,7 +540,7 @@ class TestGetLinuxDistro(CiTestCase): dist = util.get_linux_distro() self.assertEqual(('', '', ''), dist) - @mock.patch('platform.dist') + @mock.patch('platform.dist', create=True) def test_get_linux_distro_plat_data(self, m_platform_dist, m_path_exists): """Verify we get the correct platform information""" m_platform_dist.return_value = ('foo', '1.1', 'aarch64') diff --git a/cloudinit/util.py b/cloudinit/util.py index 9d9d5c72..830c8e54 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -635,8 +635,8 @@ def get_linux_distro(): else: dist = ('', '', '') try: - # Will be removed in 3.7 - dist = platform.dist() # pylint: disable=W1505 + # Was removed in 3.8 + dist = platform.dist() # pylint: disable=W1505,E1101 except Exception: pass finally: -- cgit v1.2.3 From c911afbfe5a38823971e9cdbd4d1848c6e5c16de Mon Sep 17 00:00:00 2001 From: Igor Galić Date: Fri, 10 Jan 2020 05:06:06 +0100 Subject: util: move uptime's else branch into its own boottime function (#53) Also fix bugs: - pass binary instead of string to sysctlbyname(), and - unpack the "return value" in a struct, rather than in single integer. LP: #1853160 Co-Authored-By: Ryan Harper --- cloudinit/tests/test_util.py | 15 +++++++++++++++ cloudinit/util.py | 39 +++++++++++++++++++++++++++++---------- 2 files changed, 44 insertions(+), 10 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py index be100646..11f37000 100644 --- a/cloudinit/tests/test_util.py +++ b/cloudinit/tests/test_util.py @@ -189,6 +189,21 @@ class TestUtil(CiTestCase): self.assertEqual(is_rw, False) +class TestUptime(CiTestCase): + + @mock.patch('cloudinit.util.boottime') + @mock.patch('cloudinit.util.os.path.exists') + @mock.patch('cloudinit.util.time.time') + def test_uptime_non_linux_path(self, m_time, m_exists, m_boottime): + boottime = 1000.0 + uptime = 10.0 + m_boottime.return_value = boottime + m_time.return_value = boottime + uptime + m_exists.return_value = False + result = util.uptime() + self.assertEqual(str(uptime), result) + + class TestShellify(CiTestCase): def test_input_dict_raises_type_error(self): diff --git a/cloudinit/util.py b/cloudinit/util.py index 830c8e54..76d7db78 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -10,7 +10,6 @@ import contextlib import copy as obj_copy -import ctypes import email import glob import grp @@ -1807,6 +1806,33 @@ def time_rfc2822(): return ts +def boottime(): + """Use sysctlbyname(3) via ctypes to find kern.boottime + + kern.boottime is of type struct timeval. Here we create a + private class to easier unpack it. + + @return boottime: float to be compatible with linux + """ + import ctypes + + NULL_BYTES = b"\x00" + + class timeval(ctypes.Structure): + _fields_ = [ + ("tv_sec", ctypes.c_int64), + ("tv_usec", ctypes.c_int64) + ] + libc = ctypes.CDLL('/lib/libc.so.7') + size = ctypes.c_size_t() + size.value = ctypes.sizeof(timeval) + buf = timeval() + if libc.sysctlbyname(b"kern.boottime" + NULL_BYTES, ctypes.byref(buf), + ctypes.byref(size), None, 0) != -1: + return buf.tv_sec + buf.tv_usec / 1000000.0 + raise RuntimeError("Unable to retrieve kern.boottime on this system") + + def uptime(): uptime_str = '??' method = 'unknown' @@ -1818,15 +1844,8 @@ def uptime(): uptime_str = contents.split()[0] else: method = 'ctypes' - libc = ctypes.CDLL('/lib/libc.so.7') - size = ctypes.c_size_t() - buf = ctypes.c_int() - size.value = ctypes.sizeof(buf) - libc.sysctlbyname("kern.boottime", ctypes.byref(buf), - ctypes.byref(size), None, 0) - now = time.time() - bootup = buf.value - uptime_str = now - bootup + # This is the *BSD codepath + uptime_str = str(time.time() - boottime()) except Exception: logexc(LOG, "Unable to read uptime using method: %s" % method) -- cgit v1.2.3 From 259d9c501472315e539701f00095743390eb89e5 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Thu, 16 Jan 2020 11:29:59 -0600 Subject: Ensure util.get_architecture() runs only once (#172) * Ensure util.get_architecture() runs only once util.get_architecture() recently was wrapped using python3's lru_cache() which will cache the result so we only invoke 'dpkg --print-architecture' once. In practice, cloud-init.log will show multiple invocations of the command. The source of this was that the debian Distro object implements the get_primary_arch() with this command, but it was not calling it from util, but issuing a util.subp() directly. This branch also updates cc_apt_configure methods to fetch the arch value from the distro class, and then ensure that the methods apt_configure calls pass the arch value around. * utils: remove lsb_release and get_architecture wrappers The original lsb_release wrapper was used to prevent polluting the single value we cached, however lru_cache() already handles this case by using args, kwargs values to cache different calls to the method. * rename_apt_list: use all positional parameters --- cloudinit/config/cc_apt_configure.py | 6 +++--- cloudinit/distros/debian.py | 3 +-- cloudinit/util.py | 10 +--------- tests/unittests/test_handler/test_handler_apt_source_v3.py | 7 ++++--- 4 files changed, 9 insertions(+), 17 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index f01e2aaf..4a3aed36 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -309,7 +309,7 @@ def apply_apt(cfg, cloud, target): if util.is_false(cfg.get('preserve_sources_list', False)): generate_sources_list(cfg, release, mirrors, cloud) - rename_apt_lists(mirrors, target) + rename_apt_lists(mirrors, target, arch) try: apply_apt_config(cfg, APT_PROXY_FN, APT_CONFIG_FN) @@ -427,9 +427,9 @@ def mirrorurl_to_apt_fileprefix(mirror): return string -def rename_apt_lists(new_mirrors, target=None): +def rename_apt_lists(new_mirrors, target, arch): """rename_apt_lists - rename apt lists to preserve old cache data""" - default_mirrors = get_default_mirrors(util.get_architecture(target)) + default_mirrors = get_default_mirrors(arch) pre = util.target_path(target, APT_LISTS) for (name, omirror) in default_mirrors.items(): diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py index cf082c73..79268371 100644 --- a/cloudinit/distros/debian.py +++ b/cloudinit/distros/debian.py @@ -205,8 +205,7 @@ class Distro(distros.Distro): ["update"], freq=PER_INSTANCE) def get_primary_arch(self): - (arch, _err) = util.subp(['dpkg', '--print-architecture']) - return str(arch).strip() + return util.get_architecture() def _get_wrapper_prefix(cmd, mode): diff --git a/cloudinit/util.py b/cloudinit/util.py index 76d7db78..87480767 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -86,7 +86,7 @@ def get_architecture(target=None): @lru_cache() -def _lsb_release(target=None): +def lsb_release(target=None): fmap = {'Codename': 'codename', 'Description': 'description', 'Distributor ID': 'id', 'Release': 'release'} @@ -109,14 +109,6 @@ def _lsb_release(target=None): return data -def lsb_release(target=None): - if target_path(target) != "/": - # do not use or update cache if target is provided - return _lsb_release(target) - - return _lsb_release() - - def target_path(target, path=None): # return 'path' inside target, accepting target as None if target in (None, ""): diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py index 2f21b6dc..dcffbe13 100644 --- a/tests/unittests/test_handler/test_handler_apt_source_v3.py +++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py @@ -487,7 +487,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): with mock.patch.object(os, 'rename') as mockren: with mock.patch.object(glob, 'glob', return_value=[fromfn]): - cc_apt_configure.rename_apt_lists(mirrors, TARGET) + cc_apt_configure.rename_apt_lists(mirrors, TARGET, arch) mockren.assert_any_call(fromfn, tofn) @@ -496,7 +496,8 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): target = os.path.join(self.tmp, "rename_non_slash") apt_lists_d = os.path.join(target, "./" + cc_apt_configure.APT_LISTS) - m_get_architecture.return_value = 'amd64' + arch = 'amd64' + m_get_architecture.return_value = arch mirror_path = "some/random/path/" primary = "http://test.ubuntu.com/" + mirror_path @@ -532,7 +533,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): fpath = os.path.join(apt_lists_d, opre + suff) util.write_file(fpath, content=fpath) - cc_apt_configure.rename_apt_lists(mirrors, target) + cc_apt_configure.rename_apt_lists(mirrors, target, arch) found = sorted(os.listdir(apt_lists_d)) self.assertEqual(expected, found) -- cgit v1.2.3 From 651e24066ba4b9464c7843553f60d17a459cf06e Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 16 Jan 2020 13:30:12 -0500 Subject: util: rename get_architecture to get_dpkg_architecture (#173) This makes it clearer that we should only use this in code paths that will definitely have dpkg available to them. - Rename get_architecture -> get_dpkg_architecture - Add docstring to get_dpkg_architecture --- cloudinit/config/cc_apt_configure.py | 6 +++--- cloudinit/distros/debian.py | 2 +- cloudinit/util.py | 7 ++++++- tests/cloud_tests/config.py | 2 +- tests/cloud_tests/platforms/nocloudkvm/platform.py | 10 +++++++--- .../test_handler_apt_configure_sources_list_v1.py | 2 +- .../test_handler_apt_configure_sources_list_v3.py | 2 +- .../test_handler/test_handler_apt_source_v1.py | 2 +- .../test_handler/test_handler_apt_source_v3.py | 20 +++++++++++--------- 9 files changed, 32 insertions(+), 21 deletions(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 4a3aed36..c44dec45 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -253,7 +253,7 @@ def get_default_mirrors(arch=None, target=None): architecture, for more see: https://wiki.ubuntu.com/UbuntuDevelopment/PackageArchive#Ports""" if arch is None: - arch = util.get_architecture(target) + arch = util.get_dpkg_architecture(target) if arch in PRIMARY_ARCHES: return PRIMARY_ARCH_MIRRORS.copy() if arch in PORTS_ARCHES: @@ -303,7 +303,7 @@ def apply_apt(cfg, cloud, target): LOG.debug("handling apt config: %s", cfg) release = util.lsb_release(target=target)['codename'] - arch = util.get_architecture(target) + arch = util.get_dpkg_architecture(target) mirrors = find_apt_mirror_info(cfg, cloud, arch=arch) LOG.debug("Apt Mirror info: %s", mirrors) @@ -896,7 +896,7 @@ def find_apt_mirror_info(cfg, cloud, arch=None): """ if arch is None: - arch = util.get_architecture() + arch = util.get_dpkg_architecture() LOG.debug("got arch for mirror selection: %s", arch) pmirror = get_mirror(cfg, "primary", arch, cloud) LOG.debug("got primary mirror: %s", pmirror) diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py index 79268371..128bb523 100644 --- a/cloudinit/distros/debian.py +++ b/cloudinit/distros/debian.py @@ -205,7 +205,7 @@ class Distro(distros.Distro): ["update"], freq=PER_INSTANCE) def get_primary_arch(self): - return util.get_architecture() + return util.get_dpkg_architecture() def _get_wrapper_prefix(cmd, mode): diff --git a/cloudinit/util.py b/cloudinit/util.py index 87480767..d99e82fa 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -79,7 +79,12 @@ CONTAINER_TESTS = (['systemd-detect-virt', '--quiet', '--container'], @lru_cache() -def get_architecture(target=None): +def get_dpkg_architecture(target=None): + """Return the sanitized string output by `dpkg --print-architecture`. + + N.B. This function is wrapped in functools.lru_cache, so repeated calls + won't shell out every time. + """ out, _ = subp(['dpkg', '--print-architecture'], capture=True, target=target) return out.strip() diff --git a/tests/cloud_tests/config.py b/tests/cloud_tests/config.py index 8bd569fd..06536edc 100644 --- a/tests/cloud_tests/config.py +++ b/tests/cloud_tests/config.py @@ -114,7 +114,7 @@ def load_os_config(platform_name, os_name, require_enabled=False, feature_conf = main_conf['features'] feature_groups = conf.get('feature_groups', []) overrides = merge_config(get(conf, 'features'), feature_overrides) - conf['arch'] = c_util.get_architecture() + conf['arch'] = c_util.get_dpkg_architecture() conf['features'] = merge_feature_groups( feature_conf, feature_groups, overrides) diff --git a/tests/cloud_tests/platforms/nocloudkvm/platform.py b/tests/cloud_tests/platforms/nocloudkvm/platform.py index 85933463..2d1480f5 100644 --- a/tests/cloud_tests/platforms/nocloudkvm/platform.py +++ b/tests/cloud_tests/platforms/nocloudkvm/platform.py @@ -29,9 +29,13 @@ class NoCloudKVMPlatform(Platform): """ (url, path) = s_util.path_from_mirror_url(img_conf['mirror_url'], None) - filter = filters.get_filters(['arch=%s' % c_util.get_architecture(), - 'release=%s' % img_conf['release'], - 'ftype=disk1.img']) + filter = filters.get_filters( + [ + 'arch=%s' % c_util.get_dpkg_architecture(), + 'release=%s' % img_conf['release'], + 'ftype=disk1.img', + ] + ) mirror_config = {'filters': filter, 'keep_items': False, 'max_items': 1, diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py index 23bd6e10..7c17a264 100644 --- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py +++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v1.py @@ -78,7 +78,7 @@ class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase): get_rel = rpatcher.start() get_rel.return_value = {'codename': "fakerelease"} self.addCleanup(rpatcher.stop) - apatcher = mock.patch("cloudinit.util.get_architecture") + apatcher = mock.patch("cloudinit.util.get_dpkg_architecture") get_arch = apatcher.start() get_arch.return_value = 'amd64' self.addCleanup(apatcher.stop) diff --git a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py index f7608c28..0a68cb8f 100644 --- a/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py +++ b/tests/unittests/test_handler/test_handler_apt_configure_sources_list_v3.py @@ -106,7 +106,7 @@ class TestAptSourceConfigSourceList(t_help.FilesystemMockingTestCase): get_rel = rpatcher.start() get_rel.return_value = {'codename': "fakerel"} self.addCleanup(rpatcher.stop) - apatcher = mock.patch("cloudinit.util.get_architecture") + apatcher = mock.patch("cloudinit.util.get_dpkg_architecture") get_arch = apatcher.start() get_arch.return_value = 'amd64' self.addCleanup(apatcher.stop) diff --git a/tests/unittests/test_handler/test_handler_apt_source_v1.py b/tests/unittests/test_handler/test_handler_apt_source_v1.py index a3132fbd..652d97ab 100644 --- a/tests/unittests/test_handler/test_handler_apt_source_v1.py +++ b/tests/unittests/test_handler/test_handler_apt_source_v1.py @@ -77,7 +77,7 @@ class TestAptSourceConfig(TestCase): get_rel = rpatcher.start() get_rel.return_value = {'codename': self.release} self.addCleanup(rpatcher.stop) - apatcher = mock.patch("cloudinit.util.get_architecture") + apatcher = mock.patch("cloudinit.util.get_dpkg_architecture") get_arch = apatcher.start() get_arch.return_value = 'amd64' self.addCleanup(apatcher.stop) diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py index dcffbe13..c5cf6785 100644 --- a/tests/unittests/test_handler/test_handler_apt_source_v3.py +++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py @@ -453,14 +453,14 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): self.assertFalse(os.path.isfile(self.aptlistfile2)) self.assertFalse(os.path.isfile(self.aptlistfile3)) - @mock.patch("cloudinit.config.cc_apt_configure.util.get_architecture") - def test_apt_v3_list_rename(self, m_get_architecture): + @mock.patch("cloudinit.config.cc_apt_configure.util.get_dpkg_architecture") + def test_apt_v3_list_rename(self, m_get_dpkg_architecture): """test_apt_v3_list_rename - Test find mirror and apt list renaming""" pre = "/var/lib/apt/lists" # filenames are archive dependent arch = 's390x' - m_get_architecture.return_value = arch + m_get_dpkg_architecture.return_value = arch component = "ubuntu-ports" archive = "ports.ubuntu.com" @@ -491,13 +491,13 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): mockren.assert_any_call(fromfn, tofn) - @mock.patch("cloudinit.config.cc_apt_configure.util.get_architecture") - def test_apt_v3_list_rename_non_slash(self, m_get_architecture): + @mock.patch("cloudinit.config.cc_apt_configure.util.get_dpkg_architecture") + def test_apt_v3_list_rename_non_slash(self, m_get_dpkg_architecture): target = os.path.join(self.tmp, "rename_non_slash") apt_lists_d = os.path.join(target, "./" + cc_apt_configure.APT_LISTS) arch = 'amd64' - m_get_architecture.return_value = arch + m_get_dpkg_architecture.return_value = arch mirror_path = "some/random/path/" primary = "http://test.ubuntu.com/" + mirror_path @@ -626,10 +626,12 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase): self.assertEqual(mirrors['SECURITY'], smir) - @mock.patch("cloudinit.config.cc_apt_configure.util.get_architecture") - def test_apt_v3_get_def_mir_non_intel_no_arch(self, m_get_architecture): + @mock.patch("cloudinit.config.cc_apt_configure.util.get_dpkg_architecture") + def test_apt_v3_get_def_mir_non_intel_no_arch( + self, m_get_dpkg_architecture + ): arch = 'ppc64el' - m_get_architecture.return_value = arch + m_get_dpkg_architecture.return_value = arch expected = {'PRIMARY': 'http://ports.ubuntu.com/ubuntu-ports', 'SECURITY': 'http://ports.ubuntu.com/ubuntu-ports'} self.assertEqual(expected, cc_apt_configure.get_default_mirrors()) -- cgit v1.2.3 From 3e2f7356effc9e9cccc5ae945846279804eedc46 Mon Sep 17 00:00:00 2001 From: Dimitri John Ledkov Date: Tue, 18 Feb 2020 17:03:24 +0000 Subject: utils: use SystemRandom when generating random password. (#204) As noticed by Seth Arnold, non-deterministic SystemRandom should be used when creating security sensitive random strings. --- cloudinit/util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'cloudinit/util.py') diff --git a/cloudinit/util.py b/cloudinit/util.py index d99e82fa..c02b3d9a 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -397,9 +397,10 @@ def translate_bool(val, addons=None): def rand_str(strlen=32, select_from=None): + r = random.SystemRandom() if not select_from: select_from = string.ascii_letters + string.digits - return "".join([random.choice(select_from) for _x in range(0, strlen)]) + return "".join([r.choice(select_from) for _x in range(0, strlen)]) def rand_dict_key(dictionary, postfix=None): -- cgit v1.2.3