cli: Introduce an upgrade command by olix0r · Pull Request #2564 · linkerd/linkerd2

olix0r · 2019-03-26T15:56:00Z

The install command errors when the deploy target contains an existing
Linkerd deployment. The upgrade command is introduced to reinstall or
reconfigure the Linkerd control plane.

Upgrade works as follows:

The controller config is fetched from the Kubernetes API. The Public
API is not used, because we need to be able to reinstall the control
plane when the Public API is not available; and we are not concerned
about RBAC restrictions preventing the installer from reading the
config (as we are for inject).
The install configuration is read, particularly the flags used during
the last install/upgrade. If these flags were not set again during the
upgrade, the previous values are used as if they were passed this time.
The configuration is updated from the combination of these values,
including the install configuration itself.

Note that some flags, including the linkerd-version, are omitted
since they are stored elsewhere in the configurations and don't make
sense to track as overrides..
The issuer secrets are read from the Kubernetes API so that they can
be re-used. There is currently no way to reconfigure issuer
certificates. We will need to create another workflow for
updating these credentials.
The install rendering is invoked with values and config fetched from
the cluster, synthesized with the new configuration.

Fixes #2556

l5d-bot · 2019-03-26T16:11:51Z

Integration test results for 50e2109: success 🎉
Log output: https://gist.github.com/6fcbaef5aad0fbc234f7c9639f89efed

l5d-bot · 2019-03-26T20:30:25Z

Integration test results for e8db3ce: success 🎉
Log output: https://gist.github.com/d25fd58944e95bcc90b80613f7d5017b

cli/cmd/upgrade.go

l5d-bot · 2019-03-26T22:57:54Z

Integration test results for 218d901: fail 😕
Log output: https://gist.github.com/74f87978b3427ce5a7b50c15c63758cd

cli/cmd/upgrade.go

pkg/k8s/labels.go

cli/cmd/upgrade.go

This change moves resource-templating logic into a dedicated template, creates new values types to model kubernetes resource constraints, and changes the `--ha` flag's behavior to create these resource templates instead of hardcoding the resource constraints in the various templates. This is in service of #2564.

l5d-bot · 2019-03-28T07:19:57Z

Integration test results for cf88617: fail 😕
Log output: https://gist.github.com/15b55c74b24c04ae377149e33b035356

l5d-bot · 2019-03-28T16:28:15Z

Integration test results for 12741c8: fail 😕
Log output: https://gist.github.com/d590f8432d7197f467d366ff81a9f8f4

l5d-bot · 2019-03-28T21:49:28Z

Integration test results for 50f27e3: fail 😕
Log output: https://gist.github.com/74332eeff4f232e71b96a79ea9925c77

l5d-bot · 2019-03-28T22:39:45Z

Integration test results for 14b68ed: fail 😕
Log output: https://gist.github.com/5edbe1a126e4b00bd94a668d44447c7d

l5d-bot · 2019-03-29T00:39:02Z

Integration test results for ca41e5d: fail 😕
Log output: https://gist.github.com/d1cec473a1902f06226fe892912dcda7

l5d-bot · 2019-03-29T02:46:40Z

Integration test results for cb1e71e: success 🎉
Log output: https://gist.github.com/2bccd7baa86eaece378b6faae9273bfa

l5d-bot · 2019-03-29T06:17:08Z

Integration test results for 5ef3a0d: fail 😕
Log output: https://gist.github.com/5791346685dabfb79788e26f076ecf4a

The `install` command errors when the deploy target contains an existing Linkerd deployment. The `upgrade` command is introduced to reinstall or reconfigure the Linkerd control plane. Upgrade works as follows: 1. The controller config is fetched from the Kubernetes API. The Public API is not used, because we need to be able to reinstall the control plane when the Public API is not available; and we are not concerned about RBAC restrictions preventing the installer from reading the config (as we are for inject). 2. The install configuration is read, particularly the flags used during the last install/upgrade. If these flags were not set again during the upgrade, the previous values are used as if they were passed this time. The configuration is updated from the combination of these values, including the install configuration itself. Note that some flags, including the linkerd-version, are omitted since they are stored elsewhere in the configurations and don't make sense to track as overrides.. 3. The issuer secrets are read from the Kubernetes API so that they can be re-used. There is currently no way to reconfigure issuer certificates. We will need to create _another_ workflow for updating these credentials. 4. The install rendering is invoked with values and config fetched from the cluster, synthesized with the new configuration.

l5d-bot · 2019-03-29T18:23:50Z

Integration test results for e813001: fail 😕
Log output: https://gist.github.com/25ed6354981cf960c4c618078b87700e

olix0r · 2019-03-29T20:21:50Z

@alpeb it should not be in the issuer flagset, since this field is reconfigurable but the others are not

olix0r · 2019-03-29T20:33:53Z

@alpeb issuerFlagSet is now installOnlyFlagSet and includes ignore-cluster.

grampelberg · 2019-03-29T21:01:29Z

Works for me, I tested:

stable -> this (via install)
this -> this (no options change)
this -> this + options (proxy + resource requests)

