feat(heal): optionally protect the redis master from autoscaler eviction

README.md

@@ -15,6 +15,7 @@ Redis Operator creates/configures/manages redis-failovers atop Kubernetes.
 - [Usage](#usage)
   - [Reducing update churn (`--enable-hash`)](#reducing-update-churn---enable-hash)
   - [Reconcile interval (`--resync-period`)](#reconcile-interval---resync-period)
+  - [Protect the master from cluster-autoscaler eviction](#protect-the-master-from-cluster-autoscaler-eviction)
   - [Sentinel update strategy and PodDisruptionBudget](#sentinel-update-strategy-and-poddisruptionbudget)
   - [Persistence](#persistence)
   - [NodeAffinity and Tolerations](#nodeaffinity-and-tolerations)
@@ -176,6 +177,13 @@ interval so it can recover from missed events and correct drift. This defaults t
 tuned with `--resync-period` (any Go duration, e.g. `--resync-period=1m`). Larger values reduce API
 load at the cost of slower periodic healing; smaller values react faster but poll more often.
 
+### Protect the master from cluster-autoscaler eviction
+
+Setting `redis.preventMasterEviction: true` makes the operator annotate the current master pod with
+`cluster-autoscaler.kubernetes.io/safe-to-evict: "false"` (and mark slaves `"true"`), so the
+cluster-autoscaler will not drain the node running the master and trigger an avoidable failover. The
+annotation follows the master as it moves. Defaults to `false` (no annotation is managed).
+
 ### Sentinel update strategy and PodDisruptionBudget
 
 The sentinel `Deployment` update strategy can be overridden via `sentinel.strategy` (e.g. to set

api/redisfailover/v1/types.go

@@ -77,6 +77,11 @@ type RedisSettings struct {
 	// PodDisruptionBudgetMinAvailable overrides the PodDisruptionBudget
 	// minAvailable for the redis pods. Defaults to 2 (or 1 when replicas <= 2).
 	PodDisruptionBudgetMinAvailable *intstr.IntOrString `json:"podDisruptionBudgetMinAvailable,omitempty"`
+	// PreventMasterEviction, when true, annotates the current master pod with
+	// cluster-autoscaler.kubernetes.io/safe-to-evict=false so the cluster
+	// autoscaler will not drain the node running the master. Slaves are marked
+	// evictable. Defaults to false.
+	PreventMasterEviction bool `json:"preventMasterEviction,omitempty"`
 }
 
 // SentinelSettings defines the specification of the sentinel cluster

charts/redisoperator/crds/databases.spotahome.com_redisfailovers.yaml

@@ -7325,6 +7325,13 @@ spec:
                   port:
                     format: int32
                     type: integer
+                  preventMasterEviction:
+                    description: |-
+                      PreventMasterEviction, when true, annotates the current master pod with
+                      cluster-autoscaler.kubernetes.io/safe-to-evict=false so the cluster
+                      autoscaler will not drain the node running the master. Slaves are marked
+                      evictable. Defaults to false.
+                    type: boolean
                   priorityClassName:
                     type: string
                   replicas:

manifests/databases.spotahome.com_redisfailovers.yaml

@@ -7325,6 +7325,13 @@ spec:
                   port:
                     format: int32
                     type: integer
+                  preventMasterEviction:
+                    description: |-
+                      PreventMasterEviction, when true, annotates the current master pod with
+                      cluster-autoscaler.kubernetes.io/safe-to-evict=false so the cluster
+                      autoscaler will not drain the node running the master. Slaves are marked
+                      evictable. Defaults to false.
+                    type: boolean
                   priorityClassName:
                     type: string
                   replicas:

manifests/kustomize/base/databases.spotahome.com_redisfailovers.yaml

@@ -7325,6 +7325,13 @@ spec:
                   port:
                     format: int32
                     type: integer
+                  preventMasterEviction:
+                    description: |-
+                      PreventMasterEviction, when true, annotates the current master pod with
+                      cluster-autoscaler.kubernetes.io/safe-to-evict=false so the cluster
+                      autoscaler will not drain the node running the master. Slaves are marked
+                      evictable. Defaults to false.
+                    type: boolean
                   priorityClassName:
                     type: string
                   replicas:

operator/redisfailover/service/check.go

@@ -84,22 +84,48 @@ func (r *RedisFailoverChecker) CheckSentinelNumber(rf *redisfailoverv1.RedisFail
 	return nil
 }
 
-func (r *RedisFailoverChecker) setMasterLabelIfNecessary(namespace string, pod corev1.Pod) error {
+// applyMasterEvictionAnnotation keeps the cluster-autoscaler safe-to-evict
+// annotation in sync with a pod's role, but only when the RedisFailover opts in
+// via spec.redis.preventMasterEviction. The master is pinned (false) and slaves
+// are marked evictable (true). It reads the desired state off the already-fetched
+// pod object and skips the patch when the annotation is already correct, so it is
+// safe to call on every reconcile without extra API writes.
+func applyMasterEvictionAnnotation(k8sService k8s.Services, rf *redisfailoverv1.RedisFailover, pod corev1.Pod, isMaster bool) error {
+	if !rf.Spec.Redis.PreventMasterEviction {
+		return nil
+	}
+	desired := "true"
+	if isMaster {
+		desired = "false"
+	}
+	if pod.Annotations[masterSafeToEvictAnnotation] == desired {
+		return nil
+	}
+	return k8sService.UpdatePodAnnotations(rf.Namespace, pod.Name, map[string]string{masterSafeToEvictAnnotation: desired})
+}
+
+func (r *RedisFailoverChecker) setMasterLabelIfNecessary(rf *redisfailoverv1.RedisFailover, pod corev1.Pod) error {
+	if err := applyMasterEvictionAnnotation(r.k8sService, rf, pod, true); err != nil {
+		return err
+	}
 	for labelKey, labelValue := range pod.Labels {
 		if labelKey == redisRoleLabelKey && labelValue == redisRoleLabelMaster {
 			return nil
 		}
 	}
-	return r.k8sService.UpdatePodLabels(namespace, pod.Name, generateRedisMasterRoleLabel())
+	return r.k8sService.UpdatePodLabels(rf.Namespace, pod.Name, generateRedisMasterRoleLabel())
 }
 
-func (r *RedisFailoverChecker) setSlaveLabelIfNecessary(namespace string, pod corev1.Pod) error {
+func (r *RedisFailoverChecker) setSlaveLabelIfNecessary(rf *redisfailoverv1.RedisFailover, pod corev1.Pod) error {
+	if err := applyMasterEvictionAnnotation(r.k8sService, rf, pod, false); err != nil {
+		return err
+	}
 	for labelKey, labelValue := range pod.Labels {
 		if labelKey == redisRoleLabelKey && labelValue == redisRoleLabelSlave {
 			return nil
 		}
 	}
-	return r.k8sService.UpdatePodLabels(namespace, pod.Name, generateRedisSlaveRoleLabel())
+	return r.k8sService.UpdatePodLabels(rf.Namespace, pod.Name, generateRedisSlaveRoleLabel())
 }
 
 // CheckAllSlavesFromMaster controlls that all slaves have the same master (the real one)
@@ -123,12 +149,12 @@ func (r *RedisFailoverChecker) CheckAllSlavesFromMaster(master string, rf *redis
 	var wrongMasterErr error
 	for _, rp := range rps.Items {
 		if rp.Status.PodIP == master {
-			err = r.setMasterLabelIfNecessary(rf.Namespace, rp)
+			err = r.setMasterLabelIfNecessary(rf, rp)
 			if err != nil {
 				return err
 			}
 		} else {
-			err = r.setSlaveLabelIfNecessary(rf.Namespace, rp)
+			err = r.setSlaveLabelIfNecessary(rf, rp)
 			if err != nil {
 				return err
 			}

operator/redisfailover/service/constants.go

@@ -34,3 +34,7 @@ const (
 	redisRoleLabelMaster = "master"
 	redisRoleLabelSlave  = "slave"
 )
+
+// masterSafeToEvictAnnotation is the cluster-autoscaler annotation used to keep
+// the node running the redis master from being drained during scale-down.
+const masterSafeToEvictAnnotation = "cluster-autoscaler.kubernetes.io/safe-to-evict"

operator/redisfailover/service/heal.go

@@ -43,22 +43,28 @@ func NewRedisFailoverHealer(k8sService k8s.Services, redisClient redis.Client, l
 	}
 }
 
-func (r *RedisFailoverHealer) setMasterLabelIfNecessary(namespace string, pod v1.Pod) error {
+func (r *RedisFailoverHealer) setMasterLabelIfNecessary(rf *redisfailoverv1.RedisFailover, pod v1.Pod) error {
+	if err := applyMasterEvictionAnnotation(r.k8sService, rf, pod, true); err != nil {
+		return err
+	}
 	for labelKey, labelValue := range pod.Labels {
 		if labelKey == redisRoleLabelKey && labelValue == redisRoleLabelMaster {
 			return nil
 		}
 	}
-	return r.k8sService.UpdatePodLabels(namespace, pod.Name, generateRedisMasterRoleLabel())
+	return r.k8sService.UpdatePodLabels(rf.Namespace, pod.Name, generateRedisMasterRoleLabel())
 }
 
-func (r *RedisFailoverHealer) setSlaveLabelIfNecessary(namespace string, pod v1.Pod) error {
+func (r *RedisFailoverHealer) setSlaveLabelIfNecessary(rf *redisfailoverv1.RedisFailover, pod v1.Pod) error {
+	if err := applyMasterEvictionAnnotation(r.k8sService, rf, pod, false); err != nil {
+		return err
+	}
 	for labelKey, labelValue := range pod.Labels {
 		if labelKey == redisRoleLabelKey && labelValue == redisRoleLabelSlave {
 			return nil
 		}
 	}
-	return r.k8sService.UpdatePodLabels(namespace, pod.Name, generateRedisSlaveRoleLabel())
+	return r.k8sService.UpdatePodLabels(rf.Namespace, pod.Name, generateRedisSlaveRoleLabel())
 }
 
 func (r *RedisFailoverHealer) MakeMaster(ip string, rf *redisfailoverv1.RedisFailover) error {
@@ -79,7 +85,7 @@ func (r *RedisFailoverHealer) MakeMaster(ip string, rf *redisfailoverv1.RedisFai
 	}
 	for _, rp := range rps.Items {
 		if rp.Status.PodIP == ip {
-			return r.setMasterLabelIfNecessary(rf.Namespace, rp)
+			return r.setMasterLabelIfNecessary(rf, rp)
 		}
 	}
 	return nil
@@ -117,7 +123,7 @@ func (r *RedisFailoverHealer) SetOldestAsMaster(rf *redisfailoverv1.RedisFailove
 				continue
 			}
 
-			err = r.setMasterLabelIfNecessary(rf.Namespace, pod)
+			err = r.setMasterLabelIfNecessary(rf, pod)
 			if err != nil {
 				return err
 			}
@@ -129,7 +135,7 @@ func (r *RedisFailoverHealer) SetOldestAsMaster(rf *redisfailoverv1.RedisFailove
 				r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("namespace", rf.ObjectMeta.Namespace).Errorf("Make slave failed, slave pod ip: %s, master ip: %s, error: %v", pod.Status.PodIP, newMasterIP, err)
 			}
 
-			err = r.setSlaveLabelIfNecessary(rf.Namespace, pod)
+			err = r.setSlaveLabelIfNecessary(rf, pod)
 			if err != nil {
 				return err
 			}
@@ -174,7 +180,7 @@ func (r *RedisFailoverHealer) SetMasterOnAll(masterIP string, rf *redisfailoverv
 				continue
 			}
 
-			err = r.setSlaveLabelIfNecessary(rf.Namespace, pod)
+			err = r.setSlaveLabelIfNecessary(rf, pod)
 			if err != nil {
 				return err
 			}

operator/redisfailover/service/master_eviction_annotation_test.go

@@ -0,0 +1,65 @@
+package service
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	redisfailoverv1 "github.com/dnse-tech/redis-operator/api/redisfailover/v1"
+	mK8SService "github.com/dnse-tech/redis-operator/mocks/service/k8s"
+)
+
+func rfWithEvictionProtection(enabled bool) *redisfailoverv1.RedisFailover {
+	return &redisfailoverv1.RedisFailover{
+		ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "testns"},
+		Spec: redisfailoverv1.RedisFailoverSpec{
+			Redis: redisfailoverv1.RedisSettings{PreventMasterEviction: enabled},
+		},
+	}
+}
+
+func podNamed(name string, annotations map[string]string) corev1.Pod {
+	return corev1.Pod{ObjectMeta: metav1.ObjectMeta{Name: name, Annotations: annotations}}
+}
+
+func TestApplyMasterEvictionAnnotation(t *testing.T) {
+	t.Run("flag disabled makes no annotation call", func(t *testing.T) {
+		ms := &mK8SService.Services{}
+		err := applyMasterEvictionAnnotation(ms, rfWithEvictionProtection(false), podNamed("p0", nil), true)
+		assert.NoError(t, err)
+		ms.AssertNotCalled(t, "UpdatePodAnnotations", mock.Anything, mock.Anything, mock.Anything)
+	})
+
+	t.Run("master is pinned with safe-to-evict false", func(t *testing.T) {
+		ms := &mK8SService.Services{}
+		ms.On("UpdatePodAnnotations", "testns", "p0", map[string]string{
+			masterSafeToEvictAnnotation: "false",
+		}).Once().Return(nil)
+
+		err := applyMasterEvictionAnnotation(ms, rfWithEvictionProtection(true), podNamed("p0", nil), true)
+		assert.NoError(t, err)
+		ms.AssertExpectations(t)
+	})
+
+	t.Run("slave is marked evictable with safe-to-evict true", func(t *testing.T) {
+		ms := &mK8SService.Services{}
+		ms.On("UpdatePodAnnotations", "testns", "p1", map[string]string{
+			masterSafeToEvictAnnotation: "true",
+		}).Once().Return(nil)
+
+		err := applyMasterEvictionAnnotation(ms, rfWithEvictionProtection(true), podNamed("p1", nil), false)
+		assert.NoError(t, err)
+		ms.AssertExpectations(t)
+	})
+
+	t.Run("no call when annotation already at desired value", func(t *testing.T) {
+		ms := &mK8SService.Services{}
+		pod := podNamed("p0", map[string]string{masterSafeToEvictAnnotation: "false"})
+		err := applyMasterEvictionAnnotation(ms, rfWithEvictionProtection(true), pod, true)
+		assert.NoError(t, err)
+		ms.AssertNotCalled(t, "UpdatePodAnnotations", mock.Anything, mock.Anything, mock.Anything)
+	})
+}

service/k8s/pod.go

@@ -24,6 +24,7 @@ type Pod interface {
 	DeletePod(namespace string, name string) error
 	ListPods(namespace string) (*corev1.PodList, error)
 	UpdatePodLabels(namespace, podName string, labels map[string]string) error
+	UpdatePodAnnotations(namespace, podName string, annotations map[string]string) error
 }
 
 // PodService is the pod service implementation using API calls to kubernetes.
@@ -132,3 +133,24 @@ func (p *PodService) UpdatePodLabels(namespace, podName string, labels map[strin
 	}
 	return err
 }
+
+// UpdatePodAnnotations sets the given annotations on a pod. It uses a JSON merge
+// patch so the annotations map is created when absent and existing annotations
+// are left untouched, unlike the JSON-patch "replace" used for labels.
+func (p *PodService) UpdatePodAnnotations(namespace, podName string, annotations map[string]string) error {
+	p.logger.Infof("Update pod annotations, namespace: %s, pod name: %s, annotations: %v", namespace, podName, annotations)
+
+	patch := map[string]interface{}{
+		"metadata": map[string]interface{}{
+			"annotations": annotations,
+		},
+	}
+	payloadBytes, _ := json.Marshal(patch)
+
+	_, err := p.kubeClient.CoreV1().Pods(namespace).Patch(context.TODO(), podName, types.MergePatchType, payloadBytes, metav1.PatchOptions{})
+	recordMetrics(namespace, "Pod", podName, "PATCH", err, p.metricsRecorder)
+	if err != nil {
+		p.logger.Errorf("Update pod annotations failed, namespace: %s, pod name: %s, error: %v", namespace, podName, err)
+	}
+	return err
+}