From 986f37b017134ced5d9dd38b420350916297002b Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 10 Mar 2020 13:26:05 -0400 Subject: cloudinit: move to pytest for running tests (#211) As the nose docs[0] themselves note, it has been in maintenance mode for the past several years. pytest is an actively developed, featureful and popular alternative that the nose docs themselves recommend. See [1] for more details about the thinking here. (This PR also removes stale tox definitions, instead of modifying them.) [0] https://nose.readthedocs.io/en/latest/ [1] https://lists.launchpad.net/cloud-init/msg00245.html --- tox.ini | 73 ++++++++++++++++++++++++++--------------------------------------- 1 file changed, 29 insertions(+), 44 deletions(-) (limited to 'tox.ini') diff --git a/tox.ini b/tox.ini index 8612f034..8c994a05 100644 --- a/tox.ini +++ b/tox.ini @@ -1,13 +1,13 @@ [tox] -envlist = py3, xenial, pycodestyle, pyflakes, pylint +envlist = py3, xenial-dev, pycodestyle, pyflakes, pylint recreate = True [testenv] -commands = python -m nose {posargs:tests/unittests cloudinit} +commands = {envpython} -m pytest {posargs:tests/unittests cloudinit} setenv = LC_ALL = en_US.utf-8 passenv= - NOSE_VERBOSE + PYTEST_ADDOPTS [testenv:pycodestyle] basepython = python3 @@ -32,23 +32,16 @@ commands = {envpython} -m pylint {posargs:cloudinit tests tools} [testenv:py3] basepython = python3 deps = - nose-timer -r{toxinidir}/test-requirements.txt -commands = {envpython} -m nose --with-timer --timer-top-n 10 \ - {posargs:--with-coverage --cover-erase --cover-branches \ - --cover-inclusive --cover-package=cloudinit \ +commands = {envpython} -m pytest \ + --durations 10 \ + {posargs:--cov=cloudinit --cov-branch \ tests/unittests cloudinit} [testenv:py27] basepython = python2.7 deps = -r{toxinidir}/test-requirements.txt -[testenv:py26] -deps = -r{toxinidir}/test-requirements.txt -commands = nosetests {posargs:tests/unittests cloudinit} -setenv = - LC_ALL = C - [flake8] #H102 Apache 2.0 license header not found ignore=H404,H405,H105,H301,H104,H403,H101,H102,H106,H304 @@ -62,11 +55,15 @@ commands = {envpython} -m sphinx {posargs:doc/rtd doc/rtd_html} doc8 doc/rtd -[testenv:xenial] -commands = - python ./tools/pipremove jsonschema - python -m nose {posargs:tests/unittests cloudinit} -basepython = python3 +[xenial-shared-deps] +# The version of pytest in xenial doesn't work with Python 3.8, so we define +# two xenial environments: [testenv:xenial] runs the tests with exactly the +# version of pytest present in xenial, and is used in CI. [testenv:xenial-dev] +# runs the tests with the lowest version of pytest that works with Python 3.8, +# 3.0.7, but keeps the other dependencies at xenial's level. +# +# (This section is not a testenv, it is used to maintain a single definition of +# the dependencies shared between the two xenial testenvs.) deps = # requirements jinja2==2.8 @@ -83,38 +80,26 @@ deps = # test-requirements httpretty==0.9.6 mock==1.3.0 - nose==1.3.7 unittest2==1.1.0 contextlib2==0.5.1 -[testenv:centos6] -basepython = python2.6 -commands = nosetests {posargs:tests/unittests cloudinit} +[testenv:xenial] +commands = + python ./tools/pipremove jsonschema + python -m pytest {posargs:tests/unittests cloudinit} +basepython = python3 deps = - # requirements - argparse==1.2.1 - jinja2==2.2.1 - pyyaml==3.10 - oauthlib==0.6.0 - configobj==4.6.0 - requests==2.6.0 - jsonpatch==1.2 - six==1.9.0 - -r{toxinidir}/test-requirements.txt + # Refer to the comment in [xenial-shared-deps] for details + {[xenial-shared-deps]deps} + pytest==2.8.7 -[testenv:opensusel150] -basepython = python2.7 -commands = nosetests {posargs:tests/unittests cloudinit} +[testenv:xenial-dev] +commands = {[testenv:xenial]commands} +basepython = {[testenv:xenial]basepython} deps = - # requirements - jinja2==2.10 - PyYAML==3.12 - oauthlib==2.0.6 - configobj==5.0.6 - requests==2.18.4 - jsonpatch==1.16 - six==1.11.0 - -r{toxinidir}/test-requirements.txt + # Refer to the comment in [xenial-shared-deps] for details + {[xenial-shared-deps]deps} + pytest==3.0.7 [testenv:tip-pycodestyle] commands = {envpython} -m pycodestyle {posargs:cloudinit/ tests/ tools/} -- cgit v1.2.3 From 64c33704853087c05c54e157fc42a340a4350159 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 10 Mar 2020 14:40:27 -0400 Subject: tox.ini: bump pyflakes version to 2.1.1 (#239) pyflakes versions older than 2.1.0 are incompatible with Python 3.8 (which is the Python version in the current Ubuntu development release). See https://github.com/PyCQA/pyflakes/issues/367 for details. 2.1.1 is the latest version ATM, so bump to that. --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tox.ini') diff --git a/tox.ini b/tox.ini index 8c994a05..0f9db826 100644 --- a/tox.ini +++ b/tox.ini @@ -108,7 +108,7 @@ deps = pycodestyle [testenv:pyflakes] commands = {envpython} -m pyflakes {posargs:cloudinit/ tests/ tools/} deps = - pyflakes==1.6.0 + pyflakes==2.1.1 [testenv:tip-pyflakes] commands = {envpython} -m pyflakes {posargs:cloudinit/ tests/ tools/} -- cgit v1.2.3 From 9b2e42c5cdf458de1cd7c08e554511ea04873d55 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 10 Mar 2020 22:49:52 -0400 Subject: tox.ini: use xenial version of jsonpatch in CI (#242) Now that we can distinguish between CI xenial dependencies and needed-to-run-on-dev-machine xenial depedencies, we can return to testing with the correct jsonpatch version. --- tox.ini | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'tox.ini') diff --git a/tox.ini b/tox.ini index 0f9db826..d17f493d 100644 --- a/tox.ini +++ b/tox.ini @@ -72,10 +72,6 @@ deps = pyserial==3.0.1 configobj==5.0.6 requests==2.9.1 - # jsonpatch in xenial is 1.10, not 1.19 (#839779). The oldest version - # to work with python3.6 is 1.16 as found in Artful. To keep default - # invocation of 'tox' happy, accept the difference in version here. - jsonpatch==1.16 six==1.10.0 # test-requirements httpretty==0.9.6 @@ -91,6 +87,7 @@ basepython = python3 deps = # Refer to the comment in [xenial-shared-deps] for details {[xenial-shared-deps]deps} + jsonpatch==1.10 pytest==2.8.7 [testenv:xenial-dev] @@ -99,6 +96,10 @@ basepython = {[testenv:xenial]basepython} deps = # Refer to the comment in [xenial-shared-deps] for details {[xenial-shared-deps]deps} + # jsonpatch in xenial is 1.10, not 1.19 (#839779). The oldest version + # to work with python3.6 is 1.16 as found in Artful. To keep default + # invocation of 'tox' happy, accept the difference in version here. + jsonpatch==1.16 pytest==3.0.7 [testenv:tip-pycodestyle] -- cgit v1.2.3 From 65a1b907c336786bce3917fad3f87c67f0caa7bf Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Wed, 11 Mar 2020 14:27:18 -0400 Subject: tox.ini: avoid substition syntax that causes a traceback on xenial (#245) See the added comment for details. --- tox.ini | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'tox.ini') diff --git a/tox.ini b/tox.ini index d17f493d..7591d9f3 100644 --- a/tox.ini +++ b/tox.ini @@ -80,6 +80,8 @@ deps = contextlib2==0.5.1 [testenv:xenial] +# When updating this commands definition, also update the definition in +# [testenv:xenial-dev]. See the comment there for details. commands = python ./tools/pipremove jsonschema python -m pytest {posargs:tests/unittests cloudinit} @@ -91,7 +93,15 @@ deps = pytest==2.8.7 [testenv:xenial-dev] -commands = {[testenv:xenial]commands} +# This should be: +# commands = {[testenv:xenial]commands} +# but the version of pytest in xenial has a bug +# (https://github.com/tox-dev/tox/issues/208) which means that the {posargs} +# substitution variable is misparsed and causes a traceback. Ensure that any +# changes here are reflected in [testenv:xenial]. +commands = + python ./tools/pipremove jsonschema + python -m pytest {posargs:tests/unittests cloudinit} basepython = {[testenv:xenial]basepython} deps = # Refer to the comment in [xenial-shared-deps] for details -- cgit v1.2.3 From b9ff0dc950558ecd2a8469eded26bd6ae4082771 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 17 Mar 2020 15:48:51 -0400 Subject: cloudinit: remove six from packaging/tooling (#253) --- .pylintrc | 1 - packages/pkg-deps.json | 3 --- requirements.txt | 3 --- tools/build-on-freebsd | 1 - tools/build-on-netbsd | 1 - tox.ini | 1 - 6 files changed, 10 deletions(-) (limited to 'tox.ini') diff --git a/.pylintrc b/.pylintrc index c83546a6..4d5d066d 100644 --- a/.pylintrc +++ b/.pylintrc @@ -50,7 +50,6 @@ ignored-modules= http.client, httplib, pkg_resources, - six.moves, # cloud_tests requirements. boto3, botocore, diff --git a/packages/pkg-deps.json b/packages/pkg-deps.json index 64d6d78d..d09ec33f 100644 --- a/packages/pkg-deps.json +++ b/packages/pkg-deps.json @@ -50,9 +50,6 @@ }, "requests" : { "3" : "python36-requests" - }, - "six" : { - "3" : "python36-six" } }, "requires" : [ diff --git a/requirements.txt b/requirements.txt index dd10d85d..5817da3b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,6 +32,3 @@ jsonpatch # For validating cloud-config sections per schema definitions jsonschema - -# For Python 2/3 compatibility -six diff --git a/tools/build-on-freebsd b/tools/build-on-freebsd index 876368a9..60beeee0 100755 --- a/tools/build-on-freebsd +++ b/tools/build-on-freebsd @@ -29,7 +29,6 @@ pkgs=" $py_prefix-oauthlib $py_prefix-requests $py_prefix-serial - $py_prefix-six $py_prefix-yaml sudo " diff --git a/tools/build-on-netbsd b/tools/build-on-netbsd index 1e2b9cff..d2a7067d 100755 --- a/tools/build-on-netbsd +++ b/tools/build-on-netbsd @@ -12,7 +12,6 @@ pkgs=" py37-oauthlib py37-requests py37-setuptools - py37-six py37-yaml sudo " diff --git a/tox.ini b/tox.ini index 7591d9f3..416ddcb8 100644 --- a/tox.ini +++ b/tox.ini @@ -72,7 +72,6 @@ deps = pyserial==3.0.1 configobj==5.0.6 requests==2.9.1 - six==1.10.0 # test-requirements httpretty==0.9.6 mock==1.3.0 -- 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 'tox.ini') 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 089408751cdbd7950f58616f9d85ff9dfd9aa3c7 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 14 May 2020 09:27:44 -0400 Subject: cloud_tests: emit dots on Travis while fetching images (#347) This ensures that Travis will not kill our tests if fetching images is taking a long time. In implementation terms, this introduces a context manager which will spin up a multiprocessing.Process in the background and print a dot to stdout every 10 seconds. The process is terminated when the context manager exits. This also drop the use of travis_wait, which was being used to work around this issue. --- .travis.yml | 2 +- tests/cloud_tests/platforms/__init__.py | 4 +++- tests/cloud_tests/util.py | 33 +++++++++++++++++++++++++++++++++ tox.ini | 2 +- 4 files changed, 38 insertions(+), 3 deletions(-) (limited to 'tox.ini') diff --git a/.travis.yml b/.travis.yml index 32ad4415..c67c10bc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -72,7 +72,7 @@ matrix: - sudo -E su $USER -c 'mk-sbuild xenial' - sudo -E su $USER -c 'sbuild --nolog --no-run-lintian --verbose --dist=xenial cloud-init_*.dsc' # Ubuntu LTS: Integration - - travis_wait 30 sg lxd -c 'tox -e citest -- run --verbose --preserve-data --data-dir results --os-name xenial --test modules/apt_configure_sources_list.yaml --test modules/ntp_servers --test modules/set_password_list --test modules/user_groups --deb cloud-init_*_all.deb' + - sg lxd -c 'tox -e citest -- run --verbose --preserve-data --data-dir results --os-name xenial --test modules/apt_configure_sources_list.yaml --test modules/ntp_servers --test modules/set_password_list --test modules/user_groups --deb cloud-init_*_all.deb' - python: 3.5 env: TOXENV=xenial diff --git a/tests/cloud_tests/platforms/__init__.py b/tests/cloud_tests/platforms/__init__.py index 6a410b84..e506baa0 100644 --- a/tests/cloud_tests/platforms/__init__.py +++ b/tests/cloud_tests/platforms/__init__.py @@ -6,6 +6,7 @@ from .ec2 import platform as ec2 from .lxd import platform as lxd from .nocloudkvm import platform as nocloudkvm from .azurecloud import platform as azurecloud +from ..util import emit_dots_on_travis PLATFORMS = { 'ec2': ec2.EC2Platform, @@ -17,7 +18,8 @@ PLATFORMS = { def get_image(platform, config): """Get image from platform object using os_name.""" - return platform.get_image(config) + with emit_dots_on_travis(): + return platform.get_image(config) def get_instance(snapshot, *args, **kwargs): diff --git a/tests/cloud_tests/util.py b/tests/cloud_tests/util.py index 06f7d865..e65771b1 100644 --- a/tests/cloud_tests/util.py +++ b/tests/cloud_tests/util.py @@ -5,6 +5,7 @@ import base64 import copy import glob +import multiprocessing import os import random import shlex @@ -12,7 +13,9 @@ import shutil import string import subprocess import tempfile +import time import yaml +from contextlib import contextmanager from cloudinit import util as c_util from tests.cloud_tests import LOG @@ -118,6 +121,36 @@ def current_verbosity(): return max(min(3 - int(LOG.level / 10), 2), 0) +@contextmanager +def emit_dots_on_travis(): + """ + A context manager that emits a dot every 10 seconds if running on Travis. + + Travis will kill jobs that don't emit output for a certain amount of time. + This context manager spins up a background process which will emit a dot to + stdout every 10 seconds to avoid being killed. + + It should be wrapped selectively around operations that are known to take a + long time. + """ + if os.environ.get('TRAVIS') != "true": + # If we aren't on Travis, don't do anything. + yield + return + + def emit_dots(): + while True: + print(".") + time.sleep(10) + + dot_process = multiprocessing.Process(target=emit_dots) + dot_process.start() + try: + yield + finally: + dot_process.terminate() + + def is_writable_dir(path): """Make sure dir is writable. diff --git a/tox.ini b/tox.ini index 906519c4..95a8511f 100644 --- a/tox.ini +++ b/tox.ini @@ -134,6 +134,6 @@ deps = [testenv:citest] basepython = python3 commands = {envpython} -m tests.cloud_tests {posargs} -passenv = HOME +passenv = HOME TRAVIS deps = -r{toxinidir}/integration-requirements.txt -- cgit v1.2.3 From a99c8bcea597c17d83d90bf1e09c9a5b68c3dc22 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Wed, 27 May 2020 14:28:25 -0700 Subject: testing: use flake8 again (#392) Instead of running pycodestyle and pyflakes seperately, use flake8 to get the benefits of pyflakes and also stylistic checks as well as the ability to configure the settings for the project. --- .travis.yml | 4 +--- tox.ini | 35 +++++++++++++++++------------------ 2 files changed, 18 insertions(+), 21 deletions(-) (limited to 'tox.ini') diff --git a/.travis.yml b/.travis.yml index 3de1066b..84f38533 100644 --- a/.travis.yml +++ b/.travis.yml @@ -79,8 +79,6 @@ matrix: PYTEST_ADDOPTS=-v # List all tests run by pytest dist: xenial - python: 3.6 - env: TOXENV=pycodestyle - - python: 3.6 - env: TOXENV=pyflakes + env: TOXENV=flake8 - python: 3.6 env: TOXENV=pylint diff --git a/tox.ini b/tox.ini index 95a8511f..611c3111 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py3, xenial-dev, pycodestyle, pyflakes, pylint +envlist = py3, xenial-dev, flake8, pylint recreate = True [testenv] @@ -9,11 +9,11 @@ setenv = passenv= PYTEST_ADDOPTS -[testenv:pycodestyle] +[testenv:flake8] basepython = python3 deps = - pycodestyle==2.4.0 -commands = {envpython} -m pycodestyle {posargs:cloudinit/ tests/ tools/} + flake8==3.8.2 +commands = {envpython} -m flake8 {posargs:cloudinit/ tests/ tools/} # https://github.com/gabrielfalcao/HTTPretty/issues/223 setenv = @@ -43,8 +43,16 @@ basepython = python2.7 deps = -r{toxinidir}/test-requirements.txt [flake8] -#H102 Apache 2.0 license header not found -ignore=H404,H405,H105,H301,H104,H403,H101,H102,H106,H304 +# E121: continuation line under-indented for hanging indent +# E123: closing bracket does not match indentation of opening bracket’s line +# E126: continuation line over-indented for hanging indent +# E226: missing whitespace around arithmetic operator +# E241: multiple spaces after ‘,’ +# E402: module level import not at top of file +# E741: do not use variables named ‘l’, ‘O’, or ‘I’ +# W503: line break before binary operator +# W504: line break after binary operator +ignore=E121,E123,E126,E226,E241,E402,E741,W503,W504 exclude = .venv,.tox,dist,doc,*egg,.git,build,tools [testenv:doc] @@ -109,18 +117,9 @@ deps = jsonpatch==1.16 pytest==3.0.7 -[testenv:tip-pycodestyle] -commands = {envpython} -m pycodestyle {posargs:cloudinit/ tests/ tools/} -deps = pycodestyle - -[testenv:pyflakes] -commands = {envpython} -m pyflakes {posargs:cloudinit/ tests/ tools/} -deps = - pyflakes==2.1.1 - -[testenv:tip-pyflakes] -commands = {envpython} -m pyflakes {posargs:cloudinit/ tests/ tools/} -deps = pyflakes +[testenv:tip-flake8] +commands = {envpython} -m flake8 {posargs:cloudinit/ tests/ tools/} +deps = flake8 [testenv:tip-pylint] commands = {envpython} -m pylint {posargs:cloudinit tests tools} -- cgit v1.2.3 From 4ab3303ec4bb49f029b7821d6dba53a6b02b6dc1 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Mon, 1 Jun 2020 14:20:39 -0700 Subject: test: fix all flake8 E741 errors (#401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This removes the use of variables named ‘l’, ‘O’, or ‘I’. Generally these are used in list comprehension to read the line of lines. --- cloudinit/distros/__init__.py | 2 +- cloudinit/net/__init__.py | 8 ++++---- cloudinit/sources/DataSourceOpenNebula.py | 6 +++--- cloudinit/sources/helpers/openstack.py | 17 +++++++++++------ cloudinit/ssh_util.py | 4 +++- tests/cloud_tests/platforms/instances.py | 4 ++-- tests/cloud_tests/testcases/base.py | 4 ++-- tests/unittests/test_datasource/test_ec2.py | 4 +++- tests/unittests/test_datasource/test_ovf.py | 2 +- tests/unittests/test_ds_identify.py | 6 ++++-- .../test_handler/test_handler_apt_source_v3.py | 4 +++- tests/unittests/test_sshutil.py | 2 +- tox.ini | 3 +-- 13 files changed, 39 insertions(+), 27 deletions(-) (limited to 'tox.ini') diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index e99529df..35a10590 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -582,7 +582,7 @@ class Distro(metaclass=abc.ABCMeta): # passwd must use short '-l' due to SLES11 lacking long form '--lock' lock_tools = (['passwd', '-l', name], ['usermod', '--lock', name]) try: - cmd = next(l for l in lock_tools if util.which(l[0])) + cmd = next(tool for tool in lock_tools if util.which(tool[0])) except StopIteration: raise RuntimeError(( "Unable to lock user account '%s'. No tools available. " diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index cb8c1601..8af24fa9 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -824,13 +824,13 @@ def get_interfaces_by_mac_on_freebsd(): # flatten each interface block in a single line def flatten(out): curr_block = '' - for l in out.split('\n'): - if l.startswith('\t'): - curr_block += l + for line in out.split('\n'): + if line.startswith('\t'): + curr_block += line else: if curr_block: yield curr_block - curr_block = l + curr_block = line yield curr_block # looks for interface and mac in a list of flatten block diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py index 02c9a7b8..a08ab404 100644 --- a/cloudinit/sources/DataSourceOpenNebula.py +++ b/cloudinit/sources/DataSourceOpenNebula.py @@ -417,9 +417,9 @@ def read_context_disk_dir(source_dir, asuser=None): if ssh_key_var: lines = context.get(ssh_key_var).splitlines() - results['metadata']['public-keys'] = [l for l in lines - if len(l) and not - l.startswith("#")] + results['metadata']['public-keys'] = [ + line for line in lines if len(line) and not line.startswith("#") + ] # custom hostname -- try hostname or leave cloud-init # itself create hostname from IP address later diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index e91398ea..a4373f24 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -411,8 +411,11 @@ class ConfigDriveReader(BaseReader): keydata = meta_js.get('public-keys', keydata) if keydata: lines = keydata.splitlines() - md['public-keys'] = [l for l in lines - if len(l) and not l.startswith("#")] + md['public-keys'] = [ + line + for line in lines + if len(line) and not line.startswith("#") + ] # config-drive-v1 has no way for openstack to provide the instance-id # so we copy that into metadata from the user input @@ -674,11 +677,13 @@ def convert_net_json(network_json=None, known_macs=None): raise ValueError("Unable to find a system nic for %s" % d) d['name'] = known_macs[mac] - for cfg, key, fmt, target in link_updates: - if isinstance(target, (list, tuple)): - cfg[key] = [fmt % link_id_info[l]['name'] for l in target] + for cfg, key, fmt, targets in link_updates: + if isinstance(targets, (list, tuple)): + cfg[key] = [ + fmt % link_id_info[target]['name'] for target in targets + ] else: - cfg[key] = fmt % link_id_info[target]['name'] + cfg[key] = fmt % link_id_info[targets]['name'] # Infiniband interfaces may be referenced in network_data.json by a 6 byte # Ethernet MAC-style address, and we use that address to look up the diff --git a/cloudinit/ssh_util.py b/cloudinit/ssh_util.py index c3a9b5b7..918c4aec 100644 --- a/cloudinit/ssh_util.py +++ b/cloudinit/ssh_util.py @@ -344,7 +344,9 @@ def update_ssh_config(updates, fname=DEF_SSHD_CFG): changed = update_ssh_config_lines(lines=lines, updates=updates) if changed: util.write_file( - fname, "\n".join([str(l) for l in lines]) + "\n", copy_mode=True) + fname, "\n".join( + [str(line) for line in lines] + ) + "\n", copy_mode=True) return len(changed) != 0 diff --git a/tests/cloud_tests/platforms/instances.py b/tests/cloud_tests/platforms/instances.py index 529e79cd..efc35c7f 100644 --- a/tests/cloud_tests/platforms/instances.py +++ b/tests/cloud_tests/platforms/instances.py @@ -132,8 +132,8 @@ class Instance(TargetBase): """ def clean_test(test): """Clean formatting for system ready test testcase.""" - return ' '.join(l for l in test.strip().splitlines() - if not l.lstrip().startswith('#')) + return ' '.join(line for line in test.strip().splitlines() + if not line.lstrip().startswith('#')) boot_timeout = self.config['boot_timeout'] tests = [self.config['system_ready_script']] diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index 7b67f54e..68d59111 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -141,8 +141,8 @@ class CloudTestCase(unittest.TestCase): def test_no_warnings_in_log(self): """Unexpected warnings should not be found in the log.""" warnings = [ - l for l in self.get_data_file('cloud-init.log').splitlines() - if 'WARN' in l] + line for line in self.get_data_file('cloud-init.log').splitlines() + if 'WARN' in line] joined_warnings = '\n'.join(warnings) for expected_warning in self.expected_warnings: self.assertIn( diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index d413d1cd..ad1ea595 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -612,7 +612,9 @@ class TestEc2(test_helpers.HttprettyTestCase): for log in expected_logs: self.assertIn(log, logs) self.assertEqual( - 1, len([l for l in logs.splitlines() if failed_put_log in l])) + 1, + len([line for line in logs.splitlines() if failed_put_log in line]) + ) def test_aws_token_redacted(self): """Verify that aws tokens are redacted when logged.""" diff --git a/tests/unittests/test_datasource/test_ovf.py b/tests/unittests/test_datasource/test_ovf.py index a19c35c8..486a2345 100644 --- a/tests/unittests/test_datasource/test_ovf.py +++ b/tests/unittests/test_datasource/test_ovf.py @@ -48,7 +48,7 @@ def fill_properties(props, template=OVF_ENV_CONTENT): for key, val in props.items(): lines.append(prop_tmpl.format(key=key, val=val)) indent = " " - properties = ''.join([indent + l + "\n" for l in lines]) + properties = ''.join([indent + line + "\n" for line in lines]) return template.format(properties=properties) diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index f0e96b44..c2318570 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -611,8 +611,10 @@ class TestDsIdentify(DsIdentifyBase): ret = self._check_via_dict( cust, RC_FOUND, func=".", args=[os.path.join(rootd, mpp)], rootd=rootd) - line = [l for l in ret.stdout.splitlines() if l.startswith(pre)][0] - toks = line.replace(pre, "").split(":") + match = [ + line for line in ret.stdout.splitlines() if line.startswith(pre) + ][0] + toks = match.replace(pre, "").split(":") expected = ["/sbin", "/bin", "/usr/sbin", "/usr/bin", "/mycust/path"] self.assertEqual(expected, [p for p in expected if p in toks], "path did not have expected tokens") diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py index 4762dbef..aefe26c4 100644 --- a/tests/unittests/test_handler/test_handler_apt_source_v3.py +++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py @@ -1033,7 +1033,9 @@ class TestDebconfSelections(TestCase): # assumes called with *args value. selections = m_set_sel.call_args_list[0][0][0].decode() - missing = [l for l in lines if l not in selections.splitlines()] + missing = [ + line for line in lines if line not in selections.splitlines() + ] self.assertEqual([], missing) @mock.patch("cloudinit.config.cc_apt_configure.dpkg_reconfigure") diff --git a/tests/unittests/test_sshutil.py b/tests/unittests/test_sshutil.py index 0be41924..b4767f0c 100644 --- a/tests/unittests/test_sshutil.py +++ b/tests/unittests/test_sshutil.py @@ -299,7 +299,7 @@ class TestUpdateSshConfigLines(test_helpers.CiTestCase): lines = ssh_util.parse_ssh_config_lines(list(self.exlines)) result = ssh_util.update_ssh_config_lines(lines, updates) self.assertEqual([], result) - self.assertEqual(self.exlines, [str(l) for l in lines]) + self.assertEqual(self.exlines, [str(line) for line in lines]) def test_keycase_not_modified(self): """Original case of key should not be changed on update. diff --git a/tox.ini b/tox.ini index 611c3111..57b20568 100644 --- a/tox.ini +++ b/tox.ini @@ -49,10 +49,9 @@ deps = -r{toxinidir}/test-requirements.txt # E226: missing whitespace around arithmetic operator # E241: multiple spaces after ‘,’ # E402: module level import not at top of file -# E741: do not use variables named ‘l’, ‘O’, or ‘I’ # W503: line break before binary operator # W504: line break after binary operator -ignore=E121,E123,E126,E226,E241,E402,E741,W503,W504 +ignore=E121,E123,E126,E226,E241,E402,W503,W504 exclude = .venv,.tox,dist,doc,*egg,.git,build,tools [testenv:doc] -- cgit v1.2.3 From d2b05719a6163e874ca40234a0b85dc2fea260c0 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Tue, 2 Jun 2020 06:58:24 -0700 Subject: test: ignore flake8 E402 errors in main.py (#402) This puts an ignore on the imports not at the top of the file errors. The reason for the ignore instead of fix is that the file is using imp to grab a lock and patch logging before further imports are completed. --- tox.ini | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'tox.ini') diff --git a/tox.ini b/tox.ini index 57b20568..725a0ca4 100644 --- a/tox.ini +++ b/tox.ini @@ -48,11 +48,12 @@ deps = -r{toxinidir}/test-requirements.txt # E126: continuation line over-indented for hanging indent # E226: missing whitespace around arithmetic operator # E241: multiple spaces after ‘,’ -# E402: module level import not at top of file # W503: line break before binary operator # W504: line break after binary operator -ignore=E121,E123,E126,E226,E241,E402,W503,W504 +ignore=E121,E123,E126,E226,E241,W503,W504 exclude = .venv,.tox,dist,doc,*egg,.git,build,tools +per-file-ignores = + cloudinit/cmd/main.py:E402 [testenv:doc] basepython = python3 -- cgit v1.2.3 From 5f7825e22241423322dbe628de1b00289cf34114 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Tue, 2 Jun 2020 08:24:29 -0700 Subject: test: fix all flake8 E241 (#403) Remove extra spaces after a ',' --- cloudinit/cmd/tests/test_query.py | 2 +- cloudinit/config/cc_apt_configure.py | 2 +- cloudinit/net/tests/test_dhcp.py | 2 +- cloudinit/sources/DataSourceGCE.py | 2 +- cloudinit/tests/test_url_helper.py | 2 +- tests/unittests/test_net.py | 4 ++-- tox.ini | 3 +-- 7 files changed, 8 insertions(+), 9 deletions(-) (limited to 'tox.ini') diff --git a/cloudinit/cmd/tests/test_query.py b/cloudinit/cmd/tests/test_query.py index 6d36a4ea..cb15b2d2 100644 --- a/cloudinit/cmd/tests/test_query.py +++ b/cloudinit/cmd/tests/test_query.py @@ -261,7 +261,7 @@ class TestQuery(CiTestCase): args = self.args( debug=False, dump_all=False, format=None, instance_data=self.instance_data, list_keys=True, user_data='ud', - vendor_data='vd', varname='top') + vendor_data='vd', varname='top') with mock.patch('sys.stderr', new_callable=StringIO) as m_stderr: with mock.patch('sys.stdout', new_callable=StringIO) as m_stdout: with mock.patch('os.getuid') as m_getuid: diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 9a33451d..b1c7b471 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -297,7 +297,7 @@ schema = { }, 'conf': { 'type': 'string', - 'description': dedent("""\ + 'description': dedent("""\ Specify configuration for apt, such as proxy configuration. This configuration is specified as a string. For multiline apt configuration, make sure diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index bc7bef45..7768da7c 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -211,7 +211,7 @@ class TestDHCPParseStaticRoutes(CiTestCase): "class_b": "16,172,16,10", "class_a": "8,10,10", "gateway": "0,0", - "netlen": "33,0", + "netlen": "33,0", } for rfc3442 in bad_rfc3442.values(): self.assertEqual([], parse_static_routes(rfc3442)) diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py index 6cbfbbac..0ec5f6ec 100644 --- a/cloudinit/sources/DataSourceGCE.py +++ b/cloudinit/sources/DataSourceGCE.py @@ -116,7 +116,7 @@ def _write_host_key_to_guest_attributes(key_type, key_value): resp = url_helper.readurl(url=url, data=key_value, headers=HEADERS, request_method='PUT', check_status=False) if resp.ok(): - LOG.debug('Wrote %s host key to guest attributes.', key_type) + LOG.debug('Wrote %s host key to guest attributes.', key_type) else: LOG.debug('Unable to write %s host key to guest attributes.', key_type) diff --git a/cloudinit/tests/test_url_helper.py b/cloudinit/tests/test_url_helper.py index 29b39374..364ec822 100644 --- a/cloudinit/tests/test_url_helper.py +++ b/cloudinit/tests/test_url_helper.py @@ -85,7 +85,7 @@ class TestReadFileOrUrl(CiTestCase): read_file_or_url(url, headers=headers, headers_redact=['sensitive']) logs = self.logs.getvalue() for k in headers.keys(): - self.assertEqual(headers[k], httpretty.last_request().headers[k]) + self.assertEqual(headers[k], httpretty.last_request().headers[k]) self.assertIn(REDACTED, logs) self.assertNotIn('sekret', logs) diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index e075a64c..84d3a5f0 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -944,7 +944,7 @@ NETWORK_CONFIGS = { dhcp6: true """).rstrip(' '), 'expected_sysconfig_opensuse': { - 'ifcfg-iface0': textwrap.dedent("""\ + 'ifcfg-iface0': textwrap.dedent("""\ BOOTPROTO=dhcp DHCLIENT6_MODE=managed STARTMODE=auto""") @@ -1028,7 +1028,7 @@ NETWORK_CONFIGS = { }, 'v6_and_v4': { 'expected_sysconfig_opensuse': { - 'ifcfg-iface0': textwrap.dedent("""\ + 'ifcfg-iface0': textwrap.dedent("""\ BOOTPROTO=dhcp DHCLIENT6_MODE=managed STARTMODE=auto""") diff --git a/tox.ini b/tox.ini index 725a0ca4..5ee67368 100644 --- a/tox.ini +++ b/tox.ini @@ -47,10 +47,9 @@ deps = -r{toxinidir}/test-requirements.txt # E123: closing bracket does not match indentation of opening bracket’s line # E126: continuation line over-indented for hanging indent # E226: missing whitespace around arithmetic operator -# E241: multiple spaces after ‘,’ # W503: line break before binary operator # W504: line break after binary operator -ignore=E121,E123,E126,E226,E241,W503,W504 +ignore=E121,E123,E126,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 d53921ea3396c8301c65cad3abf04b4542d4b7a0 Mon Sep 17 00:00:00 2001 From: Joshua Powers Date: Tue, 2 Jun 2020 09:06:07 -0700 Subject: test: fix all flake8 E121 and E123 errors (#404) This fixes issues with closing brackets not matching the opening bracket's line and continuation line under-idented for hanging indent. --- cloudinit/net/network_state.py | 2 +- cloudinit/reporting/handlers.py | 12 +++---- cloudinit/sources/helpers/azure.py | 6 ++-- tests/unittests/test_datasource/test_aliyun.py | 2 +- tests/unittests/test_datasource/test_azure.py | 8 ++--- tests/unittests/test_datasource/test_ec2.py | 15 +++++---- tests/unittests/test_datasource/test_scaleway.py | 39 +++++++++++++--------- tests/unittests/test_distros/test_netconfig.py | 4 +-- tests/unittests/test_ds_identify.py | 4 +-- .../unittests/test_handler/test_handler_puppet.py | 23 ++++++++----- tests/unittests/test_net.py | 18 +++++----- tox.ini | 4 +-- 12 files changed, 75 insertions(+), 62 deletions(-) (limited to 'tox.ini') diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 35c279f9..f28973dc 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -215,7 +215,7 @@ class NetworkState(object): return ( route.get('prefix') == 0 and route.get('network') in default_nets - ) + ) class NetworkStateInterpreter(metaclass=CommandHandlerMeta): diff --git a/cloudinit/reporting/handlers.py b/cloudinit/reporting/handlers.py index 946df7e0..00e8d2e5 100755 --- a/cloudinit/reporting/handlers.py +++ b/cloudinit/reporting/handlers.py @@ -219,7 +219,7 @@ class HyperVKvpReportingHandler(ReportingHandler): v = ( record_data[ self.HV_KVP_EXCHANGE_MAX_KEY_SIZE:self.HV_KVP_RECORD_SIZE - ].decode('utf-8').strip('\x00')) + ].decode('utf-8').strip('\x00')) return {'key': k, 'value': v} @@ -265,11 +265,11 @@ class HyperVKvpReportingHandler(ReportingHandler): """ key = self._event_key(event) meta_data = { - "name": event.name, - "type": event.event_type, - "ts": (datetime.utcfromtimestamp(event.timestamp) - .isoformat() + 'Z'), - } + "name": event.name, + "type": event.event_type, + "ts": (datetime.utcfromtimestamp(event.timestamp) + .isoformat() + 'Z'), + } if hasattr(event, self.RESULT_KEY): meta_data[self.RESULT_KEY] = event.result meta_data[self.MSG_KEY] = event.description diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index fc760581..82b6730c 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -64,13 +64,15 @@ def is_byte_swapped(previous_id, current_id): return ''.join(dd) parts = current_id.split('-') - swapped_id = '-'.join([ + swapped_id = '-'.join( + [ swap_bytestring(parts[0]), swap_bytestring(parts[1]), swap_bytestring(parts[2]), parts[3], parts[4] - ]) + ] + ) return previous_id == swapped_id diff --git a/tests/unittests/test_datasource/test_aliyun.py b/tests/unittests/test_datasource/test_aliyun.py index 1e66fcdb..b626229e 100644 --- a/tests/unittests/test_datasource/test_aliyun.py +++ b/tests/unittests/test_datasource/test_aliyun.py @@ -143,7 +143,7 @@ class TestAliYunDatasource(test_helpers.HttprettyTestCase): self.assertEqual('aliyun', self.ds.cloud_name) self.assertEqual('ec2', self.ds.platform) self.assertEqual( - 'metadata (http://100.100.100.200)', self.ds.subplatform) + 'metadata (http://100.100.100.200)', self.ds.subplatform) @mock.patch("cloudinit.sources.DataSourceAliYun._is_aliyun") def test_returns_false_when_not_on_aliyun(self, m_is_aliyun): diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 2f8561cb..d8d812e3 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -114,14 +114,14 @@ NETWORK_METADATA = { "ipv4": { "subnet": [ { - "prefix": "24", - "address": "10.0.0.0" + "prefix": "24", + "address": "10.0.0.0" } ], "ipAddress": [ { - "privateIpAddress": "10.0.0.4", - "publicIpAddress": "104.46.124.81" + "privateIpAddress": "10.0.0.4", + "publicIpAddress": "104.46.124.81" } ] } diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index ad1ea595..80e69c03 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -576,7 +576,8 @@ class TestEc2(test_helpers.HttprettyTestCase): md=None) conn_error = requests.exceptions.ConnectionError( - '[Errno 113] no route to host') + '[Errno 113] no route to host' + ) mock_success = mock.MagicMock(contents=b'fakesuccess') mock_success.ok.return_value = True @@ -777,12 +778,12 @@ class TestGetSecondaryAddresses(test_helpers.CiTestCase): '2600:1f16:292:100:f153:12a3:c37c:11f9/128'], ec2.get_secondary_addresses(invalid_cidr_md, self.mac)) expected_logs = [ - "WARNING: Could not parse subnet-ipv4-cidr-block" - " something-unexpected for mac 06:17:04:d7:26:ff." - " ipv4 network config prefix defaults to /24", - "WARNING: Could not parse subnet-ipv6-cidr-block" - " not/sure/what/this/is for mac 06:17:04:d7:26:ff." - " ipv6 network config prefix defaults to /128" + "WARNING: Could not parse subnet-ipv4-cidr-block" + " something-unexpected for mac 06:17:04:d7:26:ff." + " ipv4 network config prefix defaults to /24", + "WARNING: Could not parse subnet-ipv6-cidr-block" + " not/sure/what/this/is for mac 06:17:04:d7:26:ff." + " ipv6 network config prefix defaults to /128" ] logs = self.logs.getvalue() for log in expected_logs: diff --git a/tests/unittests/test_datasource/test_scaleway.py b/tests/unittests/test_datasource/test_scaleway.py index 1b4dd0ad..15441454 100644 --- a/tests/unittests/test_datasource/test_scaleway.py +++ b/tests/unittests/test_datasource/test_scaleway.py @@ -371,25 +371,32 @@ class TestDataSourceScaleway(HttprettyTestCase): m_get_cmdline.return_value = 'scaleway' fallback_nic.return_value = 'ens2' self.datasource.metadata['ipv6'] = { - 'address': '2000:abc:4444:9876::42:999', - 'gateway': '2000:abc:4444:9876::42:000', - 'netmask': '127', - } + 'address': '2000:abc:4444:9876::42:999', + 'gateway': '2000:abc:4444:9876::42:000', + 'netmask': '127', + } netcfg = self.datasource.network_config - resp = {'version': 1, - 'config': [{ - 'type': 'physical', - 'name': 'ens2', - 'subnets': [{'type': 'dhcp4'}, - {'type': 'static', - 'address': '2000:abc:4444:9876::42:999', - 'gateway': '2000:abc:4444:9876::42:000', - 'netmask': '127', } - ] - - }] + resp = { + 'version': 1, + 'config': [ + { + 'type': 'physical', + 'name': 'ens2', + 'subnets': [ + { + 'type': 'dhcp4' + }, + { + 'type': 'static', + 'address': '2000:abc:4444:9876::42:999', + 'gateway': '2000:abc:4444:9876::42:000', + 'netmask': '127', + } + ] } + ] + } self.assertEqual(netcfg, resp) @mock.patch('cloudinit.sources.DataSourceScaleway.net.find_fallback_nic') diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py index ccf66161..910207ca 100644 --- a/tests/unittests/test_distros/test_netconfig.py +++ b/tests/unittests/test_distros/test_netconfig.py @@ -532,7 +532,7 @@ class TestNetCfgDistroRedhat(TestNetCfgDistroBase): NETWORKING_IPV6=yes IPV6_AUTOCONF=no """), - } + } # rh_distro.apply_network_config(V1_NET_CFG_IPV6, False) self._apply_and_verify(self.distro.apply_network_config, V1_NET_CFG_IPV6, @@ -656,7 +656,7 @@ class TestNetCfgDistroArch(TestNetCfgDistroBase): IP=dhcp Interface=eth1 """), - } + } # ub_distro.apply_network_config(V1_NET_CFG, False) self._apply_and_verify(self.distro.apply_network_config, diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index c2318570..65a96090 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -1040,11 +1040,11 @@ VALID_CFG = { 'Ec2-E24Cloud': { 'ds': 'Ec2', 'files': {P_SYS_VENDOR: 'e24cloud\n'}, - }, + }, 'Ec2-E24Cloud-negative': { 'ds': 'Ec2', 'files': {P_SYS_VENDOR: 'e24cloudyday\n'}, - } + } } # vi: ts=4 expandtab diff --git a/tests/unittests/test_handler/test_handler_puppet.py b/tests/unittests/test_handler/test_handler_puppet.py index 04aa7d03..2506d18a 100644 --- a/tests/unittests/test_handler/test_handler_puppet.py +++ b/tests/unittests/test_handler/test_handler_puppet.py @@ -149,15 +149,20 @@ class TestPuppetHandle(CiTestCase): mycloud.distro = mock.MagicMock() cfg = { 'puppet': { - 'csr_attributes': { - 'custom_attributes': { - '1.2.840.113549.1.9.7': '342thbjkt82094y0ut' - 'hhor289jnqthpc2290'}, - 'extension_requests': { - 'pp_uuid': 'ED803750-E3C7-44F5-BB08-41A04433FE2E', - 'pp_image_name': 'my_ami_image', - 'pp_preshared_key': '342thbjkt82094y0uthhor289jnqthpc2290'} - }}} + 'csr_attributes': { + 'custom_attributes': { + '1.2.840.113549.1.9.7': + '342thbjkt82094y0uthhor289jnqthpc2290' + }, + 'extension_requests': { + 'pp_uuid': 'ED803750-E3C7-44F5-BB08-41A04433FE2E', + 'pp_image_name': 'my_ami_image', + 'pp_preshared_key': + '342thbjkt82094y0uthhor289jnqthpc2290' + } + } + } + } csr_attributes = 'cloudinit.config.cc_puppet.' \ 'PUPPET_CSR_ATTRIBUTES_PATH' with mock.patch(csr_attributes, self.csr_attributes_path): diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index 84d3a5f0..23626395 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -3332,7 +3332,7 @@ USERCTL=no USERCTL=no VLAN=yes """) - } + } self._compare_files_to_expected( expected, self._render_and_read(network_config=v2data)) @@ -3406,7 +3406,7 @@ USERCTL=no TYPE=Ethernet USERCTL=no """), - } + } for dhcp_ver in ('dhcp4', 'dhcp6'): v2data = copy.deepcopy(v2base) if dhcp_ver == 'dhcp6': @@ -4765,13 +4765,13 @@ class TestNetRenderers(CiTestCase): def test_sysconfig_available_uses_variant_mapping(self, m_distro, m_avail): m_avail.return_value = True distro_values = [ - ('opensuse', '', ''), - ('opensuse-leap', '', ''), - ('opensuse-tumbleweed', '', ''), - ('sles', '', ''), - ('centos', '', ''), - ('fedora', '', ''), - ('redhat', '', ''), + ('opensuse', '', ''), + ('opensuse-leap', '', ''), + ('opensuse-tumbleweed', '', ''), + ('sles', '', ''), + ('centos', '', ''), + ('fedora', '', ''), + ('redhat', '', ''), ] for (distro_name, distro_version, flavor) in distro_values: m_distro.return_value = (distro_name, distro_version, flavor) diff --git a/tox.ini b/tox.ini index 5ee67368..e04c7791 100644 --- a/tox.ini +++ b/tox.ini @@ -43,13 +43,11 @@ basepython = python2.7 deps = -r{toxinidir}/test-requirements.txt [flake8] -# E121: continuation line under-indented for hanging indent -# E123: closing bracket does not match indentation of opening bracket’s line # 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=E121,E123,E126,E226,W503,W504 +ignore=E126,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 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 'tox.ini') 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 66e114a660c53400e389f119781f378311b65108 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 30 Jun 2020 13:17:40 -0400 Subject: Enable use of the caplog fixture in pytest tests, and add a cc_final_message test using it (#461) caplog is only available in pytest itself from 3.0 onwards. In xenial, we only have pytest 2.8.7. However, in xenial we do have pytest-catchlog available (as python3-pytest-catchlog), so we use that where appropriate. --- .travis.yml | 2 +- HACKING.rst | 9 +++++---- cloudinit/config/tests/test_final_message.py | 19 ++++++++++++++++--- packages/bddeb | 2 ++ tox.ini | 1 + 5 files changed, 25 insertions(+), 8 deletions(-) (limited to 'tox.ini') diff --git a/.travis.yml b/.travis.yml index 95c591fd..9844c065 100644 --- a/.travis.yml +++ b/.travis.yml @@ -72,7 +72,7 @@ matrix: - cp /usr/share/doc/sbuild/examples/example.sbuildrc /home/$USER/.sbuildrc script: # Ubuntu LTS: Build - - ./packages/bddeb -S -d + - ./packages/bddeb -S -d --release xenial - | needs_caching=false if [ -e "$TRAVIS_BUILD_DIR/chroots/xenial-amd64.tar" ]; then diff --git a/HACKING.rst b/HACKING.rst index 1e8aca7a..27a38bc3 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -218,11 +218,12 @@ The following guidelines should be followed: [#fixture-list]_: * ``cache`` - * ``capsys`` * ``capfd`` - * ``record_xml_property`` + * ``caplog`` (provided by ``python3-pytest-catchlog`` on xenial) + * ``capsys`` * ``monkeypatch`` * ``pytestconfig`` + * ``record_xml_property`` * ``recwarn`` * ``tmpdir_factory`` * ``tmpdir`` @@ -328,9 +329,9 @@ variable annotations specified in `PEP-526`_ were introduced in Python .. [#fixture-list] This list of fixtures (with markup) can be reproduced by running:: - py.test-3 --fixtures -q | grep "^[^ ]" | grep -v no | sed 's/.*/* ``\0``/' + py.test-3 --fixtures -q | grep "^[^ -]" | grep -v '\(no\|capturelog\)' | sort | sed 's/.*/* ``\0``/' - in a xenial lxd container with python3-pytest installed. + in a xenial lxd container with python3-pytest-catchlog installed. Feature Flags ------------- diff --git a/cloudinit/config/tests/test_final_message.py b/cloudinit/config/tests/test_final_message.py index 76cb0ad1..46ba99b2 100644 --- a/cloudinit/config/tests/test_final_message.py +++ b/cloudinit/config/tests/test_final_message.py @@ -1,4 +1,5 @@ # This file is part of cloud-init. See LICENSE file for license information. +import logging from unittest import mock import pytest @@ -12,10 +13,19 @@ class TestHandle: # contents). @pytest.mark.parametrize( - "instance_dir_exists,file_is_written", [(True, True), (False, False)] + "instance_dir_exists,file_is_written,expected_log_substring", + [ + (True, True, None), + (False, False, "Failed to write boot finished file "), + ], ) def test_boot_finished_written( - self, instance_dir_exists, file_is_written, tmpdir + self, + instance_dir_exists, + file_is_written, + expected_log_substring, + caplog, + tmpdir, ): instance_dir = tmpdir.join("var/lib/cloud/instance") if instance_dir_exists: @@ -26,8 +36,11 @@ class TestHandle: paths=mock.Mock(boot_finished=boot_finished.strpath) ) - handle(None, {}, m_cloud, mock.Mock(), []) + handle(None, {}, m_cloud, logging.getLogger(), []) # We should not change the status of the instance directory assert instance_dir_exists == instance_dir.exists() assert file_is_written == boot_finished.exists() + + if expected_log_substring: + assert expected_log_substring in caplog.text diff --git a/packages/bddeb b/packages/bddeb index 78b1c83b..b0f219b6 100755 --- a/packages/bddeb +++ b/packages/bddeb @@ -91,6 +91,8 @@ def write_debian_folder(root, templ_data, cloud_util_deps): # NOTE: python package was moved to the front after debuild -S would fail with # 'Please add apropriate interpreter' errors (as in debian bug 861132) requires.extend(['python3'] + reqs + test_reqs) + if templ_data['debian_release'] == 'xenial': + requires.append('python3-pytest-catchlog') templater.render_to_file(util.abs_join(find_root(), 'packages', 'debian', 'control.in'), util.abs_join(deb_dir, 'control'), diff --git a/tox.ini b/tox.ini index ebcebc41..847a2fbd 100644 --- a/tox.ini +++ b/tox.ini @@ -79,6 +79,7 @@ deps = # test-requirements httpretty==0.9.6 mock==1.3.0 + pytest-catchlog==1.2.1 [testenv:xenial] # When updating this commands definition, also update the definition in -- cgit v1.2.3 From 2b727914e8cbee6810b1bb9a1cfdb90ad521ceb6 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 2 Jul 2020 13:51:28 -0400 Subject: tests: use markers to configure disable_subp_usage (#473) This is an improvement over indirect parameterisation for a few reasons: * The test code is much easier to read, the mark names are much more intuitive than the indirect parameterisation invocation, and there's less boilerplate to boot * The fixture no longer has to overload the single parameter that fixtures can take with multiple meanings --- cloudinit/tests/test_conftest.py | 14 ++-- conftest.py | 93 ++++++++++++++-------- tests/unittests/test_datasource/test_opennebula.py | 3 +- tests/unittests/test_render_cloudcfg.py | 2 +- tox.ini | 7 ++ 5 files changed, 76 insertions(+), 43 deletions(-) (limited to 'tox.ini') diff --git a/cloudinit/tests/test_conftest.py b/cloudinit/tests/test_conftest.py index a6537248..6f1263a5 100644 --- a/cloudinit/tests/test_conftest.py +++ b/cloudinit/tests/test_conftest.py @@ -17,12 +17,11 @@ class TestDisableSubpUsage: # pylint: disable=no-value-for-parameter subp.subp() - @pytest.mark.parametrize('disable_subp_usage', [False], indirect=True) + @pytest.mark.allow_all_subp def test_subp_usage_can_be_reenabled(self): subp.subp(['whoami']) - @pytest.mark.parametrize( - 'disable_subp_usage', [['whoami'], 'whoami'], indirect=True) + @pytest.mark.allow_subp_for("whoami") def test_subp_usage_can_be_conditionally_reenabled(self): # The two parameters test each potential invocation with a single # argument @@ -31,8 +30,7 @@ class TestDisableSubpUsage: assert "allowed: whoami" in str(excinfo.value) subp.subp(['whoami']) - @pytest.mark.parametrize( - 'disable_subp_usage', [['whoami', 'bash']], indirect=True) + @pytest.mark.allow_subp_for("whoami", "bash") def test_subp_usage_can_be_conditionally_reenabled_for_multiple_cmds(self): with pytest.raises(AssertionError) as excinfo: subp.subp(["some", "args"]) @@ -40,6 +38,12 @@ class TestDisableSubpUsage: subp.subp(['bash', '-c', 'true']) subp.subp(['whoami']) + @pytest.mark.allow_all_subp + @pytest.mark.allow_subp_for("bash") + def test_both_marks_raise_an_error(self): + with pytest.raises(AssertionError, match="marked both"): + subp.subp(["bash"]) + class TestDisableSubpUsageInTestSubclass(CiTestCase): """Test that disable_subp_usage doesn't impact CiTestCase's subp logic.""" diff --git a/conftest.py b/conftest.py index 251bca59..faf13804 100644 --- a/conftest.py +++ b/conftest.py @@ -5,6 +5,18 @@ import pytest from cloudinit import 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 + + @pytest.yield_fixture(autouse=True) def disable_subp_usage(request): """ @@ -15,25 +27,23 @@ def disable_subp_usage(request): imports happen before the patching here (or the CiTestCase monkey-patching) happens, so are left untouched. - To allow a particular test method or class to use subp.subp you can set the - parameter passed to this fixture to False using pytest.mark.parametrize:: + To allow a particular test method or class to use subp.subp you can mark it + as such:: - @pytest.mark.parametrize("disable_subp_usage", [False], indirect=True) + @pytest.mark.allow_all_subp def test_whoami(self): subp.subp(["whoami"]) - To instead allow subp.subp usage for a specific command, you can set the - parameter passed to this fixture to that command: + To instead allow subp.subp usage for a specific command, you can use the + ``allow_subp_for`` mark:: - @pytest.mark.parametrize("disable_subp_usage", ["bash"], indirect=True) + @pytest.mark.allow_subp_for("bash") def test_bash(self): subp.subp(["bash"]) - To specify multiple commands, set the parameter to a list (note the - double-layered list: we specify a single parameter that is itself a list): + You can pass multiple commands as values; they will all be permitted:: - @pytest.mark.parametrize( - "disable_subp_usage", ["bash", "whoami"], indirect=True) + @pytest.mark.allow_subp_for("bash", "whoami") def test_several_things(self): subp.subp(["bash"]) subp.subp(["whoami"]) @@ -43,30 +53,43 @@ def disable_subp_usage(request): tests, CiTestCase's allowed_subp does take precedence (and we have TestDisableSubpUsageInTestSubclass to confirm that). """ - should_disable = getattr(request, "param", True) - if should_disable: - if not isinstance(should_disable, (list, str)): - def side_effect(args, *other_args, **kwargs): - raise AssertionError("Unexpectedly used subp.subp") - else: - # Look this up before our patch is in place, so we have access to - # the real implementation in side_effect - real_subp = subp.subp - - if isinstance(should_disable, str): - should_disable = [should_disable] - - def side_effect(args, *other_args, **kwargs): - cmd = args[0] - if cmd not in should_disable: - raise AssertionError( - "Unexpectedly used subp.subp to call {} (allowed:" - " {})".format(cmd, ",".join(should_disable)) - ) - return real_subp(args, *other_args, **kwargs) - - with mock.patch('cloudinit.subp.subp', autospec=True) as m_subp: - m_subp.side_effect = side_effect - yield + allow_subp_for = _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) + + if allow_all_subp is not None and allow_subp_for is None: + # Only allow_all_subp specified, don't mock subp.subp + yield + return + + if allow_all_subp is None and allow_subp_for is None: + # No marks, default behaviour; disallow all subp.subp usage + def side_effect(args, *other_args, **kwargs): + raise AssertionError("Unexpectedly used subp.subp") + + elif allow_all_subp is not None and allow_subp_for is not None: + # Both marks, ambiguous request; raise an exception on all subp usage + def side_effect(args, *other_args, **kwargs): + raise AssertionError( + "Test marked both allow_all_subp and allow_subp_for: resolve" + " this either by modifying your test code, or by modifying" + " disable_subp_usage to handle precedence." + ) else: + # Look this up before our patch is in place, so we have access to + # the real implementation in side_effect + real_subp = subp.subp + + def side_effect(args, *other_args, **kwargs): + cmd = args[0] + if cmd not in allow_subp_for: + raise AssertionError( + "Unexpectedly used subp.subp to call {} (allowed:" + " {})".format(cmd, ",".join(allow_subp_for)) + ) + return real_subp(args, *other_args, **kwargs) + + with mock.patch("cloudinit.subp.subp", autospec=True) as m_subp: + m_subp.side_effect = side_effect yield diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py index 80841182..9c6070a5 100644 --- a/tests/unittests/test_datasource/test_opennebula.py +++ b/tests/unittests/test_datasource/test_opennebula.py @@ -928,8 +928,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): class TestParseShellConfig: - - @pytest.mark.parametrize('disable_subp_usage', ['bash'], indirect=True) + @pytest.mark.allow_subp_for("bash") def test_no_seconds(self): cfg = '\n'.join(["foo=bar", "SECONDS=2", "xx=foo"]) # we could test 'sleep 2', but that would make the test run slower. diff --git a/tests/unittests/test_render_cloudcfg.py b/tests/unittests/test_render_cloudcfg.py index 696915a3..495e2669 100644 --- a/tests/unittests/test_render_cloudcfg.py +++ b/tests/unittests/test_render_cloudcfg.py @@ -13,7 +13,7 @@ DISTRO_VARIANTS = ["amazon", "arch", "centos", "debian", "fedora", "freebsd", "netbsd", "openbsd", "rhel", "suse", "ubuntu", "unknown"] -@pytest.mark.parametrize('disable_subp_usage', [sys.executable], indirect=True) +@pytest.mark.allow_subp_for(sys.executable) class TestRenderCloudCfg: cmd = [sys.executable, os.path.realpath('tools/render-cloudcfg')] diff --git a/tox.ini b/tox.ini index 847a2fbd..5257f9e3 100644 --- a/tox.ini +++ b/tox.ini @@ -133,3 +133,10 @@ commands = {envpython} -m tests.cloud_tests {posargs} passenv = HOME TRAVIS deps = -r{toxinidir}/integration-requirements.txt + +[pytest] +# TODO: s/--strict/--strict-markers/ once xenial support is dropped +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) -- cgit v1.2.3 From 411dbbebd328163bcb1c676cc711f3e5ed805375 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Mon, 6 Jul 2020 11:31:33 -0400 Subject: cloudinit: fix tip-pylint failures and bump pinned pylint version (#478) Specifically: * disable E1102 in cloudinit/sources/helpers/openstack.py for reasons described in a comment, and * refactor `abs_join` to require at least one positional argument; this matches os.path.join's signature, and that mismatch is what was causing pylint to emit a warning * bump to pylint 2.4.2 --- cloudinit/sources/helpers/openstack.py | 5 ++++- cloudinit/util.py | 4 ++-- tox.ini | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) (limited to 'tox.ini') diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index c538720a..1050efb0 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -398,7 +398,10 @@ class ConfigDriveReader(BaseReader): except IOError: raise BrokenMetadata("Failed to read: %s" % path) try: - md[key] = translator(contents) + # Disable not-callable pylint check; pylint isn't able to + # determine that every member of FILES_V1 has a callable in + # the appropriate position + md[key] = translator(contents) # pylint: disable=E1102 except Exception as e: raise BrokenMetadata("Failed to process " "path %s: %s" % (path, e)) diff --git a/cloudinit/util.py b/cloudinit/util.py index 81369652..b6f1117f 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1874,8 +1874,8 @@ def make_header(comment_char="#", base='created'): return header -def abs_join(*paths): - return os.path.abspath(os.path.join(*paths)) +def abs_join(base, *paths): + return os.path.abspath(os.path.join(base, *paths)) # shellify, takes a list of commands diff --git a/tox.ini b/tox.ini index 5257f9e3..3fd96702 100644 --- a/tox.ini +++ b/tox.ini @@ -23,7 +23,7 @@ setenv = basepython = python3 deps = # requirements - pylint==2.3.1 + pylint==2.5.3 # test-requirements because unit tests are now present in cloudinit tree -r{toxinidir}/test-requirements.txt -r{toxinidir}/integration-requirements.txt -- 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 'tox.ini') 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 1212675ea30feb8726e163c82127ca3cb1951f4e Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 13 Aug 2020 16:21:42 -0400 Subject: tox.ini: pin correct version of httpretty in xenial{,-dev} envs (#531) The version was bumped in c7248059dd2faaaadfbcef5c83e8e8ea166d6767 to support running on Python 3.7+ systems. Now that we have separate `xenial` and `xenial-dev` tox environments, we can restore the correct pinning for `xenial` without breaking `xenial-dev` on developer machines. Also drop the `mock` dependency from `xenial-shared-deps`; its removal was missed in 5f8f85bb38cc972d3d2c705a1ec73db3f690f323. --- tox.ini | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'tox.ini') diff --git a/tox.ini b/tox.ini index 6c4b2e81..f619dbf5 100644 --- a/tox.ini +++ b/tox.ini @@ -77,8 +77,6 @@ deps = configobj==5.0.6 requests==2.9.1 # test-requirements - httpretty==0.9.6 - mock==1.3.0 pytest-catchlog==1.2.1 [testenv:xenial] @@ -91,6 +89,7 @@ basepython = python3 deps = # Refer to the comment in [xenial-shared-deps] for details {[xenial-shared-deps]deps} + httpretty==0.8.6 jsonpatch==1.10 pytest==2.8.7 @@ -108,6 +107,10 @@ basepython = {[testenv:xenial]basepython} deps = # Refer to the comment in [xenial-shared-deps] for details {[xenial-shared-deps]deps} + # httpretty in xenial is 0.8.6, not 0.9.5. The oldest version to work with + # Python 3.7+ is 0.9.5, because it is the first to include this commit: + # https://github.com/gabrielfalcao/HTTPretty/commit/5776d97da3992b9071db5e21faf175f6e8729060 + httpretty==0.9.5 # jsonpatch in xenial is 1.10, not 1.19 (#839779). The oldest version # to work with python3.6 is 1.16 as found in Artful. To keep default # invocation of 'tox' happy, accept the difference in version here. -- cgit v1.2.3 From 07104504ab5b30efd2d1f7a8c36effe18b8e5fe0 Mon Sep 17 00:00:00 2001 From: Paride Legovini Date: Tue, 25 Aug 2020 17:21:18 +0200 Subject: tox: bump the pylint version to 2.6.0 in the default run (#544) Changes: tox: bump the pylint version to 2.6.0 in the default run Fix pylint 2.6.0 W0707 warnings (raise-missing-from) --- cloudinit/analyze/show.py | 2 +- cloudinit/cmd/devel/make_mime.py | 6 ++-- cloudinit/config/cc_disk_setup.py | 36 ++++++++++++++-------- cloudinit/config/cc_growpart.py | 8 ++--- cloudinit/config/cc_power_state_change.py | 12 +++++--- cloudinit/config/cc_rsyslog.py | 6 ++-- cloudinit/config/cc_set_hostname.py | 2 +- cloudinit/config/cc_ubuntu_advantage.py | 2 +- cloudinit/config/schema.py | 2 +- cloudinit/distros/__init__.py | 5 +-- cloudinit/distros/arch.py | 4 +-- cloudinit/distros/parsers/resolv_conf.py | 7 +++-- cloudinit/gpg.py | 5 +-- cloudinit/handlers/jinja_template.py | 3 +- cloudinit/net/__init__.py | 7 +++-- cloudinit/net/cmdline.py | 4 +-- cloudinit/net/dhcp.py | 4 +-- cloudinit/net/network_state.py | 19 +++++++----- cloudinit/sources/DataSourceAzure.py | 2 +- cloudinit/sources/DataSourceEc2.py | 8 +++-- cloudinit/sources/DataSourceIBMCloud.py | 3 +- cloudinit/sources/DataSourceMAAS.py | 3 +- cloudinit/sources/DataSourceOpenNebula.py | 13 +++++--- cloudinit/sources/DataSourceOpenStack.py | 4 +-- cloudinit/sources/DataSourceSmartOS.py | 4 ++- cloudinit/sources/helpers/azure.py | 34 +++++++++++--------- cloudinit/sources/helpers/netlink.py | 2 +- cloudinit/sources/helpers/openstack.py | 34 +++++++++++--------- cloudinit/subp.py | 3 +- cloudinit/url_helper.py | 6 ++-- cloudinit/util.py | 8 ++--- tests/cloud_tests/platforms/azurecloud/instance.py | 7 +++-- tests/cloud_tests/platforms/azurecloud/platform.py | 23 +++++++++----- tests/cloud_tests/platforms/ec2/instance.py | 4 +-- tests/cloud_tests/platforms/ec2/platform.py | 18 ++++++----- tests/cloud_tests/platforms/lxd/instance.py | 3 +- tests/cloud_tests/platforms/platforms.py | 6 ++-- tests/cloud_tests/testcases/__init__.py | 6 ++-- tools/mock-meta.py | 14 +++++---- tox.ini | 2 +- 40 files changed, 205 insertions(+), 136 deletions(-) (limited to 'tox.ini') diff --git a/cloudinit/analyze/show.py b/cloudinit/analyze/show.py index 0c825b23..01a4d3e5 100644 --- a/cloudinit/analyze/show.py +++ b/cloudinit/analyze/show.py @@ -267,7 +267,7 @@ def gather_timestamps_using_systemd(): except OSError as err: raise RuntimeError('Could not determine container boot ' 'time from /proc/1/cmdline. ({})' - .format(err)) + .format(err)) from err status = CONTAINER_CODE else: status = FAIL_CODE diff --git a/cloudinit/cmd/devel/make_mime.py b/cloudinit/cmd/devel/make_mime.py index 77e10540..4e6a5778 100755 --- a/cloudinit/cmd/devel/make_mime.py +++ b/cloudinit/cmd/devel/make_mime.py @@ -22,8 +22,10 @@ def file_content_type(text): try: filename, content_type = text.split(":", 1) return (open(filename, 'r'), filename, content_type.strip()) - except ValueError: - raise argparse.ArgumentError(text, "Invalid value for %r" % (text)) + except ValueError as e: + raise argparse.ArgumentError( + text, "Invalid value for %r" % (text) + ) from e def get_parser(parser=None): diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py index d957cfe3..a7bdc703 100644 --- a/cloudinit/config/cc_disk_setup.py +++ b/cloudinit/config/cc_disk_setup.py @@ -251,7 +251,9 @@ def enumerate_disk(device, nodeps=False): try: info, _err = subp.subp(lsblk_cmd) except Exception as e: - raise Exception("Failed during disk check for %s\n%s" % (device, e)) + raise Exception( + "Failed during disk check for %s\n%s" % (device, e) + ) from e parts = [x for x in (info.strip()).splitlines() if len(x.split()) > 0] @@ -313,7 +315,9 @@ def check_fs(device): try: out, _err = subp.subp(blkid_cmd, rcs=[0, 2]) except Exception as e: - raise Exception("Failed during disk check for %s\n%s" % (device, e)) + raise Exception( + "Failed during disk check for %s\n%s" % (device, e) + ) from e if out: if len(out.splitlines()) == 1: @@ -428,8 +432,8 @@ def get_dyn_func(*args): else: return globals()[func_name] - except KeyError: - raise Exception("No such function %s to call!" % func_name) + except KeyError as e: + raise Exception("No such function %s to call!" % func_name) from e def get_hdd_size(device): @@ -437,7 +441,7 @@ def get_hdd_size(device): size_in_bytes, _ = subp.subp([BLKDEV_CMD, '--getsize64', device]) sector_size, _ = subp.subp([BLKDEV_CMD, '--getss', device]) except Exception as e: - raise Exception("Failed to get %s size\n%s" % (device, e)) + raise Exception("Failed to get %s size\n%s" % (device, e)) from e return int(size_in_bytes) / int(sector_size) @@ -455,8 +459,9 @@ def check_partition_mbr_layout(device, layout): try: out, _err = subp.subp(prt_cmd, data="%s\n" % layout) except Exception as e: - raise Exception("Error running partition command on %s\n%s" % ( - device, e)) + raise Exception( + "Error running partition command on %s\n%s" % (device, e) + ) from e found_layout = [] for line in out.splitlines(): @@ -485,8 +490,9 @@ def check_partition_gpt_layout(device, layout): try: out, _err = subp.subp(prt_cmd, update_env=LANG_C_ENV) except Exception as e: - raise Exception("Error running partition command on %s\n%s" % ( - device, e)) + raise Exception( + "Error running partition command on %s\n%s" % (device, e) + ) from e out_lines = iter(out.splitlines()) # Skip header. Output looks like: @@ -657,8 +663,10 @@ def purge_disk(device): try: LOG.info("Purging filesystem on /dev/%s", d['name']) subp.subp(wipefs_cmd) - except Exception: - raise Exception("Failed FS purge of /dev/%s" % d['name']) + except Exception as e: + raise Exception( + "Failed FS purge of /dev/%s" % d['name'] + ) from e purge_disk_ptable(device) @@ -700,7 +708,9 @@ def exec_mkpart_mbr(device, layout): try: subp.subp(prt_cmd, data="%s\n" % layout) except Exception as e: - raise Exception("Failed to partition device %s\n%s" % (device, e)) + raise Exception( + "Failed to partition device %s\n%s" % (device, e) + ) from e read_parttbl(device) @@ -997,6 +1007,6 @@ def mkfs(fs_cfg): try: subp.subp(fs_cmd, shell=shell) except Exception as e: - raise Exception("Failed to exec of '%s':\n%s" % (fs_cmd, e)) + raise Exception("Failed to exec of '%s':\n%s" % (fs_cmd, e)) from e # vi: ts=4 expandtab diff --git a/cloudinit/config/cc_growpart.py b/cloudinit/config/cc_growpart.py index c5d93f81..237c3d02 100644 --- a/cloudinit/config/cc_growpart.py +++ b/cloudinit/config/cc_growpart.py @@ -148,14 +148,14 @@ class ResizeGrowPart(object): if e.exit_code != 1: util.logexc(LOG, "Failed growpart --dry-run for (%s, %s)", diskdev, partnum) - raise ResizeFailedException(e) + raise ResizeFailedException(e) from e return (before, before) try: subp.subp(["growpart", diskdev, partnum]) except subp.ProcessExecutionError as e: util.logexc(LOG, "Failed: growpart %s %s", diskdev, partnum) - raise ResizeFailedException(e) + raise ResizeFailedException(e) from e return (before, get_size(partdev)) @@ -187,14 +187,14 @@ class ResizeGpart(object): except subp.ProcessExecutionError as e: if e.exit_code != 0: util.logexc(LOG, "Failed: gpart recover %s", diskdev) - raise ResizeFailedException(e) + raise ResizeFailedException(e) from e before = get_size(partdev) try: subp.subp(["gpart", "resize", "-i", partnum, diskdev]) except subp.ProcessExecutionError as e: util.logexc(LOG, "Failed: gpart resize -i %s %s", partnum, diskdev) - raise ResizeFailedException(e) + raise ResizeFailedException(e) from e # Since growing the FS requires a reboot, make sure we reboot # first when this module has finished. diff --git a/cloudinit/config/cc_power_state_change.py b/cloudinit/config/cc_power_state_change.py index ab953a0d..6fcb8a7d 100644 --- a/cloudinit/config/cc_power_state_change.py +++ b/cloudinit/config/cc_power_state_change.py @@ -196,10 +196,11 @@ def load_power_state(cfg, distro_name): try: delay = convert_delay(delay, fmt=fmt, scale=scale) - except ValueError: + except ValueError as e: raise TypeError( "power_state[delay] must be 'now' or '+m' (minutes)." - " found '%s'." % delay) + " found '%s'." % delay + ) from e args = command + [delay] if message: @@ -207,9 +208,10 @@ def load_power_state(cfg, distro_name): try: timeout = float(pstate.get('timeout', 30.0)) - except ValueError: - raise ValueError("failed to convert timeout '%s' to float." % - pstate['timeout']) + except ValueError as e: + raise ValueError( + "failed to convert timeout '%s' to float." % pstate['timeout'] + ) from e condition = pstate.get("condition", True) if not isinstance(condition, (str, list, bool)): diff --git a/cloudinit/config/cc_rsyslog.py b/cloudinit/config/cc_rsyslog.py index 1354885a..2a2bc931 100644 --- a/cloudinit/config/cc_rsyslog.py +++ b/cloudinit/config/cc_rsyslog.py @@ -347,8 +347,10 @@ class SyslogRemotesLine(object): if self.port: try: int(self.port) - except ValueError: - raise ValueError("port '%s' is not an integer" % self.port) + except ValueError as e: + raise ValueError( + "port '%s' is not an integer" % self.port + ) from e if not self.addr: raise ValueError("address is required") diff --git a/cloudinit/config/cc_set_hostname.py b/cloudinit/config/cc_set_hostname.py index c13020d8..1d23d80d 100644 --- a/cloudinit/config/cc_set_hostname.py +++ b/cloudinit/config/cc_set_hostname.py @@ -85,7 +85,7 @@ def handle(name, cfg, cloud, log, _args): except Exception as e: msg = "Failed to set the hostname to %s (%s)" % (fqdn, hostname) util.logexc(log, msg) - raise SetHostnameError("%s: %s" % (msg, e)) + raise SetHostnameError("%s: %s" % (msg, e)) from e write_json(prev_fn, {'hostname': hostname, 'fqdn': fqdn}) # vi: ts=4 expandtab diff --git a/cloudinit/config/cc_ubuntu_advantage.py b/cloudinit/config/cc_ubuntu_advantage.py index 35ded5db..d61dc655 100644 --- a/cloudinit/config/cc_ubuntu_advantage.py +++ b/cloudinit/config/cc_ubuntu_advantage.py @@ -115,7 +115,7 @@ def configure_ua(token=None, enable=None): msg = 'Failure attaching Ubuntu Advantage:\n{error}'.format( error=str(e)) util.logexc(LOG, msg) - raise RuntimeError(msg) + raise RuntimeError(msg) from e enable_errors = [] for service in enable: try: diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py index 2d8c7577..8a966aee 100644 --- a/cloudinit/config/schema.py +++ b/cloudinit/config/schema.py @@ -210,7 +210,7 @@ def validate_cloudconfig_file(config_path, schema, annotate=False): error = SchemaValidationError(errors) if annotate: print(annotated_cloudconfig_file({}, content, error.schema_errors)) - raise error + raise error from e try: validate_cloudconfig_schema( cloudconfig, schema, strict=True) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index effb4276..2537608f 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -607,10 +607,11 @@ class Distro(metaclass=abc.ABCMeta): lock_tools = (['passwd', '-l', name], ['usermod', '--lock', name]) try: cmd = next(tool for tool in lock_tools if subp.which(tool[0])) - except StopIteration: + except StopIteration as e: raise RuntimeError(( "Unable to lock user account '%s'. No tools available. " - " Tried: %s.") % (name, [c[0] for c in lock_tools])) + " Tried: %s.") % (name, [c[0] for c in lock_tools]) + ) from e try: subp.subp(cmd) except Exception as e: diff --git a/cloudinit/distros/arch.py b/cloudinit/distros/arch.py index 038aa9ac..967be168 100644 --- a/cloudinit/distros/arch.py +++ b/cloudinit/distros/arch.py @@ -61,9 +61,9 @@ class Distro(distros.Distro): def _write_network_config(self, netconfig): try: return self._supported_write_network_config(netconfig) - except RendererNotFoundError: + except RendererNotFoundError as e: # Fall back to old _write_network - raise NotImplementedError + raise NotImplementedError from e def _write_network(self, settings): entries = net_util.translate_network(settings) diff --git a/cloudinit/distros/parsers/resolv_conf.py b/cloudinit/distros/parsers/resolv_conf.py index 299d54b5..62929d03 100644 --- a/cloudinit/distros/parsers/resolv_conf.py +++ b/cloudinit/distros/parsers/resolv_conf.py @@ -150,9 +150,10 @@ class ResolvConf(object): tail = '' try: (cfg_opt, cfg_values) = head.split(None, 1) - except (IndexError, ValueError): - raise IOError("Incorrectly formatted resolv.conf line %s" - % (i + 1)) + except (IndexError, ValueError) as e: + raise IOError( + "Incorrectly formatted resolv.conf line %s" % (i + 1) + ) from e if cfg_opt not in ['nameserver', 'domain', 'search', 'sortlist', 'options']: raise IOError("Unexpected resolv.conf option %s" % (cfg_opt)) diff --git a/cloudinit/gpg.py b/cloudinit/gpg.py index 72b5ac59..be0ca0ea 100644 --- a/cloudinit/gpg.py +++ b/cloudinit/gpg.py @@ -63,10 +63,11 @@ def recv_key(key, keyserver, retries=(1, 1)): "Import failed with exit code %d, will try again in %ss", error.exit_code, naplen) time.sleep(naplen) - except StopIteration: + except StopIteration as e: raise ValueError( ("Failed to import key '%s' from keyserver '%s' " - "after %d tries: %s") % (key, keyserver, trynum, error)) + "after %d tries: %s") % (key, keyserver, trynum, error) + ) from e def delete_key(key): diff --git a/cloudinit/handlers/jinja_template.py b/cloudinit/handlers/jinja_template.py index ce3accf6..aadfbf86 100644 --- a/cloudinit/handlers/jinja_template.py +++ b/cloudinit/handlers/jinja_template.py @@ -83,7 +83,8 @@ def render_jinja_payload_from_file( if e.errno == EACCES: raise RuntimeError( 'Cannot render jinja template vars. No read permission on' - " '%s'. Try sudo" % instance_data_file) + " '%s'. Try sudo" % instance_data_file + ) from e rendered_payload = render_jinja_payload( payload, payload_fn, instance_data, debug) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 322af77b..e233149a 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -506,7 +506,9 @@ def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): try: _rename_interfaces(extract_physdevs(netcfg)) except RuntimeError as e: - raise RuntimeError('Failed to apply network config names: %s' % e) + raise RuntimeError( + 'Failed to apply network config names: %s' % e + ) from e def interface_has_own_mac(ifname, strict=False): @@ -965,7 +967,8 @@ class EphemeralIPv4Network(object): self.prefix = mask_to_net_prefix(prefix_or_mask) except ValueError as e: raise ValueError( - 'Cannot setup network: {0}'.format(e)) + 'Cannot setup network: {0}'.format(e) + ) from e self.connectivity_url = connectivity_url self.interface = interface diff --git a/cloudinit/net/cmdline.py b/cloudinit/net/cmdline.py index 7ca7262b..cc8dc17b 100755 --- a/cloudinit/net/cmdline.py +++ b/cloudinit/net/cmdline.py @@ -112,8 +112,8 @@ def _klibc_to_config_entry(content, mac_addrs=None): data = util.load_shell_content(content) try: name = data['DEVICE'] if 'DEVICE' in data else data['DEVICE6'] - except KeyError: - raise ValueError("no 'DEVICE' or 'DEVICE6' entry in data") + except KeyError as e: + raise ValueError("no 'DEVICE' or 'DEVICE6' entry in data") from e # ipconfig on precise does not write PROTO # IPv6 config gives us IPV6PROTO, not PROTO. diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index 9230cf7a..4394c68b 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -82,8 +82,8 @@ class EphemeralDHCPv4(object): try: leases = maybe_perform_dhcp_discovery( self.iface, self.dhcp_log_func) - except InvalidDHCPLeaseFileError: - raise NoDHCPLeaseError() + except InvalidDHCPLeaseFileError as e: + raise NoDHCPLeaseError() from e if not leases: raise NoDHCPLeaseError() self.lease = leases[-1] diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 7bfe8be0..b2f7d31e 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -297,9 +297,10 @@ class NetworkStateInterpreter(metaclass=CommandHandlerMeta): command_type = command['type'] try: handler = self.command_handlers[command_type] - except KeyError: - raise RuntimeError("No handler found for" - " command '%s'" % command_type) + except KeyError as e: + raise RuntimeError( + "No handler found for command '%s'" % command_type + ) from e try: handler(self, command) except InvalidCommand: @@ -316,9 +317,10 @@ class NetworkStateInterpreter(metaclass=CommandHandlerMeta): continue try: handler = self.command_handlers[command_type] - except KeyError: - raise RuntimeError("No handler found for" - " command '%s'" % command_type) + except KeyError as e: + raise RuntimeError( + "No handler found for command '%s'" % command_type + ) from e try: handler(self, command) self._v2_common(command) @@ -914,9 +916,10 @@ def _normalize_route(route): if metric: try: normal_route['metric'] = int(metric) - except ValueError: + except ValueError as e: raise TypeError( - 'Route config metric {} is not an integer'.format(metric)) + 'Route config metric {} is not an integer'.format(metric) + ) from e return normal_route diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 86cc7c28..f3c6452b 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -1147,7 +1147,7 @@ def read_azure_ovf(contents): except Exception as e: error_str = "Invalid ovf-env.xml: %s" % e report_diagnostic_event(error_str) - raise BrokenAzureDataSource(error_str) + raise BrokenAzureDataSource(error_str) from e results = find_child(dom.documentElement, lambda n: n.localName == "ProvisioningSection") diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 355b4e2f..1d09c12a 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -617,9 +617,11 @@ def parse_strict_mode(cfgval): if sleep: try: sleep = int(sleep) - except ValueError: - raise ValueError("Invalid sleep '%s' in strict_id setting '%s': " - "not an integer" % (sleep, cfgval)) + except ValueError as e: + raise ValueError( + "Invalid sleep '%s' in strict_id setting '%s': not an integer" + % (sleep, cfgval) + ) from e else: sleep = None diff --git a/cloudinit/sources/DataSourceIBMCloud.py b/cloudinit/sources/DataSourceIBMCloud.py index d2aa9a58..8d196185 100644 --- a/cloudinit/sources/DataSourceIBMCloud.py +++ b/cloudinit/sources/DataSourceIBMCloud.py @@ -303,7 +303,8 @@ def read_md(): except sources.BrokenMetadata as e: raise RuntimeError( "Failed reading IBM config disk (platform=%s path=%s): %s" % - (platform, path, e)) + (platform, path, e) + ) from e ret.update(results) return ret diff --git a/cloudinit/sources/DataSourceMAAS.py b/cloudinit/sources/DataSourceMAAS.py index c80f70c2..9156925f 100644 --- a/cloudinit/sources/DataSourceMAAS.py +++ b/cloudinit/sources/DataSourceMAAS.py @@ -226,7 +226,8 @@ def read_maas_seed_url(seed_url, read_file_or_url=None, timeout=None, except url_helper.UrlError as e: if e.code == 404 and not optional: raise MAASSeedDirMalformed( - "Missing required %s: %s" % (path, e)) + "Missing required %s: %s" % (path, e) + ) from e elif e.code != 404: raise e diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py index 12b1f94f..45481938 100644 --- a/cloudinit/sources/DataSourceOpenNebula.py +++ b/cloudinit/sources/DataSourceOpenNebula.py @@ -399,18 +399,23 @@ def read_context_disk_dir(source_dir, distro, asuser=None): if asuser is not None: try: pwd.getpwnam(asuser) - except KeyError: + except KeyError as e: raise BrokenContextDiskDir( "configured user '{user}' does not exist".format( - user=asuser)) + user=asuser) + ) from e try: path = os.path.join(source_dir, 'context.sh') content = util.load_file(path) context = parse_shell_config(content, asuser=asuser) except subp.ProcessExecutionError as e: - raise BrokenContextDiskDir("Error processing context.sh: %s" % (e)) + raise BrokenContextDiskDir( + "Error processing context.sh: %s" % (e) + ) from e except IOError as e: - raise NonContextDiskDir("Error reading context.sh: %s" % (e)) + raise NonContextDiskDir( + "Error reading context.sh: %s" % (e) + ) from e else: raise NonContextDiskDir("Missing context.sh") diff --git a/cloudinit/sources/DataSourceOpenStack.py b/cloudinit/sources/DataSourceOpenStack.py index bf539091..d4b43f44 100644 --- a/cloudinit/sources/DataSourceOpenStack.py +++ b/cloudinit/sources/DataSourceOpenStack.py @@ -194,10 +194,10 @@ class DataSourceOpenStack(openstack.SourceMixin, sources.DataSource): 'timeout': url_params.timeout_seconds}) except openstack.NonReadable as e: raise sources.InvalidMetaDataException(str(e)) - except (openstack.BrokenMetadata, IOError): + except (openstack.BrokenMetadata, IOError) as e: msg = 'Broken metadata address {addr}'.format( addr=self.metadata_address) - raise sources.InvalidMetaDataException(msg) + raise sources.InvalidMetaDataException(msg) from e return result diff --git a/cloudinit/sources/DataSourceSmartOS.py b/cloudinit/sources/DataSourceSmartOS.py index 843b3a2a..f1f903bc 100644 --- a/cloudinit/sources/DataSourceSmartOS.py +++ b/cloudinit/sources/DataSourceSmartOS.py @@ -413,7 +413,9 @@ class JoyentMetadataClient(object): response.append(byte) except OSError as exc: if exc.errno == errno.EAGAIN: - raise JoyentMetadataTimeoutException(msg % as_ascii()) + raise JoyentMetadataTimeoutException( + msg % as_ascii() + ) from exc raise def _write(self, msg): diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index 6156c75b..b968a96f 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -98,8 +98,10 @@ def get_boot_telemetry(): LOG.debug("Collecting boot telemetry") try: kernel_start = float(time.time()) - float(util.uptime()) - except ValueError: - raise RuntimeError("Failed to determine kernel start timestamp") + except ValueError as e: + raise RuntimeError( + "Failed to determine kernel start timestamp" + ) from e try: out, _ = subp.subp(['/bin/systemctl', @@ -116,12 +118,13 @@ def get_boot_telemetry(): user_start = kernel_start + (float(tsm) / 1000000) except subp.ProcessExecutionError as e: - raise RuntimeError("Failed to get UserspaceTimestampMonotonic: %s" - % e) + raise RuntimeError( + "Failed to get UserspaceTimestampMonotonic: %s" % e + ) from e except ValueError as e: - raise RuntimeError("Failed to parse " - "UserspaceTimestampMonotonic from systemd: %s" - % e) + raise RuntimeError( + "Failed to parse UserspaceTimestampMonotonic from systemd: %s" % e + ) from e try: out, _ = subp.subp(['/bin/systemctl', 'show', @@ -137,12 +140,14 @@ def get_boot_telemetry(): cloudinit_activation = kernel_start + (float(tsm) / 1000000) except subp.ProcessExecutionError as e: - raise RuntimeError("Failed to get InactiveExitTimestampMonotonic: %s" - % e) + raise RuntimeError( + "Failed to get InactiveExitTimestampMonotonic: %s" % e + ) from e except ValueError as e: - raise RuntimeError("Failed to parse " - "InactiveExitTimestampMonotonic from systemd: %s" - % e) + raise RuntimeError( + "Failed to parse InactiveExitTimestampMonotonic from systemd: %s" + % e + ) from e evt = events.ReportingEvent( BOOT_EVENT_TYPE, 'boot-telemetry', @@ -642,9 +647,10 @@ class WALinuxAgentShim: try: name = os.path.basename(hook_file).replace('.json', '') dhcp_options[name] = json.loads(util.load_file((hook_file))) - except ValueError: + except ValueError as e: raise ValueError( - '{_file} is not valid JSON data'.format(_file=hook_file)) + '{_file} is not valid JSON data'.format(_file=hook_file) + ) from e return dhcp_options @staticmethod diff --git a/cloudinit/sources/helpers/netlink.py b/cloudinit/sources/helpers/netlink.py index a74a3588..c2ad587b 100644 --- a/cloudinit/sources/helpers/netlink.py +++ b/cloudinit/sources/helpers/netlink.py @@ -74,7 +74,7 @@ def create_bound_netlink_socket(): netlink_socket.setblocking(0) except socket.error as e: msg = "Exception during netlink socket create: %s" % e - raise NetlinkCreateSocketError(msg) + raise NetlinkCreateSocketError(msg) from e LOG.debug("Created netlink socket") return netlink_socket diff --git a/cloudinit/sources/helpers/openstack.py b/cloudinit/sources/helpers/openstack.py index 1050efb0..65e020c5 100644 --- a/cloudinit/sources/helpers/openstack.py +++ b/cloudinit/sources/helpers/openstack.py @@ -280,8 +280,9 @@ class BaseReader(metaclass=abc.ABCMeta): try: data = translator(data) except Exception as e: - raise BrokenMetadata("Failed to process " - "path %s: %s" % (path, e)) + raise BrokenMetadata( + "Failed to process path %s: %s" % (path, e) + ) from e if found: results[name] = data @@ -291,8 +292,9 @@ class BaseReader(metaclass=abc.ABCMeta): try: metadata['random_seed'] = base64.b64decode(random_seed) except (ValueError, TypeError) as e: - raise BrokenMetadata("Badly formatted metadata" - " random_seed entry: %s" % e) + raise BrokenMetadata( + "Badly formatted metadata random_seed entry: %s" % e + ) from e # load any files that were provided files = {} @@ -304,8 +306,9 @@ class BaseReader(metaclass=abc.ABCMeta): try: files[path] = self._read_content_path(item) except Exception as e: - raise BrokenMetadata("Failed to read provided " - "file %s: %s" % (path, e)) + raise BrokenMetadata( + "Failed to read provided file %s: %s" % (path, e) + ) from e results['files'] = files # The 'network_config' item in metadata is a content pointer @@ -317,8 +320,9 @@ class BaseReader(metaclass=abc.ABCMeta): content = self._read_content_path(net_item, decode=True) results['network_config'] = content except IOError as e: - raise BrokenMetadata("Failed to read network" - " configuration: %s" % (e)) + raise BrokenMetadata( + "Failed to read network configuration: %s" % (e) + ) from e # To openstack, user can specify meta ('nova boot --meta=key=value') # and those will appear under metadata['meta']. @@ -370,8 +374,9 @@ class ConfigDriveReader(BaseReader): try: return util.load_json(self._path_read(path)) except Exception as e: - raise BrokenMetadata("Failed to process " - "path %s: %s" % (path, e)) + raise BrokenMetadata( + "Failed to process path %s: %s" % (path, e) + ) from e def read_v1(self): """Reads a version 1 formatted location. @@ -395,16 +400,17 @@ class ConfigDriveReader(BaseReader): path = found[name] try: contents = self._path_read(path) - except IOError: - raise BrokenMetadata("Failed to read: %s" % path) + except IOError as e: + raise BrokenMetadata("Failed to read: %s" % path) from e try: # Disable not-callable pylint check; pylint isn't able to # determine that every member of FILES_V1 has a callable in # the appropriate position md[key] = translator(contents) # pylint: disable=E1102 except Exception as e: - raise BrokenMetadata("Failed to process " - "path %s: %s" % (path, e)) + raise BrokenMetadata( + "Failed to process path %s: %s" % (path, e) + ) from e else: md[key] = copy.deepcopy(default) diff --git a/cloudinit/subp.py b/cloudinit/subp.py index 804ef3ca..3e4efa42 100644 --- a/cloudinit/subp.py +++ b/cloudinit/subp.py @@ -262,7 +262,8 @@ def subp(args, data=None, rcs=None, env=None, capture=True, raise ProcessExecutionError( cmd=args, reason=e, errno=e.errno, stdout="-" if decode else b"-", - stderr="-" if decode else b"-") + stderr="-" if decode else b"-" + ) from e finally: if devnull_fp: devnull_fp.close() diff --git a/cloudinit/url_helper.py b/cloudinit/url_helper.py index f3c0cf9c..caa88435 100644 --- a/cloudinit/url_helper.py +++ b/cloudinit/url_helper.py @@ -95,7 +95,7 @@ def read_file_or_url(url, **kwargs): code = e.errno if e.errno == ENOENT: code = NOT_FOUND - raise UrlError(cause=e, code=code, headers=None, url=url) + raise UrlError(cause=e, code=code, headers=None, url=url) from e return FileResponse(file_path, contents=contents) else: return readurl(url, **kwargs) @@ -575,8 +575,8 @@ def oauth_headers(url, consumer_key, token_key, token_secret, consumer_secret, timestamp=None): try: import oauthlib.oauth1 as oauth1 - except ImportError: - raise NotImplementedError('oauth support is not available') + except ImportError as e: + raise NotImplementedError('oauth support is not available') from e if timestamp: timestamp = str(timestamp) diff --git a/cloudinit/util.py b/cloudinit/util.py index dd263803..cf9e349f 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -359,7 +359,7 @@ def decomp_gzip(data, quiet=True, decode=True): if quiet: return data else: - raise DecompressionError(str(e)) + raise DecompressionError(str(e)) from e def extract_usergroup(ug_pair): @@ -1363,7 +1363,7 @@ def chownbyname(fname, user=None, group=None): if group: gid = grp.getgrnam(group).gr_gid except KeyError as e: - raise OSError("Unknown user or group: %s" % (e)) + raise OSError("Unknown user or group: %s" % (e)) from e chownbyid(fname, uid, gid) @@ -2387,8 +2387,8 @@ def human2bytes(size): try: num = float(num) - except ValueError: - raise ValueError("'%s' is not valid input." % size_in) + except ValueError as e: + raise ValueError("'%s' is not valid input." % size_in) from e if num < 0: raise ValueError("'%s': cannot be negative" % size_in) diff --git a/tests/cloud_tests/platforms/azurecloud/instance.py b/tests/cloud_tests/platforms/azurecloud/instance.py index a136cf0d..eedbaae8 100644 --- a/tests/cloud_tests/platforms/azurecloud/instance.py +++ b/tests/cloud_tests/platforms/azurecloud/instance.py @@ -134,9 +134,10 @@ class AzureCloudInstance(Instance): self.vm_name, vm_params) LOG.debug('creating instance %s from image_id=%s', self.vm_name, self.image_id) - except CloudError: - raise RuntimeError('failed creating instance:\n{}'.format( - traceback.format_exc())) + except CloudError as e: + raise RuntimeError( + 'failed creating instance:\n{}'.format(traceback.format_exc()) + ) from e if wait: self.instance.wait() diff --git a/tests/cloud_tests/platforms/azurecloud/platform.py b/tests/cloud_tests/platforms/azurecloud/platform.py index cb62a74b..a664f612 100644 --- a/tests/cloud_tests/platforms/azurecloud/platform.py +++ b/tests/cloud_tests/platforms/azurecloud/platform.py @@ -59,9 +59,12 @@ class AzureCloudPlatform(Platform): self.vnet = self._create_vnet() self.subnet = self._create_subnet() self.nic = self._create_nic() - except CloudError: - raise RuntimeError('failed creating a resource:\n{}'.format( - traceback.format_exc())) + except CloudError as e: + raise RuntimeError( + 'failed creating a resource:\n{}'.format( + traceback.format_exc() + ) + ) from e def create_instance(self, properties, config, features, image_id, user_data=None): @@ -105,8 +108,10 @@ class AzureCloudPlatform(Platform): if image_id.find('__') > 0: image_id = image_id.split('__')[1] LOG.debug('image_id shortened to %s', image_id) - except KeyError: - raise RuntimeError('no images found for %s' % img_conf['release']) + except KeyError as e: + raise RuntimeError( + 'no images found for %s' % img_conf['release'] + ) from e return AzureCloudImage(self, img_conf, image_id) @@ -140,9 +145,11 @@ class AzureCloudPlatform(Platform): secret=azure_creds['clientSecret'], tenant=azure_creds['tenantId']) return credentials, subscription_id - except KeyError: - raise RuntimeError('Please configure Azure service principal' - ' credentials in %s' % cred_file) + except KeyError as e: + raise RuntimeError( + 'Please configure Azure service principal' + ' credentials in %s' % cred_file + ) from e def _create_resource_group(self): """Create resource group""" diff --git a/tests/cloud_tests/platforms/ec2/instance.py b/tests/cloud_tests/platforms/ec2/instance.py index ab6037b1..d2e84047 100644 --- a/tests/cloud_tests/platforms/ec2/instance.py +++ b/tests/cloud_tests/platforms/ec2/instance.py @@ -49,11 +49,11 @@ class EC2Instance(Instance): # OutputBytes comes from platform._decode_console_output_as_bytes response = self.instance.console_output() return response['OutputBytes'] - except KeyError: + except KeyError as e: if 'Output' in response: msg = ("'OutputBytes' did not exist in console_output() but " "'Output' did: %s..." % response['Output'][0:128]) - raise util.PlatformError('console_log', msg) + raise util.PlatformError('console_log', msg) from e return ('No Console Output [%s]' % self.instance).encode() def destroy(self): diff --git a/tests/cloud_tests/platforms/ec2/platform.py b/tests/cloud_tests/platforms/ec2/platform.py index 7a3d0fe0..b61a2ffb 100644 --- a/tests/cloud_tests/platforms/ec2/platform.py +++ b/tests/cloud_tests/platforms/ec2/platform.py @@ -35,12 +35,14 @@ class EC2Platform(Platform): self.ec2_resource = b3session.resource('ec2') self.ec2_region = b3session.region_name self.key_name = self._upload_public_key(config) - except botocore.exceptions.NoRegionError: + except botocore.exceptions.NoRegionError as e: raise RuntimeError( - 'Please configure default region in $HOME/.aws/config') - except botocore.exceptions.NoCredentialsError: + 'Please configure default region in $HOME/.aws/config' + ) from e + except botocore.exceptions.NoCredentialsError as e: raise RuntimeError( - 'Please configure ec2 credentials in $HOME/.aws/credentials') + 'Please configure ec2 credentials in $HOME/.aws/credentials' + ) from e self.vpc = self._create_vpc() self.internet_gateway = self._create_internet_gateway() @@ -125,8 +127,10 @@ class EC2Platform(Platform): try: image_ami = image['id'] - except KeyError: - raise RuntimeError('No images found for %s!' % img_conf['release']) + except KeyError as e: + raise RuntimeError( + 'No images found for %s!' % img_conf['release'] + ) from e LOG.debug('found image: %s', image_ami) image = EC2Image(self, img_conf, image_ami) @@ -195,7 +199,7 @@ class EC2Platform(Platform): CidrBlock=self.ipv4_cidr, AmazonProvidedIpv6CidrBlock=True) except botocore.exceptions.ClientError as e: - raise RuntimeError(e) + raise RuntimeError(e) from e vpc.wait_until_available() self._tag_resource(vpc) diff --git a/tests/cloud_tests/platforms/lxd/instance.py b/tests/cloud_tests/platforms/lxd/instance.py index b27b9848..2b973a08 100644 --- a/tests/cloud_tests/platforms/lxd/instance.py +++ b/tests/cloud_tests/platforms/lxd/instance.py @@ -175,7 +175,8 @@ class LXDInstance(Instance): raise PlatformError( "console log", "Console log failed [%d]: stdout=%s stderr=%s" % ( - e.exit_code, e.stdout, e.stderr)) + e.exit_code, e.stdout, e.stderr) + ) from e def reboot(self, wait=True): """Reboot instance.""" diff --git a/tests/cloud_tests/platforms/platforms.py b/tests/cloud_tests/platforms/platforms.py index 58f65e52..ac3b6563 100644 --- a/tests/cloud_tests/platforms/platforms.py +++ b/tests/cloud_tests/platforms/platforms.py @@ -74,8 +74,10 @@ class Platform(object): try: return tmirror.json_entries[0] - except IndexError: - raise RuntimeError('no images found with filter: %s' % img_filter) + except IndexError as e: + raise RuntimeError( + 'no images found with filter: %s' % img_filter + ) from e class FilterMirror(mirrors.BasicMirrorWriter): diff --git a/tests/cloud_tests/testcases/__init__.py b/tests/cloud_tests/testcases/__init__.py index e8c371ca..bb9785d3 100644 --- a/tests/cloud_tests/testcases/__init__.py +++ b/tests/cloud_tests/testcases/__init__.py @@ -21,8 +21,10 @@ def discover_test(test_name): config.name_sanitize(test_name)) try: testmod = importlib.import_module(testmod_name) - except NameError: - raise ValueError('no test verifier found at: {}'.format(testmod_name)) + except NameError as e: + raise ValueError( + 'no test verifier found at: {}'.format(testmod_name) + ) from e found = [mod for name, mod in inspect.getmembers(testmod) if (inspect.isclass(mod) diff --git a/tools/mock-meta.py b/tools/mock-meta.py index a58e0260..9dd067b9 100755 --- a/tools/mock-meta.py +++ b/tools/mock-meta.py @@ -258,12 +258,14 @@ class MetaDataHandler(object): try: key_id = int(mybe_key) key_name = key_ids[key_id] - except ValueError: - raise WebException(hclient.BAD_REQUEST, - "%s: not an integer" % mybe_key) - except IndexError: - raise WebException(hclient.NOT_FOUND, - "Unknown key id %r" % mybe_key) + except ValueError as e: + raise WebException( + hclient.BAD_REQUEST, "%s: not an integer" % mybe_key + ) from e + except IndexError as e: + raise WebException( + hclient.NOT_FOUND, "Unknown key id %r" % mybe_key + ) from e # Extract the possible sub-params result = traverse(nparams[1:], { "openssh-key": "\n".join(avail_keys[key_name]), diff --git a/tox.ini b/tox.ini index f619dbf5..a92c63e0 100644 --- a/tox.ini +++ b/tox.ini @@ -23,7 +23,7 @@ setenv = basepython = python3 deps = # requirements - pylint==2.5.3 + pylint==2.6.0 # test-requirements because unit tests are now present in cloudinit tree -r{toxinidir}/test-requirements.txt -r{toxinidir}/integration-requirements.txt -- cgit v1.2.3