From 99d75597b5bd30c31245101b413d27997038ba32 Mon Sep 17 00:00:00 2001 From: Yuvaraj Kakaraparthi Date: Thu, 2 Mar 2023 01:53:46 -0800 Subject: [PATCH] MachineDeployment rolloutAfter support --- api/v1alpha3/conversion.go | 5 ++ api/v1alpha3/zz_generated.conversion.go | 16 ++-- api/v1alpha4/conversion.go | 5 ++ api/v1alpha4/zz_generated.conversion.go | 16 ++-- api/v1beta1/machinedeployment_types.go | 11 +++ api/v1beta1/zz_generated.deepcopy.go | 4 + api/v1beta1/zz_generated.openapi.go | 8 +- .../client/alpha/kubeadmcontrolplane.go | 4 +- .../client/alpha/machinedeployment.go | 6 +- .../client/alpha/rollout_restarter.go | 7 +- .../client/alpha/rollout_restarter_test.go | 37 ++++++-- .../cluster.x-k8s.io_machinedeployments.yaml | 8 ++ .../v1beta1/kubeadm_control_plane_types.go | 3 + ...cluster.x-k8s.io_kubeadmcontrolplanes.yaml | 6 +- docs/book/src/tasks/upgrading-clusters.md | 20 ++--- .../machinedeployment_sync.go | 14 +-- .../machinedeployment/mdutil/util.go | 24 +++-- .../machinedeployment/mdutil/util_test.go | 87 ++++++++++++++++--- util/collections/machine_filters.go | 3 + 19 files changed, 215 insertions(+), 69 deletions(-) diff --git a/api/v1alpha3/conversion.go b/api/v1alpha3/conversion.go index 2f2df22f1bc3..9352eb4de3af 100644 --- a/api/v1alpha3/conversion.go +++ b/api/v1alpha3/conversion.go @@ -199,6 +199,7 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout dst.Spec.Template.Spec.NodeVolumeDetachTimeout = restored.Spec.Template.Spec.NodeVolumeDetachTimeout + dst.Spec.RolloutAfter = restored.Spec.RolloutAfter dst.Status.Conditions = restored.Status.Conditions return nil } @@ -316,6 +317,10 @@ func Convert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(in *clusterv1.MachineSp return autoConvert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(in, out, s) } +func Convert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec(in *clusterv1.MachineDeploymentSpec, out *MachineDeploymentSpec, s apiconversion.Scope) error { + return autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec(in, out, s) +} + func Convert_v1beta1_MachineDeploymentStatus_To_v1alpha3_MachineDeploymentStatus(in *clusterv1.MachineDeploymentStatus, out *MachineDeploymentStatus, s apiconversion.Scope) error { // Status.Conditions was introduced in v1alpha4, thus requiring a custom conversion function; the values is going to be preserved in an annotation thus allowing roundtrip without loosing informations return autoConvert_v1beta1_MachineDeploymentStatus_To_v1alpha3_MachineDeploymentStatus(in, out, s) diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go index 97c43da3ad7c..c7ea14ba3e55 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -160,11 +160,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.MachineDeploymentSpec)(nil), (*MachineDeploymentSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec(a.(*v1beta1.MachineDeploymentSpec), b.(*MachineDeploymentSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*MachineDeploymentStatus)(nil), (*v1beta1.MachineDeploymentStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_MachineDeploymentStatus_To_v1beta1_MachineDeploymentStatus(a.(*MachineDeploymentStatus), b.(*v1beta1.MachineDeploymentStatus), scope) }); err != nil { @@ -330,6 +325,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.MachineDeploymentSpec)(nil), (*MachineDeploymentSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec(a.(*v1beta1.MachineDeploymentSpec), b.(*MachineDeploymentSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.MachineDeploymentStatus)(nil), (*MachineDeploymentStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_MachineDeploymentStatus_To_v1alpha3_MachineDeploymentStatus(a.(*v1beta1.MachineDeploymentStatus), b.(*MachineDeploymentStatus), scope) }); err != nil { @@ -772,6 +772,7 @@ func Convert_v1alpha3_MachineDeploymentSpec_To_v1beta1_MachineDeploymentSpec(in func autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec(in *v1beta1.MachineDeploymentSpec, out *MachineDeploymentSpec, s conversion.Scope) error { out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) + // WARNING: in.RolloutAfter requires manual conversion: does not exist in peer-type out.Selector = in.Selector if err := Convert_v1beta1_MachineTemplateSpec_To_v1alpha3_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err @@ -792,11 +793,6 @@ func autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec return nil } -// Convert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec is an autogenerated conversion function. -func Convert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec(in *v1beta1.MachineDeploymentSpec, out *MachineDeploymentSpec, s conversion.Scope) error { - return autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha3_MachineDeploymentSpec(in, out, s) -} - func autoConvert_v1alpha3_MachineDeploymentStatus_To_v1beta1_MachineDeploymentStatus(in *MachineDeploymentStatus, out *v1beta1.MachineDeploymentStatus, s conversion.Scope) error { out.ObservedGeneration = in.ObservedGeneration out.Selector = in.Selector diff --git a/api/v1alpha4/conversion.go b/api/v1alpha4/conversion.go index b403119362be..b667eaa3c64e 100644 --- a/api/v1alpha4/conversion.go +++ b/api/v1alpha4/conversion.go @@ -270,6 +270,7 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout dst.Spec.Template.Spec.NodeVolumeDetachTimeout = restored.Spec.Template.Spec.NodeVolumeDetachTimeout + dst.Spec.RolloutAfter = restored.Spec.RolloutAfter return nil } @@ -335,6 +336,10 @@ func Convert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(in *clusterv1.MachineSp return autoConvert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(in, out, s) } +func Convert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec(in *clusterv1.MachineDeploymentSpec, out *MachineDeploymentSpec, s apiconversion.Scope) error { + return autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec(in, out, s) +} + func Convert_v1beta1_Topology_To_v1alpha4_Topology(in *clusterv1.Topology, out *Topology, s apiconversion.Scope) error { // spec.topology.variables has been added with v1beta1. return autoConvert_v1beta1_Topology_To_v1alpha4_Topology(in, out, s) diff --git a/api/v1alpha4/zz_generated.conversion.go b/api/v1alpha4/zz_generated.conversion.go index 3598cba1c7e0..1f0c12a798fd 100644 --- a/api/v1alpha4/zz_generated.conversion.go +++ b/api/v1alpha4/zz_generated.conversion.go @@ -230,11 +230,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.MachineDeploymentSpec)(nil), (*MachineDeploymentSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec(a.(*v1beta1.MachineDeploymentSpec), b.(*MachineDeploymentSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*MachineDeploymentStatus)(nil), (*v1beta1.MachineDeploymentStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha4_MachineDeploymentStatus_To_v1beta1_MachineDeploymentStatus(a.(*MachineDeploymentStatus), b.(*v1beta1.MachineDeploymentStatus), scope) }); err != nil { @@ -460,6 +455,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.MachineDeploymentSpec)(nil), (*MachineDeploymentSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec(a.(*v1beta1.MachineDeploymentSpec), b.(*MachineDeploymentSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.MachineDeploymentTopology)(nil), (*MachineDeploymentTopology)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_MachineDeploymentTopology_To_v1alpha4_MachineDeploymentTopology(a.(*v1beta1.MachineDeploymentTopology), b.(*MachineDeploymentTopology), scope) }); err != nil { @@ -1151,6 +1151,7 @@ func Convert_v1alpha4_MachineDeploymentSpec_To_v1beta1_MachineDeploymentSpec(in func autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec(in *v1beta1.MachineDeploymentSpec, out *MachineDeploymentSpec, s conversion.Scope) error { out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) + // WARNING: in.RolloutAfter requires manual conversion: does not exist in peer-type out.Selector = in.Selector if err := Convert_v1beta1_MachineTemplateSpec_To_v1alpha4_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err @@ -1163,11 +1164,6 @@ func autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec return nil } -// Convert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec is an autogenerated conversion function. -func Convert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec(in *v1beta1.MachineDeploymentSpec, out *MachineDeploymentSpec, s conversion.Scope) error { - return autoConvert_v1beta1_MachineDeploymentSpec_To_v1alpha4_MachineDeploymentSpec(in, out, s) -} - func autoConvert_v1alpha4_MachineDeploymentStatus_To_v1beta1_MachineDeploymentStatus(in *MachineDeploymentStatus, out *v1beta1.MachineDeploymentStatus, s conversion.Scope) error { out.ObservedGeneration = in.ObservedGeneration out.Selector = in.Selector diff --git a/api/v1beta1/machinedeployment_types.go b/api/v1beta1/machinedeployment_types.go index 1ada91dc42ec..70fb378ba53c 100644 --- a/api/v1beta1/machinedeployment_types.go +++ b/api/v1beta1/machinedeployment_types.go @@ -66,6 +66,8 @@ const ( // As a result, we use the hash of the machine template while ignoring all in-place mutable fields, i.e. the // machine template with only fields that could trigger a rollout for the machine-template-hash, making it // independent of the changes to any in-place mutable fields. + // A random string is appended at the end of the label value (label value format is "-")) + // to distinguish duplicate MachineSets that have the exact same spec but were created as a result of rolloutAfter. MachineDeploymentUniqueLabel = "machine-template-hash" ) @@ -97,6 +99,15 @@ type MachineDeploymentSpec struct { // +optional Replicas *int32 `json:"replicas,omitempty"` + // RolloutAfter is a field to indicate a rollout should be performed + // after the specified time even if no changes have been made to the + // MachineDeployment. + // Example: In the YAML the time can be specified in the RFC3339 format. + // To specify the rolloutAfter target as March 9, 2023, at 9 am UTC + // use "2023-03-09T09:00:00Z". + // +optional + RolloutAfter *metav1.Time `json:"rolloutAfter,omitempty"` + // Label selector for machines. Existing MachineSets whose machines are // selected by this will be the ones affected by this deployment. // It must match the machine template's labels. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index e4a3efc07402..49ee41659261 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1007,6 +1007,10 @@ func (in *MachineDeploymentSpec) DeepCopyInto(out *MachineDeploymentSpec) { *out = new(int32) **out = **in } + if in.RolloutAfter != nil { + in, out := &in.RolloutAfter, &out.RolloutAfter + *out = (*in).DeepCopy() + } in.Selector.DeepCopyInto(&out.Selector) in.Template.DeepCopyInto(&out.Template) if in.Strategy != nil { diff --git a/api/v1beta1/zz_generated.openapi.go b/api/v1beta1/zz_generated.openapi.go index 2b59d54b3395..e2612b1a9712 100644 --- a/api/v1beta1/zz_generated.openapi.go +++ b/api/v1beta1/zz_generated.openapi.go @@ -1703,6 +1703,12 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_MachineDeploymentSpec(ref common.R Format: "int32", }, }, + "rolloutAfter": { + SchemaProps: spec.SchemaProps{ + Description: "RolloutAfter is a field to indicate a rollout should be performed after the specified time even if no changes have been made to the MachineDeployment. Example: In the YAML the time can be specified in the RFC3339 format. To specify the rolloutAfter target as March 9, 2023, at 9 am UTC use \"2023-03-09T09:00:00Z\".", + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Time"), + }, + }, "selector": { SchemaProps: spec.SchemaProps{ Description: "Label selector for machines. Existing MachineSets whose machines are selected by this will be the ones affected by this deployment. It must match the machine template's labels.", @@ -1756,7 +1762,7 @@ func schema_sigsk8sio_cluster_api_api_v1beta1_MachineDeploymentSpec(ref common.R }, }, Dependencies: []string{ - "k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector", "sigs.k8s.io/cluster-api/api/v1beta1.MachineDeploymentStrategy", "sigs.k8s.io/cluster-api/api/v1beta1.MachineTemplateSpec"}, + "k8s.io/apimachinery/pkg/apis/meta/v1.LabelSelector", "k8s.io/apimachinery/pkg/apis/meta/v1.Time", "sigs.k8s.io/cluster-api/api/v1beta1.MachineDeploymentStrategy", "sigs.k8s.io/cluster-api/api/v1beta1.MachineTemplateSpec"}, } } diff --git a/cmd/clusterctl/client/alpha/kubeadmcontrolplane.go b/cmd/clusterctl/client/alpha/kubeadmcontrolplane.go index bbfe840a1c46..4ff821f8e88e 100644 --- a/cmd/clusterctl/client/alpha/kubeadmcontrolplane.go +++ b/cmd/clusterctl/client/alpha/kubeadmcontrolplane.go @@ -46,8 +46,8 @@ func getKubeadmControlPlane(proxy cluster.Proxy, name, namespace string) (*contr return kcpObj, nil } -// setRolloutAfter sets KubeadmControlPlane.spec.rolloutAfter. -func setRolloutAfter(proxy cluster.Proxy, name, namespace string) error { +// setRolloutAfterOnKCP sets KubeadmControlPlane.spec.rolloutAfter. +func setRolloutAfterOnKCP(proxy cluster.Proxy, name, namespace string) error { patch := client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf(`{"spec":{"rolloutAfter":"%v"}}`, time.Now().Format(time.RFC3339)))) return patchKubeadmControlPlane(proxy, name, namespace, patch) } diff --git a/cmd/clusterctl/client/alpha/machinedeployment.go b/cmd/clusterctl/client/alpha/machinedeployment.go index d4466c511330..4bf1609ba91e 100644 --- a/cmd/clusterctl/client/alpha/machinedeployment.go +++ b/cmd/clusterctl/client/alpha/machinedeployment.go @@ -52,9 +52,9 @@ func getMachineDeployment(proxy cluster.Proxy, name, namespace string) (*cluster return mdObj, nil } -// setRestartedAtAnnotation sets the restartedAt annotation in the MachineDeployment's spec.template.objectmeta. -func setRestartedAtAnnotation(proxy cluster.Proxy, name, namespace string) error { - patch := client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf(`{"spec":{"template":{"metadata":{"annotations":{"cluster.x-k8s.io/restartedAt":"%v"}}}}}`, time.Now().Format(time.RFC3339)))) +// setRolloutAfterOnMachineDeployment sets MachineDeployment.spec.rolloutAfter. +func setRolloutAfterOnMachineDeployment(proxy cluster.Proxy, name, namespace string) error { + patch := client.RawPatch(types.MergePatchType, []byte(fmt.Sprintf(`{"spec":{"rolloutAfter":"%v"}}`, time.Now().Format(time.RFC3339)))) return patchMachineDeployment(proxy, name, namespace, patch) } diff --git a/cmd/clusterctl/client/alpha/rollout_restarter.go b/cmd/clusterctl/client/alpha/rollout_restarter.go index 9928b928e45d..78da02962221 100644 --- a/cmd/clusterctl/client/alpha/rollout_restarter.go +++ b/cmd/clusterctl/client/alpha/rollout_restarter.go @@ -37,7 +37,10 @@ func (r *rollout) ObjectRestarter(proxy cluster.Proxy, ref corev1.ObjectReferenc if deployment.Spec.Paused { return errors.Errorf("can't restart paused MachineDeployment (run rollout resume first): %v/%v", ref.Kind, ref.Name) } - if err := setRestartedAtAnnotation(proxy, ref.Name, ref.Namespace); err != nil { + if deployment.Spec.RolloutAfter != nil && deployment.Spec.RolloutAfter.After(time.Now()) { + return errors.Errorf("can't update MachineDeployment (remove 'spec.rolloutAfter' first): %v/%v", ref.Kind, ref.Name) + } + if err := setRolloutAfterOnMachineDeployment(proxy, ref.Name, ref.Namespace); err != nil { return err } case KubeadmControlPlane: @@ -51,7 +54,7 @@ func (r *rollout) ObjectRestarter(proxy cluster.Proxy, ref corev1.ObjectReferenc if kcp.Spec.RolloutAfter != nil && kcp.Spec.RolloutAfter.After(time.Now()) { return errors.Errorf("can't update KubeadmControlPlane (remove 'spec.rolloutAfter' first): %v/%v", ref.Kind, ref.Name) } - if err := setRolloutAfter(proxy, ref.Name, ref.Namespace); err != nil { + if err := setRolloutAfterOnKCP(proxy, ref.Name, ref.Namespace); err != nil { return err } default: diff --git a/cmd/clusterctl/client/alpha/rollout_restarter_test.go b/cmd/clusterctl/client/alpha/rollout_restarter_test.go index f968ccf89187..2b13542d0e35 100644 --- a/cmd/clusterctl/client/alpha/rollout_restarter_test.go +++ b/cmd/clusterctl/client/alpha/rollout_restarter_test.go @@ -43,7 +43,7 @@ func Test_ObjectRestarter(t *testing.T) { wantRollout bool }{ { - name: "machinedeployment should have restart annotation", + name: "machinedeployment should have rolloutAfter", fields: fields{ objs: []client.Object{ &clusterv1.MachineDeployment{ @@ -67,7 +67,7 @@ func Test_ObjectRestarter(t *testing.T) { wantRollout: true, }, { - name: "paused machinedeployment should not have restart annotation", + name: "paused machinedeployment should not have rolloutAfter", fields: fields{ objs: []client.Object{ &clusterv1.MachineDeployment{ @@ -93,6 +93,33 @@ func Test_ObjectRestarter(t *testing.T) { wantErr: true, wantRollout: false, }, + { + name: "machinedeployment with spec.rolloutAfter should not be updatable", + fields: fields{ + objs: []client.Object{ + &clusterv1.MachineDeployment{ + TypeMeta: metav1.TypeMeta{ + Kind: "MachineDeployment", + APIVersion: "cluster.x-k8s.io/v1beta1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "md-1", + }, + Spec: clusterv1.MachineDeploymentSpec{ + RolloutAfter: &metav1.Time{Time: time.Now().Local().Add(time.Hour)}, + }, + }, + }, + ref: corev1.ObjectReference{ + Kind: MachineDeployment, + Name: "md-1", + Namespace: "default", + }, + }, + wantErr: true, + wantRollout: false, + }, { name: "kubeadmcontrolplane should have rolloutAfter", fields: fields{ @@ -193,9 +220,9 @@ func Test_ObjectRestarter(t *testing.T) { err = cl.Get(context.TODO(), key, md) g.Expect(err).ToNot(HaveOccurred()) if tt.wantRollout { - g.Expect(md.Spec.Template.Annotations).To(HaveKey("cluster.x-k8s.io/restartedAt")) + g.Expect(md.Spec.RolloutAfter).NotTo(BeNil()) } else { - g.Expect(md.Spec.Template.Annotations).ToNot(HaveKey("cluster.x-k8s.io/restartedAt")) + g.Expect(md.Spec.RolloutAfter).To(BeNil()) } case *controlplanev1.KubeadmControlPlane: kcp := &controlplanev1.KubeadmControlPlane{} @@ -204,7 +231,7 @@ func Test_ObjectRestarter(t *testing.T) { if tt.wantRollout { g.Expect(kcp.Spec.RolloutAfter).NotTo(BeNil()) } else { - g.Expect(kcp.Spec.RolloutAfter).To(nil) + g.Expect(kcp.Spec.RolloutAfter).To(BeNil()) } } } diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index 38573e47507a..745aff949158 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -1080,6 +1080,14 @@ spec: Defaults to 1. format: int32 type: integer + rolloutAfter: + description: 'RolloutAfter is a field to indicate a rollout should + be performed after the specified time even if no changes have been + made to the MachineDeployment. Example: In the YAML the time can + be specified in the RFC3339 format. To specify the rolloutAfter + target as March 9, 2023, at 9 am UTC use "2023-03-09T09:00:00Z".' + format: date-time + type: string selector: description: Label selector for machines. Existing MachineSets whose machines are selected by this will be the ones affected by this diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go index 272ffd12a4a1..c3b4570e5b17 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go @@ -102,6 +102,9 @@ type KubeadmControlPlaneSpec struct { // RolloutAfter is a field to indicate a rollout should be performed // after the specified time even if no changes have been made to the // KubeadmControlPlane. + // Example: In the YAML the time can be specified in the RFC3339 format. + // To specify the rolloutAfter target as March 9, 2023, at 9 am UTC + // use "2023-03-09T09:00:00Z". // +optional RolloutAfter *metav1.Time `json:"rolloutAfter,omitempty"` diff --git a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml index 46f077a57b2b..38061b717cfc 100644 --- a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml +++ b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanes.yaml @@ -3670,9 +3670,11 @@ spec: format: int32 type: integer rolloutAfter: - description: RolloutAfter is a field to indicate a rollout should + description: 'RolloutAfter is a field to indicate a rollout should be performed after the specified time even if no changes have been - made to the KubeadmControlPlane. + made to the KubeadmControlPlane. Example: In the YAML the time can + be specified in the RFC3339 format. To specify the rolloutAfter + target as March 9, 2023, at 9 am UTC use "2023-03-09T09:00:00Z".' format: date-time type: string rolloutBefore: diff --git a/docs/book/src/tasks/upgrading-clusters.md b/docs/book/src/tasks/upgrading-clusters.md index 5c85eeda183b..67745ea9507f 100644 --- a/docs/book/src/tasks/upgrading-clusters.md +++ b/docs/book/src/tasks/upgrading-clusters.md @@ -49,11 +49,11 @@ and then both the `Version` and `InfrastructureTemplate` should be modified in a #### How to schedule a machine rollout -A `KubeadmControlPlane` resource has a field `RolloutAfter` that can be set to a timestamp -(RFC-3339) after which a rollout should be triggered regardless of whether there were any changes -to the `KubeadmControlPlane.Spec` or not. This would roll out replacement control plane nodes -which can be useful e.g. to perform certificate rotation, reflect changes to machine templates, -move to new machines, etc. +The `KubeadmControlPlane` and `MachineDepoyment` resources have a field `RolloutAfter` that can be +set to a timestamp (RFC-3339) after which a rollout should be triggered regardless of whether there +were any changes to `KubeadmControlPlane.Spec`/`MachineDeployment.Spec.Template` or not. This would +roll out replacement nodes which can be useful e.g. to perform certificate rotation, reflect changes +to machine templates, move to new machines, etc. Note that this field can only be used for triggering a rollout, not for delaying one. Specifically, a rollout can also happen before the time specified in `RolloutAfter` if any changes are made to @@ -62,19 +62,13 @@ the spec before that time. The rollout can be triggered by running the following command: ```shell +# Trigger a KubeadmControlPlane rollout. clusterctl alpha rollout restart kubeadmcontrollplane/my-kcp -``` - -To do the same for machines managed by a `MachineDeployment` it's enough to make an arbitrary -change to its `Spec.Template`, one common approach is to run: -``` shell +# Trigger a MachineDeployment rollout. clusterctl alpha rollout restart machinedeployment/my-md-0 ``` -This will modify the template by setting an `cluster.x-k8s.io/restartedAt` annotation which will -trigger a rollout. - ### Upgrading machines managed by a `MachineDeployment` Upgrades are not limited to just the control plane. This section is not related to Kubeadm control plane specifically, diff --git a/internal/controllers/machinedeployment/machinedeployment_sync.go b/internal/controllers/machinedeployment/machinedeployment_sync.go index 9bc3640bca4f..bb528c86bb05 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync.go @@ -81,10 +81,11 @@ func (r *Reconciler) sync(ctx context.Context, md *clusterv1.MachineDeployment, // Note that currently the deployment controller is using caches to avoid querying the server for reads. // This may lead to stale reads of machine sets, thus incorrect deployment status. func (r *Reconciler) getAllMachineSetsAndSyncRevision(ctx context.Context, md *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, createIfNotExisted bool) (*clusterv1.MachineSet, []*clusterv1.MachineSet, error) { - _, allOldMSs := mdutil.FindOldMachineSets(md, msList) + reconciliationTime := metav1.Now() + _, allOldMSs := mdutil.FindOldMachineSets(md, msList, &reconciliationTime) // Get new machine set with the updated revision number - newMS, err := r.getNewMachineSet(ctx, md, msList, allOldMSs, createIfNotExisted) + newMS, err := r.getNewMachineSet(ctx, md, msList, allOldMSs, createIfNotExisted, &reconciliationTime) if err != nil { return nil, nil, err } @@ -95,7 +96,7 @@ func (r *Reconciler) getAllMachineSetsAndSyncRevision(ctx context.Context, md *c // Returns a MachineSet that matches the intent of the given MachineDeployment. // If there does not exist such a MachineSet and createIfNotExisted is true, create a new MachineSet. // If there is already such a MachineSet, update it to propagate in-place mutable fields from the MachineDeployment. -func (r *Reconciler) getNewMachineSet(ctx context.Context, md *clusterv1.MachineDeployment, msList, oldMSs []*clusterv1.MachineSet, createIfNotExists bool) (*clusterv1.MachineSet, error) { +func (r *Reconciler) getNewMachineSet(ctx context.Context, md *clusterv1.MachineDeployment, msList, oldMSs []*clusterv1.MachineSet, createIfNotExists bool, reconciliationTime *metav1.Time) (*clusterv1.MachineSet, error) { // Try to find a MachineSet which matches the MachineDeployments intent, while ignore diffs between // the in-place mutable fields. // If we find a matching MachineSet we just update it to propagate any changes to the in-place mutable @@ -103,7 +104,7 @@ func (r *Reconciler) getNewMachineSet(ctx context.Context, md *clusterv1.Machine // If we don't find a matching MachineSet, we need a rollout and thus create a new MachineSet. // Note: The in-place mutable fields can be just updated inline, because they do not affect the actual machines // themselves (i.e. the infrastructure and the software running on the Machines not the Machine object). - matchingMS := mdutil.FindNewMachineSet(md, msList) + matchingMS := mdutil.FindNewMachineSet(md, msList, reconciliationTime) // If there is a MachineSet that matches the intent of the MachineDeployment, update the MachineSet // to propagate all in-place mutable fields from MachineDeployment to the MachineSet. @@ -236,7 +237,10 @@ func (r *Reconciler) computeDesiredMachineSet(deployment *clusterv1.MachineDeplo if err != nil { return nil, errors.Wrap(err, "failed to compute desired MachineSet: failed to compute machine template hash") } - uniqueIdentifierLabelValue = fmt.Sprintf("%d", templateHash) + // Append a random string at the end of template hash. This is required to distinguish MachineSets that + // could be created with the same spec as a result of rolloutAfter. If not, computeDesiredMachineSet + // will end up updating the existing MachineSet instead of creating a new one. + uniqueIdentifierLabelValue = fmt.Sprintf("%d-%s", templateHash, apirand.String(5)) name = computeNewMachineSetName(deployment.Name+"-", apirand.SafeEncodeString(uniqueIdentifierLabelValue)) diff --git a/internal/controllers/machinedeployment/mdutil/util.go b/internal/controllers/machinedeployment/mdutil/util.go index b8e672cd908d..b6f99693ecb2 100644 --- a/internal/controllers/machinedeployment/mdutil/util.go +++ b/internal/controllers/machinedeployment/mdutil/util.go @@ -400,7 +400,10 @@ func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpe return templateCopy } -// FindNewMachineSet returns the new MS this given deployment targets (the one with the same machine template, ignoring in-place mutable fields). +// FindNewMachineSet returns the new MS this given deployment targets (the one with the same machine template, ignoring +// in-place mutable fields). +// Note: If the reconciliation time is after the deployment's `rolloutAfter` time, a MS has to be newer than +// `rolloutAfter` to be considered as matching the deployment's intent. // NOTE: If we find a matching MachineSet which only differs in in-place mutable fields we can use it to // fulfill the intent of the MachineDeployment by just updating the MachineSet to propagate in-place mutable fields. // Thus we don't have to create a new MachineSet and we can avoid an unnecessary rollout. @@ -409,10 +412,11 @@ func MachineTemplateDeepCopyRolloutFields(template *clusterv1.MachineTemplateSpe // not face a case where there exists a machine set matching the old logic but there does not exist a machineset matching the new logic. // In fact previously not matching MS can now start matching the target. Since there could be multiple matches, lets choose the // MS with the most replicas so that there is minimum machine churn. -func FindNewMachineSet(deployment *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) *clusterv1.MachineSet { +func FindNewMachineSet(deployment *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, reconciliationTime *metav1.Time) *clusterv1.MachineSet { sort.Sort(MachineSetsByDecreasingReplicas(msList)) for i := range msList { - if EqualMachineTemplate(&msList[i].Spec.Template, &deployment.Spec.Template) { + if EqualMachineTemplate(&msList[i].Spec.Template, &deployment.Spec.Template) && + !shouldRolloutAfter(msList[i], reconciliationTime, deployment.Spec.RolloutAfter) { // In rare cases, such as after cluster upgrades, Deployment may end up with // having more than one new MachineSets that have the same template, // see https://github.com/kubernetes/kubernetes/issues/40415 @@ -424,14 +428,24 @@ func FindNewMachineSet(deployment *clusterv1.MachineDeployment, msList []*cluste return nil } +func shouldRolloutAfter(ms *clusterv1.MachineSet, reconciliationTime *metav1.Time, rolloutAfter *metav1.Time) bool { + if ms == nil { + return false + } + if reconciliationTime == nil || rolloutAfter == nil { + return false + } + return ms.CreationTimestamp.Before(rolloutAfter) && rolloutAfter.Before(reconciliationTime) +} + // FindOldMachineSets returns the old machine sets targeted by the given Deployment, within the given slice of MSes. // Returns two list of machine sets // - the first contains all old machine sets with non-zero replicas // - the second contains all old machine sets -func FindOldMachineSets(deployment *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet) ([]*clusterv1.MachineSet, []*clusterv1.MachineSet) { +func FindOldMachineSets(deployment *clusterv1.MachineDeployment, msList []*clusterv1.MachineSet, reconciliationTime *metav1.Time) ([]*clusterv1.MachineSet, []*clusterv1.MachineSet) { var requiredMSs []*clusterv1.MachineSet allMSs := make([]*clusterv1.MachineSet, 0, len(msList)) - newMS := FindNewMachineSet(deployment, msList) + newMS := FindNewMachineSet(deployment, msList, reconciliationTime) for _, ms := range msList { // Filter out new machine set if newMS != nil && ms.UID == newMS.UID { diff --git a/internal/controllers/machinedeployment/mdutil/util_test.go b/internal/controllers/machinedeployment/mdutil/util_test.go index 5329b1a65c0c..8feed22a63aa 100644 --- a/internal/controllers/machinedeployment/mdutil/util_test.go +++ b/internal/controllers/machinedeployment/mdutil/util_test.go @@ -304,7 +304,14 @@ func TestEqualMachineTemplate(t *testing.T) { } func TestFindNewMachineSet(t *testing.T) { + twoBeforeRolloutAfter := metav1.Now() + oneBeforeRolloutAfter := metav1.NewTime(twoBeforeRolloutAfter.Add(time.Minute)) + rolloutAfter := metav1.NewTime(oneBeforeRolloutAfter.Add(time.Minute)) + oneAfterRolloutAfter := metav1.NewTime(rolloutAfter.Add(time.Minute)) + twoAfterRolloutAfter := metav1.NewTime(oneAfterRolloutAfter.Add(time.Minute)) + deployment := generateDeployment("nginx") + deployment.Spec.RolloutAfter = &rolloutAfter matchingMS := generateMS(deployment) @@ -317,11 +324,18 @@ func TestFindNewMachineSet(t *testing.T) { oldMS := generateMS(deployment) oldMS.Spec.Template.Spec.InfrastructureRef.Name = "changed-infra-ref" + msCreatedTwoBeforeRolloutAfter := generateMS(deployment) + msCreatedTwoBeforeRolloutAfter.CreationTimestamp = twoBeforeRolloutAfter + + msCreatedAfterRolloutAfter := generateMS(deployment) + msCreatedAfterRolloutAfter.CreationTimestamp = oneAfterRolloutAfter + tests := []struct { - Name string - deployment clusterv1.MachineDeployment - msList []*clusterv1.MachineSet - expected *clusterv1.MachineSet + Name string + deployment clusterv1.MachineDeployment + msList []*clusterv1.MachineSet + reconciliationTime *metav1.Time + expected *clusterv1.MachineSet }{ { Name: "Get the MachineSet with the MachineTemplate that matches the intent of the MachineDeployment", @@ -347,20 +361,48 @@ func TestFindNewMachineSet(t *testing.T) { msList: []*clusterv1.MachineSet{&oldMS}, expected: nil, }, + { + Name: "Get the MachineSet if reconciliationTime < rolloutAfter", + deployment: deployment, + msList: []*clusterv1.MachineSet{&msCreatedTwoBeforeRolloutAfter}, + reconciliationTime: &oneBeforeRolloutAfter, + expected: &msCreatedTwoBeforeRolloutAfter, + }, + { + Name: "Get nil if reconciliationTime is > rolloutAfter and no MachineSet is created after rolloutAfter", + deployment: deployment, + msList: []*clusterv1.MachineSet{&msCreatedTwoBeforeRolloutAfter}, + reconciliationTime: &oneAfterRolloutAfter, + expected: nil, + }, + { + Name: "Get MachineSet created after RolloutAfter if reconciliationTime is > rolloutAfter", + deployment: deployment, + msList: []*clusterv1.MachineSet{&msCreatedAfterRolloutAfter, &msCreatedTwoBeforeRolloutAfter}, + reconciliationTime: &twoAfterRolloutAfter, + expected: &msCreatedAfterRolloutAfter, + }, } for _, test := range tests { t.Run(test.Name, func(t *testing.T) { g := NewWithT(t) - ms := FindNewMachineSet(&test.deployment, test.msList) + ms := FindNewMachineSet(&test.deployment, test.msList, test.reconciliationTime) g.Expect(ms).To(Equal(test.expected)) }) } } func TestFindOldMachineSets(t *testing.T) { + twoBeforeRolloutAfter := metav1.Now() + oneBeforeRolloutAfter := metav1.NewTime(twoBeforeRolloutAfter.Add(time.Minute)) + rolloutAfter := metav1.NewTime(oneBeforeRolloutAfter.Add(time.Minute)) + oneAfterRolloutAfter := metav1.NewTime(rolloutAfter.Add(time.Minute)) + twoAfterRolloutAfter := metav1.NewTime(oneAfterRolloutAfter.Add(time.Minute)) + deployment := generateDeployment("nginx") + deployment.Spec.RolloutAfter = &rolloutAfter newMS := generateMS(deployment) newMS.Name = "aa" @@ -377,12 +419,19 @@ func TestFindOldMachineSets(t *testing.T) { oldDeployment.Spec.Template.Spec.InfrastructureRef.Name = "changed-infra-ref" oldMS := generateMS(oldDeployment) + msCreatedTwoBeforeRolloutAfter := generateMS(deployment) + msCreatedTwoBeforeRolloutAfter.CreationTimestamp = twoBeforeRolloutAfter + + msCreatedAfterRolloutAfter := generateMS(deployment) + msCreatedAfterRolloutAfter.CreationTimestamp = oneAfterRolloutAfter + tests := []struct { - Name string - deployment clusterv1.MachineDeployment - msList []*clusterv1.MachineSet - expected []*clusterv1.MachineSet - expectedRequire []*clusterv1.MachineSet + Name string + deployment clusterv1.MachineDeployment + msList []*clusterv1.MachineSet + reconciliationTime *metav1.Time + expected []*clusterv1.MachineSet + expectedRequire []*clusterv1.MachineSet }{ { Name: "Get old MachineSets", @@ -419,13 +468,29 @@ func TestFindOldMachineSets(t *testing.T) { expected: []*clusterv1.MachineSet{}, expectedRequire: nil, }, + { + Name: "Get old MachineSets with new MachineSets, MachineSets created before rolloutAfter are considered new when reconciliationTime < rolloutAfter", + deployment: deployment, + msList: []*clusterv1.MachineSet{&msCreatedTwoBeforeRolloutAfter}, + reconciliationTime: &oneBeforeRolloutAfter, + expected: nil, + expectedRequire: nil, + }, + { + Name: "Get old MachineSets with new MachineSets, MachineSets created after rolloutAfter are considered new when reconciliationTime > rolloutAfter", + deployment: deployment, + msList: []*clusterv1.MachineSet{&msCreatedTwoBeforeRolloutAfter, &msCreatedAfterRolloutAfter}, + reconciliationTime: &twoAfterRolloutAfter, + expected: []*clusterv1.MachineSet{&msCreatedTwoBeforeRolloutAfter}, + expectedRequire: nil, + }, } for _, test := range tests { t.Run(test.Name, func(t *testing.T) { g := NewWithT(t) - requireMS, allMS := FindOldMachineSets(&test.deployment, test.msList) + requireMS, allMS := FindOldMachineSets(&test.deployment, test.msList, test.reconciliationTime) g.Expect(allMS).To(ConsistOf(test.expected)) // MSs are getting filtered correctly by ms.spec.replicas g.Expect(requireMS).To(ConsistOf(test.expectedRequire)) diff --git a/util/collections/machine_filters.go b/util/collections/machine_filters.go index a4f69b2fd791..d856610129c3 100644 --- a/util/collections/machine_filters.go +++ b/util/collections/machine_filters.go @@ -175,6 +175,9 @@ func ShouldRolloutAfter(reconciliationTime, rolloutAfter *metav1.Time) Func { if machine == nil { return false } + if reconciliationTime == nil || rolloutAfter == nil { + return false + } return machine.CreationTimestamp.Before(rolloutAfter) && rolloutAfter.Before(reconciliationTime) } }