I turned --proxy-auto-inject on during the middle of this and can't figure out how to turn it off. Not a blocker, but something to write up an issue and fix later.

olix0r · 2019-03-29T21:02:16Z

@grampelberg can you use upgrade --proxy-auto-inject=false ?

l5d-bot · 2019-03-29T21:17:56Z

Integration test results for 4095c7d: success 🎉
Log output: https://gist.github.com/4b6ddac7d3dfa62d2aa9c8eeab1a42f0

grampelberg · 2019-03-29T21:28:19Z

That removes it from the output, I've not tried your --prune suggestion yet.

siggy

Overall looks (and works!) well. Had a few comments around structure, docs, and tests. 👍 🚢

siggy · 2019-03-29T20:43:23Z

cli/cmd/upgrade.go

@@ -0,0 +1,192 @@
+package cmd


consider adding some tests for this file. we'll probably want integration tests as well, but that could be done in a subsequent PR.

in the interest of unblocking the release, i'm going to do this in a followup

pkg/k8s/labels.go

cli/cmd/install.go

cli/cmd/upgrade.go

l5d-bot · 2019-03-29T23:05:50Z

Integration test results for e23d85c: success 🎉
Log output: https://gist.github.com/c7181636e646ade24715efc5f77bd2f3

alpeb · 2019-03-30T21:49:00Z

That removes it from the output, I've not tried your --prune suggestion yet.

So the suggestion was
linkerd upgrade --proxy-auto-inject=false | k apply -f - -l linkerd.io/control-plane-component --prune

We confirmed this works and removes the linkerd-proxy-injector deployment, but the mutating webhook config remains.

Apparently this doesn't affect the creation of new pods. But to be able to clean it up we need to add the linkerd.io/control-plane-component (tracked in #2602) and have that config be included in the install yaml (#2176)

alpeb

Looking good to me, modulo @siggy suggestions 👍

ihcsim

LGTM. Just two comments.

Maybe in a future PR, we can consider if using sub-struct to organize the *Values and *Options will make the validateAndBuild() and validate() easier to follow (or not).

cli/cmd/upgrade.go

ihcsim · 2019-04-01T04:16:43Z

cli/cmd/install.go

