cli: new flag to skip phases of upgrade (#2310)

Co-authored-by: Daniel Weiße <66256922+daniel-weisse@users.noreply.github.com> Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com>
2024-12-25 15:39:37 -05:00 · 2023-09-08 14:55:07 +02:00 · 2023-09-08 14:55:07 +02:00 · 5960025da7
commit 5960025da7
parent 7376c6a998
6 changed files with 207 additions and 75 deletions
--- a/cli/internal/cmd/BUILD.bazel
+++ b/cli/internal/cmd/BUILD.bazel
@ -164,6 +164,7 @@ go_test(
        "@com_github_spf13_afero//:afero",
        "@com_github_spf13_cobra//:cobra",
        "@com_github_stretchr_testify//assert",
+        "@com_github_stretchr_testify//mock",
        "@com_github_stretchr_testify//require",
        "@io_k8s_api//core/v1:core",
        "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1:apiextensions",
--- a/cli/internal/cmd/upgradeapply.go
+++ b/cli/internal/cmd/upgradeapply.go
@ -12,6 +12,7 @@ import (
 	"fmt"
 	"io"
 	"path/filepath"
+	"strings"
 	"time"

 	"github.com/edgelesssys/constellation/v2/cli/internal/cloudcmd"
@ -37,6 +38,20 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 )

+const (
+	// skipInfrastructurePhase skips the terraform apply of the upgrade process.
+	skipInfrastructurePhase skipPhase = "infrastructure"
+	// skipHelmPhase skips the helm upgrade of the upgrade process.
+	skipHelmPhase skipPhase = "helm"
+	// skipImagePhase skips the image upgrade of the upgrade process.
+	skipImagePhase skipPhase = "image"
+	// skipK8sPhase skips the k8s upgrade of the upgrade process.
+	skipK8sPhase skipPhase = "k8s"
+)
+
+// skipPhase is a phase of the upgrade process that can be skipped.
+type skipPhase string
+
 func newUpgradeApplyCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "apply",
@ -53,6 +68,8 @@ func newUpgradeApplyCmd() *cobra.Command {
 		"Might be useful for slow connections or big clusters.")
 	cmd.Flags().Bool("conformance", false, "enable conformance mode")
 	cmd.Flags().Bool("skip-helm-wait", false, "install helm charts without waiting for deployments to be ready")
+	cmd.Flags().StringSlice("skip-phases", nil, "comma-separated list of upgrade phases to skip\n"+
+		"one or multiple of { infrastructure | helm | image | k8s }")
 	if err := cmd.Flags().MarkHidden("timeout"); err != nil {
 		panic(err)
 	}
@ -172,10 +189,18 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl
 		return fmt.Errorf("upgrading measurements: %w", err)
 	}

-	tfOutput, err := u.migrateTerraform(cmd, conf, upgradeDir, flags)
+	var tfOutput terraform.ApplyOutput
+	if flags.skipPhases.contains(skipInfrastructurePhase) {
+		tfOutput, err = u.clusterShower.ShowCluster(cmd.Context(), conf.GetProvider())
+		if err != nil {
+			return fmt.Errorf("getting Terraform output: %w", err)
+		}
+	} else {
+		tfOutput, err = u.migrateTerraform(cmd, conf, upgradeDir, flags)
 		if err != nil {
 			return fmt.Errorf("performing Terraform migrations: %w", err)
 		}
+	}
 	// reload idFile after terraform migration
 	// it might have been updated by the migration
 	if err := u.fileHandler.ReadJSON(constants.ClusterIDsFilename, &idFile); err != nil {
@ -197,6 +222,7 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl
 	}

 	var upgradeErr *compatibility.InvalidUpgradeError
+	if !flags.skipPhases.contains(skipHelmPhase) {
 		err = u.handleServiceUpgrade(cmd, conf, idFile, tfOutput, validK8sVersion, upgradeDir, flags)
 		switch {
 		case errors.As(err, &upgradeErr):
@ -206,8 +232,12 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl
 		case err != nil:
 			return fmt.Errorf("upgrading services: %w", err)
 		}
+	}

-	err = u.kubeUpgrader.UpgradeNodeVersion(cmd.Context(), conf, flags.force)
+	skipImageUpgrade := flags.skipPhases.contains(skipImagePhase)
+	skipK8sUpgrade := flags.skipPhases.contains(skipK8sPhase)
+	if !(skipImageUpgrade && skipK8sUpgrade) {
+		err = u.kubeUpgrader.UpgradeNodeVersion(cmd.Context(), conf, flags.force, skipImageUpgrade, skipK8sUpgrade)
 		switch {
 		case errors.Is(err, kubecmd.ErrInProgress):
 			cmd.PrintErrln("Skipping image and Kubernetes upgrades. Another upgrade is in progress.")
@ -216,7 +246,7 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl
 		case err != nil:
 			return fmt.Errorf("upgrading NodeVersion: %w", err)
 		}
-
+	}
 	return nil
 }

