Repository: kubernetes-sigs/kueue

Issue:
Flaky integration test for object retention policy


**What happened**:

The test failed on unrelated branch  ObjectRetentionPolicies with TinyTimeout when workload has finished should delete the Workload

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-sigs_kueue/5248/pull-kueue-test-e2e-customconfigs-main/1922642913863405568

**What you expected to happen**:

no failures

**How to reproduce it (as minimally and precisely as possible)**:

**Anything else we need to know?**:

```
{Timed out after 10.000s.
The function passed to Eventually failed at /home/prow/go/src/sigs.k8s.io/kueue/test/e2e/customconfigs/objectretentionpolicies_test.go:104 with:
Error matcher expects an error.  Got:
    <nil>: nil failed [FAILED] Timed out after 10.000s.
The function passed to Eventually failed at /home/prow/go/src/sigs.k8s.io/kueue/test/e2e/customconfigs/objectretentionpolicies_test.go:104 with:
Error matcher expects an error.  Got:
    <nil>: nil
In [It] at: /home/prow/go/src/sigs.k8s.io/kueue/test/e2e/customconfigs/objectretentionpolicies_test.go:105 @ 05/14/25 13:33:38.051
}
```

**Environment**:
- Kubernetes version (use `kubectl version`):
- Kueue version (use `git describe --tags --dirty --always`):
- Cloud provider or hardware configuration:
- OS (e.g: `cat /etc/os-release`):
- Kernel (e.g. `uname -a`):
- Install tools:
- Others:

Discussion:
@mimowo: /kind flake
/assign @mykysha
tentatively, as one of the main contributors working on the object retention policy
ptal

Solution patch:
diff --git a/pkg/controller/jobframework/reconciler.go b/pkg/controller/jobframework/reconciler.go
index fe945809a..63b0e9055 100644
--- a/pkg/controller/jobframework/reconciler.go
+++ b/pkg/controller/jobframework/reconciler.go
@@ -604,12 +604,12 @@ func (r *JobReconciler) ReconcileGenericJob(ctx context.Context, req ctrl.Reques
 	return ctrl.Result{}, nil
 }
 
