[component] Apply plugopts from cmdline gently

[pmoravec]

When plugopts are set both in a loaded preset and in cmdline, we should merge the options instead of blindly using cmdline ones.

This means preset plugopt options not present in cmdline are added, while preset plugopt options also present in cmdline are ignored.

Closes: #3855

Relevant: RHEL-67097

---

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

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

[pmoravec]

Reproducer:

sos report -k qpid.port=5672 --preset satellite -o qpid,apache -vvv
..
[sos.report:setup] using 'satellite' preset defaults (--log-size 100 --plugopts apache.log=on)
[sos.report:setup] effective options now: --log-size 100 --only-plugins qpid,apache --plugopts qpid.port=5672 --preset satellite -vvv

(see the apache.log=on option is ignored)

A simple solution "let merge the option lists" has a counterexample:

sos report -k qpid.port=5672,apache.log=off --preset satellite -o qpid,apache -vvv
..
[sos.report:setup] using 'satellite' preset defaults (--log-size 100 --plugopts apache.log=on)
[sos.report:setup] effective options now: --log-size 100 --only-plugins qpid,podman --plugopts qpid.port=5672,apache.log=off,apache.log=on --preset satellite -vvv

where we cant guarantee what value of apache.log will be really used. Hence the "separate value names" dance.

[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-3855
• And now you can install the packages.

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

[TurboTurtle]

Early comment to say, let's definitely get a test case written for this.

[pmoravec]

Early comment to say, let's definitely get a test case written for this.

I realized we have no test for presets. So I will add some generic plus some covering this PR.

Before I dig into it - can I extend the current https://github.com/sosreport/sos/blob/main/tests/report_tests/options_tests/options_tests.py#L13 ? By changing it like:

    files = [('options_tests_sos.conf', '/etc/sos/sos.conf')]   # sos.conf newly has plugin-timeout = 150 and cmd-timeout = 20
    sos_cmd = '--preset minimal --journal-size 20 --cmd-timeout 60'

    ..

    def test_effective_options_logged_correctly(self):
        for option in [
            "--case-id 8675309",
            "--only-plugins host,kernel",
            "--plugopts kernel.with-timer=on,kernel.trace=yes",
            "--preset minimal",
            "--cmd-timeout 60",   # cmdline beats config and preset
            "--journal-size 20",     # cmdline beats preset
            "--log-size 10",            # preset setting is honored
            "--plugin-timeout 30", # preset beats config file - this is current behaviour, is it correct..?
        ]:
            self.assertSosLogContains(f"effective options now: .* {option}")

This test will ensure the right priority of options among preset, config file and cmdline - see comments.

Can I "squeeze" the preset tests into this (renamed) class?

Additionally, I will have a class (or two) for testing the plugopts - that will work on Red Hat distro only (as plugopts are set in RH presets only and I dont want to make testing over-complex by creating a preset first and then testing it).

Does it sound a good plan?

[arif-ali]

I do plan to add presets from Canonical at some point, so if we can generalise it, that would help when I get round to it. Or, otherwise I can extend the work here once I get to it

[pmoravec]

I do plan to add presets from Canonical at some point, so if we can generalise it, that would help when I get round to it. Or, otherwise I can extend the work here once I get to it

Sounds good. So I will test the plugopts stuff via Red Hat preset for now.

Extending the current OptionsFromConfigTest to test also presets sounds reasonable? I am ok with it, to have all/most of the options priority testing within one class and one sos report execution. But if you would like to have more granular classes (more readable, but slower tests execution), let me know.