From 1ec05a9165d0b1a7cfb83fc404c13fb1f2e0f926 Mon Sep 17 00:00:00 2001 From: Lukas Hybner Date: Mon, 18 Oct 2021 15:09:08 +0200 Subject: [PATCH 1/2] Unregister and delete completed pods Signed-off-by: Lukas Hybner --- controllers/githubactionrunner_controller.go | 9 ++++++++- controllers/podrunner_types.go | 4 ++-- controllers/podutil.go | 4 ++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/controllers/githubactionrunner_controller.go b/controllers/githubactionrunner_controller.go index 65dd35db..44e73157 100644 --- a/controllers/githubactionrunner_controller.go +++ b/controllers/githubactionrunner_controller.go @@ -359,11 +359,18 @@ func (r *GithubActionRunnerReconciler) unregisterRunner(ctx context.Context, cr // handleFinalization will remove runner from github based on presence of finalizer func (r *GithubActionRunnerReconciler) handleFinalization(ctx context.Context, cr *garov1alpha1.GithubActionRunner, list podRunnerPairList) error { - for _, item := range list.getPodsBeingDeletedOrEvicted() { + for _, item := range list.getPodsBeingDeletedOrEvictedOrCompleted() { // TODO - cause of failure should be checked more closely, if it does not exist we can ignore it. If it is a comms error we should stick around if err := r.unregisterRunner(ctx, cr, item); err != nil { return err } + if item.pod.Status.Phase == corev1.PodSucceeded { + logr.FromContext(ctx).Info("Deleting pod " + item.pod.Name) + err := r.DeleteResourceIfExists(ctx, &item.pod) + if err != nil { + return err + } + } } return nil diff --git a/controllers/podrunner_types.go b/controllers/podrunner_types.go index 01715a51..6f37a9d3 100644 --- a/controllers/podrunner_types.go +++ b/controllers/podrunner_types.go @@ -93,8 +93,8 @@ func (r podRunnerPairList) getIdles(sortOrder v1alpha1.SortOrder, minTTL time.Du return idles } -func (r podRunnerPairList) getPodsBeingDeletedOrEvicted() []podRunnerPair { +func (r podRunnerPairList) getPodsBeingDeletedOrEvictedOrCompleted() []podRunnerPair { return funk.Filter(r.pairs, func(pair podRunnerPair) bool { - return util.IsBeingDeleted(&pair.pod) || isEvicted(&pair.pod) + return util.IsBeingDeleted(&pair.pod) || isEvicted(&pair.pod) || isCompleted(&pair.pod) }).([]podRunnerPair) } diff --git a/controllers/podutil.go b/controllers/podutil.go index 3dffc375..f693bcf6 100644 --- a/controllers/podutil.go +++ b/controllers/podutil.go @@ -8,3 +8,7 @@ import ( func isEvicted(pod *v1.Pod) bool { return strings.Contains(pod.Status.Reason, "Evicted") } + +func isCompleted(pod *v1.Pod) bool { + return pod.Status.Phase == v1.PodSucceeded +} From 210c5378f7907b1c98740ba7e6145f0327fd7eec Mon Sep 17 00:00:00 2001 From: Lukas Hybner Date: Tue, 19 Oct 2021 14:53:22 +0200 Subject: [PATCH 2/2] fix after review Signed-off-by: Lukas Hybner --- controllers/githubactionrunner_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controllers/githubactionrunner_controller.go b/controllers/githubactionrunner_controller.go index 44e73157..1306da33 100644 --- a/controllers/githubactionrunner_controller.go +++ b/controllers/githubactionrunner_controller.go @@ -364,8 +364,8 @@ func (r *GithubActionRunnerReconciler) handleFinalization(ctx context.Context, c if err := r.unregisterRunner(ctx, cr, item); err != nil { return err } - if item.pod.Status.Phase == corev1.PodSucceeded { - logr.FromContext(ctx).Info("Deleting pod " + item.pod.Name) + if isCompleted(&item.pod) { + logr.FromContext(ctx).Info("Deleting succeeded pod", "podname", item.pod.Name) err := r.DeleteResourceIfExists(ctx, &item.pod) if err != nil { return err