Skip to content

Commit dd30f24

Browse files
jpaodevchris-rock
authored andcommitted
fix: preserve active jobs when cleaning up completed scan jobs
Previously the controller would delete all jobs when updating CronJob specs, including active/running jobs. This could interrupt in-progress scans. The new DeleteCompletedJobs function only deletes jobs that have completed (succeeded or failed), preserving any active jobs. Includes tests for the new behavior.
1 parent 0d2138f commit dd30f24

File tree

2 files changed

+235
-1
lines changed

2 files changed

+235
-1
lines changed

pkg/utils/k8s/cron_job.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,15 @@
33

44
package k8s
55

6-
import batchv1 "k8s.io/api/batch/v1"
6+
import (
7+
"context"
8+
9+
"github.com/go-logr/logr"
10+
batchv1 "k8s.io/api/batch/v1"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/labels"
13+
"sigs.k8s.io/controller-runtime/pkg/client"
14+
)
715

816
// AreCronJobsSuccessful returns true if the latest runs of all of the provided CronJobs has been
917
// successful.
@@ -17,3 +25,35 @@ func AreCronJobsSuccessful(cs []batchv1.CronJob) bool {
1725
}
1826
return true
1927
}
28+
29+
// DeleteCompletedJobs deletes only completed or failed jobs matching the given labels.
30+
// Active/running jobs are preserved to avoid killing in-progress scans.
31+
func DeleteCompletedJobs(ctx context.Context, kubeClient client.Client, namespace string, jobLabels map[string]string, log logr.Logger) error {
32+
jobList := &batchv1.JobList{}
33+
listOpts := &client.ListOptions{
34+
Namespace: namespace,
35+
LabelSelector: labels.SelectorFromSet(jobLabels),
36+
}
37+
38+
if err := kubeClient.List(ctx, jobList, listOpts); err != nil {
39+
log.Error(err, "Failed to list Jobs", "namespace", namespace)
40+
return err
41+
}
42+
43+
for _, job := range jobList.Items {
44+
// Skip active jobs - only delete completed or failed ones
45+
if job.Status.Active > 0 {
46+
log.Info("Skipping deletion of active job", "namespace", job.Namespace, "name", job.Name)
47+
continue
48+
}
49+
50+
// Delete the job with foreground propagation to also delete its pods
51+
if err := kubeClient.Delete(ctx, &job, client.PropagationPolicy(metav1.DeletePropagationForeground)); err != nil {
52+
log.Error(err, "Failed to delete completed job", "namespace", job.Namespace, "name", job.Name)
53+
return err
54+
}
55+
log.Info("Deleted completed job", "namespace", job.Namespace, "name", job.Name)
56+
}
57+
58+
return nil
59+
}

