From 521849bb19b8a405a5a5e989e87e508e091739c6 Mon Sep 17 00:00:00 2001 From: Abhinav Pandey Date: Wed, 24 Jan 2024 00:00:03 -0800 Subject: [PATCH] Add support for in-place upgrades in MachineDeployments --- api/v1beta1/machinedeployment_types.go | 11 +- .../api/v1beta1/zz_generated.deepcopy.go | 3 +- .../cluster.x-k8s.io_clusterclasses.yaml | 1 + .../crd/bases/cluster.x-k8s.io_clusters.yaml | 1 + .../cluster.x-k8s.io_machinedeployments.yaml | 1 + .../machinedeployment_controller.go | 4 + .../machinedeployment_inplace.go | 44 +++++++ .../machinedeployment_inplace_test.go | 110 ++++++++++++++++++ .../machinedeployment/mdutil/util.go | 2 + .../test/builder/zz_generated.deepcopy.go | 23 ++++ 10 files changed, 198 insertions(+), 2 deletions(-) create mode 100644 internal/controllers/machinedeployment/machinedeployment_inplace.go create mode 100644 internal/controllers/machinedeployment/machinedeployment_inplace_test.go diff --git a/api/v1beta1/machinedeployment_types.go b/api/v1beta1/machinedeployment_types.go index 13a023d07a63..c55d17b6b00e 100644 --- a/api/v1beta1/machinedeployment_types.go +++ b/api/v1beta1/machinedeployment_types.go @@ -35,6 +35,9 @@ const ( // i.e. gradually scale down the old MachineSet and scale up the new one. RollingUpdateMachineDeploymentStrategyType MachineDeploymentStrategyType = "RollingUpdate" + // InPlaceMachineDeploymentStrategyType upgrades the machines within the same MachineSet without rolling out any new nodes. + InPlaceMachineDeploymentStrategyType MachineDeploymentStrategyType = "InPlace" + // OnDeleteMachineDeploymentStrategyType replaces old MachineSets when the deletion of the associated machines are completed. OnDeleteMachineDeploymentStrategyType MachineDeploymentStrategyType = "OnDelete" @@ -54,6 +57,12 @@ const ( // proportions in case the deployment has surge replicas. MaxReplicasAnnotation = "machinedeployment.clusters.x-k8s.io/max-replicas" + // MachineDeploymentInPlaceUpgradeAnnotation is used to denote that the MachineDeployment needs to be in-place upgraded by an external entity. + // This annotation will be added to the MD object when `strategy.type` is set to `InPlace`. + // The external upgrader entity should watch for the annotation and trigger an upgrade when it's added. + // Once the upgrade is complete, the external upgrade implementer is also responsible for removing this annotation. + MachineDeploymentInPlaceUpgradeAnnotation = "machinedeployment.clusters.x-k8s.io/in-place-upgrade-needed" + // MachineDeploymentUniqueLabel is used to uniquely identify the Machines of a MachineSet. // The MachineDeployment controller will set this label on a MachineSet when it is created. // The label is also applied to the Machines of the MachineSet and used in the MachineSet selector. @@ -154,7 +163,7 @@ type MachineDeploymentSpec struct { type MachineDeploymentStrategy struct { // Type of deployment. Allowed values are RollingUpdate and OnDelete. // The default is RollingUpdate. - // +kubebuilder:validation:Enum=RollingUpdate;OnDelete + // +kubebuilder:validation:Enum=RollingUpdate;OnDelete;InPlace // +optional Type MachineDeploymentStrategyType `json:"type,omitempty"` diff --git a/bootstrap/kubeadm/api/v1beta1/zz_generated.deepcopy.go b/bootstrap/kubeadm/api/v1beta1/zz_generated.deepcopy.go index af85cfd218ec..9fefdcb620f4 100644 --- a/bootstrap/kubeadm/api/v1beta1/zz_generated.deepcopy.go +++ b/bootstrap/kubeadm/api/v1beta1/zz_generated.deepcopy.go @@ -169,7 +169,8 @@ func (in *BottlerocketBootSettings) DeepCopyInto(out *BottlerocketBootSettings) if val == nil { (*out)[key] = nil } else { - in, out := &val, &outVal + inVal := (*in)[key] + in, out := &inVal, &outVal *out = make([]string, len(*in)) copy(*out, *in) } diff --git a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml index 34823e44bf11..328bd1c7c6f4 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml @@ -1294,6 +1294,7 @@ spec: enum: - RollingUpdate - OnDelete + - InPlace type: string type: object template: diff --git a/config/crd/bases/cluster.x-k8s.io_clusters.yaml b/config/crd/bases/cluster.x-k8s.io_clusters.yaml index ab514c177fe7..c99cdf68d97f 100644 --- a/config/crd/bases/cluster.x-k8s.io_clusters.yaml +++ b/config/crd/bases/cluster.x-k8s.io_clusters.yaml @@ -1171,6 +1171,7 @@ spec: enum: - RollingUpdate - OnDelete + - InPlace type: string type: object variables: diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index 8abaa5ec1ff5..e3319661ce7e 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -703,6 +703,7 @@ spec: enum: - RollingUpdate - OnDelete + - InPlace type: string type: object template: diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go index 84c8095ef976..f5811079617e 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller.go @@ -278,6 +278,10 @@ func (r *Reconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, return r.rolloutRolling(ctx, md, msList) } + if md.Spec.Strategy.Type == clusterv1.InPlaceMachineDeploymentStrategyType { + return r.rolloutInPlace(ctx, md, msList) + } + if md.Spec.Strategy.Type == clusterv1.OnDeleteMachineDeploymentStrategyType { return r.rolloutOnDelete(ctx, md, msList) } diff --git a/internal/controllers/machinedeployment/machinedeployment_inplace.go b/internal/controllers/machinedeployment/machinedeployment_inplace.go new file mode 100644 index 000000000000..8bca9e693797 --- /dev/null +++ b/internal/controllers/machinedeployment/machinedeployment_inplace.go @@ -0,0 +1,44 @@ +package machinedeployment + +import ( + "context" + + "github.com/pkg/errors" + kerrors "k8s.io/apimachinery/pkg/util/errors" + ctrl "sigs.k8s.io/controller-runtime" + + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/cluster-api/util/annotations" +) + +func (r *Reconciler) rolloutInPlace(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) (reterr error) { + log := ctrl.LoggerFrom(ctx) + + // For in-place upgrade, we shouldn't try to create a new MachineSet as that would trigger a rollout. + // Instead, we should try to get latest MachineSet that matches the MachineDeployment.Spec.Template/ + // If no such MachineSet exists yet, this means the MachineSet hasn't been in-place upgraded yet. + // The external in-place upgrade implementer is responsible for updating the latest MachineSet's template + // after in-place upgrade of all worker nodes belonging to the MD is complete. + // Once the MachineSet is updated, this function will return the latest MachineSet that matches the + // MachineDeployment template and thus we can deduce that the in-place upgrade is complete. + newMachineSet, oldMachineSets, err := r.getAllMachineSetsAndSyncRevision(ctx, md, msList, false) + if err != nil { + return err + } + + defer func() { + allMSs := append(oldMachineSets, newMachineSet) + + // Always attempt to sync the status + err := r.syncDeploymentStatus(allMSs, newMachineSet, md) + reterr = kerrors.NewAggregate([]error{reterr, err}) + }() + + if newMachineSet == nil { + log.Info("Changes detected, InPlace upgrade strategy detected, adding the annotation") + annotations.AddAnnotations(md, map[string]string{clusterv1.MachineDeploymentInPlaceUpgradeAnnotation: "true"}) + return errors.New("new MachineSet not found. This most likely means that the in-place upgrade hasn't finished yet") + } + + return r.sync(ctx, md, msList) +} diff --git a/internal/controllers/machinedeployment/machinedeployment_inplace_test.go b/internal/controllers/machinedeployment/machinedeployment_inplace_test.go new file mode 100644 index 000000000000..cbda181cbf21 --- /dev/null +++ b/internal/controllers/machinedeployment/machinedeployment_inplace_test.go @@ -0,0 +1,110 @@ +package machinedeployment + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" + "k8s.io/utils/pointer" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +const ( + mdName = "my-md" + msName = "my-ms" + version128 = "v1.28.0" + version129 = "v1.29.0" +) + +func getMachineDeployment(name string, version string, replicas int32) *clusterv1.MachineDeployment { + return &clusterv1.MachineDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: clusterv1.MachineDeploymentSpec{ + Strategy: &clusterv1.MachineDeploymentStrategy{ + Type: clusterv1.InPlaceMachineDeploymentStrategyType, + }, + Replicas: pointer.Int32(replicas), + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + ClusterName: "my-cluster", + Version: pointer.String(version), + }, + }, + }, + } +} + +func getMachineSet(name string, version string, replicas int32) *clusterv1.MachineSet { + return &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: clusterv1.MachineSetSpec{ + Replicas: pointer.Int32(replicas), + Template: clusterv1.MachineTemplateSpec{ + Spec: clusterv1.MachineSpec{ + ClusterName: "my-cluster", + Version: pointer.String(version), + }, + }, + }, + } +} + +func TestRolloutInPlace(t *testing.T) { + testCases := []struct { + name string + machineDeployment *clusterv1.MachineDeployment + msList []*clusterv1.MachineSet + annotationExpected bool + expectErr bool + }{ + { + name: "MD template matches MS template", + machineDeployment: getMachineDeployment(mdName, version128, 2), + msList: []*clusterv1.MachineSet{getMachineSet(msName, version128, 2)}, + annotationExpected: false, + expectErr: false, + }, + { + name: "MD template doesn't MS template", + machineDeployment: getMachineDeployment(mdName, version128, 2), + msList: []*clusterv1.MachineSet{getMachineSet(msName, version129, 2)}, + annotationExpected: true, + expectErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + + resources := []client.Object{ + tc.machineDeployment, + } + + for key := range tc.msList { + resources = append(resources, tc.msList[key]) + } + + r := &Reconciler{ + Client: fake.NewClientBuilder().WithObjects(resources...).Build(), + recorder: record.NewFakeRecorder(32), + } + + err := r.rolloutInPlace(ctx, tc.machineDeployment, tc.msList) + if tc.expectErr { + g.Expect(err).To(HaveOccurred()) + } + + _, ok := tc.machineDeployment.Annotations[clusterv1.MachineDeploymentInPlaceUpgradeAnnotation] + g.Expect(ok).To(Equal(tc.annotationExpected)) + }) + } + +} diff --git a/internal/controllers/machinedeployment/mdutil/util.go b/internal/controllers/machinedeployment/mdutil/util.go index 2cf9d3427891..203cd90f8d25 100644 --- a/internal/controllers/machinedeployment/mdutil/util.go +++ b/internal/controllers/machinedeployment/mdutil/util.go @@ -563,6 +563,8 @@ func NewMSNewReplicas(deployment *clusterv1.MachineDeployment, allMSs []*cluster // the desired number of replicas in the MachineDeployment scaleUpCount := *(deployment.Spec.Replicas) - currentMachineCount return newMSReplicas + scaleUpCount, nil + case clusterv1.InPlaceMachineDeploymentStrategyType: + return 0, nil default: return 0, fmt.Errorf("failed to compute replicas: deployment strategy %v isn't supported", deployment.Spec.Strategy.Type) } diff --git a/internal/test/builder/zz_generated.deepcopy.go b/internal/test/builder/zz_generated.deepcopy.go index 4ca1c64caf97..64e72d45b593 100644 --- a/internal/test/builder/zz_generated.deepcopy.go +++ b/internal/test/builder/zz_generated.deepcopy.go @@ -95,6 +95,10 @@ func (in *ClusterBuilder) DeepCopyInto(out *ClusterBuilder) { in, out := &in.controlPlane, &out.controlPlane *out = (*in).DeepCopy() } + if in.managedEtcd != nil { + in, out := &in.managedEtcd, &out.managedEtcd + *out = (*in).DeepCopy() + } if in.network != nil { in, out := &in.network, &out.network *out = new(v1beta1.ClusterNetwork) @@ -274,6 +278,25 @@ func (in *ControlPlaneTemplateBuilder) DeepCopy() *ControlPlaneTemplateBuilder { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EtcdPlaneBuilder) DeepCopyInto(out *EtcdPlaneBuilder) { + *out = *in + if in.obj != nil { + in, out := &in.obj, &out.obj + *out = (*in).DeepCopy() + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EtcdPlaneBuilder. +func (in *EtcdPlaneBuilder) DeepCopy() *EtcdPlaneBuilder { + if in == nil { + return nil + } + out := new(EtcdPlaneBuilder) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InfrastructureClusterBuilder) DeepCopyInto(out *InfrastructureClusterBuilder) { *out = *in