Add tests, nil checks

Author: benmoss

controlplane/kubeadm/controllers/upgrade.go

@@ -48,7 +48,6 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane(
 	}
 
 	parsedVersion, err := semver.ParseTolerant(kcp.Spec.Version)
-	parsedImageRepository := kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.ImageRepository
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to parse kubernetes version %q", kcp.Spec.Version)
 	}
@@ -65,8 +64,11 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane(
 		return ctrl.Result{}, errors.Wrap(err, "failed to update the kubernetes version in the kubeadm config map")
 	}
 
-	if err := workloadCluster.UpdateImageRepositoryInKubeadmConfigMap(ctx, parsedImageRepository); err != nil {
-		return ctrl.Result{}, errors.Wrap(err, "failed to update the kubernetes version in the kubeadm config map")
+	if kcp.Spec.KubeadmConfigSpec.ClusterConfiguration != nil {
+		imageRepository := kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.ImageRepository
+		if err := workloadCluster.UpdateImageRepositoryInKubeadmConfigMap(ctx, imageRepository); err != nil {
+			return ctrl.Result{}, errors.Wrap(err, "failed to update the image repository in the kubeadm config map")
+		}
 	}
 
 	if kcp.Spec.KubeadmConfigSpec.ClusterConfiguration != nil && kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil {

controlplane/kubeadm/internal/kubeadm_config_map.go

@@ -90,8 +90,11 @@ func (k *kubeadmConfig) UpdateKubernetesVersion(version string) error {
 	return nil
 }
 
-// UpdateImageRepository changes the kubernetes version found in the kubeadm config map
+// UpdateImageRepository changes the image repository found in the kubeadm config map
 func (k *kubeadmConfig) UpdateImageRepository(imageRepository string) error {
+	if imageRepository == "" {
+		return nil
+	}
 	data, ok := k.ConfigMap.Data[clusterConfigurationKey]
 	if !ok {
 		return errors.Errorf("unable to find %q key in kubeadm ConfigMap", clusterConfigurationKey)
@@ -101,7 +104,7 @@ func (k *kubeadmConfig) UpdateImageRepository(imageRepository string) error {
 		return errors.Wrapf(err, "unable to decode kubeadm ConfigMap's %q to Unstructured object", clusterConfigurationKey)
 	}
 	if err := unstructured.SetNestedField(configuration.UnstructuredContent(), imageRepository, configImageRepositoryKey); err != nil {
-		return errors.Wrapf(err, "unable to update %q on kubeadm ConfigMap's %q", configVersionKey, clusterConfigurationKey)
+		return errors.Wrapf(err, "unable to update %q on kubeadm ConfigMap's %q", imageRepository, clusterConfigurationKey)
 	}
 	updated, err := yaml.Marshal(configuration)
 	if err != nil {

controlplane/kubeadm/internal/kubeadm_config_map_test.go

@@ -307,3 +307,67 @@ scheduler: {}`,
 		})
 	}
 }
+
+func TestUpdateImageRepository(t *testing.T) {
+
+	tests := []struct {
+		name                      string
+		clusterConfigurationValue string
+		imageRepository           string
+		expected                  string
+		expectErr                 error
+	}{
+		{
+			name: "it should set the values, if they were empty",
+			clusterConfigurationValue: `
+apiVersion: kubeadm.k8s.io/v1beta2
+kind: ClusterConfiguration
+imageRepository: k8s.gcr.io
+`,
+			imageRepository: "example.com/k8s",
+			expected:        "example.com/k8s",
+		},
+		{
+			name: "it shouldn't write empty strings",
+			clusterConfigurationValue: `
+apiVersion: kubeadm.k8s.io/v1beta2
+kind: ClusterConfiguration
+imageRepository: k8s.gcr.io
+`,
+			imageRepository: "",
+			expected:        "k8s.gcr.io",
+		},
+		{
+			name: "it should error if it's not a valid k8s object",
+			clusterConfigurationValue: `
+imageRepository: "cool"
+`,
+			imageRepository: "example.com/k8s",
+			expectErr:       errors.New("Object 'Kind' is missing"),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			kconfig := &kubeadmConfig{
+				ConfigMap: &corev1.ConfigMap{
+					Data: map[string]string{
+						clusterConfigurationKey: test.clusterConfigurationValue,
+					},
+				},
+			}
+
+			err := kconfig.UpdateImageRepository(test.imageRepository)
+			if test.expectErr == nil {
+				g.Expect(err).ToNot(HaveOccurred())
+			} else {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(test.expectErr.Error()))
+			}
+
+			g.Expect(kconfig.ConfigMap.Data[clusterConfigurationKey]).To(ContainSubstring(test.expected))
+		})
+	}
+}