From c612e87d85b069645c677fdaca26cf1dc9a5fa31 Mon Sep 17 00:00:00 2001
From: Yusuke Kuoka <ykuoka@gmail.com>
Date: Sun, 13 Mar 2022 12:18:22 +0000
Subject: [PATCH] fix: Let RunnerDeployment scale RunnerReplicaSet to zero
 before terminating it

so that hopefully RunnerDeployment can gracefully termiante older RunnerReplicaSet on update.
---
 controllers/runnerdeployment_controller.go | 27 ++++++++++++++++++++--
 controllers/runnerreplicaset_controller.go |  4 ++++
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/controllers/runnerdeployment_controller.go b/controllers/runnerdeployment_controller.go
index 9a55ae3d..f3a2ba7d 100644
--- a/controllers/runnerdeployment_controller.go
+++ b/controllers/runnerdeployment_controller.go
@@ -222,15 +222,38 @@ func (r *RunnerDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		for i := range oldSets {
 			rs := oldSets[i]
 
+			rslog := log.WithValues("runnerreplicaset", rs.Name)
+
+			if rs.Status.Replicas != nil && *rs.Status.Replicas > 0 {
+				if rs.Spec.Replicas != nil && *rs.Spec.Replicas == 0 {
+					rslog.V(2).Info("Waiting for runnerreplicaset to scale to zero")
+
+					continue
+				}
+
+				updated := rs.DeepCopy()
+				zero := 0
+				updated.Spec.Replicas = &zero
+				if err := r.Client.Update(ctx, updated); err != nil {
+					rslog.Error(err, "Failed to scale runnerreplicaset to zero")
+
+					return ctrl.Result{}, err
+				}
+
+				rslog.Info("Scaled runnerreplicaset to zero")
+
+				continue
+			}
+
 			if err := r.Client.Delete(ctx, &rs); err != nil {
-				log.Error(err, "Failed to delete runnerreplicaset resource")
+				rslog.Error(err, "Failed to delete runnerreplicaset resource")
 
 				return ctrl.Result{}, err
 			}
 
 			r.Recorder.Event(&rd, corev1.EventTypeNormal, "RunnerReplicaSetDeleted", fmt.Sprintf("Deleted runnerreplicaset '%s'", rs.Name))
 
-			log.Info("Deleted runnerreplicaset", "runnerdeployment", rd.ObjectMeta.Name, "runnerreplicaset", rs.Name)
+			rslog.Info("Deleted runnerreplicaset")
 		}
 	}
 
diff --git a/controllers/runnerreplicaset_controller.go b/controllers/runnerreplicaset_controller.go
index 3433fc58..6ceb559b 100644
--- a/controllers/runnerreplicaset_controller.go
+++ b/controllers/runnerreplicaset_controller.go
@@ -65,6 +65,10 @@ func (r *RunnerReplicaSetReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 
 	if !rs.ObjectMeta.DeletionTimestamp.IsZero() {
+		// RunnerReplicaSet cannot be gracefuly removed.
+		// That means any runner that is running a job can be prematurely terminated.
+		// To gracefully remove a RunnerReplicaSet, scale it down to zero first, observe RunnerReplicaSet's status replicas,
+		// and remove it only after the status replicas becomes zero.
 		return ctrl.Result{}, nil
 	}