Skip to content

Commit 3a1509d

Browse files
fix(scanner): reap finished scanner jobs independent of TTLAfterFinished controller (#492)
* fix(scanner): reap finished scanner jobs independent of TTLAfterFinished controller Scanner Jobs were only cleaned up by the cluster's TTLAfterFinished controller (via TTLSecondsAfterFinished=600). That controller can be disabled in customer environments, leaving finished Jobs and their Completed pods to accumulate forever. Add a background reaper that periodically deletes the agent's own finished Jobs (cascading to their pods) once past the same 600s grace the TTL field encodes. It is strictly label- and namespace-scoped so it can never touch customer Jobs, and is a no-op where the TTL controller is present. Also add a delete_k8s_job primitive so the orchestrator can delete a Job promptly after fetching its logs. * chore: update image tags for main release --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
1 parent 66bcf98 commit 3a1509d

7 files changed

Lines changed: 303 additions & 8 deletions

File tree

‎charts/nudgebee-agent/values.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ runnerServiceAccount:
6161
runner:
6262
image:
6363
repository: ghcr.io/nudgebee/nudgebee-agent
64-
tag: 2026-06-20T09-33-11_75cc137fae195c9fc972e41d4537bda18bdc3776
64+
tag: 2026-06-23T09-16-10_66bcf983a562d8776d7e8325884f25bc50864449
6565
# Image template the pod_profiler action launches debugger pods from.
6666
# The agent substitutes `{}` for the variant (bpf, jvm, python, perf, ruby).
6767
# Surfaces as PROFILER_IMAGE; leave empty to fall back to the binary default.

‎runner/cmd/agent/main.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,9 @@ func run(ctx context.Context, logger *slog.Logger, cfg *config.Config) error {
476476
for name := range sh {
477477
lightActions[name] = struct{}{}
478478
}
479+
// Background cleanup of finished Jobs/pods, independent of the cluster's
480+
// TTLAfterFinished controller (which may be disabled in customer envs).
481+
runner.StartReaper(ctx, logger)
479482
logger.Info("scanners enabled", "namespace", cfg.ScannerNamespace, "actions", len(sh))
480483
}
481484

‎runner/pkg/scanners/handlers.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,20 @@ package scanners
22

33
import "github.com/nudgebee/nudgebee-agent/pkg/dispatch"
44

5-
// Handlers wires the three generic Job primitives into the dispatch registry.
5+
// Handlers wires the generic Job primitives into the dispatch registry.
66
//
77
// The agent owns no scanner-specific behavior. The api-server's
8-
// scan_orchestrator builds JobSpecs (image, args, security context) and
9-
// drives them through schedule_k8s_job → wait_for_k8s_job → get_k8s_job_logs.
10-
// Adding a new scanner is a pure api-server change.
8+
// scan_orchestrator builds JobSpecs (image, args, security context) and drives
9+
// them through schedule_k8s_job → wait_for_k8s_job → get_k8s_job_logs →
10+
// delete_k8s_job. Adding a new scanner is a pure api-server change.
1111
//
12-
// All three actions require HMAC sig (or RSA partial-keys for callers that
13-
// pre-sign mutations). Not light-action — these create + read Jobs.
12+
// All actions require HMAC sig (or RSA partial-keys for callers that pre-sign
13+
// mutations). Not light-action — these create, read, and delete Jobs.
1414
func Handlers(r *Runner) map[string]dispatch.Handler {
1515
return map[string]dispatch.Handler{
1616
"schedule_k8s_job": r.handleScheduleJob,
1717
"wait_for_k8s_job": r.handleWaitForJob,
1818
"get_k8s_job_logs": r.handleGetJobLogs,
19+
"delete_k8s_job": r.handleDeleteJob,
1920
}
2021
}

‎runner/pkg/scanners/primitives.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,31 @@ func (r *Runner) handleGetJobLogs(ctx context.Context, params map[string]any) (a
190190
}, nil
191191
}
192192

