Linkerd2: Disable custom cluster domain changes during control plane upgrade

Created on 20 Sep 2019  路  4Comments  路  Source: linkerd/linkerd2

PR #3360 added a new option to the linkerd install command, to support custom cluster domain. The upgrade command inherits this option, because it shares the same set of flags with install. Since upgrading the trust domain isn't supported, we should fail the upgrade process if the --cluster-domain option is used during upgrade, to avoid breaking mTLS.

areinstall help wanted

Most helpful comment

I believe the right approach is indeed to move --cluster-domain into the installOnlyFlagSet, which will remove it from the linkerd upgrade -h list and will have the CLI rightfully err when the user attempts to use the flag during upgrade. The flag won't get included into the install section of the linkerd-config ConfigMap, but it is still included into the global section, which is where it's extracted from elsewhere, so we'd be fine :+1:

All 4 comments

@ihcsim I'd be happy to take on this one, but I am certainly missing something: why, then, isn't --cluster-domain part of installOnlyFlagSet, so that it is present only during install?

@bmcstdio IIRC, installOnlyFlagSet doesn't get persisted in the linkerd-config config map. If you run kubectl -n linkerd describe cm linkerd-config, you will see there is an Install JSON blob in the config map. (It's empty by default.) That captures all the flags and their values we want to persist during linkerd upgrade. It will be worth confirming if installOnlyFlagSet does get persisted.

@ihcsim I understand. I've opened #3496 as an attempt at implementing this.

I believe the right approach is indeed to move --cluster-domain into the installOnlyFlagSet, which will remove it from the linkerd upgrade -h list and will have the CLI rightfully err when the user attempts to use the flag during upgrade. The flag won't get included into the install section of the linkerd-config ConfigMap, but it is still included into the global section, which is where it's extracted from elsewhere, so we'd be fine :+1:

Was this page helpful?
0 / 5 - 0 ratings