add patch option and clean up reconcile loop (#12)

README.md

@@ -361,6 +361,7 @@ spec:
     workVolumeClaimTemplate:  # Optional. Default is a 5Gi volume on default storage.
     limits:                   # Optional. Default: {cpu:2,memory:2000Mi}
     requests:                 # Optional. Default: {cpu:200m,memory:200Mi}
+    patch                     # Optional. Default: ""
 
 ```
 
@@ -369,6 +370,7 @@ Again most of the fields are self explanatory except maybe:
 - ScaleFactor controls how the number of queued jobs relates to the number of runners. Setting it to 0 makes it scale linearly up to maxRunners any other factor gets passed to [a simplified version of the logistic function](https://www.desmos.com/calculator/o6mpkilyxl) which allows the number of runners to be scaled up eagerly in response to demand.
 - MetricsSelector allows you to specify which metrics will be used. For instance if you wanted to target a specific workflow then you could specify "wf_name=main" or if you wanted to scale on workflows which target runners with the runner label "deploy" then you could specify "wf_runs_on_deploy".
 - Runner allows you to modify the StatefulSet that is produced, you can specify the image, labels, requests, limits and persistentVolumeClaim
+- Runner.Patch accepts a RFC6092 JSON patch which gets applied to the stateful set **spec**. This is essentially just a way of shoehorning in other changes. Be mindful that the operator is constantly reconciling. So favor replace over add operations (if you add an item to an array then it will add it over and over.)
 - Scaling allows you to modify the [ScaledObject](https://keda.sh/docs/1.4/concepts/scaling-deployments/#scaledobject-spec) that is created
 
 ## Rate limits

charts/github-runner-autoscaler/Chart.yaml

@@ -5,5 +5,5 @@ home: https://github.com/devjoes/github-runner-autoscaler
 sources:
   - https://github.com/devjoes/github-runner-autoscaler
 type: application
-version: 0.1.5
-appVersion: v0.1.5
+version: 0.1.6
+appVersion: v0.1.6

charts/github-runner-autoscaler/templates/crds.yaml

@@ -288,6 +288,8 @@ spec:
                     additionalProperties:
                       type: string
                     type: object
+                  patch:
+                    type: string
                   requests:
                     additionalProperties:
                       anyOf:

charts/github-runner-autoscaler/values.yaml

@@ -1,4 +1,4 @@
 replicaCount: 1
 
-image: joeshearn/github-runner-autoscaler-operator:v0.1.5
+image: joeshearn/github-runner-autoscaler-operator:v0.1.6
 imagePullPolicy: IfNotPresent

operator/api/v1alpha1/scaledactionrunner_types.go

@@ -62,6 +62,7 @@ type Runner struct {
 	Tolerations             []corev1.Toleration                        `json:"tolerations,omitempty"`
 	ServiceAccountName      string                                     `json:"serviceAccountName,omitempty"`
 	MountDockerSock         *bool                                      `json:"mountDockerSock,omitempty"`
+	Patch                   string                                     `json:"patch,omitempty"`
 }
 
 type Scaling struct {

operator/config/crd/bases/runner.devjoes.com_scaledactionrunners.yaml

@@ -183,6 +183,8 @@ spec:
                     additionalProperties:
                       type: string
                     type: object
+                  patch:
+                    type: string
                   requests:
                     additionalProperties:
                       anyOf:

operator/controllers/scaledactionrunner_controller.go

@@ -325,6 +325,15 @@ func (r *ScaledActionRunnerReconciler) syncStatefulSet(ctx context.Context, log
 	if err != nil && errors.IsNotFound(err) {
 		(resourceLog(log, "Creating a new StatefulSet %s", newSs))
 		ctrl.SetControllerReference(config, newSs, r.Scheme)
+		patchedSs, _, err := sargenerator.PatchStatefulSet(newSs, config)
+
+		if err != nil {
+			log.Error(err, "Failed to patch new StatefulSet "+newSs.GetResourceVersion())
+			return false, err
+		}
+		if patchedSs != nil {
+			newSs = patchedSs
+		}
 		err = r.Create(ctx, newSs)
 		if err != nil {
 			log.Error(err, "Failed to create new StatefulSet "+newSs.Name)
@@ -338,6 +347,25 @@ func (r *ScaledActionRunnerReconciler) syncStatefulSet(ctx context.Context, log
 	}
 
 	updatedSs := getScaledSetUpdates(existingSs, config, secretsHash)
+	unpatchedSs := existingSs
+	if updatedSs != nil {
+		unpatchedSs = updatedSs
+	}
+
+	patchedSs, patchedHash, err := sargenerator.PatchStatefulSet(unpatchedSs, config)
+	if err != nil {
+		log.Error(err, "Failed to patch StatefulSet "+unpatchedSs.GetResourceVersion())
+		return false, err
+	}
+
+	if existingSs != nil && existingSs.Annotations != nil && patchedHash != "" &&
+		patchedHash == existingSs.Annotations[sargenerator.AnnotationRunnerPatchHash] {
+		// The patched object's has is identical to the one deployed so ignore
+		updatedSs = nil
+	} else if patchedSs != nil {
+		updatedSs = patchedSs
+	}
+
 	if updatedSs != nil {
 		resourceLog(log, "Deleting and recreating StatefulSet %s", updatedSs)
 		changes, err := diff.Diff(existingSs, updatedSs)
@@ -443,7 +471,12 @@ func getScaledSetUpdates(oldSs *appsv1.StatefulSet, config *runnerv1alpha1.Scale
 			updatedSs.Spec.Template.Spec.Tolerations = config.Spec.Runner.Tolerations
 			updated = true
 		}
+		if oldSs.Spec.Template.Spec.ServiceAccountName != config.Spec.Runner.ServiceAccountName {
+			updatedSs.Spec.Template.Spec.ServiceAccountName = config.Spec.Runner.ServiceAccountName
+			updated = true
+		}
 	}
+
 	if updated {
 		return updatedSs
 	}

operator/coregenerator/coregenerator.go

@@ -342,7 +342,10 @@ func GeneratePrometheusServiceMonitor(c *runnerv1alpha1.ScaledActionRunnerCore)
 	smJson = strings.ReplaceAll(smJson, "sm-ns-name", c.Spec.PrometheusNamespace)
 	smJson = strings.ReplaceAll(smJson, "api-ns-name", c.Spec.ApiServerNamespace)
 	err := json.Unmarshal([]byte(smJson), &sm)
-	fmt.Println(err)
+	if err != nil {
+		fmt.Println(err)
+	}
+	sm.Annotations[CrdKey] = getKey(c)
 	sm.GetObjectKind().SetGroupVersionKind(schema.FromAPIVersionAndKind("monitoring.coreos.com/v1", "ServiceMonitor"))
 	return []client.Object{&sm}
 }
@@ -700,7 +703,8 @@ const JsonServiceMonitor = `{
 	  "labels": {
 		"app": "github-action-apiserver",
 		"release": "prometheus"
-	  }
+	  },
+	  "annotations":{}
 	},
 	"spec": {
 	  "selector": {

operator/go.mod

@@ -3,6 +3,7 @@ module github.com/devjoes/github-runner-autoscaler/operator
 go 1.15
 
 require (
+	github.com/evanphx/json-patch v4.9.0+incompatible // indirect
 	github.com/go-logr/logr v0.4.0
 	github.com/kedacore/keda/v2 v2.2.0
 	github.com/onsi/ginkgo v1.15.2

operator/sargenerator/sargenerator.go

@@ -1,10 +1,14 @@
 package sargenerator
 
 import (
+	"crypto/sha1"
+	"encoding/base64"
+	"encoding/json"
 	"fmt"
 	"reflect"
 
 	runnerv1alpha1 "github.com/devjoes/github-runner-autoscaler/operator/api/v1alpha1"
+	jsonpatch "github.com/evanphx/json-patch"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 
@@ -14,6 +18,7 @@ import (
 
 const AnnotationRunnerRef = "runner-ref"
 const AnnotationSecretsHash = "runner-secrets-hash"
+const AnnotationRunnerPatchHash = "patch-hash"
 
 func getLabels(res metav1.Object) map[string]string {
 	ls := res.GetLabels()
@@ -234,3 +239,38 @@ func SetEnvVars(c *runnerv1alpha1.ScaledActionRunner, statefulSet *appsv1.Statef
 	}
 	return modified
 }
+
+func PatchStatefulSet(statefulSet *appsv1.StatefulSet, config *runnerv1alpha1.ScaledActionRunner) (*appsv1.StatefulSet, string, error) {
+	if config.Spec.Runner == nil || config.Spec.Runner.Patch == "" {
+		return nil, "", nil
+	}
+	patch, err := jsonpatch.DecodePatch([]byte(config.Spec.Runner.Patch))
+	if err != nil {
+		return nil, "", err
+	}
+	ssJson, err := json.Marshal(statefulSet.Spec)
+	if err != nil {
+		return nil, "", err
+	}
+	patchedJson, err := patch.Apply(ssJson)
+	if err != nil {
+		return nil, "", err
+	}
+	var patchedSsSpec appsv1.StatefulSetSpec
+	err = json.Unmarshal(patchedJson, &patchedSsSpec)
+	if err != nil {
+		return nil, "", err
+	}
+
+	b := sha1.Sum([]byte(patchedJson))
+	patchedHash := base64.RawStdEncoding.EncodeToString(b[:])
+
+	anns := statefulSet.GetAnnotations()
+	if anns[AnnotationRunnerPatchHash] != patchedHash {
+		newStatefulSet := statefulSet.DeepCopy()
+		newStatefulSet.Annotations[AnnotationRunnerPatchHash] = patchedHash
+		newStatefulSet.Spec = patchedSsSpec
+		return newStatefulSet, patchedHash, nil
+	}
+	return nil, patchedHash, nil
+}

operator/sargenerator/sargenerator_test.go

@@ -5,6 +5,8 @@ import (
 
 	"github.com/devjoes/github-runner-autoscaler/operator/api/v1alpha1"
 	"github.com/stretchr/testify/assert"
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -24,4 +26,120 @@ func TestCreatesScaledObject(t *testing.T) {
 	assert.Equal(t, sar.ObjectMeta.Namespace, so.Namespace)
 }
 
+func TestDoesNothingIfPatchIsMissing(t *testing.T) {
+	ss := getTestSs()
+	result, hash, err := PatchStatefulSet(ss, &v1alpha1.ScaledActionRunner{
+		Spec: v1alpha1.ScaledActionRunnerSpec{},
+	})
+	assert.Nil(t, result)
+	assert.Equal(t, "", hash)
+	assert.Nil(t, err)
+
+	result, hash, err = PatchStatefulSet(ss, &v1alpha1.ScaledActionRunner{
+		Spec: v1alpha1.ScaledActionRunnerSpec{
+			Runner: &v1alpha1.Runner{},
+		},
+	})
+	assert.Nil(t, result)
+	assert.Equal(t, "", hash)
+	assert.Nil(t, err)
+}
+
+func TestAppliesPatchAndSetsAnnotation(t *testing.T) {
+	ss := getTestSs()
+	result, hash, err := PatchStatefulSet(ss, &v1alpha1.ScaledActionRunner{
+		Spec: v1alpha1.ScaledActionRunnerSpec{
+			Runner: &v1alpha1.Runner{
+				Patch: `[{"op": "replace", "path": "/serviceName", "value": "replaced"}]`,
+			},
+		},
+	})
+	assert.NotNil(t, result)
+	assert.Nil(t, err)
+	assert.Equal(t, "replaced", result.Spec.ServiceName)
+	assert.NotEqual(t, "", result.Annotations[AnnotationRunnerPatchHash])
+	assert.NotEqual(t, "", hash)
+}
+
+func TestDoesNotApplyPatchTwice(t *testing.T) {
+	ss := getTestSs()
+
+	result, hash1, err := PatchStatefulSet(ss, &v1alpha1.ScaledActionRunner{
+		Spec: v1alpha1.ScaledActionRunnerSpec{
+			Runner: &v1alpha1.Runner{
+				Patch: `[{"op": "replace", "path": "/serviceName", "value": "replaced"}]`,
+			},
+		},
+	})
+	assert.Nil(t, err)
+	assert.NotEqual(t, hash1, ss.ObjectMeta.Annotations[AnnotationRunnerPatchHash])
+	assert.Equal(t, hash1, result.ObjectMeta.Annotations[AnnotationRunnerPatchHash])
+	assert.NotEqual(t, "", hash1)
+
+	ss = getTestSs()
+	ss.ObjectMeta.Annotations[AnnotationRunnerPatchHash] = hash1
+
+	result, hash2, err := PatchStatefulSet(ss, &v1alpha1.ScaledActionRunner{
+		Spec: v1alpha1.ScaledActionRunnerSpec{
+			Runner: &v1alpha1.Runner{
+				Patch: `[{"op": "replace", "path": "/serviceName", "value": "replaced"}]`,
+			},
+		},
+	})
+	assert.Nil(t, result)
+	assert.Equal(t, hash1, hash2)
+	assert.Nil(t, err)
+
+}
+
+func TestTriggersDeletionIfPatchChanges(t *testing.T) {
+	ss := getTestSs()
+	_, hash1, err := PatchStatefulSet(ss, &v1alpha1.ScaledActionRunner{
+		Spec: v1alpha1.ScaledActionRunnerSpec{
+			Runner: &v1alpha1.Runner{
+				Patch: `[{"op": "replace", "path": "/serviceName", "value": "replaced"}]`,
+			},
+		},
+	})
+	assert.Nil(t, err)
+	_, hash2, err := PatchStatefulSet(ss, &v1alpha1.ScaledActionRunner{
+		Spec: v1alpha1.ScaledActionRunnerSpec{
+			Runner: &v1alpha1.Runner{
+				Patch: `[{"op": "replace", "path": "/serviceName", "value": "different"}]`,
+			},
+		},
+	})
+	assert.NotEqual(t, "", hash1)
+	assert.NotEqual(t, "", hash2)
+	assert.NotEqual(t, hash1, hash2)
+	assert.Nil(t, err)
+}
+
+func getTestSs() *appsv1.StatefulSet {
+	var replicas int32 = 2
+	ss := appsv1.StatefulSet{
+		TypeMeta:   v1.TypeMeta{},
+		ObjectMeta: v1.ObjectMeta{Name: "foo", Namespace: "bar", Annotations: map[string]string{}},
+		Spec: appsv1.StatefulSetSpec{
+			Replicas: &replicas,
+			Selector: &v1.LabelSelector{MatchLabels: map[string]string{"a": "b"}},
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: v1.ObjectMeta{Name: "foo", Namespace: "bar"},
+				Spec: corev1.PodSpec{
+					Volumes:        []corev1.Volume{},
+					InitContainers: []corev1.Container{},
+					Containers:     []corev1.Container{corev1.Container{Name: "baz"}},
+				},
+			},
+			VolumeClaimTemplates: []corev1.PersistentVolumeClaim{},
+			ServiceName:          "foo",
+			PodManagementPolicy:  "",
+			UpdateStrategy:       appsv1.StatefulSetUpdateStrategy{},
+			RevisionHistoryLimit: new(int32),
+		},
+		Status: appsv1.StatefulSetStatus{},
+	}
+	return &ss
+}
+
 // This is mostly tested in scaledactionrunner_controller_test