add nodeDrainTimeout to Machine and rety indefinitely

The machine controller now uses the configured timeout when deleting
the Node belonging to a Machine. It will also requeue the Machine for
reconciliation if deletion fails, which will lead to infinite retries.

Author: schrej

api/v1alpha3/conversion.go

@@ -96,6 +96,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
 }
@@ -138,6 +139,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
 }
@@ -191,6 +193,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
 }
@@ -303,6 +306,11 @@ func Convert_v1beta1_MachineStatus_To_v1alpha3_MachineStatus(in *v1beta1.Machine
 	return autoConvert_v1beta1_MachineStatus_To_v1alpha3_MachineStatus(in, out, s)
 }
 
+func Convert_v1beta1_MachineSpec_To_v1alpha3_MachineSpec(in *v1beta1.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 *v1beta1.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)

api/v1alpha3/zz_generated.conversion.go

@@ -119,13 +119,33 @@ func (dst *ClusterClassList) ConvertFrom(srcRaw conversion.Hub) error {
 func (src *Machine) ConvertTo(dstRaw conversion.Hub) error {
 	dst := dstRaw.(*v1beta1.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 := &v1beta1.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.(*v1beta1.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 {
@@ -143,13 +163,29 @@ func (dst *MachineList) ConvertFrom(srcRaw conversion.Hub) error {
 func (src *MachineSet) ConvertTo(dstRaw conversion.Hub) error {
 	dst := dstRaw.(*v1beta1.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 := &v1beta1.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.(*v1beta1.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 {
@@ -167,13 +203,29 @@ func (dst *MachineSetList) ConvertFrom(srcRaw conversion.Hub) error {
 func (src *MachineDeployment) ConvertTo(dstRaw conversion.Hub) error {
 	dst := dstRaw.(*v1beta1.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 := &v1beta1.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.(*v1beta1.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 {
@@ -222,6 +274,11 @@ func Convert_v1beta1_ClusterClassSpec_To_v1alpha4_ClusterClassSpec(in *v1beta1.C
 	return autoConvert_v1beta1_ClusterClassSpec_To_v1alpha4_ClusterClassSpec(in, out, s)
 }
 
+func Convert_v1beta1_MachineSpec_To_v1alpha4_MachineSpec(in *v1beta1.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 *v1beta1.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)

api/v1alpha4/conversion.go

@@ -95,6 +95,10 @@ type MachineSpec struct {
 	// NOTE: NodeDrainTimeout is different from `kubectl drain --timeout`
 	// +optional
 	NodeDrainTimeout *metav1.Duration `json:"nodeDrainTimeout,omitempty"`
+
+	// NodeDeletionTimeout is the timeout used when deleting the Node belonging to a Machine from its
+	// cluster's api. The default timeout is 10 seconds.
+	NodeDeletionTimeout *metav1.Duration `json:"nodeDeletionTimeout,omitempty"`
 }
 
 // ANCHOR_END: MachineSpec

api/v1alpha4/zz_generated.conversion.go

@@ -19,8 +19,10 @@ package v1beta1
 import (
 	"fmt"
 	"strings"
+	"time"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/cluster-api/util/version"
@@ -59,6 +61,10 @@ func (m *Machine) Default() {
 		normalizedVersion := "v" + *m.Spec.Version
 		m.Spec.Version = &normalizedVersion
 	}
+
+	if m.Spec.NodeDeletionTimeout == nil {
+		m.Spec.NodeDeletionTimeout = &v1.Duration{Duration: 10 * time.Millisecond}
+	}
 }
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.

api/v1beta1/machine_types.go

@@ -36,24 +36,33 @@ spec:
         description: Provider defines an entry in the provider inventory.
         properties:
           apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+            description: 'APIVersion defines the versioned schema of this representation
+              of an object. Servers should convert recognized schemas to the latest
+              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
             type: string
           kind:
-            description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+            description: 'Kind is a string value representing the REST resource this
+              object represents. Servers may infer this from the endpoint the client
+              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
             type: string
           metadata:
             type: object
           providerName:
             description: ProviderName indicates the name of the provider.
             type: string
           type:
-            description: Type indicates the type of the provider. See ProviderType for a list of supported values
+            description: Type indicates the type of the provider. See ProviderType
+              for a list of supported values
             type: string
           version:
             description: Version indicates the component version.
             type: string
           watchedNamespace:
-            description: 'WatchedNamespace indicates the namespace where the provider controller is is watching. if empty the provider controller is watching for objects in all namespaces. Deprecated: in clusterctl v1alpha4 all the providers watch all the namespaces; this field will be removed in a future version of this API'
+            description: 'WatchedNamespace indicates the namespace where the provider
+              controller is is watching. if empty the provider controller is watching
+              for objects in all namespaces. Deprecated: in clusterctl v1alpha4 all
+              the providers watch all the namespaces; this field will be removed in
+              a future version of this API'
             type: string
         type: object
     served: true

api/v1beta1/machine_webhook.go

@@ -1285,6 +1285,11 @@ 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 is the timeout used when
+                          deleting the Node belonging to a Machine from its cluster's
+                          api. The default timeout is 10 seconds.
+                        type: string
                       nodeDrainTimeout:
                         description: 'NodeDrainTimeout is the total amount of time
                           that the controller will spend on draining a node. The default