Extract overlapping node disruption check in dedicated method

criteo · Dec 11, 2023 · f680c89 · f680c89
1 parent 84d9555
commit f680c89
Showing 1 changed file with 32 additions and 19 deletions.
diff --git a/internal/controller/nodedisruption_controller.go b/internal/controller/nodedisruption_controller.go
@@ -178,6 +178,14 @@ func (ndr *SingleNodeDisruptionReconciler) tryTransitionToGranted(ctx context.Co
 	if err != nil {
 		return err
 	}
+
+	if !anyFailed && ndr.Config.RejectOverlappingDisruption {
+		anyFailed, statuses, err = ndr.ValidateOverlappingDisruption(ctx)
+		if err != nil {
+			return err
+		}
+	}
+
 	if !anyFailed {
 		anyFailed, statuses = ndr.ValidateWithBudgetConstraints(ctx, budgets)
 	}
@@ -237,32 +245,37 @@ func (ndr *SingleNodeDisruptionReconciler) ValidateWithInternalConstraints(ctx c
 		return true, ndr.generateRejectedStatus("No Node matching selector"), nil
 	}
 
-	if ndr.Config.RejectOverlappingDisruption {
-		allDisruptions := &nodedisruptionv1alpha1.NodeDisruptionList{}
-		err = ndr.Client.List(ctx, allDisruptions)
-		if err != nil {
-			return false, nil, err
-		}
-		for _, otherDisruption := range allDisruptions.Items {
-			if otherDisruption.Name == ndr.NodeDisruption.Name {
-				continue
-			}
-			if otherDisruption.Status.State == nodedisruptionv1alpha1.Pending || otherDisruption.Status.State == nodedisruptionv1alpha1.Granted {
-				otherDisruptedNodes := resolver.NewNodeSetFromStringList(otherDisruption.Status.DisruptedNodes)
-				if otherDisruptedNodes.Intersection(disruptedNodes).Len() > 0 {
-					return true, ndr.generateRejectedStatus(fmt.Sprintf(`Selected node(s) overlap with another disruption: ”%s"`, otherDisruption.Name)), nil
-				}
-			}
-		}
-	}
-
 	if ndr.NodeDisruption.Spec.Retry.IsAfterDeadline() {
 		return true, ndr.generateRejectedStatus("Deadline exceeded"), nil
 	}
 
 	return false, statuses, nil
 }
 
+// ValidateOverlappingDisruption checks that the current disruption doesn't overlap and existing one
+func (ndr *SingleNodeDisruptionReconciler) ValidateOverlappingDisruption(ctx context.Context) (anyFailed bool, statuses []nodedisruptionv1alpha1.DisruptedBudgetStatus, err error) {
+	disruptedNodes := resolver.NewNodeSetFromStringList(ndr.NodeDisruption.Status.DisruptedNodes)
+
+	allDisruptions := &nodedisruptionv1alpha1.NodeDisruptionList{}
+	err = ndr.Client.List(ctx, allDisruptions)
+	if err != nil {
+		return false, nil, err
+	}
+	for _, otherDisruption := range allDisruptions.Items {
+		if otherDisruption.Name == ndr.NodeDisruption.Name {
+			continue
+		}
+		if otherDisruption.Status.State == nodedisruptionv1alpha1.Pending || otherDisruption.Status.State == nodedisruptionv1alpha1.Granted {
+			otherDisruptedNodes := resolver.NewNodeSetFromStringList(otherDisruption.Status.DisruptedNodes)
+			if otherDisruptedNodes.Intersection(disruptedNodes).Len() > 0 {
+				return true, ndr.generateRejectedStatus(fmt.Sprintf(`Selected node(s) overlap with another disruption: ”%s"`, otherDisruption.Name)), nil
+			}
+		}
+	}
+
+	return false, nil, nil
+}
+
 // ValidateBudgetConstraints check that the Node Disruption is valid against the budgets defined in the cluster
 func (ndr *SingleNodeDisruptionReconciler) ValidateWithBudgetConstraints(ctx context.Context, budgets []Budget) (anyFailed bool, statuses []nodedisruptionv1alpha1.DisruptedBudgetStatus) {
 	disruptedNodes := resolver.NewNodeSetFromStringList(ndr.NodeDisruption.Status.DisruptedNodes)