diff --git a/README.md b/README.md index 9c52cc4e1..7209eebf0 100644 --- a/README.md +++ b/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 diff --git a/api/redisfailover/v1/types.go b/api/redisfailover/v1/types.go index dd36c99a8..20a74131f 100644 --- a/api/redisfailover/v1/types.go +++ b/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 diff --git a/charts/redisoperator/crds/databases.spotahome.com_redisfailovers.yaml b/charts/redisoperator/crds/databases.spotahome.com_redisfailovers.yaml index 0d07a34c4..87e30ab9a 100644 --- a/charts/redisoperator/crds/databases.spotahome.com_redisfailovers.yaml +++ b/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: diff --git a/manifests/databases.spotahome.com_redisfailovers.yaml b/manifests/databases.spotahome.com_redisfailovers.yaml index 0d07a34c4..87e30ab9a 100644 --- a/manifests/databases.spotahome.com_redisfailovers.yaml +++ b/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: diff --git a/manifests/kustomize/base/databases.spotahome.com_redisfailovers.yaml b/manifests/kustomize/base/databases.spotahome.com_redisfailovers.yaml index 0d07a34c4..87e30ab9a 100644 --- a/manifests/kustomize/base/databases.spotahome.com_redisfailovers.yaml +++ b/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: diff --git a/mocks/service/k8s/Services.go b/mocks/service/k8s/Services.go index 8c68a0d45..2297f229b 100644 --- a/mocks/service/k8s/Services.go +++ b/mocks/service/k8s/Services.go @@ -873,6 +873,20 @@ func (_m *Services) UpdatePodDisruptionBudget(namespace string, podDisruptionBud return r0 } +// UpdatePodAnnotations provides a mock function with given fields: namespace, podName, annotations +func (_m *Services) UpdatePodAnnotations(namespace string, podName string, annotations map[string]string) error { + ret := _m.Called(namespace, podName, annotations) + + var r0 error + if rf, ok := ret.Get(0).(func(string, string, map[string]string) error); ok { + r0 = rf(namespace, podName, annotations) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // UpdatePodLabels provides a mock function with given fields: namespace, podName, labels func (_m *Services) UpdatePodLabels(namespace string, podName string, labels map[string]string) error { ret := _m.Called(namespace, podName, labels) diff --git a/operator/redisfailover/service/check.go b/operator/redisfailover/service/check.go index c28aebc6f..8c40b7519 100644 --- a/operator/redisfailover/service/check.go +++ b/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 } diff --git a/operator/redisfailover/service/constants.go b/operator/redisfailover/service/constants.go index feb39ec6d..4f30a4a06 100644 --- a/operator/redisfailover/service/constants.go +++ b/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" diff --git a/operator/redisfailover/service/heal.go b/operator/redisfailover/service/heal.go index c5baa4757..b47f52543 100644 --- a/operator/redisfailover/service/heal.go +++ b/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 } diff --git a/operator/redisfailover/service/master_eviction_annotation_test.go b/operator/redisfailover/service/master_eviction_annotation_test.go new file mode 100644 index 000000000..0c9e22cd4 --- /dev/null +++ b/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) + }) +} diff --git a/service/k8s/pod.go b/service/k8s/pod.go index 50a4cee84..22fbaae0c 100644 --- a/service/k8s/pod.go +++ b/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 +}