Skip to content

[aap_containerized] Refactor plugin to use container-helper APIs - #4450

Open
snagoor wants to merge 2 commits into
sosreport:mainfrom
snagoor:refactor/aap-containerized-plugin
Open

[aap_containerized] Refactor plugin to use container-helper APIs#4450
snagoor wants to merge 2 commits into
sosreport:mainfrom
snagoor:refactor/aap-containerized-plugin

Conversation

@snagoor

@snagoor snagoor commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fixes #4449

Summary

Replaces all manual su - <user> -c 'podman ...' shell constructions in the aap_containerized plugin with the typed runtime helpers introduced in feature/container-helpers.

Changes

Container discovery

  • _get_aap_container_names() now delegates to get_containers_by_user(username, get_all=True) instead of shelling out to podman ps --format {{.Names}}.

Log collection

  • New log_lines plugin option (default 1000). add_podman_logs() passes it to add_container_logs(..., runas=username, log_lines=N); --all-logs sets it to None (unbounded) as expected.

Inspect and exec

  • add_podman_inspect() and add_podman_exec() wrap _runtime.inspect_command() and _runtime.exec_command() with runas=username, replacing the manual podman inspect / podman exec -it ... bash -c "..." shell strings.

Runtime info commands

  • setup() calls _runtime.info_command(run_debug=True) and _runtime.list_command(get_all=True) instead of hardcoded podman info --debug / podman ps -a --format json strings.

Username and directory resolution

  • Extracted _get_username(): prefers the plugin option; falls back to scanning ps -eo user,args for a single user running a rootless podman instance.
  • Extracted _get_aap_directory(): prefers the plugin option; falls back to ~<username>/aap with a warning log.

Forbidden paths

  • Added controller/data to cover the entire data directory.
  • Removed individual SECRET_KEY file exclusions (subsumed by the above).

Test plan

  • No username option — auto-detection logs a warning and identifies the correct user from the process list.
  • Explicit -k aap_containerized.username=<user> — inspect, exec, and log output appear under their respective subdirectories (aap_container_logs/, podman_inspect_logs/, podman_cmd_outputs/<container>/).
  • --all-logs — log collection is unbounded (no --tail flag).
  • -k aap_containerized.log_lines=500 — logs are tailed to 500 lines per container.
  • controller/data and eda/etc/*.cert are excluded from the archive.

Assisted-by: Claude Code

Signed-off-by: Nagoor Shaik <nshaik@redhat.com>
@packit-as-a-service

Copy link
Copy Markdown

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

  • sudo dnf install -y 'dnf*-command(copr)'
  • dnf copr enable packit/sosreport-sos-4450
  • And now you can install the packages.

Please note that the RPMs should be used only in a testing environment.

@snagoor

snagoor commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

This PR depends on #4446

Comment thread sos/report/plugins/aap_containerized.py
Comment thread sos/report/plugins/aap_containerized.py
Comment thread sos/report/plugins/aap_containerized.py
Comment thread sos/report/plugins/aap_containerized.py Outdated
Comment thread sos/report/plugins/aap_containerized.py Outdated
@snagoor
snagoor force-pushed the refactor/aap-containerized-plugin branch from f4ccd43 to e204649 Compare August 14, 2026 13:27
return []
return [
con[1]
for con in self.get_containers_by_user(username, get_all=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pylint raises error here due to missing get_containers_by_user method - that is fine, since this PR relies on the other one that adds the method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, once the PR #4446 is merged, I will rebase it so the pylint can run without any errors.

Comment on lines +283 to +284
if not getattr(self, 'aap_directory_name', None):
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we might add an explanation for skipping the obfuscation, but thta is rather a nitpick.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. Added explanation.

Signed-off-by: Nagoor Shaik <nshaik@redhat.com>

Assisted-by: Claude Code
@snagoor
snagoor force-pushed the refactor/aap-containerized-plugin branch from e204649 to 49b841e Compare August 14, 2026 17:59
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.

[aap_containerized] Replace su-wrapped podman shell commands with container-helper APIs

2 participants