Should --plugopts overwrite or add plugin options from config file or preset?

[pmoravec]

Having -k/--plugopts specified in an enabled preset and on command line(*), the later fully overwrites whole option. I.e. -k is an override option (for preset+cmdline combination). While it is additive option when you repeatedly apply on command line.

Is that expected behaviour? Or should it be additive for preset+cmdline as well?

Particular example:

# sos report --preset ocp -vvv | grep "effective options now"
[sos.report:setup] effective options now: --container-runtime crio --log-size 100 --no-report --plugopts crio.timeout=600,networking.timeout=600,networking.ethtool_namespaces=False,networking.namespaces=200 --preset ocp --skip-plugins cgroups -vvv

the --plugopts are loaded from the ocp preset. BUT:

# sos report --preset ocp -vvv -k networking.ethtool_namespaces=True | grep "effective options now"
[sos.report:setup] effective options now: --container-runtime crio --log-size 100 --no-report --plugopts networking.ethtool_namespaces=True --preset ocp --skip-plugins cgroups -vvv

OR also:

# sos report --preset ocp -vvv -k process.smaps=on | grep "effective options now"
[sos.report:setup] effective options now: --container-runtime crio --log-size 100 --no-report --plugopts process.smaps=on --preset ocp --skip-plugins cgroups -vvv

does completely throw away --plugopts from the preset (or from config file, as another possible use case).

Is this expected behaviour? Or should a plugin option from cmdline be added (or update just the one option like in --preset ocp -vvv -k networking.ethtool_namespaces=True case)?

My 2c is we should add/update plugopts, not reset it.

[mhradile]

I believe we should ideally make both things available to user for override. I imagine the UI should be working like dnf package manager --enablerepo/--disablerepo options.

Option --eneblerepo adds repos and --disablerepo removes repos from the list. If you want to redefine enabled repos you just specify this sequence order and glob dnf --disablerepo '*' --enablerepo repo1.

So we would replace --plugopts with --set-plugopts, --unset-plugopts making globs work on them.
I believe similar UI could also replace --only-plugins, --skip-plugins, and leave only new --enable-plugins, --disable-plugins with globs working on them.
Same overrides could somehow work in config and preset which might introduce more syntax more on that below.

However I'm not so sure it is worth breaking backwards compatibility causing failures and confusion for users used to override behavior.

If we need this and are unwilling to break backwards compatibility I think the option might be adding more sytax like +/- prefix.

--plugopts 'plugin1.option1=true,pugin1.option2=10' would override everything like now.
--plugopts '+plugin1.option1=true,+pugin1.option2=10' would add to the existing list.
--plugopts '-plugin1.option1=true,-pugin1.option2=10' would remove from the existing list.
--plugopts '+plugin1.option1=true,-pugin1.option2=10' would add and remove from the list accordingly.
--plugopts '+plugin1.option1=true,pugin1.option2=10 no idea what mixed (with not prefixed option) list would do.

Similar thing could be done in config '-*' expression is for ilustrating previous point.

[plugin_options]
-*
+plugin1.option1=true
+pugin1.option2=10

I'm not convinced it is worth doing outside 5.0 or similarly big release that could change the UI.

[TurboTurtle]

We use SosListOption for the extend action within argparse, so I think it shouldn't be too difficult to facilitate overriding specific preset options via the CLI - we're likely inadvertently skipping over this action handling when merging in SosOptions somewhere.

I don't think we need to change the CLI options for plugin options to support this - but if that's a path we want to go down, that's definitely a major version change item and one we'd need to discuss in depth.