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

tests: ec2 check status exit code instead of cloud-init init --local #5504

Merged

Conversation

blackboxsw
Copy link
Collaborator

@blackboxsw blackboxsw commented Jul 10, 2024

Proposed Commit Message

    tests: revert expectation of exit 2 from cloud-init init --local
    
    Commit 604d80eb introduced assertions expecting exit 2 from the
    CLI when calling cloud-init init --local. Revert this test assertion
    as only cloud-init status command exits (2) on deprecations/warnings.
    
    Invoking cloud-init's boot stages on the commmand line will only exit
    1 if critical errors are encountered to avoid degrading overall
    systemd health as seen from cloud-init systemd units. When cloud-init
    boot stages encounter recoverable_errors of any type, there is no
    need to exit non-zero as those deprecation logs are not-critical to
    the health of the system as a whole.

Additional Context

Expected integration test failure on ec2 with daily PPA tomorrow because cloud-init init --local (or any direct call to cloud-init stage doesn't actually exit non-zero because we didn't want cloud-init systemd units/services to degrade overall systemd health based on recoverable_errors.

Test Steps

# 1. build the deb locally from tip of main (or wait for daily PPA build publishing in +2 hours)

# test where DEPRECATION_INFO_BOUNDARY is 22.1 <= "24.3" defined for deprecation/info level message about invoking cloud-init from the CLI
CLOUD_INIT_OS_IMAGE=jammy CLOUD_INIT_PLATFORM=ec2 CLOUD_INIT_CLOUD_INIT_SOURCE=cloud-init_24.2~6gee44a4e2-0ubuntu1~22.04.1_all.deb tox -e integration-tests -- tests/integration_tests/datasources/test_ec2_ipv6.py  --pdb

# test where DEPRECATION_INFO_BOUNDARY=devel (oracular)
CLOUD_INIT_OS_IMAGE=oracular CLOUD_INIT_PLATFORM=ec2 CLOUD_INIT_CLOUD_INIT_SOURCE=cloud-init_24.2~6gee44a4e2-0ubuntu1_all.deb tox -e integration-tests -- tests/integration_tests/datasources/test_ec2_ipv6.py  --pdb

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>)

@blackboxsw blackboxsw requested a review from TheRealFalcon July 10, 2024 21:13
2
if should_log_deprecation(
"24.3", get_feature_flag_value(client, "DEPRECATION_INFO_BOUNDARY")
)
else 0
)
assert client.execute("cloud-init clean --logs")
assert return_code == client.execute("cloud-init init --local").return_code
assert status_code == client.execute("cloud-init status").return_code
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there's any reason to be checking cloud-init status in this test. That was never the point of this test.

I think we should just revert this file to its state prior to https://github.com/canonical/cloud-init/pull/5209/files#diff-53a08506f1820f460078130200aee26e99c5a8f151d99367acfef2d7db5020beR12

@blackboxsw blackboxsw force-pushed the tests-ec2-status-for-deprecation branch 2 times, most recently from f35ef37 to 1b9bb92 Compare July 10, 2024 21:55
Commit 604d80e introduced assertions expecting exit 2 from the
CLI when calling cloud-init init --local. Revert this test assertion
as only cloud-init status command exits (2) on deprecations/warnings.

Invoking cloud-init's boot stages on the commmand line will only exit
1 if critical errors are encountered to avoid degrading overall
systemd health as seen from cloud-init systemd units. When cloud-init
boot stages encounter recoverable_errors of any type, there is no
need to exit non-zero as those deprecation logs are not-critical to
the health of the system as a whole.
@blackboxsw blackboxsw force-pushed the tests-ec2-status-for-deprecation branch from 1b9bb92 to a6b4206 Compare July 10, 2024 21:57
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@TheRealFalcon TheRealFalcon merged commit 18d76ac into canonical:main Jul 10, 2024
23 checks passed
holmanb pushed a commit to holmanb/cloud-init that referenced this pull request Aug 2, 2024
…onical#5504)

Commit 604d80e introduced assertions expecting exit 2 from the
CLI when calling cloud-init init --local. Revert this test assertion
as only cloud-init status command exits (2) on deprecations/warnings.

Invoking cloud-init's boot stages on the commmand line will only exit
1 if critical errors are encountered to avoid degrading overall
systemd health as seen from cloud-init systemd units. When cloud-init
boot stages encounter recoverable_errors of any type, there is no
need to exit non-zero as those deprecation logs are not-critical to
the health of the system as a whole.
holmanb pushed a commit that referenced this pull request Aug 6, 2024
Commit 604d80e introduced assertions expecting exit 2 from the
CLI when calling cloud-init init --local. Revert this test assertion
as only cloud-init status command exits (2) on deprecations/warnings.

Invoking cloud-init's boot stages on the commmand line will only exit
1 if critical errors are encountered to avoid degrading overall
systemd health as seen from cloud-init systemd units. When cloud-init
boot stages encounter recoverable_errors of any type, there is no
need to exit non-zero as those deprecation logs are not-critical to
the health of the system as a whole.
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.

2 participants