[aap_containerized] Fixes #3892 - added new AAP Containerized plugin

[snagoor]

---

Please place an 'X' inside each '[]' to confirm you adhere to our Contributor Guidelines

• [x ] Is the commit message split over multiple lines and hard-wrapped at 72 characters?
• [ x] Is the subject and message clear and concise?
• [x ] Does the subject start with [plugin_name] if submitting a plugin patch or a [section_name] if part of the core sosreport code?
• [x ] Does the commit contain a Signed-off-by: First Lastname [email protected]?
• [x ] Are any related Issues or existing PRs properly referenced via a Closes (Issue) or Resolved (PR) line?
• [ x] Are all passwords or private data gathered by this PR obfuscated?

[packit-as-a-service]

Congratulations! One of the builds has completed. 🍾

You can install the built RPMs by following these steps:

• sudo yum install -y dnf-plugins-core on RHEL 8
• sudo dnf install -y dnf-plugins-core on Fedora
• dnf copr enable packit/sosreport-sos-3893
• And now you can install the packages.

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

[TurboTurtle]

Before I go through with a full review, why does this need to be a separate plugin? We typically support containerized and non-containerized within the same plugin for the same component(s), and just dynamically provide a container to run command or file collections in when needed via the container parameter that is part of both add_copy_spec() and add_cmd_output().

We do this so that we don't duplicate efforts, when the only difference between collections is if they need to be executed on the host or in a container. If the actual functioning of a component/whatever actually changes when inside a container, then that makes sense for a separate plugin.

We have several different AAP plugins today - so is there a reason we can't extend those to look for their relevant containers? It would seem awkward to me at first glance if we were to either 1) double up on the number of plugins just to account for containers or 2) had multiple plugins for host-based installs and a single "unified" plugin for containerized installs which this seems like it is trying to be.

[TurboTurtle]

Code LGTM aside from the one note around subprocess.

Still have the outstanding question of this single plugin vs. integrating containerized bits in the existing ones, but if we feel that a new single plugin makes the most sense then I'm fine with it.

[snagoor]

Code LGTM aside from the one note around subprocess.

Still have the outstanding question of this single plugin vs. integrating containerized bits in the existing ones, but if we feel that a new single plugin makes the most sense then I'm fine with it.

@TurboTurtle thank you for the review.

There is a significant difference in terms of AAP containerized and RPM based architecture. That is why we have 2 different installers for the same. IMO this can't be clubbed into the existing plugin and increase its complexity in maintaining it in future, because the storage, logs and configs are totally different.

Also, this Containerized setup is going to be future. So, having this plugin by itself is what I vouch for.

I have now updated the changes as request, please have a look when you can.

[TurboTurtle]

LGTM, just the linting alert from flake8 on line 50.

Thanks for the details, makes sense to me to split this out going forward.

[pmoravec]

Can't this be simple:

       return "awx-manage" in ps["output"] and "aap-gateway" in ps["output"]

?

Can't this provide false positive results when there would be a running process like, say, "grep awx-manage some.log"? Isn't running ps --noheaders axco command command and matching whole line a better check here, or is that already over-complicated..?

But I am OK with the current code.

[snagoor]

@pmoravec Thank you for the suggestion. Changes amended as requested.

[pmoravec]

Thanks!