From bae9b11da9ed7dd0b16fe5adeaf4774b7cc628cf Mon Sep 17 00:00:00 2001 From: James Falcon Date: Wed, 15 Dec 2021 20:16:38 -0600 Subject: Adopt Black and isort (SC-700) (#1157) Applied Black and isort, fixed any linting issues, updated tox.ini and CI. --- pyproject.toml | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 pyproject.toml (limited to 'pyproject.toml') diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 00000000..26734e62 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,8 @@ +[tool.black] +line-length = 79 + +[tool.isort] +profile = "black" +line_length = 79 +# We patch logging in main.py before certain imports +skip = ["cloudinit/cmd/main.py", ".tox", "packages", "tools"] -- cgit v1.2.3 From 9ba3ca6ce5013207737e431f52735c7eda3a2fbb Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Mon, 14 Feb 2022 12:09:01 -0500 Subject: mypy: introduce type checking (#1254) All currently failing modules are excluded from reporting errors using follow-imports=silent and an exclusion list. Future work can whittle down this failing list. This change will start enforcing new modules and those currently passing. Includes some minor alphabetical reordering in tox.ini. Signed-off-by: Chris Patterson --- .travis.yml | 2 + pyproject.toml | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ tox.ini | 38 ++++++++++++++++--- 3 files changed, 149 insertions(+), 6 deletions(-) (limited to 'pyproject.toml') diff --git a/.travis.yml b/.travis.yml index 2e79f2b7..f655fa50 100644 --- a/.travis.yml +++ b/.travis.yml @@ -132,6 +132,8 @@ matrix: dist: bionic - python: 3.6 env: TOXENV=flake8 + - python: 3.6 + env: TOXENV=mypy - python: 3.6 env: TOXENV=pylint - python: 3.6 diff --git a/pyproject.toml b/pyproject.toml index 26734e62..a131037f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,3 +6,118 @@ profile = "black" line_length = 79 # We patch logging in main.py before certain imports skip = ["cloudinit/cmd/main.py", ".tox", "packages", "tools"] + +[tool.mypy] +follow_imports = "silent" +exclude=[ + '^cloudinit/apport\.py$', + '^cloudinit/cmd/query\.py$', + '^cloudinit/config/cc_apk_configure\.py$', + '^cloudinit/config/cc_apt_configure\.py$', + '^cloudinit/config/cc_apt_pipelining\.py$', + '^cloudinit/config/cc_bootcmd\.py$', + '^cloudinit/config/cc_byobu\.py$', + '^cloudinit/config/cc_ca_certs\.py$', + '^cloudinit/config/cc_chef\.py$', + '^cloudinit/config/cc_debug\.py$', + '^cloudinit/config/cc_disable_ec2_metadata\.py$', + '^cloudinit/config/cc_disk_setup\.py$', + '^cloudinit/config/cc_install_hotplug\.py$', + '^cloudinit/config/cc_keyboard\.py$', + '^cloudinit/config/cc_landscape\.py$', + '^cloudinit/config/cc_locale\.py$', + '^cloudinit/config/cc_mcollective\.py$', + '^cloudinit/config/cc_ntp\.py$', + '^cloudinit/config/cc_resizefs\.py$', + '^cloudinit/config/cc_rsyslog\.py$', + '^cloudinit/config/cc_runcmd\.py$', + '^cloudinit/config/cc_snap\.py$', + '^cloudinit/config/cc_ubuntu_advantage\.py$', + '^cloudinit/config/cc_ubuntu_drivers\.py$', + '^cloudinit/config/cc_write_files\.py$', + '^cloudinit/config/cc_write_files_deferred\.py$', + '^cloudinit/config/cc_zypper_add_repo\.py$', + '^cloudinit/config/schema\.py$', + '^cloudinit/distros/bsd\.py$', + '^cloudinit/distros/freebsd\.py$', + '^cloudinit/distros/parsers/networkmanager_conf\.py$', + '^cloudinit/distros/parsers/resolv_conf\.py$', + '^cloudinit/distros/parsers/sys_conf\.py$', + '^cloudinit/dmi\.py$', + '^cloudinit/features\.py$', + '^cloudinit/handlers/cloud_config\.py$', + '^cloudinit/handlers/jinja_template\.py$', + '^cloudinit/importer\.py$', + '^cloudinit/net/__init__\.py$', + '^cloudinit/net/dhcp\.py$', + '^cloudinit/net/netplan\.py$', + '^cloudinit/net/sysconfig\.py$', + '^cloudinit/serial\.py$', + '^cloudinit/sources/DataSourceAzure\.py$', + '^cloudinit/sources/DataSourceAliYun\.py$', + '^cloudinit/sources/DataSourceLXD\.py$', + '^cloudinit/sources/DataSourceOracle\.py$', + '^cloudinit/sources/DataSourceScaleway\.py$', + '^cloudinit/sources/DataSourceSmartOS\.py$', + '^cloudinit/sources/DataSourceVMware\.py$', + '^cloudinit/sources/__init__\.py$', + '^cloudinit/sources/helpers/azure\.py$', + '^cloudinit/sources/helpers/vmware/imc/config_file\.py$', + '^cloudinit/stages\.py$', + '^cloudinit/templater\.py$', + '^cloudinit/url_helper\.py$', + '^conftest\.py$', + '^doc/rtd/conf\.py$', + '^setup\.py$', + '^tests/integration_tests/clouds\.py$', + '^tests/integration_tests/conftest\.py$', + '^tests/integration_tests/instances\.py$', + '^tests/integration_tests/integration_settings\.py$', + '^tests/integration_tests/modules/test_disk_setup\.py$', + '^tests/integration_tests/modules/test_growpart\.py$', + '^tests/integration_tests/modules/test_ssh_keysfile\.py$', + '^tests/unittests/__init__\.py$', + '^tests/unittests/cmd/devel/test_render\.py$', + '^tests/unittests/cmd/test_clean\.py$', + '^tests/unittests/cmd/test_cloud_id\.py$', + '^tests/unittests/cmd/test_main\.py$', + '^tests/unittests/cmd/test_query\.py$', + '^tests/unittests/cmd/test_status\.py$', + '^tests/unittests/config/test_cc_chef\.py$', + '^tests/unittests/config/test_cc_landscape\.py$', + '^tests/unittests/config/test_cc_locale\.py$', + '^tests/unittests/config/test_cc_mcollective\.py$', + '^tests/unittests/config/test_cc_rh_subscription\.py$', + '^tests/unittests/config/test_cc_set_hostname\.py$', + '^tests/unittests/config/test_cc_snap\.py$', + '^tests/unittests/config/test_cc_timezone\.py$', + '^tests/unittests/config/test_cc_ubuntu_advantage\.py$', + '^tests/unittests/config/test_cc_ubuntu_drivers\.py$', + '^tests/unittests/config/test_schema\.py$', + '^tests/unittests/helpers\.py$', + '^tests/unittests/net/test_dhcp\.py$', + '^tests/unittests/net/test_init\.py$', + '^tests/unittests/sources/test_aliyun\.py$', + '^tests/unittests/sources/test_azure\.py$', + '^tests/unittests/sources/test_ec2\.py$', + '^tests/unittests/sources/test_exoscale\.py$', + '^tests/unittests/sources/test_gce\.py$', + '^tests/unittests/sources/test_lxd\.py$', + '^tests/unittests/sources/test_opennebula\.py$', + '^tests/unittests/sources/test_openstack\.py$', + '^tests/unittests/sources/test_rbx\.py$', + '^tests/unittests/sources/test_scaleway\.py$', + '^tests/unittests/sources/test_smartos\.py$', + '^tests/unittests/test_data\.py$', + '^tests/unittests/test_ds_identify\.py$', + '^tests/unittests/test_ec2_util\.py$', + '^tests/unittests/test_net\.py$', + '^tests/unittests/test_net_activators\.py$', + '^tests/unittests/test_persistence\.py$', + '^tests/unittests/test_sshutil\.py$', + '^tests/unittests/test_subp\.py$', + '^tests/unittests/test_templating\.py$', + '^tests/unittests/test_url_helper\.py$', + '^tests/unittests/test_util\.py$', + '^tools/mock-meta\.py$', +] diff --git a/tox.ini b/tox.ini index aab6d4d4..c494cb94 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py3, lowest-supported-dev, flake8, pylint, black, isort +envlist = py3, lowest-supported-dev, black, flake8, isort, mypy, pylint recreate = True [testenv] @@ -10,10 +10,15 @@ passenv= PYTEST_ADDOPTS [format_deps] -flake8==3.9.2 -pylint==2.11.1 black==21.12b0 +flake8==3.9.2 isort==5.10.1 +mypy==0.931 +pylint==2.11.1 +pytest==7.0.0 +types-PyYAML==6.0.4 +types-requests==2.27.8 +types-setuptools==57.4.9 [testenv:flake8] deps = @@ -37,18 +42,30 @@ deps = isort=={[format_deps]isort} commands = {envpython} -m isort . --check-only +[testenv:mypy] +deps = + mypy=={[format_deps]mypy} + types-pyyaml=={[format_deps]types-PyYAML} + types-requests=={[format_deps]types-requests} + types-setuptools=={[format_deps]types-setuptools} + pytest=={[format_deps]pytest} +commands = {envpython} -m mypy . + [testenv:check_format] deps = - flake8=={[format_deps]flake8} - pylint=={[format_deps]pylint} black=={[format_deps]black} + flake8=={[format_deps]flake8} isort=={[format_deps]isort} + mypy=={[format_deps]mypy} + pylint=={[format_deps]pylint} + pytest=={[format_deps]pytest} -r{toxinidir}/test-requirements.txt -r{toxinidir}/integration-requirements.txt commands = {[testenv:black]commands} - {[testenv:isort]commands} {[testenv:flake8]commands} + {[testenv:isort]commands} + {[testenv:mypy]commands} {[testenv:pylint]commands} [testenv:do_format] @@ -122,6 +139,15 @@ commands = commands = {envpython} -m flake8 {posargs:cloudinit/ tests/ tools/ setup.py} deps = flake8 +[testenv:tip-mypy] +commands = {envpython} -m mypy --install-types --non-interactive . +deps = + mypy + pytest + types-PyYAML + types-requests + types-setuptools + [testenv:tip-pylint] commands = {envpython} -m pylint {posargs:cloudinit tests tools} deps = -- cgit v1.2.3 From 50195ec8d1052d2b7a80f47a3709ebc4e74d6764 Mon Sep 17 00:00:00 2001 From: Brett Holman Date: Mon, 14 Feb 2022 12:24:00 -0700 Subject: use PEP 589 syntax for TypeDict (#1253) Use PEP 589 syntax for TypeDict annotation. Also fixes previously broken typing MetaSchema typing implementation. --- cloudinit/config/cc_apk_configure.py | 4 ++-- cloudinit/config/cc_apt_configure.py | 4 ++-- cloudinit/config/cc_apt_pipelining.py | 4 ++-- cloudinit/config/cc_bootcmd.py | 4 ++-- cloudinit/config/cc_byobu.py | 4 ++-- cloudinit/config/cc_ca_certs.py | 4 ++-- cloudinit/config/cc_chef.py | 4 ++-- cloudinit/config/cc_debug.py | 4 ++-- cloudinit/config/cc_disable_ec2_metadata.py | 4 ++-- cloudinit/config/cc_disk_setup.py | 4 ++-- cloudinit/config/cc_install_hotplug.py | 8 ++++++-- cloudinit/config/cc_keyboard.py | 8 ++++++-- cloudinit/config/cc_locale.py | 8 ++++++-- cloudinit/config/cc_ntp.py | 8 ++++++-- cloudinit/config/cc_resizefs.py | 8 ++++++-- cloudinit/config/cc_runcmd.py | 8 ++++++-- cloudinit/config/cc_snap.py | 8 ++++++-- cloudinit/config/cc_ubuntu_advantage.py | 8 ++++++-- cloudinit/config/cc_ubuntu_drivers.py | 8 ++++++-- cloudinit/config/cc_write_files.py | 8 ++++++-- cloudinit/config/cc_zypper_add_repo.py | 4 ++-- cloudinit/importer.py | 26 ++++++++++++-------------- pyproject.toml | 19 ------------------- tests/unittests/config/test_schema.py | 26 ++++++++++++-------------- 24 files changed, 106 insertions(+), 89 deletions(-) (limited to 'pyproject.toml') diff --git a/cloudinit/config/cc_apk_configure.py b/cloudinit/config/cc_apk_configure.py index 2cb2dad1..0952c971 100644 --- a/cloudinit/config/cc_apk_configure.py +++ b/cloudinit/config/cc_apk_configure.py @@ -10,7 +10,7 @@ from textwrap import dedent from cloudinit import log as logging from cloudinit import temp_utils, templater, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) @@ -53,7 +53,7 @@ REPOSITORIES_TEMPLATE = """\ frequency = PER_INSTANCE distros = ["alpine"] -meta = { +meta: MetaSchema = { "id": "cc_apk_configure", "name": "APK Configure", "title": "Configure apk repositories file", diff --git a/cloudinit/config/cc_apt_configure.py b/cloudinit/config/cc_apt_configure.py index 7fe0e343..c558311a 100644 --- a/cloudinit/config/cc_apt_configure.py +++ b/cloudinit/config/cc_apt_configure.py @@ -17,7 +17,7 @@ from textwrap import dedent from cloudinit import gpg from cloudinit import log as logging from cloudinit import subp, templater, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) @@ -32,7 +32,7 @@ CLOUD_INIT_GPG_DIR = "/etc/apt/cloud-init.gpg.d/" frequency = PER_INSTANCE distros = ["ubuntu", "debian"] -meta = { +meta: MetaSchema = { "id": "cc_apt_configure", "name": "Apt Configure", "title": "Configure apt for the user", diff --git a/cloudinit/config/cc_apt_pipelining.py b/cloudinit/config/cc_apt_pipelining.py index 34b6ac0e..901633d3 100644 --- a/cloudinit/config/cc_apt_pipelining.py +++ b/cloudinit/config/cc_apt_pipelining.py @@ -9,7 +9,7 @@ from textwrap import dedent from cloudinit import util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_INSTANCE frequency = PER_INSTANCE @@ -24,7 +24,7 @@ APT_PIPE_TPL = ( # A value of zero MUST be specified if the remote host does not properly linger # on TCP connections - otherwise data corruption will occur. -meta = { +meta: MetaSchema = { "id": "cc_apt_pipelining", "name": "Apt Pipelining", "title": "Configure apt pipelining", diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py index 3a239376..bd14aede 100644 --- a/cloudinit/config/cc_bootcmd.py +++ b/cloudinit/config/cc_bootcmd.py @@ -13,14 +13,14 @@ import os from textwrap import dedent from cloudinit import subp, temp_utils, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_ALWAYS frequency = PER_ALWAYS distros = ["all"] -meta = { +meta: MetaSchema = { "id": "cc_bootcmd", "name": "Bootcmd", "title": "Run arbitrary commands early in the boot process", diff --git a/cloudinit/config/cc_byobu.py b/cloudinit/config/cc_byobu.py index b96736a4..fbc20410 100755 --- a/cloudinit/config/cc_byobu.py +++ b/cloudinit/config/cc_byobu.py @@ -9,7 +9,7 @@ """Byobu: Enable/disable byobu system wide and for default user.""" from cloudinit import subp, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.distros import ug_util from cloudinit.settings import PER_INSTANCE @@ -32,7 +32,7 @@ Valid configuration options for this module are: """ distros = ["ubuntu", "debian"] -meta = { +meta: MetaSchema = { "id": "cc_byobu", "name": "Byobu", "title": "Enable/disable byobu system wide and for default user", diff --git a/cloudinit/config/cc_ca_certs.py b/cloudinit/config/cc_ca_certs.py index c46d0fbe..6084cb4c 100644 --- a/cloudinit/config/cc_ca_certs.py +++ b/cloudinit/config/cc_ca_certs.py @@ -8,7 +8,7 @@ import os from textwrap import dedent from cloudinit import subp, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_INSTANCE DEFAULT_CONFIG = { @@ -45,7 +45,7 @@ can be removed from the system with the configuration option """ distros = ["alpine", "debian", "ubuntu", "rhel"] -meta = { +meta: MetaSchema = { "id": "cc_ca_certs", "name": "CA Certificates", "title": "Add ca certificates", diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index aaf7eaf1..fdb3a6e3 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -14,7 +14,7 @@ import os from textwrap import dedent from cloudinit import subp, temp_utils, templater, url_helper, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_ALWAYS RUBY_VERSION_DEFAULT = "1.8" @@ -92,7 +92,7 @@ CHEF_EXEC_DEF_ARGS = tuple(["-d", "-i", "1800", "-s", "20"]) frequency = PER_ALWAYS distros = ["all"] -meta = { +meta: MetaSchema = { "id": "cc_chef", "name": "Chef", "title": "module that configures, starts and installs chef", diff --git a/cloudinit/config/cc_debug.py b/cloudinit/config/cc_debug.py index 1a3c9346..c51818c3 100644 --- a/cloudinit/config/cc_debug.py +++ b/cloudinit/config/cc_debug.py @@ -9,7 +9,7 @@ from io import StringIO from textwrap import dedent from cloudinit import safeyaml, type_utils, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.distros import ALL_DISTROS from cloudinit.settings import PER_INSTANCE @@ -24,7 +24,7 @@ location that this cloud-init has been configured with when running. Log configurations are not output. """ -meta = { +meta: MetaSchema = { "id": "cc_debug", "name": "Debug", "title": "Helper to debug cloud-init *internal* datastructures", diff --git a/cloudinit/config/cc_disable_ec2_metadata.py b/cloudinit/config/cc_disable_ec2_metadata.py index 6a5e7eda..88cc28e2 100644 --- a/cloudinit/config/cc_disable_ec2_metadata.py +++ b/cloudinit/config/cc_disable_ec2_metadata.py @@ -11,14 +11,14 @@ from textwrap import dedent from cloudinit import subp, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.distros import ALL_DISTROS from cloudinit.settings import PER_ALWAYS REJECT_CMD_IF = ["route", "add", "-host", "169.254.169.254", "reject"] REJECT_CMD_IP = ["ip", "route", "add", "prohibit", "169.254.169.254"] -meta = { +meta: MetaSchema = { "id": "cc_disable_ec2_metadata", "name": "Disable EC2 Metadata", "title": "Disable AWS EC2 Metadata", diff --git a/cloudinit/config/cc_disk_setup.py b/cloudinit/config/cc_disk_setup.py index c59d00cd..ee05ea87 100644 --- a/cloudinit/config/cc_disk_setup.py +++ b/cloudinit/config/cc_disk_setup.py @@ -13,7 +13,7 @@ import shlex from textwrap import dedent from cloudinit import subp, util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.distros import ALL_DISTROS from cloudinit.settings import PER_INSTANCE @@ -50,7 +50,7 @@ the ``fs_setup`` directive. This config directive accepts a list of filesystem configs. """ -meta = { +meta: MetaSchema = { "id": "cc_disk_setup", "name": "Disk Setup", "title": "Configure partitions and filesystems", diff --git a/cloudinit/config/cc_install_hotplug.py b/cloudinit/config/cc_install_hotplug.py index 952d9f13..34c4557e 100644 --- a/cloudinit/config/cc_install_hotplug.py +++ b/cloudinit/config/cc_install_hotplug.py @@ -4,7 +4,11 @@ import os from textwrap import dedent from cloudinit import stages, subp, util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.distros import ALL_DISTROS from cloudinit.event import EventScope, EventType from cloudinit.settings import PER_INSTANCE @@ -12,7 +16,7 @@ from cloudinit.settings import PER_INSTANCE frequency = PER_INSTANCE distros = [ALL_DISTROS] -meta = { +meta: MetaSchema = { "id": "cc_install_hotplug", "name": "Install Hotplug", "title": "Install hotplug if supported and enabled", diff --git a/cloudinit/config/cc_keyboard.py b/cloudinit/config/cc_keyboard.py index 17eb9a54..98ef326a 100644 --- a/cloudinit/config/cc_keyboard.py +++ b/cloudinit/config/cc_keyboard.py @@ -10,7 +10,11 @@ from textwrap import dedent from cloudinit import distros from cloudinit import log as logging -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_INSTANCE frequency = PER_INSTANCE @@ -22,7 +26,7 @@ distros = distros.Distro.expand_osfamily(osfamilies) DEFAULT_KEYBOARD_MODEL = "pc105" -meta = { +meta: MetaSchema = { "id": "cc_keyboard", "name": "Keyboard", "title": "Set keyboard layout", diff --git a/cloudinit/config/cc_locale.py b/cloudinit/config/cc_locale.py index 487f58f7..29f6a9b6 100644 --- a/cloudinit/config/cc_locale.py +++ b/cloudinit/config/cc_locale.py @@ -11,12 +11,16 @@ from textwrap import dedent from cloudinit import util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_INSTANCE frequency = PER_INSTANCE distros = ["all"] -meta = { +meta: MetaSchema = { "id": "cc_locale", "name": "Locale", "title": "Set system locale", diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index a31da9bb..25bba764 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -12,7 +12,11 @@ from textwrap import dedent from cloudinit import log as logging from cloudinit import subp, temp_utils, templater, type_utils, util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) @@ -148,7 +152,7 @@ DISTRO_CLIENT_CONFIG = { # configuration options before actually attempting to deploy with said # configuration. -meta = { +meta: MetaSchema = { "id": "cc_ntp", "name": "NTP", "title": "enable and configure ntp", diff --git a/cloudinit/config/cc_resizefs.py b/cloudinit/config/cc_resizefs.py index b009c392..19b923a8 100644 --- a/cloudinit/config/cc_resizefs.py +++ b/cloudinit/config/cc_resizefs.py @@ -14,7 +14,11 @@ import stat from textwrap import dedent from cloudinit import subp, util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_ALWAYS NOBLOCK = "noblock" @@ -22,7 +26,7 @@ NOBLOCK = "noblock" frequency = PER_ALWAYS distros = ["all"] -meta = { +meta: MetaSchema = { "id": "cc_resizefs", "name": "Resizefs", "title": "Resize filesystem", diff --git a/cloudinit/config/cc_runcmd.py b/cloudinit/config/cc_runcmd.py index 15cbaf1a..ef37a823 100644 --- a/cloudinit/config/cc_runcmd.py +++ b/cloudinit/config/cc_runcmd.py @@ -12,7 +12,11 @@ import os from textwrap import dedent from cloudinit import util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.distros import ALL_DISTROS from cloudinit.settings import PER_INSTANCE @@ -24,7 +28,7 @@ from cloudinit.settings import PER_INSTANCE distros = [ALL_DISTROS] -meta = { +meta: MetaSchema = { "id": "cc_runcmd", "name": "Runcmd", "title": "Run arbitrary commands", diff --git a/cloudinit/config/cc_snap.py b/cloudinit/config/cc_snap.py index 9c38046c..9f343df0 100644 --- a/cloudinit/config/cc_snap.py +++ b/cloudinit/config/cc_snap.py @@ -9,7 +9,11 @@ from textwrap import dedent from cloudinit import log as logging from cloudinit import subp, util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_INSTANCE from cloudinit.subp import prepend_base_command @@ -18,7 +22,7 @@ frequency = PER_INSTANCE LOG = logging.getLogger(__name__) -meta = { +meta: MetaSchema = { "id": "cc_snap", "name": "Snap", "title": "Install, configure and manage snapd and snap packages", diff --git a/cloudinit/config/cc_ubuntu_advantage.py b/cloudinit/config/cc_ubuntu_advantage.py index 9239f7de..e469bb22 100644 --- a/cloudinit/config/cc_ubuntu_advantage.py +++ b/cloudinit/config/cc_ubuntu_advantage.py @@ -6,14 +6,18 @@ from textwrap import dedent from cloudinit import log as logging from cloudinit import subp, util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_INSTANCE UA_URL = "https://ubuntu.com/advantage" distros = ["ubuntu"] -meta = { +meta: MetaSchema = { "id": "cc_ubuntu_advantage", "name": "Ubuntu Advantage", "title": "Configure Ubuntu Advantage support services", diff --git a/cloudinit/config/cc_ubuntu_drivers.py b/cloudinit/config/cc_ubuntu_drivers.py index 6c8494c8..44a3bdb4 100644 --- a/cloudinit/config/cc_ubuntu_drivers.py +++ b/cloudinit/config/cc_ubuntu_drivers.py @@ -7,14 +7,18 @@ from textwrap import dedent from cloudinit import log as logging from cloudinit import subp, temp_utils, type_utils, util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_INSTANCE LOG = logging.getLogger(__name__) frequency = PER_INSTANCE distros = ["ubuntu"] -meta = { +meta: MetaSchema = { "id": "cc_ubuntu_drivers", "name": "Ubuntu Drivers", "title": "Interact with third party drivers in Ubuntu.", diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py index 2c580328..37dae392 100644 --- a/cloudinit/config/cc_write_files.py +++ b/cloudinit/config/cc_write_files.py @@ -12,7 +12,11 @@ from textwrap import dedent from cloudinit import log as logging from cloudinit import util -from cloudinit.config.schema import get_meta_doc, validate_cloudconfig_schema +from cloudinit.config.schema import ( + MetaSchema, + get_meta_doc, + validate_cloudconfig_schema, +) from cloudinit.settings import PER_INSTANCE frequency = PER_INSTANCE @@ -43,7 +47,7 @@ supported_encoding_types = [ "base64", ] -meta = { +meta: MetaSchema = { "id": "cc_write_files", "name": "Write Files", "title": "write arbitrary files", diff --git a/cloudinit/config/cc_zypper_add_repo.py b/cloudinit/config/cc_zypper_add_repo.py index 41605b97..be444cce 100644 --- a/cloudinit/config/cc_zypper_add_repo.py +++ b/cloudinit/config/cc_zypper_add_repo.py @@ -12,12 +12,12 @@ import configobj from cloudinit import log as logging from cloudinit import util -from cloudinit.config.schema import get_meta_doc +from cloudinit.config.schema import MetaSchema, get_meta_doc from cloudinit.settings import PER_ALWAYS distros = ["opensuse", "sles"] -meta = { +meta: MetaSchema = { "id": "cc_zypper_add_repo", "name": "ZypperAddRepo", "title": "Configure zypper behavior and add zypper repositories", diff --git a/cloudinit/importer.py b/cloudinit/importer.py index f84ff4da..2bc210dd 100644 --- a/cloudinit/importer.py +++ b/cloudinit/importer.py @@ -12,21 +12,19 @@ import sys import typing # annotations add value for development, but don't break old versions -# pyver: 3.5 -> 3.8 +# pyver: 3.6 -> 3.8 # pylint: disable=E1101 -if sys.version_info >= (3, 8) and hasattr(typing, "TypeDict"): - MetaSchema = typing.TypedDict( - "MetaSchema", - { - "name": str, - "id": str, - "title": str, - "description": str, - "distros": typing.List[str], - "examples": typing.List[str], - "frequency": str, - }, - ) +if sys.version_info >= (3, 8): + + class MetaSchema(typing.TypedDict): + name: str + id: str + title: str + description: str + distros: typing.List[str] + examples: typing.List[str] + frequency: str + else: MetaSchema = dict # pylint: enable=E1101 diff --git a/pyproject.toml b/pyproject.toml index a131037f..52093fac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,29 +12,11 @@ follow_imports = "silent" exclude=[ '^cloudinit/apport\.py$', '^cloudinit/cmd/query\.py$', - '^cloudinit/config/cc_apk_configure\.py$', - '^cloudinit/config/cc_apt_configure\.py$', - '^cloudinit/config/cc_apt_pipelining\.py$', - '^cloudinit/config/cc_bootcmd\.py$', - '^cloudinit/config/cc_byobu\.py$', - '^cloudinit/config/cc_ca_certs\.py$', '^cloudinit/config/cc_chef\.py$', - '^cloudinit/config/cc_debug\.py$', - '^cloudinit/config/cc_disable_ec2_metadata\.py$', - '^cloudinit/config/cc_disk_setup\.py$', - '^cloudinit/config/cc_install_hotplug\.py$', '^cloudinit/config/cc_keyboard\.py$', '^cloudinit/config/cc_landscape\.py$', - '^cloudinit/config/cc_locale\.py$', '^cloudinit/config/cc_mcollective\.py$', - '^cloudinit/config/cc_ntp\.py$', - '^cloudinit/config/cc_resizefs\.py$', '^cloudinit/config/cc_rsyslog\.py$', - '^cloudinit/config/cc_runcmd\.py$', - '^cloudinit/config/cc_snap\.py$', - '^cloudinit/config/cc_ubuntu_advantage\.py$', - '^cloudinit/config/cc_ubuntu_drivers\.py$', - '^cloudinit/config/cc_write_files\.py$', '^cloudinit/config/cc_write_files_deferred\.py$', '^cloudinit/config/cc_zypper_add_repo\.py$', '^cloudinit/config/schema\.py$', @@ -47,7 +29,6 @@ exclude=[ '^cloudinit/features\.py$', '^cloudinit/handlers/cloud_config\.py$', '^cloudinit/handlers/jinja_template\.py$', - '^cloudinit/importer\.py$', '^cloudinit/net/__init__\.py$', '^cloudinit/net/dhcp\.py$', '^cloudinit/net/netplan\.py$', diff --git a/tests/unittests/config/test_schema.py b/tests/unittests/config/test_schema.py index 2f43d9e7..1d48056a 100644 --- a/tests/unittests/config/test_schema.py +++ b/tests/unittests/config/test_schema.py @@ -420,20 +420,18 @@ class GetSchemaDocTest(CiTestCase): "frequency": "frequency", "distros": ["debian", "rhel"], } - self.meta = MetaSchema( - { - "title": "title", - "description": "description", - "id": "id", - "name": "name", - "frequency": "frequency", - "distros": ["debian", "rhel"], - "examples": [ - 'ex1:\n [don\'t, expand, "this"]', - "ex2: true", - ], - } - ) + self.meta: MetaSchema = { + "title": "title", + "description": "description", + "id": "id", + "name": "name", + "frequency": "frequency", + "distros": ["debian", "rhel"], + "examples": [ + 'ex1:\n [don\'t, expand, "this"]', + "ex2: true", + ], + } def test_get_meta_doc_returns_restructured_text(self): """get_meta_doc returns restructured text for a cloudinit schema.""" -- cgit v1.2.3 From 101a62f2389a933676e9d0d20d9f59303b1f1833 Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Tue, 15 Feb 2022 12:19:19 -0500 Subject: sources/azure: report ready in local phase (#1265) Pre-provisioned instances report ready early in the local phase and again in the non-local phase, during setup(). Non-PPS only reports ready during non-local phase. Update the process to report ready during the local phase for all cases. Only attempt to do so if networking is up to prevent stalling boot. We've already waited at least 20 minutes for DHCP if we're provisioning, or 5 minutes for DHCP on normal boot requesting updated network configuration. - Extend _report_ready() with pubkey_info and raise exception on error to consolidate reporting done in _negotiate() and _report_ready(). - Remove setup(), moving relevant logic into crawl_metadata(). - Move remaining _negotiate() logic into _cleanup_markers() and _determine_wireserver_pubkey_info(). These changes effectively fix two issues that were present: (1) _negotiated is incorrectly set to True When failing to report ready. _negotiate() squashed the exception and the return value was not checked. This was probably masked due to the forced removal of obj.pkl on Ubuntu instances, but would be preferable once we start persisting it to prevent unnecessary re-negotiation. (2) provisioning media is not ejected for non-PPS _negotiate() did not pass iso_dev parameter when reporting ready. The host will ensure this operation takes place, but it is preferable to eject /dev/sr0 from within the guest when we're done with it. Lastly, this removes any need for lease file parsing as the wireserver addressed is tracked for ephemeral DHCP. A follow-up PR will remove this now-unused logic. Signed-off-by: Chris Patterson --- cloudinit/sources/DataSourceAzure.py | 114 ++++---- pyproject.toml | 2 - tests/unittests/sources/test_azure.py | 533 +++++++++++++++++++++++++++++++++- 3 files changed, 577 insertions(+), 72 deletions(-) (limited to 'pyproject.toml') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index f8e1dd02..359dfbde 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -317,17 +317,16 @@ class DataSourceAzure(sources.DataSource): [util.get_cfg_by_path(sys_cfg, DS_CFG_PATH, {}), BUILTIN_DS_CONFIG] ) self.dhclient_lease_file = self.ds_cfg.get("dhclient_lease_file") + self._iso_dev = None self._network_config = None self._ephemeral_dhcp_ctx = None self._wireserver_endpoint = DEFAULT_WIRESERVER_ENDPOINT - self.iso_dev = None def _unpickle(self, ci_pkl_version: int) -> None: super()._unpickle(ci_pkl_version) self._ephemeral_dhcp_ctx = None - if not hasattr(self, "iso_dev"): - self.iso_dev = None + self._iso_dev = None self._wireserver_endpoint = DEFAULT_WIRESERVER_ENDPOINT def __str__(self): @@ -441,7 +440,6 @@ class DataSourceAzure(sources.DataSource): cfg = {} files = {} - iso_dev = None if os.path.isfile(REPROVISION_MARKER_FILE): metadata_source = "IMDS" report_diagnostic_event( @@ -462,7 +460,7 @@ class DataSourceAzure(sources.DataSource): src, load_azure_ds_dir ) # save the device for ejection later - iso_dev = src + self._iso_dev = src else: md, userdata_raw, cfg, files = load_azure_ds_dir(src) ovf_is_accessible = True @@ -497,7 +495,7 @@ class DataSourceAzure(sources.DataSource): # not have UDF support. In either case, require IMDS metadata. # If we require IMDS metadata, try harder to obtain networking, waiting # for at least 20 minutes. Otherwise only wait 5 minutes. - requires_imds_metadata = bool(iso_dev) or not ovf_is_accessible + requires_imds_metadata = bool(self._iso_dev) or not ovf_is_accessible timeout_minutes = 5 if requires_imds_metadata else 20 try: self._setup_ephemeral_networking(timeout_minutes=timeout_minutes) @@ -514,8 +512,6 @@ class DataSourceAzure(sources.DataSource): report_diagnostic_event(msg) raise sources.InvalidMetaDataException(msg) - self.iso_dev = iso_dev - # Refresh PPS type using metadata. pps_type = self._determine_pps_type(cfg, imds_md) if pps_type != PPSType.NONE: @@ -612,9 +608,23 @@ class DataSourceAzure(sources.DataSource): crawled_data["metadata"]["random_seed"] = seed crawled_data["metadata"]["instance-id"] = self._iid() - if pps_type != PPSType.NONE: - LOG.info("Reporting ready to Azure after getting ReprovisionData") - self._report_ready() + if self._negotiated is False and self._is_ephemeral_networking_up(): + # Report ready and fetch public-keys from Wireserver, if required. + pubkey_info = self._determine_wireserver_pubkey_info( + cfg=cfg, imds_md=imds_md + ) + try: + ssh_keys = self._report_ready(pubkey_info=pubkey_info) + except Exception: + # Failed to report ready, but continue with best effort. + pass + else: + LOG.debug("negotiating returned %s", ssh_keys) + if ssh_keys: + crawled_data["metadata"]["public-keys"] = ssh_keys + + self._cleanup_markers() + self._negotiated = True return crawled_data @@ -843,24 +853,6 @@ class DataSourceAzure(sources.DataSource): return previous return iid - @azure_ds_telemetry_reporter - def setup(self, is_new_instance): - if self._negotiated is False: - LOG.debug( - "negotiating for %s (new_instance=%s)", - self.get_instance_id(), - is_new_instance, - ) - ssh_keys = self._negotiate() - LOG.debug("negotiating returned %s", ssh_keys) - if ssh_keys: - self.metadata["public-keys"] = ssh_keys - self._negotiated = True - else: - LOG.debug( - "negotiating already done for %s", self.get_instance_id() - ) - @azure_ds_telemetry_reporter def _wait_for_nic_detach(self, nl_sock): """Use the netlink socket provided to wait for nic detach event. @@ -983,11 +975,12 @@ class DataSourceAzure(sources.DataSource): :raises sources.InvalidMetaDataException: On error reporting ready. """ - report_ready_succeeded = self._report_ready() - if not report_ready_succeeded: + try: + self._report_ready() + except Exception as error: msg = "Failed reporting ready while in the preprovisioning pool." report_diagnostic_event(msg, logger_func=LOG.error) - raise sources.InvalidMetaDataException(msg) + raise sources.InvalidMetaDataException(msg) from error self._create_report_ready_marker() @@ -1400,25 +1393,36 @@ class DataSourceAzure(sources.DataSource): return False - def _report_ready(self) -> bool: + @azure_ds_telemetry_reporter + def _report_ready( + self, *, pubkey_info: Optional[List[str]] = None + ) -> Optional[List[str]]: """Tells the fabric provisioning has completed. - @return: The success status of sending the ready signal. + :param pubkey_info: Fingerprints of keys to request from Wireserver. + + :raises Exception: if failed to report. + + :returns: List of SSH keys, if requested. """ try: - get_metadata_from_fabric( + data = get_metadata_from_fabric( fallback_lease_file=None, dhcp_opts=self._wireserver_endpoint, - iso_dev=self.iso_dev, + iso_dev=self._iso_dev, + pubkey_info=pubkey_info, ) - return True except Exception as e: report_diagnostic_event( "Error communicating with Azure fabric; You may experience " "connectivity issues: %s" % e, logger_func=LOG.warning, ) - return False + raise + + # Reporting ready ejected OVF media, no need to do so again. + self._iso_dev = None + return data def _ppstype_from_imds(self, imds_md: dict) -> Optional[str]: try: @@ -1464,6 +1468,7 @@ class DataSourceAzure(sources.DataSource): "{pid}: {time}\n".format(pid=os.getpid(), time=time()), ) + @azure_ds_telemetry_reporter def _reprovision(self): """Initiate the reprovisioning workflow. @@ -1479,40 +1484,29 @@ class DataSourceAzure(sources.DataSource): return (md, ud, cfg, {"ovf-env.xml": contents}) @azure_ds_telemetry_reporter - def _negotiate(self): - """Negotiate with fabric and return data from it. + def _determine_wireserver_pubkey_info( + self, *, cfg: dict, imds_md: dict + ) -> Optional[List[str]]: + """Determine the fingerprints we need to retrieve from Wireserver. - On success, returns a dictionary including 'public_keys'. - On failure, returns False. + :return: List of keys to request from Wireserver, if any, else None. """ - pubkey_info = None + pubkey_info: Optional[List[str]] = None try: - self._get_public_keys_from_imds(self.metadata["imds"]) + self._get_public_keys_from_imds(imds_md) except (KeyError, ValueError): - pubkey_info = self.cfg.get("_pubkeys", None) + pubkey_info = cfg.get("_pubkeys", None) log_msg = "Retrieved {} fingerprints from OVF".format( len(pubkey_info) if pubkey_info is not None else 0 ) report_diagnostic_event(log_msg, logger_func=LOG.debug) + return pubkey_info - LOG.debug("negotiating with fabric") - try: - ssh_keys = get_metadata_from_fabric( - fallback_lease_file=self.dhclient_lease_file, - pubkey_info=pubkey_info, - ) - except Exception as e: - report_diagnostic_event( - "Error communicating with Azure fabric; You may experience " - "connectivity issues: %s" % e, - logger_func=LOG.warning, - ) - return False - + def _cleanup_markers(self): + """Cleanup any marker files.""" util.del_file(REPORTED_READY_MARKER_FILE) util.del_file(REPROVISION_MARKER_FILE) util.del_file(REPROVISION_NIC_DETACHED_MARKER_FILE) - return ssh_keys @azure_ds_telemetry_reporter def activate(self, cfg, is_new_instance): diff --git a/pyproject.toml b/pyproject.toml index 52093fac..324d6f35 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,6 @@ exclude=[ '^cloudinit/net/netplan\.py$', '^cloudinit/net/sysconfig\.py$', '^cloudinit/serial\.py$', - '^cloudinit/sources/DataSourceAzure\.py$', '^cloudinit/sources/DataSourceAliYun\.py$', '^cloudinit/sources/DataSourceLXD\.py$', '^cloudinit/sources/DataSourceOracle\.py$', @@ -42,7 +41,6 @@ exclude=[ '^cloudinit/sources/DataSourceSmartOS\.py$', '^cloudinit/sources/DataSourceVMware\.py$', '^cloudinit/sources/__init__\.py$', - '^cloudinit/sources/helpers/azure\.py$', '^cloudinit/sources/helpers/vmware/imc/config_file\.py$', '^cloudinit/stages\.py$', '^cloudinit/templater\.py$', diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py index ecedc54d..5f956a63 100644 --- a/tests/unittests/sources/test_azure.py +++ b/tests/unittests/sources/test_azure.py @@ -41,24 +41,162 @@ MOCKPATH = "cloudinit.sources.DataSourceAzure." @pytest.fixture -def azure_ds(request, paths): +def azure_ds(paths): """Provide DataSourceAzure instance with mocks for minimal test case.""" with mock.patch(MOCKPATH + "_is_platform_viable", return_value=True): yield dsaz.DataSourceAzure(sys_cfg={}, distro=mock.Mock(), paths=paths) +@pytest.fixture +def mock_azure_helper_readurl(): + with mock.patch( + "cloudinit.sources.helpers.azure.url_helper.readurl", autospec=True + ) as m: + yield m + + +@pytest.fixture +def mock_azure_get_metadata_from_fabric(): + with mock.patch( + MOCKPATH + "get_metadata_from_fabric", + autospec=True, + ) as m: + yield m + + +@pytest.fixture +def mock_azure_report_failure_to_fabric(): + with mock.patch( + MOCKPATH + "report_failure_to_fabric", + autospec=True, + ) as m: + yield m + + +@pytest.fixture +def mock_dmi_read_dmi_data(): + def fake_read(key: str) -> str: + if key == "system-uuid": + return "fake-system-uuid" + raise RuntimeError() + + with mock.patch( + MOCKPATH + "dmi.read_dmi_data", + side_effect=fake_read, + autospec=True, + ) as m: + yield m + + +@pytest.fixture +def mock_net_dhcp_maybe_perform_dhcp_discovery(): + with mock.patch( + "cloudinit.net.dhcp.maybe_perform_dhcp_discovery", + return_value=[ + { + "unknown-245": "aa:bb:cc:dd", + "interface": "ethBoot0", + "fixed-address": "192.168.2.9", + "routers": "192.168.2.1", + "subnet-mask": "255.255.255.0", + } + ], + autospec=True, + ) as m: + yield m + + +@pytest.fixture +def mock_net_dhcp_EphemeralIPv4Network(): + with mock.patch( + "cloudinit.net.dhcp.EphemeralIPv4Network", + autospec=True, + ) as m: + yield m + + @pytest.fixture def mock_get_interfaces(): - """Mock for net.get_interfaces().""" with mock.patch(MOCKPATH + "net.get_interfaces", return_value=[]) as m: yield m @pytest.fixture def mock_get_interface_mac(): - """Mock for net.get_interface_mac().""" with mock.patch( - MOCKPATH + "net.get_interface_mac", return_value="001122334455" + MOCKPATH + "net.get_interface_mac", + return_value="001122334455", + ) as m: + yield m + + +@pytest.fixture +def mock_netlink(): + with mock.patch( + MOCKPATH + "netlink", + autospec=True, + ) as m: + yield m + + +@pytest.fixture +def mock_os_path_isfile(): + with mock.patch(MOCKPATH + "os.path.isfile", autospec=True) as m: + yield m + + +@pytest.fixture +def mock_readurl(): + with mock.patch(MOCKPATH + "readurl", autospec=True) as m: + yield m + + +@pytest.fixture +def mock_subp_subp(): + with mock.patch(MOCKPATH + "subp.subp", side_effect=[]) as m: + yield m + + +@pytest.fixture +def mock_util_ensure_dir(): + with mock.patch( + MOCKPATH + "util.ensure_dir", + autospec=True, + ) as m: + yield m + + +@pytest.fixture +def mock_util_find_devs_with(): + with mock.patch(MOCKPATH + "util.find_devs_with", autospec=True) as m: + yield m + + +@pytest.fixture +def mock_util_load_file(): + with mock.patch( + MOCKPATH + "util.load_file", + autospec=True, + return_value=b"", + ) as m: + yield m + + +@pytest.fixture +def mock_util_mount_cb(): + with mock.patch( + MOCKPATH + "util.mount_cb", + autospec=True, + return_value=({}, "", {}, {}), + ) as m: + yield m + + +@pytest.fixture +def mock_util_write_file(): + with mock.patch( + MOCKPATH + "util.write_file", + autospec=True, ) as m: yield m @@ -1259,7 +1397,10 @@ scbus-1 on xpt0 bus 0 dsrc.crawl_metadata() - assert m_report_ready.mock_calls == [mock.call(), mock.call()] + assert m_report_ready.mock_calls == [ + mock.call(), + mock.call(pubkey_info=None), + ] def test_waagent_d_has_0700_perms(self): # we expect /var/lib/waagent to be created 0700 @@ -1637,12 +1778,23 @@ scbus-1 on xpt0 bus 0 def test_dsaz_report_ready_returns_true_when_report_succeeds(self): dsrc = self._get_ds({"ovfcontent": construct_valid_ovf_env()}) - self.assertTrue(dsrc._report_ready()) + assert dsrc._report_ready() == [] - def test_dsaz_report_ready_returns_false_and_does_not_propagate_exc(self): + @mock.patch(MOCKPATH + "report_diagnostic_event") + def test_dsaz_report_ready_failure_reports_telemetry(self, m_report_diag): dsrc = self._get_ds({"ovfcontent": construct_valid_ovf_env()}) - self.m_get_metadata_from_fabric.side_effect = Exception - self.assertFalse(dsrc._report_ready()) + self.m_get_metadata_from_fabric.side_effect = Exception("foo") + + with pytest.raises(Exception): + dsrc._report_ready() + + assert m_report_diag.mock_calls == [ + mock.call( + "Error communicating with Azure fabric; " + "You may experience connectivity issues: foo", + logger_func=dsaz.LOG.warning, + ) + ] def test_dsaz_report_failure_returns_true_when_report_succeeds(self): dsrc = self._get_ds({"ovfcontent": construct_valid_ovf_env()}) @@ -3316,7 +3468,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): } ] m_media_switch.return_value = None - m_report_ready.return_value = False + m_report_ready.side_effect = [Exception("fail")] dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths) self.assertFalse(os.path.exists(report_file)) with mock.patch(MOCKPATH + "REPORTED_READY_MARKER_FILE", report_file): @@ -3568,6 +3720,367 @@ class TestRandomSeed(CiTestCase): self.assertEqual(deserialized["seed"], result) +class TestProvisioning: + @pytest.fixture(autouse=True) + def provisioning_setup( + self, + azure_ds, + mock_azure_get_metadata_from_fabric, + mock_azure_report_failure_to_fabric, + mock_net_dhcp_maybe_perform_dhcp_discovery, + mock_net_dhcp_EphemeralIPv4Network, + mock_dmi_read_dmi_data, + mock_get_interfaces, + mock_get_interface_mac, + mock_netlink, + mock_os_path_isfile, + mock_readurl, + mock_subp_subp, + mock_util_ensure_dir, + mock_util_find_devs_with, + mock_util_load_file, + mock_util_mount_cb, + mock_util_write_file, + ): + self.azure_ds = azure_ds + self.mock_azure_get_metadata_from_fabric = ( + mock_azure_get_metadata_from_fabric + ) + self.mock_azure_report_failure_to_fabric = ( + mock_azure_report_failure_to_fabric + ) + self.mock_net_dhcp_maybe_perform_dhcp_discovery = ( + mock_net_dhcp_maybe_perform_dhcp_discovery + ) + self.mock_net_dhcp_EphemeralIPv4Network = ( + mock_net_dhcp_EphemeralIPv4Network + ) + self.mock_dmi_read_dmi_data = mock_dmi_read_dmi_data + self.mock_get_interfaces = mock_get_interfaces + self.mock_get_interface_mac = mock_get_interface_mac + self.mock_netlink = mock_netlink + self.mock_os_path_isfile = mock_os_path_isfile + self.mock_readurl = mock_readurl + self.mock_subp_subp = mock_subp_subp + self.mock_util_ensure_dir = mock_util_ensure_dir + self.mock_util_find_devs_with = mock_util_find_devs_with + self.mock_util_load_file = mock_util_load_file + self.mock_util_mount_cb = mock_util_mount_cb + self.mock_util_write_file = mock_util_write_file + + self.imds_md = { + "extended": {"compute": {"ppsType": "None"}}, + "network": { + "interface": [ + { + "ipv4": { + "ipAddress": [ + { + "privateIpAddress": "10.0.0.22", + "publicIpAddress": "", + } + ], + "subnet": [ + {"address": "10.0.0.0", "prefix": "24"} + ], + }, + "ipv6": {"ipAddress": []}, + "macAddress": "011122334455", + }, + ] + }, + } + + def test_no_pps(self): + self.mock_readurl.side_effect = [ + mock.MagicMock(contents=json.dumps(self.imds_md).encode()), + ] + self.mock_azure_get_metadata_from_fabric.return_value = [] + self.mock_os_path_isfile.side_effect = [False, False, False] + + self.azure_ds._get_data() + + assert self.mock_os_path_isfile.mock_calls == [ + mock.call("/var/lib/cloud/data/poll_imds"), + mock.call( + os.path.join( + self.azure_ds.paths.cloud_dir, "seed/azure/ovf-env.xml" + ) + ), + mock.call("/var/lib/cloud/data/poll_imds"), + ] + + assert self.mock_readurl.mock_calls == [ + mock.call( + "http://169.254.169.254/metadata/instance?" + "api-version=2021-08-01&extended=true", + timeout=2, + headers={"Metadata": "true"}, + retries=0, + exception_cb=dsaz.retry_on_url_exc, + infinite=False, + ), + ] + + # Verify DHCP is setup once. + assert self.mock_net_dhcp_maybe_perform_dhcp_discovery.mock_calls == [ + mock.call(None, dsaz.dhcp_log_cb) + ] + assert self.azure_ds._wireserver_endpoint == "aa:bb:cc:dd" + assert self.azure_ds._is_ephemeral_networking_up() is False + + # Verify DMI usage. + assert self.mock_dmi_read_dmi_data.mock_calls == [ + mock.call("system-uuid") + ] + assert self.azure_ds.metadata["instance-id"] == "fake-system-uuid" + + # Verify IMDS metadata. + assert self.azure_ds.metadata["imds"] == self.imds_md + + # Verify reporting ready once. + assert self.mock_azure_get_metadata_from_fabric.mock_calls == [ + mock.call( + fallback_lease_file=None, + dhcp_opts="aa:bb:cc:dd", + iso_dev="/dev/sr0", + pubkey_info=None, + ) + ] + + # Verify netlink. + assert self.mock_netlink.mock_calls == [] + + def test_running_pps(self): + self.imds_md["extended"]["compute"]["ppsType"] = "Running" + ovf_data = {"HostName": "myhost", "UserName": "myuser"} + + nl_sock = mock.MagicMock() + self.mock_netlink.create_bound_netlink_socket.return_value = nl_sock + self.mock_readurl.side_effect = [ + mock.MagicMock(contents=json.dumps(self.imds_md).encode()), + mock.MagicMock( + contents=construct_valid_ovf_env(data=ovf_data).encode() + ), + mock.MagicMock(contents=json.dumps(self.imds_md).encode()), + ] + self.mock_azure_get_metadata_from_fabric.return_value = [] + self.mock_os_path_isfile.side_effect = [False, False, False, False] + + self.azure_ds._get_data() + + assert self.mock_os_path_isfile.mock_calls == [ + mock.call("/var/lib/cloud/data/poll_imds"), + mock.call( + os.path.join( + self.azure_ds.paths.cloud_dir, "seed/azure/ovf-env.xml" + ) + ), + mock.call("/var/lib/cloud/data/poll_imds"), + mock.call("/var/lib/cloud/data/reported_ready"), + ] + + assert self.mock_readurl.mock_calls == [ + mock.call( + "http://169.254.169.254/metadata/instance?" + "api-version=2021-08-01&extended=true", + timeout=2, + headers={"Metadata": "true"}, + retries=0, + exception_cb=dsaz.retry_on_url_exc, + infinite=False, + ), + mock.call( + "http://169.254.169.254/metadata/reprovisiondata?" + "api-version=2019-06-01", + timeout=2, + headers={"Metadata": "true"}, + exception_cb=mock.ANY, + infinite=True, + log_req_resp=False, + ), + mock.call( + "http://169.254.169.254/metadata/instance?" + "api-version=2021-08-01&extended=true", + timeout=2, + headers={"Metadata": "true"}, + retries=0, + exception_cb=dsaz.retry_on_url_exc, + infinite=False, + ), + ] + + # Verify DHCP is setup twice. + assert self.mock_net_dhcp_maybe_perform_dhcp_discovery.mock_calls == [ + mock.call(None, dsaz.dhcp_log_cb), + mock.call(None, dsaz.dhcp_log_cb), + ] + assert self.azure_ds._wireserver_endpoint == "aa:bb:cc:dd" + assert self.azure_ds._is_ephemeral_networking_up() is False + + # Verify DMI usage. + assert self.mock_dmi_read_dmi_data.mock_calls == [ + mock.call("system-uuid") + ] + assert self.azure_ds.metadata["instance-id"] == "fake-system-uuid" + + # Verify IMDS metadata. + assert self.azure_ds.metadata["imds"] == self.imds_md + + # Verify reporting ready twice. + assert self.mock_azure_get_metadata_from_fabric.mock_calls == [ + mock.call( + fallback_lease_file=None, + dhcp_opts="aa:bb:cc:dd", + iso_dev="/dev/sr0", + pubkey_info=None, + ), + mock.call( + fallback_lease_file=None, + dhcp_opts="aa:bb:cc:dd", + iso_dev=None, + pubkey_info=None, + ), + ] + + # Verify netlink operations for Running PPS. + assert self.mock_netlink.mock_calls == [ + mock.call.create_bound_netlink_socket(), + mock.call.wait_for_media_disconnect_connect(mock.ANY, "ethBoot0"), + mock.call.create_bound_netlink_socket().__bool__(), + mock.call.create_bound_netlink_socket().close(), + ] + + def test_savable_pps(self): + self.imds_md["extended"]["compute"]["ppsType"] = "Savable" + ovf_data = {"HostName": "myhost", "UserName": "myuser"} + + nl_sock = mock.MagicMock() + self.mock_netlink.create_bound_netlink_socket.return_value = nl_sock + self.mock_netlink.wait_for_nic_detach_event.return_value = "eth9" + self.mock_netlink.wait_for_nic_attach_event.return_value = ( + "ethAttached1" + ) + self.mock_readurl.side_effect = [ + mock.MagicMock(contents=json.dumps(self.imds_md).encode()), + mock.MagicMock( + contents=json.dumps(self.imds_md["network"]).encode() + ), + mock.MagicMock( + contents=construct_valid_ovf_env(data=ovf_data).encode() + ), + mock.MagicMock(contents=json.dumps(self.imds_md).encode()), + ] + self.mock_azure_get_metadata_from_fabric.return_value = [] + self.mock_os_path_isfile.side_effect = [ + False, # /var/lib/cloud/data/poll_imds + False, # seed/azure/ovf-env.xml + False, # /var/lib/cloud/data/poll_imds + False, # /var/lib/cloud/data/reported_ready + False, # /var/lib/cloud/data/reported_ready + False, # /var/lib/cloud/data/nic_detached + True, # /var/lib/cloud/data/reported_ready + ] + self.azure_ds._fallback_interface = False + + self.azure_ds._get_data() + + assert self.mock_os_path_isfile.mock_calls == [ + mock.call("/var/lib/cloud/data/poll_imds"), + mock.call( + os.path.join( + self.azure_ds.paths.cloud_dir, "seed/azure/ovf-env.xml" + ) + ), + mock.call("/var/lib/cloud/data/poll_imds"), + mock.call("/var/lib/cloud/data/reported_ready"), + mock.call("/var/lib/cloud/data/reported_ready"), + mock.call("/var/lib/cloud/data/nic_detached"), + mock.call("/var/lib/cloud/data/reported_ready"), + ] + + assert self.mock_readurl.mock_calls == [ + mock.call( + "http://169.254.169.254/metadata/instance?" + "api-version=2021-08-01&extended=true", + timeout=2, + headers={"Metadata": "true"}, + retries=0, + exception_cb=dsaz.retry_on_url_exc, + infinite=False, + ), + mock.call( + "http://169.254.169.254/metadata/instance/network?" + "api-version=2019-06-01", + timeout=2, + headers={"Metadata": "true"}, + retries=0, + exception_cb=mock.ANY, + infinite=True, + ), + mock.call( + "http://169.254.169.254/metadata/reprovisiondata?" + "api-version=2019-06-01", + timeout=2, + headers={"Metadata": "true"}, + exception_cb=mock.ANY, + infinite=True, + log_req_resp=False, + ), + mock.call( + "http://169.254.169.254/metadata/instance?" + "api-version=2021-08-01&extended=true", + timeout=2, + headers={"Metadata": "true"}, + retries=0, + exception_cb=dsaz.retry_on_url_exc, + infinite=False, + ), + ] + + # Verify DHCP is setup twice. + assert self.mock_net_dhcp_maybe_perform_dhcp_discovery.mock_calls == [ + mock.call(None, dsaz.dhcp_log_cb), + mock.call("ethAttached1", dsaz.dhcp_log_cb), + ] + assert self.azure_ds._wireserver_endpoint == "aa:bb:cc:dd" + assert self.azure_ds._is_ephemeral_networking_up() is False + + # Verify DMI usage. + assert self.mock_dmi_read_dmi_data.mock_calls == [ + mock.call("system-uuid") + ] + assert self.azure_ds.metadata["instance-id"] == "fake-system-uuid" + + # Verify IMDS metadata. + assert self.azure_ds.metadata["imds"] == self.imds_md + + # Verify reporting ready twice. + assert self.mock_azure_get_metadata_from_fabric.mock_calls == [ + mock.call( + fallback_lease_file=None, + dhcp_opts="aa:bb:cc:dd", + iso_dev="/dev/sr0", + pubkey_info=None, + ), + mock.call( + fallback_lease_file=None, + dhcp_opts="aa:bb:cc:dd", + iso_dev=None, + pubkey_info=None, + ), + ] + + # Verify netlink operations for Savable PPS. + assert self.mock_netlink.mock_calls == [ + mock.call.create_bound_netlink_socket(), + mock.call.wait_for_nic_detach_event(nl_sock), + mock.call.wait_for_nic_attach_event(nl_sock, ["ethAttached1"]), + mock.call.create_bound_netlink_socket().__bool__(), + mock.call.create_bound_netlink_socket().close(), + ] + + class TestValidateIMDSMetadata: @pytest.mark.parametrize( "mac,expected", -- cgit v1.2.3