diff -Nru cloud-init-23.4/ChangeLog cloud-init-23.4.2/ChangeLog --- cloud-init-23.4/ChangeLog 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/ChangeLog 2024-01-24 17:06:27.000000000 +0000 @@ -1,3 +1,10 @@ +23.4.2 + - fix: Handle invalid user configuration gracefully (#4797) + (LP: #2051147) + +23.4.1 + - fix: Handle systemctl commands when dbus not ready (#4681) + 23.4 - tests: datasourcenone use client.restart to block until done (#4635) - tests: increase number of retries across reboot to 90 (#4651) @@ -1236,7 +1243,7 @@ (#1277) - sources/azure: ensure retries on IMDS request failure (#1271) [Chris Patterson] - - sources/azure: removed unused saveable PPS paths (#1268) [Chris Patterson] + - sources/azure: removed unused savable PPS paths (#1268) [Chris Patterson] - integration tests: fix Azure failures (#1269) 22.1 diff -Nru cloud-init-23.4/cloudinit/cmd/status.py cloud-init-23.4.2/cloudinit/cmd/status.py --- cloud-init-23.4/cloudinit/cmd/status.py 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/cloudinit/cmd/status.py 2024-01-24 17:06:27.000000000 +0000 @@ -137,7 +137,7 @@ """Handle calls to 'cloud-init status' as a subcommand.""" # Read configured paths paths = read_cfg_paths() - details = get_status_details(paths) + details = get_status_details(paths, args.wait) if args.wait: while details.status in ( UXAppStatus.NOT_RUN, @@ -147,7 +147,7 @@ if args.format == "tabular": sys.stdout.write(".") sys.stdout.flush() - details = get_status_details(paths) + details = get_status_details(paths, args.wait) sleep(0.25) details_dict: Dict[str, Union[None, str, List[str], Dict[str, Any]]] = { "datasource": details.datasource, @@ -272,12 +272,15 @@ return (bootstatus_code, reason) -def _get_systemd_status() -> Optional[UXAppStatus]: - """Get status from systemd. +def _get_error_or_running_from_systemd() -> Optional[UXAppStatus]: + """Get if systemd is in error or running state. Using systemd, we can get more fine-grained status of the individual unit. Determine if we're still - running or if there's an error we haven't otherwise detected + running or if there's an error we haven't otherwise detected. + + If we don't detect error or running, return None as we don't want to + report any other particular status based on systemd. """ for service in [ "cloud-final.service", @@ -324,7 +327,42 @@ return None -def get_status_details(paths: Optional[Paths] = None) -> StatusDetails: +def _get_error_or_running_from_systemd_with_retry( + existing_status: UXAppStatus, *, wait: bool +) -> Optional[UXAppStatus]: + """Get systemd status and retry if dbus isn't ready. + + If cloud-init has determined that we're still running, then we can + ignore the status from systemd. However, if cloud-init has detected error, + then we should retry on systemd status so we don't incorrectly report + error state while cloud-init is still running. + """ + while True: + try: + return _get_error_or_running_from_systemd() + except subp.ProcessExecutionError as e: + last_exception = e + if existing_status in ( + UXAppStatus.DEGRADED_RUNNING, + UXAppStatus.RUNNING, + ): + return None + if wait: + sleep(0.25) + else: + break + print( + "Failed to get status from systemd. " + "Cloud-init status may be inaccurate. ", + f"Error from systemctl: {last_exception.stderr}", + file=sys.stderr, + ) + return None + + +def get_status_details( + paths: Optional[Paths] = None, wait: bool = False +) -> StatusDetails: """Return a dict with status, details and errors. @param paths: An initialized cloudinit.helpers.paths object. @@ -395,7 +433,9 @@ UXAppStatus.NOT_RUN, UXAppStatus.DISABLED, ): - systemd_status = _get_systemd_status() + systemd_status = _get_error_or_running_from_systemd_with_retry( + status, wait=wait + ) if systemd_status: status = systemd_status diff -Nru cloud-init-23.4/cloudinit/version.py cloud-init-23.4.2/cloudinit/version.py --- cloud-init-23.4/cloudinit/version.py 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/cloudinit/version.py 2024-01-24 17:06:27.000000000 +0000 @@ -4,7 +4,7 @@ # # This file is part of cloud-init. See LICENSE file for license information. -__VERSION__ = "23.4" +__VERSION__ = "23.4.2" _PACKAGED_VERSION = "@@PACKAGED_VERSION@@" FEATURES = [ diff -Nru cloud-init-23.4/debian/changelog cloud-init-23.4.2/debian/changelog --- cloud-init-23.4/debian/changelog 2023-12-05 13:17:28.000000000 +0000 +++ cloud-init-23.4.2/debian/changelog 2024-01-24 17:57:50.000000000 +0000 @@ -1,8 +1,48 @@ +cloud-init (23.4.2-0ubuntu0~23.10.1) mantic; urgency=medium + + * Upstream snapshot based on 23.4.2. (LP: #2045582). + List of changes from upstream can be found at + https://raw.githubusercontent.com/canonical/cloud-init/23.4.2/ChangeLog + - Bugs fixed in this snapshot: (LP: #2051147) + + -- Alberto Contreras Wed, 24 Jan 2024 18:57:50 +0100 + +cloud-init (23.4.1-0ubuntu1~23.10.2) mantic; urgency=medium + + * d/p/status-retain-recoverable-error-exit-code.patch: + Retain exit code in cloud-init status for recoverable errors. + (LP: #2048522). + + -- Alberto Contreras Mon, 08 Jan 2024 17:00:52 +0100 + +cloud-init (23.4.1-0ubuntu1~23.10.1) mantic; urgency=medium + + * d/p/retain-apt-pre-deb822.patch: + - Disable apt source list generation with DEB822 style + * d/p/do-not-block-user-login.patch: + - revert redacted patch content introduced in 23.4-0 + * refresh patches: + - d/p/status-do-not-remove-duplicated-data.patch + * d/changelog: amend 23.4-0 refresh patches and dropped cherry-picks entry + * Upstream snapshot based on 23.4.1. (LP: #2045582). + List of changes from upstream can be found at + https://raw.githubusercontent.com/canonical/cloud-init/23.4.1/ChangeLog + + -- Alberto Contreras Fri, 15 Dec 2023 12:00:50 +0100 + cloud-init (23.4-0ubuntu1~23.10.1) mantic; urgency=medium * d/p/status-do-not-remove-duplicated-data.patch: - Revert behavior downstream, leave duplicate data * d/control: add python3-apt as Recommends to read APT config from apt_pkg + * refresh patches: + - d/p/do-not-block-user-login.patch + * drop the following cherry-picks now included: + - cpick-0d9f149a-Pytestify-apt-config-test-modules-4424 + - cpick-5023e9f9-Refactor-test_apt_source_v1.py-to-use-pytest-4427 + - cpick-e9cdd7e3-Install-gnupg-if-gpg-not-found-4431 + - cpick-015543d3-apt-install-software-properties-common-when-absent-but + - cpick-2ab1f340-fix-cc_apt_configure-avoid-unneeded-call-to-apt-install * Upstream snapshot based on 23.4. (LP: #2045582). List of changes from upstream can be found at https://raw.githubusercontent.com/canonical/cloud-init/23.4/ChangeLog diff -Nru cloud-init-23.4/debian/patches/do-not-block-user-login.patch cloud-init-23.4.2/debian/patches/do-not-block-user-login.patch --- cloud-init-23.4/debian/patches/do-not-block-user-login.patch 2023-12-05 13:17:28.000000000 +0000 +++ cloud-init-23.4.2/debian/patches/do-not-block-user-login.patch 2024-01-24 17:57:50.000000000 +0000 @@ -16,3 +16,13 @@ Wants=network-online.target cloud-config.target ConditionPathExists=!/etc/cloud/cloud-init.disabled ConditionKernelCommandLine=!cloud-init=disabled +--- a/systemd/cloud-init.service.tmpl ++++ b/systemd/cloud-init.service.tmpl +@@ -38,6 +38,7 @@ Conflicts=shutdown.target + Before=shutdown.target + Conflicts=shutdown.target + {% endif %} ++Before=systemd-user-sessions.service + ConditionPathExists=!/etc/cloud/cloud-init.disabled + ConditionKernelCommandLine=!cloud-init=disabled + ConditionEnvironment=!KERNEL_CMDLINE=cloud-init=disabled diff -Nru cloud-init-23.4/debian/patches/retain-apt-pre-deb822.patch cloud-init-23.4.2/debian/patches/retain-apt-pre-deb822.patch --- cloud-init-23.4/debian/patches/retain-apt-pre-deb822.patch 1970-01-01 00:00:00.000000000 +0000 +++ cloud-init-23.4.2/debian/patches/retain-apt-pre-deb822.patch 2024-01-24 17:57:50.000000000 +0000 @@ -0,0 +1,19 @@ +Description: Disable apt source list generation with DEB822 style + To avoid change of behavior, supported releases won't generate apt source + files with DEB822 style. +Author: Alberto Contreras +Forwarded: not-needed +Last-Update: 2023-12-14 +--- +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ +--- a/cloudinit/features.py ++++ b/cloudinit/features.py +@@ -80,7 +80,7 @@ separators. + (This flag can be removed when Jammy is no longer supported.) + """ + +-APT_DEB822_SOURCE_LIST_FILE = True ++APT_DEB822_SOURCE_LIST_FILE = False + """ + On Debian and Ubuntu systems, cc_apt_configure will write a deb822 compatible + /etc/apt/sources.list.d/(debian|ubuntu).sources file. When set False, continue diff -Nru cloud-init-23.4/debian/patches/series cloud-init-23.4.2/debian/patches/series --- cloud-init-23.4/debian/patches/series 2023-12-05 13:17:28.000000000 +0000 +++ cloud-init-23.4.2/debian/patches/series 2024-01-24 17:57:50.000000000 +0000 @@ -1,2 +1,4 @@ do-not-block-user-login.patch status-do-not-remove-duplicated-data.patch +retain-apt-pre-deb822.patch +status-retain-recoverable-error-exit-code.patch diff -Nru cloud-init-23.4/debian/patches/status-do-not-remove-duplicated-data.patch cloud-init-23.4.2/debian/patches/status-do-not-remove-duplicated-data.patch --- cloud-init-23.4/debian/patches/status-do-not-remove-duplicated-data.patch 2023-12-05 13:17:28.000000000 +0000 +++ cloud-init-23.4.2/debian/patches/status-do-not-remove-duplicated-data.patch 2024-01-24 17:57:50.000000000 +0000 @@ -18,7 +18,7 @@ prefix = "\n" if args.wait else "" --- a/tests/unittests/cmd/test_status.py +++ b/tests/unittests/cmd/test_status.py -@@ -479,6 +479,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr +@@ -487,6 +487,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr dedent( """\ --- @@ -26,7 +26,7 @@ boot_status_code: enabled-by-kernel-cmdline datasource: '' detail: 'Running in stage: init' -@@ -492,6 +493,23 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr +@@ -500,6 +501,23 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr start: 123.45 last_update: Thu, 01 Jan 1970 00:02:04 +0000 recoverable_errors: {} @@ -50,7 +50,7 @@ stage: init status: running ... -@@ -524,6 +542,25 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr +@@ -532,6 +550,25 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr "init-local": {"finished": 123.46, "start": 123.45}, "last_update": "Thu, 01 Jan 1970 00:02:04 +0000", "recoverable_errors": {}, @@ -76,7 +76,7 @@ "stage": "init", }, id="running_json_format", -@@ -555,6 +592,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr +@@ -563,6 +600,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr MyArgs(long=False, wait=False, format="json"), 1, { @@ -84,7 +84,7 @@ "boot_status_code": "enabled-by-kernel-cmdline", "datasource": "nocloud", "detail": ( -@@ -576,6 +614,32 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr +@@ -584,6 +622,32 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr }, "last_update": "Thu, 01 Jan 1970 00:02:05 +0000", "recoverable_errors": {}, @@ -117,7 +117,7 @@ "stage": None, }, id="running_json_format_with_errors", -@@ -638,6 +702,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr +@@ -646,6 +710,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr MyArgs(long=False, wait=False, format="json"), 2, { @@ -125,7 +125,7 @@ "boot_status_code": "enabled-by-kernel-cmdline", "datasource": "nocloud", "detail": ( -@@ -697,6 +762,89 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr +@@ -705,6 +770,89 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr "don't try to open the hatch or we'll all be soup" ], }, diff -Nru cloud-init-23.4/debian/patches/status-retain-recoverable-error-exit-code.patch cloud-init-23.4.2/debian/patches/status-retain-recoverable-error-exit-code.patch --- cloud-init-23.4/debian/patches/status-retain-recoverable-error-exit-code.patch 1970-01-01 00:00:00.000000000 +0000 +++ cloud-init-23.4.2/debian/patches/status-retain-recoverable-error-exit-code.patch 2024-01-24 17:57:50.000000000 +0000 @@ -0,0 +1,28 @@ +Description: Retain exit code in cloud-init status for recoverable errors + (LP: #2048522). +Author: Alberto Contreras +Last-Update: 2024-01-08 +--- +This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ +--- a/cloudinit/cmd/status.py ++++ b/cloudinit/cmd/status.py +@@ -227,7 +227,7 @@ def handle_status_args(name, args) -> in + return 1 + # Recoverable error + elif details.status in UXAppStatusDegradedMap.values(): +- return 2 ++ return 0 + return 0 + + +--- a/tests/unittests/cmd/test_status.py ++++ b/tests/unittests/cmd/test_status.py +@@ -708,7 +708,7 @@ PATH=/usr/local/sbin:/usr/local/bin:/usr + }, + None, + MyArgs(long=False, wait=False, format="json"), +- 2, ++ 0, + { + "_schema_version": "1", + "boot_status_code": "enabled-by-kernel-cmdline", diff -Nru cloud-init-23.4/requirements.txt cloud-init-23.4.2/requirements.txt --- cloud-init-23.4/requirements.txt 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/requirements.txt 2024-01-24 17:06:27.000000000 +0000 @@ -28,7 +28,7 @@ jsonpatch # For validating cloud-config sections per schema definitions -jsonschema +jsonschema<=4.20.0 # Used by DataSourceVMware to inspect the host's network configuration during # the "setup()" function. diff -Nru cloud-init-23.4/test-requirements.txt cloud-init-23.4.2/test-requirements.txt --- cloud-init-23.4/test-requirements.txt 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/test-requirements.txt 2024-01-24 17:06:27.000000000 +0000 @@ -9,6 +9,6 @@ pytest-cov pytest-mock setuptools -jsonschema +jsonschema<=4.20.0 responses passlib diff -Nru cloud-init-23.4/tests/unittests/cmd/test_status.py cloud-init-23.4.2/tests/unittests/cmd/test_status.py --- cloud-init-23.4/tests/unittests/cmd/test_status.py 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/tests/unittests/cmd/test_status.py 2024-01-24 17:06:27.000000000 +0000 @@ -9,9 +9,14 @@ import pytest +from cloudinit import subp from cloudinit.atomic_helper import write_json from cloudinit.cmd import status -from cloudinit.cmd.status import UXAppStatus, _get_systemd_status +from cloudinit.cmd.status import ( + UXAppStatus, + _get_error_or_running_from_systemd, + _get_error_or_running_from_systemd_with_retry, +) from cloudinit.subp import SubpResult from cloudinit.util import ensure_file from tests.unittests.helpers import wrap_and_call @@ -59,7 +64,10 @@ "Cloud-init enabled by systemd cloud-init-generator", ), ) - @mock.patch(f"{M_PATH}_get_systemd_status", return_value=None) + @mock.patch( + f"{M_PATH}_get_error_or_running_from_systemd_with_retry", + return_value=None, + ) def test_get_status_details_ds_none( self, m_get_systemd_status, @@ -704,7 +712,10 @@ ], ) @mock.patch(M_PATH + "read_cfg_paths") - @mock.patch(f"{M_PATH}_get_systemd_status", return_value=None) + @mock.patch( + f"{M_PATH}_get_error_or_running_from_systemd_with_retry", + return_value=None, + ) def test_status_output( self, m_get_systemd_status, @@ -745,7 +756,10 @@ assert out == expected_status @mock.patch(M_PATH + "read_cfg_paths") - @mock.patch(f"{M_PATH}_get_systemd_status", return_value=None) + @mock.patch( + f"{M_PATH}_get_error_or_running_from_systemd_with_retry", + return_value=None, + ) def test_status_wait_blocks_until_done( self, m_get_systemd_status, m_read_cfg_paths, config: Config, capsys ): @@ -796,7 +810,10 @@ assert out == "....\nstatus: done\n" @mock.patch(M_PATH + "read_cfg_paths") - @mock.patch(f"{M_PATH}_get_systemd_status", return_value=None) + @mock.patch( + f"{M_PATH}_get_error_or_running_from_systemd_with_retry", + return_value=None, + ) def test_status_wait_blocks_until_error( self, m_get_systemd_status, m_read_cfg_paths, config: Config, capsys ): @@ -849,7 +866,10 @@ assert out == "....\nstatus: error\n" @mock.patch(M_PATH + "read_cfg_paths") - @mock.patch(f"{M_PATH}_get_systemd_status", return_value=None) + @mock.patch( + f"{M_PATH}_get_error_or_running_from_systemd_with_retry", + return_value=None, + ) def test_status_main( self, m_get_systemd_status, m_read_cfg_paths, config: Config, capsys ): @@ -873,7 +893,12 @@ assert out == "status: running\n" -class TestSystemdStatusDetails: +class TestGetErrorOrRunningFromSystemd: + @pytest.fixture(autouse=True) + def common_mocks(self, mocker): + mocker.patch("cloudinit.cmd.status.sleep") + yield + @pytest.mark.parametrize( ["active_state", "unit_file_state", "sub_state", "main_pid", "status"], [ @@ -881,7 +906,7 @@ # enabled, enabled-runtime, and static into an "enabled" state # and everything else functionally disabled. # Additionally, SubStates are undocumented and may mean something - # different depending on the ActiveState they are mapped too. + # different depending on the ActiveState they are mapped to. # Because of this I'm only testing SubState combinations seen # in real-world testing (or using "any" string if we dont care). ("activating", "enabled", "start", "123", UXAppStatus.RUNNING), @@ -910,7 +935,7 @@ ("failed", "invalid", "failed", "0", UXAppStatus.ERROR), ], ) - def test_get_systemd_status( + def test_get_error_or_running_from_systemd( self, active_state, unit_file_state, sub_state, main_pid, status ): with mock.patch( @@ -923,4 +948,70 @@ stderr=None, ), ): - assert _get_systemd_status() == status + assert _get_error_or_running_from_systemd() == status + + def test_exception_while_running(self, mocker, capsys): + m_subp = mocker.patch( + f"{M_PATH}subp.subp", + side_effect=subp.ProcessExecutionError( + "Message recipient disconnected from message bus without" + " replying" + ), + ) + assert ( + _get_error_or_running_from_systemd_with_retry( + UXAppStatus.RUNNING, wait=True + ) + is None + ) + assert 1 == m_subp.call_count + assert "Failed to get status" not in capsys.readouterr().err + + def test_retry(self, mocker, capsys): + m_subp = mocker.patch( + f"{M_PATH}subp.subp", + side_effect=[ + subp.ProcessExecutionError( + "Message recipient disconnected from message bus without" + " replying" + ), + subp.ProcessExecutionError( + "Message recipient disconnected from message bus without" + " replying" + ), + SubpResult( + "ActiveState=activating\nUnitFileState=enabled\n" + "SubState=start\nMainPID=123\n", + stderr=None, + ), + ], + ) + assert ( + _get_error_or_running_from_systemd_with_retry( + UXAppStatus.ERROR, wait=True + ) + is UXAppStatus.RUNNING + ) + assert 3 == m_subp.call_count + assert "Failed to get status" not in capsys.readouterr().err + + def test_retry_no_wait(self, mocker, capsys): + m_subp = mocker.patch( + f"{M_PATH}subp.subp", + side_effect=subp.ProcessExecutionError( + "Message recipient disconnected from message bus without" + " replying" + ), + ) + mocker.patch("time.time", side_effect=[1, 2, 50]) + assert ( + _get_error_or_running_from_systemd_with_retry( + UXAppStatus.ERROR, wait=False + ) + is None + ) + assert 1 == m_subp.call_count + assert ( + "Failed to get status from systemd. " + "Cloud-init status may be inaccurate." + ) in capsys.readouterr().err diff -Nru cloud-init-23.4/tests/unittests/test_ds_identify.py cloud-init-23.4.2/tests/unittests/test_ds_identify.py --- cloud-init-23.4/tests/unittests/test_ds_identify.py 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/tests/unittests/test_ds_identify.py 2024-01-24 17:06:27.000000000 +0000 @@ -6,6 +6,8 @@ from textwrap import dedent from uuid import uuid4 +import pytest + from cloudinit import atomic_helper, safeyaml, subp, util from cloudinit.sources import DataSourceIBMCloud as ds_ibm from cloudinit.sources import DataSourceOracle as ds_oracle @@ -57,146 +59,6 @@ ] -DEFAULT_CLOUD_CONFIG = """\ -# The top level settings are used as module -# and base configuration. -# A set of users which may be applied and/or used by various modules -# when a 'default' entry is found it will reference the 'default_user' -# from the distro configuration specified below -users: - - default - -# If this is set, 'root' will not be able to ssh in and they -# will get a message to login instead as the default $user -disable_root: true - -# This will cause the set+update hostname module to not operate (if true) -preserve_hostname: false - -# If you use datasource_list array, keep array items in a single line. -# If you use multi line array, ds-identify script won't read array items. -# Example datasource config -# datasource: -# Ec2: -# metadata_urls: [ 'blah.com' ] -# timeout: 5 # (defaults to 50 seconds) -# max_wait: 10 # (defaults to 120 seconds) - -# The modules that run in the 'init' stage -cloud_init_modules: - - migrator - - seed_random - - bootcmd - - write-files - - growpart - - resizefs - - disk_setup - - mounts - - set_hostname - - update_hostname - - update_etc_hosts - - ca-certs - - rsyslog - - users-groups - - ssh - -# The modules that run in the 'config' stage -cloud_config_modules: - - wireguard - - snap - - ubuntu_autoinstall - - ssh-import-id - - keyboard - - locale - - set-passwords - - grub-dpkg - - apt-pipelining - - apt-configure - - ubuntu-advantage - - ntp - - timezone - - disable-ec2-metadata - - runcmd - - byobu - -# The modules that run in the 'final' stage -cloud_final_modules: - - package-update-upgrade-install - - fan - - landscape - - lxd - - ubuntu-drivers - - write-files-deferred - - puppet - - chef - - ansible - - mcollective - - salt-minion - - reset_rmc - - refresh_rmc_and_interface - - rightscale_userdata - - scripts-vendor - - scripts-per-once - - scripts-per-boot - - scripts-per-instance - - scripts-user - - ssh-authkey-fingerprints - - keys-to-console - - install-hotplug - - phone-home - - final-message - - power-state-change - -# System and/or distro specific settings -# (not accessible to handlers/transforms) -system_info: - # This will affect which distro class gets used - distro: ubuntu - # Default user name + that default users groups (if added/used) - default_user: - name: ubuntu - lock_passwd: True - gecos: Ubuntu - groups: [adm, audio, cdrom, floppy, lxd, netdev, plugdev, sudo, video] - sudo: ["ALL=(ALL) NOPASSWD:ALL"] - shell: /bin/bash - network: - renderers: ['netplan', 'eni', 'sysconfig'] - activators: ['netplan', 'eni', 'network-manager', 'networkd'] - # Automatically discover the best ntp_client - ntp_client: auto - # Other config here will be given to the distro class and/or path classes - paths: - cloud_dir: /var/lib/cloud/ - templates_dir: /etc/cloud/templates/ - package_mirrors: - - arches: [i386, amd64] - failsafe: - primary: http://archive.ubuntu.com/ubuntu - security: http://security.ubuntu.com/ubuntu - search: - primary: - - http://%(ec2_region)s.ec2.archive.ubuntu.com/ubuntu/ - - http://%(availability_zone)s.clouds.archive.ubuntu.com/ubuntu/ - - http://%(region)s.clouds.archive.ubuntu.com/ubuntu/ - security: [] - - arches: [arm64, armel, armhf] - failsafe: - primary: http://ports.ubuntu.com/ubuntu-ports - security: http://ports.ubuntu.com/ubuntu-ports - search: - primary: - - http://%(ec2_region)s.ec2.ports.ubuntu.com/ubuntu-ports/ - - http://%(availability_zone)s.clouds.ports.ubuntu.com/ubuntu-ports/ - - http://%(region)s.clouds.ports.ubuntu.com/ubuntu-ports/ - security: [] - - arches: [default] - failsafe: - primary: http://ports.ubuntu.com/ubuntu-ports - security: http://ports.ubuntu.com/ubuntu-ports - ssh_svcname: ssh -""" - POLICY_FOUND_ONLY = "search,found=all,maybe=none,notfound=disabled" POLICY_FOUND_OR_MAYBE = "search,found=all,maybe=all,notfound=disabled" DI_DEFAULT_POLICY = "search,found=all,maybe=all,notfound=disabled" @@ -279,10 +141,6 @@ if files is None: files = {} - cloudcfg = "etc/cloud/cloud.cfg" - if cloudcfg not in files: - files[cloudcfg] = DEFAULT_CLOUD_CONFIG - if rootd is None: rootd = self.tmp_dir() @@ -449,6 +307,65 @@ for var in expected_vars: self.assertIn("{0}=".format(var), err) + @pytest.mark.xfail(reason="GH-4796") + def test_maas_not_detected_1(self): + """Don't incorrectly identify maas + + In ds-identify the function check_config() attempts to parse yaml keys + in bash, but it sometimes introduces false positives. The maas + datasource uses check_config() and the existence of a "MAAS" key to + identify itself (which is a very poor identifier - clouds should have + stricter identifiers). Since the MAAS datasource is at the begining of + the list, this is particularly troublesome and more concerning than + NoCloud false positives, for example. + config = "LXD-kvm-not-MAAS-1" + self._test_ds_found(config) + """ + + def test_maas_not_detected_2(self): + """Don't incorrectly identify maas + + The bug reported in 4794 combined with the previously existing bug + reported in 4796 made for very loose MAAS false-positives. + + In ds-identify the function check_config() attempts to parse yaml keys + in bash, but it sometimes introduces false positives. The maas + datasource uses check_config() and the existence of a "MAAS" key to + identify itself (which is a very poor identifier - clouds should have + stricter identifiers). Since the MAAS datasource is at the begining of + the list, this is particularly troublesome and more concerning than + NoCloud false positives, for example. + """ + config = "LXD-kvm-not-MAAS-2" + self._test_ds_found(config) + + def test_maas_not_detected_3(self): + """Don't incorrectly identify maas + + The bug reported in 4794 combined with the previously existing bug + reported in 4796 made for very loose MAAS false-positives. + + In ds-identify the function check_config() attempts to parse yaml keys + in bash, but it sometimes introduces false positives. The maas + datasource uses check_config() and the existence of a "MAAS" key to + identify itself (which is a very poor identifier - clouds should have + stricter identifiers). Since the MAAS datasource is at the begining of + the list, this is particularly troublesome and more concerning than + NoCloud false positives, for example. + """ + config = "LXD-kvm-not-MAAS-3" + self._test_ds_found(config) + + def test_azure_invalid_configuration(self): + """Don't detect incorrect config when invalid datasource_list provided + + If unparsable list is provided we just ignore it. Some users + might assume that since the rest of the configuration is yaml that + multi-line yaml lists are valid (they aren't). When this happens, just + run ds-identify and figure it out for ourselves which platform to run. + """ + self._test_ds_found("Azure-parse-invalid") + def test_azure_dmi_detection_from_chassis_asset_tag(self): """Azure datasource is detected from DMI chassis-asset-tag""" self._test_ds_found("Azure-dmi-detection") @@ -1223,6 +1140,15 @@ os.path.join(P_SEED_DIR, "azure", "ovf-env.xml"): "present\n", }, }, + "Azure-parse-invalid": { + "ds": "Azure", + "files": { + P_CHASSIS_ASSET_TAG: "7783-7084-3265-9085-8269-3286-77\n", + "etc/cloud/cloud.cfg.d/91-azure_datasource.cfg": ( + "datasource_list:\n - Azure" + ), + }, + }, "Ec2-hvm": { "ds": "Ec2", "mocks": [{"name": "detect_virt", "RET": "kvm", "ret": 0}], @@ -1263,6 +1189,38 @@ "mocks": [{"name": "is_socket_file", "ret": 1}, MOCK_VIRT_IS_KVM], "no_mocks": ["dscheck_LXD"], # Don't default mock dscheck_LXD }, + "LXD-kvm-not-MAAS-1": { + "ds": "LXD", + "files": { + P_BOARD_NAME: "LXD\n", + "etc/cloud/cloud.cfg.d/92-broken-maas.cfg": ( + "datasource:\n MAAS:\n metadata_urls: [ 'blah.com' ]" + ), + }, + # /dev/lxd/sock does not exist and KVM virt-type + "mocks": [{"name": "is_socket_file", "ret": 1}, MOCK_VIRT_IS_KVM], + "no_mocks": ["dscheck_LXD"], # Don't default mock dscheck_LXD + }, + "LXD-kvm-not-MAAS-2": { + "ds": "LXD", + "files": { + P_BOARD_NAME: "LXD\n", + "etc/cloud/cloud.cfg.d/92-broken-maas.cfg": ("#MAAS: None"), + }, + # /dev/lxd/sock does not exist and KVM virt-type + "mocks": [{"name": "is_socket_file", "ret": 1}, MOCK_VIRT_IS_KVM], + "no_mocks": ["dscheck_LXD"], # Don't default mock dscheck_LXD + }, + "LXD-kvm-not-MAAS-3": { + "ds": "LXD", + "files": { + P_BOARD_NAME: "LXD\n", + "etc/cloud/cloud.cfg.d/92-broken-maas.cfg": ("MAAS: None"), + }, + # /dev/lxd/sock does not exist and KVM virt-type + "mocks": [{"name": "is_socket_file", "ret": 1}, MOCK_VIRT_IS_KVM], + "no_mocks": ["dscheck_LXD"], # Don't default mock dscheck_LXD + }, "LXD-kvm-qemu-kernel-gt-5.10": { # LXD host > 5.10 kvm launch virt==qemu "ds": "LXD", "files": {P_BOARD_NAME: "LXD\n"}, @@ -1305,7 +1263,7 @@ # Also include a datasource list of more than just # [NoCloud, None], because that would automatically select # NoCloud without checking - "etc/cloud/cloud.cfg": dedent( + "/etc/cloud/cloud.cfg": dedent( """\ datasource_list: [ Azure, Openstack, NoCloud, None ] datasource: diff -Nru cloud-init-23.4/tools/ds-identify cloud-init-23.4.2/tools/ds-identify --- cloud-init-23.4/tools/ds-identify 2023-12-04 17:51:50.000000000 +0000 +++ cloud-init-23.4.2/tools/ds-identify 2024-01-24 17:06:27.000000000 +0000 @@ -777,24 +777,21 @@ if [ "$1" = "$files" -a ! -f "$1" ]; then return 1 fi - local line="" ret="" found=0 found_fn="" oifs="$IFS" out="" - out=$(grep "$key\"\?:" "$@" 2>/dev/null) - IFS=${CR} - for line in $out; do - # drop '# comment' - line=${line%%#*} - # if more than one file was 'grep'ed, then grep will output filename: - # but if only one file, line will not be prefixed. - if [ $# -eq 1 ]; then - found_fn="$1" - else - found_fn="${line%%:*}" - line=${line#*:} - fi - ret=${line#*: }; - found=$((found+1)) + local fname="" line="" ret="" found=0 found_fn="" + # shellcheck disable=2094 + for fname in "$@"; do + [ -f "$fname" ] || continue + while read line; do + line=${line%%#*} + case "$line" in + $key:\ *|"${key}":) + ret=${line#*:}; + ret=${ret# }; + found=$((found+1)) + found_fn="$fname";; + esac + done <"$fname" done - IFS="$oifs" if [ $found -ne 0 ]; then _RET="$ret" _RET_fname="$found_fn"