Skip to content

fix: ensure connection plugin installed and env. var. set correctly#233

Merged
richm merged 1 commit intolinux-system-roles:mainfrom
richm:fix-container-tests
Mar 13, 2026
Merged

fix: ensure connection plugin installed and env. var. set correctly#233
richm merged 1 commit intolinux-system-roles:mainfrom
richm:fix-container-tests

Conversation

@richm
Copy link
Copy Markdown
Contributor

@richm richm commented Mar 13, 2026

The previous commit broke the logic for setting up connection plugin.

  • The connection plugin should always be installed - there is no reason to skip
  • The setup_callback_plugins is already "skipable" and it is idempotent, so no reason to have SKIP_CALLBACK_PLUGINS
  • Ensure the connection plugin is installed in a path in ANSIBLE_CONNECTION_PLUGINS
  • Ensure ANSIBLE_CONNECTION_PLUGINS is set correctly always
  • Ensure that the connection plugin is installed in the local .tox/connection_plugins if not
    already in ANSIBLE_CONNECTION_PLUGINS
  • Ensure that the local .tox/connection_plugins is in ANSIBLE_CONNECTION_PLUGINS if not
    already
  • Refactor the code by creating a separate function for setup_connection_plugin

And clean up some other debugging code

Signed-off-by: Rich Megginson rmeggins@redhat.com

Summary by Sourcery

Ensure Ansible connection plugins are always installed and configured correctly in the test container script.

Bug Fixes:

  • Always install the appropriate podman or buildah Ansible connection plugin and ensure it is available on the connection plugin path.
  • Correctly set and preserve the ANSIBLE_CONNECTION_PLUGINS environment variable so the connection plugin directory is consistently included.

Enhancements:

  • Split callback and connection plugin setup into separate functions for clearer responsibilities in the test container script.
  • Add optional debug logging controlled by LSR_DEBUG and reduce unconditional directory listings to run only in debug mode.

The previous commit broke the logic for setting up connection plugin.

* The connection plugin should always be installed - there is no reason to skip
* Ensure the connection plugin is installed in a path in ANSIBLE_CONNECTION_PLUGINS
* Ensure ANSIBLE_CONNECTION_PLUGINS is set correctly always
* Ensure that the connection_plugins is installed in the local .tox/connection_plugins if not
  already in ANSIBLE_CONNECTION_PLUGINS
* Ensure that the local .tox/connection_plugins is in ANSIBLE_CONNECTION_PLUGINS if not
  already
* Refactor the code by creating a separate function for setup_connection_plugin

And clean up some other debugging code

Signed-off-by: Rich Megginson <rmeggins@redhat.com>
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Mar 13, 2026

Reviewer's Guide

Refactors the container test script to always install and correctly wire up Ansible callback and connection plugins, ensuring ANSIBLE_CONNECTION_PLUGINS is consistently set and that a local .tox/connection_plugins directory is used as needed, while tightening debug logging behavior and removing the ability to skip plugin setup.

File-Level Changes

Change Details Files
Split plugin setup into dedicated callback and connection setup functions and always invoke both.
  • Rename the generic setup_plugins helper to setup_callback_plugins, limiting it to callback plugin configuration and related environment variables.
  • Introduce setup_connection_plugin to encapsulate all logic for selecting the appropriate connection plugin (podman.py/buildah.py) and configuring ANSIBLE_CONNECTION_PLUGINS.
  • Change the main execution flow to unconditionally call setup_callback_plugins and setup_connection_plugin after requirements installation.
src/tox_lsr/test_scripts/runcontainer.sh
Make connection plugin installation robust and ensure ANSIBLE_CONNECTION_PLUGINS is correctly constructed and exported.
  • Replace the previous conditional that sometimes skipped installing the connection plugin with logic that always ensures the plugin is available either in ANSIBLE_CONNECTION_PLUGINS or in a local connection_plugins directory under the tox work dir.
  • Parse ANSIBLE_CONNECTION_PLUGINS as a colon-separated list, track whether the desired plugin is already present, and prepend the local connection_plugins path when needed.
  • Ensure the local connection_plugins directory exists, then either install the containers.podman collection and move the plugin there or copy it from an already-installed collection under the tox work dir.
  • Rebuild ANSIBLE_CONNECTION_PLUGINS from the updated path list and export it so subsequent Ansible invocations see the correct plugin search path.
src/tox_lsr/test_scripts/runcontainer.sh
Adjust debug and CLI behavior related to plugin setup.
  • Introduce a debug helper that logs only when LSR_DEBUG is true and use it for optional directory listings, instead of always running ls.
  • Remove the --skip-callback-plugins CLI flag and the SKIP_CALLBACK_PLUGINS variable so plugin setup is no longer skippable.
  • Drop noisy always-on env/ls logging around Ansible env vars in favor of the new debug mechanism.
src/tox_lsr/test_scripts/runcontainer.sh

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In setup_connection_plugin, when ANSIBLE_CONNECTION_PLUGINS is empty or contains leading/trailing : separators, read -ra will produce empty path elements that are then preserved in new_con_plugin_paths; consider explicitly filtering out empty con_plugin_path entries to avoid unintentionally adding the current directory or empty segments to the search path.
  • The use of local -a and array operations in setup_connection_plugin assumes a Bash-compatible shell; if this script may be run under a more limited /bin/sh, it would be safer to either enforce a Bash shebang or avoid Bash-specific array syntax.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `setup_connection_plugin`, when `ANSIBLE_CONNECTION_PLUGINS` is empty or contains leading/trailing `:` separators, `read -ra` will produce empty path elements that are then preserved in `new_con_plugin_paths`; consider explicitly filtering out empty `con_plugin_path` entries to avoid unintentionally adding the current directory or empty segments to the search path.
- The use of `local -a` and array operations in `setup_connection_plugin` assumes a Bash-compatible shell; if this script may be run under a more limited `/bin/sh`, it would be safer to either enforce a Bash shebang or avoid Bash-specific array syntax.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@richm richm merged commit 8a948c8 into linux-system-roles:main Mar 13, 2026
3 of 8 checks passed
@richm richm deleted the fix-container-tests branch March 13, 2026 15:04
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.

1 participant