Merge pull request #2807 from benmoss/update-image-repo

🐛 KCP should update Kubeadm config map when setting `imageRepository`
kubernetes-sigs · Mar 27, 2020 · ecff70a · ecff70a
2 parents 1be9d30 + bedda68
commit ecff70a
Show file tree

Hide file tree

Showing 4 changed files with 122 additions and 8 deletions.
diff --git a/controlplane/kubeadm/controllers/upgrade.go b/controlplane/kubeadm/controllers/upgrade.go
@@ -64,6 +64,13 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane(
 		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 {
 		meta := kcp.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local.ImageMeta
 		if err := workloadCluster.UpdateEtcdVersionInKubeadmConfigMap(ctx, meta.ImageRepository, meta.ImageTag); err != nil {

diff --git a/controlplane/kubeadm/internal/kubeadm_config_map.go b/controlplane/kubeadm/internal/kubeadm_config_map.go
@@ -27,14 +27,15 @@ import (
 )
 
 const (
-	clusterStatusKey        = "ClusterStatus"
-	clusterConfigurationKey = "ClusterConfiguration"
-	statusAPIEndpointsKey   = "apiEndpoints"
-	configVersionKey        = "kubernetesVersion"
-	dnsKey                  = "dns"
-	dnsTypeKey              = "type"
-	dnsImageRepositoryKey   = "imageRepository"
-	dnsImageTagKey          = "imageTag"
+	clusterStatusKey         = "ClusterStatus"
+	clusterConfigurationKey  = "ClusterConfiguration"
+	statusAPIEndpointsKey    = "apiEndpoints"
+	configVersionKey         = "kubernetesVersion"
+	dnsKey                   = "dns"
+	dnsTypeKey               = "type"
+	dnsImageRepositoryKey    = "imageRepository"
+	dnsImageTagKey           = "imageTag"
+	configImageRepositoryKey = "imageRepository"
 )
 
 // kubeadmConfig wraps up interactions necessary for modifying the kubeadm config during an upgrade.
@@ -89,6 +90,30 @@ func (k *kubeadmConfig) UpdateKubernetesVersion(version string) error {
 	return nil
 }
 
+// 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)
+	}
+	configuration, err := yamlToUnstructured([]byte(data))
+	if err != nil {
+		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", imageRepository, clusterConfigurationKey)
+	}
+	updated, err := yaml.Marshal(configuration)
+	if err != nil {
+		return errors.Wrapf(err, "unable to encode kubeadm ConfigMap's %q to YAML", clusterConfigurationKey)
+	}
+	k.ConfigMap.Data[clusterConfigurationKey] = string(updated)
+	return nil
+}
+
 // UpdateEtcdMeta sets the local etcd's configuration's image repository and image tag
 func (k *kubeadmConfig) UpdateEtcdMeta(imageRepository, imageTag string) (bool, error) {
 	data, ok := k.ConfigMap.Data[clusterConfigurationKey]

diff --git a/controlplane/kubeadm/internal/kubeadm_config_map_test.go b/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))
+		})
+	}
+}
diff --git a/controlplane/kubeadm/internal/workload_cluster.go b/controlplane/kubeadm/internal/workload_cluster.go
@@ -68,6 +68,7 @@ type WorkloadCluster interface {
 	ReconcileKubeletRBACBinding(ctx context.Context, version semver.Version) error
 	ReconcileKubeletRBACRole(ctx context.Context, version semver.Version) error
 	UpdateKubernetesVersionInKubeadmConfigMap(ctx context.Context, version semver.Version) error
+	UpdateImageRepositoryInKubeadmConfigMap(ctx context.Context, imageRepository string) error
 	UpdateEtcdVersionInKubeadmConfigMap(ctx context.Context, imageRepository, imageTag string) error
 	UpdateKubeletConfigMap(ctx context.Context, version semver.Version) error
 	UpdateKubeProxyImageInfo(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane) error
@@ -308,6 +309,23 @@ func (w *Workload) UpdateEtcdVersionInKubeadmConfigMap(ctx context.Context, imag
 	return nil
 }
 
+// UpdateKubernetesVersionInKubeadmConfigMap updates the kubernetes version in the kubeadm config map.
+func (w *Workload) UpdateImageRepositoryInKubeadmConfigMap(ctx context.Context, imageRepository string) error {
+	configMapKey := ctrlclient.ObjectKey{Name: "kubeadm-config", Namespace: metav1.NamespaceSystem}
+	kubeadmConfigMap, err := w.getConfigMap(ctx, configMapKey)
+	if err != nil {
+		return err
+	}
+	config := &kubeadmConfig{ConfigMap: kubeadmConfigMap}
+	if err := config.UpdateImageRepository(imageRepository); err != nil {
+		return err
+	}
+	if err := w.Client.Update(ctx, config.ConfigMap); err != nil {
+		return errors.Wrap(err, "error updating kubeadm ConfigMap")
+	}
+	return nil
+}
+
 // UpdateKubernetesVersionInKubeadmConfigMap updates the kubernetes version in the kubeadm config map.
 func (w *Workload) UpdateKubernetesVersionInKubeadmConfigMap(ctx context.Context, version semver.Version) error {
 	configMapKey := ctrlclient.ObjectKey{Name: "kubeadm-config", Namespace: metav1.NamespaceSystem}