193+
// delete_k8s_job
194+
// --------------
195+
// Inbound params: { "job_name": "<name>" }
196+
//
197+
// Outbound: { "deleted": true } (also returned when the Job is already gone)
198+
//
199+
// The orchestrator calls this right after it has fetched a Job's logs so the
200+
// Job (and its pods) are cleaned up promptly on the happy path, instead of
201+
// waiting out the TTL/reaper grace window. The background reaper is the backstop
202+
// for crash/timeout paths where this call never happens.
203+
//
204+
// Only the runner's own namespace is touched, and only by Job name handed back
205+
// from schedule_k8s_job — the agent never deletes arbitrary cluster objects.
206+
func (r *Runner) handleDeleteJob(ctx context.Context, params map[string]any) (any, error) {
207+
jobName, _ := params["job_name"].(string)
208+
if jobName == "" {
209+
return nil, errors.New("delete_k8s_job: job_name is required")
210+
}
211+
if err := r.deleteJobCascade(ctx, jobName); err != nil && !apierrors.IsNotFound(err) {
212+
return nil, fmt.Errorf("delete_k8s_job: %w", err)
213+
}
214+
slog.Info("delete_k8s_job: deleted", "job_name", jobName, "namespace", r.Namespace)
215+
return map[string]any{"deleted": true}, nil
216+
}
217+
193218
// decodeJobSpec re-marshals the params["spec"] map through encoding/json so
194219
// JobSpec's struct tags drive the decode (handles nested corev1.Volume /
195220
// VolumeMount cleanly without bespoke parsing).

‎runner/pkg/scanners/reaper.go‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
package scanners
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
"time"
7+
8+
batchv1 "k8s.io/api/batch/v1"
9+
corev1 "k8s.io/api/core/v1"
10+
apierrors "k8s.io/apimachinery/pkg/api/errors"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
)
13+
14+
// Job cleanup is normally handled by Kubernetes' TTLAfterFinished controller
15+
// (we stamp every Job with TTLSecondsAfterFinished=jobTTLSeconds). That
16+
// controller is GA and on by default since k8s 1.21, but a customer cluster
17+
// can have it disabled (kube-controller-manager --controllers=-ttl-after-finished,
18+
// hardened/managed distros, or pre-1.21). When it's off, finished scanner Jobs
19+
// and their Completed pods accumulate forever.
20+
//
21+
// The reaper makes cleanup independent of that controller: the agent itself
22+
// periodically deletes its own finished Jobs once they're past the same grace
23+
// window the TTL field encodes. It only ever touches Jobs carrying BOTH the
24+
// managed-by and orchestrator labels, scoped to the runner's namespace, so it
25+
// can never delete a customer's Jobs. Where the TTL controller IS present this
26+
// is a harmless no-op (the controller usually wins the race first).
27+
const (
28+
// reaperInterval is how often the sweep runs.
29+
reaperInterval = 60 * time.Second
30+
// reaperSweepTimeout bounds a single sweep's API calls, safely below
31+
// reaperInterval so a hung API server can't wedge the loop.
32+
reaperSweepTimeout = 45 * time.Second
33+
// reaperListLimit caps how many Jobs a single sweep lists/deletes, so a large
34+
// accumulated backlog (TTL controller off for a long time) is drained in
35+
// bounded batches across successive sweeps instead of one giant List that
36+
// could spike agent memory or overload the API server.
37+
reaperListLimit = 100
38+
// reapGraceSeconds is how long after a Job finishes before it's eligible for
39+
// deletion. Matched to jobTTLSeconds so behavior mirrors the TTL field: the
40+
// orchestrator's completion→log-fetch gap is seconds, so a 10-min grace
41+
// preserves the existing window for retries/manual debugging of logs.
42+
reapGraceSeconds = jobTTLSeconds
43+
)
44+
45+
// StartReaper launches the background cleanup loop and returns immediately. The
46+
// loop runs until ctx is cancelled (agent shutdown). Safe to call once per
47+
// Runner from the agent's main wiring.
48+
func (r *Runner) StartReaper(ctx context.Context, logger *slog.Logger) {
49+
if logger == nil {
50+
logger = slog.Default()
51+
}
52+
logger = logger.With("component", "scanner_job_reaper", "namespace", r.Namespace)
53+
go func() {
54+
ticker := time.NewTicker(reaperInterval)
55+
defer ticker.Stop()
56+
logger.Info("scanner job reaper started",
57+
"interval", reaperInterval.String(), "grace_seconds", reapGraceSeconds)
58+
for {
59+
select {
60+
case <-ctx.Done():
61+
logger.Info("scanner job reaper stopped")
62+
return
63+
case <-ticker.C:
64+
// Bound each sweep below the tick interval so an unresponsive API
65+
// server can't wedge the loop indefinitely.
66+
sweepCtx, cancel := context.WithTimeout(ctx, reaperSweepTimeout)
67+
if n := r.reapFinishedJobs(sweepCtx, logger, time.Now()); n > 0 {
68+
logger.Info("reaped finished scanner jobs", "deleted", n)
69+
}
70+
cancel()
71+
}
72+
}
73+
}()
74+
}
75+
76+
// reapFinishedJobs deletes managed Jobs that finished more than reapGraceSeconds
77+
// before now. Returns the number deleted. Errors are logged and skipped — a
78+
// single bad Job must not stall the sweep. `now` is a parameter so tests can
79+
// drive it deterministically.
80+
func (r *Runner) reapFinishedJobs(ctx context.Context, logger *slog.Logger, now time.Time) int {
81+
jobs, err := r.Client.BatchV1().Jobs(r.Namespace).List(ctx, metav1.ListOptions{
82+
LabelSelector: managedByLabel + "=" + managedByValue + "," + orchestratorLabel + "=" + orchestratorValue,
83+
Limit: reaperListLimit,
84+
})
85+
if err != nil {
86+
// Suppress expected cancellation noise during agent shutdown.
87+
if ctx.Err() == nil {
88+
logger.Warn("reaper: list managed jobs failed", "error", err)
89+
}
90+
return 0
91+
}
92+
93+
grace := time.Duration(reapGraceSeconds) * time.Second
94+
deleted := 0
95+
for i := range jobs.Items {
96+
job := &jobs.Items[i]
97+
finishedAt, ok := jobFinishedAt(job)
98+
if !ok {
99+
continue // still running / no terminal condition yet
100+
}
101+
if now.Sub(finishedAt) < grace {
102+
continue // within the grace window — leave it for log fetch / debugging
103+
}
104+
if err := r.deleteJobCascade(ctx, job.Name); err != nil && !apierrors.IsNotFound(err) {
105+
if ctx.Err() == nil {
106+
logger.Warn("reaper: delete job failed", "job_name", job.Name, "error", err)
107+
}
108+
continue
109+
}
110+
deleted++
111+
logger.Info("reaper: deleted finished job",
112+
"job_name", job.Name, "finished_at", finishedAt.UTC().Format(time.RFC3339))
113+
}
114+
return deleted
115+
}
116+
117+
// deleteJobCascade deletes a Job with background propagation so the Job's pods
118+
// are garbage-collected along with it (the default foreground/orphan behavior
119+
// would otherwise leave the Completed pods behind).
120+
func (r *Runner) deleteJobCascade(ctx context.Context, jobName string) error {
121+
policy := metav1.DeletePropagationBackground
122+
return r.Client.BatchV1().Jobs(r.Namespace).Delete(ctx, jobName, metav1.DeleteOptions{
123+
PropagationPolicy: &policy,
124+
})
125+
}
126+
127+
// jobFinishedAt returns when a Job reached a terminal (Complete/Failed)
128+
// condition. Complete Jobs carry Status.CompletionTime; Failed Jobs don't, so
129+
// we fall back to the terminal condition's LastTransitionTime. Returns false
130+
// when the Job has not finished.
131+
func jobFinishedAt(j *batchv1.Job) (time.Time, bool) {
132+
if !jobIsTerminal(j) {
133+
return time.Time{}, false
134+
}
135+
if j.Status.CompletionTime != nil {
136+
return j.Status.CompletionTime.Time, true
137+
}
138+
for _, c := range j.Status.Conditions {
139+
if c.Status != corev1.ConditionTrue {
140+
continue
141+
}
142+
if c.Type == batchv1.JobComplete || c.Type == batchv1.JobFailed {
143+
return c.LastTransitionTime.Time, true
144+
}
145+
}
146+
return time.Time{}, false
147+
}

