Skip to content

Commit

Permalink
Merge pull request #10154 from adityabhatia/kcpFeatureGatesMutation
Browse files Browse the repository at this point in the history
✨ enable kubeadm feature gates mutation
  • Loading branch information
k8s-ci-robot authored Feb 22, 2024
2 parents a045507 + d86ba52 commit 065c072
Show file tree
Hide file tree
Showing 11 changed files with 260 additions and 156 deletions.
3 changes: 2 additions & 1 deletion controlplane/kubeadm/internal/controllers/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
10 changes: 7 additions & 3 deletions controlplane/kubeadm/internal/controllers/fakes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
Expand Down
50 changes: 21 additions & 29 deletions controlplane/kubeadm/internal/controllers/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ const (
ntp = "ntp"
ignition = "ignition"
diskSetup = "diskSetup"
featureGates = "featureGates"
)

const minimumCertificatesExpiryDays = 7
Expand All @@ -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},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
66 changes: 40 additions & 26 deletions controlplane/kubeadm/internal/workload_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions controlplane/kubeadm/internal/workload_cluster_coredns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions controlplane/kubeadm/internal/workload_cluster_coredns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (

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/util/yaml"
utilyaml "sigs.k8s.io/cluster-api/util/yaml"
)

func TestUpdateCoreDNS(t *testing.T) {
Expand Down Expand Up @@ -124,7 +124,7 @@ func TestUpdateCoreDNS(t *testing.T) {
Namespace: metav1.NamespaceSystem,
},
Data: map[string]string{
"ClusterConfiguration": yaml.Raw(`
"ClusterConfiguration": utilyaml.Raw(`
apiServer:
apiVersion: kubeadm.k8s.io/v1beta2
dns:
Expand All @@ -140,7 +140,7 @@ func TestUpdateCoreDNS(t *testing.T) {
Namespace: metav1.NamespaceSystem,
},
Data: map[string]string{
"ClusterConfiguration": yaml.Raw(`
"ClusterConfiguration": utilyaml.Raw(`
apiServer:
apiVersion: kubeadm.k8s.io/v1beta2
dns:
Expand Down Expand Up @@ -1410,7 +1410,7 @@ func TestUpdateCoreDNSImageInfoInKubeadmConfigMap(t *testing.T) {
}{
{
name: "it should set the DNS image config",
clusterConfigurationData: yaml.Raw(`
clusterConfigurationData: utilyaml.Raw(`
apiVersion: kubeadm.k8s.io/v1beta2
kind: ClusterConfiguration
`),
Expand All @@ -1420,7 +1420,7 @@ func TestUpdateCoreDNSImageInfoInKubeadmConfigMap(t *testing.T) {
ImageTag: "v1.2.3",
},
},
wantClusterConfiguration: yaml.Raw(`
wantClusterConfiguration: utilyaml.Raw(`
apiServer: {}
apiVersion: kubeadm.k8s.io/v1beta2
controllerManager: {}
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 065c072

Please sign in to comment.