fix: RunnerDeployment should clean up old RunnerReplicaSets ASAP

Since the initial implementation of RunnerDeployment and until this change, any update to a runner deployment has been leaving old runner replicasets until the next resync interval. This fixes that, by continusouly retrying the reconcilation 10 seconds later to see if there are any old runner replicasets that can be removed.

In addition to that, the cleanup of old runner replicasets has been improved to be deferred until all the runners of the newest replica set to be available. This gives you hopefully zero or at less downtime updates of runner deployments.

Fixes #24

README.md

@@ -16,7 +16,9 @@ First, install *actions-runner-controller* with a manifest file. This will creat
 $ kubectl apply -f https://github.com/summerwind/actions-runner-controller/releases/latest/download/actions-runner-controller.yaml
 ```
 
-Set your access token of GitHub to the secret. `${GITHUB_TOKEN}` is the value you must replace with your access token. This token is used to register Self-hosted runner by *actions-runner-controller*.
+Next, from an account that has `admin` privileges for the repository, create a [personal access token](https://github.com/settings/tokens) with `repo` scope. This token is used to register a self-hosted runner by *actions-runner-controller*.
+
+Then, create a Kubernetes secret, replacing `${GITHUB_TOKEN}` with your token.
 
 ```
 $ kubectl create secret generic controller-manager --from-literal=github_token=${GITHUB_TOKEN} -n actions-runner-system
