Make patching of reference resource optional (#169)

* Added grafana.com/rollout-mirror-replicas-from-resource-update-status-replicas annotation to optionally disable patching of reference resource when using scaling based on reference resource.

* Review findings.

* CHANGELOG entry.

CHANGELOG.md

@@ -11,6 +11,7 @@
   * `k8s.io/client-go` from `v0.30.0` to `v0.30.3`
   * `sigs.k8s.io/controller-runtime` from `v0.18.1` to `v0.18.5`
 * [ENHANCEMENT] Update Go to `1.23`. #168
+* [ENHANCEMENT] When mirroring replicas of statefulset, rollout-operator can now skip writing back number of replicas to reference resource, by setting `grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas` annotation to `false`. #169
 
 ## v0.18.0
 

README.md

@@ -72,8 +72,12 @@ Rollout-operator can use custom resource with `scale` and `status` subresources
 * `grafana.com/rollout-mirror-replicas-from-resource-name`
 * `grafana.com/rollout-mirror-replicas-from-resource-kind`
 * `grafana.com/rollout-mirror-replicas-from-resource-api-version`
+* `grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas`
 
-These annotations must be set on StatefulSet that rollout-operator will scale (ie. target statefulset). Number of replicas in target statefulset will follow replicas in reference resource (from `scale` subresource), while reference resource's `status` subresource will be updated with current number of replicas in target statefulset.
+These annotations must be set on StatefulSet that rollout-operator will scale (ie. target statefulset).
+Number of replicas in target statefulset will follow replicas in reference resource (from `scale` subresource).
+Reference resource's `status` subresource will be updated with current number of replicas in target statefulset,
+unless explicitly disabled by setting `grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas` annotation to `false`.
 
 This is similar to using `grafana.com/rollout-downscale-leader`, but reference resource can be any kind of resource, not just statefulset. Furthermore `grafana.com/min-time-between-zones-downscale` is not respected when using scaling based on reference resource.
 

pkg/config/config.go

@@ -37,7 +37,8 @@ const (
 	// replicas based on replicas in this resource (its scale subresource).
 	RolloutMirrorReplicasFromResourceNameAnnotationKey       = rolloutMirrorReplicasFromResourceAnnotationKeyPrefix + "-name"
 	RolloutMirrorReplicasFromResourceKindAnnotationKey       = rolloutMirrorReplicasFromResourceAnnotationKeyPrefix + "-kind"
-	RolloutMirrorReplicasFromResourceAPIVersionAnnotationKey = rolloutMirrorReplicasFromResourceAnnotationKeyPrefix + "-api-version" // optional
+	RolloutMirrorReplicasFromResourceAPIVersionAnnotationKey = rolloutMirrorReplicasFromResourceAnnotationKeyPrefix + "-api-version"                // optional
+	RolloutMirrorReplicasFromResourceWriteBackStatusReplicas = rolloutMirrorReplicasFromResourceAnnotationKeyPrefix + "-write-back-status-replicas" // optional
 
 	// RolloutDelayedDownscaleAnnotationKey configures delay for downscaling. Prepare-url must be configured as well, and must support GET, POST and DELETE methods.
 	RolloutDelayedDownscaleAnnotationKey = "grafana.com/rollout-delayed-downscale"

pkg/controller/controller_test.go

@@ -410,6 +410,20 @@ func TestRolloutController_Reconcile(t *testing.T) {
 			expectedPatchedSets:               map[string][]string{"ingester-zone-b": {`{"spec":{"replicas":5}}`}},
 			expectedPatchedResources:          map[string][]string{"my.group/v1/customresources/test/status": {`{"status":{"replicas":5}}`}},
 		},
+		"should return early and scale up statefulset based on reference custom resource, but not patch the resource since it's disabled": {
+			statefulSets: []runtime.Object{
+				mockStatefulSet("ingester-zone-b", withReplicas(2, 2), withAnnotations(map[string]string{
+					"grafana.com/rollout-mirror-replicas-from-resource-name":                       "test",
+					"grafana.com/rollout-mirror-replicas-from-resource-kind":                       customResourceGVK.Kind,
+					"grafana.com/rollout-mirror-replicas-from-resource-api-version":                customResourceGVK.GroupVersion().String(),
+					"grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas": "false",
+				})),
+			},
+			customResourceScaleSpecReplicas:   5,
+			customResourceScaleStatusReplicas: 2,
+			expectedPatchedSets:               map[string][]string{"ingester-zone-b": {`{"spec":{"replicas":5}}`}},
+			expectedPatchedResources:          nil, // Reference resource is not patched.
+		},
 		"should return early and scale down statefulset based on reference custom resource": {
 			statefulSets: []runtime.Object{
 				mockStatefulSet("ingester-zone-b", withReplicas(3, 3), withAnnotations(map[string]string{
@@ -423,6 +437,20 @@ func TestRolloutController_Reconcile(t *testing.T) {
 			expectedPatchedSets:               map[string][]string{"ingester-zone-b": {`{"spec":{"replicas":2}}`}},
 			expectedPatchedResources:          map[string][]string{"my.group/v1/customresources/test/status": {`{"status":{"replicas":2}}`}},
 		},
+		"should return early and scale down statefulset based on reference custom resource, but not patch the resource since it's disabled": {
+			statefulSets: []runtime.Object{
+				mockStatefulSet("ingester-zone-b", withReplicas(3, 3), withAnnotations(map[string]string{
+					"grafana.com/rollout-mirror-replicas-from-resource-name":                       "test",
+					"grafana.com/rollout-mirror-replicas-from-resource-kind":                       customResourceGVK.Kind,
+					"grafana.com/rollout-mirror-replicas-from-resource-api-version":                customResourceGVK.GroupVersion().String(),
+					"grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas": "false",
+				})),
+			},
+			customResourceScaleSpecReplicas:   2,
+			customResourceScaleStatusReplicas: 3,
+			expectedPatchedSets:               map[string][]string{"ingester-zone-b": {`{"spec":{"replicas":2}}`}},
+			expectedPatchedResources:          nil, // Reference resource is not patched.
+		},
 		"should patch scale subresource status.replicas if it doesn't match statefulset": {
 			statefulSets: []runtime.Object{
 				mockStatefulSet("ingester-zone-b", withReplicas(3, 3), withAnnotations(map[string]string{
@@ -436,6 +464,20 @@ func TestRolloutController_Reconcile(t *testing.T) {
 			expectedPatchedSets:               nil,
 			expectedPatchedResources:          map[string][]string{"my.group/v1/customresources/test/status": {`{"status":{"replicas":3}}`}},
 		},
+		"should not patch scale subresource status.replicas since it's disabled, even though spec.replicas != statefulset.spec.replicas": {
+			statefulSets: []runtime.Object{
+				mockStatefulSet("ingester-zone-b", withReplicas(3, 3), withAnnotations(map[string]string{
+					"grafana.com/rollout-mirror-replicas-from-resource-name":                       "test",
+					"grafana.com/rollout-mirror-replicas-from-resource-kind":                       customResourceGVK.Kind,
+					"grafana.com/rollout-mirror-replicas-from-resource-api-version":                customResourceGVK.GroupVersion().String(),
+					"grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas": "false",
+				})),
+			},
+			customResourceScaleSpecReplicas:   3,
+			customResourceScaleStatusReplicas: 5,
+			expectedPatchedSets:               nil,
+			expectedPatchedResources:          nil, // Reference resource is not patched.
+		},
 		"should NOT patch scale subresource status.replicas if it already matches statefulset": {
 			statefulSets: []runtime.Object{
 				mockStatefulSet("ingester-zone-b", withReplicas(3, 3), withAnnotations(map[string]string{
@@ -478,16 +520,50 @@ func TestRolloutController_Reconcile(t *testing.T) {
 					"grafana.com/rollout-mirror-replicas-from-resource-kind":        customResourceGVK.Kind,
 					"grafana.com/rollout-mirror-replicas-from-resource-api-version": customResourceGVK.GroupVersion().String(),
 				})),
-				mockStatefulSet("ingester-zone-c", withReplicas(2, 2), withAnnotations(map[string]string{
+				mockStatefulSet("ingester-zone-c", withReplicas(5, 5), withAnnotations(map[string]string{
+					"grafana.com/rollout-mirror-replicas-from-resource-name":                       "test",
+					"grafana.com/rollout-mirror-replicas-from-resource-kind":                       customResourceGVK.Kind,
+					"grafana.com/rollout-mirror-replicas-from-resource-api-version":                customResourceGVK.GroupVersion().String(),
+					"grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas": "true", // Patching of reference resource is explicitly enabled.
+				})),
+				mockStatefulSet("ingester-zone-d", withReplicas(2, 2), withAnnotations(map[string]string{
 					"grafana.com/rollout-mirror-replicas-from-resource-name":        "test",
 					"grafana.com/rollout-mirror-replicas-from-resource-kind":        customResourceGVK.Kind,
 					"grafana.com/rollout-mirror-replicas-from-resource-api-version": customResourceGVK.GroupVersion().String(),
 				})),
 			},
 			customResourceScaleSpecReplicas:   5,
 			customResourceScaleStatusReplicas: 2,
-			expectedPatchedSets:               map[string][]string{"ingester-zone-c": {`{"spec":{"replicas":5}}`}},
-			expectedPatchedResources:          map[string][]string{"my.group/v1/customresources/test/status": {`{"status":{"replicas":5}}`, `{"status":{"replicas":5}}`}},
+			expectedPatchedSets:               map[string][]string{"ingester-zone-d": {`{"spec":{"replicas":5}}`}},
+			expectedPatchedResources:          map[string][]string{"my.group/v1/customresources/test/status": {`{"status":{"replicas":5}}`, `{"status":{"replicas":5}}`, `{"status":{"replicas":5}}`}},
+		},
+		"all statefulsets are considered, but only one patches the reference resource": {
+			statefulSets: []runtime.Object{
+				// Does NOT have scaling annotations
+				mockStatefulSet("ingester-zone-a", withReplicas(4, 4)),
+				// Is already scaled.
+				mockStatefulSet("ingester-zone-b", withReplicas(5, 5), withAnnotations(map[string]string{
+					"grafana.com/rollout-mirror-replicas-from-resource-name":                       "test",
+					"grafana.com/rollout-mirror-replicas-from-resource-kind":                       customResourceGVK.Kind,
+					"grafana.com/rollout-mirror-replicas-from-resource-api-version":                customResourceGVK.GroupVersion().String(),
+					"grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas": "false", // Explicitly disabled.
+				})),
+				mockStatefulSet("ingester-zone-c", withReplicas(5, 5), withAnnotations(map[string]string{
+					"grafana.com/rollout-mirror-replicas-from-resource-name":                       "test",
+					"grafana.com/rollout-mirror-replicas-from-resource-kind":                       customResourceGVK.Kind,
+					"grafana.com/rollout-mirror-replicas-from-resource-api-version":                customResourceGVK.GroupVersion().String(),
+					"grafana.com/rollout-mirror-replicas-from-resource-write-back-status-replicas": "bad value", // Disabled through wrong value.
+				})),
+				mockStatefulSet("ingester-zone-d", withReplicas(2, 2), withAnnotations(map[string]string{
+					"grafana.com/rollout-mirror-replicas-from-resource-name":        "test",
+					"grafana.com/rollout-mirror-replicas-from-resource-kind":        customResourceGVK.Kind,
+					"grafana.com/rollout-mirror-replicas-from-resource-api-version": customResourceGVK.GroupVersion().String(),
+				})),
+			},
+			customResourceScaleSpecReplicas:   5,
+			customResourceScaleStatusReplicas: 2,
+			expectedPatchedSets:               map[string][]string{"ingester-zone-d": {`{"spec":{"replicas":5}}`}},
+			expectedPatchedResources:          map[string][]string{"my.group/v1/customresources/test/status": {`{"status":{"replicas":5}}`}},
 		},
 	}
 
@@ -559,7 +635,7 @@ func TestRolloutController_Reconcile(t *testing.T) {
 			// Assert updated StatefulSets.
 			assert.Equal(t, testData.expectedUpdatedSets, updatedStsNames)
 
-			// Assert patched StatefulSets.
+			// Assert patched StatefulSets and resources.
 			assert.Equal(t, testData.expectedPatchedSets, convertEmptyMapToNil(patchedStatefulSets))
 			assert.Equal(t, testData.expectedPatchedResources, convertEmptyMapToNil(patchedStatuses))
 

pkg/controller/custom_resource_replicas.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"strconv"
 
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
@@ -148,20 +149,37 @@ func scaleForResourceMappings(ctx context.Context, namespace, name string, mappi
 
 // updateStatusReplicasOnReferenceResourceIfNeeded makes sure that scaleObject's status.replicas field is up-to-date.
 // if update fails, error is logged, but not returned to caller.
-func updateStatusReplicasOnReferenceResourceIfNeeded(ctx context.Context, log log.Logger, dynamicClient dynamic.Interface, sts *appsv1.StatefulSet, scaleObj *autoscalingv1.Scale, gvr schema.GroupVersionResource, resName string, replicas int32) {
+func updateStatusReplicasOnReferenceResourceIfNeeded(ctx context.Context, logger log.Logger, dynamicClient dynamic.Interface, sts *appsv1.StatefulSet, scaleObj *autoscalingv1.Scale, gvr schema.GroupVersionResource, resName string, replicas int32) {
 	if scaleObj.Status.Replicas == replicas {
 		// Nothing to do.
 		return
 	}
 
 	referenceResource := fmt.Sprintf("%s/%s", gvr.Resource, resName)
 
-	level.Info(log).Log("msg", "updating status.replicas on resource to match current replicas of statefulset", "name", sts.GetName(), "replicas", replicas, "referenceResource", referenceResource)
+	// Add common fields to logger.
+	logger = log.With(logger, "name", sts.GetName(), "replicas", replicas, "referenceResource", referenceResource)
+
+	// If annotation is not present, or equals to "true", we update. If annotation equals to "false" or fails to parse, we don't update.
+	updateReplicas, ok := sts.Annotations[config.RolloutMirrorReplicasFromResourceWriteBackStatusReplicas]
+	if ok {
+		update, err := strconv.ParseBool(updateReplicas)
+		if err != nil {
+			level.Info(logger).Log("msg", "not updating status.replicas on reference resource to match current replicas of statefulset, failed to parse "+config.RolloutMirrorReplicasFromResourceWriteBackStatusReplicas+" annotation", "err", err)
+			return
+		}
+		if !update {
+			level.Info(logger).Log("msg", "not updating status.replicas on reference resource to match current replicas of statefulset, updating disabled")
+			return
+		}
+	}
+
+	level.Info(logger).Log("msg", "updating status.replicas on reference resource to match current replicas of statefulset")
 
 	// We need to update status.replicas on the resource (status subresource), not on the scale subresource.
 	patch := fmt.Sprintf(`{"status":{"replicas":%d}}`, replicas)
 	_, err := dynamicClient.Resource(gvr).Namespace(sts.Namespace).Patch(ctx, resName, types.MergePatchType, []byte(patch), metav1.PatchOptions{}, "status")
 	if err != nil {
-		level.Warn(log).Log("msg", "updating status.replicas on reference resource to match current replicas of statefulset failed", "name", sts.GetName(), "replicas", replicas, "referenceResource", referenceResource, "err", err)
+		level.Warn(logger).Log("msg", "updating status.replicas on reference resource to match current replicas of statefulset failed", "err", err)
 	}
 }