cli: fix upgrade apply for image-only upgrades (#1468)

This fixes a bug where `upgrade apply` fails if only the image is
upgraded, due to mishandling of an empty configmap.
Making stubStableClient more complex is needed since it is called
with multiple configMaps now.

cli/internal/cloudcmd/upgrade.go

@@ -115,13 +115,24 @@ func (u *Upgrader) UpgradeNodeVersion(ctx context.Context, conf *config.Config)
 	upgradeErrs := []error{}
 	upgradeErr := &compatibility.InvalidUpgradeError{}
 	err = u.updateImage(&nodeVersion, imageReference, imageVersion.Version)
-	if errors.As(err, &upgradeErr) {
+	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)
 	}
 
 	components, err := u.updateK8s(&nodeVersion, versionConfig.ClusterVersion, versionConfig.KubernetesComponents)
-	if errors.As(err, &upgradeErr) {
+	switch {
+	case err == nil:
+		err := u.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 {
@@ -131,32 +142,41 @@ func (u *Upgrader) UpgradeNodeVersion(ctx context.Context, conf *config.Config)
 	if err := u.updateMeasurements(ctx, conf.GetMeasurements()); err != nil {
 		return fmt.Errorf("updating measurements: %w", err)
 	}
-	updatedNodeVersion, err := u.applyUpgrade(ctx, &components, nodeVersion)
+
+	updatedNodeVersion, err := u.applyNodeVersion(ctx, nodeVersion)
 	if err != nil {
 		return fmt.Errorf("applying upgrade: %w", err)
 	}
-	if updatedNodeVersion.Spec.ImageReference != imageReference ||
+	switch {
-		updatedNodeVersion.Spec.ImageVersion != imageVersion.Version ||
+	case updatedNodeVersion.Spec.ImageReference != imageReference:
-		updatedNodeVersion.Spec.KubernetesComponentsReference != components.ObjectMeta.Name ||
+		return fmt.Errorf("expected NodeVersion to contain %s, got %s", imageReference, updatedNodeVersion.Spec.ImageReference)
-		updatedNodeVersion.Spec.KubernetesClusterVersion != versionConfig.ClusterVersion {
+	case updatedNodeVersion.Spec.ImageVersion != imageVersion.Version:
-		return errors.New("unexpected value in updated nodeVersion object")
+		return fmt.Errorf("expected NodeVersion to contain %s, got %s", imageVersion.Version, updatedNodeVersion.Spec.ImageVersion)
+	case updatedNodeVersion.Spec.KubernetesComponentsReference != nodeVersion.Spec.KubernetesComponentsReference:
+		return fmt.Errorf("expected NodeVersion to contain %s, got %s", nodeVersion.Spec.KubernetesComponentsReference, updatedNodeVersion.Spec.KubernetesComponentsReference)
+	case updatedNodeVersion.Spec.KubernetesClusterVersion != versionConfig.ClusterVersion:
+		return fmt.Errorf("expected NodeVersion to contain %s, got %s", versionConfig.ClusterVersion, updatedNodeVersion.Spec.KubernetesClusterVersion)
 	}
 
 	return errors.Join(upgradeErrs...)
 }
 
-func (u *Upgrader) applyUpgrade(ctx context.Context, components *corev1.ConfigMap, nodeVersion updatev1alpha1.NodeVersion) (updatev1alpha1.NodeVersion, error) {
+// applyComponentsCM applies the k8s components ConfigMap to the cluster.
+func (u *Upgrader) applyComponentsCM(ctx context.Context, components *corev1.ConfigMap) error {
 	_, err := u.stableInterface.createConfigMap(ctx, components)
 	// If the map already exists we can use that map and assume it has the same content as 'configMap'.
 	if err != nil && !k8serrors.IsAlreadyExists(err) {
-		return updatev1alpha1.NodeVersion{}, fmt.Errorf("creating k8s-components ConfigMap: %w. %T", err, err)
+		return fmt.Errorf("creating k8s-components ConfigMap: %w. %T", err, err)
 	}
+	return nil
+}
 
+func (u *Upgrader) applyNodeVersion(ctx context.Context, nodeVersion updatev1alpha1.NodeVersion) (updatev1alpha1.NodeVersion, error) {
 	raw, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&nodeVersion)
 	if err != nil {
 		return updatev1alpha1.NodeVersion{}, fmt.Errorf("converting nodeVersion to unstructured: %w", err)
 	}
-	u.log.Debugf("Triggering Kubernetes version upgrade now")
+	u.log.Debugf("Triggering NodeVersion upgrade now")
 	// Send the updated NodeVersion resource
 	updated, err := u.dynamicInterface.update(ctx, &unstructured.Unstructured{Object: raw})
 	if err != nil {
@@ -199,21 +219,21 @@ func (u *Upgrader) updateImage(nodeVersion *updatev1alpha1.NodeVersion, newImage
 	return nil
 }
 
-func (u *Upgrader) updateK8s(nodeVersion *updatev1alpha1.NodeVersion, newClusterVersion string, components components.Components) (corev1.ConfigMap, error) {
+func (u *Upgrader) updateK8s(nodeVersion *updatev1alpha1.NodeVersion, newClusterVersion string, components components.Components) (*corev1.ConfigMap, error) {
 	configMap, err := internalk8s.ConstructK8sComponentsCM(components, newClusterVersion)
 	if err != nil {
-		return corev1.ConfigMap{}, fmt.Errorf("constructing k8s-components ConfigMap: %w", err)
+		return nil, fmt.Errorf("constructing k8s-components ConfigMap: %w", err)
 	}
 
 	if err := compatibility.IsValidUpgrade(nodeVersion.Spec.KubernetesClusterVersion, newClusterVersion); err != nil {
-		return corev1.ConfigMap{}, err
+		return nil, err
 	}
 
 	u.log.Debugf("Updating local copy of nodeVersion Kubernetes version from %s to %s", nodeVersion.Spec.KubernetesClusterVersion, newClusterVersion)
 	nodeVersion.Spec.KubernetesComponentsReference = configMap.ObjectMeta.Name
 	nodeVersion.Spec.KubernetesClusterVersion = newClusterVersion
 
-	return configMap, nil
+	return &configMap, nil
 }
 
 // KubernetesVersion returns the version of Kubernetes the Constellation is currently running on.

cli/internal/cloudcmd/upgrade_test.go

@@ -52,10 +52,8 @@ func TestUpgradeNodeVersion(t *testing.T) {
 			currentImageVersion:   "v1.2.2",
 			currentClusterVersion: versions.SupportedK8sVersions()[0],
 			stable: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`,
-					},
 				},
 			},
 			wantUpdate: true,
@@ -70,10 +68,8 @@ func TestUpgradeNodeVersion(t *testing.T) {
 			currentImageVersion:   "v1.2.2",
 			currentClusterVersion: versions.SupportedK8sVersions()[0],
 			stable: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`,
-					},
 				},
 			},
 			wantUpdate: true,
@@ -93,10 +89,8 @@ func TestUpgradeNodeVersion(t *testing.T) {
 			currentImageVersion:   "v1.2.2",
 			currentClusterVersion: versions.SupportedK8sVersions()[0],
 			stable: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`,
-					},
 				},
 			},
 			wantUpdate: true,
@@ -211,10 +205,8 @@ func TestUpdateMeasurements(t *testing.T) {
 	}{
 		"success": {
 			updater: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`,
-					},
 				},
 			},
 			newMeasurements: measurements.M{
@@ -224,10 +216,8 @@ func TestUpdateMeasurements(t *testing.T) {
 		},
 		"measurements are the same": {
 			updater: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`,
-					},
 				},
 			},
 			newMeasurements: measurements.M{
@@ -236,10 +226,8 @@ func TestUpdateMeasurements(t *testing.T) {
 		},
 		"trying to set warnOnly to true results in error": {
 			updater: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`,
-					},
 				},
 			},
 			newMeasurements: measurements.M{
@@ -249,10 +237,8 @@ func TestUpdateMeasurements(t *testing.T) {
 		},
 		"setting warnOnly to false is allowed": {
 			updater: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":true}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":true}}`,
-					},
 				},
 			},
 			newMeasurements: measurements.M{
@@ -266,10 +252,8 @@ func TestUpdateMeasurements(t *testing.T) {
 		},
 		"update error": {
 			updater: &stubStableClient{
-				configMap: &corev1.ConfigMap{
+				configMaps: map[string]*corev1.ConfigMap{
-					Data: map[string]string{
+					constants.JoinConfigMap: newJoinConfigMap(`{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`),
-						constants.MeasurementsFilename: `{"0":{"expected":"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA","warnOnly":false}}`,
-					},
 				},
 				updateErr: someErr,
 			},
@@ -297,9 +281,9 @@ func TestUpdateMeasurements(t *testing.T) {
 			if tc.wantUpdate {
 				newMeasurementsJSON, err := json.Marshal(tc.newMeasurements)
 				require.NoError(t, err)
-				assert.JSONEq(string(newMeasurementsJSON), tc.updater.updatedConfigMap.Data[constants.MeasurementsFilename])
+				assert.JSONEq(string(newMeasurementsJSON), tc.updater.updatedConfigMaps[constants.JoinConfigMap].Data[constants.MeasurementsFilename])
 			} else {
-				assert.Nil(tc.updater.updatedConfigMap)
+				assert.Nil(tc.updater.updatedConfigMaps)
 			}
 		})
 	}
@@ -424,6 +408,17 @@ func TestUpdateK8s(t *testing.T) {
 	}
 }
 
+func newJoinConfigMap(data string) *corev1.ConfigMap {
+	return &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: constants.JoinConfigMap,
+		},
+		Data: map[string]string{
+			constants.MeasurementsFilename: data,
+		},
+	}
+}
+
 type stubDynamicClient struct {
 	object        *unstructured.Unstructured
 	updatedObject *unstructured.Unstructured
@@ -441,8 +436,8 @@ func (u *stubDynamicClient) update(_ context.Context, updatedObject *unstructure
 }
 
 type stubStableClient struct {
-	configMap        *corev1.ConfigMap
+	configMaps        map[string]*corev1.ConfigMap
-	updatedConfigMap *corev1.ConfigMap
+	updatedConfigMaps map[string]*corev1.ConfigMap
 	k8sVersion        string
 	getErr            error
 	updateErr         error
@@ -450,18 +445,24 @@ type stubStableClient struct {
 	k8sErr            error
 }
 
-func (s *stubStableClient) getCurrentConfigMap(_ context.Context, _ string) (*corev1.ConfigMap, error) {
+func (s *stubStableClient) getCurrentConfigMap(_ context.Context, name string) (*corev1.ConfigMap, error) {
-	return s.configMap, s.getErr
+	return s.configMaps[name], s.getErr
 }
 
 func (s *stubStableClient) updateConfigMap(_ context.Context, configMap *corev1.ConfigMap) (*corev1.ConfigMap, error) {
-	s.updatedConfigMap = configMap
+	if s.updatedConfigMaps == nil {
-	return s.updatedConfigMap, s.updateErr
+		s.updatedConfigMaps = map[string]*corev1.ConfigMap{}
+	}
+	s.updatedConfigMaps[configMap.ObjectMeta.Name] = configMap
+	return s.updatedConfigMaps[configMap.ObjectMeta.Name], s.updateErr
 }
 
 func (s *stubStableClient) createConfigMap(_ context.Context, configMap *corev1.ConfigMap) (*corev1.ConfigMap, error) {
-	s.configMap = configMap
+	if s.configMaps == nil {
-	return s.configMap, s.createErr
+		s.configMaps = map[string]*corev1.ConfigMap{}
+	}
+	s.configMaps[configMap.ObjectMeta.Name] = configMap
+	return s.configMaps[configMap.ObjectMeta.Name], s.createErr
 }
 
 func (s *stubStableClient) kubernetesVersion() (string, error) {