From 5960025da752ade41f09dce3d2362da8eb3c94cc Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Fri, 8 Sep 2023 14:55:07 +0200 Subject: [PATCH] cli: new flag to skip phases of upgrade (#2310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Daniel Weiße <66256922+daniel-weisse@users.noreply.github.com> Co-authored-by: Thomas Tendyck <51411342+thomasten@users.noreply.github.com> --- cli/internal/cmd/BUILD.bazel | 1 + cli/internal/cmd/upgradeapply.go | 102 ++++++++++++++++++++------ cli/internal/cmd/upgradeapply_test.go | 91 +++++++++++++++++++---- cli/internal/kubecmd/kubecmd.go | 72 +++++++++--------- cli/internal/kubecmd/kubecmd_test.go | 2 +- docs/docs/reference/cli.md | 14 ++-- 6 files changed, 207 insertions(+), 75 deletions(-) diff --git a/cli/internal/cmd/BUILD.bazel b/cli/internal/cmd/BUILD.bazel index 2428cc7bc..a188e396c 100644 --- 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", diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index cdd2781a0..6fa6e6fdc 100644 --- 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,9 +189,17 @@ 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) - if err != nil { - return fmt.Errorf("performing Terraform migrations: %w", err) + 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 @@ -197,26 +222,31 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl } var upgradeErr *compatibility.InvalidUpgradeError - err = u.handleServiceUpgrade(cmd, conf, idFile, tfOutput, validK8sVersion, upgradeDir, flags) - switch { - case errors.As(err, &upgradeErr): - cmd.PrintErrln(err) - case err == nil: - cmd.Println("Successfully upgraded Constellation services.") - case err != nil: - return fmt.Errorf("upgrading services: %w", err) + if !flags.skipPhases.contains(skipHelmPhase) { + err = u.handleServiceUpgrade(cmd, conf, idFile, tfOutput, validK8sVersion, upgradeDir, flags) + switch { + case errors.As(err, &upgradeErr): + cmd.PrintErrln(err) + case err == nil: + cmd.Println("Successfully upgraded Constellation services.") + case err != nil: + return fmt.Errorf("upgrading services: %w", err) + } } - err = u.kubeUpgrader.UpgradeNodeVersion(cmd.Context(), conf, flags.force) - switch { - case errors.Is(err, kubecmd.ErrInProgress): - cmd.PrintErrln("Skipping image and Kubernetes upgrades. Another upgrade is in progress.") - case errors.As(err, &upgradeErr): - cmd.PrintErrln(err) - case err != nil: - return fmt.Errorf("upgrading NodeVersion: %w", err) + 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.") + case errors.As(err, &upgradeErr): + cmd.PrintErrln(err) + 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 diff --git a/cli/internal/cmd/upgradeapply_test.go b/cli/internal/cmd/upgradeapply_test.go index 6b2e68629..ce8d10c6a 100644 --- 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") }) } } -type stubKubernetesUpgrader struct { - nodeVersionErr error - currentConfig config.AttestationCfg +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) } -func (u stubKubernetesUpgrader) BackupCRDs(_ context.Context, _ string) ([]apiextensionsv1.CustomResourceDefinition, error) { +type stubKubernetesUpgrader struct { + nodeVersionErr error + currentConfig config.AttestationCfg + calledNodeUpgrade bool +} + +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) +} diff --git a/cli/internal/kubecmd/kubecmd.go b/cli/internal/kubecmd/kubecmd.go index 41c88c633..fee16e32e 100644 --- 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,40 +121,43 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f upgradeErrs := []error{} var upgradeErr *compatibility.InvalidUpgradeError - - err = k.isValidImageUpgrade(nodeVersion, imageVersion.Version(), force) - switch { - case errors.As(err, &upgradeErr): - upgradeErrs = append(upgradeErrs, fmt.Errorf("skipping image upgrades: %w", err)) - case err != nil: - return fmt.Errorf("updating image version: %w", err) - } - 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() - - // 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 - currentK8sVersion, err := versions.NewValidK8sVersion(conf.KubernetesVersion, true) - if err != nil { - innerErr := fmt.Errorf("unsupported Kubernetes version, supported versions are %s", strings.Join(versions.SupportedK8sVersions(), ", ")) - err = compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, conf.KubernetesVersion, innerErr) - } else { - versionConfig := versions.VersionConfigs[currentK8sVersion] - components, err = k.updateK8s(&nodeVersion, versionConfig.ClusterVersion, versionConfig.KubernetesComponents, force) - } - - switch { - case err == nil: - err := k.applyComponentsCM(ctx, components) - if err != nil { - return fmt.Errorf("applying k8s components ConfigMap: %w", err) + if !skipImage { + err = k.isValidImageUpgrade(nodeVersion, imageVersion.Version(), force) + switch { + case errors.As(err, &upgradeErr): + upgradeErrs = append(upgradeErrs, fmt.Errorf("skipping image upgrades: %w", err)) + case err != nil: + return fmt.Errorf("updating image version: %w", err) + } + 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 + currentK8sVersion, err := versions.NewValidK8sVersion(conf.KubernetesVersion, true) + if err != nil { + innerErr := fmt.Errorf("unsupported Kubernetes version, supported versions are %s", strings.Join(versions.SupportedK8sVersions(), ", ")) + err = compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, conf.KubernetesVersion, innerErr) + } else { + versionConfig := versions.VersionConfigs[currentK8sVersion] + components, err = k.updateK8s(&nodeVersion, versionConfig.ClusterVersion, versionConfig.KubernetesComponents, force) + } + + switch { + case err == nil: + err := k.applyComponentsCM(ctx, components) + if err != nil { + return fmt.Errorf("applying k8s components ConfigMap: %w", err) + } + case errors.As(err, &upgradeErr): + upgradeErrs = append(upgradeErrs, fmt.Errorf("skipping Kubernetes upgrades: %w", err)) + default: + return fmt.Errorf("updating Kubernetes version: %w", err) } - case errors.As(err, &upgradeErr): - upgradeErrs = append(upgradeErrs, fmt.Errorf("skipping Kubernetes upgrades: %w", err)) - default: - return fmt.Errorf("updating Kubernetes version: %w", err) } if len(upgradeErrs) == 2 { diff --git a/cli/internal/kubecmd/kubecmd_test.go b/cli/internal/kubecmd/kubecmd_test.go index 312d2c833..ec24a274b 100644 --- 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) diff --git a/docs/docs/reference/cli.md b/docs/docs/reference/cli.md index 56a3344b3..2d9a8ebff 100644 --- a/docs/docs/reference/cli.md +++ b/docs/docs/reference/cli.md @@ -472,12 +472,14 @@ constellation upgrade apply [flags] ### Options ``` - --conformance enable conformance mode - -h, --help help for apply - --skip-helm-wait install helm charts without waiting for deployments to be ready - -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. + --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. ``` ### Options inherited from parent commands