From fa639704f67539d9c1d8668383f755cb0213fd4a Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 4 Mar 2020 15:19:43 -0700 Subject: instance-data: write redacted cfg to instance-data.json (#233) When cloud-init persisted instance metadata to instance-data.json if failed to redact the sensitive value. Currently, the only sensitive key 'security-credentials' is omitted as cloud-init does not fetch this value from IMDS. Fix this by properly redacting the content from the public instance-metadata.json file while retaining the value in the root-only instance-data-sensitive.json file. LP: #1865947 --- cloudinit/sources/tests/test_init.py | 57 +++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 7 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index f73b37ed..6db127e7 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -318,8 +318,8 @@ class TestDataSource(CiTestCase): self.assertEqual(0o644, stat.S_IMODE(file_stat.st_mode)) self.assertEqual(expected, util.load_json(content)) - def test_get_data_writes_json_instance_data_sensitive(self): - """get_data writes INSTANCE_JSON_SENSITIVE_FILE as readonly root.""" + def test_get_data_writes_redacted_public_json_instance_data(self): + """get_data writes redacted content to public INSTANCE_JSON_FILE.""" tmp = self.tmp_dir() datasource = DataSourceTestSubclassNet( self.sys_cfg, self.distro, Paths({'run_dir': tmp}), @@ -333,11 +333,53 @@ class TestDataSource(CiTestCase): ('security-credentials',), datasource.sensitive_metadata_keys) datasource.get_data() json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) - sensitive_json_file = self.tmp_path(INSTANCE_JSON_SENSITIVE_FILE, tmp) redacted = util.load_json(util.load_file(json_file)) + expected = { + 'base64_encoded_keys': [], + 'sensitive_keys': ['ds/meta_data/some/security-credentials'], + 'v1': { + '_beta_keys': ['subplatform'], + 'availability-zone': 'myaz', + 'availability_zone': 'myaz', + 'cloud-name': 'subclasscloudname', + 'cloud_name': 'subclasscloudname', + 'instance-id': 'iid-datasource', + 'instance_id': 'iid-datasource', + 'local-hostname': 'test-subclass-hostname', + 'local_hostname': 'test-subclass-hostname', + 'platform': 'mytestsubclass', + 'public_ssh_keys': [], + 'region': 'myregion', + 'subplatform': 'unknown'}, + 'ds': { + '_doc': EXPERIMENTAL_TEXT, + 'meta_data': { + 'availability_zone': 'myaz', + 'local-hostname': 'test-subclass-hostname', + 'region': 'myregion', + 'some': {'security-credentials': REDACT_SENSITIVE_VALUE}}} + } + self.assertEqual(expected, redacted) + file_stat = os.stat(json_file) + self.assertEqual(0o644, stat.S_IMODE(file_stat.st_mode)) + + def test_get_data_writes_json_instance_data_sensitive(self): + """ + get_data writes unmodified data to sensitive file as root-readonly. + """ + tmp = self.tmp_dir() + datasource = DataSourceTestSubclassNet( + self.sys_cfg, self.distro, Paths({'run_dir': tmp}), + custom_metadata={ + 'availability_zone': 'myaz', + 'local-hostname': 'test-subclass-hostname', + 'region': 'myregion', + 'some': {'security-credentials': { + 'cred1': 'sekret', 'cred2': 'othersekret'}}}) self.assertEqual( - {'cred1': 'sekret', 'cred2': 'othersekret'}, - redacted['ds']['meta_data']['some']['security-credentials']) + ('security-credentials',), datasource.sensitive_metadata_keys) + datasource.get_data() + sensitive_json_file = self.tmp_path(INSTANCE_JSON_SENSITIVE_FILE, tmp) content = util.load_file(sensitive_json_file) expected = { 'base64_encoded_keys': [], @@ -362,9 +404,10 @@ class TestDataSource(CiTestCase): 'availability_zone': 'myaz', 'local-hostname': 'test-subclass-hostname', 'region': 'myregion', - 'some': {'security-credentials': REDACT_SENSITIVE_VALUE}}} + 'some': { + 'security-credentials': + {'cred1': 'sekret', 'cred2': 'othersekret'}}}} } - self.maxDiff = None self.assertEqual(expected, util.load_json(content)) file_stat = os.stat(sensitive_json_file) self.assertEqual(0o600, stat.S_IMODE(file_stat.st_mode)) -- cgit v1.2.3 From 71af48df3514ca831c90b77dc71ba0a121dec401 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 10 Mar 2020 08:22:22 -0600 Subject: instance-data: add cloud-init merged_cfg and sys_info keys to json (#214) Cloud-config userdata provided as jinja templates are now distro, platform and merged cloud config aware. The cloud-init query command will also surface this config data. Now users can selectively render portions of cloud-config based on: * distro name, version, release * python version * merged cloud config values * machine platform * kernel To support template handling of this config, add new top-level keys to /run/cloud-init/instance-data.json. The new 'merged_cfg' key represents merged cloud config from /etc/cloud/cloud.cfg and /etc/cloud/cloud.cfg.d/*. The new 'sys_info' key which captures distro and platform info from cloudinit.util.system_info. Cloud config userdata templates can render conditional content based on these additional environmental checks such as the following simple example: ``` ## template: jinja #cloud-config runcmd: {% if distro == 'opensuse' %} - sh /custom-setup-sles {% elif distro == 'centos' %} - sh /custom-setup-centos {% elif distro == 'debian' %} - sh /custom-setup-debian {% endif %} ``` To see all values: sudo cloud-init query --all Any keys added to the standardized v1 keys are guaranteed to not change or drop on future released of cloud-init. 'v1' keys will be retained for backward-compatibility even if a new standardized 'v2' set of keys are introduced The following standardized v1 keys are added: * distro, distro_release, distro_version, kernel_version, machine, python_version, system_platform, variant LP: #1865969 --- cloudinit/sources/__init__.py | 43 ++- cloudinit/sources/tests/test_init.py | 98 +++++- cloudinit/util.py | 2 +- doc/rtd/topics/instancedata.rst | 360 +++++++++++++++------ tests/cloud_tests/testcases/base.py | 55 +++- tests/unittests/test_datasource/test_cloudsigma.py | 6 +- 6 files changed, 426 insertions(+), 138 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 805d803d..a6e6d202 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -89,26 +89,26 @@ def process_instance_metadata(metadata, key_path='', sensitive_keys=()): @return Dict copy of processed metadata. """ md_copy = copy.deepcopy(metadata) - md_copy['base64_encoded_keys'] = [] - md_copy['sensitive_keys'] = [] + base64_encoded_keys = [] + sens_keys = [] for key, val in metadata.items(): if key_path: sub_key_path = key_path + '/' + key else: sub_key_path = key if key in sensitive_keys or sub_key_path in sensitive_keys: - md_copy['sensitive_keys'].append(sub_key_path) + sens_keys.append(sub_key_path) if isinstance(val, str) and val.startswith('ci-b64:'): - md_copy['base64_encoded_keys'].append(sub_key_path) + base64_encoded_keys.append(sub_key_path) md_copy[key] = val.replace('ci-b64:', '') if isinstance(val, dict): return_val = process_instance_metadata( val, sub_key_path, sensitive_keys) - md_copy['base64_encoded_keys'].extend( - return_val.pop('base64_encoded_keys')) - md_copy['sensitive_keys'].extend( - return_val.pop('sensitive_keys')) + base64_encoded_keys.extend(return_val.pop('base64_encoded_keys')) + sens_keys.extend(return_val.pop('sensitive_keys')) md_copy[key] = return_val + md_copy['base64_encoded_keys'] = sorted(base64_encoded_keys) + md_copy['sensitive_keys'] = sorted(sens_keys) return md_copy @@ -193,7 +193,7 @@ class DataSource(metaclass=abc.ABCMeta): # N-tuple of keypaths or keynames redact from instance-data.json for # non-root users - sensitive_metadata_keys = ('security-credentials',) + sensitive_metadata_keys = ('merged_cfg', 'security-credentials',) def __init__(self, sys_cfg, distro, paths, ud_proc=None): self.sys_cfg = sys_cfg @@ -218,14 +218,15 @@ class DataSource(metaclass=abc.ABCMeta): def __str__(self): return type_utils.obj_name(self) - def _get_standardized_metadata(self): + def _get_standardized_metadata(self, instance_data): """Return a dictionary of standardized metadata keys.""" local_hostname = self.get_hostname() instance_id = self.get_instance_id() availability_zone = self.availability_zone # In the event of upgrade from existing cloudinit, pickled datasource # will not contain these new class attributes. So we need to recrawl - # metadata to discover that content. + # metadata to discover that content + sysinfo = instance_data["sys_info"] return { 'v1': { '_beta_keys': ['subplatform'], @@ -233,14 +234,22 @@ class DataSource(metaclass=abc.ABCMeta): 'availability_zone': availability_zone, 'cloud-name': self.cloud_name, 'cloud_name': self.cloud_name, + 'distro': sysinfo["dist"][0], + 'distro_version': sysinfo["dist"][1], + 'distro_release': sysinfo["dist"][2], 'platform': self.platform_type, 'public_ssh_keys': self.get_public_ssh_keys(), + 'python_version': sysinfo["python"], 'instance-id': instance_id, 'instance_id': instance_id, + 'kernel_release': sysinfo["uname"][2], 'local-hostname': local_hostname, 'local_hostname': local_hostname, + 'machine': sysinfo["uname"][4], 'region': self.region, - 'subplatform': self.subplatform}} + 'subplatform': self.subplatform, + 'system_platform': sysinfo["platform"], + 'variant': sysinfo["variant"]}} def clear_cached_attrs(self, attr_defaults=()): """Reset any cached metadata attributes to datasource defaults. @@ -299,9 +308,15 @@ class DataSource(metaclass=abc.ABCMeta): ec2_metadata = getattr(self, 'ec2_metadata') if ec2_metadata != UNSET: instance_data['ds']['ec2_metadata'] = ec2_metadata - instance_data.update( - self._get_standardized_metadata()) instance_data['ds']['_doc'] = EXPERIMENTAL_TEXT + # Add merged cloud.cfg and sys info for jinja templates and cli query + instance_data['merged_cfg'] = copy.deepcopy(self.sys_cfg) + instance_data['merged_cfg']['_doc'] = ( + 'Merged cloud-init system config from /etc/cloud/cloud.cfg and' + ' /etc/cloud/cloud.cfg.d/') + instance_data['sys_info'] = util.system_info() + instance_data.update( + self._get_standardized_metadata(instance_data)) try: # Process content base64encoding unserializable values content = util.json_dumps(instance_data) diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 6db127e7..541cbbeb 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -55,6 +55,7 @@ class InvalidDataSourceTestSubclassNet(DataSource): class TestDataSource(CiTestCase): with_logs = True + maxDiff = None def setUp(self): super(TestDataSource, self).setUp() @@ -288,27 +289,47 @@ class TestDataSource(CiTestCase): tmp = self.tmp_dir() datasource = DataSourceTestSubclassNet( self.sys_cfg, self.distro, Paths({'run_dir': tmp})) - datasource.get_data() + sys_info = { + "python": "3.7", + "platform": + "Linux-5.4.0-24-generic-x86_64-with-Ubuntu-20.04-focal", + "uname": ["Linux", "myhost", "5.4.0-24-generic", "SMP blah", + "x86_64"], + "variant": "ubuntu", "dist": ["ubuntu", "20.04", "focal"]} + with mock.patch("cloudinit.util.system_info", return_value=sys_info): + datasource.get_data() json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) content = util.load_file(json_file) expected = { 'base64_encoded_keys': [], - 'sensitive_keys': [], + 'merged_cfg': REDACT_SENSITIVE_VALUE, + 'sensitive_keys': ['merged_cfg'], + 'sys_info': sys_info, 'v1': { '_beta_keys': ['subplatform'], 'availability-zone': 'myaz', 'availability_zone': 'myaz', 'cloud-name': 'subclasscloudname', 'cloud_name': 'subclasscloudname', + 'distro': 'ubuntu', + 'distro_release': 'focal', + 'distro_version': '20.04', 'instance-id': 'iid-datasource', 'instance_id': 'iid-datasource', 'local-hostname': 'test-subclass-hostname', 'local_hostname': 'test-subclass-hostname', + 'kernel_release': '5.4.0-24-generic', + 'machine': 'x86_64', 'platform': 'mytestsubclass', 'public_ssh_keys': [], + 'python_version': '3.7', 'region': 'myregion', - 'subplatform': 'unknown'}, + 'system_platform': + 'Linux-5.4.0-24-generic-x86_64-with-Ubuntu-20.04-focal', + 'subplatform': 'unknown', + 'variant': 'ubuntu'}, 'ds': { + '_doc': EXPERIMENTAL_TEXT, 'meta_data': {'availability_zone': 'myaz', 'local-hostname': 'test-subclass-hostname', @@ -329,28 +350,49 @@ class TestDataSource(CiTestCase): 'region': 'myregion', 'some': {'security-credentials': { 'cred1': 'sekret', 'cred2': 'othersekret'}}}) - self.assertEqual( - ('security-credentials',), datasource.sensitive_metadata_keys) - datasource.get_data() + self.assertItemsEqual( + ('merged_cfg', 'security-credentials',), + datasource.sensitive_metadata_keys) + sys_info = { + "python": "3.7", + "platform": + "Linux-5.4.0-24-generic-x86_64-with-Ubuntu-20.04-focal", + "uname": ["Linux", "myhost", "5.4.0-24-generic", "SMP blah", + "x86_64"], + "variant": "ubuntu", "dist": ["ubuntu", "20.04", "focal"]} + with mock.patch("cloudinit.util.system_info", return_value=sys_info): + datasource.get_data() json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) redacted = util.load_json(util.load_file(json_file)) expected = { 'base64_encoded_keys': [], - 'sensitive_keys': ['ds/meta_data/some/security-credentials'], + 'merged_cfg': REDACT_SENSITIVE_VALUE, + 'sensitive_keys': [ + 'ds/meta_data/some/security-credentials', 'merged_cfg'], + 'sys_info': sys_info, 'v1': { '_beta_keys': ['subplatform'], 'availability-zone': 'myaz', 'availability_zone': 'myaz', 'cloud-name': 'subclasscloudname', 'cloud_name': 'subclasscloudname', + 'distro': 'ubuntu', + 'distro_release': 'focal', + 'distro_version': '20.04', 'instance-id': 'iid-datasource', 'instance_id': 'iid-datasource', 'local-hostname': 'test-subclass-hostname', 'local_hostname': 'test-subclass-hostname', + 'kernel_release': '5.4.0-24-generic', + 'machine': 'x86_64', 'platform': 'mytestsubclass', 'public_ssh_keys': [], + 'python_version': '3.7', 'region': 'myregion', - 'subplatform': 'unknown'}, + 'system_platform': + 'Linux-5.4.0-24-generic-x86_64-with-Ubuntu-20.04-focal', + 'subplatform': 'unknown', + 'variant': 'ubuntu'}, 'ds': { '_doc': EXPERIMENTAL_TEXT, 'meta_data': { @@ -359,7 +401,7 @@ class TestDataSource(CiTestCase): 'region': 'myregion', 'some': {'security-credentials': REDACT_SENSITIVE_VALUE}}} } - self.assertEqual(expected, redacted) + self.assertItemsEqual(expected, redacted) file_stat = os.stat(json_file) self.assertEqual(0o644, stat.S_IMODE(file_stat.st_mode)) @@ -376,28 +418,54 @@ class TestDataSource(CiTestCase): 'region': 'myregion', 'some': {'security-credentials': { 'cred1': 'sekret', 'cred2': 'othersekret'}}}) - self.assertEqual( - ('security-credentials',), datasource.sensitive_metadata_keys) - datasource.get_data() + sys_info = { + "python": "3.7", + "platform": + "Linux-5.4.0-24-generic-x86_64-with-Ubuntu-20.04-focal", + "uname": ["Linux", "myhost", "5.4.0-24-generic", "SMP blah", + "x86_64"], + "variant": "ubuntu", "dist": ["ubuntu", "20.04", "focal"]} + + self.assertItemsEqual( + ('merged_cfg', 'security-credentials',), + datasource.sensitive_metadata_keys) + with mock.patch("cloudinit.util.system_info", return_value=sys_info): + datasource.get_data() sensitive_json_file = self.tmp_path(INSTANCE_JSON_SENSITIVE_FILE, tmp) content = util.load_file(sensitive_json_file) expected = { 'base64_encoded_keys': [], - 'sensitive_keys': ['ds/meta_data/some/security-credentials'], + 'merged_cfg': { + '_doc': ( + 'Merged cloud-init system config from ' + '/etc/cloud/cloud.cfg and /etc/cloud/cloud.cfg.d/'), + 'datasource': {'_undef': {'key1': False}}}, + 'sensitive_keys': [ + 'ds/meta_data/some/security-credentials', 'merged_cfg'], + 'sys_info': sys_info, 'v1': { '_beta_keys': ['subplatform'], 'availability-zone': 'myaz', 'availability_zone': 'myaz', 'cloud-name': 'subclasscloudname', 'cloud_name': 'subclasscloudname', + 'distro': 'ubuntu', + 'distro_release': 'focal', + 'distro_version': '20.04', 'instance-id': 'iid-datasource', 'instance_id': 'iid-datasource', + 'kernel_release': '5.4.0-24-generic', 'local-hostname': 'test-subclass-hostname', 'local_hostname': 'test-subclass-hostname', + 'machine': 'x86_64', 'platform': 'mytestsubclass', 'public_ssh_keys': [], + 'python_version': '3.7', 'region': 'myregion', - 'subplatform': 'unknown'}, + 'subplatform': 'unknown', + 'system_platform': + 'Linux-5.4.0-24-generic-x86_64-with-Ubuntu-20.04-focal', + 'variant': 'ubuntu'}, 'ds': { '_doc': EXPERIMENTAL_TEXT, 'meta_data': { @@ -408,7 +476,7 @@ class TestDataSource(CiTestCase): 'security-credentials': {'cred1': 'sekret', 'cred2': 'othersekret'}}}} } - self.assertEqual(expected, util.load_json(content)) + self.assertItemsEqual(expected, util.load_json(content)) file_stat = os.stat(sensitive_json_file) self.assertEqual(0o600, stat.S_IMODE(file_stat.st_mode)) self.assertEqual(expected, util.load_json(content)) diff --git a/cloudinit/util.py b/cloudinit/util.py index c02b3d9a..132f6051 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -656,7 +656,7 @@ def system_info(): 'system': platform.system(), 'release': platform.release(), 'python': platform.python_version(), - 'uname': platform.uname(), + 'uname': list(platform.uname()), 'dist': get_linux_distro() } system = info['system'].lower() diff --git a/doc/rtd/topics/instancedata.rst b/doc/rtd/topics/instancedata.rst index 4227c4fd..845098bb 100644 --- a/doc/rtd/topics/instancedata.rst +++ b/doc/rtd/topics/instancedata.rst @@ -76,6 +76,11 @@ There are three basic top-level keys: 'security sensitive'. Only the keys listed here will be redacted from instance-data.json for non-root users. +* **merged_cfg**: Merged cloud-init 'system_config' from `/etc/cloud/cloud.cfg` + and `/etc/cloud/cloud-cfg.d`. Values under this key could contain sensitive + information such as passwords, so it is included in the **sensitive-keys** + list which is only readable by root. + * **ds**: Datasource-specific metadata crawled for the specific cloud platform. It should closely represent the structure of the cloud metadata crawled. The structure of content and details provided are entirely @@ -83,6 +88,9 @@ There are three basic top-level keys: The content exposed under the 'ds' key is currently **experimental** and expected to change slightly in the upcoming cloud-init release. +* **sys_info**: Information about the underlying os, python, architecture and + kernel. This represents the data collected by `cloudinit.util.system_info`. + * **v1**: Standardized cloud-init metadata keys, these keys are guaranteed to exist on all cloud platforms. They will also retain their current behavior and format and will be carried forward even if cloud-init introduces a new @@ -117,6 +125,21 @@ Example output: - nocloud - ovf +v1.distro, v1.distro_version, v1.distro_release +----------------------------------------------- +This shall be the distro name, version and release as determined by +`cloudinit.util.get_linux_distro`. + +Example output: + +- centos, 7.5, core +- debian, 9, stretch +- freebsd, 12.0-release-p10, +- opensuse, 42.3, x86_64 +- opensuse-tumbleweed, 20180920, x86_64 +- redhat, 7.5, 'maipo' +- sles, 12.3, x86_64 +- ubuntu, 20.04, focal v1.instance_id -------------- @@ -126,6 +149,14 @@ Examples output: - i- +v1.kernel_release +----------------- +This shall be the running kernel `uname -r` + +Example output: + +- 5.3.0-1010-aws + v1.local_hostname ----------------- The internal or local hostname of the system. @@ -135,6 +166,17 @@ Examples output: - ip-10-41-41-70 - +v1.machine +---------- +This shall be the running cpu machine architecture `uname -m` + +Example output: + +- x86_64 +- i686 +- ppc64le +- s390x + v1.platform ------------- An attempt to identify the cloud platfrom instance that the system is running @@ -154,7 +196,7 @@ v1.subplatform Additional platform details describing the specific source or type of metadata used. The format of subplatform will be: -`` (`` +`` ()`` Examples output: @@ -171,6 +213,15 @@ Examples output: - ['ssh-rsa AA...', ...] +v1.python_version +----------------- +The version of python that is running cloud-init as determined by +`cloudinit.util.system_info` + +Example output: + +- 3.7.6 + v1.region --------- The physical region/data center in which the instance is deployed. @@ -192,164 +243,265 @@ Examples output: Example Output -------------- -Below is an example of ``/run/cloud-init/instance_data.json`` on an EC2 -instance: +Below is an example of ``/run/cloud-init/instance-data-sensitive.json`` on an +EC2 instance: .. sourcecode:: json { + "_beta_keys": [ + "subplatform" + ], + "availability_zone": "us-east-1b", "base64_encoded_keys": [], + "merged_cfg": { + "_doc": "Merged cloud-init system config from /etc/cloud/cloud.cfg and /etc/cloud/cloud.cfg.d/", + "_log": [ + "[loggers]\nkeys=root,cloudinit\n\n[handlers]\nkeys=consoleHandler,cloudLogHandler\n\n[formatters]\nkeys=simpleFormatter,arg0Formatter\n\n[logger_root]\nlevel=DEBUG\nhandlers=consoleHandler,cloudLogHandler\n\n[logger_cloudinit]\nlevel=DEBUG\nqualname=cloudinit\nhandlers=\npropagate=1\n\n[handler_consoleHandler]\nclass=StreamHandler\nlevel=WARNING\nformatter=arg0Formatter\nargs=(sys.stderr,)\n\n[formatter_arg0Formatter]\nformat=%(asctime)s - %(filename)s[%(levelname)s]: %(message)s\n\n[formatter_simpleFormatter]\nformat=[CLOUDINIT] %(filename)s[%(levelname)s]: %(message)s\n", + "[handler_cloudLogHandler]\nclass=FileHandler\nlevel=DEBUG\nformatter=arg0Formatter\nargs=('/var/log/cloud-init.log',)\n", + "[handler_cloudLogHandler]\nclass=handlers.SysLogHandler\nlevel=DEBUG\nformatter=simpleFormatter\nargs=(\"/dev/log\", handlers.SysLogHandler.LOG_USER)\n" + ], + "cloud_config_modules": [ + "emit_upstart", + "snap", + "ssh-import-id", + "locale", + "set-passwords", + "grub-dpkg", + "apt-pipelining", + "apt-configure", + "ubuntu-advantage", + "ntp", + "timezone", + "disable-ec2-metadata", + "runcmd", + "byobu" + ], + "cloud_final_modules": [ + "package-update-upgrade-install", + "fan", + "landscape", + "lxd", + "ubuntu-drivers", + "puppet", + "chef", + "mcollective", + "salt-minion", + "rightscale_userdata", + "scripts-vendor", + "scripts-per-once", + "scripts-per-boot", + "scripts-per-instance", + "scripts-user", + "ssh-authkey-fingerprints", + "keys-to-console", + "phone-home", + "final-message", + "power-state-change" + ], + "cloud_init_modules": [ + "migrator", + "seed_random", + "bootcmd", + "write-files", + "growpart", + "resizefs", + "disk_setup", + "mounts", + "set_hostname", + "update_hostname", + "update_etc_hosts", + "ca-certs", + "rsyslog", + "users-groups", + "ssh" + ], + "datasource_list": [ + "Ec2", + "None" + ], + "def_log_file": "/var/log/cloud-init.log", + "disable_root": true, + "log_cfgs": [ + [ + "[loggers]\nkeys=root,cloudinit\n\n[handlers]\nkeys=consoleHandler,cloudLogHandler\n\n[formatters]\nkeys=simpleFormatter,arg0Formatter\n\n[logger_root]\nlevel=DEBUG\nhandlers=consoleHandler,cloudLogHandler\n\n[logger_cloudinit]\nlevel=DEBUG\nqualname=cloudinit\nhandlers=\npropagate=1\n\n[handler_consoleHandler]\nclass=StreamHandler\nlevel=WARNING\nformatter=arg0Formatter\nargs=(sys.stderr,)\n\n[formatter_arg0Formatter]\nformat=%(asctime)s - %(filename)s[%(levelname)s]: %(message)s\n\n[formatter_simpleFormatter]\nformat=[CLOUDINIT] %(filename)s[%(levelname)s]: %(message)s\n", + "[handler_cloudLogHandler]\nclass=FileHandler\nlevel=DEBUG\nformatter=arg0Formatter\nargs=('/var/log/cloud-init.log',)\n" + ] + ], + "output": { + "all": "| tee -a /var/log/cloud-init-output.log" + }, + "preserve_hostname": false, + "syslog_fix_perms": [ + "syslog:adm", + "root:adm", + "root:wheel", + "root:root" + ], + "users": [ + "default" + ], + "vendor_data": { + "enabled": true, + "prefix": [] + } + }, + "cloud_name": "aws", + "distro": "ubuntu", + "distro_release": "focal", + "distro_version": "20.04", "ds": { "_doc": "EXPERIMENTAL: The structure and format of content scoped under the 'ds' key may change in subsequent releases of cloud-init.", "_metadata_api_version": "2016-09-02", "dynamic": { - "instance-identity": { + "instance_identity": { "document": { - "accountId": "437526006925", + "accountId": "329910648901", "architecture": "x86_64", - "availabilityZone": "us-east-2b", + "availabilityZone": "us-east-1b", "billingProducts": null, "devpayProductCodes": null, - "imageId": "ami-079638aae7046bdd2", - "instanceId": "i-075f088c72ad3271c", + "imageId": "ami-02e8aa396f8be3b6d", + "instanceId": "i-0929128ff2f73a2f1", "instanceType": "t2.micro", "kernelId": null, "marketplaceProductCodes": null, - "pendingTime": "2018-10-05T20:10:43Z", - "privateIp": "10.41.41.95", + "pendingTime": "2020-02-27T20:46:18Z", + "privateIp": "172.31.81.43", "ramdiskId": null, - "region": "us-east-2", + "region": "us-east-1", "version": "2017-09-30" }, "pkcs7": [ - "MIAGCSqGSIb3DQEHAqCAMIACAQExCzAJBgUrDgMCGgUAMIAGCSqGSIb3DQEHAaCAJIAEggHbewog", - "ICJkZXZwYXlQcm9kdWN0Q29kZXMiIDogbnVsbCwKICAibWFya2V0cGxhY2VQcm9kdWN0Q29kZXMi", - "IDogbnVsbCwKICAicHJpdmF0ZUlwIiA6ICIxMC40MS40MS45NSIsCiAgInZlcnNpb24iIDogIjIw", - "MTctMDktMzAiLAogICJpbnN0YW5jZUlkIiA6ICJpLTA3NWYwODhjNzJhZDMyNzFjIiwKICAiYmls", - "bGluZ1Byb2R1Y3RzIiA6IG51bGwsCiAgImluc3RhbmNlVHlwZSIgOiAidDIubWljcm8iLAogICJh", - "Y2NvdW50SWQiIDogIjQzNzUyNjAwNjkyNSIsCiAgImF2YWlsYWJpbGl0eVpvbmUiIDogInVzLWVh", - "c3QtMmIiLAogICJrZXJuZWxJZCIgOiBudWxsLAogICJyYW1kaXNrSWQiIDogbnVsbCwKICAiYXJj", - "aGl0ZWN0dXJlIiA6ICJ4ODZfNjQiLAogICJpbWFnZUlkIiA6ICJhbWktMDc5NjM4YWFlNzA0NmJk", - "ZDIiLAogICJwZW5kaW5nVGltZSIgOiAiMjAxOC0xMC0wNVQyMDoxMDo0M1oiLAogICJyZWdpb24i", - "IDogInVzLWVhc3QtMiIKfQAAAAAAADGCARcwggETAgEBMGkwXDELMAkGA1UEBhMCVVMxGTAXBgNV", - "BAgTEFdhc2hpbmd0b24gU3RhdGUxEDAOBgNVBAcTB1NlYXR0bGUxIDAeBgNVBAoTF0FtYXpvbiBX", - "ZWIgU2VydmljZXMgTExDAgkAlrpI2eVeGmcwCQYFKw4DAhoFAKBdMBgGCSqGSIb3DQEJAzELBgkq", - "hkiG9w0BBwEwHAYJKoZIhvcNAQkFMQ8XDTE4MTAwNTIwMTA0OFowIwYJKoZIhvcNAQkEMRYEFK0k", - "Tz6n1A8/zU1AzFj0riNQORw2MAkGByqGSM44BAMELjAsAhRNrr174y98grPBVXUforN/6wZp8AIU", - "JLZBkrB2GJA8A4WJ1okq++jSrBIAAAAAAAA=" + "MIAGCSqGSIb3DQ...", + "REDACTED", + "AhQUgq0iPWqPTVnT96tZE6L1XjjLHQAAAAAAAA==" ], "rsa2048": [ - "MIAGCSqGSIb3DQEHAqCAMIACAQExDzANBglghkgBZQMEAgEFADCABgkqhkiG9w0BBwGggCSABIIB", - "23sKICAiZGV2cGF5UHJvZHVjdENvZGVzIiA6IG51bGwsCiAgIm1hcmtldHBsYWNlUHJvZHVjdENv", - "ZGVzIiA6IG51bGwsCiAgInByaXZhdGVJcCIgOiAiMTAuNDEuNDEuOTUiLAogICJ2ZXJzaW9uIiA6", - "ICIyMDE3LTA5LTMwIiwKICAiaW5zdGFuY2VJZCIgOiAiaS0wNzVmMDg4YzcyYWQzMjcxYyIsCiAg", - "ImJpbGxpbmdQcm9kdWN0cyIgOiBudWxsLAogICJpbnN0YW5jZVR5cGUiIDogInQyLm1pY3JvIiwK", - "ICAiYWNjb3VudElkIiA6ICI0Mzc1MjYwMDY5MjUiLAogICJhdmFpbGFiaWxpdHlab25lIiA6ICJ1", - "cy1lYXN0LTJiIiwKICAia2VybmVsSWQiIDogbnVsbCwKICAicmFtZGlza0lkIiA6IG51bGwsCiAg", - "ImFyY2hpdGVjdHVyZSIgOiAieDg2XzY0IiwKICAiaW1hZ2VJZCIgOiAiYW1pLTA3OTYzOGFhZTcw", - "NDZiZGQyIiwKICAicGVuZGluZ1RpbWUiIDogIjIwMTgtMTAtMDVUMjA6MTA6NDNaIiwKICAicmVn", - "aW9uIiA6ICJ1cy1lYXN0LTIiCn0AAAAAAAAxggH/MIIB+wIBATBpMFwxCzAJBgNVBAYTAlVTMRkw", - "FwYDVQQIExBXYXNoaW5ndG9uIFN0YXRlMRAwDgYDVQQHEwdTZWF0dGxlMSAwHgYDVQQKExdBbWF6", - "b24gV2ViIFNlcnZpY2VzIExMQwIJAM07oeX4xevdMA0GCWCGSAFlAwQCAQUAoGkwGAYJKoZIhvcN", - "AQkDMQsGCSqGSIb3DQEHATAcBgkqhkiG9w0BCQUxDxcNMTgxMDA1MjAxMDQ4WjAvBgkqhkiG9w0B", - "CQQxIgQgkYz0pZk3zJKBi4KP4egeOKJl/UYwu5UdE7id74pmPwMwDQYJKoZIhvcNAQEBBQAEggEA", - "dC3uIGGNul1OC1mJKSH3XoBWsYH20J/xhIdftYBoXHGf2BSFsrs9ZscXd2rKAKea4pSPOZEYMXgz", - "lPuT7W0WU89N3ZKviy/ReMSRjmI/jJmsY1lea6mlgcsJXreBXFMYucZvyeWGHdnCjamoKWXkmZlM", - "mSB1gshWy8Y7DzoKviYPQZi5aI54XK2Upt4kGme1tH1NI2Cq+hM4K+adxTbNhS3uzvWaWzMklUuU", - "QHX2GMmjAVRVc8vnA8IAsBCJJp+gFgYzi09IK+cwNgCFFPADoG6jbMHHf4sLB3MUGpiA+G9JlCnM", - "fmkjI2pNRB8spc0k4UG4egqLrqCz67WuK38tjwAAAAAAAA==" + "MIAGCSqGSIb...", + "REDACTED", + "clYQvuE45xXm7Yreg3QtQbrP//owl1eZHj6s350AAAAAAAA=" ], "signature": [ - "Tsw6h+V3WnxrNVSXBYIOs1V4j95YR1mLPPH45XnhX0/Ei3waJqf7/7EEKGYP1Cr4PTYEULtZ7Mvf", - "+xJpM50Ivs2bdF7o0c4vnplRWe3f06NI9pv50dr110j/wNzP4MZ1pLhJCqubQOaaBTF3LFutgRrt", - "r4B0mN3p7EcqD8G+ll0=" + "dA+QV+LLCWCRNddnrKleYmh2GvYo+t8urDkdgmDSsPi", + "REDACTED", + "kDT4ygyJLFkd3b4qjAs=" ] } }, - "meta-data": { - "ami-id": "ami-079638aae7046bdd2", - "ami-launch-index": "0", - "ami-manifest-path": "(unknown)", - "block-device-mapping": { + "meta_data": { + "ami_id": "ami-02e8aa396f8be3b6d", + "ami_launch_index": "0", + "ami_manifest_path": "(unknown)", + "block_device_mapping": { "ami": "/dev/sda1", - "ephemeral0": "sdb", - "ephemeral1": "sdc", "root": "/dev/sda1" }, - "hostname": "ip-10-41-41-95.us-east-2.compute.internal", - "instance-action": "none", - "instance-id": "i-075f088c72ad3271c", - "instance-type": "t2.micro", - "local-hostname": "ip-10-41-41-95.us-east-2.compute.internal", - "local-ipv4": "10.41.41.95", - "mac": "06:74:8f:39:cd:a6", + "hostname": "ip-172-31-81-43.ec2.internal", + "instance_action": "none", + "instance_id": "i-0929128ff2f73a2f1", + "instance_type": "t2.micro", + "local_hostname": "ip-172-31-81-43.ec2.internal", + "local_ipv4": "172.31.81.43", + "mac": "12:7e:c9:93:29:af", "metrics": { "vhostmd": "" }, "network": { "interfaces": { "macs": { - "06:74:8f:39:cd:a6": { - "device-number": "0", - "interface-id": "eni-052058bbd7831eaae", - "ipv4-associations": { - "18.218.221.122": "10.41.41.95" - }, - "local-hostname": "ip-10-41-41-95.us-east-2.compute.internal", - "local-ipv4s": "10.41.41.95", - "mac": "06:74:8f:39:cd:a6", - "owner-id": "437526006925", - "public-hostname": "ec2-18-218-221-122.us-east-2.compute.amazonaws.com", - "public-ipv4s": "18.218.221.122", - "security-group-ids": "sg-828247e9", - "security-groups": "Cloud-init integration test secgroup", - "subnet-id": "subnet-282f3053", - "subnet-ipv4-cidr-block": "10.41.41.0/24", - "subnet-ipv6-cidr-blocks": "2600:1f16:b80:ad00::/64", - "vpc-id": "vpc-252ef24d", - "vpc-ipv4-cidr-block": "10.41.0.0/16", - "vpc-ipv4-cidr-blocks": "10.41.0.0/16", - "vpc-ipv6-cidr-blocks": "2600:1f16:b80:ad00::/56" - } + "12:7e:c9:93:29:af": { + "device_number": "0", + "interface_id": "eni-0c07a0474339b801d", + "ipv4_associations": { + "3.89.187.177": "172.31.81.43" + }, + "local_hostname": "ip-172-31-81-43.ec2.internal", + "local_ipv4s": "172.31.81.43", + "mac": "12:7e:c9:93:29:af", + "owner_id": "329910648901", + "public_hostname": "ec2-3-89-187-177.compute-1.amazonaws.com", + "public_ipv4s": "3.89.187.177", + "security_group_ids": "sg-0100038b68aa79986", + "security_groups": "launch-wizard-3", + "subnet_id": "subnet-04e2d12a", + "subnet_ipv4_cidr_block": "172.31.80.0/20", + "vpc_id": "vpc-210b4b5b", + "vpc_ipv4_cidr_block": "172.31.0.0/16", + "vpc_ipv4_cidr_blocks": "172.31.0.0/16" + } } } }, "placement": { - "availability-zone": "us-east-2b" + "availability_zone": "us-east-1b" }, "profile": "default-hvm", - "public-hostname": "ec2-18-218-221-122.us-east-2.compute.amazonaws.com", - "public-ipv4": "18.218.221.122", - "public-keys": { - "cloud-init-integration": [ - "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSL7uWGj8cgWyIOaspgKdVy0cKJ+UTjfv7jBOjG2H/GN8bJVXy72XAvnhM0dUM+CCs8FOf0YlPX+Frvz2hKInrmRhZVwRSL129PasD12MlI3l44u6IwS1o/W86Q+tkQYEljtqDOo0a+cOsaZkvUNzUyEXUwz/lmYa6G4hMKZH4NBj7nbAAF96wsMCoyNwbWryBnDYUr6wMbjRR1J9Pw7Xh7WRC73wy4Va2YuOgbD3V/5ZrFPLbWZW/7TFXVrql04QVbyei4aiFR5n//GvoqwQDNe58LmbzX/xvxyKJYdny2zXmdAhMxbrpFQsfpkJ9E/H5w0yOdSvnWbUoG5xNGoOB cloud-init-integration" - ] - }, - "reservation-id": "r-0594a20e31f6cfe46", - "security-groups": "Cloud-init integration test secgroup", + "public_hostname": "ec2-3-89-187-177.compute-1.amazonaws.com", + "public_ipv4": "3.89.187.177", + "reservation_id": "r-0c481643d15766a02", + "security_groups": "launch-wizard-3", "services": { "domain": "amazonaws.com", "partition": "aws" } } }, + "instance_id": "i-0929128ff2f73a2f1", + "kernel_release": "5.3.0-1010-aws", + "local_hostname": "ip-172-31-81-43", + "machine": "x86_64", + "platform": "ec2", + "public_ssh_keys": [], + "python_version": "3.7.6", + "region": "us-east-1", "sensitive_keys": [], + "subplatform": "metadata (http://169.254.169.254)", + "sys_info": { + "dist": [ + "ubuntu", + "20.04", + "focal" + ], + "platform": "Linux-5.3.0-1010-aws-x86_64-with-Ubuntu-20.04-focal", + "python": "3.7.6", + "release": "5.3.0-1010-aws", + "system": "Linux", + "uname": [ + "Linux", + "ip-172-31-81-43", + "5.3.0-1010-aws", + "#11-Ubuntu SMP Thu Jan 16 07:59:32 UTC 2020", + "x86_64", + "x86_64" + ], + "variant": "ubuntu" + }, + "system_platform": "Linux-5.3.0-1010-aws-x86_64-with-Ubuntu-20.04-focal", + "userdata": "#cloud-config\nssh_import_id: []\n...", "v1": { "_beta_keys": [ "subplatform" ], - "availability-zone": "us-east-2b", - "availability_zone": "us-east-2b", + "availability_zone": "us-east-1b", "cloud_name": "aws", - "instance_id": "i-075f088c72ad3271c", - "local_hostname": "ip-10-41-41-95", + "distro": "ubuntu", + "distro_release": "focal", + "distro_version": "20.04", + "instance_id": "i-0929128ff2f73a2f1", + "kernel": "5.3.0-1010-aws", + "local_hostname": "ip-172-31-81-43", + "machine": "x86_64", "platform": "ec2", - "public_ssh_keys": [ - "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDSL7uWGj8cgWyIOaspgKdVy0cKJ+UTjfv7jBOjG2H/GN8bJVXy72XAvnhM0dUM+CCs8FOf0YlPX+Frvz2hKInrmRhZVwRSL129PasD12MlI3l44u6IwS1o/W86Q+tkQYEljtqDOo0a+cOsaZkvUNzUyEXUwz/lmYa6G4hMKZH4NBj7nbAAF96wsMCoyNwbWryBnDYUr6wMbjRR1J9Pw7Xh7WRC73wy4Va2YuOgbD3V/5ZrFPLbWZW/7TFXVrql04QVbyei4aiFR5n//GvoqwQDNe58LmbzX/xvxyKJYdny2zXmdAhMxbrpFQsfpkJ9E/H5w0yOdSvnWbUoG5xNGoOB cloud-init-integration" - ], - "region": "us-east-2", - "subplatform": "metadata (http://169.254.169.254)" - } + "public_ssh_keys": [], + "python": "3.7.6", + "region": "us-east-1", + "subplatform": "metadata (http://169.254.169.254)", + "system_platform": "Linux-5.3.0-1010-aws-x86_64-with-Ubuntu-20.04-focal", + "variant": "ubuntu" + }, + "variant": "ubuntu", + "vendordata": "" } diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index fd12d87b..5976e234 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -172,9 +172,7 @@ class CloudTestCase(unittest2.TestCase): 'Skipping instance-data.json test.' ' OS: %s not bionic or newer' % self.os_name) instance_data = json.loads(out) - self.assertItemsEqual( - [], - instance_data['base64_encoded_keys']) + self.assertItemsEqual(['ci_cfg'], instance_data['sensitive_keys']) ds = instance_data.get('ds', {}) v1_data = instance_data.get('v1', {}) metadata = ds.get('meta-data', {}) @@ -201,6 +199,23 @@ class CloudTestCase(unittest2.TestCase): self.assertIn('i-', v1_data['instance_id']) self.assertIn('ip-', v1_data['local_hostname']) self.assertIsNotNone(v1_data['region'], 'expected ec2 region') + self.assertIsNotNone( + re.match(r'\d\.\d+\.\d+-\d+-aws', v1_data['kernel_release'])) + self.assertEqual( + 'redacted for non-root user', instance_data['merged_cfg']) + self.assertEqual(self.os_cfg['os'], v1_data['variant']) + self.assertEqual(self.os_cfg['os'], v1_data['distro']) + self.assertEqual( + self.os_cfg['os'], instance_data["sys_info"]['dist'][0], + "Unexpected sys_info dist value") + self.assertEqual(self.os_name, v1_data['distro_release']) + self.assertEqual( + str(self.os_cfg['version']), v1_data['distro_version']) + self.assertEqual('x86_64', v1_data['machine']) + self.assertIsNotNone( + re.match(r'3.\d\.\d', v1_data['python_version']), + "unexpected python version: {ver}".format( + ver=v1_data["python_version"])) def test_instance_data_json_lxd(self): """Validate instance-data.json content by lxd platform. @@ -237,6 +252,23 @@ class CloudTestCase(unittest2.TestCase): self.assertIsNone( v1_data['region'], 'found unexpected lxd region %s' % v1_data['region']) + self.assertIsNotNone( + re.match(r'\d\.\d+\.\d+-\d+', v1_data['kernel_release'])) + self.assertEqual( + 'redacted for non-root user', instance_data['merged_cfg']) + self.assertEqual(self.os_cfg['os'], v1_data['variant']) + self.assertEqual(self.os_cfg['os'], v1_data['distro']) + self.assertEqual( + self.os_cfg['os'], instance_data["sys_info"]['dist'][0], + "Unexpected sys_info dist value") + self.assertEqual(self.os_name, v1_data['distro_release']) + self.assertEqual( + str(self.os_cfg['version']), v1_data['distro_version']) + self.assertEqual('x86_64', v1_data['machine']) + self.assertIsNotNone( + re.match(r'3.\d\.\d', v1_data['python_version']), + "unexpected python version: {ver}".format( + ver=v1_data["python_version"])) def test_instance_data_json_kvm(self): """Validate instance-data.json content by nocloud-kvm platform. @@ -278,6 +310,23 @@ class CloudTestCase(unittest2.TestCase): self.assertIsNone( v1_data['region'], 'found unexpected lxd region %s' % v1_data['region']) + self.assertIsNotNone( + re.match(r'\d\.\d+\.\d+-\d+', v1_data['kernel_release'])) + self.assertEqual( + 'redacted for non-root user', instance_data['merged_cfg']) + self.assertEqual(self.os_cfg['os'], v1_data['variant']) + self.assertEqual(self.os_cfg['os'], v1_data['distro']) + self.assertEqual( + self.os_cfg['os'], instance_data["sys_info"]['dist'][0], + "Unexpected sys_info dist value") + self.assertEqual(self.os_name, v1_data['distro_release']) + self.assertEqual( + str(self.os_cfg['version']), v1_data['distro_version']) + self.assertEqual('x86_64', v1_data['machine']) + self.assertIsNotNone( + re.match(r'3.\d\.\d', v1_data['python_version']), + "unexpected python version: {ver}".format( + ver=v1_data["python_version"])) class PasswordListTest(CloudTestCase): diff --git a/tests/unittests/test_datasource/test_cloudsigma.py b/tests/unittests/test_datasource/test_cloudsigma.py index d62d542b..7aa3b1d1 100644 --- a/tests/unittests/test_datasource/test_cloudsigma.py +++ b/tests/unittests/test_datasource/test_cloudsigma.py @@ -3,6 +3,7 @@ import copy from cloudinit.cs_utils import Cepko +from cloudinit import distros from cloudinit import helpers from cloudinit import sources from cloudinit.sources import DataSourceCloudSigma @@ -47,8 +48,11 @@ class DataSourceCloudSigmaTest(test_helpers.CiTestCase): self.paths = helpers.Paths({'run_dir': self.tmp_dir()}) self.add_patch(DS_PATH + '.is_running_in_cloudsigma', "m_is_container", return_value=True) + + distro_cls = distros.fetch("ubuntu") + distro = distro_cls("ubuntu", cfg={}, paths=self.paths) self.datasource = DataSourceCloudSigma.DataSourceCloudSigma( - "", "", paths=self.paths) + sys_cfg={}, distro=distro, paths=self.paths) self.datasource.cepko = CepkoMock(SERVER_CONTEXT) def test_get_hostname(self): -- cgit v1.2.3 From ade47866aa2f81ab0f3baabbb1fc1e484abdb741 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 19 Mar 2020 12:01:15 -0400 Subject: cloudinit/tests: remove unneeded with_logs configuration (#263) These classes don't use `self.logs` anywhere in their body, so we can remove the `with_logs = True` setting from them. These instances were found using astpath[0], with the following invocation: astpath "//Name[@id='with_logs' and not(ancestor::ClassDef//Attribute[@attr='logs'])]" [0] https://github.com/hchasestevens/astpath --- cloudinit/cmd/tests/test_main.py | 2 -- cloudinit/config/tests/test_disable_ec2_metadata.py | 2 -- cloudinit/net/tests/test_init.py | 6 ------ cloudinit/sources/tests/test_oracle.py | 2 -- tests/unittests/test_datasource/test_azure.py | 2 -- tests/unittests/test_datasource/test_maas.py | 1 - tests/unittests/test_handler/test_handler_locale.py | 2 -- tests/unittests/test_handler/test_handler_puppet.py | 2 -- tests/unittests/test_util.py | 1 - 9 files changed, 20 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/cmd/tests/test_main.py b/cloudinit/cmd/tests/test_main.py index 384fddc6..585b3b0e 100644 --- a/cloudinit/cmd/tests/test_main.py +++ b/cloudinit/cmd/tests/test_main.py @@ -18,8 +18,6 @@ myargs = namedtuple('MyArgs', 'debug files force local reporter subcommand') class TestMain(FilesystemMockingTestCase): - with_logs = True - def setUp(self): super(TestMain, self).setUp() self.new_root = self.tmp_dir() diff --git a/cloudinit/config/tests/test_disable_ec2_metadata.py b/cloudinit/config/tests/test_disable_ec2_metadata.py index 67646b03..823917c7 100644 --- a/cloudinit/config/tests/test_disable_ec2_metadata.py +++ b/cloudinit/config/tests/test_disable_ec2_metadata.py @@ -15,8 +15,6 @@ DISABLE_CFG = {'disable_ec2_metadata': 'true'} class TestEC2MetadataRoute(CiTestCase): - with_logs = True - @mock.patch('cloudinit.config.cc_disable_ec2_metadata.util.which') @mock.patch('cloudinit.config.cc_disable_ec2_metadata.util.subp') def test_disable_ifconfig(self, m_subp, m_which): diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index 5081a337..a965699a 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -341,8 +341,6 @@ class TestGenerateFallbackConfig(CiTestCase): class TestNetFindFallBackNic(CiTestCase): - with_logs = True - def setUp(self): super(TestNetFindFallBackNic, self).setUp() sys_mock = mock.patch('cloudinit.net.get_sys_class_path') @@ -995,8 +993,6 @@ class TestExtractPhysdevs(CiTestCase): class TestWaitForPhysdevs(CiTestCase): - with_logs = True - def setUp(self): super(TestWaitForPhysdevs, self).setUp() self.add_patch('cloudinit.net.get_interfaces_by_mac', @@ -1071,8 +1067,6 @@ class TestWaitForPhysdevs(CiTestCase): class TestNetFailOver(CiTestCase): - with_logs = True - def setUp(self): super(TestNetFailOver, self).setUp() self.add_patch('cloudinit.net.util', 'm_util') diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index abf3d359..316efc4f 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -588,8 +588,6 @@ class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): class TestNetworkConfigFiltersNetFailover(test_helpers.CiTestCase): - with_logs = True - def setUp(self): super(TestNetworkConfigFiltersNetFailover, self).setUp() self.add_patch(DS_PATH + '.get_interfaces_by_mac', diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index a809fd87..2c6aa44d 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -383,8 +383,6 @@ class TestGetMetadataFromIMDS(HttprettyTestCase): class TestAzureDataSource(CiTestCase): - with_logs = True - def setUp(self): super(TestAzureDataSource, self).setUp() self.tmp = self.tmp_dir() diff --git a/tests/unittests/test_datasource/test_maas.py b/tests/unittests/test_datasource/test_maas.py index 2a81d3f5..41b6c27b 100644 --- a/tests/unittests/test_datasource/test_maas.py +++ b/tests/unittests/test_datasource/test_maas.py @@ -158,7 +158,6 @@ class TestMAASDataSource(CiTestCase): @mock.patch("cloudinit.sources.DataSourceMAAS.url_helper.OauthUrlHelper") class TestGetOauthHelper(CiTestCase): - with_logs = True base_cfg = {'consumer_key': 'FAKE_CONSUMER_KEY', 'token_key': 'FAKE_TOKEN_KEY', 'token_secret': 'FAKE_TOKEN_SECRET', diff --git a/tests/unittests/test_handler/test_handler_locale.py b/tests/unittests/test_handler/test_handler_locale.py index 2b22559f..407aa6c4 100644 --- a/tests/unittests/test_handler/test_handler_locale.py +++ b/tests/unittests/test_handler/test_handler_locale.py @@ -29,8 +29,6 @@ LOG = logging.getLogger(__name__) class TestLocale(t_help.FilesystemMockingTestCase): - with_logs = True - def setUp(self): super(TestLocale, self).setUp() self.new_root = tempfile.mkdtemp() diff --git a/tests/unittests/test_handler/test_handler_puppet.py b/tests/unittests/test_handler/test_handler_puppet.py index 1494177d..04aa7d03 100644 --- a/tests/unittests/test_handler/test_handler_puppet.py +++ b/tests/unittests/test_handler/test_handler_puppet.py @@ -16,8 +16,6 @@ LOG = logging.getLogger(__name__) @mock.patch('cloudinit.config.cc_puppet.os') class TestAutostartPuppet(CiTestCase): - with_logs = True - def test_wb_autostart_puppet_updates_puppet_default(self, m_os, m_util): """Update /etc/default/puppet to autostart if it exists.""" diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 9ff17f52..5b2eaa69 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -737,7 +737,6 @@ class TestReadSeeded(helpers.TestCase): class TestSubp(helpers.CiTestCase): - with_logs = True allowed_subp = [BASH, 'cat', helpers.CiTestCase.SUBP_SHELL_TRUE, BOGUS_COMMAND, sys.executable] -- cgit v1.2.3 From 0173e852a760d2b9a697b5f07330a226839e350b Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 20 Mar 2020 15:51:07 -0400 Subject: test_oracle,DataSourceOracle: sort imports (#266) * test_oracle: sort imports * DataSourceOracle: sort imports --- cloudinit/sources/DataSourceOracle.py | 18 ++++++++++-------- cloudinit/sources/tests/test_oracle.py | 17 +++++++++-------- 2 files changed, 19 insertions(+), 16 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 83fcfc16..90e1881a 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -15,17 +15,19 @@ Notes: * Both bare-metal and vms provide chassis-asset-tag of OracleCloud.com """ -from cloudinit.url_helper import combine_url, readurl, UrlError -from cloudinit.net import dhcp, get_interfaces_by_mac, is_netfail_master -from cloudinit import net -from cloudinit import sources -from cloudinit import util -from cloudinit.net import cmdline -from cloudinit import log as logging - import json import re +from cloudinit import log as logging +from cloudinit import net, sources, util +from cloudinit.net import ( + cmdline, + dhcp, + get_interfaces_by_mac, + is_netfail_master, +) +from cloudinit.url_helper import UrlError, combine_url, readurl + LOG = logging.getLogger(__name__) BUILTIN_DS_CONFIG = { diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 316efc4f..4e135df9 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -1,20 +1,21 @@ # This file is part of cloud-init. See LICENSE file for license information. -from cloudinit.sources import DataSourceOracle as oracle -from cloudinit.sources import BrokenMetadata, NetworkConfigSource -from cloudinit import helpers - -from cloudinit.tests import helpers as test_helpers - -from textwrap import dedent import argparse import copy -import httpretty import json import os import uuid +from textwrap import dedent from unittest import mock +import httpretty + +from cloudinit import helpers +from cloudinit.sources import BrokenMetadata +from cloudinit.sources import DataSourceOracle as oracle +from cloudinit.sources import NetworkConfigSource +from cloudinit.tests import helpers as test_helpers + DS_PATH = "cloudinit.sources.DataSourceOracle" MD_VER = "2013-10-17" -- cgit v1.2.3 From 1905ff47e9e688d9a48d6132fd45b341f2d66fe4 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 27 Mar 2020 11:45:45 -0400 Subject: sources/tests/test_init: drop use of deprecated inspect.getargspec (#285) --- cloudinit/sources/tests/test_init.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 541cbbeb..5b3648e9 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -551,9 +551,7 @@ class TestDataSource(CiTestCase): def test_get_hostname_subclass_support(self): """Validate get_hostname signature on all subclasses of DataSource.""" - # Use inspect.getfullargspec when we drop py2.6 and py2.7 - get_args = inspect.getargspec # pylint: disable=W1505 - base_args = get_args(DataSource.get_hostname) # pylint: disable=W1505 + base_args = inspect.getfullargspec(DataSource.get_hostname) # Import all DataSource subclasses so we can inspect them. modules = util.find_modules(os.path.dirname(os.path.dirname(__file__))) for _loc, name in modules.items(): @@ -565,13 +563,13 @@ class TestDataSource(CiTestCase): continue self.assertEqual( base_args, - get_args(child.get_hostname), # pylint: disable=W1505 + inspect.getfullargspec(child.get_hostname), '%s does not implement DataSource.get_hostname params' % child) for grandchild in child.__subclasses__(): self.assertEqual( base_args, - get_args(grandchild.get_hostname), # pylint: disable=W1505 + inspect.getfullargspec(grandchild.get_hostname), '%s does not implement DataSource.get_hostname params' % grandchild) -- cgit v1.2.3 From d7cad8b61a3b5929a65202e0964aa9b4624e06c4 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Mon, 20 Apr 2020 14:50:37 -0400 Subject: tests: add missing mocks for get_interfaces_by_mac (#326) We currently have a test system where get_interfaces_by_mac raises an exception, which is causing these tests to fail as they aren't mocking get_interfaces_by_mac out. LP: #1873910 --- cloudinit/sources/tests/test_oracle.py | 2 ++ tests/unittests/test_datasource/test_opennebula.py | 1 + 2 files changed, 3 insertions(+) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 4e135df9..2265327b 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -186,6 +186,7 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertEqual(self.my_md['uuid'], ds.get_instance_id()) self.assertEqual(my_userdata, ds.userdata_raw) + @mock.patch(DS_PATH + ".get_interfaces_by_mac", mock.Mock(return_value={})) @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", side_effect=lambda network_config: network_config) @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") @@ -207,6 +208,7 @@ class TestDataSourceOracle(test_helpers.CiTestCase): self.assertEqual([mock.call()], m_initramfs_config.call_args_list) self.assertFalse(distro.generate_fallback_config.called) + @mock.patch(DS_PATH + ".get_interfaces_by_mac", mock.Mock(return_value={})) @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", side_effect=lambda network_config: network_config) @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py index bb399f6d..de896a9e 100644 --- a/tests/unittests/test_datasource/test_opennebula.py +++ b/tests/unittests/test_datasource/test_opennebula.py @@ -355,6 +355,7 @@ class TestOpenNebulaDataSource(CiTestCase): util.find_devs_with = orig_find_devs_with +@mock.patch(DS_PATH + '.net.get_interfaces_by_mac', mock.Mock(return_value={})) class TestOpenNebulaNetwork(unittest.TestCase): system_nics = ('eth0', 'ens3') -- cgit v1.2.3 From 38a7e6e9756fdab31264c0d6e93d20432ed111ac Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 24 Apr 2020 09:26:51 -0400 Subject: cloudinit: drop dependencies on unittest2 and contextlib2 (#322) These libraries provide backports of Python 3's stdlib components to Python 2. As we only support Python 3, we can simply use the stdlib now. This pull request does the following: * removes some unneeded compatibility code for the old spelling of `assertRaisesRegex` * replaces invocations of the Python 2-only `assertItemsEqual` with its new name, `assertCountEqual` * replaces all usage of `unittest2` with `unittest` * replaces all usage of `contextlib2` with `contextlib` * drops `unittest2` and `contextlib2` from requirements files and tox.ini It also rewrites some `test_azure` helpers to use bare asserts. We were seeing a strange error in xenial builds of this branch which appear to be stemming from the AssertionError that pytest produces being _different_ from the standard AssertionError. This means that the modified helpers weren't behaving correctly, because they weren't catching AssertionErrors as one would expect. (I believe this is related, in some way, to https://github.com/pytest-dev/pytest/issues/645, but the only version of pytest where we're affected is so far in the past that it's not worth pursuing it any further as we have a workaround.) --- cloudinit/config/tests/test_users_groups.py | 10 ++++---- cloudinit/net/tests/test_dhcp.py | 10 ++++---- cloudinit/net/tests/test_init.py | 2 +- cloudinit/sources/tests/test_init.py | 10 ++++---- cloudinit/tests/helpers.py | 28 ++++++---------------- integration-requirements.txt | 1 - packages/pkg-deps.json | 3 --- test-requirements.txt | 2 -- tests/cloud_tests/testcases/__init__.py | 8 +++---- tests/cloud_tests/testcases/base.py | 12 +++++----- tests/cloud_tests/testcases/modules/ntp_chrony.py | 4 ++-- tests/cloud_tests/verify.py | 4 ++-- tests/unittests/test_builtin_handlers.py | 6 ++--- tests/unittests/test_datasource/test_azure.py | 8 +++---- .../unittests/test_datasource/test_azure_helper.py | 6 ++--- tests/unittests/test_datasource/test_smartos.py | 12 +++++----- .../test_handler/test_handler_ca_certs.py | 6 +---- tests/unittests/test_handler/test_handler_chef.py | 8 +++---- .../test_handler/test_handler_growpart.py | 6 +---- tests/unittests/test_handler/test_schema.py | 4 ++-- tox.ini | 2 -- 21 files changed, 61 insertions(+), 91 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/config/tests/test_users_groups.py b/cloudinit/config/tests/test_users_groups.py index f620b597..df89ddb3 100644 --- a/cloudinit/config/tests/test_users_groups.py +++ b/cloudinit/config/tests/test_users_groups.py @@ -39,7 +39,7 @@ class TestHandleUsersGroups(CiTestCase): cloud = self.tmp_cloud( distro='ubuntu', sys_cfg=sys_cfg, metadata=metadata) cc_users_groups.handle('modulename', cfg, cloud, None, None) - self.assertItemsEqual( + self.assertCountEqual( m_user.call_args_list, [mock.call('ubuntu', groups='lxd,sudo', lock_passwd=True, shell='/bin/bash'), @@ -65,7 +65,7 @@ class TestHandleUsersGroups(CiTestCase): cloud = self.tmp_cloud( distro='freebsd', sys_cfg=sys_cfg, metadata=metadata) cc_users_groups.handle('modulename', cfg, cloud, None, None) - self.assertItemsEqual( + self.assertCountEqual( m_fbsd_user.call_args_list, [mock.call('freebsd', groups='wheel', lock_passwd=True, shell='/bin/tcsh'), @@ -86,7 +86,7 @@ class TestHandleUsersGroups(CiTestCase): cloud = self.tmp_cloud( distro='ubuntu', sys_cfg=sys_cfg, metadata=metadata) cc_users_groups.handle('modulename', cfg, cloud, None, None) - self.assertItemsEqual( + self.assertCountEqual( m_user.call_args_list, [mock.call('ubuntu', groups='lxd,sudo', lock_passwd=True, shell='/bin/bash'), @@ -107,7 +107,7 @@ class TestHandleUsersGroups(CiTestCase): cloud = self.tmp_cloud( distro='ubuntu', sys_cfg=sys_cfg, metadata=metadata) cc_users_groups.handle('modulename', cfg, cloud, None, None) - self.assertItemsEqual( + self.assertCountEqual( m_user.call_args_list, [mock.call('ubuntu', groups='lxd,sudo', lock_passwd=True, shell='/bin/bash'), @@ -146,7 +146,7 @@ class TestHandleUsersGroups(CiTestCase): cloud = self.tmp_cloud( distro='ubuntu', sys_cfg=sys_cfg, metadata=metadata) cc_users_groups.handle('modulename', cfg, cloud, None, None) - self.assertItemsEqual( + self.assertCountEqual( m_user.call_args_list, [mock.call('ubuntu', groups='lxd,sudo', lock_passwd=True, shell='/bin/bash'), diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index c3fa1e04..bc7bef45 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -62,7 +62,7 @@ class TestParseDHCPLeasesFile(CiTestCase): {'interface': 'wlp3s0', 'fixed-address': '192.168.2.74', 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1'}] write_file(lease_file, content) - self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file)) + self.assertCountEqual(expected, parse_dhcp_lease_file(lease_file)) class TestDHCPRFC3442(CiTestCase): @@ -88,7 +88,7 @@ class TestDHCPRFC3442(CiTestCase): 'renew': '4 2017/07/27 18:02:30', 'expire': '5 2017/07/28 07:08:15'}] write_file(lease_file, content) - self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file)) + self.assertCountEqual(expected, parse_dhcp_lease_file(lease_file)) def test_parse_lease_finds_classless_static_routes(self): """ @@ -114,7 +114,7 @@ class TestDHCPRFC3442(CiTestCase): 'renew': '4 2017/07/27 18:02:30', 'expire': '5 2017/07/28 07:08:15'}] write_file(lease_file, content) - self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file)) + self.assertCountEqual(expected, parse_dhcp_lease_file(lease_file)) @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') @@ -324,7 +324,7 @@ class TestDHCPDiscoveryClean(CiTestCase): """) write_file(self.tmp_path('dhcp.leases', tmpdir), lease_content) - self.assertItemsEqual( + self.assertCountEqual( [{'interface': 'eth9', 'fixed-address': '192.168.2.74', 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1'}], dhcp_discovery(dhclient_script, 'eth9', tmpdir)) @@ -389,7 +389,7 @@ class TestDHCPDiscoveryClean(CiTestCase): write_file(pid_file, "%d\n" % my_pid) m_getppid.return_value = 1 # Indicate that dhclient has daemonized - self.assertItemsEqual( + self.assertCountEqual( [{'interface': 'eth9', 'fixed-address': '192.168.2.74', 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1'}], dhcp_discovery(dhclient_script, 'eth9', tmpdir)) diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index 32e70b4c..835ed807 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -397,7 +397,7 @@ class TestGetDeviceList(CiTestCase): """get_devicelist returns a directory listing for SYS_CLASS_NET.""" write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), 'up') write_file(os.path.join(self.sysdir, 'eth1', 'operstate'), 'up') - self.assertItemsEqual(['eth0', 'eth1'], net.get_devicelist()) + self.assertCountEqual(['eth0', 'eth1'], net.get_devicelist()) class TestGetInterfaceMAC(CiTestCase): diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 5b3648e9..5b6f1b3f 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -350,7 +350,7 @@ class TestDataSource(CiTestCase): 'region': 'myregion', 'some': {'security-credentials': { 'cred1': 'sekret', 'cred2': 'othersekret'}}}) - self.assertItemsEqual( + self.assertCountEqual( ('merged_cfg', 'security-credentials',), datasource.sensitive_metadata_keys) sys_info = { @@ -401,7 +401,7 @@ class TestDataSource(CiTestCase): 'region': 'myregion', 'some': {'security-credentials': REDACT_SENSITIVE_VALUE}}} } - self.assertItemsEqual(expected, redacted) + self.assertCountEqual(expected, redacted) file_stat = os.stat(json_file) self.assertEqual(0o644, stat.S_IMODE(file_stat.st_mode)) @@ -426,7 +426,7 @@ class TestDataSource(CiTestCase): "x86_64"], "variant": "ubuntu", "dist": ["ubuntu", "20.04", "focal"]} - self.assertItemsEqual( + self.assertCountEqual( ('merged_cfg', 'security-credentials',), datasource.sensitive_metadata_keys) with mock.patch("cloudinit.util.system_info", return_value=sys_info): @@ -476,7 +476,7 @@ class TestDataSource(CiTestCase): 'security-credentials': {'cred1': 'sekret', 'cred2': 'othersekret'}}}} } - self.assertItemsEqual(expected, util.load_json(content)) + self.assertCountEqual(expected, util.load_json(content)) file_stat = os.stat(sensitive_json_file) self.assertEqual(0o600, stat.S_IMODE(file_stat.st_mode)) self.assertEqual(expected, util.load_json(content)) @@ -542,7 +542,7 @@ class TestDataSource(CiTestCase): json_file = self.tmp_path(INSTANCE_JSON_FILE, tmp) content = util.load_file(json_file) instance_json = util.load_json(content) - self.assertItemsEqual( + self.assertCountEqual( ['ds/meta_data/key2/key2.1'], instance_json['base64_encoded_keys']) self.assertEqual( diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index f4db5827..477e14c2 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -13,15 +13,10 @@ import string import sys import tempfile import time +import unittest +from contextlib import ExitStack, contextmanager from unittest import mock - -import unittest2 -from unittest2.util import strclass - -try: - from contextlib import ExitStack, contextmanager -except ImportError: - from contextlib2 import ExitStack, contextmanager +from unittest.util import strclass from cloudinit.config.schema import ( SchemaValidationError, validate_cloudconfig_schema) @@ -35,8 +30,8 @@ from cloudinit import util _real_subp = util.subp # Used for skipping tests -SkipTest = unittest2.SkipTest -skipIf = unittest2.skipIf +SkipTest = unittest.SkipTest +skipIf = unittest.skipIf # Makes the old path start @@ -73,7 +68,7 @@ def retarget_many_wrapper(new_base, am, old_func): return wrapper -class TestCase(unittest2.TestCase): +class TestCase(unittest.TestCase): def reset_global_state(self): """Reset any global state to its original settings. @@ -372,7 +367,7 @@ class HttprettyTestCase(CiTestCase): super(HttprettyTestCase, self).tearDown() -class SchemaTestCaseMixin(unittest2.TestCase): +class SchemaTestCaseMixin(unittest.TestCase): def assertSchemaValid(self, cfg, msg="Valid Schema failed validation."): """Assert the config is valid per self.schema. @@ -504,13 +499,4 @@ if not hasattr(mock.Mock, 'assert_not_called'): raise AssertionError(msg) mock.Mock.assert_not_called = __mock_assert_not_called - -# older unittest2.TestCase (centos6) have only the now-deprecated -# assertRaisesRegexp. Simple assignment makes pylint complain, about -# users of assertRaisesRegex so we use getattr to trick it. -# https://github.com/PyCQA/pylint/issues/1946 -if not hasattr(unittest2.TestCase, 'assertRaisesRegex'): - unittest2.TestCase.assertRaisesRegex = ( - getattr(unittest2.TestCase, 'assertRaisesRegexp')) - # vi: ts=4 expandtab diff --git a/integration-requirements.txt b/integration-requirements.txt index 897d6110..44e45c1b 100644 --- a/integration-requirements.txt +++ b/integration-requirements.txt @@ -5,7 +5,6 @@ # the packages/pkg-deps.json file as well. # -unittest2 # ec2 backend boto3==1.5.9 diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json index d09ec33f..f02e8348 100644 --- a/packages/pkg-deps.json +++ b/packages/pkg-deps.json @@ -10,9 +10,6 @@ "2" : "python-yaml", "3" : "python3-yaml" }, - "contextlib2" : { - "2" : "python-contextlib2" - }, "pyserial" : { "2" : "python-serial", "3" : "python3-serial" diff --git a/test-requirements.txt b/test-requirements.txt index 057115b6..0a6a04d4 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,9 +1,7 @@ # Needed generally in tests httpretty>=0.7.1 pytest -unittest2 pytest-cov # Only really needed on older versions of python -contextlib2 setuptools diff --git a/tests/cloud_tests/testcases/__init__.py b/tests/cloud_tests/testcases/__init__.py index 6bb39f77..e8c371ca 100644 --- a/tests/cloud_tests/testcases/__init__.py +++ b/tests/cloud_tests/testcases/__init__.py @@ -4,7 +4,7 @@ import importlib import inspect -import unittest2 +import unittest from cloudinit.util import read_conf @@ -48,7 +48,7 @@ def get_test_class(test_name, test_data, test_conf): def __str__(self): return "%s (%s)" % (self._testMethodName, - unittest2.util.strclass(self._realclass)) + unittest.util.strclass(self._realclass)) @classmethod def setUpClass(cls): @@ -62,9 +62,9 @@ def get_suite(test_name, data, conf): @return_value: a test suite """ - suite = unittest2.TestSuite() + suite = unittest.TestSuite() suite.addTest( - unittest2.defaultTestLoader.loadTestsFromTestCase( + unittest.defaultTestLoader.loadTestsFromTestCase( get_test_class(test_name, data, conf))) return suite diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index b0dfc144..7b67f54e 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -5,15 +5,15 @@ import crypt import json import re -import unittest2 +import unittest from cloudinit import util as c_util -SkipTest = unittest2.SkipTest +SkipTest = unittest.SkipTest -class CloudTestCase(unittest2.TestCase): +class CloudTestCase(unittest.TestCase): """Base test class for verifiers.""" # data gets populated in get_suite.setUpClass @@ -172,7 +172,7 @@ class CloudTestCase(unittest2.TestCase): 'Skipping instance-data.json test.' ' OS: %s not bionic or newer' % self.os_name) instance_data = json.loads(out) - self.assertItemsEqual(['merged_cfg'], instance_data['sensitive_keys']) + self.assertCountEqual(['merged_cfg'], instance_data['sensitive_keys']) ds = instance_data.get('ds', {}) v1_data = instance_data.get('v1', {}) metadata = ds.get('meta-data', {}) @@ -237,7 +237,7 @@ class CloudTestCase(unittest2.TestCase): ' OS: %s not bionic or newer' % self.os_name) instance_data = json.loads(out) v1_data = instance_data.get('v1', {}) - self.assertItemsEqual([], sorted(instance_data['base64_encoded_keys'])) + self.assertCountEqual([], sorted(instance_data['base64_encoded_keys'])) self.assertEqual('unknown', v1_data['cloud_name']) self.assertEqual('lxd', v1_data['platform']) self.assertEqual( @@ -291,7 +291,7 @@ class CloudTestCase(unittest2.TestCase): ' OS: %s not bionic or newer' % self.os_name) instance_data = json.loads(out) v1_data = instance_data.get('v1', {}) - self.assertItemsEqual([], instance_data['base64_encoded_keys']) + self.assertCountEqual([], instance_data['base64_encoded_keys']) self.assertEqual('unknown', v1_data['cloud_name']) self.assertEqual('nocloud', v1_data['platform']) subplatform = v1_data['subplatform'] diff --git a/tests/cloud_tests/testcases/modules/ntp_chrony.py b/tests/cloud_tests/testcases/modules/ntp_chrony.py index 0f4c3d08..7d341773 100644 --- a/tests/cloud_tests/testcases/modules/ntp_chrony.py +++ b/tests/cloud_tests/testcases/modules/ntp_chrony.py @@ -1,7 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. """cloud-init Integration Test Verify Script.""" -import unittest2 +import unittest from tests.cloud_tests.testcases import base @@ -13,7 +13,7 @@ class TestNtpChrony(base.CloudTestCase): """Skip this suite of tests on lxd and artful or older.""" if self.platform == 'lxd': if self.is_distro('ubuntu') and self.os_version_cmp('artful') <= 0: - raise unittest2.SkipTest( + raise unittest.SkipTest( 'No support for chrony on containers <= artful.' ' LP: #1589780') return super(TestNtpChrony, self).setUp() diff --git a/tests/cloud_tests/verify.py b/tests/cloud_tests/verify.py index 7018f4d5..0295af40 100644 --- a/tests/cloud_tests/verify.py +++ b/tests/cloud_tests/verify.py @@ -3,7 +3,7 @@ """Verify test results.""" import os -import unittest2 +import unittest from tests.cloud_tests import (config, LOG, util, testcases) @@ -18,7 +18,7 @@ def verify_data(data_dir, platform, os_name, tests): @return_value: {: {passed: True/False, failures: []}} """ base_dir = os.sep.join((data_dir, platform, os_name)) - runner = unittest2.TextTestRunner(verbosity=util.current_verbosity()) + runner = unittest.TextTestRunner(verbosity=util.current_verbosity()) res = {} for test_name in tests: LOG.debug('verifying test data for %s', test_name) diff --git a/tests/unittests/test_builtin_handlers.py b/tests/unittests/test_builtin_handlers.py index b92ffc79..9045e743 100644 --- a/tests/unittests/test_builtin_handlers.py +++ b/tests/unittests/test_builtin_handlers.py @@ -109,7 +109,7 @@ class TestJinjaTemplatePartHandler(CiTestCase): cloudconfig_handler = CloudConfigPartHandler(self.paths) h = JinjaTemplatePartHandler( self.paths, sub_handlers=[script_handler, cloudconfig_handler]) - self.assertItemsEqual( + self.assertCountEqual( ['text/cloud-config', 'text/cloud-config-jsonp', 'text/x-shellscript'], h.sub_handlers) @@ -120,7 +120,7 @@ class TestJinjaTemplatePartHandler(CiTestCase): cloudconfig_handler = CloudConfigPartHandler(self.paths) h = JinjaTemplatePartHandler( self.paths, sub_handlers=[script_handler, cloudconfig_handler]) - self.assertItemsEqual( + self.assertCountEqual( ['text/cloud-config', 'text/cloud-config-jsonp', 'text/x-shellscript'], h.sub_handlers) @@ -302,7 +302,7 @@ class TestConvertJinjaInstanceData(CiTestCase): expected_data.update({'v1key1': 'v1.1', 'v2key1': 'v2.1'}) converted_data = convert_jinja_instance_data(data=data) - self.assertItemsEqual( + self.assertCountEqual( ['ds', 'v1', 'v2', 'v1key1', 'v2key1'], converted_data.keys()) self.assertEqual( expected_data, diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 2c6aa44d..2f8561cb 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -528,14 +528,14 @@ scbus-1 on xpt0 bus 0 def tags_exists(x, y): for tag in x.keys(): - self.assertIn(tag, y) + assert tag in y for tag in y.keys(): - self.assertIn(tag, x) + assert tag in x def tags_equal(x, y): for x_val in x.values(): y_val = y.get(x_val.tag) - self.assertEqual(x_val.text, y_val.text) + assert x_val.text == y_val.text old_cnt = create_tag_index(oxml) new_cnt = create_tag_index(nxml) @@ -649,7 +649,7 @@ scbus-1 on xpt0 bus 0 crawled_metadata = dsrc.crawl_metadata() - self.assertItemsEqual( + self.assertCountEqual( crawled_metadata.keys(), ['cfg', 'files', 'metadata', 'userdata_raw']) self.assertEqual(crawled_metadata['cfg'], expected_cfg) diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index 007df09f..6344b0bb 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -1,7 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. import os -import unittest2 +import unittest from textwrap import dedent from cloudinit.sources.helpers import azure as azure_helper @@ -332,7 +332,7 @@ class TestOpenSSLManagerActions(CiTestCase): path = 'tests/data/azure' return os.path.join(path, name) - @unittest2.skip("todo move to cloud_test") + @unittest.skip("todo move to cloud_test") def test_pubkey_extract(self): cert = load_file(self._data_file('pubkey_extract_cert')) good_key = load_file(self._data_file('pubkey_extract_ssh_key')) @@ -344,7 +344,7 @@ class TestOpenSSLManagerActions(CiTestCase): fingerprint = sslmgr._get_fingerprint_from_cert(cert) self.assertEqual(good_fingerprint, fingerprint) - @unittest2.skip("todo move to cloud_test") + @unittest.skip("todo move to cloud_test") @mock.patch.object(azure_helper.OpenSSLManager, '_decrypt_certs_from_xml') def test_parse_certificates(self, mock_decrypt_certs): """Azure control plane puts private keys as well as certificates diff --git a/tests/unittests/test_datasource/test_smartos.py b/tests/unittests/test_datasource/test_smartos.py index 62084de5..0f0b42bb 100644 --- a/tests/unittests/test_datasource/test_smartos.py +++ b/tests/unittests/test_datasource/test_smartos.py @@ -22,7 +22,7 @@ import os.path import re import signal import stat -import unittest2 +import unittest import uuid from cloudinit import serial @@ -1095,11 +1095,11 @@ class TestNetworkConversion(CiTestCase): self.assertEqual(expected, found) -@unittest2.skipUnless(get_smartos_environ() == SMARTOS_ENV_KVM, - "Only supported on KVM and bhyve guests under SmartOS") -@unittest2.skipUnless(os.access(SERIAL_DEVICE, os.W_OK), - "Requires write access to " + SERIAL_DEVICE) -@unittest2.skipUnless(HAS_PYSERIAL is True, "pyserial not available") +@unittest.skipUnless(get_smartos_environ() == SMARTOS_ENV_KVM, + "Only supported on KVM and bhyve guests under SmartOS") +@unittest.skipUnless(os.access(SERIAL_DEVICE, os.W_OK), + "Requires write access to " + SERIAL_DEVICE) +@unittest.skipUnless(HAS_PYSERIAL is True, "pyserial not available") class TestSerialConcurrency(CiTestCase): """ This class tests locking on an actual serial port, and as such can only diff --git a/tests/unittests/test_handler/test_handler_ca_certs.py b/tests/unittests/test_handler/test_handler_ca_certs.py index 5b4105dd..286ef771 100644 --- a/tests/unittests/test_handler/test_handler_ca_certs.py +++ b/tests/unittests/test_handler/test_handler_ca_certs.py @@ -11,13 +11,9 @@ import logging import shutil import tempfile import unittest +from contextlib import ExitStack from unittest import mock -try: - from contextlib import ExitStack -except ImportError: - from contextlib2 import ExitStack - class TestNoConfig(unittest.TestCase): def setUp(self): diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py index 2dab3a54..513c18b5 100644 --- a/tests/unittests/test_handler/test_handler_chef.py +++ b/tests/unittests/test_handler/test_handler_chef.py @@ -65,18 +65,18 @@ class TestInstallChefOmnibus(HttprettyTestCase): cc_chef.install_chef_from_omnibus() expected_kwargs = {'retries': cc_chef.OMNIBUS_URL_RETRIES, 'url': cc_chef.OMNIBUS_URL} - self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[0][1]) + self.assertCountEqual(expected_kwargs, m_rdurl.call_args_list[0][1]) cc_chef.install_chef_from_omnibus(retries=10) expected_kwargs = {'retries': 10, 'url': cc_chef.OMNIBUS_URL} - self.assertItemsEqual(expected_kwargs, m_rdurl.call_args_list[1][1]) + self.assertCountEqual(expected_kwargs, m_rdurl.call_args_list[1][1]) expected_subp_kwargs = { 'args': ['-v', '2.0'], 'basename': 'chef-omnibus-install', 'blob': m_rdurl.return_value.contents, 'capture': False } - self.assertItemsEqual( + self.assertCountEqual( expected_subp_kwargs, m_subp_blob.call_args_list[0][1]) @@ -97,7 +97,7 @@ class TestInstallChefOmnibus(HttprettyTestCase): 'blob': response, 'capture': False } - self.assertItemsEqual(expected_kwargs, called_kwargs) + self.assertCountEqual(expected_kwargs, called_kwargs) class TestChef(FilesystemMockingTestCase): diff --git a/tests/unittests/test_handler/test_handler_growpart.py b/tests/unittests/test_handler/test_handler_growpart.py index 43b53745..501bcca5 100644 --- a/tests/unittests/test_handler/test_handler_growpart.py +++ b/tests/unittests/test_handler/test_handler_growpart.py @@ -11,13 +11,9 @@ import logging import os import re import unittest +from contextlib import ExitStack from unittest import mock -try: - from contextlib import ExitStack -except ImportError: - from contextlib2 import ExitStack - # growpart: # mode: auto # off, on, auto, 'growpart' # devices: ['root'] diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py index abde02d3..98628120 100644 --- a/tests/unittests/test_handler/test_schema.py +++ b/tests/unittests/test_handler/test_schema.py @@ -20,7 +20,7 @@ class GetSchemaTest(CiTestCase): def test_get_schema_coalesces_known_schema(self): """Every cloudconfig module with schema is listed in allOf keyword.""" schema = get_schema() - self.assertItemsEqual( + self.assertCountEqual( [ 'cc_bootcmd', 'cc_ntp', @@ -38,7 +38,7 @@ class GetSchemaTest(CiTestCase): schema['$schema']) # FULL_SCHEMA is updated by the get_schema call from cloudinit.config.schema import FULL_SCHEMA - self.assertItemsEqual(['id', '$schema', 'allOf'], FULL_SCHEMA.keys()) + self.assertCountEqual(['id', '$schema', 'allOf'], FULL_SCHEMA.keys()) def test_get_schema_returns_global_when_set(self): """When FULL_SCHEMA global is already set, get_schema returns it.""" diff --git a/tox.ini b/tox.ini index 416ddcb8..906519c4 100644 --- a/tox.ini +++ b/tox.ini @@ -75,8 +75,6 @@ deps = # test-requirements httpretty==0.9.6 mock==1.3.0 - unittest2==1.1.0 - contextlib2==0.5.1 [testenv:xenial] # When updating this commands definition, also update the definition in -- cgit v1.2.3 From f3bd42659efeed4b092ffcdfd5df7f24813f2d3e Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Wed, 10 Jun 2020 07:39:29 -0700 Subject: test: fix all flake8 E126 errors (#425) --- cloudinit/cmd/devel/render.py | 5 +- cloudinit/cmd/query.py | 5 +- cloudinit/distros/netbsd.py | 10 +- cloudinit/distros/openbsd.py | 7 +- cloudinit/net/__init__.py | 5 +- cloudinit/net/bsd.py | 5 +- cloudinit/net/netbsd.py | 5 +- cloudinit/net/openbsd.py | 5 +- cloudinit/net/sysconfig.py | 2 +- cloudinit/reporting/handlers.py | 14 +- cloudinit/sources/DataSourceAzure.py | 33 +++-- cloudinit/sources/helpers/tests/test_netlink.py | 165 +++++++++++++-------- cloudinit/sources/tests/test_init.py | 9 +- tests/cloud_tests/testcases/base.py | 2 +- tests/unittests/test_datasource/test_azure.py | 33 +++-- tests/unittests/test_datasource/test_scaleway.py | 42 ++++-- tests/unittests/test_distros/test_bsd_utils.py | 5 +- .../unittests/test_handler/test_handler_mounts.py | 7 +- tests/unittests/test_reporting_hyperv.py | 11 +- tests/unittests/test_sshutil.py | 18 ++- tox.ini | 3 +- 21 files changed, 238 insertions(+), 153 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/cmd/devel/render.py b/cloudinit/cmd/devel/render.py index 1bc22406..1090aa16 100755 --- a/cloudinit/cmd/devel/render.py +++ b/cloudinit/cmd/devel/render.py @@ -57,8 +57,9 @@ def handle_args(name, args): paths.run_dir, INSTANCE_JSON_SENSITIVE_FILE) if not os.path.exists(instance_data_fn): LOG.warning( - 'Missing root-readable %s. Using redacted %s instead.', - instance_data_fn, redacted_data_fn) + 'Missing root-readable %s. Using redacted %s instead.', + instance_data_fn, redacted_data_fn + ) instance_data_fn = redacted_data_fn else: instance_data_fn = redacted_data_fn diff --git a/cloudinit/cmd/query.py b/cloudinit/cmd/query.py index e3db8679..0fb48ebd 100644 --- a/cloudinit/cmd/query.py +++ b/cloudinit/cmd/query.py @@ -90,8 +90,9 @@ def handle_args(name, args): instance_data_fn = sensitive_data_fn else: LOG.warning( - 'Missing root-readable %s. Using redacted %s instead.', - sensitive_data_fn, redacted_data_fn) + 'Missing root-readable %s. Using redacted %s instead.', + sensitive_data_fn, redacted_data_fn + ) instance_data_fn = redacted_data_fn else: instance_data_fn = redacted_data_fn diff --git a/cloudinit/distros/netbsd.py b/cloudinit/distros/netbsd.py index 066737a8..f1a9b182 100644 --- a/cloudinit/distros/netbsd.py +++ b/cloudinit/distros/netbsd.py @@ -100,8 +100,9 @@ class NetBSD(cloudinit.distros.bsd.BSD): else: method = crypt.METHOD_BLOWFISH # pylint: disable=E1101 hashed_pw = crypt.crypt( - passwd, - crypt.mksalt(method)) + passwd, + crypt.mksalt(method) + ) try: subp.subp(['usermod', '-p', hashed_pw, user]) @@ -143,8 +144,9 @@ class NetBSD(cloudinit.distros.bsd.BSD): os_arch = platform.machine() e = os.environ.copy() e['PKG_PATH'] = ( - 'http://cdn.netbsd.org/pub/pkgsrc/' - 'packages/NetBSD/%s/%s/All') % (os_arch, os_release) + 'http://cdn.netbsd.org/pub/pkgsrc/' + 'packages/NetBSD/%s/%s/All' + ) % (os_arch, os_release) return e def update_package_sources(self): diff --git a/cloudinit/distros/openbsd.py b/cloudinit/distros/openbsd.py index 07c76530..720c9cf3 100644 --- a/cloudinit/distros/openbsd.py +++ b/cloudinit/distros/openbsd.py @@ -42,9 +42,10 @@ class Distro(cloudinit.distros.netbsd.NetBSD): os_arch = platform.machine() e = os.environ.copy() e['PKG_PATH'] = ( - 'ftp://ftp.openbsd.org/pub/OpenBSD/{os_release}/' - 'packages/{os_arch}/').format( - os_arch=os_arch, os_release=os_release) + 'ftp://ftp.openbsd.org/pub/OpenBSD/{os_release}/' + 'packages/{os_arch}/').format( + os_arch=os_arch, os_release=os_release + ) return e diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index a57fea0a..b40cb154 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -849,8 +849,9 @@ def get_interfaces_by_mac_on_freebsd(): def get_interfaces_by_mac_on_netbsd(): ret = {} re_field_match = ( - r"(?P\w+).*address:\s" - r"(?P([\da-f]{2}[:-]){5}([\da-f]{2})).*") + r"(?P\w+).*address:\s" + r"(?P([\da-f]{2}[:-]){5}([\da-f]{2})).*" + ) (out, _) = subp.subp(['ifconfig', '-a']) if_lines = re.sub(r'\n\s+', ' ', out).splitlines() for line in if_lines: diff --git a/cloudinit/net/bsd.py b/cloudinit/net/bsd.py index 1c355a98..e34e0454 100644 --- a/cloudinit/net/bsd.py +++ b/cloudinit/net/bsd.py @@ -66,8 +66,9 @@ class BSDRenderer(renderer.Renderer): if subnet.get('type') == 'static': if not subnet.get('netmask'): LOG.debug( - 'Skipping IP %s, because there is no netmask', - subnet.get('address')) + 'Skipping IP %s, because there is no netmask', + subnet.get('address') + ) continue LOG.debug('Configuring dev %s with %s / %s', device_name, subnet.get('address'), subnet.get('netmask')) diff --git a/cloudinit/net/netbsd.py b/cloudinit/net/netbsd.py index 30437b5f..71b38ee6 100644 --- a/cloudinit/net/netbsd.py +++ b/cloudinit/net/netbsd.py @@ -17,8 +17,9 @@ class Renderer(cloudinit.net.bsd.BSDRenderer): if self.dhcp_interfaces(): self.set_rc_config_value('dhcpcd', 'YES') self.set_rc_config_value( - 'dhcpcd_flags', - ' '.join(self.dhcp_interfaces())) + 'dhcpcd_flags', + ' '.join(self.dhcp_interfaces()) + ) for device_name, v in self.interface_configurations.items(): if isinstance(v, dict): self.set_rc_config_value( diff --git a/cloudinit/net/openbsd.py b/cloudinit/net/openbsd.py index 489ea48b..166d77e6 100644 --- a/cloudinit/net/openbsd.py +++ b/cloudinit/net/openbsd.py @@ -19,8 +19,9 @@ class Renderer(cloudinit.net.bsd.BSDRenderer): elif isinstance(v, dict): try: content = "inet {address} {netmask}\n".format( - address=v['address'], - netmask=v['netmask']) + address=v['address'], + netmask=v['netmask'] + ) except KeyError: LOG.error( "Invalid static configuration for %s", diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py index f36c300f..0a5d481d 100644 --- a/cloudinit/net/sysconfig.py +++ b/cloudinit/net/sysconfig.py @@ -505,7 +505,7 @@ class Renderer(renderer.Renderer): iface_cfg['IPADDR6_%d' % ipv6_index] = ipv6_cidr else: iface_cfg['IPV6ADDR_SECONDARIES'] += \ - " " + ipv6_cidr + " " + ipv6_cidr else: ipv4_index = ipv4_index + 1 suff = "" if ipv4_index == 0 else str(ipv4_index) diff --git a/cloudinit/reporting/handlers.py b/cloudinit/reporting/handlers.py index 00e8d2e5..6b9127b6 100755 --- a/cloudinit/reporting/handlers.py +++ b/cloudinit/reporting/handlers.py @@ -139,7 +139,8 @@ class HyperVKvpReportingHandler(ReportingHandler): self.event_key_prefix = u"{0}|{1}".format(self.EVENT_PREFIX, self.incarnation_no) self.publish_thread = threading.Thread( - target=self._publish_event_routine) + target=self._publish_event_routine + ) self.publish_thread.daemon = True self.publish_thread.start() @@ -202,10 +203,15 @@ class HyperVKvpReportingHandler(ReportingHandler): uuid.uuid4()) def _encode_kvp_item(self, key, value): - data = (struct.pack("%ds%ds" % ( + data = struct.pack( + "%ds%ds" + % ( self.HV_KVP_EXCHANGE_MAX_KEY_SIZE, - self.HV_KVP_EXCHANGE_MAX_VALUE_SIZE), - key.encode('utf-8'), value.encode('utf-8'))) + self.HV_KVP_EXCHANGE_MAX_VALUE_SIZE, + ), + key.encode("utf-8"), + value.encode("utf-8"), + ) return data def _decode_kvp_item(self, record_data): diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 89312b9e..6d569057 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -523,8 +523,9 @@ class DataSourceAzure(sources.DataSource): try: crawled_data = util.log_time( - logfunc=LOG.debug, msg='Crawl of metadata service', - func=self.crawl_metadata) + logfunc=LOG.debug, msg='Crawl of metadata service', + func=self.crawl_metadata + ) except sources.InvalidMetaDataException as e: LOG.warning('Could not crawl Azure metadata: %s', e) return False @@ -893,9 +894,10 @@ def can_dev_be_reformatted(devpath, preserve_ntfs): (cand_part, cand_path, devpath)) with events.ReportEventStack( - name="mount-ntfs-and-count", - description="mount-ntfs-and-count", - parent=azure_ds_reporter) as evt: + name="mount-ntfs-and-count", + description="mount-ntfs-and-count", + parent=azure_ds_reporter + ) as evt: try: file_count = util.mount_cb(cand_path, count_files, mtype="ntfs", update_env_for_mount={'LANG': 'C'}) @@ -924,9 +926,10 @@ def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=120, # wait for ephemeral disk to come up naplen = .2 with events.ReportEventStack( - name="wait-for-ephemeral-disk", - description="wait for ephemeral disk", - parent=azure_ds_reporter): + name="wait-for-ephemeral-disk", + description="wait for ephemeral disk", + parent=azure_ds_reporter + ): missing = util.wait_for_files([devpath], maxwait=maxwait, naplen=naplen, @@ -1334,9 +1337,10 @@ def parse_network_config(imds_metadata): @return: Dictionary containing network version 2 standard configuration. """ with events.ReportEventStack( - name="parse_network_config", - description="", - parent=azure_ds_reporter) as evt: + name="parse_network_config", + description="", + parent=azure_ds_reporter + ) as evt: if imds_metadata != sources.UNSET and imds_metadata: netconfig = {'version': 2, 'ethernets': {}} LOG.debug('Azure: generating network configuration from IMDS') @@ -1480,9 +1484,10 @@ def maybe_remove_ubuntu_network_config_scripts(paths=None): def _is_platform_viable(seed_dir): with events.ReportEventStack( - name="check-platform-viability", - description="found azure asset tag", - parent=azure_ds_reporter) as evt: + name="check-platform-viability", + description="found azure asset tag", + parent=azure_ds_reporter + ) as evt: """Check platform environment to report if this datasource may run.""" asset_tag = util.read_dmi_data('chassis-asset-tag') diff --git a/cloudinit/sources/helpers/tests/test_netlink.py b/cloudinit/sources/helpers/tests/test_netlink.py index 58c3adc6..10760bd6 100644 --- a/cloudinit/sources/helpers/tests/test_netlink.py +++ b/cloudinit/sources/helpers/tests/test_netlink.py @@ -180,17 +180,22 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): other_ifname = "eth1" expected_ifname = "eth0" data_op_down_eth1 = self._media_switch_data( - other_ifname, RTM_NEWLINK, OPER_DOWN) + other_ifname, RTM_NEWLINK, OPER_DOWN + ) data_op_up_eth1 = self._media_switch_data( - other_ifname, RTM_NEWLINK, OPER_UP) + other_ifname, RTM_NEWLINK, OPER_UP + ) data_op_down_eth0 = self._media_switch_data( - expected_ifname, RTM_NEWLINK, OPER_DOWN) + expected_ifname, RTM_NEWLINK, OPER_DOWN + ) data_op_up_eth0 = self._media_switch_data( - expected_ifname, RTM_NEWLINK, OPER_UP) - m_read_netlink_socket.side_effect = [data_op_down_eth1, - data_op_up_eth1, - data_op_down_eth0, - data_op_up_eth0] + expected_ifname, RTM_NEWLINK, OPER_UP) + m_read_netlink_socket.side_effect = [ + data_op_down_eth1, + data_op_up_eth1, + data_op_down_eth0, + data_op_up_eth0 + ] wait_for_media_disconnect_connect(m_socket, expected_ifname) self.assertIn('Ignored netlink event on interface %s' % other_ifname, self.logs.getvalue()) @@ -207,17 +212,23 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): ''' ifname = "eth0" data_getlink_down = self._media_switch_data( - ifname, RTM_GETLINK, OPER_DOWN) + ifname, RTM_GETLINK, OPER_DOWN + ) data_getlink_up = self._media_switch_data( - ifname, RTM_GETLINK, OPER_UP) + ifname, RTM_GETLINK, OPER_UP + ) data_newlink_down = self._media_switch_data( - ifname, RTM_NEWLINK, OPER_DOWN) + ifname, RTM_NEWLINK, OPER_DOWN + ) data_newlink_up = self._media_switch_data( - ifname, RTM_NEWLINK, OPER_UP) - m_read_netlink_socket.side_effect = [data_getlink_down, - data_getlink_up, - data_newlink_down, - data_newlink_up] + ifname, RTM_NEWLINK, OPER_UP + ) + m_read_netlink_socket.side_effect = [ + data_getlink_down, + data_getlink_up, + data_newlink_down, + data_newlink_up + ] wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 4) @@ -233,19 +244,25 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): ''' ifname = "eth0" data_setlink_down = self._media_switch_data( - ifname, RTM_SETLINK, OPER_DOWN) + ifname, RTM_SETLINK, OPER_DOWN + ) data_setlink_up = self._media_switch_data( - ifname, RTM_SETLINK, OPER_UP) + ifname, RTM_SETLINK, OPER_UP + ) data_newlink_down = self._media_switch_data( - ifname, RTM_NEWLINK, OPER_DOWN) + ifname, RTM_NEWLINK, OPER_DOWN + ) data_newlink_up = self._media_switch_data( - ifname, RTM_NEWLINK, OPER_UP) - m_read_netlink_socket.side_effect = [data_setlink_down, - data_setlink_up, - data_newlink_down, - data_newlink_up, - data_newlink_down, - data_newlink_up] + ifname, RTM_NEWLINK, OPER_UP + ) + m_read_netlink_socket.side_effect = [ + data_setlink_down, + data_setlink_up, + data_newlink_down, + data_newlink_up, + data_newlink_down, + data_newlink_up + ] wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 4) @@ -255,23 +272,30 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): ifname = "eth0" data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) - data_op_dormant = self._media_switch_data(ifname, RTM_NEWLINK, - OPER_DORMANT) - data_op_notpresent = self._media_switch_data(ifname, RTM_NEWLINK, - OPER_NOTPRESENT) - data_op_lowerdown = self._media_switch_data(ifname, RTM_NEWLINK, - OPER_LOWERLAYERDOWN) - data_op_testing = self._media_switch_data(ifname, RTM_NEWLINK, - OPER_TESTING) - data_op_unknown = self._media_switch_data(ifname, RTM_NEWLINK, - OPER_UNKNOWN) - m_read_netlink_socket.side_effect = [data_op_up, data_op_up, - data_op_dormant, data_op_up, - data_op_notpresent, data_op_up, - data_op_lowerdown, data_op_up, - data_op_testing, data_op_up, - data_op_unknown, data_op_up, - data_op_down, data_op_up] + data_op_dormant = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_DORMANT + ) + data_op_notpresent = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_NOTPRESENT + ) + data_op_lowerdown = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_LOWERLAYERDOWN + ) + data_op_testing = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_TESTING + ) + data_op_unknown = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_UNKNOWN + ) + m_read_netlink_socket.side_effect = [ + data_op_up, data_op_up, + data_op_dormant, data_op_up, + data_op_notpresent, data_op_up, + data_op_lowerdown, data_op_up, + data_op_testing, data_op_up, + data_op_unknown, data_op_up, + data_op_down, data_op_up + ] wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 14) @@ -281,12 +305,14 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): ifname = "eth0" data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) - data_op_dormant = self._media_switch_data(ifname, RTM_NEWLINK, - OPER_DORMANT) - data_op_unknown = self._media_switch_data(ifname, RTM_NEWLINK, - OPER_UNKNOWN) - m_read_netlink_socket.side_effect = [data_op_down, data_op_dormant, - data_op_unknown, data_op_up] + data_op_dormant = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_DORMANT) + data_op_unknown = self._media_switch_data( + ifname, RTM_NEWLINK, OPER_UNKNOWN) + m_read_netlink_socket.side_effect = [ + data_op_down, data_op_dormant, + data_op_unknown, data_op_up + ] wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 4) @@ -300,9 +326,11 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) data_op_invalid = self._media_switch_data(ifname, RTM_NEWLINK, 7) - m_read_netlink_socket.side_effect = [data_op_invalid, data_op_up, - data_op_down, data_op_invalid, - data_op_up] + m_read_netlink_socket.side_effect = [ + data_op_invalid, data_op_up, + data_op_down, data_op_invalid, + data_op_up + ] wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 5) @@ -333,8 +361,9 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): data_invalid2 = self._media_switch_data(ifname, RTM_NEWLINK, None) data_op_down = self._media_switch_data(ifname, RTM_NEWLINK, OPER_DOWN) data_op_up = self._media_switch_data(ifname, RTM_NEWLINK, OPER_UP) - m_read_netlink_socket.side_effect = [data_invalid1, data_invalid2, - data_op_down, data_op_up] + m_read_netlink_socket.side_effect = [ + data_invalid1, data_invalid2, data_op_down, data_op_up + ] wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 4) @@ -344,11 +373,15 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): bytes = ifname.encode("utf-8") data = bytearray(96) struct.pack_into("=LHHLL", data, 0, 48, RTM_NEWLINK, 0, 0, 0) - struct.pack_into("HH4sHHc", data, RTATTR_START_OFFSET, 8, 3, - bytes, 5, 16, int_to_bytes(OPER_DOWN)) + struct.pack_into( + "HH4sHHc", data, RTATTR_START_OFFSET, 8, 3, + bytes, 5, 16, int_to_bytes(OPER_DOWN) + ) struct.pack_into("=LHHLL", data, 48, 48, RTM_NEWLINK, 0, 0, 0) - struct.pack_into("HH4sHHc", data, 48 + RTATTR_START_OFFSET, 8, - 3, bytes, 5, 16, int_to_bytes(OPER_UP)) + struct.pack_into( + "HH4sHHc", data, 48 + RTATTR_START_OFFSET, 8, + 3, bytes, 5, 16, int_to_bytes(OPER_UP) + ) m_read_netlink_socket.return_value = data wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 1) @@ -360,14 +393,18 @@ class TestWaitForMediaDisconnectConnect(CiTestCase): data1 = bytearray(112) data2 = bytearray(32) struct.pack_into("=LHHLL", data1, 0, 48, RTM_NEWLINK, 0, 0, 0) - struct.pack_into("HH4sHHc", data1, RTATTR_START_OFFSET, 8, 3, - bytes, 5, 16, int_to_bytes(OPER_DOWN)) + struct.pack_into( + "HH4sHHc", data1, RTATTR_START_OFFSET, 8, 3, + bytes, 5, 16, int_to_bytes(OPER_DOWN) + ) struct.pack_into("=LHHLL", data1, 48, 48, RTM_NEWLINK, 0, 0, 0) - struct.pack_into("HH4sHHc", data1, 80, 8, 3, bytes, 5, 16, - int_to_bytes(OPER_DOWN)) + struct.pack_into( + "HH4sHHc", data1, 80, 8, 3, bytes, 5, 16, int_to_bytes(OPER_DOWN) + ) struct.pack_into("=LHHLL", data1, 96, 48, RTM_NEWLINK, 0, 0, 0) - struct.pack_into("HH4sHHc", data2, 16, 8, 3, bytes, 5, 16, - int_to_bytes(OPER_UP)) + struct.pack_into( + "HH4sHHc", data2, 16, 8, 3, bytes, 5, 16, int_to_bytes(OPER_UP) + ) m_read_netlink_socket.side_effect = [data1, data2] wait_for_media_disconnect_connect(m_socket, ifname) self.assertEqual(m_read_netlink_socket.call_count, 2) diff --git a/cloudinit/sources/tests/test_init.py b/cloudinit/sources/tests/test_init.py index 5b6f1b3f..1420a988 100644 --- a/cloudinit/sources/tests/test_init.py +++ b/cloudinit/sources/tests/test_init.py @@ -436,10 +436,11 @@ class TestDataSource(CiTestCase): expected = { 'base64_encoded_keys': [], 'merged_cfg': { - '_doc': ( - 'Merged cloud-init system config from ' - '/etc/cloud/cloud.cfg and /etc/cloud/cloud.cfg.d/'), - 'datasource': {'_undef': {'key1': False}}}, + '_doc': ( + 'Merged cloud-init system config from ' + '/etc/cloud/cloud.cfg and /etc/cloud/cloud.cfg.d/' + ), + 'datasource': {'_undef': {'key1': False}}}, 'sensitive_keys': [ 'ds/meta_data/some/security-credentials', 'merged_cfg'], 'sys_info': sys_info, diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index 68d59111..2e7c6686 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -321,7 +321,7 @@ class CloudTestCase(unittest.TestCase): "Unexpected sys_info dist value") self.assertEqual(self.os_name, v1_data['distro_release']) self.assertEqual( - str(self.os_cfg['version']), v1_data['distro_version']) + str(self.os_cfg['version']), v1_data['distro_version']) self.assertEqual('x86_64', v1_data['machine']) self.assertIsNotNone( re.match(r'3.\d\.\d', v1_data['python_version']), diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 05552a1e..a99cbd41 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -683,15 +683,17 @@ scbus-1 on xpt0 bus 0 'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready') @mock.patch('cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds') def test_crawl_metadata_on_reprovision_reports_ready( - self, poll_imds_func, - report_ready_func, - m_write, m_dhcp): + self, poll_imds_func, report_ready_func, m_write, m_dhcp + ): """If reprovisioning, report ready at the end""" ovfenv = construct_valid_ovf_env( - platform_settings={"PreprovisionedVm": "True"}) + platform_settings={"PreprovisionedVm": "True"} + ) - data = {'ovfcontent': ovfenv, - 'sys_cfg': {}} + data = { + 'ovfcontent': ovfenv, + 'sys_cfg': {} + } dsrc = self._get_ds(data) poll_imds_func.return_value = ovfenv dsrc.crawl_metadata() @@ -706,15 +708,18 @@ scbus-1 on xpt0 bus 0 @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') @mock.patch('cloudinit.sources.DataSourceAzure.readurl') def test_crawl_metadata_on_reprovision_reports_ready_using_lease( - self, m_readurl, m_dhcp, - m_net, report_ready_func, - m_media_switch, m_write): + self, m_readurl, m_dhcp, m_net, report_ready_func, + m_media_switch, m_write + ): """If reprovisioning, report ready using the obtained lease""" ovfenv = construct_valid_ovf_env( - platform_settings={"PreprovisionedVm": "True"}) + platform_settings={"PreprovisionedVm": "True"} + ) - data = {'ovfcontent': ovfenv, - 'sys_cfg': {}} + data = { + 'ovfcontent': ovfenv, + 'sys_cfg': {} + } dsrc = self._get_ds(data) lease = { @@ -1955,8 +1960,8 @@ class TestPreprovisioningPollIMDS(CiTestCase): response = requests.Response() response.status_code = 404 if self.tries == 2 else 410 raise requests.exceptions.HTTPError( - "fake {}".format(response.status_code), - response=response) + "fake {}".format(response.status_code), response=response + ) # Third try should succeed and stop retries or redhcp return mock.MagicMock(status_code=200, text="good", content="good") diff --git a/tests/unittests/test_datasource/test_scaleway.py b/tests/unittests/test_datasource/test_scaleway.py index 15441454..9d82bda9 100644 --- a/tests/unittests/test_datasource/test_scaleway.py +++ b/tests/unittests/test_datasource/test_scaleway.py @@ -353,12 +353,16 @@ class TestDataSourceScaleway(HttprettyTestCase): self.datasource.metadata['ipv6'] = None netcfg = self.datasource.network_config - resp = {'version': 1, - 'config': [{ - 'type': 'physical', - 'name': 'ens2', - 'subnets': [{'type': 'dhcp4'}]}] + resp = { + 'version': 1, + 'config': [ + { + 'type': 'physical', + 'name': 'ens2', + 'subnets': [{'type': 'dhcp4'}] } + ] + } self.assertEqual(netcfg, resp) @mock.patch('cloudinit.sources.DataSourceScaleway.net.find_fallback_nic') @@ -424,12 +428,16 @@ class TestDataSourceScaleway(HttprettyTestCase): self.datasource.metadata['ipv6'] = None self.datasource._network_config = sources.UNSET - resp = {'version': 1, - 'config': [{ - 'type': 'physical', - 'name': 'ens2', - 'subnets': [{'type': 'dhcp4'}]}] + resp = { + 'version': 1, + 'config': [ + { + 'type': 'physical', + 'name': 'ens2', + 'subnets': [{'type': 'dhcp4'}] } + ] + } netcfg = self.datasource.network_config self.assertEqual(netcfg, resp) @@ -448,12 +456,16 @@ class TestDataSourceScaleway(HttprettyTestCase): self.datasource.metadata['ipv6'] = None self.datasource._network_config = None - resp = {'version': 1, - 'config': [{ - 'type': 'physical', - 'name': 'ens2', - 'subnets': [{'type': 'dhcp4'}]}] + resp = { + 'version': 1, + 'config': [ + { + 'type': 'physical', + 'name': 'ens2', + 'subnets': [{'type': 'dhcp4'}] } + ] + } netcfg = self.datasource.network_config self.assertEqual(netcfg, resp) diff --git a/tests/unittests/test_distros/test_bsd_utils.py b/tests/unittests/test_distros/test_bsd_utils.py index b38e4af5..3a68f2a9 100644 --- a/tests/unittests/test_distros/test_bsd_utils.py +++ b/tests/unittests/test_distros/test_bsd_utils.py @@ -62,5 +62,6 @@ class TestBsdUtils(CiTestCase): self.load_file.return_value = RC_FILE.format(hostname='foo') bsd_utils.set_rc_config_value('hostname', 'bar') self.write_file.assert_called_with( - '/etc/rc.conf', - RC_FILE.format(hostname='bar')) + '/etc/rc.conf', + RC_FILE.format(hostname='bar') + ) diff --git a/tests/unittests/test_handler/test_handler_mounts.py b/tests/unittests/test_handler/test_handler_mounts.py index 80c53c83..b643e3ae 100644 --- a/tests/unittests/test_handler/test_handler_mounts.py +++ b/tests/unittests/test_handler/test_handler_mounts.py @@ -260,8 +260,11 @@ class TestFstabHandling(test_helpers.FilesystemMockingTestCase): '/dev/vdb /mnt auto defaults,noexec,comment=cloudconfig 0 2\n' ) fstab_expected_content = fstab_original_content - cc = {'mounts': [ - ['/dev/vdb', '/mnt', 'auto', 'defaults,noexec']]} + cc = { + 'mounts': [ + ['/dev/vdb', '/mnt', 'auto', 'defaults,noexec'] + ] + } with open(cc_mounts.FSTAB_PATH, 'w') as fd: fd.write(fstab_original_content) with open(cc_mounts.FSTAB_PATH, 'r') as fd: diff --git a/tests/unittests/test_reporting_hyperv.py b/tests/unittests/test_reporting_hyperv.py index fa8f8859..b60a66ab 100644 --- a/tests/unittests/test_reporting_hyperv.py +++ b/tests/unittests/test_reporting_hyperv.py @@ -93,10 +93,15 @@ class TextKvpReporter(CiTestCase): def test_not_truncate_kvp_file_modified_after_boot(self): with open(self.tmp_file_path, "wb+") as f: kvp = {'key': 'key1', 'value': 'value1'} - data = (struct.pack("%ds%ds" % ( + data = struct.pack( + "%ds%ds" + % ( HyperVKvpReportingHandler.HV_KVP_EXCHANGE_MAX_KEY_SIZE, - HyperVKvpReportingHandler.HV_KVP_EXCHANGE_MAX_VALUE_SIZE), - kvp['key'].encode('utf-8'), kvp['value'].encode('utf-8'))) + HyperVKvpReportingHandler.HV_KVP_EXCHANGE_MAX_VALUE_SIZE, + ), + kvp["key"].encode("utf-8"), + kvp["value"].encode("utf-8"), + ) f.write(data) cur_time = time.time() os.utime(self.tmp_file_path, (cur_time, cur_time)) diff --git a/tests/unittests/test_sshutil.py b/tests/unittests/test_sshutil.py index b4767f0c..d15fc60b 100644 --- a/tests/unittests/test_sshutil.py +++ b/tests/unittests/test_sshutil.py @@ -374,13 +374,13 @@ class TestMultipleSshAuthorizedKeysFile(test_helpers.CiTestCase): sshd_config = self.tmp_path('sshd_config') util.write_file( - sshd_config, - "AuthorizedKeysFile %s %s" % (authorized_keys, user_keys)) + sshd_config, + "AuthorizedKeysFile %s %s" % (authorized_keys, user_keys) + ) (auth_key_fn, auth_key_entries) = ssh_util.extract_authorized_keys( - fpw.pw_name, sshd_config) - content = ssh_util.update_authorized_keys( - auth_key_entries, []) + fpw.pw_name, sshd_config) + content = ssh_util.update_authorized_keys(auth_key_entries, []) self.assertEqual("%s/.ssh/authorized_keys" % fpw.pw_dir, auth_key_fn) self.assertTrue(VALID_CONTENT['rsa'] in content) @@ -398,11 +398,13 @@ class TestMultipleSshAuthorizedKeysFile(test_helpers.CiTestCase): sshd_config = self.tmp_path('sshd_config') util.write_file( - sshd_config, - "AuthorizedKeysFile %s %s" % (authorized_keys, user_keys)) + sshd_config, + "AuthorizedKeysFile %s %s" % (authorized_keys, user_keys) + ) (auth_key_fn, auth_key_entries) = ssh_util.extract_authorized_keys( - fpw.pw_name, sshd_config) + fpw.pw_name, sshd_config + ) content = ssh_util.update_authorized_keys(auth_key_entries, []) self.assertEqual("%s/.ssh/authorized_keys" % fpw.pw_dir, auth_key_fn) diff --git a/tox.ini b/tox.ini index e04c7791..ebcebc41 100644 --- a/tox.ini +++ b/tox.ini @@ -43,11 +43,10 @@ basepython = python2.7 deps = -r{toxinidir}/test-requirements.txt [flake8] -# E126: continuation line over-indented for hanging indent # E226: missing whitespace around arithmetic operator # W503: line break before binary operator # W504: line break after binary operator -ignore=E126,E226,W503,W504 +ignore=E226,W503,W504 exclude = .venv,.tox,dist,doc,*egg,.git,build,tools per-file-ignores = cloudinit/cmd/main.py:E402 -- cgit v1.2.3 From a6bb375aef93a31395af9ce0985c49ada9fb7139 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Mon, 10 Aug 2020 15:11:23 -0400 Subject: DataSourceOracle: refactor to use only OPC v1 endpoint (#493) The /opc/v1/ metadata endpoints[0] are universally available in Oracle Cloud Infrastructure and the OpenStack endpoints are considered deprecated, so we can refactor the data source to use the OPC endpoints exclusively. This simplifies the datasource code substantially, and enables use of OPC-specific attributes in future. [0] https://docs.cloud.oracle.com/en-us/iaas/Content/Compute/Tasks/gettingmetadata.htm --- cloudinit/sources/DataSourceOracle.py | 178 +++----- cloudinit/sources/tests/test_oracle.py | 776 ++++++++++++++++----------------- conftest.py | 116 ++++- tox.ini | 1 + 4 files changed, 538 insertions(+), 533 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 90e1881a..f113d364 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -1,22 +1,20 @@ # This file is part of cloud-init. See LICENSE file for license information. """Datasource for Oracle (OCI/Oracle Cloud Infrastructure) -OCI provides a OpenStack like metadata service which provides only -'2013-10-17' and 'latest' versions.. - Notes: - * This datasource does not support the OCI-Classic. OCI-Classic - provides an EC2 lookalike metadata service. - * The uuid provided in DMI data is not the same as the meta-data provided + * This datasource does not support OCI Classic. OCI Classic provides an EC2 + lookalike metadata service. + * The UUID provided in DMI data is not the same as the meta-data provided instance-id, but has an equivalent lifespan. * We do need to support upgrade from an instance that cloud-init identified as OpenStack. - * Both bare-metal and vms use iscsi root - * Both bare-metal and vms provide chassis-asset-tag of OracleCloud.com + * Bare metal instances use iSCSI root, virtual machine instances do not. + * Both bare metal and virtual machine instances provide a chassis-asset-tag of + OracleCloud.com. """ +import base64 import json -import re from cloudinit import log as logging from cloudinit import net, sources, util @@ -26,7 +24,7 @@ from cloudinit.net import ( get_interfaces_by_mac, is_netfail_master, ) -from cloudinit.url_helper import UrlError, combine_url, readurl +from cloudinit.url_helper import readurl LOG = logging.getLogger(__name__) @@ -35,8 +33,9 @@ BUILTIN_DS_CONFIG = { 'configure_secondary_nics': False, } CHASSIS_ASSET_TAG = "OracleCloud.com" -METADATA_ENDPOINT = "http://169.254.169.254/openstack/" -VNIC_METADATA_URL = 'http://169.254.169.254/opc/v1/vnics/' +METADATA_ROOT = "http://169.254.169.254/opc/v1/" +METADATA_ENDPOINT = METADATA_ROOT + "instance/" +VNIC_METADATA_URL = METADATA_ROOT + "vnics/" # https://docs.cloud.oracle.com/iaas/Content/Network/Troubleshoot/connectionhang.htm#Overview, # indicates that an MTU of 9000 is used within OCI MTU = 9000 @@ -189,53 +188,39 @@ class DataSourceOracle(sources.DataSource): if not self._is_platform_viable(): return False + self.system_uuid = _read_system_uuid() + # network may be configured if iscsi root. If that is the case # then read_initramfs_config will return non-None. if _is_iscsi_root(): - data = self.crawl_metadata() + data = read_opc_metadata() else: with dhcp.EphemeralDHCPv4(net.find_fallback_nic()): - data = self.crawl_metadata() + data = read_opc_metadata() self._crawled_metadata = data - vdata = data['2013-10-17'] - - self.userdata_raw = vdata.get('user_data') - self.system_uuid = vdata['system_uuid'] - - vd = vdata.get('vendor_data') - if vd: - self.vendordata_pure = vd - try: - self.vendordata_raw = sources.convert_vendordata(vd) - except ValueError as e: - LOG.warning("Invalid content in vendor-data: %s", e) - self.vendordata_raw = None - - mdcopies = ('public_keys',) - md = dict([(k, vdata['meta_data'].get(k)) - for k in mdcopies if k in vdata['meta_data']]) - - mdtrans = ( - # oracle meta_data.json name, cloudinit.datasource.metadata name - ('availability_zone', 'availability-zone'), - ('hostname', 'local-hostname'), - ('launch_index', 'launch-index'), - ('uuid', 'instance-id'), - ) - for dsname, ciname in mdtrans: - if dsname in vdata['meta_data']: - md[ciname] = vdata['meta_data'][dsname] - self.metadata = md - return True + self.metadata = { + "availability-zone": data["ociAdName"], + "instance-id": data["id"], + "launch-index": 0, + "local-hostname": data["hostname"], + "name": data["displayName"], + } + + if "metadata" in data: + user_data = data["metadata"].get("user_data") + if user_data: + self.userdata_raw = base64.b64decode(user_data) + self.metadata["public_keys"] = data["metadata"].get( + "ssh_authorized_keys" + ) - def crawl_metadata(self): - return read_metadata() + return True def _get_subplatform(self): """Return the subplatform metadata source details.""" - return 'metadata (%s)' % METADATA_ENDPOINT + return "metadata ({})".format(METADATA_ROOT) def check_instance_id(self, sys_cfg): """quickly check (local only) if self.instance_id is still valid @@ -292,72 +277,15 @@ def _is_iscsi_root(): return bool(cmdline.read_initramfs_config()) -def _load_index(content): - """Return a list entries parsed from content. - - OpenStack's metadata service returns a newline delimited list - of items. Oracle's implementation has html formatted list of links. - The parser here just grabs targets from - and throws away "../". - - Oracle has accepted that to be buggy and may fix in the future - to instead return a '\n' delimited plain text list. This function - will continue to work if that change is made.""" - if not content.lower().startswith(""): - return content.splitlines() - items = re.findall( - r'href="(?P[^"]*)"', content, re.MULTILINE | re.IGNORECASE) - return [i for i in items if not i.startswith(".")] - - -def read_metadata(endpoint_base=METADATA_ENDPOINT, sys_uuid=None, - version='2013-10-17'): - """Read metadata, return a dictionary. - - Each path listed in the index will be represented in the dictionary. - If the path ends in .json, then the content will be decoded and - populated into the dictionary. - - The system uuid (/sys/class/dmi/id/product_uuid) is also populated. - Example: given paths = ('user_data', 'meta_data.json') - This would return: - {version: {'user_data': b'blob', 'meta_data': json.loads(blob.decode()) - 'system_uuid': '3b54f2e0-3ab2-458d-b770-af9926eee3b2'}} +def read_opc_metadata(): """ - endpoint = combine_url(endpoint_base, version) + "/" - if sys_uuid is None: - sys_uuid = _read_system_uuid() - if not sys_uuid: - raise sources.BrokenMetadata("Failed to read system uuid.") - - try: - resp = readurl(endpoint) - if not resp.ok(): - raise sources.BrokenMetadata( - "Bad response from %s: %s" % (endpoint, resp.code)) - except UrlError as e: - raise sources.BrokenMetadata( - "Failed to read index at %s: %s" % (endpoint, e)) - - entries = _load_index(resp.contents.decode('utf-8')) - LOG.debug("index url %s contained: %s", endpoint, entries) - - # meta_data.json is required. - mdj = 'meta_data.json' - if mdj not in entries: - raise sources.BrokenMetadata( - "Required field '%s' missing in index at %s" % (mdj, endpoint)) - - ret = {'system_uuid': sys_uuid} - for path in entries: - response = readurl(combine_url(endpoint, path)) - if path.endswith(".json"): - ret[path.rpartition(".")[0]] = ( - json.loads(response.contents.decode('utf-8'))) - else: - ret[path] = response.contents + Fetch metadata from the /opc/ routes. - return {version: ret} + :return: + The JSON-decoded value of the /opc/v1/instance/ endpoint on the IMDS. + """ + # retries=1 as requested by Oracle to address a potential race condition + return json.loads(readurl(METADATA_ENDPOINT, retries=1)._response.text) # Used to match classes to dependencies @@ -373,17 +301,21 @@ def get_datasource_list(depends): if __name__ == "__main__": import argparse - import os - - parser = argparse.ArgumentParser(description='Query Oracle Cloud Metadata') - parser.add_argument("--endpoint", metavar="URL", - help="The url of the metadata service.", - default=METADATA_ENDPOINT) - args = parser.parse_args() - sys_uuid = "uuid-not-available-not-root" if os.geteuid() != 0 else None - - data = read_metadata(endpoint_base=args.endpoint, sys_uuid=sys_uuid) - data['is_platform_viable'] = _is_platform_viable() - print(util.json_dumps(data)) + + description = """ + Query Oracle Cloud metadata and emit a JSON object with two keys: + `read_opc_metadata` and `_is_platform_viable`. The values of each are + the return values of the corresponding functions defined in + DataSourceOracle.py.""" + parser = argparse.ArgumentParser(description=description) + parser.parse_args() + print( + util.json_dumps( + { + "read_opc_metadata": read_opc_metadata(), + "_is_platform_viable": _is_platform_viable(), + } + ) + ) # vi: ts=4 expandtab diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 2265327b..9ee6e7fa 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -1,23 +1,19 @@ # This file is part of cloud-init. See LICENSE file for license information. -import argparse +import base64 import copy import json -import os -import uuid -from textwrap import dedent +from contextlib import ExitStack from unittest import mock -import httpretty +import pytest -from cloudinit import helpers -from cloudinit.sources import BrokenMetadata from cloudinit.sources import DataSourceOracle as oracle from cloudinit.sources import NetworkConfigSource from cloudinit.tests import helpers as test_helpers +from cloudinit.url_helper import UrlError DS_PATH = "cloudinit.sources.DataSourceOracle" -MD_VER = "2013-10-17" # `curl -L http://169.254.169.254/opc/v1/vnics/` on a Oracle Bare Metal Machine # with a secondary VNIC attached (vnicId truncated for Python line length) @@ -60,330 +56,80 @@ OPC_VM_SECONDARY_VNIC_RESPONSE = """\ } ]""" -class TestDataSourceOracle(test_helpers.CiTestCase): - """Test datasource DataSourceOracle.""" - - with_logs = True - - ds_class = oracle.DataSourceOracle - - my_uuid = str(uuid.uuid4()) - my_md = {"uuid": "ocid1.instance.oc1.phx.abyhqlj", - "name": "ci-vm1", "availability_zone": "phx-ad-3", - "hostname": "ci-vm1hostname", - "launch_index": 0, "files": [], - "public_keys": {"0": "ssh-rsa AAAAB3N...== user@host"}, - "meta": {}} - - def _patch_instance(self, inst, patches): - """Patch an instance of a class 'inst'. - for each name, kwargs in patches: - inst.name = mock.Mock(**kwargs) - returns a namespace object that has - namespace.name = mock.Mock(**kwargs) - Do not bother with cleanup as instance is assumed transient.""" - mocks = argparse.Namespace() - for name, kwargs in patches.items(): - imock = mock.Mock(name=name, spec=getattr(inst, name), **kwargs) - setattr(mocks, name, imock) - setattr(inst, name, imock) - return mocks - - def _get_ds(self, sys_cfg=None, distro=None, paths=None, ud_proc=None, - patches=None): - if sys_cfg is None: - sys_cfg = {} - if patches is None: - patches = {} - if paths is None: - tmpd = self.tmp_dir() - dirs = {'cloud_dir': self.tmp_path('cloud_dir', tmpd), - 'run_dir': self.tmp_path('run_dir')} - for d in dirs.values(): - os.mkdir(d) - paths = helpers.Paths(dirs) - - ds = self.ds_class(sys_cfg=sys_cfg, distro=distro, - paths=paths, ud_proc=ud_proc) - - return ds, self._patch_instance(ds, patches) - - def test_platform_not_viable_returns_false(self): - ds, mocks = self._get_ds( - patches={'_is_platform_viable': {'return_value': False}}) - self.assertFalse(ds._get_data()) - mocks._is_platform_viable.assert_called_once_with() - - def test_platform_info(self): - """Return platform-related information for Oracle Datasource.""" - ds, _mocks = self._get_ds() - self.assertEqual('oracle', ds.cloud_name) - self.assertEqual('oracle', ds.platform_type) - self.assertEqual( - 'metadata (http://169.254.169.254/openstack/)', ds.subplatform) - - def test_sys_cfg_can_enable_configure_secondary_nics(self): - # Confirm that behaviour is toggled by sys_cfg - ds, _mocks = self._get_ds() - self.assertFalse(ds.ds_cfg['configure_secondary_nics']) - - sys_cfg = { - 'datasource': {'Oracle': {'configure_secondary_nics': True}}} - ds, _mocks = self._get_ds(sys_cfg=sys_cfg) - self.assertTrue(ds.ds_cfg['configure_secondary_nics']) - - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_without_userdata(self, m_is_iscsi_root): - """If no user-data is provided, it should not be in return dict.""" - ds, mocks = self._get_ds(patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md}}}}) - self.assertTrue(ds._get_data()) - mocks._is_platform_viable.assert_called_once_with() - mocks.crawl_metadata.assert_called_once_with() - self.assertEqual(self.my_uuid, ds.system_uuid) - self.assertEqual(self.my_md['availability_zone'], ds.availability_zone) - self.assertIn(self.my_md["public_keys"]["0"], ds.get_public_ssh_keys()) - self.assertEqual(self.my_md['uuid'], ds.get_instance_id()) - self.assertIsNone(ds.userdata_raw) - - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_with_vendordata(self, m_is_iscsi_root): - """Test with vendor data.""" - vd = {'cloud-init': '#cloud-config\nkey: value'} - ds, mocks = self._get_ds(patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md, - 'vendor_data': vd}}}}) - self.assertTrue(ds._get_data()) - mocks._is_platform_viable.assert_called_once_with() - mocks.crawl_metadata.assert_called_once_with() - self.assertEqual(vd, ds.vendordata_pure) - self.assertEqual(vd['cloud-init'], ds.vendordata_raw) - - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_with_userdata(self, m_is_iscsi_root): - """Ensure user-data is populated if present and is binary.""" - my_userdata = b'abcdefg' - ds, mocks = self._get_ds(patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md, - 'user_data': my_userdata}}}}) - self.assertTrue(ds._get_data()) - mocks._is_platform_viable.assert_called_once_with() - mocks.crawl_metadata.assert_called_once_with() - self.assertEqual(self.my_uuid, ds.system_uuid) - self.assertIn(self.my_md["public_keys"]["0"], ds.get_public_ssh_keys()) - self.assertEqual(self.my_md['uuid'], ds.get_instance_id()) - self.assertEqual(my_userdata, ds.userdata_raw) - - @mock.patch(DS_PATH + ".get_interfaces_by_mac", mock.Mock(return_value={})) - @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", - side_effect=lambda network_config: network_config) - @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_network_cmdline(self, m_is_iscsi_root, m_initramfs_config, - _m_add_network_config_from_opc_imds): - """network_config should read kernel cmdline.""" - distro = mock.MagicMock() - ds, _ = self._get_ds(distro=distro, patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md}}}}) - ncfg = {'version': 1, 'config': [{'a': 'b'}]} - m_initramfs_config.return_value = ncfg - self.assertTrue(ds._get_data()) - self.assertEqual(ncfg, ds.network_config) - self.assertEqual([mock.call()], m_initramfs_config.call_args_list) - self.assertFalse(distro.generate_fallback_config.called) - - @mock.patch(DS_PATH + ".get_interfaces_by_mac", mock.Mock(return_value={})) - @mock.patch(DS_PATH + "._add_network_config_from_opc_imds", - side_effect=lambda network_config: network_config) - @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_network_fallback(self, m_is_iscsi_root, m_initramfs_config, - _m_add_network_config_from_opc_imds): - """test that fallback network is generated if no kernel cmdline.""" - distro = mock.MagicMock() - ds, _ = self._get_ds(distro=distro, patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md}}}}) - ncfg = {'version': 1, 'config': [{'a': 'b'}]} - m_initramfs_config.return_value = None - self.assertTrue(ds._get_data()) - ncfg = {'version': 1, 'config': [{'distro1': 'value'}]} - distro.generate_fallback_config.return_value = ncfg - self.assertEqual(ncfg, ds.network_config) - self.assertEqual([mock.call()], m_initramfs_config.call_args_list) - distro.generate_fallback_config.assert_called_once_with() - - # test that the result got cached, and the methods not re-called. - self.assertEqual(ncfg, ds.network_config) - self.assertEqual(1, m_initramfs_config.call_count) - - @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") - @mock.patch(DS_PATH + ".cmdline.read_initramfs_config", - return_value={'some': 'config'}) - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_secondary_nics_added_to_network_config_if_enabled( - self, _m_is_iscsi_root, _m_initramfs_config, - m_add_network_config_from_opc_imds): - - needle = object() - - def network_config_side_effect(network_config): - network_config['secondary_added'] = needle - - m_add_network_config_from_opc_imds.side_effect = ( - network_config_side_effect) - - distro = mock.MagicMock() - ds, _ = self._get_ds(distro=distro, patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md}}}}) - ds.ds_cfg['configure_secondary_nics'] = True - self.assertEqual(needle, ds.network_config['secondary_added']) - - @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") - @mock.patch(DS_PATH + ".cmdline.read_initramfs_config", - return_value={'some': 'config'}) - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_secondary_nics_not_added_to_network_config_by_default( - self, _m_is_iscsi_root, _m_initramfs_config, - m_add_network_config_from_opc_imds): - - def network_config_side_effect(network_config): - network_config['secondary_added'] = True - - m_add_network_config_from_opc_imds.side_effect = ( - network_config_side_effect) - - distro = mock.MagicMock() - ds, _ = self._get_ds(distro=distro, patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md}}}}) - self.assertNotIn('secondary_added', ds.network_config) - - @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") - @mock.patch(DS_PATH + ".cmdline.read_initramfs_config") - @mock.patch(DS_PATH + "._is_iscsi_root", return_value=True) - def test_secondary_nic_failure_isnt_blocking( - self, _m_is_iscsi_root, m_initramfs_config, - m_add_network_config_from_opc_imds): - - m_add_network_config_from_opc_imds.side_effect = Exception() - - distro = mock.MagicMock() - ds, _ = self._get_ds(distro=distro, patches={ - '_is_platform_viable': {'return_value': True}, - 'crawl_metadata': { - 'return_value': { - MD_VER: {'system_uuid': self.my_uuid, - 'meta_data': self.my_md}}}}) - ds.ds_cfg['configure_secondary_nics'] = True - self.assertEqual(ds.network_config, m_initramfs_config.return_value) - self.assertIn('Failed to fetch secondary network configuration', - self.logs.getvalue()) - - def test_ds_network_cfg_preferred_over_initramfs(self): - """Ensure that DS net config is preferred over initramfs config""" - network_config_sources = oracle.DataSourceOracle.network_config_sources - self.assertLess( - network_config_sources.index(NetworkConfigSource.ds), - network_config_sources.index(NetworkConfigSource.initramfs) +# Fetched with `curl http://169.254.169.254/opc/v1/instance/` (and then +# truncated for line length) +OPC_V1_METADATA = """\ +{ + "availabilityDomain" : "qIZq:PHX-AD-1", + "faultDomain" : "FAULT-DOMAIN-2", + "compartmentId" : "ocid1.tenancy.oc1..aaaaaaaao7f7cccogqrg5emjxkxmTRUNCATED", + "displayName" : "instance-20200320-1400", + "hostname" : "instance-20200320-1400", + "id" : "ocid1.instance.oc1.phx.anyhqljtniwq6syc3nex55sep5w34qbwmw6TRUNCATED", + "image" : "ocid1.image.oc1.phx.aaaaaaaagmkn4gdhvvx24kiahh2b2qchsicTRUNCATED", + "metadata" : { + "ssh_authorized_keys" : "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQ truncated", + "user_data" : "IyEvYmluL3NoCnRvdWNoIC90bXAvZm9v" + }, + "region" : "phx", + "canonicalRegionName" : "us-phoenix-1", + "ociAdName" : "phx-ad-3", + "shape" : "VM.Standard2.1", + "state" : "Running", + "timeCreated" : 1584727285318, + "agentConfig" : { + "monitoringDisabled" : true, + "managementDisabled" : true + } +}""" + + +@pytest.yield_fixture +def oracle_ds(request, fixture_utils, paths): + """ + Return an instantiated DataSourceOracle. + + This also performs the mocking required for the default test case: + * ``_read_system_uuid`` returns something, + * ``_is_platform_viable`` returns True, + * ``_is_iscsi_root`` returns True (the simpler code path), + * ``read_opc_metadata`` returns ``OPC_V1_METADATA`` + + (This uses the paths fixture for the required helpers.Paths object, and the + fixture_utils fixture for fetching markers.) + """ + sys_cfg = fixture_utils.closest_marker_first_arg_or( + request, "ds_sys_cfg", mock.MagicMock() + ) + with mock.patch(DS_PATH + "._read_system_uuid", return_value="someuuid"): + with mock.patch(DS_PATH + "._is_platform_viable", return_value=True): + with mock.patch(DS_PATH + "._is_iscsi_root", return_value=True): + with mock.patch( + DS_PATH + ".read_opc_metadata", + return_value=json.loads(OPC_V1_METADATA), + ): + yield oracle.DataSourceOracle( + sys_cfg=sys_cfg, distro=mock.Mock(), paths=paths, + ) + + +class TestDataSourceOracle: + def test_platform_info(self, oracle_ds): + assert "oracle" == oracle_ds.cloud_name + assert "oracle" == oracle_ds.platform_type + assert ( + "metadata (http://169.254.169.254/opc/v1/)" + == oracle_ds.subplatform ) + def test_secondary_nics_disabled_by_default(self, oracle_ds): + assert not oracle_ds.ds_cfg["configure_secondary_nics"] -@mock.patch(DS_PATH + "._read_system_uuid", return_value=str(uuid.uuid4())) -class TestReadMetaData(test_helpers.HttprettyTestCase): - """Test the read_metadata which interacts with http metadata service.""" - - mdurl = oracle.METADATA_ENDPOINT - my_md = {"uuid": "ocid1.instance.oc1.phx.abyhqlj", - "name": "ci-vm1", "availability_zone": "phx-ad-3", - "hostname": "ci-vm1hostname", - "launch_index": 0, "files": [], - "public_keys": {"0": "ssh-rsa AAAAB3N...== user@host"}, - "meta": {}} - - def populate_md(self, data): - """call httppretty.register_url for each item dict 'data', - including valid indexes. Text values converted to bytes.""" - httpretty.register_uri( - httpretty.GET, self.mdurl + MD_VER + "/", - '\n'.join(data.keys()).encode('utf-8')) - for k, v in data.items(): - httpretty.register_uri( - httpretty.GET, self.mdurl + MD_VER + "/" + k, - v if not isinstance(v, str) else v.encode('utf-8')) - - def test_broken_no_sys_uuid(self, m_read_system_uuid): - """Datasource requires ability to read system_uuid and true return.""" - m_read_system_uuid.return_value = None - self.assertRaises(BrokenMetadata, oracle.read_metadata) - - def test_broken_no_metadata_json(self, m_read_system_uuid): - """Datasource requires meta_data.json.""" - httpretty.register_uri( - httpretty.GET, self.mdurl + MD_VER + "/", - '\n'.join(['user_data']).encode('utf-8')) - with self.assertRaises(BrokenMetadata) as cm: - oracle.read_metadata() - self.assertIn("Required field 'meta_data.json' missing", - str(cm.exception)) - - def test_with_userdata(self, m_read_system_uuid): - data = {'user_data': b'#!/bin/sh\necho hi world\n', - 'meta_data.json': json.dumps(self.my_md)} - self.populate_md(data) - result = oracle.read_metadata()[MD_VER] - self.assertEqual(data['user_data'], result['user_data']) - self.assertEqual(self.my_md, result['meta_data']) - - def test_without_userdata(self, m_read_system_uuid): - data = {'meta_data.json': json.dumps(self.my_md)} - self.populate_md(data) - result = oracle.read_metadata()[MD_VER] - self.assertNotIn('user_data', result) - self.assertEqual(self.my_md, result['meta_data']) - - def test_unknown_fields_included(self, m_read_system_uuid): - """Unknown fields listed in index should be included. - And those ending in .json should be decoded.""" - some_data = {'key1': 'data1', 'subk1': {'subd1': 'subv'}} - some_vendor_data = {'cloud-init': 'foo'} - data = {'meta_data.json': json.dumps(self.my_md), - 'some_data.json': json.dumps(some_data), - 'vendor_data.json': json.dumps(some_vendor_data), - 'other_blob': b'this is blob'} - self.populate_md(data) - result = oracle.read_metadata()[MD_VER] - self.assertNotIn('user_data', result) - self.assertEqual(self.my_md, result['meta_data']) - self.assertEqual(some_data, result['some_data']) - self.assertEqual(some_vendor_data, result['vendor_data']) - self.assertEqual(data['other_blob'], result['other_blob']) + @pytest.mark.ds_sys_cfg( + {"datasource": {"Oracle": {"configure_secondary_nics": True}}} + ) + def test_sys_cfg_can_enable_configure_secondary_nics(self, oracle_ds): + assert oracle_ds.ds_cfg["configure_secondary_nics"] class TestIsPlatformViable(test_helpers.CiTestCase): @@ -407,73 +153,6 @@ class TestIsPlatformViable(test_helpers.CiTestCase): m_read_dmi_data.assert_has_calls([mock.call('chassis-asset-tag')]) -class TestLoadIndex(test_helpers.CiTestCase): - """_load_index handles parsing of an index into a proper list. - The tests here guarantee correct parsing of html version or - a fixed version. See the function docstring for more doc.""" - - _known_html_api_versions = dedent("""\ - - Index of /openstack/ - -

Index of /openstack/

../
-        2013-10-17/   27-Jun-2018 12:22  -
-        latest/           27-Jun-2018 12:22  -
-
- """) - - _known_html_contents = dedent("""\ - - Index of /openstack/2013-10-17/ - -

Index of /openstack/2013-10-17/

../
-        meta_data.json  27-Jun-2018 12:22  679
-        user_data            27-Jun-2018 12:22  146
-
- """) - - def test_parse_html(self): - """Test parsing of lower case html.""" - self.assertEqual( - ['2013-10-17/', 'latest/'], - oracle._load_index(self._known_html_api_versions)) - self.assertEqual( - ['meta_data.json', 'user_data'], - oracle._load_index(self._known_html_contents)) - - def test_parse_html_upper(self): - """Test parsing of upper case html, although known content is lower.""" - def _toupper(data): - return data.replace("", "HTML>") - - self.assertEqual( - ['2013-10-17/', 'latest/'], - oracle._load_index(_toupper(self._known_html_api_versions))) - self.assertEqual( - ['meta_data.json', 'user_data'], - oracle._load_index(_toupper(self._known_html_contents))) - - def test_parse_newline_list_with_endl(self): - """Test parsing of newline separated list with ending newline.""" - self.assertEqual( - ['2013-10-17/', 'latest/'], - oracle._load_index("\n".join(["2013-10-17/", "latest/", ""]))) - self.assertEqual( - ['meta_data.json', 'user_data'], - oracle._load_index("\n".join(["meta_data.json", "user_data", ""]))) - - def test_parse_newline_list_without_endl(self): - """Test parsing of newline separated list with no ending newline. - - Actual openstack implementation does not include trailing newline.""" - self.assertEqual( - ['2013-10-17/', 'latest/'], - oracle._load_index("\n".join(["2013-10-17/", "latest/"]))) - self.assertEqual( - ['meta_data.json', 'user_data'], - oracle._load_index("\n".join(["meta_data.json", "user_data"]))) - - class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): with_logs = True @@ -733,4 +412,309 @@ class TestNetworkConfigFiltersNetFailover(test_helpers.CiTestCase): self.assertEqual(expected_cfg, netcfg) +class TestReadOpcMetadata: + # See https://docs.pytest.org/en/stable/example + # /parametrize.html#parametrizing-conditional-raising + does_not_raise = ExitStack + + @pytest.fixture(autouse=True) + def configure_opc_metadata_in_httpretty(self, httpretty): + """Configure HTTPretty with the various OPC metadata endpoints.""" + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v1/instance/", + OPC_V1_METADATA, + ) + + def test_json_decoded_value_returned(self): + # read_opc_metadata should JSON decode the response and return it + expected = json.loads(OPC_V1_METADATA) + assert expected == oracle.read_opc_metadata() + + # No need to actually wait between retries in the tests + @mock.patch("cloudinit.url_helper.time.sleep", lambda _: None) + @pytest.mark.parametrize( + "failure_count,expectation", + [(1, does_not_raise()), (2, pytest.raises(UrlError))], + ) + def test_retries(self, expectation, failure_count, httpretty): + responses = [httpretty.Response("", status=404)] * failure_count + responses.append(httpretty.Response(OPC_V1_METADATA)) + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v1/instance/", + responses=responses, + ) + expected = json.loads(OPC_V1_METADATA) + with expectation: + assert expected == oracle.read_opc_metadata() + + +class TestCommon_GetDataBehaviour: + """This test class tests behaviour common to iSCSI and non-iSCSI root. + + It defines a fixture, parameterized_oracle_ds, which is used in all the + tests herein to test that the commonly expected behaviour is the same with + iSCSI root and without. + + (As non-iSCSI root behaviour is a superset of iSCSI root behaviour this + class is implicitly also testing all iSCSI root behaviour so there is no + separate class for that case.) + """ + + @pytest.yield_fixture(params=[True, False]) + def parameterized_oracle_ds(self, request, oracle_ds): + """oracle_ds parameterized for iSCSI and non-iSCSI root respectively""" + is_iscsi_root = request.param + with ExitStack() as stack: + stack.enter_context( + mock.patch( + DS_PATH + "._is_iscsi_root", return_value=is_iscsi_root + ) + ) + if not is_iscsi_root: + stack.enter_context( + mock.patch(DS_PATH + ".net.find_fallback_nic") + ) + stack.enter_context( + mock.patch(DS_PATH + ".dhcp.EphemeralDHCPv4") + ) + yield oracle_ds + + @mock.patch( + DS_PATH + "._is_platform_viable", mock.Mock(return_value=False) + ) + def test_false_if_platform_not_viable( + self, parameterized_oracle_ds, + ): + assert not parameterized_oracle_ds._get_data() + + @pytest.mark.parametrize( + "keyname,expected_value", + ( + ("availability-zone", "phx-ad-3"), + ("launch-index", 0), + ("local-hostname", "instance-20200320-1400"), + ( + "instance-id", + "ocid1.instance.oc1.phx" + ".anyhqljtniwq6syc3nex55sep5w34qbwmw6TRUNCATED", + ), + ("name", "instance-20200320-1400"), + ( + "public_keys", + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQ truncated", + ), + ), + ) + def test_metadata_keys_set_correctly( + self, keyname, expected_value, parameterized_oracle_ds, + ): + assert parameterized_oracle_ds._get_data() + assert expected_value == parameterized_oracle_ds.metadata[keyname] + + @pytest.mark.parametrize( + "attribute_name,expected_value", + [ + ("_crawled_metadata", json.loads(OPC_V1_METADATA)), + ( + "userdata_raw", + base64.b64decode(b"IyEvYmluL3NoCnRvdWNoIC90bXAvZm9v"), + ), + ("system_uuid", "my-test-uuid"), + ], + ) + @mock.patch( + DS_PATH + "._read_system_uuid", mock.Mock(return_value="my-test-uuid") + ) + def test_attributes_set_correctly( + self, attribute_name, expected_value, parameterized_oracle_ds, + ): + assert parameterized_oracle_ds._get_data() + assert expected_value == getattr( + parameterized_oracle_ds, attribute_name + ) + + @pytest.mark.parametrize( + "ssh_keys,expected_value", + [ + # No SSH keys in metadata => no keys detected + (None, []), + # Empty SSH keys in metadata => no keys detected + ("", []), + # Single SSH key in metadata => single key detected + ("ssh-rsa ... test@test", ["ssh-rsa ... test@test"]), + # Multiple SSH keys in metadata => multiple keys detected + ( + "ssh-rsa ... test@test\nssh-rsa ... test2@test2", + ["ssh-rsa ... test@test", "ssh-rsa ... test2@test2"], + ), + ], + ) + def test_public_keys_handled_correctly( + self, ssh_keys, expected_value, parameterized_oracle_ds + ): + metadata = json.loads(OPC_V1_METADATA) + if ssh_keys is None: + del metadata["metadata"]["ssh_authorized_keys"] + else: + metadata["metadata"]["ssh_authorized_keys"] = ssh_keys + with mock.patch( + DS_PATH + ".read_opc_metadata", mock.Mock(return_value=metadata), + ): + assert parameterized_oracle_ds._get_data() + assert ( + expected_value == parameterized_oracle_ds.get_public_ssh_keys() + ) + + def test_missing_user_data_handled_gracefully( + self, parameterized_oracle_ds + ): + metadata = json.loads(OPC_V1_METADATA) + del metadata["metadata"]["user_data"] + with mock.patch( + DS_PATH + ".read_opc_metadata", mock.Mock(return_value=metadata), + ): + assert parameterized_oracle_ds._get_data() + + assert parameterized_oracle_ds.userdata_raw is None + + def test_missing_metadata_handled_gracefully( + self, parameterized_oracle_ds + ): + metadata = json.loads(OPC_V1_METADATA) + del metadata["metadata"] + with mock.patch( + DS_PATH + ".read_opc_metadata", mock.Mock(return_value=metadata), + ): + assert parameterized_oracle_ds._get_data() + + assert parameterized_oracle_ds.userdata_raw is None + assert [] == parameterized_oracle_ds.get_public_ssh_keys() + + +@mock.patch(DS_PATH + "._is_iscsi_root", lambda: False) +class TestNonIscsiRoot_GetDataBehaviour: + @mock.patch(DS_PATH + ".dhcp.EphemeralDHCPv4") + @mock.patch(DS_PATH + ".net.find_fallback_nic") + def test_read_opc_metadata_called_with_ephemeral_dhcp( + self, m_find_fallback_nic, m_EphemeralDHCPv4, oracle_ds + ): + in_context_manager = False + + def enter_context_manager(): + nonlocal in_context_manager + in_context_manager = True + + def exit_context_manager(*args): + nonlocal in_context_manager + in_context_manager = False + + m_EphemeralDHCPv4.return_value.__enter__.side_effect = ( + enter_context_manager + ) + m_EphemeralDHCPv4.return_value.__exit__.side_effect = ( + exit_context_manager + ) + + def assert_in_context_manager(): + assert in_context_manager + return mock.MagicMock() + + with mock.patch( + DS_PATH + ".read_opc_metadata", + mock.Mock(side_effect=assert_in_context_manager), + ): + assert oracle_ds._get_data() + + assert [ + mock.call(m_find_fallback_nic.return_value) + ] == m_EphemeralDHCPv4.call_args_list + + +@mock.patch(DS_PATH + ".get_interfaces_by_mac", lambda: {}) +@mock.patch(DS_PATH + ".cmdline.read_initramfs_config") +class TestNetworkConfig: + def test_network_config_cached(self, m_read_initramfs_config, oracle_ds): + """.network_config should be cached""" + assert 0 == m_read_initramfs_config.call_count + oracle_ds.network_config # pylint: disable=pointless-statement + assert 1 == m_read_initramfs_config.call_count + oracle_ds.network_config # pylint: disable=pointless-statement + assert 1 == m_read_initramfs_config.call_count + + def test_network_cmdline(self, m_read_initramfs_config, oracle_ds): + """network_config should prefer initramfs config over fallback""" + ncfg = {"version": 1, "config": [{"a": "b"}]} + m_read_initramfs_config.return_value = copy.deepcopy(ncfg) + + assert ncfg == oracle_ds.network_config + assert 0 == oracle_ds.distro.generate_fallback_config.call_count + + def test_network_fallback(self, m_read_initramfs_config, oracle_ds): + """network_config should prefer initramfs config over fallback""" + ncfg = {"version": 1, "config": [{"a": "b"}]} + + m_read_initramfs_config.return_value = None + oracle_ds.distro.generate_fallback_config.return_value = copy.deepcopy( + ncfg + ) + + assert ncfg == oracle_ds.network_config + + @pytest.mark.parametrize( + "configure_secondary_nics,expect_secondary_nics", + [(True, True), (False, False), (None, False)], + ) + @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") + def test_secondary_nic_addition( + self, + m_add_network_config_from_opc_imds, + m_read_initramfs_config, + configure_secondary_nics, + expect_secondary_nics, + oracle_ds, + ): + """Test that _add_network_config_from_opc_imds is called as expected + + (configure_secondary_nics=None is used to test the default behaviour.) + """ + m_read_initramfs_config.return_value = {"version": 1, "config": []} + + def side_effect(network_config): + network_config["secondary_added"] = mock.sentinel.needle + + m_add_network_config_from_opc_imds.side_effect = side_effect + + if configure_secondary_nics is not None: + oracle_ds.ds_cfg[ + "configure_secondary_nics" + ] = configure_secondary_nics + + was_secondary_added = "secondary_added" in oracle_ds.network_config + assert expect_secondary_nics == was_secondary_added + + @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") + def test_secondary_nic_failure_isnt_blocking( + self, + m_add_network_config_from_opc_imds, + m_read_initramfs_config, + caplog, + oracle_ds, + ): + m_add_network_config_from_opc_imds.side_effect = Exception() + + oracle_ds.ds_cfg["configure_secondary_nics"] = True + + assert m_read_initramfs_config.return_value == oracle_ds.network_config + assert "Failed to fetch secondary network configuration" in caplog.text + + def test_ds_network_cfg_preferred_over_initramfs(self, _m): + """Ensure that DS net config is preferred over initramfs config""" + config_sources = oracle.DataSourceOracle.network_config_sources + ds_idx = config_sources.index(NetworkConfigSource.ds) + initramfs_idx = config_sources.index(NetworkConfigSource.initramfs) + assert ds_idx < initramfs_idx + + # vi: ts=4 expandtab diff --git a/conftest.py b/conftest.py index faf13804..76e9000a 100644 --- a/conftest.py +++ b/conftest.py @@ -1,24 +1,64 @@ +import os from unittest import mock import pytest +import httpretty as _httpretty -from cloudinit import subp +from cloudinit import helpers, subp -def _closest_marker_args_or(request, marker_name: str, default): - """Get the args for the closest ``marker_name`` or return ``default``""" - try: - marker = request.node.get_closest_marker(marker_name) - except AttributeError: - # Older versions of pytest don't have the new API - marker = request.node.get_marker(marker_name) - if marker is not None: - return marker.args - return default +class _FixtureUtils: + """A namespace for fixture helper functions, used by fixture_utils. + + These helper functions are all defined as staticmethods so they are + effectively functions; they are defined in a class only to give us a + namespace so calling them can look like + ``fixture_utils.fixture_util_function()`` in test code. + """ + + @staticmethod + def closest_marker_args_or(request, marker_name: str, default): + """Get the args for closest ``marker_name`` or return ``default`` + + :param request: + A pytest request, as passed to a fixture. + :param marker_name: + The name of the marker to look for + :param default: + The value to return if ``marker_name`` is not found. + + :return: + The args for the closest ``marker_name`` marker, or ``default`` + if no such marker is found. + """ + try: + marker = request.node.get_closest_marker(marker_name) + except AttributeError: + # Older versions of pytest don't have the new API + marker = request.node.get_marker(marker_name) + if marker is not None: + return marker.args + return default + + @staticmethod + def closest_marker_first_arg_or(request, marker_name: str, default): + """Get the first arg for closest ``marker_name`` or return ``default`` + + This is a convenience wrapper around closest_marker_args_or, see there + for full details. + """ + result = _FixtureUtils.closest_marker_args_or( + request, marker_name, [default] + ) + if not result: + raise TypeError( + "Missing expected argument to {} marker".format(marker_name) + ) + return result[0] @pytest.yield_fixture(autouse=True) -def disable_subp_usage(request): +def disable_subp_usage(request, fixture_utils): """ Across all (pytest) tests, ensure that subp.subp is not invoked. @@ -53,10 +93,14 @@ def disable_subp_usage(request): tests, CiTestCase's allowed_subp does take precedence (and we have TestDisableSubpUsageInTestSubclass to confirm that). """ - allow_subp_for = _closest_marker_args_or(request, "allow_subp_for", None) + allow_subp_for = fixture_utils.closest_marker_args_or( + request, "allow_subp_for", None + ) # Because the mark doesn't take arguments, `allow_all_subp` will be set to # [] if the marker is present, so explicit None checks are required - allow_all_subp = _closest_marker_args_or(request, "allow_all_subp", None) + allow_all_subp = fixture_utils.closest_marker_args_or( + request, "allow_all_subp", None + ) if allow_all_subp is not None and allow_subp_for is None: # Only allow_all_subp specified, don't mock subp.subp @@ -93,3 +137,47 @@ def disable_subp_usage(request): with mock.patch("cloudinit.subp.subp", autospec=True) as m_subp: m_subp.side_effect = side_effect yield + + +@pytest.fixture(scope="session") +def fixture_utils(): + """Return a namespace containing fixture utility functions. + + See :py:class:`_FixtureUtils` for further details.""" + return _FixtureUtils + + +@pytest.yield_fixture +def httpretty(): + """ + Enable HTTPretty for duration of the testcase, resetting before and after. + + This will also ensure allow_net_connect is set to False, and temporarily + unset http_proxy in os.environ if present (to work around + https://github.com/gabrielfalcao/HTTPretty/issues/122). + """ + restore_proxy = os.environ.pop("http_proxy", None) + _httpretty.HTTPretty.allow_net_connect = False + _httpretty.reset() + _httpretty.enable() + + yield _httpretty + + _httpretty.disable() + _httpretty.reset() + if restore_proxy is not None: + os.environ["http_proxy"] = restore_proxy + + +@pytest.fixture +def paths(tmpdir): + """ + Return a helpers.Paths object configured to use a tmpdir. + + (This uses the builtin tmpdir fixture.) + """ + dirs = { + "cloud_dir": tmpdir.mkdir("cloud_dir").strpath, + "run_dir": tmpdir.mkdir("run_dir").strpath, + } + return helpers.Paths(dirs) diff --git a/tox.ini b/tox.ini index 3fd96702..6c4b2e81 100644 --- a/tox.ini +++ b/tox.ini @@ -140,3 +140,4 @@ addopts = --strict markers = allow_subp_for: allow subp usage for the given commands (disable_subp_usage) allow_all_subp: allow all subp usage (disable_subp_usage) + ds_sys_cfg: a sys_cfg dict to be used by datasource fixtures -- cgit v1.2.3 From 546617c449f2e973717191a07c243ec1b6bfc8da Mon Sep 17 00:00:00 2001 From: James Falcon Date: Thu, 13 Aug 2020 14:38:37 -0500 Subject: Support Oracle IMDSv2 API (#528) * v2 of the API is now default with fallback to v1. * Refactored the Oracle datasource to fetch version, instance, and vnic metadata simultaneously. --- cloudinit/sources/DataSourceOracle.py | 221 ++++++++++++--------- cloudinit/sources/tests/test_oracle.py | 339 +++++++++++++++++++-------------- 2 files changed, 330 insertions(+), 230 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index f113d364..2354648b 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -14,7 +14,8 @@ Notes: """ import base64 -import json +from collections import namedtuple +from contextlib import suppress as noop from cloudinit import log as logging from cloudinit import net, sources, util @@ -24,7 +25,7 @@ from cloudinit.net import ( get_interfaces_by_mac, is_netfail_master, ) -from cloudinit.url_helper import readurl +from cloudinit.url_helper import UrlError, readurl LOG = logging.getLogger(__name__) @@ -33,80 +34,13 @@ BUILTIN_DS_CONFIG = { 'configure_secondary_nics': False, } CHASSIS_ASSET_TAG = "OracleCloud.com" -METADATA_ROOT = "http://169.254.169.254/opc/v1/" -METADATA_ENDPOINT = METADATA_ROOT + "instance/" -VNIC_METADATA_URL = METADATA_ROOT + "vnics/" +METADATA_ROOT = "http://169.254.169.254/opc/v{version}/" +METADATA_PATTERN = METADATA_ROOT + "{path}/" # https://docs.cloud.oracle.com/iaas/Content/Network/Troubleshoot/connectionhang.htm#Overview, # indicates that an MTU of 9000 is used within OCI MTU = 9000 - -def _add_network_config_from_opc_imds(network_config): - """ - Fetch data from Oracle's IMDS, generate secondary NIC config, merge it. - - The primary NIC configuration should not be modified based on the IMDS - values, as it should continue to be configured for DHCP. As such, this - takes an existing network_config dict which is expected to have the primary - NIC configuration already present. It will mutate the given dict to - include the secondary VNICs. - - :param network_config: - A v1 or v2 network config dict with the primary NIC already configured. - This dict will be mutated. - - :raises: - Exceptions are not handled within this function. Likely exceptions are - those raised by url_helper.readurl (if communicating with the IMDS - fails), ValueError/JSONDecodeError (if the IMDS returns invalid JSON), - and KeyError/IndexError (if the IMDS returns valid JSON with unexpected - contents). - """ - resp = readurl(VNIC_METADATA_URL) - vnics = json.loads(str(resp)) - - if 'nicIndex' in vnics[0]: - # TODO: Once configure_secondary_nics defaults to True, lower the level - # of this log message. (Currently, if we're running this code at all, - # someone has explicitly opted-in to secondary VNIC configuration, so - # we should warn them that it didn't happen. Once it's default, this - # would be emitted on every Bare Metal Machine launch, which means INFO - # or DEBUG would be more appropriate.) - LOG.warning( - 'VNIC metadata indicates this is a bare metal machine; skipping' - ' secondary VNIC configuration.' - ) - return - - interfaces_by_mac = get_interfaces_by_mac() - - for vnic_dict in vnics[1:]: - # We skip the first entry in the response because the primary interface - # is already configured by iSCSI boot; applying configuration from the - # IMDS is not required. - mac_address = vnic_dict['macAddr'].lower() - if mac_address not in interfaces_by_mac: - LOG.debug('Interface with MAC %s not found; skipping', mac_address) - continue - name = interfaces_by_mac[mac_address] - - if network_config['version'] == 1: - subnet = { - 'type': 'static', - 'address': vnic_dict['privateIp'], - } - network_config['config'].append({ - 'name': name, - 'type': 'physical', - 'mac_address': mac_address, - 'mtu': MTU, - 'subnets': [subnet], - }) - elif network_config['version'] == 2: - network_config['ethernets'][name] = { - 'addresses': [vnic_dict['privateIp']], - 'mtu': MTU, 'dhcp4': False, 'dhcp6': False, - 'match': {'macaddress': mac_address}} +OpcMetadata = namedtuple("OpcMetadata", "version instance_data vnics_data") def _ensure_netfailover_safe(network_config): @@ -175,6 +109,7 @@ class DataSourceOracle(sources.DataSource): def __init__(self, sys_cfg, *args, **kwargs): super(DataSourceOracle, self).__init__(sys_cfg, *args, **kwargs) + self._vnics_data = None self.ds_cfg = util.mergemanydict([ util.get_cfg_by_path(sys_cfg, ['datasource', self.dsname], {}), @@ -192,13 +127,23 @@ class DataSourceOracle(sources.DataSource): # network may be configured if iscsi root. If that is the case # then read_initramfs_config will return non-None. - if _is_iscsi_root(): - data = read_opc_metadata() - else: - with dhcp.EphemeralDHCPv4(net.find_fallback_nic()): - data = read_opc_metadata() + fetch_vnics_data = self.ds_cfg.get( + 'configure_secondary_nics', + BUILTIN_DS_CONFIG["configure_secondary_nics"] + ) + network_context = noop() + if not _is_iscsi_root(): + network_context = dhcp.EphemeralDHCPv4(net.find_fallback_nic()) + with network_context: + fetched_metadata = read_opc_metadata( + fetch_vnics_data=fetch_vnics_data + ) - self._crawled_metadata = data + data = self._crawled_metadata = fetched_metadata.instance_data + self.metadata_address = METADATA_ROOT.format( + version=fetched_metadata.version + ) + self._vnics_data = fetched_metadata.vnics_data self.metadata = { "availability-zone": data["ociAdName"], @@ -218,10 +163,6 @@ class DataSourceOracle(sources.DataSource): return True - def _get_subplatform(self): - """Return the subplatform metadata source details.""" - return "metadata ({})".format(METADATA_ROOT) - def check_instance_id(self, sys_cfg): """quickly check (local only) if self.instance_id is still valid @@ -246,14 +187,18 @@ class DataSourceOracle(sources.DataSource): # this is now v2 self._network_config = self.distro.generate_fallback_config() - if self.ds_cfg.get('configure_secondary_nics'): + if self.ds_cfg.get( + 'configure_secondary_nics', + BUILTIN_DS_CONFIG["configure_secondary_nics"] + ): try: - # Mutate self._network_config to include secondary VNICs - _add_network_config_from_opc_imds(self._network_config) + # Mutate self._network_config to include secondary + # VNICs + self._add_network_config_from_opc_imds() except Exception: util.logexc( LOG, - "Failed to fetch secondary network configuration!") + "Failed to parse secondary network configuration!") # we need to verify that the nic selected is not a netfail over # device and, if it is a netfail master, then we need to avoid @@ -262,6 +207,70 @@ class DataSourceOracle(sources.DataSource): return self._network_config + def _add_network_config_from_opc_imds(self): + """Generate secondary NIC config from IMDS and merge it. + + The primary NIC configuration should not be modified based on the IMDS + values, as it should continue to be configured for DHCP. As such, this + uses the instance's network config dict which is expected to have the + primary NIC configuration already present. + It will mutate the network config to include the secondary VNICs. + + :raises: + Exceptions are not handled within this function. Likely + exceptions are KeyError/IndexError + (if the IMDS returns valid JSON with unexpected contents). + """ + if self._vnics_data is None: + LOG.warning( + "Secondary NIC data is UNSET but should not be") + return + + if 'nicIndex' in self._vnics_data[0]: + # TODO: Once configure_secondary_nics defaults to True, lower the + # level of this log message. (Currently, if we're running this + # code at all, someone has explicitly opted-in to secondary + # VNIC configuration, so we should warn them that it didn't + # happen. Once it's default, this would be emitted on every Bare + # Metal Machine launch, which means INFO or DEBUG would be more + # appropriate.) + LOG.warning( + 'VNIC metadata indicates this is a bare metal machine; ' + 'skipping secondary VNIC configuration.' + ) + return + + interfaces_by_mac = get_interfaces_by_mac() + + for vnic_dict in self._vnics_data[1:]: + # We skip the first entry in the response because the primary + # interface is already configured by iSCSI boot; applying + # configuration from the IMDS is not required. + mac_address = vnic_dict['macAddr'].lower() + if mac_address not in interfaces_by_mac: + LOG.debug('Interface with MAC %s not found; skipping', + mac_address) + continue + name = interfaces_by_mac[mac_address] + + if self._network_config['version'] == 1: + subnet = { + 'type': 'static', + 'address': vnic_dict['privateIp'], + } + self._network_config['config'].append({ + 'name': name, + 'type': 'physical', + 'mac_address': mac_address, + 'mtu': MTU, + 'subnets': [subnet], + }) + elif self._network_config['version'] == 2: + self._network_config['ethernets'][name] = { + 'addresses': [vnic_dict['privateIp']], + 'mtu': MTU, 'dhcp4': False, 'dhcp6': False, + 'match': {'macaddress': mac_address}} + def _read_system_uuid(): sys_uuid = util.read_dmi_data('system-uuid') @@ -277,15 +286,43 @@ def _is_iscsi_root(): return bool(cmdline.read_initramfs_config()) -def read_opc_metadata(): - """ - Fetch metadata from the /opc/ routes. +def read_opc_metadata(*, fetch_vnics_data: bool = False): + """Fetch metadata from the /opc/ routes. :return: - The JSON-decoded value of the /opc/v1/instance/ endpoint on the IMDS. + A namedtuple containing: + The metadata version as an integer + The JSON-decoded value of the instance data endpoint on the IMDS + The JSON-decoded value of the vnics data endpoint if + `fetch_vnics_data` is True, else None + """ - # retries=1 as requested by Oracle to address a potential race condition - return json.loads(readurl(METADATA_ENDPOINT, retries=1)._response.text) + retries = 1 + + def _fetch(metadata_version: int, path: str) -> dict: + headers = { + "Authorization": "Bearer Oracle"} if metadata_version > 1 else None + return readurl( + url=METADATA_PATTERN.format(version=metadata_version, path=path), + headers=headers, + retries=retries, + )._response.json() + + metadata_version = 2 + try: + instance_data = _fetch(metadata_version, path="instance") + except UrlError: + metadata_version = 1 + instance_data = _fetch(metadata_version, path="instance") + + vnics_data = None + if fetch_vnics_data: + try: + vnics_data = _fetch(metadata_version, path="vnics") + except UrlError: + util.logexc(LOG, + "Failed to fetch secondary network configuration!") + return OpcMetadata(metadata_version, instance_data, vnics_data) # Used to match classes to dependencies diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 9ee6e7fa..2fb9a20a 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -10,6 +10,7 @@ import pytest from cloudinit.sources import DataSourceOracle as oracle from cloudinit.sources import NetworkConfigSource +from cloudinit.sources.DataSourceOracle import OpcMetadata from cloudinit.tests import helpers as test_helpers from cloudinit.url_helper import UrlError @@ -58,7 +59,7 @@ OPC_VM_SECONDARY_VNIC_RESPONSE = """\ # Fetched with `curl http://169.254.169.254/opc/v1/instance/` (and then # truncated for line length) -OPC_V1_METADATA = """\ +OPC_V2_METADATA = """\ { "availabilityDomain" : "qIZq:PHX-AD-1", "faultDomain" : "FAULT-DOMAIN-2", @@ -83,9 +84,17 @@ OPC_V1_METADATA = """\ } }""" +# Just a small meaningless change to differentiate the two metadatas +OPC_V1_METADATA = OPC_V2_METADATA.replace("ocid1.instance", "ocid2.instance") + + +@pytest.fixture +def metadata_version(): + return 2 + @pytest.yield_fixture -def oracle_ds(request, fixture_utils, paths): +def oracle_ds(request, fixture_utils, paths, metadata_version): """ Return an instantiated DataSourceOracle. @@ -101,12 +110,13 @@ def oracle_ds(request, fixture_utils, paths): sys_cfg = fixture_utils.closest_marker_first_arg_or( request, "ds_sys_cfg", mock.MagicMock() ) + metadata = OpcMetadata(metadata_version, json.loads(OPC_V2_METADATA), None) with mock.patch(DS_PATH + "._read_system_uuid", return_value="someuuid"): with mock.patch(DS_PATH + "._is_platform_viable", return_value=True): with mock.patch(DS_PATH + "._is_iscsi_root", return_value=True): with mock.patch( DS_PATH + ".read_opc_metadata", - return_value=json.loads(OPC_V1_METADATA), + return_value=metadata, ): yield oracle.DataSourceOracle( sys_cfg=sys_cfg, distro=mock.Mock(), paths=paths, @@ -117,10 +127,20 @@ class TestDataSourceOracle: def test_platform_info(self, oracle_ds): assert "oracle" == oracle_ds.cloud_name assert "oracle" == oracle_ds.platform_type - assert ( - "metadata (http://169.254.169.254/opc/v1/)" - == oracle_ds.subplatform - ) + + def test_subplatform_before_fetch(self, oracle_ds): + assert 'unknown' == oracle_ds.subplatform + + def test_platform_info_after_fetch(self, oracle_ds): + oracle_ds._get_data() + assert 'metadata (http://169.254.169.254/opc/v2/)' == \ + oracle_ds.subplatform + + @pytest.mark.parametrize('metadata_version', [1]) + def test_v1_platform_info_after_fetch(self, oracle_ds): + oracle_ds._get_data() + assert 'metadata (http://169.254.169.254/opc/v1/)' == \ + oracle_ds.subplatform def test_secondary_nics_disabled_by_default(self, oracle_ds): assert not oracle_ds.ds_cfg["configure_secondary_nics"] @@ -153,119 +173,95 @@ class TestIsPlatformViable(test_helpers.CiTestCase): m_read_dmi_data.assert_has_calls([mock.call('chassis-asset-tag')]) -class TestNetworkConfigFromOpcImds(test_helpers.CiTestCase): - - with_logs = True - - def setUp(self): - super(TestNetworkConfigFromOpcImds, self).setUp() - self.add_patch(DS_PATH + '.readurl', 'm_readurl') - self.add_patch(DS_PATH + '.get_interfaces_by_mac', - 'm_get_interfaces_by_mac') - - def test_failure_to_readurl(self): - # readurl failures should just bubble out to the caller - self.m_readurl.side_effect = Exception('oh no') - with self.assertRaises(Exception) as excinfo: - oracle._add_network_config_from_opc_imds({}) - self.assertEqual(str(excinfo.exception), 'oh no') - - def test_empty_response(self): - # empty response error should just bubble out to the caller - self.m_readurl.return_value = '' - with self.assertRaises(Exception): - oracle._add_network_config_from_opc_imds([]) - - def test_invalid_json(self): - # invalid JSON error should just bubble out to the caller - self.m_readurl.return_value = '{' - with self.assertRaises(Exception): - oracle._add_network_config_from_opc_imds([]) - - def test_no_secondary_nics_does_not_mutate_input(self): - self.m_readurl.return_value = json.dumps([{}]) - # We test this by passing in a non-dict to ensure that no dict +class TestNetworkConfigFromOpcImds: + def test_no_secondary_nics_does_not_mutate_input(self, oracle_ds): + oracle_ds._vnics_data = [{}] + # We test this by using in a non-dict to ensure that no dict # operations are used; failure would be seen as exceptions - oracle._add_network_config_from_opc_imds(object()) + oracle_ds._network_config = object() + oracle_ds._add_network_config_from_opc_imds() - def test_bare_metal_machine_skipped(self): + def test_bare_metal_machine_skipped(self, oracle_ds, caplog): # nicIndex in the first entry indicates a bare metal machine - self.m_readurl.return_value = OPC_BM_SECONDARY_VNIC_RESPONSE - # We test this by passing in a non-dict to ensure that no dict + oracle_ds._vnics_data = json.loads(OPC_BM_SECONDARY_VNIC_RESPONSE) + # We test this by using a non-dict to ensure that no dict # operations are used - self.assertFalse(oracle._add_network_config_from_opc_imds(object())) - self.assertIn('bare metal machine', self.logs.getvalue()) + oracle_ds._network_config = object() + oracle_ds._add_network_config_from_opc_imds() + assert 'bare metal machine' in caplog.text - def test_missing_mac_skipped(self): - self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE - self.m_get_interfaces_by_mac.return_value = {} + def test_missing_mac_skipped(self, oracle_ds, caplog): + oracle_ds._vnics_data = json.loads(OPC_VM_SECONDARY_VNIC_RESPONSE) - network_config = {'version': 1, 'config': [{'primary': 'nic'}]} - oracle._add_network_config_from_opc_imds(network_config) + oracle_ds._network_config = { + 'version': 1, 'config': [{'primary': 'nic'}] + } + with mock.patch(DS_PATH + ".get_interfaces_by_mac", return_value={}): + oracle_ds._add_network_config_from_opc_imds() - self.assertEqual(1, len(network_config['config'])) - self.assertIn( - 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping', - self.logs.getvalue()) + assert 1 == len(oracle_ds.network_config['config']) + assert 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping' in \ + caplog.text - def test_missing_mac_skipped_v2(self): - self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE - self.m_get_interfaces_by_mac.return_value = {} + def test_missing_mac_skipped_v2(self, oracle_ds, caplog): + oracle_ds._vnics_data = json.loads(OPC_VM_SECONDARY_VNIC_RESPONSE) - network_config = {'version': 2, 'ethernets': {'primary': {'nic': {}}}} - oracle._add_network_config_from_opc_imds(network_config) + oracle_ds._network_config = { + 'version': 2, 'ethernets': {'primary': {'nic': {}}} + } + with mock.patch(DS_PATH + ".get_interfaces_by_mac", return_value={}): + oracle_ds._add_network_config_from_opc_imds() - self.assertEqual(1, len(network_config['ethernets'])) - self.assertIn( - 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping', - self.logs.getvalue()) + assert 1 == len(oracle_ds.network_config['ethernets']) + assert 'Interface with MAC 00:00:17:02:2b:b1 not found; skipping' in \ + caplog.text - def test_secondary_nic(self): - self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE - mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' - self.m_get_interfaces_by_mac.return_value = { - mac_addr: nic_name, + def test_secondary_nic(self, oracle_ds): + oracle_ds._vnics_data = json.loads(OPC_VM_SECONDARY_VNIC_RESPONSE) + oracle_ds._network_config = { + 'version': 1, 'config': [{'primary': 'nic'}] } - - network_config = {'version': 1, 'config': [{'primary': 'nic'}]} - oracle._add_network_config_from_opc_imds(network_config) + mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' + with mock.patch(DS_PATH + ".get_interfaces_by_mac", + return_value={mac_addr: nic_name}): + oracle_ds._add_network_config_from_opc_imds() # The input is mutated - self.assertEqual(2, len(network_config['config'])) + assert 2 == len(oracle_ds.network_config['config']) - secondary_nic_cfg = network_config['config'][1] - self.assertEqual(nic_name, secondary_nic_cfg['name']) - self.assertEqual('physical', secondary_nic_cfg['type']) - self.assertEqual(mac_addr, secondary_nic_cfg['mac_address']) - self.assertEqual(9000, secondary_nic_cfg['mtu']) + secondary_nic_cfg = oracle_ds.network_config['config'][1] + assert nic_name == secondary_nic_cfg['name'] + assert 'physical' == secondary_nic_cfg['type'] + assert mac_addr == secondary_nic_cfg['mac_address'] + assert 9000 == secondary_nic_cfg['mtu'] - self.assertEqual(1, len(secondary_nic_cfg['subnets'])) + assert 1 == len(secondary_nic_cfg['subnets']) subnet_cfg = secondary_nic_cfg['subnets'][0] # These values are hard-coded in OPC_VM_SECONDARY_VNIC_RESPONSE - self.assertEqual('10.0.0.231', subnet_cfg['address']) + assert '10.0.0.231' == subnet_cfg['address'] - def test_secondary_nic_v2(self): - self.m_readurl.return_value = OPC_VM_SECONDARY_VNIC_RESPONSE - mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' - self.m_get_interfaces_by_mac.return_value = { - mac_addr: nic_name, + def test_secondary_nic_v2(self, oracle_ds): + oracle_ds._vnics_data = json.loads(OPC_VM_SECONDARY_VNIC_RESPONSE) + oracle_ds._network_config = { + 'version': 2, 'ethernets': {'primary': {'nic': {}}} } - - network_config = {'version': 2, 'ethernets': {'primary': {'nic': {}}}} - oracle._add_network_config_from_opc_imds(network_config) + mac_addr, nic_name = '00:00:17:02:2b:b1', 'ens3' + with mock.patch(DS_PATH + ".get_interfaces_by_mac", + return_value={mac_addr: nic_name}): + oracle_ds._add_network_config_from_opc_imds() # The input is mutated - self.assertEqual(2, len(network_config['ethernets'])) + assert 2 == len(oracle_ds.network_config['ethernets']) - secondary_nic_cfg = network_config['ethernets']['ens3'] - self.assertFalse(secondary_nic_cfg['dhcp4']) - self.assertFalse(secondary_nic_cfg['dhcp6']) - self.assertEqual(mac_addr, secondary_nic_cfg['match']['macaddress']) - self.assertEqual(9000, secondary_nic_cfg['mtu']) + secondary_nic_cfg = oracle_ds.network_config['ethernets']['ens3'] + assert secondary_nic_cfg['dhcp4'] is False + assert secondary_nic_cfg['dhcp6'] is False + assert mac_addr == secondary_nic_cfg['match']['macaddress'] + assert 9000 == secondary_nic_cfg['mtu'] - self.assertEqual(1, len(secondary_nic_cfg['addresses'])) + assert 1 == len(secondary_nic_cfg['addresses']) # These values are hard-coded in OPC_VM_SECONDARY_VNIC_RESPONSE - self.assertEqual('10.0.0.231', secondary_nic_cfg['addresses'][0]) + assert '10.0.0.231' == secondary_nic_cfg['addresses'][0] class TestNetworkConfigFiltersNetFailover(test_helpers.CiTestCase): @@ -412,42 +408,103 @@ class TestNetworkConfigFiltersNetFailover(test_helpers.CiTestCase): self.assertEqual(expected_cfg, netcfg) +def _mock_v2_urls(httpretty): + def instance_callback(request, uri, response_headers): + print(response_headers) + assert request.headers.get("Authorization") == "Bearer Oracle" + return [200, response_headers, OPC_V2_METADATA] + + def vnics_callback(request, uri, response_headers): + assert request.headers.get("Authorization") == "Bearer Oracle" + return [200, response_headers, OPC_BM_SECONDARY_VNIC_RESPONSE] + + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v2/instance/", + body=instance_callback + ) + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v2/vnics/", + body=vnics_callback + ) + + +def _mock_no_v2_urls(httpretty): + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v2/instance/", + status=404, + ) + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v1/instance/", + body=OPC_V1_METADATA + ) + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v1/vnics/", + body=OPC_BM_SECONDARY_VNIC_RESPONSE + ) + + class TestReadOpcMetadata: # See https://docs.pytest.org/en/stable/example # /parametrize.html#parametrizing-conditional-raising does_not_raise = ExitStack - @pytest.fixture(autouse=True) - def configure_opc_metadata_in_httpretty(self, httpretty): - """Configure HTTPretty with the various OPC metadata endpoints.""" - httpretty.register_uri( - httpretty.GET, - "http://169.254.169.254/opc/v1/instance/", - OPC_V1_METADATA, - ) + @mock.patch("cloudinit.url_helper.time.sleep", lambda _: None) + @pytest.mark.parametrize( + 'version,setup_urls,instance_data,fetch_vnics,vnics_data', [ + (2, _mock_v2_urls, json.loads(OPC_V2_METADATA), True, + json.loads(OPC_BM_SECONDARY_VNIC_RESPONSE)), + (2, _mock_v2_urls, json.loads(OPC_V2_METADATA), False, None), + (1, _mock_no_v2_urls, json.loads(OPC_V1_METADATA), True, + json.loads(OPC_BM_SECONDARY_VNIC_RESPONSE)), + (1, _mock_no_v2_urls, json.loads(OPC_V1_METADATA), False, None), + ] + ) + def test_metadata_returned( + self, version, setup_urls, instance_data, + fetch_vnics, vnics_data, httpretty + ): + setup_urls(httpretty) + metadata = oracle.read_opc_metadata(fetch_vnics_data=fetch_vnics) - def test_json_decoded_value_returned(self): - # read_opc_metadata should JSON decode the response and return it - expected = json.loads(OPC_V1_METADATA) - assert expected == oracle.read_opc_metadata() + assert version == metadata.version + assert instance_data == metadata.instance_data + assert vnics_data == metadata.vnics_data # No need to actually wait between retries in the tests @mock.patch("cloudinit.url_helper.time.sleep", lambda _: None) @pytest.mark.parametrize( - "failure_count,expectation", - [(1, does_not_raise()), (2, pytest.raises(UrlError))], + "v2_failure_count,v1_failure_count,expected_body,expectation", + [ + (1, 0, json.loads(OPC_V2_METADATA), does_not_raise()), + (2, 0, json.loads(OPC_V1_METADATA), does_not_raise()), + (2, 1, json.loads(OPC_V1_METADATA), does_not_raise()), + (2, 2, None, pytest.raises(UrlError)), + ] ) - def test_retries(self, expectation, failure_count, httpretty): - responses = [httpretty.Response("", status=404)] * failure_count - responses.append(httpretty.Response(OPC_V1_METADATA)) + def test_retries(self, v2_failure_count, v1_failure_count, + expected_body, expectation, httpretty): + v2_responses = [httpretty.Response("", status=404)] * v2_failure_count + v2_responses.append(httpretty.Response(OPC_V2_METADATA)) + v1_responses = [httpretty.Response("", status=404)] * v1_failure_count + v1_responses.append(httpretty.Response(OPC_V1_METADATA)) + httpretty.register_uri( httpretty.GET, "http://169.254.169.254/opc/v1/instance/", - responses=responses, + responses=v1_responses, + ) + httpretty.register_uri( + httpretty.GET, + "http://169.254.169.254/opc/v2/instance/", + responses=v2_responses, ) - expected = json.loads(OPC_V1_METADATA) with expectation: - assert expected == oracle.read_opc_metadata() + assert expected_body == oracle.read_opc_metadata().instance_data class TestCommon_GetDataBehaviour: @@ -516,7 +573,7 @@ class TestCommon_GetDataBehaviour: @pytest.mark.parametrize( "attribute_name,expected_value", [ - ("_crawled_metadata", json.loads(OPC_V1_METADATA)), + ("_crawled_metadata", json.loads(OPC_V2_METADATA)), ( "userdata_raw", base64.b64decode(b"IyEvYmluL3NoCnRvdWNoIC90bXAvZm9v"), @@ -554,11 +611,12 @@ class TestCommon_GetDataBehaviour: def test_public_keys_handled_correctly( self, ssh_keys, expected_value, parameterized_oracle_ds ): - metadata = json.loads(OPC_V1_METADATA) + instance_data = json.loads(OPC_V1_METADATA) if ssh_keys is None: - del metadata["metadata"]["ssh_authorized_keys"] + del instance_data["metadata"]["ssh_authorized_keys"] else: - metadata["metadata"]["ssh_authorized_keys"] = ssh_keys + instance_data["metadata"]["ssh_authorized_keys"] = ssh_keys + metadata = OpcMetadata(None, instance_data, None) with mock.patch( DS_PATH + ".read_opc_metadata", mock.Mock(return_value=metadata), ): @@ -570,8 +628,9 @@ class TestCommon_GetDataBehaviour: def test_missing_user_data_handled_gracefully( self, parameterized_oracle_ds ): - metadata = json.loads(OPC_V1_METADATA) - del metadata["metadata"]["user_data"] + instance_data = json.loads(OPC_V1_METADATA) + del instance_data["metadata"]["user_data"] + metadata = OpcMetadata(None, instance_data, None) with mock.patch( DS_PATH + ".read_opc_metadata", mock.Mock(return_value=metadata), ): @@ -582,8 +641,9 @@ class TestCommon_GetDataBehaviour: def test_missing_metadata_handled_gracefully( self, parameterized_oracle_ds ): - metadata = json.loads(OPC_V1_METADATA) - del metadata["metadata"] + instance_data = json.loads(OPC_V1_METADATA) + del instance_data["metadata"] + metadata = OpcMetadata(None, instance_data, None) with mock.patch( DS_PATH + ".read_opc_metadata", mock.Mock(return_value=metadata), ): @@ -617,7 +677,7 @@ class TestNonIscsiRoot_GetDataBehaviour: exit_context_manager ) - def assert_in_context_manager(): + def assert_in_context_manager(**kwargs): assert in_context_manager return mock.MagicMock() @@ -666,10 +726,8 @@ class TestNetworkConfig: "configure_secondary_nics,expect_secondary_nics", [(True, True), (False, False), (None, False)], ) - @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") def test_secondary_nic_addition( self, - m_add_network_config_from_opc_imds, m_read_initramfs_config, configure_secondary_nics, expect_secondary_nics, @@ -681,33 +739,38 @@ class TestNetworkConfig: """ m_read_initramfs_config.return_value = {"version": 1, "config": []} - def side_effect(network_config): - network_config["secondary_added"] = mock.sentinel.needle - - m_add_network_config_from_opc_imds.side_effect = side_effect - if configure_secondary_nics is not None: oracle_ds.ds_cfg[ "configure_secondary_nics" ] = configure_secondary_nics - was_secondary_added = "secondary_added" in oracle_ds.network_config + def side_effect(self): + self._network_config["secondary_added"] = mock.sentinel.needle + + oracle_ds._vnics_data = 'DummyData' + with mock.patch.object( + oracle.DataSourceOracle, "_add_network_config_from_opc_imds", + new=side_effect, + ): + was_secondary_added = "secondary_added" in oracle_ds.network_config assert expect_secondary_nics == was_secondary_added - @mock.patch(DS_PATH + "._add_network_config_from_opc_imds") def test_secondary_nic_failure_isnt_blocking( self, - m_add_network_config_from_opc_imds, m_read_initramfs_config, caplog, oracle_ds, ): - m_add_network_config_from_opc_imds.side_effect = Exception() - oracle_ds.ds_cfg["configure_secondary_nics"] = True + oracle_ds._vnics_data = "DummyData" - assert m_read_initramfs_config.return_value == oracle_ds.network_config - assert "Failed to fetch secondary network configuration" in caplog.text + with mock.patch.object( + oracle.DataSourceOracle, "_add_network_config_from_opc_imds", + side_effect=Exception() + ): + network_config = oracle_ds.network_config + assert network_config == m_read_initramfs_config.return_value + assert "Failed to parse secondary network configuration" in caplog.text def test_ds_network_cfg_preferred_over_initramfs(self, _m): """Ensure that DS net config is preferred over initramfs config""" -- cgit v1.2.3 From 2d3533b59c7bf00affbda9c2c94fb5f214ffcb11 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Fri, 14 Aug 2020 11:37:33 -0400 Subject: DataSourceOracle: retry twice (and document why we retry at all) (#536) --- cloudinit/sources/DataSourceOracle.py | 5 ++++- cloudinit/sources/tests/test_oracle.py | 8 +++++--- 2 files changed, 9 insertions(+), 4 deletions(-) (limited to 'cloudinit/sources/tests') diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index 2354648b..20d6487d 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -297,7 +297,10 @@ def read_opc_metadata(*, fetch_vnics_data: bool = False): `fetch_vnics_data` is True, else None """ - retries = 1 + # Per Oracle, there are short windows (measured in milliseconds) throughout + # an instance's lifetime where the IMDS is being updated and may 404 as a + # result. To work around these windows, we retry a couple of times. + retries = 2 def _fetch(metadata_version: int, path: str) -> dict: headers = { diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index 2fb9a20a..7bd23813 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -481,9 +481,11 @@ class TestReadOpcMetadata: "v2_failure_count,v1_failure_count,expected_body,expectation", [ (1, 0, json.loads(OPC_V2_METADATA), does_not_raise()), - (2, 0, json.loads(OPC_V1_METADATA), does_not_raise()), - (2, 1, json.loads(OPC_V1_METADATA), does_not_raise()), - (2, 2, None, pytest.raises(UrlError)), + (2, 0, json.loads(OPC_V2_METADATA), does_not_raise()), + (3, 0, json.loads(OPC_V1_METADATA), does_not_raise()), + (3, 1, json.loads(OPC_V1_METADATA), does_not_raise()), + (3, 2, json.loads(OPC_V1_METADATA), does_not_raise()), + (3, 3, None, pytest.raises(UrlError)), ] ) def test_retries(self, v2_failure_count, v1_failure_count, -- cgit v1.2.3