‎runner/pkg/scanners/reaper_test.go‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package scanners
2+
3+
import (
4+
"context"
5+
"io"
6+
"log/slog"
7+
"testing"
8+
"time"
9+
10+
batchv1 "k8s.io/api/batch/v1"
11+
corev1 "k8s.io/api/core/v1"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/client-go/kubernetes/fake"
14+
)
15+
16+
func managedJob(name string) *batchv1.Job {
17+
return &batchv1.Job{
18+
ObjectMeta: metav1.ObjectMeta{
19+
Name: name,
20+
Namespace: "ns",
21+
Labels: map[string]string{
22+
managedByLabel: managedByValue,
23+
orchestratorLabel: orchestratorValue,
24+
},
25+
},
26+
}
27+
}
28+
29+
func completedAt(j *batchv1.Job, t time.Time) *batchv1.Job {
30+
ct := metav1.NewTime(t)
31+
j.Status.CompletionTime = &ct
32+
j.Status.Conditions = []batchv1.JobCondition{{
33+
Type: batchv1.JobComplete,
34+
Status: corev1.ConditionTrue,
35+
LastTransitionTime: ct,
36+
}}
37+
return j
38+
}
39+
40+
func failedAt(j *batchv1.Job, t time.Time) *batchv1.Job {
41+
// Failed Jobs carry no CompletionTime — only the terminal condition's time.
42+
j.Status.Conditions = []batchv1.JobCondition{{
43+
Type: batchv1.JobFailed,
44+
Status: corev1.ConditionTrue,
45+
LastTransitionTime: metav1.NewTime(t),
46+
}}
47+
return j
48+
}
49+
50+
func discardLogger() *slog.Logger {
51+
return slog.New(slog.NewTextHandler(io.Discard, nil))
52+
}
53+
54+
func TestReap_DeletesFinishedJobsPastGrace(t *testing.T) {
55+
now := time.Now()
56+
grace := time.Duration(reapGraceSeconds) * time.Second
57+
58+
staleComplete := completedAt(managedJob("trivy-image-scan-old"), now.Add(-grace-time.Minute))
59+
staleFailed := failedAt(managedJob("trivy-image-scan-failed"), now.Add(-grace-time.Minute))
60+
freshComplete := completedAt(managedJob("trivy-image-scan-fresh"), now.Add(-time.Second))
61+
running := managedJob("trivy-image-scan-running") // no terminal condition
62+
63+
r := NewRunner(fake.NewClientset(staleComplete, staleFailed, freshComplete, running), "ns", "sa")
64+
65+
if n := r.reapFinishedJobs(context.Background(), discardLogger(), now); n != 2 {
66+
t.Fatalf("reaped = %d; want 2 (both stale jobs)", n)
67+
}
68+
69+
remaining, err := r.Client.BatchV1().Jobs("ns").List(context.Background(), metav1.ListOptions{})
70+
if err != nil {
71+
t.Fatal(err)
72+
}
73+
got := map[string]bool{}
74+
for _, j := range remaining.Items {
75+
got[j.Name] = true
76+
}
77+
if got["trivy-image-scan-old"] || got["trivy-image-scan-failed"] {
78+
t.Errorf("stale jobs not deleted: %v", got)
79+
}
80+
if !got["trivy-image-scan-fresh"] || !got["trivy-image-scan-running"] {
81+
t.Errorf("fresh/running jobs must survive: %v", got)
82+
}
83+
}
84+
85+
func TestReap_IgnoresUnmanagedJobs(t *testing.T) {
86+
now := time.Now()
87+
old := metav1.NewTime(now.Add(-24 * time.Hour))
88+
89+
// A finished Job that is NOT ours (e.g. a customer CronJob) — must be left alone.
90+
customer := &batchv1.Job{
91+
ObjectMeta: metav1.ObjectMeta{Name: "customer-backup", Namespace: "ns"},
92+
Status: batchv1.JobStatus{
93+
CompletionTime: &old,
94+
Conditions: []batchv1.JobCondition{{
95+
Type: batchv1.JobComplete, Status: corev1.ConditionTrue, LastTransitionTime: old,
96+
}},
97+
},
98+
}
99+
r := NewRunner(fake.NewClientset(customer), "ns", "sa")
100+
101+
if n := r.reapFinishedJobs(context.Background(), discardLogger(), now); n != 0 {
102+
t.Fatalf("reaped = %d; want 0 (unmanaged job must be untouched)", n)
103+
}
104+
if _, err := r.Client.BatchV1().Jobs("ns").Get(context.Background(), "customer-backup", metav1.GetOptions{}); err != nil {
105+
t.Errorf("customer job was deleted: %v", err)
106+
}
107+
}
108+
109+
func TestDeleteJobCascade_BackgroundPropagation(t *testing.T) {
110+
job := completedAt(managedJob("trivy-image-scan-x"), time.Now())
111+
r := NewRunner(fake.NewClientset(job), "ns", "sa")
112+
113+
if err := r.deleteJobCascade(context.Background(), "trivy-image-scan-x"); err != nil {
114+
t.Fatalf("deleteJobCascade: %v", err)
115+
}
116+
if _, err := r.Client.BatchV1().Jobs("ns").Get(context.Background(), "trivy-image-scan-x", metav1.GetOptions{}); err == nil {
117+
t.Error("job still present after cascade delete")
118+
}
119+
}

‎runner/pkg/scanners/scanners_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ func TestGetJobLogs_RejectsMissingName(t *testing.T) {
511511
func TestHandlers_RegistersOnlyPrimitives(t *testing.T) {
512512
r := NewRunner(fake.NewClientset(), "ns", "")
513513
hs := Handlers(r)
514-
want := []string{"schedule_k8s_job", "wait_for_k8s_job", "get_k8s_job_logs"}
514+
want := []string{"schedule_k8s_job", "wait_for_k8s_job", "get_k8s_job_logs", "delete_k8s_job"}
515515
if len(hs) != len(want) {
516516
t.Fatalf("Handlers count = %d; want %d (primitives only)", len(hs), len(want))
517517
}

0 commit comments

Comments
 (0)