Skip to content

testing: Fix console_log tests (SC-992)#1437

Merged
holmanb merged 3 commits into
canonical:mainfrom
TheRealFalcon:fix-console-log
May 12, 2022
Merged

testing: Fix console_log tests (SC-992)#1437
holmanb merged 3 commits into
canonical:mainfrom
TheRealFalcon:fix-console-log

Conversation

@TheRealFalcon
Copy link
Copy Markdown
Contributor

Proposed Commit Message

testing: Fix console_log tests

Bring in the pycloudlib change effecting the console_log on ec2.
Additionally, provide a helper function for retrieving the console log
to reduce duplication.

Moved the retry decorator into a new file to avoid cyclic dependency
and had to update call sites accordingly.

Additional Context

https://jenkins.canonical.com/server-team/view/cloud-init/job/cloud-init-integration-bionic-ec2/15/testReport/tests.integration_tests.modules.test_keys_to_console/TestKeysToConsoleEnabled/test_duplicate_messaging_console_log/

@TheRealFalcon TheRealFalcon changed the title testing: Fix console_log tests testing: Fix console_log tests (SC-992) May 6, 2022
Bring in the pycloudlib change effecting the console_log on ec2.
Additionally, provide a helper function for retrieving the console log
to reduce duplication.

Moved the retry decorator into a new file to avoid cyclic dependency
and had to update call sites accordingly.
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.

+1 good consolidation of utility into a separate better named module. I think this is probably good as-is.

Not for this PR, I wonder we can introspect the underlying feature of the IntegrationInstance with a mark like pytest.mark.cloud_has_feature("console_log") where the feature lookup could be performed on the class definition and provide an earlier skip for a given feature if it's not implemented on the class. The reason I ask is because our test runner wastes time setting up test images and launching instances which ultimately result in a pytest.skip at runtime. The test case I'm talking about is below:

When testing this on Azure (which doesn't implement instance.console_log, this detection and pytest.skip comes way after the the base image is created, this results in a ~5 min cost locally when trying to build, launch and validate a test that ultimately will be skipped:

 CLOUD_INIT_CLOUD_INIT_SOURCE=ppa:cloud-init-dev/daily CLOUD_INIT_PLATFORM=azure CLOUD_INIT_OS_IMAGE=Canonical:0001-com-ubuntu-server-impish-daily:21_10-daily::ubuntu::impish tox -e integration-tests tests/integration_tests/modules/test_keys_to_console.py::test_duplicate_messaging_console_log

Comment thread tests/integration_tests/modules/test_keys_to_console.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.

Question answered :)

@TheRealFalcon
Copy link
Copy Markdown
Contributor Author

TheRealFalcon commented May 12, 2022

When testing this on Azure (which doesn't implement instance.console_log, this detection and pytest.skip comes way after the the base image is created, this results in a ~5 min cost locally when trying to build, launch and validate a test that ultimately will be skipped:

Oh...I can actually bring back the marks I removed. I didn't realize we didn't have that on Azure, and that was probably the point of specifying them that way.

@holmanb holmanb merged commit 4c35af7 into canonical:main May 12, 2022
aciba90 pushed a commit to aciba90/cloud-init that referenced this pull request May 13, 2022
Bring in the pycloudlib change effecting the console_log on ec2.
Additionally, provide a helper function for retrieving the console log
to reduce duplication.

Move the retry decorator into a new file to avoid cyclic dependency
and update call sites accordingly.
@TheRealFalcon TheRealFalcon deleted the fix-console-log branch March 21, 2025 18:48
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