Adds Oldest and Newest delete policies and annotation-based Simple delete policy

config/crds/cluster_v1alpha1_machine.yaml

@@ -47,15 +47,15 @@ spec:
                 by the provider. This field must match the provider ID as seen on
                 the node object corresponding to this machine. This field is required
                 by higher level consumers of cluster-api. Example use case is cluster
-                autoscaler with cluster-api as provider. Clean-up login in the autoscaler
-                compares machines v/s nodes to find out machines at provider which
+                autoscaler with cluster-api as provider. Clean-up logic in the autoscaler
+                compares machines to nodes to find out machines at provider which
                 could not get registered as Kubernetes nodes. With cluster-api as
                 a generic out-of-tree provider for autoscaler, this field is required
                 by autoscaler to be able to have a provider view of the list of machines.
-                Another list of nodes is queries from the k8s apiserver and then comparison
+                Another list of nodes is queried from the k8s apiserver and then comparison
                 is done to find out unregistered machines and are marked for delete.
                 This field will be set by the actuators and consumed by higher level
-                entities like autoscaler  who will be interfacing with cluster-api
+                entities like autoscaler that will be interfacing with cluster-api
                 as generic provider.
               type: string
             providerSpec:

config/crds/cluster_v1alpha1_machinedeployment.yaml

@@ -142,16 +142,16 @@ spec:
                         ID as seen on the node object corresponding to this machine.
                         This field is required by higher level consumers of cluster-api.
                         Example use case is cluster autoscaler with cluster-api as
-                        provider. Clean-up login in the autoscaler compares machines
-                        v/s nodes to find out machines at provider which could not
+                        provider. Clean-up logic in the autoscaler compares machines
+                        to nodes to find out machines at provider which could not
                         get registered as Kubernetes nodes. With cluster-api as a
                         generic out-of-tree provider for autoscaler, this field is
                         required by autoscaler to be able to have a provider view
-                        of the list of machines. Another list of nodes is queries
-                        from the k8s apiserver and then comparison is done to find
+                        of the list of machines. Another list of nodes is queried
+                        from the k8s apiserver and then a comparison is done to find
                         out unregistered machines and are marked for delete. This
                         field will be set by the actuators and consumed by higher
-                        level entities like autoscaler  who will be interfacing with
+                        level entities like autoscaler that will be interfacing with
                         cluster-api as generic provider.
                       type: string
                     providerSpec:

config/crds/cluster_v1alpha1_machineset.yaml

@@ -34,6 +34,15 @@ spec:
           type: object
         spec:
           properties:
+            deletePolicy:
+              description: DeletePolicy defines the policy used to identify nodes
+                to delete when downscaling. Defaults to "Random".  Valid values are
+                "Random, "Newest", "Oldest"
+              enum:
+              - Random
+              - Newest
+              - Oldest
+              type: string
             minReadySeconds:
               description: MinReadySeconds is the minimum number of seconds for which
                 a newly created machine should be ready. Defaults to 0 (machine will
@@ -82,16 +91,16 @@ spec:
                         ID as seen on the node object corresponding to this machine.
                         This field is required by higher level consumers of cluster-api.
                         Example use case is cluster autoscaler with cluster-api as
-                        provider. Clean-up login in the autoscaler compares machines
-                        v/s nodes to find out machines at provider which could not
+                        provider. Clean-up logic in the autoscaler compares machines
+                        to nodes to find out machines at provider which could not
                         get registered as Kubernetes nodes. With cluster-api as a
                         generic out-of-tree provider for autoscaler, this field is
                         required by autoscaler to be able to have a provider view
-                        of the list of machines. Another list of nodes is queries
-                        from the k8s apiserver and then comparison is done to find
+                        of the list of machines. Another list of nodes is queried
+                        from the k8s apiserver and then a comparison is done to find
                         out unregistered machines and are marked for delete. This
                         field will be set by the actuators and consumed by higher
-                        level entities like autoscaler  who will be interfacing with
+                        level entities like autoscaler that will be interfacing with
                         cluster-api as generic provider.
                       type: string
                     providerSpec:

pkg/apis/cluster/v1alpha1/machine_types.go

@@ -87,12 +87,12 @@ type MachineSpec struct {
 	// ProviderID is the identification ID of the machine provided by the provider.
 	// This field must match the provider ID as seen on the node object corresponding to this machine.
 	// This field is required by higher level consumers of cluster-api. Example use case is cluster autoscaler
-	// with cluster-api as provider. Clean-up login in the autoscaler compares machines v/s nodes to find out
+	// with cluster-api as provider. Clean-up logic in the autoscaler compares machines to nodes to find out
 	// machines at provider which could not get registered as Kubernetes nodes. With cluster-api as a
 	// generic out-of-tree provider for autoscaler, this field is required by autoscaler to be
-	// able to have a provider view of the list of machines. Another list of nodes is queries from the k8s apiserver
-	// and then comparison is done to find out unregistered machines and are marked for delete.
-	// This field will be set by the actuators and consumed by higher level entities like autoscaler  who will
+	// able to have a provider view of the list of machines. Another list of nodes is queried from the k8s apiserver
+	// and then a comparison is done to find out unregistered machines and are marked for delete.
+	// This field will be set by the actuators and consumed by higher level entities like autoscaler that will
 	// be interfacing with cluster-api as generic provider.
 	// +optional
 	ProviderID *string `json:"providerID,omitempty"`

pkg/apis/cluster/v1alpha1/machineset_types.go

@@ -58,6 +58,11 @@ type MachineSetSpec struct {
 	// +optional
 	MinReadySeconds int32 `json:"minReadySeconds,omitempty"`
 
+	// DeletePolicy defines the policy used to identify nodes to delete when downscaling.
+	// Defaults to "Random".  Valid values are "Random, "Newest", "Oldest"
+	// +kubebuilder:validation:Enum=Random,Newest,Oldest
+	DeletePolicy string `json:"deletePolicy,omitempty"`
+
 	// Selector is a label query over machines that should match the replica count.
 	// Label keys and values that must match in order to be controlled by this MachineSet.
 	// It must match the machine template's labels.
@@ -70,6 +75,30 @@ type MachineSetSpec struct {
 	Template MachineTemplateSpec `json:"template,omitempty"`
 }
 
+// MachineSetDeletePolicy defines how priority is assigned to nodes to delete when
+// downscaling a MachineSet. Defaults to "Random".
+type MachineSetDeletePolicy string
+
+const (
+	// RandomMachineSetDeletePolicy prioritizes both Machines that have the annotation
+	// "cluster.k8s.io/delete-machine=yes" and Machines that are unhealthy
+	// (Status.ErrorReason or Status.ErrorMessage are set to a non-empty value).
+	// Finally, it picks Machines at random to delete.
+	RandomMachineSetDeletePolicy MachineSetDeletePolicy = "Random"
+
+	// NewestMachineSetDeletePolicy prioritizes both Machines that have the annotation
+	// "cluster.k8s.io/delete-machine=yes" and Machines that are unhealthy
+	// (Status.ErrorReason or Status.ErrorMessage are set to a non-empty value).
+	// It then prioritizes the newest Machines for deletion based on the Machine's CreationTimestamp.
+	NewestMachineSetDeletePolicy MachineSetDeletePolicy = "Newest"
+
+	// OldestMachineSetDeletePolicy prioritizes both Machines that have the annotation
+	// "cluster.k8s.io/delete-machine=yes" and Machines that are unhealthy
+	// (Status.ErrorReason or Status.ErrorMessage are set to a non-empty value).
+	// It then prioritizes the oldest Machines for deletion based on the Machine's CreationTimestamp.
+	OldestMachineSetDeletePolicy MachineSetDeletePolicy = "Oldest"
+)
+
 /// [MachineSetSpec] // doxygen marker
 
 /// [MachineTemplateSpec] // doxygen marker
@@ -170,6 +199,12 @@ func (m *MachineSet) Default() {
 	if len(m.Namespace) == 0 {
 		m.Namespace = metav1.NamespaceDefault
 	}
+
+	if m.Spec.DeletePolicy == "" {
+		randomPolicy := string(RandomMachineSetDeletePolicy)
+		log.Printf("Defaulting to %s\n", randomPolicy)
+		m.Spec.DeletePolicy = randomPolicy
+	}
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/apis/cluster/v1alpha1/machineset_types_test.go

@@ -64,3 +64,14 @@ func TestStorageMachineSet(t *testing.T) {
 		t.Error("expected error getting machineset")
 	}
 }
+
+func TestDefaults(t *testing.T) {
+	ms := &MachineSet{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}
+	ms.Default()
+
+	expected := string(RandomMachineSetDeletePolicy)
+	got := ms.Spec.DeletePolicy
+	if got != expected {
+		t.Errorf("expected default machineset delete policy '%s', got '%s'", expected, got)
+	}
+}

pkg/controller/machineset/BUILD.bazel

@@ -43,6 +43,7 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//pkg/apis:go_default_library",
+        "//pkg/apis/cluster/common:go_default_library",
         "//pkg/apis/cluster/v1alpha1:go_default_library",
         "//vendor/golang.org/x/net/context:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

pkg/controller/machineset/controller.go

@@ -329,8 +329,13 @@ func (r *ReconcileMachineSet) syncReplicas(ms *clusterv1alpha1.MachineSet, machi
 		klog.Infof("Too many replicas for %v %s/%s, need %d, deleting %d",
 			controllerKind, ms.Namespace, ms.Name, *(ms.Spec.Replicas), diff)
 
+		deletePriorityFunc, err := getDeletePriorityFunc(ms)
+		if err != nil {
+			return err
+		}
+		klog.Infof("Found %s delete policy", ms.Spec.DeletePolicy)
 		// Choose which Machines to delete.
-		machinesToDelete := getMachinesToDeletePrioritized(machines, diff, simpleDeletePriority)
+		machinesToDelete := getMachinesToDeletePrioritized(machines, diff, deletePriorityFunc)
 
 		// TODO: Add cap to limit concurrent delete calls.
 		errCh := make(chan error, diff)

pkg/controller/machineset/delete_policy.go

@@ -17,57 +17,118 @@ limitations under the License.
 package machineset
 
 import (
+	"math"
+	"sort"
+
+	"github.com/pkg/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1"
 )
 
-type deletePriority int
+type deletePriority float64
 
 const (
-	mustDelete   deletePriority = 100
-	betterDelete deletePriority = 50
-	couldDelete  deletePriority = 20
-	// mustNotDelete deletePriority = 0
+
+	// DeleteNodeAnnotation marks nodes that will be given priority for deletion
+	// when a machineset scales down. This annotation is given top priority on all delete policies.
+	DeleteNodeAnnotation = "cluster.k8s.io/delete-machine"
+
+	mustDelete    deletePriority = 100.0
+	betterDelete  deletePriority = 50.0
+	couldDelete   deletePriority = 20.0
+	mustNotDelete deletePriority = 0.0
+
+	secondsPerTenDays float64 = 864000
 )
 
 type deletePriorityFunc func(machine *v1alpha1.Machine) deletePriority
 
-func simpleDeletePriority(machine *v1alpha1.Machine) deletePriority {
+// maps the creation timestamp onto the 0-100 priority range
+func oldestDeletePriority(machine *v1alpha1.Machine) deletePriority {
+	if machine.DeletionTimestamp != nil && !machine.DeletionTimestamp.IsZero() {
+		return mustDelete
+	}
+	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
+		return mustDelete
+	}
+	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
+		return mustDelete
+	}
+	if machine.ObjectMeta.CreationTimestamp.Time.IsZero() {
+		return mustNotDelete
+	}
+	d := metav1.Now().Sub(machine.ObjectMeta.CreationTimestamp.Time)
+	if d.Seconds() < 0 {
+		return mustNotDelete
+	}
+	return deletePriority(float64(mustDelete) * (1.0 - math.Exp(-d.Seconds()/secondsPerTenDays)))
+}
+
+func newestDeletePriority(machine *v1alpha1.Machine) deletePriority {
+	if machine.DeletionTimestamp != nil && !machine.DeletionTimestamp.IsZero() {
+		return mustDelete
+	}
+	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
+		return mustDelete
+	}
+	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
+		return mustDelete
+	}
+	return mustDelete - oldestDeletePriority(machine)
+}
+
+func randomDeletePolicy(machine *v1alpha1.Machine) deletePriority {
 	if machine.DeletionTimestamp != nil && !machine.DeletionTimestamp.IsZero() {
 		return mustDelete
 	}
+	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
+		return betterDelete
+	}
 	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
 		return betterDelete
 	}
 	return couldDelete
 }
 
-// TODO: Define machines deletion policies.
-// see: https://github.com/kubernetes/kube-deploy/issues/625
+type sortableMachines struct {
+	machines []*v1alpha1.Machine
+	priority deletePriorityFunc
+}
+
+func (m sortableMachines) Len() int      { return len(m.machines) }
+func (m sortableMachines) Swap(i, j int) { m.machines[i], m.machines[j] = m.machines[j], m.machines[i] }
+func (m sortableMachines) Less(i, j int) bool {
+	return m.priority(m.machines[j]) < m.priority(m.machines[i]) // high to low
+}
+
 func getMachinesToDeletePrioritized(filteredMachines []*v1alpha1.Machine, diff int, fun deletePriorityFunc) []*v1alpha1.Machine {
 	if diff >= len(filteredMachines) {
 		return filteredMachines
 	} else if diff <= 0 {
 		return []*v1alpha1.Machine{}
 	}
 
-	machines := make(map[deletePriority][]*v1alpha1.Machine)
-
-	for _, machine := range filteredMachines {
-		priority := fun(machine)
-		machines[priority] = append(machines[priority], machine)
+	sortable := sortableMachines{
+		machines: filteredMachines,
+		priority: fun,
 	}
+	sort.Sort(sortable)
 
-	result := []*v1alpha1.Machine{}
-	for _, priority := range []deletePriority{
-		mustDelete,
-		betterDelete,
-		couldDelete,
-	} {
-		result = append(result, machines[priority]...)
-		if len(result) >= diff {
-			break
-		}
-	}
+	return sortable.machines[:diff]
+}
 
-	return result[:diff]
+func getDeletePriorityFunc(ms *v1alpha1.MachineSet) (deletePriorityFunc, error) {
+	// Map the Spec.DeletePolicy value to the appropriate delete priority function
+	switch msdp := v1alpha1.MachineSetDeletePolicy(ms.Spec.DeletePolicy); msdp {
+	case v1alpha1.RandomMachineSetDeletePolicy:
+		return randomDeletePolicy, nil
+	case v1alpha1.NewestMachineSetDeletePolicy:
+		return newestDeletePriority, nil
+	case v1alpha1.OldestMachineSetDeletePolicy:
+		return oldestDeletePriority, nil
+	case "":
+		return randomDeletePolicy, nil
+	default:
+		return nil, errors.Errorf("Unsupported delete policy %s. Must be one of 'Random', 'Newest', or 'Oldest'", msdp)
+	}
 }