-func shouldHandleDeletionOfDeactivatedWorkload(afterDeactivatedByKueue *time.Duration, wl *kueue.Workload) bool {
-	return afterDeactivatedByKueue != nil && !workload.IsActive(wl) && workload.IsEvictedDueToDeactivationByKueue(wl)
+func (r *JobReconciler) shouldHandleDeletionOfDeactivatedWorkload(wl *kueue.Workload) bool {
+	return r.workloadRetentionPolicy.AfterDeactivatedByKueue != nil && !workload.IsActive(wl) && workload.IsEvictedDueToDeactivationByKueue(wl)
 }
 
 func (r *JobReconciler) handleWorkloadAfterDeactivatedPolicy(ctx context.Context, job GenericJob, wl *kueue.Workload) (time.Duration, error) {
-	if !shouldHandleDeletionOfDeactivatedWorkload(r.workloadRetentionPolicy.AfterDeactivatedByKueue, wl) {
+	if !r.shouldHandleDeletionOfDeactivatedWorkload(wl) {
 		return 0, nil
 	}
 



Test patch:
diff --git a/pkg/controller/jobs/job/job_controller_test.go b/pkg/controller/jobs/job/job_controller_test.go
index f5180589f..e81cb65da 100644
--- a/pkg/controller/jobs/job/job_controller_test.go
+++ b/pkg/controller/jobs/job/job_controller_test.go
@@ -1068,7 +1068,7 @@ func TestReconciler(t *testing.T) {
 				},
 			},
 		},
-		"when workload is reactivated; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0": {
+		"when workload is active after deactivation; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0; should not delete the job": {
 			enableObjectRetentionPolicies: true,
 			reconcilerOptions: []jobframework.Option{
 				jobframework.WithObjectRetentionPolicies(&configapi.ObjectRetentionPolicies{
@@ -1159,7 +1159,7 @@ func TestReconciler(t *testing.T) {
 				},
 			},
 		},
-		"when workload is manually deactivated; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0": {
+		"when workload is manually deactivated; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0; should not delete the job": {
 			enableObjectRetentionPolicies: true,
 			reconcilerOptions: []jobframework.Option{
 				jobframework.WithObjectRetentionPolicies(&configapi.ObjectRetentionPolicies{
@@ -1250,7 +1250,7 @@ func TestReconciler(t *testing.T) {
 				},
 			},
 		},
-		"when workload is deactivated by kueue; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0": {
+		"when workload is deactivated by kueue; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0; should delete the job": {
 			enableObjectRetentionPolicies: true,
 			reconcilerOptions: []jobframework.Option{
 				jobframework.WithObjectRetentionPolicies(&configapi.ObjectRetentionPolicies{
diff --git a/pkg/controller/jobs/pod/pod_controller_test.go b/pkg/controller/jobs/pod/pod_controller_test.go
index d74804b1f..1488192ea 100644
--- a/pkg/controller/jobs/pod/pod_controller_test.go
+++ b/pkg/controller/jobs/pod/pod_controller_test.go
@@ -5447,7 +5447,7 @@ func TestReconciler(t *testing.T) {
 			workloadCmpOpts: defaultWorkloadCmpOpts,
 			wantErr:         jobframework.ErrPrebuiltWorkloadNotFound,
 		},
-		"when workload is deactivated by kueue; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0": {
+		"when workload is deactivated by kueue; objectRetentionPolicies.workloads.afterDeactivatedByKueue=0; should delete the job": {
 			enableObjectRetentionPolicies: true,
 			reconcilerOptions: []jobframework.Option{
 				jobframework.WithObjectRetentionPolicies(&configapi.ObjectRetentionPolicies{
diff --git a/test/e2e/customconfigs/objectretentionpolicies_test.go b/test/e2e/customconfigs/objectretentionpolicies_test.go
index def4fa560..5774b4a63 100644
--- a/test/e2e/customconfigs/objectretentionpolicies_test.go
+++ b/test/e2e/customconfigs/objectretentionpolicies_test.go
@@ -102,7 +102,7 @@ var _ = ginkgo.Describe("ObjectRetentionPolicies with TinyTimeout", ginkgo.Order
 			ginkgo.By("Checking that the Workload is deleted after it is finished", func() {
 				gomega.Eventually(func(g gomega.Gomega) {
 					g.Expect(k8sClient.Get(ctx, wlKey, wl)).To(testing.BeNotFoundError())
-				}, util.Timeout, util.Interval).Should(gomega.Succeed())
+				}, util.LongTimeout, util.Interval).Should(gomega.Succeed())
 			})
 
 			ginkgo.By("Checking that the Job is not deleted", func() {
diff --git a/test/integration/singlecluster/controller/jobs/job/job_controller_test.go b/test/integration/singlecluster/controller/jobs/job/job_controller_test.go
index 14ec9698f..60088c2ee 100644
--- a/test/integration/singlecluster/controller/jobs/job/job_controller_test.go
+++ b/test/integration/singlecluster/controller/jobs/job/job_controller_test.go
@@ -2827,11 +2827,7 @@ var _ = ginkgo.Describe("Job controller with ObjectRetentionPolicies", ginkgo.Or
 				ginkgo.By("Checking that the Job is deleted", func() {
 					createdJob := &batchv1.Job{}
 					gomega.Eventually(func(g gomega.Gomega) {
-						err := k8sClient.Get(ctx, client.ObjectKeyFromObject(job), createdJob)
-						if apierrors.IsNotFound(err) {
-							return
-						}
-						g.Expect(err).NotTo(gomega.HaveOccurred())
+						g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(job), createdJob)).Should(gomega.Succeed())
 						g.Expect(createdJob.GetDeletionTimestamp()).To(gomega.BeNil())
 					}, util.Timeout, util.Interval).Should(gomega.Succeed())
 				})