Skip to content

Fix cloud-init status --wait when no datasource found#1349

Merged
blackboxsw merged 4 commits into
canonical:mainfrom
TheRealFalcon:generate-disabled
Mar 23, 2022
Merged

Fix cloud-init status --wait when no datasource found#1349
blackboxsw merged 4 commits into
canonical:mainfrom
TheRealFalcon:generate-disabled

Conversation

@TheRealFalcon
Copy link
Copy Markdown
Contributor

Proposed Commit Message

Fix cloud-init status --wait when no datasource found

In 0de7acb1, we modified status checks to wait until we get an "enabled"
or "disabled" file from our generator. It never outputs a
"disabled" file, so "status --wait" will wait indefinitely if no
datasource is found.

LP: #1966085

Additional Context

See: #1162

Test Steps

Run tests/integration_tests/cmd/test_status.py

I also manually checked the generator script to verify that enabled and disabled files get created as expected and are mutually exclusive.

In 0de7acb, we modified status checks to wait until we get an "enabled"
or "disabled" file from ds-identiy. ds-identify never outputs a
"disabled" file, so "status --wait" will wait indefinitely if no
datasource is found.

LP: #1966085
Comment thread tests/integration_tests/cmd/test_status.py
Copy link
Copy Markdown
Member

@holmanb holmanb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Comment thread tests/integration_tests/clouds.py
Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. minor suggestion integration test explicit test. I'm running this patch through now on Jammy and impish to validate

Comment thread tests/integration_tests/clouds.py
_remove_nocloud_dir_and_reboot(client)
status = _wait_for_cloud_init(client)
assert status.stdout.strip() == "status: disabled"
assert client.execute("cloud-init status --wait").ok
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change suggestion just to make sure our status is reporting the correct reason for the disable

diff --git a/tests/integration_tests/cmd/test_status.py b/tests/integration_tests/cmd/test_status.py
index 085680154..53bab5fd6 100644
--- a/tests/integration_tests/cmd/test_status.py
+++ b/tests/integration_tests/cmd/test_status.py
@@ -13,7 +13,7 @@ def _wait_for_cloud_init(client: IntegrationInstance):
     last_exception = None
     for _ in range(30):
         try:
-            result = client.execute("cloud-init status")
+            result = client.execute("cloud-init status --long")
             if result and result.ok:
                 return result
         except Exception as e:
@@ -59,6 +59,7 @@ def test_wait_when_no_datasource(session_cloud: IntegrationCloud, setup_image):
             "impish",
         ]:
             _remove_nocloud_dir_and_reboot(client)
-        status = _wait_for_cloud_init(client)
-        assert status.stdout.strip() == "status: disabled"
+        status_out = _wait_for_cloud_init(client).stdout.strip()
+        assert "status: disabled" in status_out
+        assert "Cloud-init disabled by cloud-init-generator" in status_out
         assert client.execute("cloud-init status --wait").ok

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed success with a deb from this PR on impish and jammy runs with the above integration test patch. See expected failure using CLOUD_INIT_CLOUD_INIT_SOURCE=ppa:cloud-init-dev/proposed deb without your fix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, thanks. Applied your patch.

@blackboxsw
Copy link
Copy Markdown
Collaborator

+1 will await CI run on this

@blackboxsw blackboxsw merged commit eee6032 into canonical:main Mar 23, 2022
@blackboxsw
Copy link
Copy Markdown
Collaborator

@valentindavid just FYI this fix is SRU'd to Bionic, Focal and Impish -updates pocket as version 22.1-14-g2e17a0d6-0ubuntu1~20.04.3 so daily cloudimages should have this fix you need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants