K8s-9372: Some modifications and change on node reference

pkg/apis/cluster/v1alpha1/machineset_types.go

@@ -100,10 +100,10 @@ const (
 	// It then prioritizes the oldest Machines for deletion based on the Machine's CreationTimestamp.
 	OldestMachineSetDeletePolicy MachineSetDeletePolicy = "Oldest"
 
-	// CustomMachineSetDeletePolicy prioritizes deletion of newer machines or the ones
-	// that are with a different status than "Ready". If then prioritizes the deletion
-	// of machines that have errors or deletion annotations added.
-	CustomMachineSetDeletePolicy MachineSetDeletePolicy = "Custom"
+	// DefaultDeletePolicy prioritizes deletion of newer machines or the ones
+	// referenced to a K8s node (relation between machine (OpenStack) - node (Kubernetes)). 
+	// If then prioritizes the deletion of machines that have errors or deletion annotations added.
+	DefaultDeletePolicy MachineSetDeletePolicy = "Default"
 )
 
 /// [MachineSetSpec] // doxygen marker
@@ -208,9 +208,9 @@ func (m *MachineSet) Default() {
 	}
 
 	if m.Spec.DeletePolicy == "" {
-		customPolicy := string(CustomMachineSetDeletePolicy)
-		log.Printf("Defaulting to %s\n", customPolicy)
-		m.Spec.DeletePolicy = customPolicy
+		defaultPolicy := string(DefaultDeletePolicy)
+		log.Printf("Defaulting to %s\n", defaultPolicy)
+		m.Spec.DeletePolicy = defaultPolicy
 	}
 }
 

pkg/controller/machineset/delete_policy.go

@@ -22,8 +22,6 @@ import (
 
 	"github.com/pkg/errors"
 
-	corev1 "k8s.io/api/core/v1"
-
 	"github.com/kubermatic/machine-controller/pkg/apis/cluster/v1alpha1"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -76,39 +74,32 @@ func oldestDeletePriority(machine *v1alpha1.Machine) deletePriority {
 	return deletePriority(float64(mustDelete) * (1.0 - math.Exp(-d.Seconds()/secondsPerTenDays)))
 }
 
-
-func customDeletePolicy(machine *v1alpha1.Machine) deletePriority {
-	// Iterate through conditions to get if machine is not "Ready"
-	// Check types.go
-	for _, c := range machine.Status.Conditions {
-		if c.Type != corev1.NodeReady {
-			return mustDelete
-		}
+// Default policies try to delete the machines that has no reference to a K8s node.
+// If a reference exists, then continue with same conditions from "Newest" in different order.
+func defaultDeletePolicy(machine *v1alpha1.Machine) deletePriority {
+	if !machine.DeletionTimestamp.IsZero() {
+		return mustDelete
 	}
-	// Rest of conditions are extracted from the "Newest" policy, that is the one that
-	// matches the idea of deleting the newest machines (more possibilities to be WIP in the attachment to cluster)
-	// than the old ones.
-	if machine.DeletionTimestamp != nil && !machine.DeletionTimestamp.IsZero() {
+
+	if machine.Status.NodeRef == nil{
 		return mustDelete
 	}
-	// If annotations are set + DeleteAnnotation is present
-	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
+
+	if v, ok := machine.ObjectMeta.Annotations[DeleteNodeAnnotation]; ok && v != "" {
 		return mustDelete
 	}
-	// If there are errors on machine, delete it
 	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
 		return mustDelete
 	}
-	// Thinking about this, maybe it's a good idea to delegate in oldestDeletePriority, like
-	// newestDeletePriority is doing if no match is done, or just return couldDelete
+	// If not condition is matched from above, retrieve points from Newer to Older machines.
 	return mustDelete - oldestDeletePriority(machine)
 }
 
 func newestDeletePriority(machine *v1alpha1.Machine) deletePriority {
-	if machine.DeletionTimestamp != nil && !machine.DeletionTimestamp.IsZero() {
+	if !machine.DeletionTimestamp.IsZero() {
 		return mustDelete
 	}
-	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
+	if v, ok := machine.ObjectMeta.Annotations[DeleteNodeAnnotation]; ok && v != "" {
 		return mustDelete
 	}
 	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
@@ -118,10 +109,10 @@ func newestDeletePriority(machine *v1alpha1.Machine) deletePriority {
 }
 
 func randomDeletePolicy(machine *v1alpha1.Machine) deletePriority {
-	if machine.DeletionTimestamp != nil && !machine.DeletionTimestamp.IsZero() {
+	if !machine.DeletionTimestamp.IsZero() {
 		return mustDelete
 	}
-	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
+	if v, ok := machine.ObjectMeta.Annotations[DeleteNodeAnnotation]; ok && v != "" {
 		return betterDelete
 	}
 	if machine.Status.ErrorReason != nil || machine.Status.ErrorMessage != nil {
@@ -161,18 +152,18 @@ func getMachinesToDeletePrioritized(filteredMachines []*v1alpha1.Machine, diff i
 
 func getDeletePriorityFunc(ms *v1alpha1.MachineSet) (deletePriorityFunc, error) {
 	// Map the Spec.DeletePolicy value to the appropriate delete priority function
-	// Defaults to customDeletePolicy if not specified
+	// Defaults to defaultDeletePolicy if not specified
 	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 v1alpha1.CustomMachineSetDeletePolicy:
-		return customDeletePolicy, nil
+	case v1alpha1.DefaultDeletePolicy:
+		return defaultDeletePolicy, nil
 	case "":
-		return customDeletePolicy, nil
+		return defaultDeletePolicy, nil
 	default:
 		return nil, errors.Errorf("Unsupported delete policy %q. Must be one of 'Random', 'Newest', or 'Oldest'", msdp)
 	}