-
Notifications
You must be signed in to change notification settings - Fork 880
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,7 +126,8 @@ def append_or_create_list( | |
maybe_list = [value] | ||
return maybe_list | ||
|
||
traceback_texts = [] | ||
if ignore_tracebacks is None: | ||
ignore_tracebacks = [] | ||
# Define exceptions by matrix of platform and Ubuntu release | ||
if "azure" == PLATFORM: | ||
# Consistently on all Azure launches: | ||
|
@@ -147,17 +148,19 @@ def append_or_create_list( | |
ignore_errors = append_or_create_list( | ||
ignore_warnings, "Stderr: RTNETLINK answers: File exists" | ||
) | ||
traceback_texts.append("Stderr: RTNETLINK answers: File exists") | ||
if isinstance(ignore_tracebacks, list): | ||
ignore_tracebacks.append("Stderr: RTNETLINK answers: File exists") | ||
# LP: #1833446 | ||
ignore_warnings = append_or_create_list( | ||
ignore_warnings, | ||
"UrlError: 404 Client Error: Not Found for url: " | ||
"http://169.254.169.254/latest/meta-data/", | ||
) | ||
traceback_texts.append( | ||
"UrlError: 404 Client Error: Not Found for url: " | ||
"http://169.254.169.254/latest/meta-data/" | ||
) | ||
if isinstance(ignore_tracebacks, list): | ||
ignore_tracebacks.append( | ||
"UrlError: 404 Client Error: Not Found for url: " | ||
"http://169.254.169.254/latest/meta-data/" | ||
) | ||
# Oracle has a file in /etc/cloud/cloud.cfg.d that contains | ||
# users: | ||
# - default | ||
|
@@ -168,7 +171,7 @@ def append_or_create_list( | |
ignore_warnings, | ||
"Unable to disable SSH logins for opc given ssh_redirect_user", | ||
) | ||
|
||
# Preserve platform-specific tracebacks expected | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
_verify_clean_boot( | ||
instance, | ||
ignore_deprecations=ignore_deprecations, | ||
|
@@ -538,10 +541,20 @@ def get_console_log(client: "IntegrationInstance"): | |
console_log = client.instance.console_log() | ||
except NotImplementedError: | ||
pytest.skip("NotImplementedError when requesting console log") | ||
except RuntimeError as e: | ||
if "open : no such file or directory" in str(e): | ||
if hasattr(client, "lxc_log"): | ||
return client.lxc_log | ||
raise e | ||
if console_log is None: | ||
pytest.skip("Console log has not been setup") | ||
if console_log.lower().startswith("no console output"): | ||
pytest.fail("no console output") | ||
if PLATFORM in ("lxd_vm", "lxd_container"): | ||
# Preserve non empty console log on lxc platforms because | ||
# lxc console --show-log can be called once and the log is flushed. | ||
# Multiple calls to --show-log error on "no such file or directory". | ||
client.lxc_log = console_log # type: ignore[attr-defined] | ||
return console_log | ||
|
||
|
||
|
There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.