-// If we cannot determine whether the configuration exists, an error is returned.
-func linkerdConfigAlreadyExistsInCluster() (bool, error) {
-	api, err := k8s.NewAPI(kubeconfigPath, kubeContext)
+func exitIfClusterExists() {


Other than the error handling, this looks very similar to the newK8s() and fetchConfigs() in upgrade.go. If we can remove the dependency on upgradeOption(), can we re-use them here?

func exitIfClusterExists() { - kubeConfig, err := k8s.GetConfig(kubeconfigPath, kubeContext) + k, err := newK8s() if err != nil { fmt.Fprintln(os.Stderr, "Unable to build a Kubernetes client to check for configuration. If this expected, use the --ignore-cluster flag.") fmt.Fprintf(os.Stderr, "Error: %s\n", err) os.Exit(1) } - k, err := kubernetes.NewForConfig(kubeConfig) + _, err = fetchConfigs(k) if err != nil { - fmt.Fprintln(os.Stderr, "Unable to build a Kubernetes client to check for configuration. If this expected, use the --ignore-cluster flag.") - fmt.Fprintf(os.Stderr, "Error: %s\n", err) - os.Exit(1) - } - - c := k.CoreV1().ConfigMaps(controlPlaneNamespace) - if _, err = c.Get(k8s.ConfigConfigMapName, metav1.GetOptions{}); err != nil { if kerrors.IsNotFound(err) { return }

siggy

one nit then 🚢

siggy · 2019-04-01T19:57:43Z

cli/cmd/install.go


 	// Some flags are not available during upgrade, etc.
-	cmd.PersistentFlags().AddFlagSet(options.installOnlyFlagSet(pflag.ExitOnError))
+	cmd.PersistentFlags().AddFlagSet(options.flagSet(pflag.ExitOnError))


i think this is redundant with line 204: flags := options.flagSet(pflag.ExitOnError) ?

l5d-bot · 2019-04-01T20:02:38Z

Integration test results for 7068101: success 🎉
Log output: https://gist.github.com/e7025d4f186fbfda4aa0e3f49027da7f

l5d-bot · 2019-04-01T20:17:33Z

Integration test results for eead6d9: success 🎉
Log output: https://gist.github.com/ae7420a4fa92bfc3a395447a7275ddd7

olix0r · 2019-04-01T20:27:02Z

CI passes but the issue isn't getting notified https://travis-ci.org/linkerd/linkerd2/jobs/514357190

l5d-bot · 2019-04-01T20:27:08Z

Integration test results for 374562e: success 🎉
Log output: https://gist.github.com/106a023318627ce0ea94c4f22beea5ef

The `install` command errors when the deploy target contains an existing Linkerd deployment. The `upgrade` command is introduced to reinstall or reconfigure the Linkerd control plane. Upgrade works as follows: 1. The controller config is fetched from the Kubernetes API. The Public API is not used, because we need to be able to reinstall the control plane when the Public API is not available; and we are not concerned about RBAC restrictions preventing the installer from reading the config (as we are for inject). 2. The install configuration is read, particularly the flags used during the last install/upgrade. If these flags were not set again during the upgrade, the previous values are used as if they were passed this time. The configuration is updated from the combination of these values, including the install configuration itself. Note that some flags, including the linkerd-version, are omitted since they are stored elsewhere in the configurations and don't make sense to track as overrides.. 3. The issuer secrets are read from the Kubernetes API so that they can be re-used. There is currently no way to reconfigure issuer certificates. We will need to create _another_ workflow for updating these credentials. 4. The install rendering is invoked with values and config fetched from the cluster, synthesized with the new configuration. Signed-off-by: [email protected] <[email protected]>

olix0r self-assigned this Mar 26, 2019

olix0r requested review from alpeb and ihcsim March 26, 2019 15:56

alpeb reviewed Mar 26, 2019

View reviewed changes

cli/cmd/upgrade.go Outdated Show resolved Hide resolved

olix0r force-pushed the ver/upgrade branch from e8db3ce to 218d901 Compare March 26, 2019 22:51

siggy added area/cli area/install labels Mar 26, 2019

siggy reviewed Mar 27, 2019

View reviewed changes

olix0r mentioned this pull request Mar 27, 2019

install: Replace EnableHA with resource values #2572

Merged

alpeb mentioned this pull request Mar 27, 2019

config: Store install parameters with global config #2577

Merged

olix0r force-pushed the ver/upgrade branch from 218d901 to cf88617 Compare March 28, 2019 07:12

olix0r force-pushed the ver/upgrade branch from 5ef3a0d to c4383da Compare March 29, 2019 17:47

olix0r marked this pull request as ready for review March 29, 2019 17:52

olix0r changed the title ~~WIP: cli: Introduce an upgrade command~~ cli: Introduce an upgrade command Mar 29, 2019

olix0r added 2 commits March 29, 2019 17:53

const up issuer file names

292f1da

fixup doc

e813001

alpeb mentioned this pull request Mar 29, 2019

cli: add an Upgrade command #2556

Closed

Only expose flags when they can be used

f394fae

remove stupid printlns

4095c7d

siggy approved these changes Mar 29, 2019

View reviewed changes

fixup kube client constructor

e23d85c

alpeb approved these changes Mar 30, 2019

View reviewed changes

ihcsim approved these changes Apr 1, 2019

View reviewed changes

olix0r added 2 commits April 1, 2019 19:21

fixup per review

85447ac

flagSet reorg

7068101

siggy approved these changes Apr 1, 2019

View reviewed changes

clarify recordableFlagSet

eead6d9

bump ci

374562e

olix0r merged commit d74ca1b into master Apr 1, 2019

olix0r deleted the ver/upgrade branch April 1, 2019 20:27

admc mentioned this pull request Apr 2, 2019

Linkerd 2.3 release test plan #2459

Closed

25 tasks

siggy mentioned this pull request Apr 4, 2019

upgrade command unit tests #2637

Closed

klingerf mentioned this pull request Apr 4, 2019

Add Upgrade command #2601

Closed

Conversation

olix0r commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

l5d-bot commented Mar 26, 2019

Uh oh!

l5d-bot commented Mar 26, 2019

Uh oh!

Uh oh!

l5d-bot commented Mar 26, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

l5d-bot commented Mar 28, 2019

Uh oh!

l5d-bot commented Mar 28, 2019

Uh oh!

l5d-bot commented Mar 28, 2019

Uh oh!

l5d-bot commented Mar 28, 2019

Uh oh!

l5d-bot commented Mar 29, 2019

Uh oh!

l5d-bot commented Mar 29, 2019

Uh oh!

l5d-bot commented Mar 29, 2019

Uh oh!

l5d-bot commented Mar 29, 2019

Uh oh!

olix0r commented Mar 29, 2019

Uh oh!

olix0r commented Mar 29, 2019

Uh oh!

grampelberg commented Mar 29, 2019

Uh oh!

olix0r commented Mar 29, 2019

Uh oh!

l5d-bot commented Mar 29, 2019

Uh oh!

grampelberg commented Mar 29, 2019

Uh oh!

siggy left a comment

Choose a reason for hiding this comment

Uh oh!

siggy Mar 29, 2019

Choose a reason for hiding this comment

Uh oh!

olix0r Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

l5d-bot commented Mar 29, 2019

Uh oh!

alpeb commented Mar 30, 2019

Uh oh!

alpeb left a comment

Choose a reason for hiding this comment

Uh oh!

ihcsim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ihcsim Apr 1, 2019

Choose a reason for hiding this comment

Uh oh!

siggy left a comment

Choose a reason for hiding this comment

Uh oh!

siggy Apr 1, 2019

Choose a reason for hiding this comment

olix0r commented Mar 26, 2019 •

edited

Loading

olix0r Apr 1, 2019 •

edited

Loading