diff --git a/api/v1alpha3/conversion.go b/api/v1alpha3/conversion.go index d761d7adc942..4b3c5dd99e9f 100644 --- a/api/v1alpha3/conversion.go +++ b/api/v1alpha3/conversion.go @@ -97,6 +97,7 @@ func (src *Machine) ConvertTo(dstRaw conversion.Hub) error { return err } + dst.Spec.NodeDeletionTimeout = restored.Spec.NodeDeletionTimeout dst.Status.NodeInfo = restored.Status.NodeInfo return nil } @@ -139,6 +140,7 @@ func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error { if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { return err } + dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout dst.Status.Conditions = restored.Status.Conditions return nil } @@ -192,6 +194,7 @@ func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.Strategy.RollingUpdate.DeletePolicy = restored.Spec.Strategy.RollingUpdate.DeletePolicy } + dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout dst.Status.Conditions = restored.Status.Conditions return nil } @@ -304,6 +307,11 @@ func Convert_v1beta1_MachineStatus_To_v1alpha3_MachineStatus(in *clusterv1.Machi return autoConvert_v1beta1_MachineStatus_To_v1alpha3_MachineStatus(in, out, s) } +func Convert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(in *clusterv1.MachineSpec, out *MachineSpec, s apiconversion.Scope) error { + // spec.nodeDeletionTimeout has been added with v1beta1. + return autoConvert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(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 4d3cbed70601..9661e6cc3f18 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -270,11 +270,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.MachineSpec)(nil), (*MachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(a.(*v1beta1.MachineSpec), b.(*MachineSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*MachineTemplateSpec)(nil), (*v1beta1.MachineTemplateSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha3_MachineTemplateSpec_To_v1beta1_MachineTemplateSpec(a.(*MachineTemplateSpec), b.(*v1beta1.MachineTemplateSpec), scope) }); err != nil { @@ -355,6 +350,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.MachineSpec)(nil), (*MachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(a.(*v1beta1.MachineSpec), b.(*MachineSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.MachineStatus)(nil), (*MachineStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_MachineStatus_To_v1alpha3_MachineStatus(a.(*v1beta1.MachineStatus), b.(*MachineStatus), scope) }); err != nil { @@ -1221,14 +1221,10 @@ func autoConvert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(in *v1beta1.Machine out.ProviderID = (*string)(unsafe.Pointer(in.ProviderID)) out.FailureDomain = (*string)(unsafe.Pointer(in.FailureDomain)) out.NodeDrainTimeout = (*metav1.Duration)(unsafe.Pointer(in.NodeDrainTimeout)) + // WARNING: in.NodeDeletionTimeout requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec is an autogenerated conversion function. -func Convert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(in *v1beta1.MachineSpec, out *MachineSpec, s conversion.Scope) error { - return autoConvert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(in, out, s) -} - func autoConvert_v1alpha3_MachineStatus_To_v1beta1_MachineStatus(in *MachineStatus, out *v1beta1.MachineStatus, s conversion.Scope) error { out.NodeRef = (*v1.ObjectReference)(unsafe.Pointer(in.NodeRef)) out.LastUpdated = (*metav1.Time)(unsafe.Pointer(in.LastUpdated)) diff --git a/api/v1alpha4/conversion.go b/api/v1alpha4/conversion.go index 3bfb31ba0949..5f789ae6f000 100644 --- a/api/v1alpha4/conversion.go +++ b/api/v1alpha4/conversion.go @@ -131,13 +131,33 @@ func (dst *ClusterClassList) ConvertFrom(srcRaw conversion.Hub) error { func (src *Machine) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*clusterv1.Machine) - return Convert_v1alpha4_Machine_To_v1beta1_Machine(src, dst, nil) + if err := Convert_v1alpha4_Machine_To_v1beta1_Machine(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &clusterv1.Machine{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.NodeDeletionTimeout = restored.Spec.NodeDeletionTimeout + return nil } func (dst *Machine) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*clusterv1.Machine) - return Convert_v1beta1_Machine_To_v1alpha4_Machine(src, dst, nil) + if err := Convert_v1beta1_Machine_To_v1alpha4_Machine(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion except for metadata + if err := utilconversion.MarshalData(src, dst); err != nil { + return err + } + + return nil } func (src *MachineList) ConvertTo(dstRaw conversion.Hub) error { @@ -155,13 +175,29 @@ func (dst *MachineList) ConvertFrom(srcRaw conversion.Hub) error { func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*clusterv1.MachineSet) - return Convert_v1alpha4_MachineSet_To_v1beta1_MachineSet(src, dst, nil) + if err := Convert_v1alpha4_MachineSet_To_v1beta1_MachineSet(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &clusterv1.MachineSet{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout + return nil } func (dst *MachineSet) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*clusterv1.MachineSet) - return Convert_v1beta1_MachineSet_To_v1alpha4_MachineSet(src, dst, nil) + if err := Convert_v1beta1_MachineSet_To_v1alpha4_MachineSet(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion except for metadata + return utilconversion.MarshalData(src, dst) } func (src *MachineSetList) ConvertTo(dstRaw conversion.Hub) error { @@ -179,13 +215,29 @@ func (dst *MachineSetList) ConvertFrom(srcRaw conversion.Hub) error { func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*clusterv1.MachineDeployment) - return Convert_v1alpha4_MachineDeployment_To_v1beta1_MachineDeployment(src, dst, nil) + if err := Convert_v1alpha4_MachineDeployment_To_v1beta1_MachineDeployment(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &clusterv1.MachineDeployment{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + + dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout + return nil } func (dst *MachineDeployment) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*clusterv1.MachineDeployment) - return Convert_v1beta1_MachineDeployment_To_v1alpha4_MachineDeployment(src, dst, nil) + if err := Convert_v1beta1_MachineDeployment_To_v1alpha4_MachineDeployment(src, dst, nil); err != nil { + return err + } + + // Preserve Hub data on down-conversion except for metadata + return utilconversion.MarshalData(src, dst) } func (src *MachineDeploymentList) ConvertTo(dstRaw conversion.Hub) error { @@ -234,6 +286,11 @@ func Convert_v1beta1_ClusterClassSpec_To_v1alpha4_ClusterClassSpec(in *clusterv1 return autoConvert_v1beta1_ClusterClassSpec_To_v1alpha4_ClusterClassSpec(in, out, s) } +func Convert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(in *clusterv1.MachineSpec, out *MachineSpec, s apiconversion.Scope) error { + // spec.nodeDeletionTimeout has been added with v1beta1. + return autoConvert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(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 a8aeec191bca..9a9e3d3dc1ab 100644 --- a/api/v1alpha4/zz_generated.conversion.go +++ b/api/v1alpha4/zz_generated.conversion.go @@ -375,11 +375,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.MachineSpec)(nil), (*MachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(a.(*v1beta1.MachineSpec), b.(*MachineSpec), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*v1beta1.MachineStatus)(nil), (*MachineStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_MachineStatus_To_v1alpha4_MachineStatus(a.(*v1beta1.MachineStatus), b.(*MachineStatus), scope) }); err != nil { @@ -475,6 +470,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.MachineSpec)(nil), (*MachineSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(a.(*v1beta1.MachineSpec), b.(*MachineSpec), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.Topology)(nil), (*Topology)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_Topology_To_v1alpha4_Topology(a.(*v1beta1.Topology), b.(*Topology), scope) }); err != nil { @@ -1084,7 +1084,17 @@ func Convert_v1beta1_MachineDeploymentClassTemplate_To_v1alpha4_MachineDeploymen func autoConvert_v1alpha4_MachineDeploymentList_To_v1beta1_MachineDeploymentList(in *MachineDeploymentList, out *v1beta1.MachineDeploymentList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1beta1.MachineDeployment)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1beta1.MachineDeployment, len(*in)) + for i := range *in { + if err := Convert_v1alpha4_MachineDeployment_To_v1beta1_MachineDeployment(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1095,7 +1105,17 @@ func Convert_v1alpha4_MachineDeploymentList_To_v1beta1_MachineDeploymentList(in func autoConvert_v1beta1_MachineDeploymentList_To_v1alpha4_MachineDeploymentList(in *v1beta1.MachineDeploymentList, out *MachineDeploymentList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]MachineDeployment)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]MachineDeployment, len(*in)) + for i := range *in { + if err := Convert_v1beta1_MachineDeployment_To_v1alpha4_MachineDeployment(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1445,7 +1465,17 @@ func Convert_v1beta1_MachineSet_To_v1alpha4_MachineSet(in *v1beta1.MachineSet, o func autoConvert_v1alpha4_MachineSetList_To_v1beta1_MachineSetList(in *MachineSetList, out *v1beta1.MachineSetList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1beta1.MachineSet)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1beta1.MachineSet, len(*in)) + for i := range *in { + if err := Convert_v1alpha4_MachineSet_To_v1beta1_MachineSet(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1456,7 +1486,17 @@ func Convert_v1alpha4_MachineSetList_To_v1beta1_MachineSetList(in *MachineSetLis func autoConvert_v1beta1_MachineSetList_To_v1alpha4_MachineSetList(in *v1beta1.MachineSetList, out *MachineSetList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]MachineSet)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]MachineSet, len(*in)) + for i := range *in { + if err := Convert_v1beta1_MachineSet_To_v1alpha4_MachineSet(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1563,14 +1603,10 @@ func autoConvert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(in *v1beta1.Machine out.ProviderID = (*string)(unsafe.Pointer(in.ProviderID)) out.FailureDomain = (*string)(unsafe.Pointer(in.FailureDomain)) out.NodeDrainTimeout = (*metav1.Duration)(unsafe.Pointer(in.NodeDrainTimeout)) + // WARNING: in.NodeDeletionTimeout requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec is an autogenerated conversion function. -func Convert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(in *v1beta1.MachineSpec, out *MachineSpec, s conversion.Scope) error { - return autoConvert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(in, out, s) -} - func autoConvert_v1alpha4_MachineStatus_To_v1beta1_MachineStatus(in *MachineStatus, out *v1beta1.MachineStatus, s conversion.Scope) error { out.NodeRef = (*v1.ObjectReference)(unsafe.Pointer(in.NodeRef)) out.NodeInfo = (*v1.NodeSystemInfo)(unsafe.Pointer(in.NodeInfo)) diff --git a/api/v1beta1/machine_types.go b/api/v1beta1/machine_types.go index b50ce9fbaa3c..073610fa03bf 100644 --- a/api/v1beta1/machine_types.go +++ b/api/v1beta1/machine_types.go @@ -96,6 +96,12 @@ type MachineSpec struct { // NOTE: NodeDrainTimeout is different from `kubectl drain --timeout` // +optional NodeDrainTimeout *metav1.Duration `json:"nodeDrainTimeout,omitempty"` + + // NodeDeletionTimeout defines how long the controller will attempt to delete the Node that the Machine + // hosts after the Machine is marked for deletion. A duration of 0 will retry deletion indefinitely. + // Defaults to 10 seconds. + // +optional + NodeDeletionTimeout *metav1.Duration `json:"nodeDeletionTimeout,omitempty"` } // ANCHOR_END: MachineSpec diff --git a/api/v1beta1/machine_webhook.go b/api/v1beta1/machine_webhook.go index dd27f8300d53..ec7058c533eb 100644 --- a/api/v1beta1/machine_webhook.go +++ b/api/v1beta1/machine_webhook.go @@ -19,8 +19,10 @@ package v1beta1 import ( "fmt" "strings" + "time" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" ctrl "sigs.k8s.io/controller-runtime" @@ -29,6 +31,8 @@ import ( "sigs.k8s.io/cluster-api/util/version" ) +const defaultNodeDeletionTimeout = 10 * time.Second + func (m *Machine) SetupWebhookWithManager(mgr ctrl.Manager) error { return ctrl.NewWebhookManagedBy(mgr). For(m). @@ -60,6 +64,10 @@ func (m *Machine) Default() { normalizedVersion := "v" + *m.Spec.Version m.Spec.Version = &normalizedVersion } + + if m.Spec.NodeDeletionTimeout == nil { + m.Spec.NodeDeletionTimeout = &metav1.Duration{Duration: defaultNodeDeletionTimeout} + } } // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. diff --git a/api/v1beta1/machine_webhook_test.go b/api/v1beta1/machine_webhook_test.go index 71383d43050b..0218ffa6f854 100644 --- a/api/v1beta1/machine_webhook_test.go +++ b/api/v1beta1/machine_webhook_test.go @@ -46,6 +46,7 @@ func TestMachineDefault(t *testing.T) { g.Expect(m.Spec.Bootstrap.ConfigRef.Namespace).To(Equal(m.Namespace)) g.Expect(m.Spec.InfrastructureRef.Namespace).To(Equal(m.Namespace)) g.Expect(*m.Spec.Version).To(Equal("v1.17.5")) + g.Expect(m.Spec.NodeDeletionTimeout.Duration).To(Equal(defaultNodeDeletionTimeout)) } func TestMachineBootstrapValidation(t *testing.T) { diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 958ed279fb5f..686dd2cbbb64 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1323,6 +1323,11 @@ func (in *MachineSpec) DeepCopyInto(out *MachineSpec) { *out = new(metav1.Duration) **out = **in } + if in.NodeDeletionTimeout != nil { + in, out := &in.NodeDeletionTimeout, &out.NodeDeletionTimeout + *out = new(metav1.Duration) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachineSpec. diff --git a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml index 40d28f1fd07a..0bb0b9093921 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinedeployments.yaml @@ -1285,6 +1285,12 @@ spec: description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' type: string type: object + nodeDeletionTimeout: + description: NodeDeletionTimeout defines how long the controller + will attempt to delete the Node that the Machine hosts after + the Machine is marked for deletion. A duration of 0 will + retry deletion indefinitely. Defaults to 10 seconds. + type: string nodeDrainTimeout: description: 'NodeDrainTimeout is the total amount of time that the controller will spend on draining a node. The default diff --git a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml index abc9350babbb..c4d81d8ea7db 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinepools.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinepools.yaml @@ -1165,6 +1165,12 @@ spec: description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' type: string type: object + nodeDeletionTimeout: + description: NodeDeletionTimeout defines how long the controller + will attempt to delete the Node that the Machine hosts after + the Machine is marked for deletion. A duration of 0 will + retry deletion indefinitely. Defaults to 10 seconds. + type: string nodeDrainTimeout: description: 'NodeDrainTimeout is the total amount of time that the controller will spend on draining a node. The default diff --git a/config/crd/bases/cluster.x-k8s.io_machines.yaml b/config/crd/bases/cluster.x-k8s.io_machines.yaml index 40a2779948be..4665fba88ed4 100644 --- a/config/crd/bases/cluster.x-k8s.io_machines.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machines.yaml @@ -886,6 +886,12 @@ spec: description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' type: string type: object + nodeDeletionTimeout: + description: NodeDeletionTimeout defines how long the controller will + attempt to delete the Node that the Machine hosts after the Machine + is marked for deletion. A duration of 0 will retry deletion indefinitely. + Defaults to 10 seconds. + type: string nodeDrainTimeout: description: 'NodeDrainTimeout is the total amount of time that the controller will spend on draining a node. The default value is 0, diff --git a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml index 48cd9047a8b7..596563b9581d 100644 --- a/config/crd/bases/cluster.x-k8s.io_machinesets.yaml +++ b/config/crd/bases/cluster.x-k8s.io_machinesets.yaml @@ -1091,6 +1091,12 @@ spec: description: 'UID of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#uids' type: string type: object + nodeDeletionTimeout: + description: NodeDeletionTimeout defines how long the controller + will attempt to delete the Node that the Machine hosts after + the Machine is marked for deletion. A duration of 0 will + retry deletion indefinitely. Defaults to 10 seconds. + type: string nodeDrainTimeout: description: 'NodeDrainTimeout is the total amount of time that the controller will spend on draining a node. The default diff --git a/controlplane/kubeadm/api/v1alpha3/conversion.go b/controlplane/kubeadm/api/v1alpha3/conversion.go index cd7c1913a743..124b8933b39a 100644 --- a/controlplane/kubeadm/api/v1alpha3/conversion.go +++ b/controlplane/kubeadm/api/v1alpha3/conversion.go @@ -39,6 +39,7 @@ func (src *KubeadmControlPlane) ConvertTo(dstRaw conversion.Hub) error { } dst.Spec.MachineTemplate.ObjectMeta = restored.Spec.MachineTemplate.ObjectMeta + dst.Spec.MachineTemplate.NodeDeletionTimeout = restored.Spec.MachineTemplate.NodeDeletionTimeout dst.Status.Version = restored.Status.Version if restored.Spec.KubeadmConfigSpec.JoinConfiguration != nil && restored.Spec.KubeadmConfigSpec.JoinConfiguration.NodeRegistration.IgnorePreflightErrors != nil { diff --git a/controlplane/kubeadm/api/v1alpha4/conversion.go b/controlplane/kubeadm/api/v1alpha4/conversion.go index 7147736f29d9..61eeec1a4fa5 100644 --- a/controlplane/kubeadm/api/v1alpha4/conversion.go +++ b/controlplane/kubeadm/api/v1alpha4/conversion.go @@ -46,6 +46,8 @@ func (src *KubeadmControlPlane) ConvertTo(dstRaw conversion.Hub) error { dst.Spec.KubeadmConfigSpec.JoinConfiguration.Patches = restored.Spec.KubeadmConfigSpec.JoinConfiguration.Patches } + dst.Spec.MachineTemplate.NodeDeletionTimeout = restored.Spec.MachineTemplate.NodeDeletionTimeout + return nil } @@ -93,6 +95,11 @@ func (src *KubeadmControlPlaneTemplate) ConvertTo(dstRaw conversion.Hub) error { if restored.Spec.Template.Spec.KubeadmConfigSpec.JoinConfiguration != nil { dst.Spec.Template.Spec.KubeadmConfigSpec.JoinConfiguration.Patches = restored.Spec.Template.Spec.KubeadmConfigSpec.JoinConfiguration.Patches } + if dst.Spec.Template.Spec.MachineTemplate == nil { + dst.Spec.Template.Spec.MachineTemplate = restored.Spec.Template.Spec.MachineTemplate + } else if restored.Spec.Template.Spec.MachineTemplate != nil { + dst.Spec.Template.Spec.MachineTemplate.NodeDeletionTimeout = restored.Spec.Template.Spec.MachineTemplate.NodeDeletionTimeout + } return nil } @@ -175,3 +182,8 @@ func Convert_v1beta1_KubeadmControlPlaneTemplateResourceSpec_To_v1alpha4_Kubeadm return nil } + +func Convert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmControlPlaneMachineTemplate(in *controlplanev1.KubeadmControlPlaneMachineTemplate, out *KubeadmControlPlaneMachineTemplate, s apiconversion.Scope) error { + // .NodeDrainTimeout was added in v1beta1. + return autoConvert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmControlPlaneMachineTemplate(in, out, s) +} diff --git a/controlplane/kubeadm/api/v1alpha4/zz_generated.conversion.go b/controlplane/kubeadm/api/v1alpha4/zz_generated.conversion.go index eec854984534..8e8241b7e5b4 100644 --- a/controlplane/kubeadm/api/v1alpha4/zz_generated.conversion.go +++ b/controlplane/kubeadm/api/v1alpha4/zz_generated.conversion.go @@ -67,11 +67,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta1.KubeadmControlPlaneMachineTemplate)(nil), (*KubeadmControlPlaneMachineTemplate)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmControlPlaneMachineTemplate(a.(*v1beta1.KubeadmControlPlaneMachineTemplate), b.(*KubeadmControlPlaneMachineTemplate), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*KubeadmControlPlaneSpec)(nil), (*v1beta1.KubeadmControlPlaneSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha4_KubeadmControlPlaneSpec_To_v1beta1_KubeadmControlPlaneSpec(a.(*KubeadmControlPlaneSpec), b.(*v1beta1.KubeadmControlPlaneSpec), scope) }); err != nil { @@ -157,6 +152,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta1.KubeadmControlPlaneMachineTemplate)(nil), (*KubeadmControlPlaneMachineTemplate)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmControlPlaneMachineTemplate(a.(*v1beta1.KubeadmControlPlaneMachineTemplate), b.(*KubeadmControlPlaneMachineTemplate), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1beta1.KubeadmControlPlaneTemplateResourceSpec)(nil), (*KubeadmControlPlaneSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_KubeadmControlPlaneTemplateResourceSpec_To_v1alpha4_KubeadmControlPlaneSpec(a.(*v1beta1.KubeadmControlPlaneTemplateResourceSpec), b.(*KubeadmControlPlaneSpec), scope) }); err != nil { @@ -259,14 +259,10 @@ func autoConvert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmC } out.InfrastructureRef = in.InfrastructureRef out.NodeDrainTimeout = (*v1.Duration)(unsafe.Pointer(in.NodeDrainTimeout)) + // WARNING: in.NodeDeletionTimeout requires manual conversion: does not exist in peer-type return nil } -// Convert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmControlPlaneMachineTemplate is an autogenerated conversion function. -func Convert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmControlPlaneMachineTemplate(in *v1beta1.KubeadmControlPlaneMachineTemplate, out *KubeadmControlPlaneMachineTemplate, s conversion.Scope) error { - return autoConvert_v1beta1_KubeadmControlPlaneMachineTemplate_To_v1alpha4_KubeadmControlPlaneMachineTemplate(in, out, s) -} - func autoConvert_v1alpha4_KubeadmControlPlaneSpec_To_v1beta1_KubeadmControlPlaneSpec(in *KubeadmControlPlaneSpec, out *v1beta1.KubeadmControlPlaneSpec, s conversion.Scope) error { out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) out.Version = in.Version diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go index e7e41c19136f..46d81813ea8f 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_types.go @@ -101,6 +101,12 @@ type KubeadmControlPlaneMachineTemplate struct { // NOTE: NodeDrainTimeout is different from `kubectl drain --timeout` // +optional NodeDrainTimeout *metav1.Duration `json:"nodeDrainTimeout,omitempty"` + + // NodeDeletionTimeout defines how long the machine controller will attempt to delete the Node that the Machine + // hosts after the Machine is marked for deletion. A duration of 0 will retry deletion indefinitely. + // If no value is provided, the default value for this property of the Machine resource will be used. + // +optional + NodeDeletionTimeout *metav1.Duration `json:"nodeDeletionTimeout,omitempty"` } // RolloutStrategy describes how to replace existing machines diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go index 896ef5d0e36c..4a7543a0e67f 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook.go @@ -152,6 +152,7 @@ func (in *KubeadmControlPlane) ValidateUpdate(old runtime.Object) error { {spec, "machineTemplate", "infrastructureRef", "apiVersion"}, {spec, "machineTemplate", "infrastructureRef", "name"}, {spec, "machineTemplate", "nodeDrainTimeout"}, + {spec, "machineTemplate", "nodeDeletionTimeout"}, {spec, "replicas"}, {spec, "version"}, {spec, "rolloutAfter"}, diff --git a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go index 9923c95274b9..19d23538fccc 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadm_control_plane_webhook_test.go @@ -239,7 +239,8 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { Namespace: "foo", Name: "infraTemplate", }, - NodeDrainTimeout: &metav1.Duration{Duration: time.Second}, + NodeDrainTimeout: &metav1.Duration{Duration: time.Second}, + NodeDeletionTimeout: &metav1.Duration{Duration: time.Second}, }, Replicas: pointer.Int32Ptr(1), RolloutStrategy: &RolloutStrategy{ @@ -365,6 +366,7 @@ func TestKubeadmControlPlaneValidateUpdate(t *testing.T) { validUpdate.Spec.MachineTemplate.InfrastructureRef.APIVersion = "test/v1alpha2" validUpdate.Spec.MachineTemplate.InfrastructureRef.Name = "orange" validUpdate.Spec.MachineTemplate.NodeDrainTimeout = &metav1.Duration{Duration: 10 * time.Second} + validUpdate.Spec.MachineTemplate.NodeDeletionTimeout = &metav1.Duration{Duration: 10 * time.Second} validUpdate.Spec.Replicas = pointer.Int32Ptr(5) now := metav1.NewTime(time.Now()) validUpdate.Spec.RolloutAfter = &now diff --git a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go index e5e3e15f2862..575b6b8e0771 100644 --- a/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go +++ b/controlplane/kubeadm/api/v1beta1/kubeadmcontrolplanetemplate_types.go @@ -99,4 +99,10 @@ type KubeadmControlPlaneTemplateMachineTemplate struct { // NOTE: NodeDrainTimeout is different from `kubectl drain --timeout` // +optional NodeDrainTimeout *metav1.Duration `json:"nodeDrainTimeout,omitempty"` + + // NodeDeletionTimeout defines how long the machine controller will attempt to delete the Node that the Machine + // hosts after the Machine is marked for deletion. A duration of 0 will retry deletion indefinitely. + // If no value is provided, the default value for this property of the Machine resource will be used. + // +optional + NodeDeletionTimeout *metav1.Duration `json:"nodeDeletionTimeout,omitempty"` } diff --git a/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go b/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go index 15fac74acc5d..ff16517906e7 100644 --- a/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go +++ b/controlplane/kubeadm/api/v1beta1/zz_generated.deepcopy.go @@ -97,6 +97,11 @@ func (in *KubeadmControlPlaneMachineTemplate) DeepCopyInto(out *KubeadmControlPl *out = new(v1.Duration) **out = **in } + if in.NodeDeletionTimeout != nil { + in, out := &in.NodeDeletionTimeout, &out.NodeDeletionTimeout + *out = new(v1.Duration) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubeadmControlPlaneMachineTemplate. @@ -238,6 +243,11 @@ func (in *KubeadmControlPlaneTemplateMachineTemplate) DeepCopyInto(out *KubeadmC *out = new(v1.Duration) **out = **in } + if in.NodeDeletionTimeout != nil { + in, out := &in.NodeDeletionTimeout, &out.NodeDeletionTimeout + *out = new(v1.Duration) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubeadmControlPlaneTemplateMachineTemplate. 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 bdd6b0cef72e..9a5a779a5183 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 @@ -3510,6 +3510,14 @@ spec: More info: http://kubernetes.io/docs/user-guide/labels' type: object type: object + nodeDeletionTimeout: + description: NodeDeletionTimeout defines how long the machine + controller will attempt to delete the Node that the Machine + hosts after the Machine is marked for deletion. A duration of + 0 will retry deletion indefinitely. If no value is provided, + the default value for this property of the Machine resource + will be used. + type: string nodeDrainTimeout: description: 'NodeDrainTimeout is the total amount of time that the controller will spend on draining a controlplane node The diff --git a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml index fabde6b0f911..eaf234e59b9d 100644 --- a/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml +++ b/controlplane/kubeadm/config/crd/bases/controlplane.cluster.x-k8s.io_kubeadmcontrolplanetemplates.yaml @@ -2277,6 +2277,14 @@ spec: machines should be shaped when creating or updating a control plane. properties: + nodeDeletionTimeout: + description: NodeDeletionTimeout defines how long the + machine controller will attempt to delete the Node that + the Machine hosts after the Machine is marked for deletion. + A duration of 0 will retry deletion indefinitely. If + no value is provided, the default value for this property + of the Machine resource will be used. + type: string nodeDrainTimeout: description: 'NodeDrainTimeout is the total amount of time that the controller will spend on draining a controlplane diff --git a/controlplane/kubeadm/internal/controllers/helpers.go b/controlplane/kubeadm/internal/controllers/helpers.go index be9fa5a70af3..31034141ca9f 100644 --- a/controlplane/kubeadm/internal/controllers/helpers.go +++ b/controlplane/kubeadm/internal/controllers/helpers.go @@ -289,6 +289,9 @@ func (r *KubeadmControlPlaneReconciler) generateMachine(ctx context.Context, kcp NodeDrainTimeout: kcp.Spec.MachineTemplate.NodeDrainTimeout, }, } + if kcp.Spec.MachineTemplate.NodeDeletionTimeout != nil { + machine.Spec.NodeDeletionTimeout = kcp.Spec.MachineTemplate.NodeDeletionTimeout + } // Machine's bootstrap config may be missing ClusterConfiguration if it is not the first machine in the control plane. // We store ClusterConfiguration as annotation here to detect any changes in KCP ClusterConfiguration and rollout the machine if any. diff --git a/docs/book/src/developer/architecture/controllers/control-plane.md b/docs/book/src/developer/architecture/controllers/control-plane.md index 5525561c4857..ffc70ccb80e6 100644 --- a/docs/book/src/developer/architecture/controllers/control-plane.md +++ b/docs/book/src/developer/architecture/controllers/control-plane.md @@ -97,7 +97,12 @@ documentation][scale]. * `machineTemplate.nodeDrainTimeout` - is a *metav1.Duration defining the total amount of time that the controller will spend on draining a control plane node. The default value is 0, meaning that the node can be drained without any time limitations. - + +* `machineTemplate.nodeDeletionTimeout` - is a *metav1.Duration defining how long the controller + will attempt to delete the Node that is hosted by a Machine after the Machine is marked for + deletion. A duration of 0 will retry deletion indefinitely. It defaults to 10 seconds on the + Machine. + #### Required `status` fields The `ImplementationControlPlane` object **must** have a `status` object. diff --git a/exp/api/v1alpha3/conversion.go b/exp/api/v1alpha3/conversion.go index 47cedb064ccf..a06e3994d6a9 100644 --- a/exp/api/v1alpha3/conversion.go +++ b/exp/api/v1alpha3/conversion.go @@ -22,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/conversion" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" + utilconversion "sigs.k8s.io/cluster-api/util/conversion" ) // Convert_v1alpha3_MachinePoolSpec_To_v1beta1_MachinePoolSpec is an autogenerated conversion function. @@ -60,13 +61,27 @@ func Convert_v1beta1_MachinePool_To_v1alpha3_MachinePool(in *expv1.MachinePool, func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*expv1.MachinePool) - return Convert_v1alpha3_MachinePool_To_v1beta1_MachinePool(src, dst, nil) + if err := Convert_v1alpha3_MachinePool_To_v1beta1_MachinePool(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &expv1.MachinePool{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout + return nil } func (dst *MachinePool) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*expv1.MachinePool) - return Convert_v1beta1_MachinePool_To_v1alpha3_MachinePool(src, dst, nil) + if err := Convert_v1beta1_MachinePool_To_v1alpha3_MachinePool(src, dst, nil); err != nil { + return err + } + + return utilconversion.MarshalData(src, dst) } func (src *MachinePoolList) ConvertTo(dstRaw conversion.Hub) error { diff --git a/exp/api/v1alpha4/conversion.go b/exp/api/v1alpha4/conversion.go index 609d15cff4b5..a9408d3cf6af 100644 --- a/exp/api/v1alpha4/conversion.go +++ b/exp/api/v1alpha4/conversion.go @@ -20,18 +20,32 @@ import ( "sigs.k8s.io/controller-runtime/pkg/conversion" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" + utilconversion "sigs.k8s.io/cluster-api/util/conversion" ) func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error { dst := dstRaw.(*expv1.MachinePool) - return Convert_v1alpha4_MachinePool_To_v1beta1_MachinePool(src, dst, nil) + if err := Convert_v1alpha4_MachinePool_To_v1beta1_MachinePool(src, dst, nil); err != nil { + return err + } + + // Manually restore data. + restored := &expv1.MachinePool{} + if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok { + return err + } + dst.Spec.Template.Spec.NodeDeletionTimeout = restored.Spec.Template.Spec.NodeDeletionTimeout + return nil } func (dst *MachinePool) ConvertFrom(srcRaw conversion.Hub) error { src := srcRaw.(*expv1.MachinePool) - return Convert_v1beta1_MachinePool_To_v1alpha4_MachinePool(src, dst, nil) + if err := Convert_v1beta1_MachinePool_To_v1alpha4_MachinePool(src, dst, nil); err != nil { + return err + } + return utilconversion.MarshalData(src, dst) } func (src *MachinePoolList) ConvertTo(dstRaw conversion.Hub) error { diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index 4ff747c76e90..9da2cd58c854 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -86,6 +86,10 @@ type Reconciler struct { controller controller.Controller recorder record.EventRecorder externalTracker external.ObjectTracker + + // nodeDeletionRetryTimeout determines how long the controller will retry deleting a node + // during a single reconciliation. + nodeDeletionRetryTimeout time.Duration } func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { @@ -94,6 +98,10 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt return err } + if r.nodeDeletionRetryTimeout.Nanoseconds() == 0 { + r.nodeDeletionRetryTimeout = 10 * time.Second + } + controller, err := ctrl.NewControllerManagedBy(mgr). For(&clusterv1.Machine{}). WithOptions(options). @@ -380,16 +388,22 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu log.Info("Deleting node", "node", m.Status.NodeRef.Name) var deleteNodeErr error - waitErr := wait.PollImmediate(2*time.Second, 10*time.Second, func() (bool, error) { + waitErr := wait.PollImmediate(2*time.Second, r.nodeDeletionRetryTimeout, func() (bool, error) { if deleteNodeErr = r.deleteNode(ctx, cluster, m.Status.NodeRef.Name); deleteNodeErr != nil && !apierrors.IsNotFound(errors.Cause(deleteNodeErr)) { return false, nil } return true, nil }) if waitErr != nil { - log.Error(deleteNodeErr, "Timed out deleting node, moving on", "node", m.Status.NodeRef.Name) + log.Error(deleteNodeErr, "Timed out deleting node", "node", m.Status.NodeRef.Name) conditions.MarkFalse(m, clusterv1.MachineNodeHealthyCondition, clusterv1.DeletionFailedReason, clusterv1.ConditionSeverityWarning, "") r.recorder.Eventf(m, corev1.EventTypeWarning, "FailedDeleteNode", "error deleting Machine's node: %v", deleteNodeErr) + + // If the node deletion timeout is not expired yet, requeue the Machine for reconciliation. + if m.Spec.NodeDeletionTimeout == nil || m.Spec.NodeDeletionTimeout.Nanoseconds() == 0 || m.DeletionTimestamp.Add(m.Spec.NodeDeletionTimeout.Duration).After(time.Now()) { + return ctrl.Result{}, deleteNodeErr + } + log.Info("Node deletion timeout expired, continuing without Node deletion.") } } diff --git a/internal/controllers/machine/machine_controller_test.go b/internal/controllers/machine/machine_controller_test.go index 3d9f7c38a729..ae494f459ac8 100644 --- a/internal/controllers/machine/machine_controller_test.go +++ b/internal/controllers/machine/machine_controller_test.go @@ -17,6 +17,8 @@ limitations under the License. package machine import ( + "context" + "fmt" "testing" "time" @@ -27,9 +29,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" "k8s.io/utils/pointer" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -1856,6 +1860,193 @@ func TestNodeToMachine(t *testing.T) { } } +type fakeClientWithNodeDeletionErr struct { + client.Client +} + +func (fc fakeClientWithNodeDeletionErr) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { + gvk, err := apiutil.GVKForObject(obj, fakeScheme) + if err == nil && gvk.Kind == "Node" { + return fmt.Errorf("fake error") + } + return fc.Client.Delete(ctx, obj, opts...) +} + +func TestNodeDeletion(t *testing.T) { + g := NewWithT(t) + + deletionTime := metav1.Now().Add(-1 * time.Second) + + testCluster := clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Namespace: metav1.NamespaceDefault, + }, + } + + node := &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + Spec: corev1.NodeSpec{ProviderID: "test://id-1"}, + } + + testMachine := clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{ + clusterv1.MachineControlPlaneLabelName: "", + }, + Annotations: map[string]string{ + "machine.cluster.x-k8s.io/exclude-node-draining": "", + }, + Finalizers: []string{clusterv1.MachineFinalizer}, + DeletionTimestamp: &metav1.Time{Time: deletionTime}, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{ + APIVersion: "infrastructure.cluster.x-k8s.io/v1beta1", + Kind: "GenericInfrastructureMachine", + Name: "infra-config1", + }, + Bootstrap: clusterv1.Bootstrap{DataSecretName: pointer.StringPtr("data")}, + }, + Status: clusterv1.MachineStatus{ + NodeRef: &corev1.ObjectReference{ + Name: "test", + }, + }, + } + + cpmachine1 := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cp1", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{ + clusterv1.ClusterLabelName: "test-cluster", + clusterv1.MachineControlPlaneLabelName: "", + }, + Finalizers: []string{clusterv1.MachineFinalizer}, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{}, + Bootstrap: clusterv1.Bootstrap{DataSecretName: pointer.StringPtr("data")}, + }, + Status: clusterv1.MachineStatus{ + NodeRef: &corev1.ObjectReference{ + Name: "cp1", + }, + }, + } + + testCases := []struct { + name string + deletionTimeout *metav1.Duration + resultErr bool + clusterDeleted bool + expectNodeDeletion bool + createFakeClient func(...client.Object) client.Client + }{ + { + name: "should return no error when deletion is successful", + deletionTimeout: &metav1.Duration{Duration: time.Second}, + resultErr: false, + expectNodeDeletion: true, + createFakeClient: func(initObjs ...client.Object) client.Client { + return fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + }, + }, + { + name: "should return an error when timeout is not expired and node deletion fails", + deletionTimeout: &metav1.Duration{Duration: time.Hour}, + resultErr: true, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + { + name: "should return an error when timeout is infinite and node deletion fails", + deletionTimeout: &metav1.Duration{Duration: 0}, // should lead to infinite timeout + resultErr: true, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + { + name: "should not return an error when timeout is expired and node deletion fails", + deletionTimeout: &metav1.Duration{Duration: time.Millisecond}, + resultErr: false, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + { + name: "should not delete the node or return an error when the cluster is marked for deletion", + deletionTimeout: nil, // should lead to infinite timeout + resultErr: false, + clusterDeleted: true, + expectNodeDeletion: false, + createFakeClient: func(initObjs ...client.Object) client.Client { + fc := fake.NewClientBuilder(). + WithObjects(initObjs...). + Build() + return fakeClientWithNodeDeletionErr{fc} + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + m := testMachine.DeepCopy() + m.Spec.NodeDeletionTimeout = tc.deletionTimeout + + fakeClient := tc.createFakeClient(node, m, cpmachine1) + tracker := remote.NewTestClusterCacheTracker(ctrl.Log, fakeClient, fakeScheme, client.ObjectKeyFromObject(&testCluster)) + + r := &Reconciler{ + Client: fakeClient, + Tracker: tracker, + recorder: record.NewFakeRecorder(10), + nodeDeletionRetryTimeout: 10 * time.Millisecond, + } + + cluster := testCluster.DeepCopy() + if tc.clusterDeleted { + cluster.DeletionTimestamp = &metav1.Time{Time: deletionTime.Add(time.Hour)} + } + + _, err := r.reconcileDelete(context.Background(), cluster, m) + + if tc.resultErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + if tc.expectNodeDeletion { + n := &corev1.Node{} + g.Expect(fakeClient.Get(context.Background(), client.ObjectKeyFromObject(node), n)).NotTo(Succeed()) + } + } + }) + } +} + // adds a condition list to an external object. func addConditionsToExternal(u *unstructured.Unstructured, newConditions clusterv1.Conditions) { existingConditions := clusterv1.Conditions{}