config: Store install parameters with global config #2577
Conversation
When installing Linkerd, a user may override default settings, or may explicitly configure defaults. Consider install options like `--ha --controller-replicas=4` -- the `--ha` flag sets a new default value for the controller-replicas, and then we override it. When we later upgrade this cluster, how can we know how to configure the cluster? We could store EnableHA and ControllerReplicas configurations in the config, but what if, in a later upgrade, the default value changes? How can we know whether the user specified an override or just used the default? To solve this, we add an `InstallContext` message onto the `Global` config. This message includes (at least) the CLI flags used to invoke install. upgrade does not specify defaults for install/proxy-options fields and, instead, uses the persisted install flags to populate default values, before applying overrides from the upgrade invocation. This change breaks the protobuf compability by altering the `installation_uuid` field introduced in 9c442f6. Because this change was not yet released (even in an edge release), we feel that it is safe to break. Fixes #2574
|
Integration test results for 7c22b24: fail 😕 |
|
Integration test results for c7f1d6c: fail 😕 |
|
TIOLI: How do you feel about black-listing some of the Global flags from Also we still gotta figure the issue you brought in #2564: how does one disable |
This changes install and inject to produce FlagSets containing each of their flags. These flags are then used to record the install flags without extraneous flags such as --kubeconfig.
|
Integration test results for 73b9647: fail 😕 |
|
I got concerned about serializing the CLI flags themselves this morning. It'll make it difficult for a helm chart to create these settings. Thinking about it some more, helm would never call out to the control plane to read those settings, so it should be okay ... with the caveat that once you use a helm chart you can never to back (or we change the serialization strategy in the future). So, tldr, I can't come up with a major reason not to do it and a whole bunch to do it =) |
|
Integration test results for 0f21537: fail 😕 |
cli/cmd/install.go
Outdated
| if f.Changed { | ||
| switch f.Name { | ||
| case "ignore-cluster", "linkerd-version": | ||
| // Thse flags don't make sense to record. |
There was a problem hiding this comment.
typo: These
Also how about including here as well api-addr, context, kubeconfig and verbose? Most likely these are particular to some user, so shouldn't be stored in the config shared by all.
There was a problem hiding this comment.
We only have to handle flags set by options.flagSet now! This was the main rational in building FlagSet instead of appending to PersistenFlags.
I've confirmed this with manual testing.
There was a problem hiding this comment.
And what do you think about identity-issuer-certificate-file, identity-issuer-key-file and identity-trust-anchors-file? I believe linkerd upgrade will ignore them but they don't belong in the shared config IMO.
There was a problem hiding this comment.
Good call. These should be omitted as well.
There was a problem hiding this comment.
PTAL. I moved these issuer flags onto a separate flagset so they are just never considered to be recorded (and also means we won't expose them from upgrade).
|
Integration test results for 2219d8f: success 🎉 |
|
Integration test results for e22c84c: success 🎉 |
|
Integration test results for 0e2fdb5: success 🎉 |
|
Integration test results for ad1c6fc: success 🎉 |
When installing Linkerd, a user may override default settings, or may
explicitly configure defaults. Consider install options like
--ha --controller-replicas=4-- the--haflag sets a new default value forthe controller-replicas, and then we override it.
When we later upgrade this cluster, how can we know how to configure the
cluster?
We could store EnableHA and ControllerReplicas configurations in the
config, but what if, in a later upgrade, the default value changes? How
can we know whether the user specified an override or just used the
default?
To solve this, we add an
InstallContextmessage onto theGlobalconfig. This message includes (at least) the CLI flags used to invoke
install.
upgrade does not specify defaults for install/proxy-options fields and,
instead, uses the persisted install flags to populate default values,
before applying overrides from the upgrade invocation.
This change breaks the protobuf compability by altering the
installation_uuidfield introduced in 9c442f6.Because this change was not yet released (even in an edge release), we
feel that it is safe to break.
Fixes #2574