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(lxd): avoid failure on multiple calls to --show-log #5811

Merged

Conversation

blackboxsw
Copy link
Collaborator

@blackboxsw blackboxsw commented Oct 10, 2024

Rebase as separate commits

Proposed Commit Message

*  tests(minimal): rsyslog not in minimal images expect warning
    
    Adapt test_no_problems to use verify_clean_boot instead of
    verify_clean_log.
    
    Fix verify_clean_boot to retain platform-specific ignored tracebacks
    before passing it into _verify_clean_boot.

* tests(lxd): avoid failure on multiple calls to --show-log

Cope with repeated calls to lxc console --show-log which exits 1
with the error "no such file or directory" when the log entries
have already been processed and no new log entries exist.

Preserve lxc_log on the client instance and return former log content
on lxd platforms when "no such file or directory" runtime errors are raised

Additional Context

Resolves test_keys_to_console errors seen in https://jenkins.canonical.com/server-team/view/cloud-init/job/cloud-init-integration-noble-lxd_container-minimal/lastSuccessfulBuild/#showFailuresLink

Test Steps

 CLOUD_INIT_OS_IMAGE_TYPE=minimal CLOUD_INIT_PLATFORM=lxd_container CLOUD_INIT_CLOUD_INIT_SOURCE=IN_PLACE CLOUD_INIT_OS_IMAGE_TYPE=minimal CLOUD_INIT_OS_IMAGE=jammy tox -e integration-tests -- --last-failed tests/integration_tests/modules/test_keys_to_console.py

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 force-pushed the SC-1750-lxd-minimal-integration-testing branch from 9524a63 to 31a1c81 Compare October 10, 2024 13:49
@blackboxsw blackboxsw changed the title tests(lxd): avoid failure on multiple calls to --show-logs tests(lxd): avoid failure on multiple calls to --show-log Oct 10, 2024
@blackboxsw blackboxsw force-pushed the SC-1750-lxd-minimal-integration-testing branch 2 times, most recently from ba9b772 to 5fd63d2 Compare October 10, 2024 16:56
…5811)

Cope with repeated calls to lxc console --show-log which exits 1
with the error "no such file or directory" when the log entries
have already been processed and no new log entries exist.

Preserve lxc_log on the client instance and return former log content
on lxd platforms when "no such file or directory" runtime errors are
raised
blackboxsw added a commit to blackboxsw/cloud-init that referenced this pull request Oct 10, 2024
…al#5811)

Adapt test_no_problems to use verify_clean_boot instead of
verify_clean_log.

Fix verify_clean_boot to retain platform-specific ignored tracebacks
before passing it into _verify_clean_boot.
@blackboxsw blackboxsw force-pushed the SC-1750-lxd-minimal-integration-testing branch from 1e909fe to d6c6792 Compare October 10, 2024 17:39
Copy link
Collaborator

@a-dubs a-dubs left a comment

Choose a reason for hiding this comment

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

minimal is OTW 👀

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.

Left questions/comments, but nothing blocking

@@ -168,7 +168,11 @@ def append_or_create_list(
ignore_warnings,
"Unable to disable SSH logins for opc given ssh_redirect_user",
)

# Preserve platform-specific tracebacks expected
Copy link
Member

Choose a reason for hiding this comment

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

So was traceback_texts just not being used before when it was supposed to? If None or a list is passed as ignore_tracebacks, should we just use that use and append to that rather than keeping a separate list here?

Copy link
Collaborator Author

@blackboxsw blackboxsw Oct 11, 2024

Choose a reason for hiding this comment

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

Yes traceback_texts was defined and left unused. If False is provided, we want to report any tracebacks. If None is provided, that's the default and in default case I think we want to ignore expected tracebacks per platform. If a list is passed then we should append to that list instead of keeping the separate list here.

assert f"[INFO]: {boundary_message}" in log
verify_clean_boot(
Copy link
Member

Choose a reason for hiding this comment

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

Nothing that needs to change here, but it does seem a little odd calling verify_clean_boot multiple times in multiple tests in the class given that they're all operating on the same instance and the log won't be any different between tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I had wondered about just consolidating the deprecation test and the no_problems test. I have though consolidated the rsyslog verify_clean_boot call with testy_no_problems case as they are both testing and asserting the same log warning condition based on presence of rsyslogd.

blackboxsw added a commit to blackboxsw/cloud-init that referenced this pull request Oct 11, 2024
…al#5811)

Adapt test_no_problems to use verify_clean_boot instead of
verify_clean_log.

Fix verify_clean_boot to retain platform-specific ignored tracebacks
before passing it into _verify_clean_boot.
@blackboxsw blackboxsw force-pushed the SC-1750-lxd-minimal-integration-testing branch from d6c6792 to fbcfaf2 Compare October 11, 2024 03:52
…al#5811)

Adapt test_no_problems to use verify_clean_boot instead of
verify_clean_log.

Fix verify_clean_boot to retain platform-specific ignored tracebacks
before passing it into _verify_clean_boot.
@blackboxsw blackboxsw force-pushed the SC-1750-lxd-minimal-integration-testing branch from fbcfaf2 to ae328ad Compare October 11, 2024 03: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!

@blackboxsw blackboxsw merged commit 39f2742 into canonical:main Oct 11, 2024
21 checks passed
blackboxsw added a commit that referenced this pull request Oct 11, 2024
Cope with repeated calls to lxc console --show-log which exits 1
with the error "no such file or directory" when the log entries
have already been processed and no new log entries exist.

Preserve lxc_log on the client instance and return former log content
on lxd platforms when "no such file or directory" runtime errors are
raised
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