Support specifiying different image and additional args to fluxctl install

[hiddeco]

As discussed in #2552, this adds support to fluxctl install for configuring a different image using --flux-image and for supplying extra arguments using --extra-flux-arguments.

Both flags have been implemented in the e2e tests to get rid of the sed replace logic.

This PR has potential of superseding #2530.

[squaremo]

One reason I'm against having a flag for supplying anything you like is that it makes validation difficult. So for example, we insist on people providing a git user (at least an email), because it's something we don't want to leave to default; in #2544 I have to go to some lengths to verify that a value has been supplied as an argument or in the supplied config file. Adding arbitrary arguments would make that more complicated still. (But probably still possible?)

Another reason is that it enables a poor user experience. If you can direct people to just supply extra arguments, you have an excuse for not thinking about your UI. A bit like introducing a "misc" category tends to destroy any filing scheme.

[hiddeco]

I have to go to some lengths to verify that a value has been supplied as an argument or in the supplied config file. Adding arbitrary arguments would make that more complicated still.

None of the additional arguments are mandatory, validation would thus only go as far as 'do not supply things twice'. I would even argue that joining the arrays while preferring the values from the command line if duplicates exist may be better, as this is a common practice for functionalities like this to provide customizations on top of a predefined config (from file).

Another reason is that it enables a poor user experience. If you can direct people to just supply extra arguments, you have an excuse for not thinking about your UI.

The experience as is can also be considered as 'poor UX', as any configuration that uses a combination of flag-enabled features can not be setup using this utility, and the alternative offered isn't scriptable in a reliable way.

I think as long as the goal of the install command is to provide dedicated knobs for mandatory setup options, and everything else can be provided using --extra-flux-arguments, the command has a very clear and lean goal / UX, while still giving enough tools to power users to work with.

[squaremo]

I have to go to some lengths to verify that a value has been supplied as an argument or in the supplied config file. Adding arbitrary arguments would make that more complicated still.

None of the additional arguments are mandatory, validation would thus only go as far as 'do not supply things twice'.

Not quite -- if the mandatory value is missing in the named args and missing in the config file, it would have to check that it had not been passed in the extra args.

But the point is really more general than that: it's that once you build in an escape mechanism, you lose some ability to interpret what's being attempted by the user, and that means you are less effective at guiding them.

The experience as is can also be considered as 'poor UX', as any configuration that uses a combination of flag-enabled features can not be setup using this utility, and the alternative offered isn't scriptable in a reliable way.

It depends what you consider to be the purpose of fluxctl install. I personally don't consider it to be a general-purpose flux configuration generator -- it should do enough to produce an output like the files in deploy/, with the things that must be specialised to a particular deployment e.g., the git repo, and some of the more common (and fiddly to set up, requiring mounts etc.) options. Power users have an abundance of other tools available to do whatever else they want.

[2opremio]

I understand @squaremo 's concerns, but I still find the extra parameters useful (and in fact AFAICR, the helm chart also supports extra arguments).

An alternative (also solving @squaremo 's concerns, I think) could be exporting all the flags from fluxd through a symbol/function, and reusing them in fluxctl install? We could even reuse part of the validation.

[stefanprodan]

+1 for exporting all fluxd flags in fluxctl install

[squaremo]

+1 for exporting all fluxd flags in fluxctl install

Some flags require other modifications to the manifest, which cannot be effected with fluxctl install. Are you going to look for those flags and do the modifications? (Probably not: in general, they would need special treatment, to e.g., supply a secret or whatever to mount). Or will you exclude them? Or give people with the ability to supply the flag, but not generate a usable output?

[2opremio]

@squaremo would you be more comfortable if we add flags on demand (as people request them or we need them internally)?

I. am thinking that may be a non-ending task but can't think about anything else right now

[squaremo]

would you be more comfortable if we add flags on demand (as people request them or we need them internally)?

I would be comfortable if we considered each such request and made additions only when

• it's an argument that users must supply (e.g,. --git-url)
• it's a flag many users will want to consider using (e.g., --git-path)
• it's a flag some users will want to consider, and it needs careful treatment (e.g., --git-gpg-key-import, which probably needs to be formulated in terms of which keys to export from a local keyring and form into a secret, or something along those lines)

I think it's a never-ending task to make fluxctl install fully support all flags and other optionalia. Half-supporting all flags (by letting you supply them, but not always producing usable output when you do) would be worse than sticking to a small subset, as we have now.

[sebikul]

I think a good compromise would be to add the most important flags that Flux supports, and document that for more versatile or advanced installs, the Helm chart or a custom deployment would be advisable.

I found myself implementing a flag we need in our deployment in #2530 and did not find it too complicated. I didn't want to pollute the args that the install command accepts implementing the rest, but it is true that validation and understanding what the user is trying to achieve will help catching invalid settings and giving a poor UX in the end.

[2opremio]

@hiddeco Should we close this?

[hiddeco]

Yep, I think I found the appropriate workaround by using helm template.