pkg/utils/k8s/cron_job_test.go

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
// Copyright (c) Mondoo, Inc.
2+
// SPDX-License-Identifier: BUSL-1.1
3+
4+
package k8s
5+
6+
import (
7+
"context"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
12+
batchv1 "k8s.io/api/batch/v1"
13+
corev1 "k8s.io/api/core/v1"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/runtime"
16+
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
17+
ctrl "sigs.k8s.io/controller-runtime"
18+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
19+
)
20+
21+
func TestAreCronJobsSuccessful(t *testing.T) {
22+
tests := []struct {
23+
name string
24+
cronJobs []batchv1.CronJob
25+
expected bool
26+
}{
27+
{
28+
name: "empty list is successful",
29+
cronJobs: []batchv1.CronJob{},
30+
expected: true,
31+
},
32+
{
33+
name: "active job is successful",
34+
cronJobs: []batchv1.CronJob{
35+
{
36+
Status: batchv1.CronJobStatus{
37+
Active: []corev1.ObjectReference{{Name: "job-1"}},
38+
},
39+
},
40+
},
41+
expected: true,
42+
},
43+
}
44+
45+
for _, tt := range tests {
46+
t.Run(tt.name, func(t *testing.T) {
47+
result := AreCronJobsSuccessful(tt.cronJobs)
48+
assert.Equal(t, tt.expected, result)
49+
})
50+
}
51+
}
52+
53+
func TestDeleteCompletedJobs(t *testing.T) {
54+
scheme := runtime.NewScheme()
55+
require.NoError(t, clientgoscheme.AddToScheme(scheme))
56+
57+
labels := map[string]string{"app": "test-scan", "mondoo_cr": "test"}
58+
namespace := "test-ns"
59+
60+
tests := []struct {
61+
name string
62+
existingJobs []batchv1.Job
63+
expectedRemaining int
64+
expectedDeleted int
65+
}{
66+
{
67+
name: "no jobs to delete",
68+
existingJobs: []batchv1.Job{},
69+
expectedRemaining: 0,
70+
expectedDeleted: 0,
71+
},
72+
{
73+
name: "delete completed job",
74+
existingJobs: []batchv1.Job{
75+
{
76+
ObjectMeta: metav1.ObjectMeta{
77+
Name: "completed-job",
78+
Namespace: namespace,
79+
Labels: labels,
80+
},
81+
Status: batchv1.JobStatus{
82+
Active: 0,
83+
Succeeded: 1,
84+
},
85+
},
86+
},
87+
expectedRemaining: 0,
88+
expectedDeleted: 1,
89+
},
90+
{
91+
name: "delete failed job",
92+
existingJobs: []batchv1.Job{
93+
{
94+
ObjectMeta: metav1.ObjectMeta{
95+
Name: "failed-job",
96+
Namespace: namespace,
97+
Labels: labels,
98+
},
99+
Status: batchv1.JobStatus{
100+
Active: 0,
101+
Failed: 1,
102+
},
103+
},
104+
},
105+
expectedRemaining: 0,
106+
expectedDeleted: 1,
107+
},
108+
{
109+
name: "preserve active job",
110+
existingJobs: []batchv1.Job{
111+
{
112+
ObjectMeta: metav1.ObjectMeta{
113+
Name: "active-job",
114+
Namespace: namespace,
115+
Labels: labels,
116+
},
117+
Status: batchv1.JobStatus{
118+
Active: 1,
119+
},
120+
},
121+
},
122+
expectedRemaining: 1,
123+
expectedDeleted: 0,
124+
},
125+
{
126+
name: "mixed jobs - delete completed, preserve active",
127+
existingJobs: []batchv1.Job{
128+
{
129+
ObjectMeta: metav1.ObjectMeta{
130+
Name: "active-job",
131+
Namespace: namespace,
132+
Labels: labels,
133+
},
134+
Status: batchv1.JobStatus{
135+
Active: 1,
136+
},
137+
},
138+
{
139+
ObjectMeta: metav1.ObjectMeta{
140+
Name: "completed-job",
141+
Namespace: namespace,
142+
Labels: labels,
143+
},
144+
Status: batchv1.JobStatus{
145+
Active: 0,
146+
Succeeded: 1,
147+
},
148+
},
149+
{
150+
ObjectMeta: metav1.ObjectMeta{
151+
Name: "failed-job",
152+
Namespace: namespace,
153+
Labels: labels,
154+
},
155+
Status: batchv1.JobStatus{
156+
Active: 0,
157+
Failed: 1,
158+
},
159+
},
160+
},
161+
expectedRemaining: 1,
162+
expectedDeleted: 2,
163+
},
164+
}
165+
166+
for _, tt := range tests {
167+
t.Run(tt.name, func(t *testing.T) {
168+
// Build fake client with existing jobs
169+
objs := make([]runtime.Object, len(tt.existingJobs))
170+
for i := range tt.existingJobs {
171+
objs[i] = &tt.existingJobs[i]
172+
}
173+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(objs...).Build()
174+
175+
log := ctrl.Log.WithName("test")
176+
177+
// Call DeleteCompletedJobs
178+
err := DeleteCompletedJobs(context.Background(), fakeClient, namespace, labels, log)
179+
require.NoError(t, err)
180+
181+
// Verify remaining jobs
182+
jobList := &batchv1.JobList{}
183+
err = fakeClient.List(context.Background(), jobList)
184+
require.NoError(t, err)
185+
186+
assert.Equal(t, tt.expectedRemaining, len(jobList.Items), "unexpected number of remaining jobs")
187+
188+
// Verify that remaining jobs are all active
189+
for _, job := range jobList.Items {
190+
assert.Greater(t, job.Status.Active, int32(0), "remaining job should be active: %s", job.Name)
191+
}
192+
})
193+
}
194+
}

0 commit comments

Comments
 (0)