upgrade: unit tests

This change introduces some unit tests on individual methods in the upgrade code path, along with some minor cleanup. Part of #2637 Signed-off-by: Andrew Seigner <[email protected]>
linkerd · Apr 6, 2019 · e3c5396 · e3c5396
1 parent 65a228f
commit e3c5396
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 32 deletions.
diff --git a/cli/cmd/install.go b/cli/cmd/install.go
@@ -207,7 +207,7 @@ func newCmdInstall() *cobra.Command {
 
 	// The base flags are recorded separately s that they can be serialized into
 	// the configuration in validateAndBuild.
-	flags := options.recordableFlagSet(pflag.ExitOnError)
+	flags := options.recordableFlagSet()
 
 	cmd := &cobra.Command{
 		Use:   "install [flags]",
@@ -230,7 +230,7 @@ func newCmdInstall() *cobra.Command {
 	cmd.PersistentFlags().AddFlagSet(flags)
 
 	// Some flags are not available during upgrade, etc.
-	cmd.PersistentFlags().AddFlagSet(options.installOnlyFlagSet(pflag.ExitOnError))
+	cmd.PersistentFlags().AddFlagSet(options.installOnlyFlagSet())
 
 	return cmd
 }
@@ -258,7 +258,9 @@ func (options *installOptions) validateAndBuild(flags *pflag.FlagSet) (*installV
 }
 
 // recordableFlagSet returns flags usable during install or upgrade.
-func (options *installOptions) recordableFlagSet(e pflag.ErrorHandling) *pflag.FlagSet {
+func (options *installOptions) recordableFlagSet() *pflag.FlagSet {
+	e := pflag.ExitOnError
+
 	flags := pflag.NewFlagSet("install", e)
 
 	flags.AddFlagSet(options.proxyConfigOptions.flagSet(e))
@@ -306,8 +308,8 @@ func (options *installOptions) recordableFlagSet(e pflag.ErrorHandling) *pflag.F
 
 // installOnlyFlagSet includes flags that are only accessible at install-time
 // and not at upgrade-time.
-func (options *installOptions) installOnlyFlagSet(e pflag.ErrorHandling) *pflag.FlagSet {
-	flags := pflag.NewFlagSet("install-only", e)
+func (options *installOptions) installOnlyFlagSet() *pflag.FlagSet {
+	flags := pflag.NewFlagSet("install-only", pflag.ExitOnError)
 
 	flags.StringVar(
 		&options.identityOptions.trustDomain, "identity-trust-domain", options.identityOptions.trustDomain,

diff --git a/cli/cmd/upgrade.go b/cli/cmd/upgrade.go
@@ -32,7 +32,7 @@ func newUpgradeOptionsWithDefaults() *upgradeOptions {
 
 func newCmdUpgrade() *cobra.Command {
 	options := newUpgradeOptionsWithDefaults()
-	flags := options.recordableFlagSet(pflag.ExitOnError)
+	flags := options.recordableFlagSet()
 
 	cmd := &cobra.Command{
 		Use:   "upgrade [flags]",
@@ -43,8 +43,17 @@ Note that the default flag values for this command come from the Linkerd control
 plane. The default values displayed in the Flags section below only apply to the
 install command.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if options.ignoreCluster {
+				panic("ignore cluster must be unset") // Programmer error.
+			}
+
 			// We need a Kubernetes client to fetch configs and issuer secrets.
-			k, err := options.newK8s()
+			c, err := k8s.GetConfig(kubeconfigPath, kubeContext)
+			if err != nil {
+				upgradeErrorf("Failed to get kubernetes config: %s", err)
+			}
+
+			k, err := kubernetes.NewForConfig(c)
 			if err != nil {
 				upgradeErrorf("Failed to create a kubernetes client: %s", err)
 			}
@@ -74,6 +83,10 @@ install command.`,
 }
 
 func (options *upgradeOptions) validateAndBuild(k kubernetes.Interface, flags *pflag.FlagSet) (*installValues, *pb.All, error) {
+	if err := options.validate(); err != nil {
+		return nil, nil, err
+	}
+
 	// We fetch the configs directly from kubernetes because we need to be able
 	// to upgrade/reinstall the control plane when the API is not available; and
 	// this also serves as a passive check that we have privileges to access this
@@ -85,18 +98,14 @@ func (options *upgradeOptions) validateAndBuild(k kubernetes.Interface, flags *p
 
 	// If the install config needs to be repaired--either because it did not
 	// exist or because it is missing expected fields, repair it.
-	options.repairInstall(configs.Install)
+	repairInstall(options.generateUUID, configs.Install)
 
 	// We recorded flags during a prior install. If we haven't overridden the
 	// flag on this upgrade, reset that prior value as if it were specified now.
 	//
 	// This implies that the default flag values for the upgrade command come
 	// from the control-plane, and not from the defaults specified in the FlagSet.
-	setOptionsFromInstall(flags, configs.GetInstall())
-
-	if err = options.validate(); err != nil {
-		return nil, nil, err
-	}
+	setFlagsFromInstall(flags, configs.GetInstall().GetFlags())
 
 	// Save off the updated set of flags into the installOptions so it gets
 	// persisted with the upgraded config.
@@ -120,35 +129,22 @@ func (options *upgradeOptions) validateAndBuild(k kubernetes.Interface, flags *p
 	return values, configs, nil
 }
 
-func setOptionsFromInstall(flags *pflag.FlagSet, install *pb.Install) {
-	for _, i := range install.GetFlags() {
+func setFlagsFromInstall(flags *pflag.FlagSet, installFlags []*pb.Install_Flag) {
+	for _, i := range installFlags {
 		if f := flags.Lookup(i.GetName()); f != nil && !f.Changed {
 			f.Value.Set(i.GetValue())
 			f.Changed = true
 		}
 	}
 }
 
-func (options *upgradeOptions) newK8s() (kubernetes.Interface, error) {
-	if options.ignoreCluster {
-		panic("ignore cluster must be unset") // Programmer error.
-	}
-
-	c, err := k8s.GetConfig(kubeconfigPath, kubeContext)
-	if err != nil {
-		return nil, err
-	}
-
-	return kubernetes.NewForConfig(c)
-}
-
-func (options *upgradeOptions) repairInstall(install *pb.Install) {
+func repairInstall(generateUUID func() string, install *pb.Install) {
 	if install == nil {
 		install = &pb.Install{}
 	}
 
 	if install.GetUuid() == "" {
-		install.Uuid = options.generateUUID()
+		install.Uuid = generateUUID()
 	}
 
 	// ALWAYS update the CLI version to the most recent.

diff --git a/cli/cmd/upgrade_test.go b/cli/cmd/upgrade_test.go
@@ -2,10 +2,13 @@ package cmd
 
 import (
 	"bytes"
+	"errors"
+	"fmt"
 	"testing"
 
+	"github.com/golang/protobuf/proto"
+	pb "github.com/linkerd/linkerd2/controller/gen/config"
 	"github.com/linkerd/linkerd2/pkg/k8s"
-	"github.com/spf13/pflag"
 )
 
 func TestRenderUpgrade(t *testing.T) {
@@ -43,7 +46,7 @@ data:
 	}
 
 	options := newUpgradeOptionsWithDefaults()
-	flags := options.recordableFlagSet(pflag.ExitOnError)
+	flags := options.recordableFlagSet()
 
 	clientset, _, err := k8s.NewFakeClientSets(k8sConfigs...)
 	if err != nil {
@@ -61,3 +64,78 @@ data:
 	}
 	diffTestdata(t, "upgrade_default.golden", buf.String())
 }
+
+func TestFetchConfigs(t *testing.T) {
+	options := testInstallOptions()
+	values, _, err := options.validateAndBuild(nil)
+	if err != nil {
+		t.Fatalf("Unexpected error validating options: %v", err)
+	}
+	exp := options.configs(values.Identity.toIdentityContext())
+
+	testCases := []struct {
+		k8sConfigs []string
+		expected   *pb.All
+		err        error
+	}{
+		{
+			[]string{`
+kind: ConfigMap
+apiVersion: v1
+metadata:
+  name: linkerd-config
+  namespace: linkerd
+data:
+  global: |
+    {"linkerdNamespace":"linkerd","cniEnabled":false,"version":"dev-undefined","identityContext":{"trustDomain":"cluster.local","trustAnchorsPem":"-----BEGIN CERTIFICATE-----\nMIIBYDCCAQegAwIBAgIBATAKBggqhkjOPQQDAjAYMRYwFAYDVQQDEw1jbHVzdGVy\nLmxvY2FsMB4XDTE5MDMwMzAxNTk1MloXDTI5MDIyODAyMDM1MlowGDEWMBQGA1UE\nAxMNY2x1c3Rlci5sb2NhbDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABAChpAt0\nxtgO9qbVtEtDK80N6iCL2Htyf2kIv2m5QkJ1y0TFQi5hTVe3wtspJ8YpZF0pl364\n6TiYeXB8tOOhIACjQjBAMA4GA1UdDwEB/wQEAwIBBjAdBgNVHSUEFjAUBggrBgEF\nBQcDAQYIKwYBBQUHAwIwDwYDVR0TAQH/BAUwAwEB/zAKBggqhkjOPQQDAgNHADBE\nAiBQ/AAwF8kG8VOmRSUTPakSSa/N4mqK2HsZuhQXCmiZHwIgZEzI5DCkpU7w3SIv\nOLO4Zsk1XrGZHGsmyiEyvYF9lpY=\n-----END CERTIFICATE-----\n","issuanceLifetime":"86400s","clockSkewAllowance":"20s"},"autoInjectContext":null}
+  proxy: |
+    {"proxyImage":{"imageName":"gcr.io/linkerd-io/proxy","pullPolicy":"IfNotPresent"},"proxyInitImage":{"imageName":"gcr.io/linkerd-io/proxy-init","pullPolicy":"IfNotPresent"},"controlPort":{"port":4190},"ignoreInboundPorts":[],"ignoreOutboundPorts":[],"inboundPort":{"port":4143},"adminPort":{"port":4191},"outboundPort":{"port":4140},"resource":{"requestCpu":"","requestMemory":"","limitCpu":"","limitMemory":""},"proxyUid":"2102","logLevel":{"level":"warn,linkerd2_proxy=info"},"disableExternalProfiles":true}
+  install: |
+    {"uuid":"deaab91a-f4ab-448a-b7d1-c832a2fa0a60","cliVersion":"dev-undefined","flags":[]}`,
+			},
+			exp,
+			nil,
+		},
+		{
+			[]string{`
+kind: ConfigMap
+apiVersion: v1
+metadata:
+  name: linkerd-config
+  namespace: linkerd
+data:
+  global: "{}"
+  proxy: "{}"
+  install: "{}"`,
+			},
+			&pb.All{Global: &pb.Global{}, Proxy: &pb.Proxy{}, Install: &pb.Install{}},
+			nil,
+		},
+		{
+			nil,
+			nil,
+			errors.New("configmaps \"linkerd-config\" not found"),
+		},
+	}
+
+	for i, tc := range testCases {
+		tc := tc // pin
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			clientset, _, err := k8s.NewFakeClientSets(tc.k8sConfigs...)
+			if err != nil {
+				t.Fatalf("Unexpected error: %s", err)
+			}
+
+			configs, err := fetchConfigs(clientset)
+			if (err == nil && tc.err != nil) ||
+				(err != nil && tc.err == nil) ||
+				((err != nil && tc.err != nil) && (err.Error() != tc.err.Error())) {
+				t.Fatalf("Expected \"%s\", got \"%s\"", tc.err, err)
+			}
+
+			if !proto.Equal(configs, tc.expected) {
+				t.Fatalf("Unexpected config:\nExpected:\n%+v\nGot:\n%+v", tc.expected, configs)
+			}
+		})
+	}
+}