From 4a3ae69e7e4002a7b06364bdc5984a44cd640872 Mon Sep 17 00:00:00 2001 From: frankvollenweider Date: Thu, 24 Sep 2026 20:26:47 +0200 Subject: [PATCH] fix: delete a garbage collected Job's Pods with it gc jobs deleted Jobs with empty DeleteOptions. For batch/v1 Jobs the API server then orphans the Job's Pods, and says so in a warning on every delete. The orphaned Pods have no ownerReferences, so nothing collects them afterwards. With the default jx-gcjobs CronJob this leaves one completed jx-boot Pod in jx-git-operator per boot Job, and they accumulate until the nodes are replaced. Delete with background propagation, as kubectl delete job does, so the garbage collector removes the Pods together with the Job. Fixes #1103 Signed-off-by: frankvollenweider --- pkg/cmd/gc/jobs/jobs.go | 4 ++- pkg/cmd/gc/jobs/jobs_test.go | 57 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 pkg/cmd/gc/jobs/jobs_test.go diff --git a/pkg/cmd/gc/jobs/jobs.go b/pkg/cmd/gc/jobs/jobs.go index ae9f6e34..d9b44d96 100644 --- a/pkg/cmd/gc/jobs/jobs.go +++ b/pkg/cmd/gc/jobs/jobs.go @@ -93,7 +93,9 @@ func (o *Options) Run() error { return err } - deleteOptions := metav1.DeleteOptions{} + // without a propagation policy the API server keeps a deleted Job's Pods + propagation := metav1.DeletePropagationBackground + deleteOptions := metav1.DeleteOptions{PropagationPolicy: &propagation} errors := []error{} // we need to keep all jobs, don't waste time sorting or iterating diff --git a/pkg/cmd/gc/jobs/jobs_test.go b/pkg/cmd/gc/jobs/jobs_test.go new file mode 100644 index 00000000..af73a9aa --- /dev/null +++ b/pkg/cmd/gc/jobs/jobs_test.go @@ -0,0 +1,57 @@ +package jobs + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + batchv1 "k8s.io/api/batch/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" +) + +func TestGCJobsDeletesPodsWithJob(t *testing.T) { + t.Parallel() + + ns := "jx-git-operator" + oldStart := metav1.NewTime(time.Now().Add(-2 * time.Hour)) + newStart := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + + kubeClient := fake.NewClientset( + &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "old", Namespace: ns}, + Status: batchv1.JobStatus{StartTime: &oldStart, Succeeded: 1}, + }, + &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{Name: "new", Namespace: ns}, + Status: batchv1.JobStatus{StartTime: &newStart, Succeeded: 1}, + }, + ) + + o := &Options{ + Namespace: ns, + Age: time.Hour, + Keep: 0, + KubeClient: kubeClient, + } + require.NoError(t, o.Run()) + + var deletes []k8stesting.DeleteActionImpl + for _, action := range kubeClient.Actions() { + if d, ok := action.(k8stesting.DeleteActionImpl); ok { + deletes = append(deletes, d) + } + } + require.Len(t, deletes, 1) + assert.Equal(t, "old", deletes[0].GetName()) + require.NotNil(t, deletes[0].DeleteOptions.PropagationPolicy, "a Job deleted without a propagation policy leaves its Pods behind") + assert.Equal(t, metav1.DeletePropagationBackground, *deletes[0].DeleteOptions.PropagationPolicy) + + remaining, err := kubeClient.BatchV1().Jobs(ns).List(context.TODO(), metav1.ListOptions{}) + require.NoError(t, err) + require.Len(t, remaining.Items, 1) + assert.Equal(t, "new", remaining.Items[0].Name) +}