From db7d2c5bdd2868b38ca14436eb9f2eaa51a719ac Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Mon, 11 Sep 2023 15:51:04 +0200 Subject: [PATCH 1/2] init --- cli/internal/cmd/upgradeapply.go | 24 ++- cli/internal/cmd/upgradeapply_test.go | 7 +- cli/internal/kubecmd/kubecmd.go | 100 +++++----- cli/internal/kubecmd/kubecmd_test.go | 273 +++++++++++++++----------- 4 files changed, 231 insertions(+), 173 deletions(-) diff --git a/cli/internal/cmd/upgradeapply.go b/cli/internal/cmd/upgradeapply.go index 6fa6e6fdc3..cad0ee0ebe 100644 --- a/cli/internal/cmd/upgradeapply.go +++ b/cli/internal/cmd/upgradeapply.go @@ -233,20 +233,29 @@ func (u *upgradeApplyCmd) upgradeApply(cmd *cobra.Command, upgradeDir string, fl return fmt.Errorf("upgrading services: %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) + // image upgrades need to be done before k8s upgrade because image upgrades are not allowed during another upgrade + if !flags.skipPhases.contains(skipImagePhase) { + err = u.kubeUpgrader.UpgradeImageVersion(cmd.Context(), conf, flags.force) switch { case errors.Is(err, kubecmd.ErrInProgress): - cmd.PrintErrln("Skipping image and Kubernetes upgrades. Another upgrade is in progress.") + cmd.PrintErrln("Skipping image upgrade. Another upgrade is in progress.") case errors.As(err, &upgradeErr): cmd.PrintErrln(err) case err != nil: return fmt.Errorf("upgrading NodeVersion: %w", err) } } + if !flags.skipPhases.contains(skipK8sPhase) { + err = u.kubeUpgrader.UpgradeK8sVersion(cmd.Context(), validK8sVersion, flags.force) + switch { + case errors.As(err, &upgradeErr): + cmd.PrintErrln(err) + case err == nil: + cmd.Println("Successfully upgraded Kubernetes.") + case err != nil: + return fmt.Errorf("upgrading Kubernetes: %w", err) + } + } return nil } @@ -621,7 +630,8 @@ func (s skipPhases) contains(phase skipPhase) bool { } type kubernetesUpgrader interface { - UpgradeNodeVersion(ctx context.Context, conf *config.Config, force, skipImage, skipK8s bool) error + UpgradeImageVersion(ctx context.Context, conf *config.Config, force bool) error + UpgradeK8sVersion(ctx context.Context, k8sVersion versions.ValidK8sVersion, force 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 a489de3004..29667b2cae 100644 --- a/cli/internal/cmd/upgradeapply_test.go +++ b/cli/internal/cmd/upgradeapply_test.go @@ -180,6 +180,7 @@ func TestUpgradeApplyFlagsForSkipPhases(t *testing.T) { type stubKubernetesUpgrader struct { nodeVersionErr error + k8sErr error currentConfig config.AttestationCfg calledNodeUpgrade bool } @@ -192,11 +193,15 @@ func (u *stubKubernetesUpgrader) BackupCRs(_ context.Context, _ []apiextensionsv return nil } -func (u *stubKubernetesUpgrader) UpgradeNodeVersion(_ context.Context, _ *config.Config, _, _, _ bool) error { +func (u *stubKubernetesUpgrader) UpgradeImageVersion(_ context.Context, _ *config.Config, _ bool) error { u.calledNodeUpgrade = true return u.nodeVersionErr } +func (u *stubKubernetesUpgrader) UpgradeK8sVersion(_ context.Context, _ versions.ValidK8sVersion, _ bool) error { + return u.k8sErr +} + func (u *stubKubernetesUpgrader) ApplyJoinConfig(_ context.Context, _ config.AttestationCfg, _ []byte) error { return nil } diff --git a/cli/internal/kubecmd/kubecmd.go b/cli/internal/kubecmd/kubecmd.go index fee16e32e0..e6db339532 100644 --- a/cli/internal/kubecmd/kubecmd.go +++ b/cli/internal/kubecmd/kubecmd.go @@ -97,10 +97,44 @@ func New(outWriter io.Writer, kubeConfigPath string, fileHandler file.Handler, l }, nil } -// UpgradeNodeVersion upgrades the cluster's NodeVersion object and in turn triggers image & k8s version upgrades. +// UpgradeK8sVersion upgrades the cluster's Kubernetes version. +func (k *KubeCmd) UpgradeK8sVersion(ctx context.Context, k8sVersion versions.ValidK8sVersion, force bool) error { + nodeVersion, err := k.getConstellationVersion(ctx) + if err != nil { + return err + } + // 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 + _, err = versions.NewValidK8sVersion(string(k8sVersion), true) + if err != nil { + innerErr := fmt.Errorf("unsupported Kubernetes version, supported versions are %s", strings.Join(versions.SupportedK8sVersions(), ", ")) + return compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, string(k8sVersion), innerErr) + } + versionConfig, ok := versions.VersionConfigs[k8sVersion] + if !ok { + return compatibility.NewInvalidUpgradeError(nodeVersion.Spec.KubernetesClusterVersion, + string(k8sVersion), fmt.Errorf("no version config matching K8s %s", k8sVersion)) + } + components, err = k.updateK8s(&nodeVersion, versionConfig.ClusterVersion, versionConfig.KubernetesComponents, force) + if err == nil { + err = k.applyComponentsCM(ctx, components) + if err != nil { + return fmt.Errorf("applying k8s components ConfigMap: %w", err) + } + } + fmt.Println("Successfully updated Kubernetes version", nodeVersion.Spec.KubernetesClusterVersion, "conf", k8sVersion) + updatedNodeVersion, err := k.applyNodeVersion(ctx, nodeVersion) + if err != nil { + return fmt.Errorf("applying upgrade: %w", err) + } + return checkForApplyError(nodeVersion, updatedNodeVersion) +} + +// UpgradeImageVersion upgrades the cluster's image. // The versions set in the config are validated against the versions running in the cluster. // 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 { +func (k *KubeCmd) UpgradeImageVersion(ctx context.Context, conf *config.Config, force bool) error { provider := conf.GetProvider() attestationVariant := conf.GetAttestationConfig().GetVariant() region := conf.GetRegion() @@ -119,60 +153,22 @@ func (k *KubeCmd) UpgradeNodeVersion(ctx context.Context, conf *config.Config, f return err } - upgradeErrs := []error{} var upgradeErr *compatibility.InvalidUpgradeError - 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) - } - } - - if len(upgradeErrs) == 2 { - return errors.Join(upgradeErrs...) - } - + err = k.isValidImageUpgrade(nodeVersion, imageVersion.Version(), force) + switch { + case errors.As(err, &upgradeErr): + return 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\n", nodeVersion.Spec.ImageVersion, imageVersion.Version()) + nodeVersion.Spec.ImageReference = imageReference + nodeVersion.Spec.ImageVersion = imageVersion.Version() updatedNodeVersion, err := k.applyNodeVersion(ctx, nodeVersion) if err != nil { return fmt.Errorf("applying upgrade: %w", err) } - - if err := checkForApplyError(nodeVersion, updatedNodeVersion); err != nil { - return err - } - return errors.Join(upgradeErrs...) + return checkForApplyError(nodeVersion, updatedNodeVersion) } // ClusterStatus returns a map from node name to NodeStatus. @@ -432,7 +428,7 @@ func (k *KubeCmd) updateK8s(nodeVersion *updatev1alpha1.NodeVersion, newClusterV } } - k.log.Debugf("Updating local copy of nodeVersion Kubernetes version from %s to %s", nodeVersion.Spec.KubernetesClusterVersion, newClusterVersion) + k.log.Debugf("Updating local copy of nodeVersion Kubernetes version from %s to %s\n", nodeVersion.Spec.KubernetesClusterVersion, newClusterVersion) nodeVersion.Spec.KubernetesComponentsReference = configMap.ObjectMeta.Name nodeVersion.Spec.KubernetesClusterVersion = newClusterVersion diff --git a/cli/internal/kubecmd/kubecmd_test.go b/cli/internal/kubecmd/kubecmd_test.go index ec24a274bd..93b94b2516 100644 --- a/cli/internal/kubecmd/kubecmd_test.go +++ b/cli/internal/kubecmd/kubecmd_test.go @@ -36,75 +36,36 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) -func TestUpgradeNodeVersion(t *testing.T) { +func TestUpgradeImageVersion(t *testing.T) { testCases := map[string]struct { - kubectl *stubKubectl - conditions []metav1.Condition - currentImageVersion string - newImageReference string - badImageVersion string - currentClusterVersion string - conf *config.Config - force bool - getCRErr error - wantErr bool - wantUpdate bool - assertCorrectError func(t *testing.T, err error) bool - customClientFn func(nodeVersion updatev1alpha1.NodeVersion) unstructuredInterface + kubectl *stubKubectl + conditions []metav1.Condition + currentImageVersion string + newImageReference string + badImageVersion string + // currentClusterVersion string + conf *config.Config + force bool + getCRErr error + wantErr bool + wantUpdate bool + assertCorrectError func(t *testing.T, err error) bool + customClientFn func(nodeVersion updatev1alpha1.NodeVersion) unstructuredInterface }{ - "success": { - conf: func() *config.Config { - conf := config.Default() - conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] - return conf - }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], - kubectl: &stubKubectl{ - configMaps: map[string]*corev1.ConfigMap{ - constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), - }, - }, - wantUpdate: true, - }, - "only k8s upgrade": { - conf: func() *config.Config { - conf := config.Default() - conf.Image = "v1.2.2" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] - return conf - }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], - kubectl: &stubKubectl{ - configMaps: map[string]*corev1.ConfigMap{ - constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), - }, - }, - wantUpdate: true, - wantErr: true, - assertCorrectError: func(t *testing.T, err error) bool { - var upgradeErr *compatibility.InvalidUpgradeError - return assert.ErrorAs(t, err, &upgradeErr) - }, - }, - "only image upgrade": { + "image upgrade": { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" conf.KubernetesVersion = versions.SupportedK8sVersions()[0] return conf }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentImageVersion: "v1.2.2", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), }, }, wantUpdate: true, - wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { var upgradeErr *compatibility.InvalidUpgradeError return assert.ErrorAs(t, err, &upgradeErr) @@ -117,10 +78,9 @@ func TestUpgradeNodeVersion(t *testing.T) { conf.KubernetesVersion = versions.SupportedK8sVersions()[0] return conf }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], - kubectl: &stubKubectl{}, - wantErr: true, + currentImageVersion: "v1.2.2", + kubectl: &stubKubectl{}, + wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { var upgradeErr *compatibility.InvalidUpgradeError return assert.ErrorAs(t, err, &upgradeErr) @@ -137,10 +97,9 @@ func TestUpgradeNodeVersion(t *testing.T) { Type: updatev1alpha1.ConditionOutdated, Status: metav1.ConditionTrue, }}, - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], - kubectl: &stubKubectl{}, - wantErr: true, + currentImageVersion: "v1.2.2", + kubectl: &stubKubectl{}, + wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { return assert.ErrorIs(t, err, ErrInProgress) }, @@ -156,11 +115,10 @@ func TestUpgradeNodeVersion(t *testing.T) { Type: updatev1alpha1.ConditionOutdated, Status: metav1.ConditionTrue, }}, - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], - kubectl: &stubKubectl{}, - force: true, - wantUpdate: true, + currentImageVersion: "v1.2.2", + kubectl: &stubKubectl{}, + force: true, + wantUpdate: true, }, "get error": { conf: func() *config.Config { @@ -169,8 +127,7 @@ func TestUpgradeNodeVersion(t *testing.T) { conf.KubernetesVersion = versions.SupportedK8sVersions()[1] return conf }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentImageVersion: "v1.2.2", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -182,23 +139,20 @@ func TestUpgradeNodeVersion(t *testing.T) { return assert.ErrorIs(t, err, assert.AnError) }, }, - "image too new valid k8s": { + "image too new": { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.4.2" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] return conf }(), - newImageReference: "path/to/image:v1.4.2", - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + newImageReference: "path/to/image:v1.4.2", + currentImageVersion: "v1.2.2", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":true}}`), }, }, - wantUpdate: true, - wantErr: true, + wantErr: true, assertCorrectError: func(t *testing.T, err error) bool { var upgradeErr *compatibility.InvalidUpgradeError return assert.ErrorAs(t, err, &upgradeErr) @@ -211,9 +165,8 @@ func TestUpgradeNodeVersion(t *testing.T) { conf.KubernetesVersion = versions.SupportedK8sVersions()[1] return conf }(), - newImageReference: "path/to/image:v1.4.2", - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + newImageReference: "path/to/image:v1.4.2", + currentImageVersion: "v1.2.2", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -222,16 +175,15 @@ func TestUpgradeNodeVersion(t *testing.T) { wantUpdate: true, force: true, }, - "apply returns bad object": { + "node version apply returns wrong image": { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" conf.KubernetesVersion = versions.SupportedK8sVersions()[1] return conf }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], - badImageVersion: "v3.2.1", + currentImageVersion: "v1.2.2", + badImageVersion: "v3.2.1", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -244,36 +196,13 @@ func TestUpgradeNodeVersion(t *testing.T) { return assert.ErrorAs(t, err, &target) }, }, - "outdated k8s version skips k8s upgrade": { - conf: func() *config.Config { - conf := config.Default() - conf.Image = "v1.2.2" - conf.KubernetesVersion = "v1.25.8" - return conf - }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], - kubectl: &stubKubectl{ - configMaps: map[string]*corev1.ConfigMap{ - constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), - }, - }, - wantUpdate: false, - wantErr: true, - assertCorrectError: func(t *testing.T, err error) bool { - var upgradeErr *compatibility.InvalidUpgradeError - return assert.ErrorAs(t, err, &upgradeErr) - }, - }, "succeed after update retry when the updated node object is outdated": { conf: func() *config.Config { conf := config.Default() conf.Image = "v1.2.3" - conf.KubernetesVersion = versions.SupportedK8sVersions()[1] return conf }(), - currentImageVersion: "v1.2.2", - currentClusterVersion: versions.SupportedK8sVersions()[0], + currentImageVersion: "v1.2.2", kubectl: &stubKubectl{ configMaps: map[string]*corev1.ConfigMap{ constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), @@ -297,8 +226,7 @@ func TestUpgradeNodeVersion(t *testing.T) { nodeVersion := updatev1alpha1.NodeVersion{ Spec: updatev1alpha1.NodeVersionSpec{ - ImageVersion: tc.currentImageVersion, - KubernetesClusterVersion: tc.currentClusterVersion, + ImageVersion: tc.currentImageVersion, }, Status: updatev1alpha1.NodeVersionStatus{ Conditions: tc.conditions, @@ -307,8 +235,12 @@ func TestUpgradeNodeVersion(t *testing.T) { var badUpdatedObject *unstructured.Unstructured if tc.badImageVersion != "" { - nodeVersion.Spec.ImageVersion = tc.badImageVersion - unstrBadNodeVersion, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&nodeVersion) + badVersion := updatev1alpha1.NodeVersion{ + Spec: updatev1alpha1.NodeVersionSpec{ + ImageVersion: tc.badImageVersion, + }, + } + unstrBadNodeVersion, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&badVersion) require.NoError(err) badUpdatedObject = &unstructured.Unstructured{Object: unstrBadNodeVersion} } @@ -332,7 +264,122 @@ func TestUpgradeNodeVersion(t *testing.T) { outWriter: io.Discard, } - err = upgrader.UpgradeNodeVersion(context.Background(), tc.conf, tc.force, false, false) + err = upgrader.UpgradeImageVersion(context.Background(), tc.conf, tc.force) + // 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) + } else { + assert.Nil(unstructuredClient.updatedObject) + } + + if tc.wantErr { + assert.Error(err) + tc.assertCorrectError(t, err) + return + } + assert.NoError(err) + }) + } +} + +func TestUpgradeK8s(t *testing.T) { + testCases := map[string]struct { + kubectl *stubKubectl + conditions []metav1.Condition + currentClusterVersion string + conf *config.Config + force bool + getCRErr error + wantErr bool + wantUpdate bool + assertCorrectError func(t *testing.T, err error) bool + customClientFn func(nodeVersion updatev1alpha1.NodeVersion) unstructuredInterface + }{ + "upgrade k8s": { + conf: func() *config.Config { + conf := config.Default() + conf.KubernetesVersion = versions.SupportedK8sVersions()[1] + return conf + }(), + currentClusterVersion: versions.SupportedK8sVersions()[0], + kubectl: &stubKubectl{ + configMaps: map[string]*corev1.ConfigMap{ + constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), + }, + }, + wantUpdate: true, + }, + "outdated k8s version": { + conf: func() *config.Config { + conf := config.Default() + conf.KubernetesVersion = "v1.0.0" + return conf + }(), + currentClusterVersion: versions.SupportedK8sVersions()[0], + kubectl: &stubKubectl{ + configMaps: map[string]*corev1.ConfigMap{ + constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), + }, + }, + wantUpdate: false, + wantErr: true, + assertCorrectError: func(t *testing.T, err error) bool { + var upgradeErr *compatibility.InvalidUpgradeError + return assert.ErrorAs(t, err, &upgradeErr) + }, + }, + "fail even with force for too new k8s version": { + conf: func() *config.Config { + conf := config.Default() + conf.KubernetesVersion = "v1.99.0" + return conf + }(), + currentClusterVersion: versions.SupportedK8sVersions()[0], + kubectl: &stubKubectl{ + configMaps: map[string]*corev1.ConfigMap{ + constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`), + }, + }, + force: true, + wantErr: true, + assertCorrectError: func(t *testing.T, err error) bool { + var upgradeErr *compatibility.InvalidUpgradeError + return assert.ErrorAs(t, err, &upgradeErr) + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + nodeVersion := updatev1alpha1.NodeVersion{ + Spec: updatev1alpha1.NodeVersionSpec{ + KubernetesClusterVersion: tc.currentClusterVersion, + }, + Status: updatev1alpha1.NodeVersionStatus{ + Conditions: tc.conditions, + }, + } + unstrNodeVersion, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&nodeVersion) + require.NoError(err) + unstructuredClient := &stubUnstructuredClient{ + object: &unstructured.Unstructured{Object: unstrNodeVersion}, + getCRErr: tc.getCRErr, + } + tc.kubectl.unstructuredInterface = unstructuredClient + if tc.customClientFn != nil { + tc.kubectl.unstructuredInterface = tc.customClientFn(nodeVersion) + } + + upgrader := KubeCmd{ + kubectl: tc.kubectl, + log: logger.NewTest(t), + outWriter: io.Discard, + } + assert.Nil(unstructuredClient.updatedObject) + + err = upgrader.UpgradeK8sVersion(context.Background(), versions.ValidK8sVersion(tc.conf.KubernetesVersion), tc.force) // 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) From f70110b53761853448a04bd1dbcf7806b83dcdad Mon Sep 17 00:00:00 2001 From: Adrian Stobbe Date: Tue, 12 Sep 2023 11:47:28 +0200 Subject: [PATCH 2/2] better error handling --- cli/internal/kubecmd/kubecmd.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/cli/internal/kubecmd/kubecmd.go b/cli/internal/kubecmd/kubecmd.go index e6db339532..08dde7c80b 100644 --- a/cli/internal/kubecmd/kubecmd.go +++ b/cli/internal/kubecmd/kubecmd.go @@ -117,13 +117,12 @@ func (k *KubeCmd) UpgradeK8sVersion(ctx context.Context, k8sVersion versions.Val string(k8sVersion), fmt.Errorf("no version config matching K8s %s", k8sVersion)) } components, err = k.updateK8s(&nodeVersion, versionConfig.ClusterVersion, versionConfig.KubernetesComponents, force) - if err == nil { - err = k.applyComponentsCM(ctx, components) - if err != nil { - return fmt.Errorf("applying k8s components ConfigMap: %w", err) - } + if err != nil { + return fmt.Errorf("updating K8s: %w", err) + } + if err := k.applyComponentsCM(ctx, components); err != nil { + return fmt.Errorf("applying k8s components ConfigMap: %w", err) } - fmt.Println("Successfully updated Kubernetes version", nodeVersion.Spec.KubernetesClusterVersion, "conf", k8sVersion) updatedNodeVersion, err := k.applyNodeVersion(ctx, nodeVersion) if err != nil { return fmt.Errorf("applying upgrade: %w", err) @@ -133,7 +132,6 @@ func (k *KubeCmd) UpgradeK8sVersion(ctx context.Context, k8sVersion versions.Val // UpgradeImageVersion upgrades the cluster's image. // The versions set in the config are validated against the versions running in the cluster. -// TODO(elchead): AB#3434 Split K8s and image upgrade of UpgradeNodeVersion. func (k *KubeCmd) UpgradeImageVersion(ctx context.Context, conf *config.Config, force bool) error { provider := conf.GetProvider() attestationVariant := conf.GetAttestationConfig().GetVariant()