ci: tox-lsr 3.17.1 - previous update broke container tests, this fixes them [citest_skip]#265
Merged
ci: tox-lsr 3.17.1 - previous update broke container tests, this fixes them [citest_skip]#265
Conversation
…s them [citest_skip] tox-lsr 3.17.1 has a fix for the broken container tests There was one shell function for both setting up the callback plugins and the connection plugin. When this function was skipped, the ANSIBLE_CONNECTION_PLUGINS environment variable was not set, so all subsequent tests failed. The connection plugin must be present and the env. var. must be set in order to run any container tests. The code was fixed to ensure that there is always a connection plugin installed in the correct location and that ANSIBLE_CONNECTION_PLUGINS is always set and contains this path. Also, setting up the callback plugins and the connection plugin is already idempotent, so no reason to skip them. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates GitHub Actions workflows to use tox-lsr 3.17.1 so container tests get a properly installed connection plugin and ANSIBLE_CONNECTION_PLUGINS set, and stops skipping callback/connection plugin setup between test runs in qemu-kvm integration tests. Sequence diagram for CI container tests with tox_lsr 3_17_1 connection plugin setupsequenceDiagram
actor Developer
participant GitHub_Actions as GitHub_Actions_Workflow
participant tox_lsr as tox_lsr_3_17_1
participant Ansible as Ansible_Test_Harness
participant Container as Container_Tests
Developer->>GitHub_Actions: push_changes
GitHub_Actions->>GitHub_Actions: pip_install_tox_lsr(version_3_17_1)
GitHub_Actions->>tox_lsr: invoke_test_environment_setup
tox_lsr->>tox_lsr: install_connection_plugin_if_missing
tox_lsr->>tox_lsr: ensure_callback_plugins_installed
tox_lsr->>tox_lsr: set_env_ANSIBLE_CONNECTION_PLUGINS
tox_lsr-->>GitHub_Actions: environment_ready
GitHub_Actions->>Ansible: run_ansible_tests
Ansible->>Container: run_container_based_tests
Container-->>Ansible: test_results
Ansible-->>GitHub_Actions: summarize_results
GitHub_Actions-->>Developer: CI_status_report
Flow diagram for idempotent plugin setup in qemu_kvm integration testsflowchart TD
A[start_qemu_kvm_integration_workflow]
B[install_tox_lsr_3_17_1_via_pip]
C[run_plugin_setup_step]
D[connection_plugin_present?]
E[install_connection_plugin]
F[callback_plugins_present?]
G[install_callback_plugins]
H[set_ANSIBLE_CONNECTION_PLUGINS_env_var]
I[run_qemu_kvm_container_tests]
J[collect_and_report_results]
A --> B --> C --> D
D -->|yes| F
D -->|no| E --> F
F -->|yes| H
F -->|no| G --> H
H --> I --> J --> A
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The tox-lsr version string is duplicated across multiple workflows; consider centralizing it via a reusable workflow, composite action, or a single workflow env variable so future upgrades only require changing it in one place.
- Since SKIP_CALLBACK_PLUGINS is no longer used in the workflow, double-check that there are no remaining references to this variable or its behavior in any scripts or tooling invoked by these jobs to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tox-lsr version string is duplicated across multiple workflows; consider centralizing it via a reusable workflow, composite action, or a single workflow env variable so future upgrades only require changing it in one place.
- Since SKIP_CALLBACK_PLUGINS is no longer used in the workflow, double-check that there are no remaining references to this variable or its behavior in any scripts or tooling invoked by these jobs to avoid confusion.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tox-lsr 3.17.1 has a fix for the broken container tests
There was one shell function for both setting up the callback plugins and the connection plugin.
When this function was skipped, the ANSIBLE_CONNECTION_PLUGINS environment variable was not
set, so all subsequent tests failed. The connection plugin must be present and the env. var.
must be set in order to run any container tests. The code was fixed to ensure that there is
always a connection plugin installed in the correct location and that ANSIBLE_CONNECTION_PLUGINS
is always set and contains this path.
Also, setting up the callback plugins and the connection plugin is already idempotent, so no
reason to skip them.
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by Sourcery
Update CI workflows to use tox-lsr 3.17.1 and ensure container test plugins are always configured.
CI: