From 885fd17c189554186cb1464c5162ca25ac84c890 Mon Sep 17 00:00:00 2001 From: Aditya Bhatia <7274741+adityabhatia@users.noreply.github.com> Date: Tue, 20 Feb 2024 00:06:33 +0100 Subject: [PATCH] enable kubeadm feature flags mutation --- .../internal/controllers/controller_test.go | 3 +- .../internal/controllers/fakes_test.go | 10 ++- .../kubeadm/internal/controllers/upgrade.go | 50 +++++------- .../webhooks/kubeadm_control_plane.go | 3 + .../webhooks/kubeadm_control_plane_test.go | 4 +- .../kubeadm/internal/workload_cluster.go | 66 +++++++++------- .../internal/workload_cluster_coredns.go | 8 +- .../internal/workload_cluster_coredns_test.go | 2 +- .../kubeadm/internal/workload_cluster_etcd.go | 23 +++--- .../internal/workload_cluster_etcd_test.go | 75 ++++++++++++------ .../kubeadm/internal/workload_cluster_test.go | 78 +++++++++++++++++-- 11 files changed, 213 insertions(+), 109 deletions(-) diff --git a/controlplane/kubeadm/internal/controllers/controller_test.go b/controlplane/kubeadm/internal/controllers/controller_test.go index aaf7c09a180b..6459174a918a 100644 --- a/controlplane/kubeadm/internal/controllers/controller_test.go +++ b/controlplane/kubeadm/internal/controllers/controller_test.go @@ -1279,7 +1279,8 @@ dns: type: CoreDNS imageRepository: registry.k8s.io kind: ClusterConfiguration -kubernetesVersion: metav1.16.1`, +kubernetesVersion: metav1.16.1 +`, }, } g.Expect(env.Create(ctx, kubeadmCM)).To(Succeed()) diff --git a/controlplane/kubeadm/internal/controllers/fakes_test.go b/controlplane/kubeadm/internal/controllers/fakes_test.go index 3c7348bc4831..cf9fcbafe66e 100644 --- a/controlplane/kubeadm/internal/controllers/fakes_test.go +++ b/controlplane/kubeadm/internal/controllers/fakes_test.go @@ -108,11 +108,11 @@ func (f fakeWorkloadCluster) ReconcileKubeletRBACBinding(_ context.Context, _ se return nil } -func (f fakeWorkloadCluster) UpdateKubernetesVersionInKubeadmConfigMap(_ context.Context, _ semver.Version) error { +func (f fakeWorkloadCluster) UpdateKubernetesVersionInKubeadmConfigMap(semver.Version) func(*bootstrapv1.ClusterConfiguration) { return nil } -func (f fakeWorkloadCluster) UpdateEtcdVersionInKubeadmConfigMap(_ context.Context, _, _ string, _ semver.Version) error { +func (f fakeWorkloadCluster) UpdateEtcdLocalInKubeadmConfigMap(*bootstrapv1.LocalEtcd) func(*bootstrapv1.ClusterConfiguration) { return nil } @@ -132,13 +132,17 @@ func (f fakeWorkloadCluster) EtcdMembers(_ context.Context) ([]string, error) { return f.EtcdMembersResult, nil } +func (f fakeWorkloadCluster) UpdateClusterConfiguration(context.Context, semver.Version, ...func(*bootstrapv1.ClusterConfiguration)) error { + return nil +} + type fakeMigrator struct { migrateCalled bool migrateErr error migratedCorefile string } -func (m *fakeMigrator) Migrate(_, _, _ string, _ bool) (string, error) { +func (m *fakeMigrator) Migrate(string, string, string, bool) (string, error) { m.migrateCalled = true if m.migrateErr != nil { return "", m.migrateErr diff --git a/controlplane/kubeadm/internal/controllers/upgrade.go b/controlplane/kubeadm/internal/controllers/upgrade.go index 6abf136947ab..651d0c2a798c 100644 --- a/controlplane/kubeadm/internal/controllers/upgrade.go +++ b/controlplane/kubeadm/internal/controllers/upgrade.go @@ -23,6 +23,7 @@ import ( "github.com/pkg/errors" ctrl "sigs.k8s.io/controller-runtime" + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" "sigs.k8s.io/cluster-api/util" @@ -73,9 +74,8 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane( return ctrl.Result{}, errors.Wrap(err, "failed to set cluster-admin ClusterRoleBinding for kubeadm") } - if err := workloadCluster.UpdateKubernetesVersionInKubeadmConfigMap(ctx, parsedVersion); err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to update the kubernetes version in the kubeadm config map") - } + kubeadmCMMutators := make([]func(*bootstrapv1.ClusterConfiguration), 0) + kubeadmCMMutators = append(kubeadmCMMutators, workloadCluster.UpdateKubernetesVersionInKubeadmConfigMap(parsedVersion)) if controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration != nil { // We intentionally only parse major/minor/patch so that the subsequent code @@ -84,38 +84,30 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane( if err != nil { return ctrl.Result{}, errors.Wrapf(err, "failed to parse kubernetes version %q", controlPlane.KCP.Spec.Version) } + // Get the imageRepository or the correct value if nothing is set and a migration is necessary. imageRepository := internal.ImageRepositoryFromClusterConfig(controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration, parsedVersionTolerant) - if err := workloadCluster.UpdateImageRepositoryInKubeadmConfigMap(ctx, imageRepository, parsedVersion); err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to update the image repository in the kubeadm config map") - } - } - - if controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration != nil && controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil { - meta := controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local.ImageMeta - if err := workloadCluster.UpdateEtcdVersionInKubeadmConfigMap(ctx, meta.ImageRepository, meta.ImageTag, parsedVersion); err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to update the etcd version in the kubeadm config map") - } - - extraArgs := controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local.ExtraArgs - if err := workloadCluster.UpdateEtcdExtraArgsInKubeadmConfigMap(ctx, extraArgs, parsedVersion); err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to update the etcd extra args in the kubeadm config map") + kubeadmCMMutators = append(kubeadmCMMutators, + workloadCluster.UpdateImageRepositoryInKubeadmConfigMap(imageRepository), + workloadCluster.UpdateFeatureGatesInKubeadmConfigMap(controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.FeatureGates), + workloadCluster.UpdateAPIServerInKubeadmConfigMap(controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer), + workloadCluster.UpdateControllerManagerInKubeadmConfigMap(controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.ControllerManager), + workloadCluster.UpdateSchedulerInKubeadmConfigMap(controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Scheduler)) + + // Etcd local and external are mutually exclusive and they cannot be switched, once set. + if controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local != nil { + kubeadmCMMutators = append(kubeadmCMMutators, + workloadCluster.UpdateEtcdLocalInKubeadmConfigMap(controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.Local)) + } else { + kubeadmCMMutators = append(kubeadmCMMutators, + workloadCluster.UpdateEtcdExternalInKubeadmConfigMap(controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Etcd.External)) } } - if controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration != nil { - if err := workloadCluster.UpdateAPIServerInKubeadmConfigMap(ctx, controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.APIServer, parsedVersion); err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to update api server in the kubeadm config map") - } - - if err := workloadCluster.UpdateControllerManagerInKubeadmConfigMap(ctx, controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.ControllerManager, parsedVersion); err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to update controller manager in the kubeadm config map") - } - - if err := workloadCluster.UpdateSchedulerInKubeadmConfigMap(ctx, controlPlane.KCP.Spec.KubeadmConfigSpec.ClusterConfiguration.Scheduler, parsedVersion); err != nil { - return ctrl.Result{}, errors.Wrap(err, "failed to update scheduler in the kubeadm config map") - } + // collectively update Kubeadm config map + if err = workloadCluster.UpdateClusterConfiguration(ctx, parsedVersion, kubeadmCMMutators...); err != nil { + return ctrl.Result{}, err } if err := workloadCluster.UpdateKubeletConfigMap(ctx, parsedVersion); err != nil { diff --git a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go index 4338b8b11b15..d848d4616bba 100644 --- a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go +++ b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane.go @@ -150,6 +150,7 @@ const ( ntp = "ntp" ignition = "ignition" diskSetup = "diskSetup" + featureGates = "featureGates" ) const minimumCertificatesExpiryDays = 7 @@ -176,6 +177,8 @@ func (webhook *KubeadmControlPlane) ValidateUpdate(_ context.Context, oldObj, ne {spec, kubeadmConfigSpec, clusterConfiguration, "dns", "imageRepository"}, {spec, kubeadmConfigSpec, clusterConfiguration, "dns", "imageTag"}, {spec, kubeadmConfigSpec, clusterConfiguration, "imageRepository"}, + {spec, kubeadmConfigSpec, clusterConfiguration, featureGates}, + {spec, kubeadmConfigSpec, clusterConfiguration, featureGates, "*"}, {spec, kubeadmConfigSpec, clusterConfiguration, apiServer}, {spec, kubeadmConfigSpec, clusterConfiguration, apiServer, "*"}, {spec, kubeadmConfigSpec, clusterConfiguration, controllerManager}, diff --git a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane_test.go b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane_test.go index 699d3776d0e1..9c62b22ad4e6 100644 --- a/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane_test.go +++ b/controlplane/kubeadm/internal/webhooks/kubeadm_control_plane_test.go @@ -886,8 +886,8 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { kcp: imageRepository, }, { - name: "should fail when making a change to the cluster config's featureGates", - expectErr: true, + name: "should succeed when making a change to the cluster config's featureGates", + expectErr: false, before: before, kcp: featureGates, }, diff --git a/controlplane/kubeadm/internal/workload_cluster.go b/controlplane/kubeadm/internal/workload_cluster.go index d4c41eb89f4c..9034dd1e05e8 100644 --- a/controlplane/kubeadm/internal/workload_cluster.go +++ b/controlplane/kubeadm/internal/workload_cluster.go @@ -105,13 +105,14 @@ type WorkloadCluster interface { // Upgrade related tasks. 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, version semver.Version) error - UpdateEtcdVersionInKubeadmConfigMap(ctx context.Context, imageRepository, imageTag string, version semver.Version) error - UpdateEtcdExtraArgsInKubeadmConfigMap(ctx context.Context, extraArgs map[string]string, version semver.Version) error - UpdateAPIServerInKubeadmConfigMap(ctx context.Context, apiServer bootstrapv1.APIServer, version semver.Version) error - UpdateControllerManagerInKubeadmConfigMap(ctx context.Context, controllerManager bootstrapv1.ControlPlaneComponent, version semver.Version) error - UpdateSchedulerInKubeadmConfigMap(ctx context.Context, scheduler bootstrapv1.ControlPlaneComponent, version semver.Version) error + UpdateKubernetesVersionInKubeadmConfigMap(version semver.Version) func(*bootstrapv1.ClusterConfiguration) + UpdateImageRepositoryInKubeadmConfigMap(imageRepository string) func(*bootstrapv1.ClusterConfiguration) + UpdateFeatureGatesInKubeadmConfigMap(featureGates map[string]bool) func(*bootstrapv1.ClusterConfiguration) + UpdateEtcdLocalInKubeadmConfigMap(localEtcd *bootstrapv1.LocalEtcd) func(*bootstrapv1.ClusterConfiguration) + UpdateEtcdExternalInKubeadmConfigMap(externalEtcd *bootstrapv1.ExternalEtcd) func(*bootstrapv1.ClusterConfiguration) + UpdateAPIServerInKubeadmConfigMap(apiServer bootstrapv1.APIServer) func(*bootstrapv1.ClusterConfiguration) + UpdateControllerManagerInKubeadmConfigMap(controllerManager bootstrapv1.ControlPlaneComponent) func(*bootstrapv1.ClusterConfiguration) + UpdateSchedulerInKubeadmConfigMap(scheduler bootstrapv1.ControlPlaneComponent) func(*bootstrapv1.ClusterConfiguration) UpdateKubeletConfigMap(ctx context.Context, version semver.Version) error UpdateKubeProxyImageInfo(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, version semver.Version) error UpdateCoreDNS(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, version semver.Version) error @@ -121,6 +122,7 @@ type WorkloadCluster interface { ForwardEtcdLeadership(ctx context.Context, machine *clusterv1.Machine, leaderCandidate *clusterv1.Machine) error AllowBootstrapTokensToGetNodes(ctx context.Context) error AllowClusterAdminPermissions(ctx context.Context, version semver.Version) error + UpdateClusterConfiguration(ctx context.Context, version semver.Version, mutators ...func(*bootstrapv1.ClusterConfiguration)) error // State recovery tasks. ReconcileEtcdMembers(ctx context.Context, nodeNames []string, version semver.Version) ([]string, error) @@ -173,20 +175,30 @@ func (w *Workload) getConfigMap(ctx context.Context, configMap ctrlclient.Object } // UpdateImageRepositoryInKubeadmConfigMap updates the image repository in the kubeadm config map. -func (w *Workload) UpdateImageRepositoryInKubeadmConfigMap(ctx context.Context, imageRepository string, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { +func (w *Workload) UpdateImageRepositoryInKubeadmConfigMap(imageRepository string) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { if imageRepository == "" { return } + c.ImageRepository = imageRepository - }, version) + } +} + +// UpdateFeatureGatesInKubeadmConfigMap updates the feature gates in the kubeadm config map. +func (w *Workload) UpdateFeatureGatesInKubeadmConfigMap(featureGates map[string]bool) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { + // Even if featureGates is nil, reset it to ClusterConfiguration + // to override any previously set feature gates. + c.FeatureGates = featureGates + } } // UpdateKubernetesVersionInKubeadmConfigMap updates the kubernetes version in the kubeadm config map. -func (w *Workload) UpdateKubernetesVersionInKubeadmConfigMap(ctx context.Context, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { +func (w *Workload) UpdateKubernetesVersionInKubeadmConfigMap(version semver.Version) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { c.KubernetesVersion = fmt.Sprintf("v%s", version.String()) - }, version) + } } // UpdateKubeletConfigMap will create a new kubelet-config-1.x config map for a new version of the kubelet. @@ -270,24 +282,24 @@ func (w *Workload) UpdateKubeletConfigMap(ctx context.Context, version semver.Ve } // UpdateAPIServerInKubeadmConfigMap updates api server configuration in kubeadm config map. -func (w *Workload) UpdateAPIServerInKubeadmConfigMap(ctx context.Context, apiServer bootstrapv1.APIServer, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { +func (w *Workload) UpdateAPIServerInKubeadmConfigMap(apiServer bootstrapv1.APIServer) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { c.APIServer = apiServer - }, version) + } } // UpdateControllerManagerInKubeadmConfigMap updates controller manager configuration in kubeadm config map. -func (w *Workload) UpdateControllerManagerInKubeadmConfigMap(ctx context.Context, controllerManager bootstrapv1.ControlPlaneComponent, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { +func (w *Workload) UpdateControllerManagerInKubeadmConfigMap(controllerManager bootstrapv1.ControlPlaneComponent) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { c.ControllerManager = controllerManager - }, version) + } } // UpdateSchedulerInKubeadmConfigMap updates scheduler configuration in kubeadm config map. -func (w *Workload) UpdateSchedulerInKubeadmConfigMap(ctx context.Context, scheduler bootstrapv1.ControlPlaneComponent, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { +func (w *Workload) UpdateSchedulerInKubeadmConfigMap(scheduler bootstrapv1.ControlPlaneComponent) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { c.Scheduler = scheduler - }, version) + } } // RemoveMachineFromKubeadmConfigMap removes the entry for the machine from the kubeadm configmap. @@ -350,11 +362,11 @@ func (w *Workload) updateClusterStatus(ctx context.Context, mutator func(status }) } -// updateClusterConfiguration gets the ClusterConfiguration kubeadm-config ConfigMap, converts it to the +// UpdateClusterConfiguration gets the ClusterConfiguration kubeadm-config ConfigMap, converts it to the // Cluster API representation, and then applies a mutation func; if changes are detected, the // data are converted back into the Kubeadm API version in use for the target Kubernetes version and the // kubeadm-config ConfigMap updated. -func (w *Workload) updateClusterConfiguration(ctx context.Context, mutator func(*bootstrapv1.ClusterConfiguration), version semver.Version) error { +func (w *Workload) UpdateClusterConfiguration(ctx context.Context, version semver.Version, mutators ...func(*bootstrapv1.ClusterConfiguration)) error { return retry.RetryOnConflict(retry.DefaultBackoff, func() error { key := ctrlclient.ObjectKey{Name: kubeadmConfigKey, Namespace: metav1.NamespaceSystem} configMap, err := w.getConfigMap(ctx, key) @@ -373,7 +385,9 @@ func (w *Workload) updateClusterConfiguration(ctx context.Context, mutator func( } updatedObj := currentObj.DeepCopy() - mutator(updatedObj) + for i := range mutators { + mutators[i](updatedObj) + } if !reflect.DeepEqual(currentObj, updatedObj) { updatedData, err := kubeadmtypes.MarshalClusterConfigurationForVersion(updatedObj, version) @@ -382,7 +396,7 @@ func (w *Workload) updateClusterConfiguration(ctx context.Context, mutator func( } configMap.Data[clusterConfigurationKey] = updatedData if err := w.Client.Update(ctx, configMap); err != nil { - return errors.Wrap(err, "failed to upgrade the kubeadmConfigMap") + return errors.Wrap(err, "failed to upgrade cluster configuration in the kubeadmConfigMap") } } return nil diff --git a/controlplane/kubeadm/internal/workload_cluster_coredns.go b/controlplane/kubeadm/internal/workload_cluster_coredns.go index 5699c9c06656..deb5d712d708 100644 --- a/controlplane/kubeadm/internal/workload_cluster_coredns.go +++ b/controlplane/kubeadm/internal/workload_cluster_coredns.go @@ -145,7 +145,7 @@ func (w *Workload) UpdateCoreDNS(ctx context.Context, kcp *controlplanev1.Kubead } // Perform the upgrade. - if err := w.updateCoreDNSImageInfoInKubeadmConfigMap(ctx, &clusterConfig.DNS, version); err != nil { + if err := w.UpdateClusterConfiguration(ctx, version, w.updateCoreDNSImageInfoInKubeadmConfigMap(&clusterConfig.DNS)); err != nil { return err } if err := w.updateCoreDNSCorefile(ctx, info); err != nil { @@ -270,11 +270,11 @@ func (w *Workload) updateCoreDNSDeployment(ctx context.Context, info *coreDNSInf } // updateCoreDNSImageInfoInKubeadmConfigMap updates the kubernetes version in the kubeadm config map. -func (w *Workload) updateCoreDNSImageInfoInKubeadmConfigMap(ctx context.Context, dns *bootstrapv1.DNS, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { +func (w *Workload) updateCoreDNSImageInfoInKubeadmConfigMap(dns *bootstrapv1.DNS) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { c.DNS.ImageRepository = dns.ImageRepository c.DNS.ImageTag = dns.ImageTag - }, version) + } } // updateCoreDNSClusterRole updates the CoreDNS ClusterRole when necessary. diff --git a/controlplane/kubeadm/internal/workload_cluster_coredns_test.go b/controlplane/kubeadm/internal/workload_cluster_coredns_test.go index 12bf01c42863..a9f1f2a8c369 100644 --- a/controlplane/kubeadm/internal/workload_cluster_coredns_test.go +++ b/controlplane/kubeadm/internal/workload_cluster_coredns_test.go @@ -1451,7 +1451,7 @@ func TestUpdateCoreDNSImageInfoInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.updateCoreDNSImageInfoInKubeadmConfigMap(ctx, &tt.newDNS, semver.MustParse("1.19.1")) + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.updateCoreDNSImageInfoInKubeadmConfigMap(&tt.newDNS)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap diff --git a/controlplane/kubeadm/internal/workload_cluster_etcd.go b/controlplane/kubeadm/internal/workload_cluster_etcd.go index bb4c4d417960..48c06bc3f567 100644 --- a/controlplane/kubeadm/internal/workload_cluster_etcd.go +++ b/controlplane/kubeadm/internal/workload_cluster_etcd.go @@ -92,23 +92,22 @@ loopmembers: return removedMembers, errs } -// UpdateEtcdVersionInKubeadmConfigMap sets the imageRepository or the imageTag or both in the kubeadm config map. -func (w *Workload) UpdateEtcdVersionInKubeadmConfigMap(ctx context.Context, imageRepository, imageTag string, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { +// UpdateEtcdLocalInKubeadmConfigMap sets etcd local configuration in the kubeadm config map. +func (w *Workload) UpdateEtcdLocalInKubeadmConfigMap(etcdLocal *bootstrapv1.LocalEtcd) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { if c.Etcd.Local != nil { - c.Etcd.Local.ImageRepository = imageRepository - c.Etcd.Local.ImageTag = imageTag + c.Etcd.Local = etcdLocal } - }, version) + } } -// UpdateEtcdExtraArgsInKubeadmConfigMap sets extraArgs in the kubeadm config map. -func (w *Workload) UpdateEtcdExtraArgsInKubeadmConfigMap(ctx context.Context, extraArgs map[string]string, version semver.Version) error { - return w.updateClusterConfiguration(ctx, func(c *bootstrapv1.ClusterConfiguration) { - if c.Etcd.Local != nil { - c.Etcd.Local.ExtraArgs = extraArgs +// UpdateEtcdExternalInKubeadmConfigMap sets etcd external configuration in the kubeadm config map. +func (w *Workload) UpdateEtcdExternalInKubeadmConfigMap(etcdExternal *bootstrapv1.ExternalEtcd) func(*bootstrapv1.ClusterConfiguration) { + return func(c *bootstrapv1.ClusterConfiguration) { + if c.Etcd.External != nil { + c.Etcd.External = etcdExternal } - }, version) + } } // RemoveEtcdMemberForMachine removes the etcd member from the target cluster's etcd cluster. diff --git a/controlplane/kubeadm/internal/workload_cluster_etcd_test.go b/controlplane/kubeadm/internal/workload_cluster_etcd_test.go index e80e869a51ba..1b72ac3f5cb2 100644 --- a/controlplane/kubeadm/internal/workload_cluster_etcd_test.go +++ b/controlplane/kubeadm/internal/workload_cluster_etcd_test.go @@ -32,58 +32,69 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd" fake2 "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd/fake" "sigs.k8s.io/cluster-api/util/yaml" ) -func TestUpdateEtcdVersionInKubeadmConfigMap(t *testing.T) { +func TestUpdateEtcdExternalInKubeadmConfigMap(t *testing.T) { tests := []struct { name string clusterConfigurationData string - newImageRepository string - newImageTag string + externalEtcd *bootstrapv1.ExternalEtcd wantClusterConfiguration string }{ { - name: "it should set etcd version when local etcd", + name: "it should set external etcd configuration with external etcd", clusterConfigurationData: yaml.Raw(` apiVersion: kubeadm.k8s.io/v1beta2 kind: ClusterConfiguration etcd: - local: {} + external: {} `), - newImageRepository: "example.com/k8s", - newImageTag: "v1.6.0", + externalEtcd: &bootstrapv1.ExternalEtcd{ + Endpoints: []string{"1.2.3.4"}, + CAFile: "/tmp/ca_file.pem", + CertFile: "/tmp/cert_file.crt", + KeyFile: "/tmp/key_file.key", + }, wantClusterConfiguration: yaml.Raw(` apiServer: {} apiVersion: kubeadm.k8s.io/v1beta2 controllerManager: {} dns: {} etcd: - local: - imageRepository: example.com/k8s - imageTag: v1.6.0 + external: + caFile: /tmp/ca_file.pem + certFile: /tmp/cert_file.crt + endpoints: + - 1.2.3.4 + keyFile: /tmp/key_file.key kind: ClusterConfiguration networking: {} scheduler: {} `), }, { - name: "no op when external etcd", + name: "no op when local etcd configuration already exists", clusterConfigurationData: yaml.Raw(` apiVersion: kubeadm.k8s.io/v1beta2 kind: ClusterConfiguration etcd: - external: {} + local: {} `), - newImageRepository: "example.com/k8s", - newImageTag: "v1.6.0", + externalEtcd: &bootstrapv1.ExternalEtcd{ + Endpoints: []string{"1.2.3.4"}, + CAFile: "/tmp/ca_file.pem", + CertFile: "/tmp/cert_file.crt", + KeyFile: "/tmp/key_file.key", + }, wantClusterConfiguration: yaml.Raw(` apiVersion: kubeadm.k8s.io/v1beta2 kind: ClusterConfiguration etcd: - external: {} + local: {} `), }, } @@ -104,7 +115,7 @@ func TestUpdateEtcdVersionInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.UpdateEtcdVersionInKubeadmConfigMap(ctx, tt.newImageRepository, tt.newImageTag, semver.MustParse("1.19.1")) + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.UpdateEtcdExternalInKubeadmConfigMap(tt.externalEtcd)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap @@ -118,23 +129,29 @@ func TestUpdateEtcdVersionInKubeadmConfigMap(t *testing.T) { } } -func TestUpdateEtcdExtraArgsInKubeadmConfigMap(t *testing.T) { +func TestUpdateEtcdLocalInKubeadmConfigMap(t *testing.T) { tests := []struct { name string clusterConfigurationData string - newExtraArgs map[string]string + localEtcd *bootstrapv1.LocalEtcd wantClusterConfiguration string }{ { - name: "it should set etcd extraArgs when local etcd", + name: "it should set local etcd configuration with local etcd", clusterConfigurationData: yaml.Raw(` apiVersion: kubeadm.k8s.io/v1beta2 kind: ClusterConfiguration etcd: local: {} `), - newExtraArgs: map[string]string{ - "foo": "bar", + localEtcd: &bootstrapv1.LocalEtcd{ + ImageMeta: bootstrapv1.ImageMeta{ + ImageRepository: "example.com/k8s", + ImageTag: "v1.6.0", + }, + ExtraArgs: map[string]string{ + "foo": "bar", + }, }, wantClusterConfiguration: yaml.Raw(` apiServer: {} @@ -145,21 +162,29 @@ func TestUpdateEtcdExtraArgsInKubeadmConfigMap(t *testing.T) { local: extraArgs: foo: bar + imageRepository: example.com/k8s + imageTag: v1.6.0 kind: ClusterConfiguration networking: {} scheduler: {} `), }, { - name: "no op when external etcd", + name: "no op when external etcd configuration already exists", clusterConfigurationData: yaml.Raw(` apiVersion: kubeadm.k8s.io/v1beta2 kind: ClusterConfiguration etcd: external: {} `), - newExtraArgs: map[string]string{ - "foo": "bar", + localEtcd: &bootstrapv1.LocalEtcd{ + ImageMeta: bootstrapv1.ImageMeta{ + ImageRepository: "example.com/k8s", + ImageTag: "v1.6.0", + }, + ExtraArgs: map[string]string{ + "foo": "bar", + }, }, wantClusterConfiguration: yaml.Raw(` apiVersion: kubeadm.k8s.io/v1beta2 @@ -186,7 +211,7 @@ func TestUpdateEtcdExtraArgsInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.UpdateEtcdExtraArgsInKubeadmConfigMap(ctx, tt.newExtraArgs, semver.MustParse("1.19.1")) + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.UpdateEtcdLocalInKubeadmConfigMap(tt.localEtcd)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap diff --git a/controlplane/kubeadm/internal/workload_cluster_test.go b/controlplane/kubeadm/internal/workload_cluster_test.go index bb15b6d99da2..242d661008c4 100644 --- a/controlplane/kubeadm/internal/workload_cluster_test.go +++ b/controlplane/kubeadm/internal/workload_cluster_test.go @@ -30,6 +30,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + yaml2 "sigs.k8s.io/yaml" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1beta1" @@ -686,7 +687,7 @@ func TestUpdateUpdateClusterConfigurationInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.updateClusterConfiguration(ctx, tt.mutator, tt.version) + err := w.UpdateClusterConfiguration(ctx, tt.version, tt.mutator) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return @@ -882,7 +883,8 @@ func TestUpdateKubernetesVersionInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.UpdateKubernetesVersionInKubeadmConfigMap(ctx, tt.version) + + err := w.UpdateClusterConfiguration(ctx, tt.version, w.UpdateKubernetesVersionInKubeadmConfigMap(tt.version)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap @@ -938,7 +940,7 @@ func TestUpdateImageRepositoryInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.UpdateImageRepositoryInKubeadmConfigMap(ctx, tt.newImageRepository, semver.MustParse("1.19.1")) + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.UpdateImageRepositoryInKubeadmConfigMap(tt.newImageRepository)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap @@ -1016,7 +1018,7 @@ func TestUpdateApiServerInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.UpdateAPIServerInKubeadmConfigMap(ctx, tt.newAPIServer, semver.MustParse("1.19.1")) + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.UpdateAPIServerInKubeadmConfigMap(tt.newAPIServer)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap @@ -1092,7 +1094,7 @@ func TestUpdateControllerManagerInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.UpdateControllerManagerInKubeadmConfigMap(ctx, tt.newControllerManager, semver.MustParse("1.19.1")) + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.UpdateControllerManagerInKubeadmConfigMap(tt.newControllerManager)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap @@ -1167,7 +1169,7 @@ func TestUpdateSchedulerInKubeadmConfigMap(t *testing.T) { w := &Workload{ Client: fakeClient, } - err := w.UpdateSchedulerInKubeadmConfigMap(ctx, tt.newScheduler, semver.MustParse("1.19.1")) + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.UpdateSchedulerInKubeadmConfigMap(tt.newScheduler)) g.Expect(err).ToNot(HaveOccurred()) var actualConfig corev1.ConfigMap @@ -1260,6 +1262,70 @@ func TestClusterStatus(t *testing.T) { } } +func TestUpdateFeatureGatesInKubeadmConfigMap(t *testing.T) { + tests := []struct { + name string + clusterConfigurationData string + newFeatureGates map[string]bool + wantFeatureGates map[string]bool + }{ + { + name: "it updates feature gates", + clusterConfigurationData: yaml.Raw(` + apiVersion: kubeadm.k8s.io/v1beta2 + kind: ClusterConfiguration`), + newFeatureGates: map[string]bool{"EtcdLearnerMode": true}, + wantFeatureGates: map[string]bool{"EtcdLearnerMode": true}, + }, + { + name: "it should override feature gates even if new value is nil", + clusterConfigurationData: yaml.Raw(` + apiVersion: kubeadm.k8s.io/v1beta2 + kind: ClusterConfiguration + featureGates: + EtcdLearnerMode: true + `), + newFeatureGates: nil, + wantFeatureGates: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithObjects(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: kubeadmConfigKey, + Namespace: metav1.NamespaceSystem, + }, + Data: map[string]string{ + clusterConfigurationKey: tt.clusterConfigurationData, + }, + }).Build() + + w := &Workload{ + Client: fakeClient, + } + err := w.UpdateClusterConfiguration(ctx, semver.MustParse("1.19.1"), w.UpdateFeatureGatesInKubeadmConfigMap(tt.newFeatureGates)) + g.Expect(err).ToNot(HaveOccurred()) + + var actualConfig corev1.ConfigMap + g.Expect(w.Client.Get( + ctx, + client.ObjectKey{Name: kubeadmConfigKey, Namespace: metav1.NamespaceSystem}, + &actualConfig, + )).To(Succeed()) + + actualConfiguration := bootstrapv1.ClusterConfiguration{} + err = yaml2.Unmarshal([]byte(actualConfig.Data[clusterConfigurationKey]), &actualConfiguration) + if err != nil { + return + } + g.Expect(actualConfiguration.FeatureGates).Should(Equal(tt.wantFeatureGates)) + }) + } +} + func getProxyImageInfo(ctx context.Context, c client.Client) (string, error) { ds := &appsv1.DaemonSet{}