Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cmd-modules): exit code when --mode init #5017

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

aciba90
Copy link
Contributor

@aciba90 aciba90 commented Mar 6, 2024

Proposed Commit Message

fix(cmd-modules): exit code when --mode init

Co-authored-by: Chad Smith <chad.smith@canonical.com>

Additional Context

Supersedes #4939
Execute cloud-init modules --mode init on any instance.

Test Steps

Checklist

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@aciba90 aciba90 changed the title chore(cmd-modules): fix exit code when --mode init fix(cmd-modules): exit code when --mode init Mar 6, 2024
@blackboxsw blackboxsw self-assigned this Mar 7, 2024
Copy link
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.

Beyond using class_client, l let's also add a failure test as well.

--- a/tests/integration_tests/modules/test_cli.py
+++ b/tests/integration_tests/modules/test_cli.py
@@ -20,6 +20,12 @@ runcmd:
   - echo 'hi' > /var/tmp/test
 """
 
+FAILING_USER_DATA_RUNCMD = """\
+#cloud-config
+runcmd:
+    - exit 1
+"""
+
 # The '-' in 'hashed-password' fails schema validation
 INVALID_USER_DATA_SCHEMA = """\
 #cloud-config
@@ -37,25 +43,25 @@ users:
 
 @pytest.mark.user_data(VALID_USER_DATA)
 class TestValidUserData:
-    def test_schema_status(self, client: IntegrationInstance):
+    def test_schema_status(self, class_client: IntegrationInstance):
         """Test `cloud-init schema` with valid userdata.
 
         PR #575
         """
-        result = client.execute("cloud-init schema --system")
+        result = class_client.execute("cloud-init schema --system")
         assert result.ok
         assert "Valid schema user-data" in result.stdout.strip()
-        result = client.execute("cloud-init status --long")
+        result = class_client.execute("cloud-init status --long")
         assert 0 == result.return_code, (
             f"Unexpected exit {result.return_code} from cloud-init status:"
             f" {result}"
         )
 
-    @pytest.mark.parametrize("mode", ["init", "config", "final"])
-    def test_modules_init(self, mode, client: IntegrationInstance):
-        result = client.execute(f"cloud-init modules --mode {mode}")
-        assert result.ok
-        assert f"'modules:{mode}'" in result.stdout.strip()
+    def test_modules_init(self, class_client: IntegrationInstance):
+        for mode in ("init", "config", "final"):
+            result = class_client.execute(f"cloud-init modules --mode {mode}")
+            assert result.ok
+            assert f"'modules:{mode}'" in result.stdout.strip()
 
 
 @pytest.mark.skipif(
@@ -105,3 +111,12 @@ def test_invalid_userdata_schema(client: IntegrationInstance):
     )
     assert warning in log
     assert "asdfasdf" not in log
+
+
+@pytest.mark.user_data(FAILING_USER_DATA_RUNCMD)
+def test_failing_userdata_exit_codes(client: IntegrationInstance):
+    """Test failing in modules representd in exit status"""
+    for mode in ("init", "config", "final"):
+        result = client.execute(f"cloud-init modules --mode {mode}")
+        assert result.failed if mode == "init" else result.ok
+        assert f"'modules:{mode}'" in result.stdout.strip()

tests/integration_tests/modules/test_cli.py Outdated Show resolved Hide resolved
Copy link
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.

Approved for landing once we minimally have class_client and your decision on the failure test.

Co-authored-by: Chad Smith <chad.smith@canonical.com>
@aciba90 aciba90 force-pushed the fix-cmd-modules-init-retval branch from f86c610 to 94fbaa9 Compare March 7, 2024 08:43
@aciba90
Copy link
Contributor Author

aciba90 commented Mar 7, 2024

Approved for landing once we minimally have class_client and your decision on the failure test.

The sole intention of making those test belong to the same class was to use class_client, but I forgot to sed it xD, thanks for catching that up.

I have included your failure test with a small modification: to make cloud-init modules -m init fail, I added a failing bootcmd config stanza, as runcmd does not run as part of init config modules.

Thanks for the review and suggestions!

Copy link
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

@blackboxsw blackboxsw merged commit 621c522 into canonical:main Mar 7, 2024
29 checks passed
@aciba90 aciba90 deleted the fix-cmd-modules-init-retval branch March 11, 2024 11:23
catmsred pushed a commit to catmsred/cloud-init that referenced this pull request Mar 11, 2024
Co-authored-by: Chad Smith <chad.smith@canonical.com>
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.

None yet

2 participants