@ -516,6 +546,21 @@ func parseUpgradeApplyFlags(cmd *cobra.Command) (upgradeApplyFlags, error) {
 	if skipHelmWait {
 		helmWaitMode = helm.WaitModeNone
 	}
+
+	rawSkipPhases, err := cmd.Flags().GetStringSlice("skip-phases")
+	if err != nil {
+		return upgradeApplyFlags{}, fmt.Errorf("parsing skip-phases flag: %w", err)
+	}
+	var skipPhases []skipPhase
+	for _, phase := range rawSkipPhases {
+		switch skipPhase(phase) {
+		case skipInfrastructurePhase, skipHelmPhase, skipImagePhase, skipK8sPhase:
+			skipPhases = append(skipPhases, skipPhase(phase))
+		default:
+			return upgradeApplyFlags{}, fmt.Errorf("invalid phase %s", phase)
+		}
+	}
+
 	return upgradeApplyFlags{
 		pf:                pathprefix.New(workDir),
 		yes:               yes,
@ -524,6 +569,7 @@ func parseUpgradeApplyFlags(cmd *cobra.Command) (upgradeApplyFlags, error) {
 		terraformLogLevel: logLevel,
 		conformance:       conformance,
 		helmWaitMode:      helmWaitMode,
+		skipPhases:        skipPhases,
 	}, nil
 }

@ -558,10 +604,24 @@ type upgradeApplyFlags struct {
 	terraformLogLevel terraform.LogLevel
 	conformance       bool
 	helmWaitMode      helm.WaitMode
+	skipPhases        skipPhases
+}
+
+// skipPhases is a list of phases that can be skipped during the upgrade process.
+type skipPhases []skipPhase
+
+// contains returns true if the list of phases contains the given phase.
+func (s skipPhases) contains(phase skipPhase) bool {
+	for _, p := range s {
+		if strings.EqualFold(string(p), string(phase)) {
+			return true
+		}
+	}
+	return false
 }

 type kubernetesUpgrader interface {
-	UpgradeNodeVersion(ctx context.Context, conf *config.Config, force bool) error
+	UpgradeNodeVersion(ctx context.Context, conf *config.Config, force, skipImage, skipK8s bool) error
 	ExtendClusterConfigCertSANs(ctx context.Context, alternativeNames []string) error
 	GetClusterAttestationConfig(ctx context.Context, variant variant.Variant) (config.AttestationCfg, error)
 	ApplyJoinConfig(ctx context.Context, newAttestConfig config.AttestationCfg, measurementSalt []byte) error
--- a/cli/internal/cmd/upgradeapply_test.go
+++ b/cli/internal/cmd/upgradeapply_test.go
@ -13,6 +13,7 @@ import (
 	"testing"

 	"github.com/edgelesssys/constellation/v2/cli/internal/clusterid"
+	"github.com/edgelesssys/constellation/v2/cli/internal/helm"
 	"github.com/edgelesssys/constellation/v2/cli/internal/kubecmd"
 	"github.com/edgelesssys/constellation/v2/cli/internal/terraform"
 	"github.com/edgelesssys/constellation/v2/internal/attestation/variant"
@ -22,17 +23,19 @@ import (
 	"github.com/edgelesssys/constellation/v2/internal/file"
 	"github.com/edgelesssys/constellation/v2/internal/kms/uri"
 	"github.com/edgelesssys/constellation/v2/internal/logger"
+	"github.com/edgelesssys/constellation/v2/internal/versions"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 )

 func TestUpgradeApply(t *testing.T) {
 	testCases := map[string]struct {
-		helmUpgrader      stubApplier
+		helmUpgrader      helmApplier
 		kubeUpgrader      *stubKubernetesUpgrader
-		terraformUpgrader *stubTerraformUpgrader
+		terraformUpgrader clusterUpgrader
 		wantErr           bool
 		flags             upgradeApplyFlags
 		stdin             string
@ -101,6 +104,28 @@ func TestUpgradeApply(t *testing.T) {
 			wantErr: true,
 			flags:   upgradeApplyFlags{yes: true},
 		},
+		"skip all upgrade phases": {
+			kubeUpgrader: &stubKubernetesUpgrader{
+				currentConfig: config.DefaultForAzureSEVSNP(),
+			},
+			helmUpgrader:      &mockApplier{}, // mocks ensure that no methods are called
+			terraformUpgrader: &mockTerraformUpgrader{},
+			flags: upgradeApplyFlags{
+				skipPhases: []skipPhase{skipInfrastructurePhase, skipHelmPhase, skipK8sPhase, skipImagePhase},
+				yes:        true,
+			},
+		},
+		"skip all phases except node upgrade": {
+			kubeUpgrader: &stubKubernetesUpgrader{
+				currentConfig: config.DefaultForAzureSEVSNP(),
+			},
+			helmUpgrader:      &mockApplier{}, // mocks ensure that no methods are called
+			terraformUpgrader: &mockTerraformUpgrader{},
+			flags: upgradeApplyFlags{
+				skipPhases: []skipPhase{skipInfrastructurePhase, skipHelmPhase, skipK8sPhase},
+				yes:        true,
+			},
+		},
 	}

 	for name, tc := range testCases {
@ -134,46 +159,63 @@ func TestUpgradeApply(t *testing.T) {
 				return
 			}
 			assert.NoError(err)
+			assert.Equal(!tc.flags.skipPhases.contains(skipImagePhase), tc.kubeUpgrader.calledNodeUpgrade,
+				"incorrect node upgrade skipping behavior")
 		})
 	}
 }

+func TestUpgradeApplyFlagsForSkipPhases(t *testing.T) {
+	cmd := newUpgradeApplyCmd()
+	cmd.Flags().String("workspace", "", "")  // register persistent flag manually
+	cmd.Flags().Bool("force", true, "")      // register persistent flag manually
+	cmd.Flags().String("tf-log", "NONE", "") // register persistent flag manually
+	require.NoError(t, cmd.Flags().Set("skip-phases", "infrastructure,helm,k8s,image"))
+	result, err := parseUpgradeApplyFlags(cmd)
+	if err != nil {
+		t.Fatalf("Error while parsing flags: %v", err)
+	}
+	assert.ElementsMatch(t, []skipPhase{skipInfrastructurePhase, skipHelmPhase, skipK8sPhase, skipImagePhase}, result.skipPhases)
+}
+
 type stubKubernetesUpgrader struct {
 	nodeVersionErr    error
 	currentConfig     config.AttestationCfg
+	calledNodeUpgrade bool
 }

-func (u stubKubernetesUpgrader) BackupCRDs(_ context.Context, _ string) ([]apiextensionsv1.CustomResourceDefinition, error) {
+func (u *stubKubernetesUpgrader) BackupCRDs(_ context.Context, _ string) ([]apiextensionsv1.CustomResourceDefinition, error) {
 	return []apiextensionsv1.CustomResourceDefinition{}, nil
 }

-func (u stubKubernetesUpgrader) BackupCRs(_ context.Context, _ []apiextensionsv1.CustomResourceDefinition, _ string) error {
+func (u *stubKubernetesUpgrader) BackupCRs(_ context.Context, _ []apiextensionsv1.CustomResourceDefinition, _ string) error {
 	return nil
 }

-func (u stubKubernetesUpgrader) UpgradeNodeVersion(_ context.Context, _ *config.Config, _ bool) error {
+func (u *stubKubernetesUpgrader) UpgradeNodeVersion(_ context.Context, _ *config.Config, _, _, _ bool) error {
+	u.calledNodeUpgrade = true
 	return u.nodeVersionErr
 }

-func (u stubKubernetesUpgrader) ApplyJoinConfig(_ context.Context, _ config.AttestationCfg, _ []byte) error {
+func (u *stubKubernetesUpgrader) ApplyJoinConfig(_ context.Context, _ config.AttestationCfg, _ []byte) error {
 	return nil
 }

-func (u stubKubernetesUpgrader) GetClusterAttestationConfig(_ context.Context, _ variant.Variant) (config.AttestationCfg, error) {
+func (u *stubKubernetesUpgrader) GetClusterAttestationConfig(_ context.Context, _ variant.Variant) (config.AttestationCfg, error) {
 	return u.currentConfig, nil
 }

-func (u stubKubernetesUpgrader) ExtendClusterConfigCertSANs(_ context.Context, _ []string) error {
+func (u *stubKubernetesUpgrader) ExtendClusterConfigCertSANs(_ context.Context, _ []string) error {
 	return nil
 }

 // TODO(v2.11): Remove this function.
-func (u stubKubernetesUpgrader) RemoveAttestationConfigHelmManagement(_ context.Context) error {
+func (u *stubKubernetesUpgrader) RemoveAttestationConfigHelmManagement(_ context.Context) error {
 	return nil
 }

 // TODO(v2.12): Remove this function.
-func (u stubKubernetesUpgrader) RemoveHelmKeepAnnotation(_ context.Context) error {
+func (u *stubKubernetesUpgrader) RemoveHelmKeepAnnotation(_ context.Context) error {
 	return nil
 }

@ -190,3 +232,26 @@ func (u stubTerraformUpgrader) PlanClusterUpgrade(_ context.Context, _ io.Writer
 func (u stubTerraformUpgrader) ApplyClusterUpgrade(_ context.Context, _ cloudprovider.Provider) (terraform.ApplyOutput, error) {
 	return terraform.ApplyOutput{}, u.applyTerraformErr
 }
+
+type mockTerraformUpgrader struct {
+	mock.Mock
+}
+
+func (m *mockTerraformUpgrader) PlanClusterUpgrade(ctx context.Context, w io.Writer, variables terraform.Variables, provider cloudprovider.Provider) (bool, error) {
+	args := m.Called(ctx, w, variables, provider)
+	return args.Bool(0), args.Error(1)
+}
+
+func (m *mockTerraformUpgrader) ApplyClusterUpgrade(ctx context.Context, provider cloudprovider.Provider) (terraform.ApplyOutput, error) {
+	args := m.Called(ctx, provider)
+	return args.Get(0).(terraform.ApplyOutput), args.Error(1)
+}
+
+type mockApplier struct {
+	mock.Mock
+}
+
+func (m *mockApplier) PrepareApply(cfg *config.Config, k8sVersion versions.ValidK8sVersion, clusterID clusterid.File, helmOpts helm.Options, terraformOut terraform.ApplyOutput, str string, masterSecret uri.MasterSecret) (helm.Applier, bool, error) {
+	args := m.Called(cfg, k8sVersion, clusterID, helmOpts, terraformOut, str, masterSecret)
+	return args.Get(0).(helm.Applier), args.Bool(1), args.Error(2)
+}
--- a/cli/internal/kubecmd/kubecmd.go
+++ b/cli/internal/kubecmd/kubecmd.go
@ -99,7 +99,8 @@ func New(outWriter io.Writer, kubeConfigPath string, fileHandler file.Handler, l

 // UpgradeNodeVersion upgrades the cluster's NodeVersion object and in turn triggers image & k8s version upgrades.
 // The versions set in the config are validated against the versions running in the cluster.
-func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, force bool) error {
+// TODO(elchead): AB#3434 Split K8s and image upgrade of UpgradeNodeVersion.
+func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, force, skipImage, skipK8s bool) error {
 	provider := conf.GetProvider()
 	attestationVariant := conf.GetAttestationConfig().GetVariant()
 	region := conf.GetRegion()
@ -120,7 +121,7 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f

 	upgradeErrs := []error{}
 	var upgradeErr *compatibility.InvalidUpgradeError
-
+	if !skipImage {
 		err = k.isValidImageUpgrade(nodeVersion, imageVersion.Version(), force)
 		switch {
 		case errors.As(err, &upgradeErr):
@ -131,7 +132,9 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f
 		k.log.Debugf("Updating local copy of nodeVersion image version from %s to %s", nodeVersion.Spec.ImageVersion, imageVersion.Version())
 		nodeVersion.Spec.ImageReference = imageReference
 		nodeVersion.Spec.ImageVersion = imageVersion.Version()
+	}

+	if !skipK8s {
 		// We have to allow users to specify outdated k8s patch versions.
 		// Therefore, this code has to skip k8s updates if a user configures an outdated (i.e. invalid) k8s version.
 		var components *corev1.ConfigMap
@ -155,6 +158,7 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f
 		default:
 			return fmt.Errorf("updating Kubernetes version: %w", err)
 		}
+	}

 	if len(upgradeErrs) == 2 {
 		return errors.Join(upgradeErrs...)
--- a/cli/internal/kubecmd/kubecmd_test.go
+++ b/cli/internal/kubecmd/kubecmd_test.go
@ -332,7 +332,7 @@ func TestUpgradeNodeVersion(t *testing.T) {
 				outWriter:    io.Discard,
 			}

-			err = upgrader.UpgradeNodeVersion(context.Background(), tc.conf, tc.force)
+			err = upgrader.UpgradeNodeVersion(context.Background(), tc.conf, tc.force, false, false)
 			// Check upgrades first because if we checked err first, UpgradeImage may error due to other reasons and still trigger an upgrade.
 			if tc.wantUpdate {
 				assert.NotNil(unstructuredClient.updatedObject)
--- a/docs/docs/reference/cli.md
+++ b/docs/docs/reference/cli.md
@ -475,6 +475,8 @@ constellation upgrade apply [flags]
      --conformance           enable conformance mode
  -h, --help                  help for apply
      --skip-helm-wait        install helm charts without waiting for deployments to be ready
+      --skip-phases strings   comma-separated list of upgrade phases to skip
+                              one or multiple of { infrastructure | helm | image | k8s }
  -y, --yes                   run upgrades without further confirmation
                              WARNING: might delete your resources in case you are using cert-manager in your cluster. Please read the docs.
                              WARNING: might unintentionally overwrite measurements in the running cluster.