@@ -67,7 +69,7 @@ NAME           READY   STATUS    RESTARTS   AGE
 example-runner 2/2     Running   0          1m
 ```
 
-The runner you created has been registerd to your repository.
+The runner you created has been registered to your repository.
 
 <img width="756" alt="Actions tab in your repository settings" src="https://user-images.githubusercontent.com/230145/73618667-8cbf9700-466c-11ea-80b6-c67e6d3f70e7.png">
 

config/manager/manager.yaml

@@ -35,6 +35,25 @@ spec:
             secretKeyRef:
               name: controller-manager
               key: github_token
+              optional: true
+        - name: GITHUB_APP_ID
+          valueFrom:
+            secretKeyRef:
+              name: controller-manager
+              key: github_app_id
+              optional: true
+        - name: GITHUB_APP_INSTALLATION_ID
+          valueFrom:
+            secretKeyRef:
+              name: controller-manager
+              key: github_app_installation_id
+              optional: true
+        - name: GITHUB_APP_PRIVATE_KEY
+          value: /etc/actions-runner-controller/github_app_private_key
+        volumeMounts:
+        - name: controller-manager
+          mountPath: "/etc/actions-runner-controller"
+          readOnly: true
         resources:
           limits:
             cpu: 100m
@@ -42,4 +61,8 @@ spec:
           requests:
             cpu: 100m
             memory: 20Mi
+      volumes:
+      - name: controller-manager
+        secret:
+          secretName: controller-manager
       terminationGracePeriodSeconds: 10

config/rbac/role.yaml

@@ -66,6 +66,13 @@ rules:
   - get
   - patch
   - update
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
 - apiGroups:
   - ""
   resources:

controllers/runner_controller.go

@@ -72,6 +72,7 @@ type RunnerReconciler struct {
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=core,resources=pods,verbs=get;list;watch;create;update;patch;delete
+// +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch
 
 func (r *RunnerReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	ctx := context.Background()

controllers/runnerdeployment_controller.go

@@ -20,7 +20,9 @@ import (
 	"context"
 	"fmt"
 	"hash/fnv"
+	"k8s.io/apimachinery/pkg/types"
 	"sort"
+	"time"
 
 	"github.com/davecgh/go-spew/spew"
 	"github.com/go-logr/logr"
@@ -54,10 +56,11 @@ type RunnerDeploymentReconciler struct {
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerdeployments/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerreplicasets,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerreplicasets/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch
 
 func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	ctx := context.Background()
-	log := r.Log.WithValues("runnerreplicaset", req.NamespacedName)
+	log := r.Log.WithValues("runnerdeployment", req.NamespacedName)
 
 	var rd v1alpha1.RunnerDeployment
 	if err := r.Get(ctx, req.NamespacedName, &rd); err != nil {
@@ -129,12 +132,19 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e
 			return ctrl.Result{}, err
 		}
 
-		return ctrl.Result{}, nil
+		// We requeue in order to clean up old runner replica sets later.
+		// Otherwise, they aren't cleaned up until the next re-sync interval.
+		return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
 	}
 
+	const defaultReplicas = 1
+
+	currentDesiredReplicas := getIntOrDefault(newestSet.Spec.Replicas, defaultReplicas)
+	newDesiredReplicas := getIntOrDefault(desiredRS.Spec.Replicas, defaultReplicas)
+
 	// Please add more conditions that we can in-place update the newest runnerreplicaset without disruption
-	if newestSet.Spec.Replicas != desiredRS.Spec.Replicas {
-		newestSet.Spec.Replicas = desiredRS.Spec.Replicas
+	if currentDesiredReplicas != newDesiredReplicas {
+		newestSet.Spec.Replicas = &newDesiredReplicas
 
 		if err := r.Client.Update(ctx, newestSet); err != nil {
 			log.Error(err, "Failed to update runnerreplicaset resource")
@@ -142,25 +152,49 @@ func (r *RunnerDeploymentReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e
 			return ctrl.Result{}, err
 		}
 
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, err
 	}
 
-	for i := range oldSets {
-		rs := oldSets[i]
+	// Do we old runner replica sets that should eventually deleted?
+	if len(oldSets) > 0 {
+		readyReplicas := newestSet.Status.ReadyReplicas
 
-		if err := r.Client.Delete(ctx, &rs); err != nil {
-			log.Error(err, "Failed to delete runner resource")
+		if readyReplicas < currentDesiredReplicas {
+			log.WithValues("runnerreplicaset", types.NamespacedName{
+				Namespace: newestSet.Namespace,
+				Name:      newestSet.Name,
+			}).
+				Info("Waiting until the newest runner replica set to be 100% available")
 
-			return ctrl.Result{}, err
+			return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 		}
 
-		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)
+		for i := range oldSets {
+			rs := oldSets[i]
+
+			if err := r.Client.Delete(ctx, &rs); err != nil {
+				log.Error(err, "Failed to delete runner 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)
+		}
 	}
 
 	return ctrl.Result{}, nil
 }
 
+func getIntOrDefault(p *int, d int) int {
+	if p == nil {
+		return d
+	}
+
+	return *p
+}
+
 func getTemplateHash(rs *v1alpha1.RunnerReplicaSet) (string, bool) {
 	hash, ok := rs.Labels[LabelKeyRunnerTemplateHash]
 

controllers/runnerreplicaset_controller.go

@@ -45,6 +45,7 @@ type RunnerReplicaSetReconciler struct {
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runnerreplicasets/status,verbs=get;update;patch
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=actions.summerwind.dev,resources=runners/status,verbs=get;update;patch
+// +kubebuilder:rbac:groups=core,resources=events,verbs=create;patch
 
 func (r *RunnerReplicaSetReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	ctx := context.Background()

main.go

@@ -20,8 +20,11 @@ import (
 	"context"
 	"flag"
 	"fmt"
+	"net/http"
 	"os"
+	"strconv"
 
+	"github.com/bradleyfalzon/ghinstallation"
 	"github.com/google/go-github/v29/github"
 	actionsv1alpha1 "github.com/summerwind/actions-runner-controller/api/v1alpha1"
 	"github.com/summerwind/actions-runner-controller/controllers"
@@ -58,7 +61,13 @@ func main() {
 
 		runnerImage string
 		dockerImage string
-		ghToken     string
+
+		ghToken             string
+		ghAppID             int64
+		ghAppInstallationID int64
+		ghAppPrivateKey     string
+
+		ghClient *github.Client
 	)
 
 	flag.StringVar(&metricsAddr, "metrics-addr", ":8080", "The address the metric endpoint binds to.")
@@ -66,21 +75,57 @@ func main() {
 		"Enable leader election for controller manager. Enabling this will ensure there is only one active controller manager.")
 	flag.StringVar(&runnerImage, "runner-image", defaultRunnerImage, "The image name of self-hosted runner container.")
 	flag.StringVar(&dockerImage, "docker-image", defaultDockerImage, "The image name of docker sidecar container.")
-	flag.StringVar(&ghToken, "github-token", "", "The access token of GitHub.")
+	flag.StringVar(&ghToken, "github-token", "", "The personal access token of GitHub.")
+	flag.Int64Var(&ghAppID, "github-app-id", 0, "The application ID of GitHub App.")
+	flag.Int64Var(&ghAppInstallationID, "github-app-installation-id", 0, "The installation ID of GitHub App.")
+	flag.StringVar(&ghAppPrivateKey, "github-app-private-key", "", "The path of a private key file to authenticate as a GitHub App")
 	flag.Parse()
 
 	if ghToken == "" {
 		ghToken = os.Getenv("GITHUB_TOKEN")
 	}
-	if ghToken == "" {
-		fmt.Fprintln(os.Stderr, "Error: GitHub access token must be specified.")
-		os.Exit(1)
+	if ghAppID == 0 {
+		appID, err := strconv.ParseInt(os.Getenv("GITHUB_APP_ID"), 10, 64)
+		if err == nil {
+			ghAppID = appID
+		}
+	}
+	if ghAppInstallationID == 0 {
+		appInstallationID, err := strconv.ParseInt(os.Getenv("GITHUB_APP_INSTALLATION_ID"), 10, 64)
+		if err == nil {
+			ghAppInstallationID = appInstallationID
+		}
+	}
+	if ghAppPrivateKey == "" {
+		ghAppPrivateKey = os.Getenv("GITHUB_APP_PRIVATE_KEY")
 	}
 
-	tc := oauth2.NewClient(context.Background(), oauth2.StaticTokenSource(
-		&oauth2.Token{AccessToken: ghToken},
-	))
-	ghClient := github.NewClient(tc)
+	if ghAppID != 0 {
+		if ghAppInstallationID == 0 {
+			fmt.Fprintln(os.Stderr, "Error: The installation ID must be specified.")
+			os.Exit(1)
+		}
+
+		if ghAppPrivateKey == "" {
+			fmt.Fprintln(os.Stderr, "Error: The path of a private key file must be specified.")
+			os.Exit(1)
+		}
+
+		tr, err := ghinstallation.NewKeyFromFile(http.DefaultTransport, ghAppID, ghAppInstallationID, ghAppPrivateKey)
+		if err != nil {
+			fmt.Fprintf(os.Stderr, "Error: Invalid GitHub App credentials: %v\n", err)
+			os.Exit(1)
+		}
+		ghClient = github.NewClient(&http.Client{Transport: tr})
+	} else if ghToken != "" {
+		tc := oauth2.NewClient(context.Background(), oauth2.StaticTokenSource(
+			&oauth2.Token{AccessToken: ghToken},
+		))
+		ghClient = github.NewClient(tc)
+	} else {
+		fmt.Fprintln(os.Stderr, "Error: GitHub App credentials or personal access token must be specified.")
+		os.Exit(1)
+	}
 
 	ctrl.SetLogger(zap.New(func(o *zap.Options) {
 		o.Development = true

runner/Dockerfile

@@ -4,14 +4,16 @@ ARG RUNNER_VERSION
 ARG DOCKER_VERSION
 
 RUN apt update \
-  && apt install curl ca-certificates -y --no-install-recommends \
+  && apt install sudo curl ca-certificates -y --no-install-recommends \
   && curl -L -o docker.tgz https://download.docker.com/linux/static/stable/x86_64/docker-${DOCKER_VERSION}.tgz \
   && tar zxvf docker.tgz \
   && install -o root -g root -m 755 docker/docker /usr/local/bin/docker \
   && rm -rf docker docker.tgz \
   && curl -L -o /usr/local/bin/dumb-init https://github.com/Yelp/dumb-init/releases/download/v1.2.2/dumb-init_1.2.2_amd64 \
   && chmod +x /usr/local/bin/dumb-init \
-  && adduser --disabled-password --gecos "" --uid 1000 runner
+  && adduser --disabled-password --gecos "" --uid 1000 runner \
+  && usermod -aG sudo runner \
+  && echo "%sudo   ALL=(ALL:ALL) NOPASSWD:ALL" > /etc/sudoers
 
 RUN mkdir -p /runner \
   && cd /runner \

runner/Makefile

@@ -1,7 +1,7 @@
 NAME ?= summerwind/actions-runner
 
-RUNNER_VERSION ?= 2.165.2
-DOCKER_VERSION ?= 19.03.6
+RUNNER_VERSION ?= 2.168.0
+DOCKER_VERSION ?= 19.03.8
 
 docker-build:
 	docker build --build-arg RUNNER_VERSION=${RUNNER_VERSION} --build-arg DOCKER_VERSION=${DOCKER_VERSION} -t ${NAME}:latest -t ${NAME}:v${RUNNER_VERSION} .