Skip to content

Commit

Permalink
Fix tracking of deleted Jobs
Browse files Browse the repository at this point in the history
  • Loading branch information
bastjan committed Oct 4, 2023
1 parent 0d33cbe commit 7be3f27
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
15 changes: 13 additions & 2 deletions controllers/upgradejob_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,19 @@ func (r *UpgradeJobReconciler) trackHookJobs(ctx context.Context, req ctrl.Reque
tr = append(tr, s)
}

if !sets.New(uj.Status.HookJobTracker...).Equal(sets.New(tr...)) {
uj.Status.HookJobTracker = tr
currentAndCompleted := sets.New(tr...)
// Add already completed jobs that might have been deleted back.
for _, s := range uj.Status.HookJobTracker {
if s.Status != managedupgradev1beta1.HookJobTrackerStatusActive {
currentAndCompleted.Insert(s)
}
}

if !sets.New(uj.Status.HookJobTracker...).Equal(currentAndCompleted) {
uj.Status.HookJobTracker = currentAndCompleted.UnsortedList()
slices.SortFunc(uj.Status.HookJobTracker, func(a, b managedupgradev1beta1.HookJobTracker) int {
return strings.Compare(a.UpgradeJobHookName+a.HookEvent, b.UpgradeJobHookName+b.HookEvent)
})
if err := r.Status().Update(ctx, &uj); err != nil {
return err
}
Expand Down
18 changes: 17 additions & 1 deletion controllers/upgradejob_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,10 @@ func checkAndCompleteHook(t *testing.T, c client.WithWatch, subject *UpgradeJobR
if fail {
ct = batchv1.JobFailed
}
expectedTrackedStatus := managedupgradev1beta1.HookJobTrackerStatusComplete
if fail {
expectedTrackedStatus = managedupgradev1beta1.HookJobTrackerStatusFailed
}

job := jobs.Items[0]
job.Status.Conditions = append(job.Status.Conditions, batchv1.JobCondition{
Expand All @@ -1110,7 +1114,19 @@ func checkAndCompleteHook(t *testing.T, c client.WithWatch, subject *UpgradeJobR
var uj managedupgradev1beta1.UpgradeJob
require.NoError(t, c.Get(ctx, requestForObject(upgradeJob).NamespacedName, &uj))
tracked := findTrackedHookJob(upgradeJobHook.Name, string(event), uj)
require.NotEmpty(t, tracked, "should have tracked hook job")
require.Len(t, tracked, 1, "should have tracked hook job")
require.Equal(t, expectedTrackedStatus, tracked[0].Status, "should track correct state")

// People might cleanup finished jobs or the TTL controller deletes them https://kubernetes.io/docs/concepts/workloads/controllers/ttlafterfinished/
// The controller should not fail in this case or try to recreate the job
require.NoError(t, c.Delete(ctx, &job))
reconcileNTimes(t, subject, ctx, requestForObject(upgradeJob), 3)
require.NoError(t, c.Get(ctx, requestForObject(upgradeJob).NamespacedName, &uj))
tracked = findTrackedHookJob(upgradeJobHook.Name, string(event), uj)
require.Len(t, tracked, 1, "should still have completed tracked hook job")
require.Equal(t, expectedTrackedStatus, tracked[0].Status, "should have kept the state")
require.NoError(t, c.List(ctx, &jobs, sel))
require.Lenf(t, jobs.Items, 0, "should not have recreated the deleted job")

return job
}

0 comments on commit 7be3f27

Please sign in to comment.