Delete Slice if Deformed status.

Author: mbobrovskyi

slice/internal/controller/workload_controller.go

@@ -219,12 +219,18 @@ func (r *WorkloadReconciler) cleanupSlice(ctx context.Context, wl *kueue.Workloa
 	}
 
 	if !slice.DeletionTimestamp.IsZero() {
-		return core.Deformed(&slice), nil
+		log.V(3).Info("Slice already deleted, finishing cleanup")
+		return true, nil
 	}
 
-	terminated, err := r.ownerPodsFinished(ctx, wl)
-	if err != nil || !terminated {
-		return false, err
+	if !core.Deformed(&slice) {
+		terminated, err := r.ownerPodsFinished(ctx, wl)
+		if err != nil || !terminated {
+			return false, err
+		}
+	} else {
+		log.V(3).Info("Slice in deformed state")
+		// We still need to delete the Slice because requeueing causes a conflict error during Slice creation.
 	}
 
 	log.V(3).Info("Deleting the Slice")
@@ -236,7 +242,7 @@ func (r *WorkloadReconciler) cleanupSlice(ctx context.Context, wl *kueue.Workloa
 		log.Error(err, "Failed to delete the Slice")
 	}
 
-	return false, err
+	return true, err
 }
 
 func (r *WorkloadReconciler) ownerPodsFinished(ctx context.Context, wl *kueue.Workload) (bool, error) {

slice/internal/controller/workload_controller_test.go

@@ -213,42 +213,29 @@ func TestWorkloadReconciler(t *testing.T) {
 			request: baseRequest,
 			objs: []client.Object{
 				baseAdmissionCheckWrapper.DeepCopy(),
+				baseJobSetWrapper.DeepCopy(),
+				basePod1Wrapper.DeepCopy(),
 				baseWorkloadWrapperWithAdmissionAndOwner.Clone().
 					Active(false).
 					Finalizers(SliceControllerName).
 					Obj(),
-				baseSliceWrapper.Clone().
-					Finalizers(SliceControllerName).
-					DeletionTimestamp(now).
-					Deformed().
-					Obj(),
+				baseSliceWrapper.Clone().Deformed().Obj(),
 			},
 			wantWorkloads: []kueue.Workload{
-				*baseWorkloadWrapperWithAdmissionAndOwner.Clone().
-					Active(false).
-					Obj(),
-			},
-			wantSlices: []slice.Slice{
-				*baseSliceWrapper.Clone().
-					Finalizers(SliceControllerName).
-					DeletionTimestamp(now).
-					Deformed().
-					Obj(),
+				*baseWorkloadWrapperWithAdmissionAndOwner.Clone().Active(false).Obj(),
 			},
 		},
 		"shouldn't delete the finalizer because the Slice status Degraded": {
 			request: baseRequest,
 			objs: []client.Object{
 				baseAdmissionCheckWrapper.DeepCopy(),
+				baseJobSetWrapper.DeepCopy(),
+				basePod1Wrapper.DeepCopy(),
 				baseWorkloadWrapperWithAdmissionAndOwner.Clone().
 					Active(false).
 					Finalizers(SliceControllerName).
 					Obj(),
-				baseSliceWrapper.Clone().
-					Finalizers(SliceControllerName).
-					DeletionTimestamp(now).
-					Degraded().
-					Obj(),
+				baseSliceWrapper.Clone().Degraded().Obj(),
 			},
 			wantWorkloads: []kueue.Workload{
 				*baseWorkloadWrapperWithAdmissionAndOwner.Clone().
@@ -257,11 +244,7 @@ func TestWorkloadReconciler(t *testing.T) {
 					Obj(),
 			},
 			wantSlices: []slice.Slice{
-				*baseSliceWrapper.Clone().
-					Finalizers(SliceControllerName).
-					DeletionTimestamp(now).
-					Degraded().
-					Obj(),
+				*baseSliceWrapper.Clone().Degraded().Obj(),
 			},
 		},
 		"should delete the Slice because the Pod Status Succeeded": {
@@ -273,17 +256,13 @@ func TestWorkloadReconciler(t *testing.T) {
 					Active(false).
 					Finalizers(SliceControllerName).
 					Obj(),
-				baseSliceWrapper.Clone().Finalizers(SliceControllerName).Obj(),
+				baseSliceWrapper.DeepCopy(),
 			},
 			wantWorkloads: []kueue.Workload{
 				*baseWorkloadWrapperWithAdmissionAndOwner.Clone().
 					Active(false).
-					Finalizers(SliceControllerName).
 					Obj(),
 			},
-			wantSlices: []slice.Slice{
-				*baseSliceWrapper.Clone().Finalizers(SliceControllerName).DeletionTimestamp(now).Obj(),
-			},
 		},
 		"should delete the Slice because the Pod Status PodFailed": {
 			request: baseRequest,
@@ -295,17 +274,13 @@ func TestWorkloadReconciler(t *testing.T) {
 					Active(false).
 					Finalizers(SliceControllerName).
 					Obj(),
-				baseSliceWrapper.Clone().Finalizers(SliceControllerName).Obj(),
+				baseSliceWrapper.DeepCopy(),
 			},
 			wantWorkloads: []kueue.Workload{
 				*baseWorkloadWrapperWithAdmissionAndOwner.Clone().
 					Active(false).
-					Finalizers(SliceControllerName).
 					Obj(),
 			},
-			wantSlices: []slice.Slice{
-				*baseSliceWrapper.Clone().Finalizers(SliceControllerName).DeletionTimestamp(now).Obj(),
-			},
 		},
 		"shouldn't delete the Slice because the Pods still running": {
 			request: baseRequest,
@@ -318,7 +293,7 @@ func TestWorkloadReconciler(t *testing.T) {
 					Active(false).
 					Finalizers(SliceControllerName).
 					Obj(),
-				baseSliceWrapper.Clone().Finalizers(SliceControllerName).Obj(),
+				baseSliceWrapper.DeepCopy(),
 			},
 			wantWorkloads: []kueue.Workload{
 				*baseWorkloadWrapperWithAdmissionAndOwner.Clone().
@@ -327,7 +302,7 @@ func TestWorkloadReconciler(t *testing.T) {
 					Obj(),
 			},
 			wantSlices: []slice.Slice{
-				*baseSliceWrapper.Clone().Finalizers(SliceControllerName).Obj(),
+				*baseSliceWrapper.DeepCopy(),
 			},
 		},
 		"shouldn't delete the Slice because one of the Pods still running": {
@@ -341,7 +316,7 @@ func TestWorkloadReconciler(t *testing.T) {
 					Active(false).
 					Finalizers(SliceControllerName).
 					Obj(),
-				baseSliceWrapper.Clone().Finalizers(SliceControllerName).Obj(),
+				baseSliceWrapper.DeepCopy(),
 			},
 			wantWorkloads: []kueue.Workload{
 				*baseWorkloadWrapperWithAdmissionAndOwner.Clone().
@@ -350,7 +325,7 @@ func TestWorkloadReconciler(t *testing.T) {
 					Obj(),
 			},
 			wantSlices: []slice.Slice{
-				*baseSliceWrapper.Clone().Finalizers(SliceControllerName).Obj(),
+				*baseSliceWrapper.DeepCopy(),
 			},
 		},
 		"shouldn't add finalizer because invalid TPU topology annotation": {