refactor: Split CRD & Control Plane upgrade logic (#8423)

This change follows on 4f3c374, which split the install logic for CRDs
and the core control plane, by splitting the upgrade logic for the CRDs
and the core control plane.

Signed-off-by: Oliver Gould <ver@buoyant.io>

cli/cmd/install.go

@@ -97,11 +97,11 @@ func newCmdInstall() *cobra.Command {
 
 This command provides all Kubernetes configs necessary to install the Linkerd
 control plane.`,
-		Example: `  # Default install.
+		Example: `  # Install CRDs first.
-  linkerd install | kubectl apply -f -
+  linkerd install --crds | kubectl apply -f -
 
-  # Install Linkerd into a non-default namespace.
+  # Install the core control plane.
-  linkerd install -L linkerdtest | kubectl apply -f -
+  linkerd install | kubectl apply -f -
 
 The installation can be configured by using the --set, --values, --set-string and --set-file flags.
 A full list of configurable values can be found at https://www.github.com/linkerd/linkerd2/tree/main/charts/linkerd2/README.md`,
@@ -130,7 +130,9 @@ A full list of configurable values can be found at https://www.github.com/linker
 			}
 
 			if crds {
-				if err = installCRDs(cmd.Context(), k8sAPI, os.Stdout, values, options); err != nil {
+				// The CRD chart is not configurable.
+				// TODO(ver): Error if values have been configured?
+				if err = installCRDs(cmd.Context(), k8sAPI, os.Stdout); err != nil {
 					return err
 				}
 
@@ -175,18 +177,12 @@ func checkNoConfig(ctx context.Context, k8sAPI *k8s.KubernetesAPI) error {
 	return nil
 }
 
-func installCRDs(ctx context.Context, k8sAPI *k8s.KubernetesAPI, w io.Writer, values *l5dcharts.Values, options valuespkg.Options) error {
+func installCRDs(ctx context.Context, k8sAPI *k8s.KubernetesAPI, w io.Writer) error {
 	if err := checkNoConfig(ctx, k8sAPI); err != nil {
 		return err
 	}
 
-	// Create values override
+	return renderCRDs(w)
-	valuesOverrides, err := options.MergeValues(nil)
-	if err != nil {
-		return err
-	}
-
-	return renderCRDs(w, values, valuesOverrides)
 }
 
 func installControlPlane(ctx context.Context, k8sAPI *k8s.KubernetesAPI, w io.Writer, values *l5dcharts.Values, flags []flag.Flag, options valuespkg.Options) error {
@@ -303,7 +299,7 @@ func renderChartToBuffer(files []*loader.BufferedFile, values *l5dcharts.Values,
 	return &buf, vals, nil
 }
 
-func renderCRDs(w io.Writer, values *l5dcharts.Values, valuesOverrides map[string]interface{}) error {
+func renderCRDs(w io.Writer) error {
 	files := []*loader.BufferedFile{
 		{Name: chartutil.ChartfileName},
 	}
@@ -314,7 +310,12 @@ func renderCRDs(w io.Writer, values *l5dcharts.Values, valuesOverrides map[strin
 		return err
 	}
 
-	buf, _, err := renderChartToBuffer(files, values, valuesOverrides)
+	// The CRD chart does not take any value configuration.
+	values, err := l5dcharts.NewValues()
+	if err != nil {
+		return err
+	}
+	buf, _, err := renderChartToBuffer(files, values, map[string]interface{}{})
 	if err != nil {
 		return err
 	}

cli/cmd/install_test.go

@@ -265,7 +265,7 @@ func TestRenderCRDs(t *testing.T) {
 	addFakeTLSSecrets(defaultValues)
 
 	var buf bytes.Buffer
-	if err := renderCRDs(&buf, defaultValues, map[string]interface{}{}); err != nil {
+	if err := renderCRDs(&buf); err != nil {
 		t.Fatalf("Failed to render templates: %v", err)
 	}
 	testDataDiffer.DiffTestdata(t, "install_crds.golden", buf.String())

cli/cmd/upgrade.go

@@ -91,12 +91,21 @@ A full list of configurable values can be found at https://www.github.com/linker
   --prune-whitelist=rbac.authorization.k8s.io/v1/clusterrolebinding \
   --prune-whitelist=apiregistration.k8s.io/v1/apiservice -f -`,
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if crds {
+				// The CRD chart is not configurable.
+				// TODO(ver): Error if values have been configured?
+				if _, err := upgradeCRDs().WriteTo(os.Stdout); err != nil {
+					fmt.Fprintln(os.Stderr, err.Error())
+					os.Exit(1)
+				}
+				return nil
+			}
+
 			k, err := k8sClient(manifests)
 			if err != nil {
 				return err
 			}
-			err = upgradeRunE(cmd.Context(), k, flags, crds, options)
+			if err = upgradeControlPlaneRunE(cmd.Context(), k, flags, options); err != nil {
-			if err != nil {
 				fmt.Fprintln(os.Stderr, err.Error())
 				os.Exit(1)
 			}
@@ -154,9 +163,8 @@ func makeUpgradeFlags() *pflag.FlagSet {
 	return upgradeFlags
 }
 
-func upgradeRunE(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, crds bool, options valuespkg.Options) error {
+func upgradeControlPlaneRunE(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, options valuespkg.Options) error {
-
+	buf, err := upgradeControlPlane(ctx, k, flags, options)
-	buf, err := upgrade(ctx, k, flags, crds, options)
 	if err != nil {
 		return err
 	}
@@ -167,22 +175,29 @@ func upgradeRunE(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, c
 		}
 	}
 
-	buf.WriteTo(os.Stdout)
+	_, err = buf.WriteTo(os.Stdout)
-
+	return err
-	return nil
 }
 
-func upgrade(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, crds bool, options valuespkg.Options) (bytes.Buffer, error) {
+func upgradeCRDs() *bytes.Buffer {
+	var buf bytes.Buffer
+	if err := renderCRDs(&buf); err != nil {
+		upgradeErrorf("Could not render upgrade configuration: %s", err)
+	}
+	return &buf
+}
+
+func upgradeControlPlane(ctx context.Context, k *k8s.KubernetesAPI, flags []flag.Flag, options valuespkg.Options) (*bytes.Buffer, error) {
 	values, err := loadStoredValues(ctx, k)
 	if err != nil {
-		return bytes.Buffer{}, fmt.Errorf("failed to load stored values: %w", err)
+		return nil, fmt.Errorf("failed to load stored values: %w", err)
 	}
 
 	// If values is still nil, then the linkerd-config-overrides secret was not found.
 	// This means either means that Linkerd was installed with Helm or that the installation
 	// needs to be repaired.
 	if values == nil {
-		return bytes.Buffer{}, errors.New(
+		return nil, errors.New(
 			`Could not find the Linkerd config. If Linkerd was installed with Helm, please
 use Helm to perform upgrades. If Linkerd was not installed with Helm, please use
 the 'linkerd repair' command to repair the Linkerd config`)
@@ -190,54 +205,45 @@ the 'linkerd repair' command to repair the Linkerd config`)
 
 	err = flag.ApplySetFlags(values, flags)
 	if err != nil {
-		return bytes.Buffer{}, err
+		return nil, err
 	}
 
 	if values.Identity.Issuer.Scheme == string(corev1.SecretTypeTLS) {
 		for _, flag := range flags {
 			if (flag.Name() == "identity-issuer-certificate-file" || flag.Name() == "identity-issuer-key-file") && flag.IsSet() {
-				return bytes.Buffer{}, errors.New("cannot update issuer certificates if you are using external cert management solution")
+				return nil, errors.New("cannot update issuer certificates if you are using external cert management solution")
 			}
 		}
 	}
 
 	err = validateValues(ctx, k, values)
 	if err != nil {
-		return bytes.Buffer{}, err
+		return nil, err
 	}
 	if !force && values.Identity.Issuer.Scheme == k8s.IdentityIssuerSchemeLinkerd {
 		err = ensureIssuerCertWorksWithAllProxies(ctx, k, values)
 		if err != nil {
-			return bytes.Buffer{}, err
+			return nil, err
 		}
 	}
 
 	// Create values override
 	valuesOverrides, err := options.MergeValues(nil)
 	if err != nil {
-		return bytes.Buffer{}, err
+		return nil, err
 	}
 	if !isRunAsRoot(valuesOverrides) {
 		err = healthcheck.CheckNodesHaveNonDockerRuntime(ctx, k)
 		if err != nil {
-			return bytes.Buffer{}, err
+			return nil, err
 		}
 	}
 
-	// rendering to a buffer and printing full contents of buffer after
-	// render is complete, to ensure that okStatus prints separately
 	var buf bytes.Buffer
-	if crds {
-		if err = renderCRDs(&buf, values, valuesOverrides); err != nil {
-			upgradeErrorf("Could not render upgrade configuration: %s", err)
-		}
-		return buf, nil
-	}
-
 	if err = renderControlPlane(&buf, values, valuesOverrides); err != nil {
 		upgradeErrorf("Could not render upgrade configuration: %s", err)
 	}
-	return buf, nil
+	return &buf, nil
 }
 
 func loadStoredValues(ctx context.Context, k *k8s.KubernetesAPI) (*charts.Values, error) {

cli/cmd/upgrade_test.go

@@ -564,27 +564,37 @@ func pathMatch(path []string, template []string) bool {
 	return true
 }
 
-func renderInstall(t *testing.T, values *linkerd2.Values) bytes.Buffer {
+func renderInstall(t *testing.T, values *linkerd2.Values) *bytes.Buffer {
-	var installBuf bytes.Buffer
+	var buf bytes.Buffer
-	if err := renderControlPlane(&installBuf, values, nil); err != nil {
+	if err := renderCRDs(&buf); err != nil {
 		t.Fatalf("could not render install manifests: %s", err)
 	}
-	return installBuf
+	buf.WriteString("---\n")
+	if err := renderControlPlane(&buf, values, nil); err != nil {
+		t.Fatalf("could not render install manifests: %s", err)
+	}
+	return &buf
 }
 
-func renderUpgrade(installManifest string, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (bytes.Buffer, error) {
+func renderUpgrade(installManifest string, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (*bytes.Buffer, error) {
 	k, err := k8s.NewFakeAPIFromManifests([]io.Reader{strings.NewReader(installManifest)})
 	if err != nil {
-		return bytes.Buffer{}, fmt.Errorf("failed to initialize fake API: %w\n\n%s", err, installManifest)
+		return nil, fmt.Errorf("failed to initialize fake API: %w\n\n%s", err, installManifest)
 	}
-	return upgrade(context.Background(), k, upgradeOpts, false, templateOpts)
+	buf := upgradeCRDs()
+	buf.WriteString("---\n")
+	cpbuf, err := upgradeControlPlane(context.Background(), k, upgradeOpts, templateOpts)
+	if err != nil {
+		return nil, err
+	}
+	buf.Write(cpbuf.Bytes())
+	return buf, nil
 }
 
-func renderInstallAndUpgrade(t *testing.T, installOpts *charts.Values, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (bytes.Buffer, bytes.Buffer, error) {
+func renderInstallAndUpgrade(t *testing.T, installOpts *charts.Values, upgradeOpts []flag.Flag, templateOpts valuespkg.Options) (*bytes.Buffer, *bytes.Buffer, error) {
-
 	err := validateValues(context.Background(), nil, installOpts)
 	if err != nil {
-		return bytes.Buffer{}, bytes.Buffer{}, fmt.Errorf("failed to validate values: %w", err)
+		return nil, nil, fmt.Errorf("failed to validate values: %w", err)
 	}
 	installBuf := renderInstall(t, installOpts)
 	upgradeBuf, err := renderUpgrade(installBuf.String(), upgradeOpts, templateOpts)