diff --git a/charts/redisoperator/templates/service-account.yaml b/charts/redisoperator/templates/service-account.yaml index fc9805b88..14810b4a6 100644 --- a/charts/redisoperator/templates/service-account.yaml +++ b/charts/redisoperator/templates/service-account.yaml @@ -68,6 +68,26 @@ rules: - patch - update - watch + - apiGroups: + - "" + resources: + - pods/resize + verbs: + - patch + - update + - apiGroups: + - "" + resources: + - pods/eviction + verbs: + - create + - apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list - apiGroups: - "" resources: diff --git a/go.mod b/go.mod index 28208ca3e..7a8786b9a 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( golang.org/x/oauth2 v0.30.0 // indirect golang.org/x/sys v0.45.0 // indirect golang.org/x/term v0.43.0 // indirect - golang.org/x/text v0.37.0 // indirect + golang.org/x/text v0.39.0 // indirect golang.org/x/time v0.9.0 // indirect google.golang.org/protobuf v1.36.8 // indirect gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect diff --git a/go.sum b/go.sum index 757f08e7f..fc3a3fc5c 100644 --- a/go.sum +++ b/go.sum @@ -138,16 +138,16 @@ golang.org/x/term v0.43.0 h1:S4RLU2sB31O/NCl+zFN9Aru9A/Cq2aqKpTZJ6B+DwT4= golang.org/x/term v0.43.0/go.mod h1:lrhlHNdQJHO+1qVYiHfFKVuVioJIheAc3fBSMFYEIsk= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc= -golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38= +golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus= +golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM= golang.org/x/time v0.9.0 h1:EsRrnYcQiGH+5FfbgvV4AP7qEZstoyrHB0DzarOQ4ZY= golang.org/x/time v0.9.0/go.mod h1:3BpzKBy/shNhVucY/MWOyx10tF3SFh9QdLuxbVysPQM= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= -golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= +golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q= +golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/manifests/kustomize/components/rbac/clusterrole.yaml b/manifests/kustomize/components/rbac/clusterrole.yaml index 28b05f9a2..50397cad5 100644 --- a/manifests/kustomize/components/rbac/clusterrole.yaml +++ b/manifests/kustomize/components/rbac/clusterrole.yaml @@ -38,6 +38,20 @@ rules: - persistentvolumeclaims/finalizers verbs: - "*" + - apiGroups: + - "" + resources: + - pods/resize + - pods/eviction + verbs: + - "*" + - apiGroups: + - "" + resources: + - nodes + verbs: + - get + - list - apiGroups: - apps resources: diff --git a/mocks/operator/redisfailover/service/RedisFailoverCheck.go b/mocks/operator/redisfailover/service/RedisFailoverCheck.go index b1b60a181..8a326d472 100644 --- a/mocks/operator/redisfailover/service/RedisFailoverCheck.go +++ b/mocks/operator/redisfailover/service/RedisFailoverCheck.go @@ -7,6 +7,8 @@ import ( time "time" + resource "k8s.io/apimachinery/pkg/api/resource" + v1 "github.com/freshworks/redis-operator/api/redisfailover/v1" ) @@ -271,6 +273,189 @@ func (_m *RedisFailoverCheck) GetMasterIP(rFailover *v1.RedisFailover) (string, } // GetMaxRedisPodTime provides a mock function with given fields: rFailover +func (_m *RedisFailoverCheck) ComputeRequiredHeadroom(rFailover *v1.RedisFailover, nodeName string, masterPodName string) (resource.Quantity, resource.Quantity, error) { + ret := _m.Called(rFailover, nodeName, masterPodName) + + if len(ret) == 0 { + panic("no return value specified for ComputeRequiredHeadroom") + } + + var r0 resource.Quantity + var r1 resource.Quantity + var r2 error + if rf, ok := ret.Get(0).(func(*v1.RedisFailover, string, string) (resource.Quantity, resource.Quantity, error)); ok { + return rf(rFailover, nodeName, masterPodName) + } + if rf, ok := ret.Get(0).(func(*v1.RedisFailover, string, string) resource.Quantity); ok { + r0 = rf(rFailover, nodeName, masterPodName) + } else { + r0 = ret.Get(0).(resource.Quantity) + } + + if rf, ok := ret.Get(1).(func(*v1.RedisFailover, string, string) resource.Quantity); ok { + r1 = rf(rFailover, nodeName, masterPodName) + } else { + r1 = ret.Get(1).(resource.Quantity) + } + + if rf, ok := ret.Get(2).(func(*v1.RedisFailover, string, string) error); ok { + r2 = rf(rFailover, nodeName, masterPodName) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + +func (_m *RedisFailoverCheck) GetStatefulSetResizeOnly(rFailover *v1.RedisFailover) (bool, error) { + ret := _m.Called(rFailover) + + if len(ret) == 0 { + panic("no return value specified for GetStatefulSetResizeOnly") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(*v1.RedisFailover) (bool, error)); ok { + return rf(rFailover) + } + if rf, ok := ret.Get(0).(func(*v1.RedisFailover) bool); ok { + r0 = rf(rFailover) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(*v1.RedisFailover) error); ok { + r1 = rf(rFailover) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +func (_m *RedisFailoverCheck) GetPodNode(podName string, rFailover *v1.RedisFailover) (string, error) { + ret := _m.Called(podName, rFailover) + + if len(ret) == 0 { + panic("no return value specified for GetPodNode") + } + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) (string, error)); ok { + return rf(podName, rFailover) + } + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) string); ok { + r0 = rf(podName, rFailover) + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func(string, *v1.RedisFailover) error); ok { + r1 = rf(podName, rFailover) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +func (_m *RedisFailoverCheck) GetPodResizeCondition(podName string, rFailover *v1.RedisFailover) (bool, string, error) { + ret := _m.Called(podName, rFailover) + + if len(ret) == 0 { + panic("no return value specified for GetPodResizeCondition") + } + + var r0 bool + var r1 string + var r2 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) (bool, string, error)); ok { + return rf(podName, rFailover) + } + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) bool); ok { + r0 = rf(podName, rFailover) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(string, *v1.RedisFailover) string); ok { + r1 = rf(podName, rFailover) + } else { + r1 = ret.Get(1).(string) + } + + if rf, ok := ret.Get(2).(func(string, *v1.RedisFailover) error); ok { + r2 = rf(podName, rFailover) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + +func (_m *RedisFailoverCheck) GetResizeState(podName string, rFailover *v1.RedisFailover) (time.Time, bool, error) { + ret := _m.Called(podName, rFailover) + + if len(ret) == 0 { + panic("no return value specified for GetResizeState") + } + + var r0 time.Time + var r1 bool + var r2 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) (time.Time, bool, error)); ok { + return rf(podName, rFailover) + } + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) time.Time); ok { + r0 = rf(podName, rFailover) + } else { + r0 = ret.Get(0).(time.Time) + } + + if rf, ok := ret.Get(1).(func(string, *v1.RedisFailover) bool); ok { + r1 = rf(podName, rFailover) + } else { + r1 = ret.Get(1).(bool) + } + + if rf, ok := ret.Get(2).(func(string, *v1.RedisFailover) error); ok { + r2 = rf(podName, rFailover) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} + +func (_m *RedisFailoverCheck) PodResourcesMatchDesired(podName string, rFailover *v1.RedisFailover) (bool, error) { + ret := _m.Called(podName, rFailover) + + if len(ret) == 0 { + panic("no return value specified for PodResourcesMatchDesired") + } + + var r0 bool + var r1 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) (bool, error)); ok { + return rf(podName, rFailover) + } + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) bool); ok { + r0 = rf(podName, rFailover) + } else { + r0 = ret.Get(0).(bool) + } + + if rf, ok := ret.Get(1).(func(string, *v1.RedisFailover) error); ok { + r1 = rf(podName, rFailover) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + func (_m *RedisFailoverCheck) GetMaxRedisPodTime(rFailover *v1.RedisFailover) (time.Duration, error) { ret := _m.Called(rFailover) diff --git a/mocks/operator/redisfailover/service/RedisFailoverHeal.go b/mocks/operator/redisfailover/service/RedisFailoverHeal.go index 3c9d48d4f..5112e42ca 100644 --- a/mocks/operator/redisfailover/service/RedisFailoverHeal.go +++ b/mocks/operator/redisfailover/service/RedisFailoverHeal.go @@ -5,6 +5,8 @@ package mocks import ( mock "github.com/stretchr/testify/mock" + resource "k8s.io/apimachinery/pkg/api/resource" + v1 "github.com/freshworks/redis-operator/api/redisfailover/v1" ) @@ -31,6 +33,91 @@ func (_m *RedisFailoverHeal) DeletePod(podName string, rFailover *v1.RedisFailov return r0 } +func (_m *RedisFailoverHeal) FreeResizeHeadroom(rFailover *v1.RedisFailover, nodeName string, requiredCPU resource.Quantity, requiredMemory resource.Quantity) error { + ret := _m.Called(rFailover, nodeName, requiredCPU, requiredMemory) + + if len(ret) == 0 { + panic("no return value specified for FreeResizeHeadroom") + } + + var r0 error + if rf, ok := ret.Get(0).(func(*v1.RedisFailover, string, resource.Quantity, resource.Quantity) error); ok { + r0 = rf(rFailover, nodeName, requiredCPU, requiredMemory) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +func (_m *RedisFailoverHeal) SetResizeStartedAt(podName string, rFailover *v1.RedisFailover) error { + ret := _m.Called(podName, rFailover) + + if len(ret) == 0 { + panic("no return value specified for SetResizeStartedAt") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) error); ok { + r0 = rf(podName, rFailover) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +func (_m *RedisFailoverHeal) ClearResizeState(podName string, rFailover *v1.RedisFailover) error { + ret := _m.Called(podName, rFailover) + + if len(ret) == 0 { + panic("no return value specified for ClearResizeState") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) error); ok { + r0 = rf(podName, rFailover) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +func (_m *RedisFailoverHeal) RelabelPodRevision(podName string, rFailover *v1.RedisFailover, revision string) error { + ret := _m.Called(podName, rFailover, revision) + + if len(ret) == 0 { + panic("no return value specified for RelabelPodRevision") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover, string) error); ok { + r0 = rf(podName, rFailover, revision) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +func (_m *RedisFailoverHeal) ResizePod(podName string, rFailover *v1.RedisFailover) error { + ret := _m.Called(podName, rFailover) + + if len(ret) == 0 { + panic("no return value specified for ResizePod") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, *v1.RedisFailover) error); ok { + r0 = rf(podName, rFailover) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // MakeMaster provides a mock function with given fields: ip, rFailover func (_m *RedisFailoverHeal) MakeMaster(ip string, rFailover *v1.RedisFailover) error { ret := _m.Called(ip, rFailover) diff --git a/mocks/service/k8s/Services.go b/mocks/service/k8s/Services.go index 9654e1261..1b4241255 100644 --- a/mocks/service/k8s/Services.go +++ b/mocks/service/k8s/Services.go @@ -388,6 +388,98 @@ func (_m *Services) DeletePod(namespace string, name string) error { } // DeletePodDisruptionBudget provides a mock function with given fields: namespace, name +func (_m *Services) GetNode(name string) (*v1.Node, error) { + ret := _m.Called(name) + + if len(ret) == 0 { + panic("no return value specified for GetNode") + } + + var r0 *v1.Node + var r1 error + if rf, ok := ret.Get(0).(func(string) (*v1.Node, error)); ok { + return rf(name) + } + if rf, ok := ret.Get(0).(func(string) *v1.Node); ok { + r0 = rf(name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1.Node) + } + } + + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(name) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +func (_m *Services) ListPodsOnNode(nodeName string, labelSelector string) (*v1.PodList, error) { + ret := _m.Called(nodeName, labelSelector) + + if len(ret) == 0 { + panic("no return value specified for ListPodsOnNode") + } + + var r0 *v1.PodList + var r1 error + if rf, ok := ret.Get(0).(func(string, string) (*v1.PodList, error)); ok { + return rf(nodeName, labelSelector) + } + if rf, ok := ret.Get(0).(func(string, string) *v1.PodList); ok { + r0 = rf(nodeName, labelSelector) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*v1.PodList) + } + } + + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(nodeName, labelSelector) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +func (_m *Services) EvictPod(namespace string, podName string) error { + ret := _m.Called(namespace, podName) + + if len(ret) == 0 { + panic("no return value specified for EvictPod") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, string) error); ok { + r0 = rf(namespace, podName) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +func (_m *Services) ResizePod(namespace string, podName string, containerName string, resources v1.ResourceRequirements) error { + ret := _m.Called(namespace, podName, containerName, resources) + + if len(ret) == 0 { + panic("no return value specified for ResizePod") + } + + var r0 error + if rf, ok := ret.Get(0).(func(string, string, string, v1.ResourceRequirements) error); ok { + r0 = rf(namespace, podName, containerName, resources) + } else { + r0 = ret.Error(0) + } + + return r0 +} + func (_m *Services) DeletePodDisruptionBudget(namespace string, name string) error { ret := _m.Called(namespace, name) diff --git a/operator/redisfailover/checker.go b/operator/redisfailover/checker.go index d63100872..4d4e9f6ec 100644 --- a/operator/redisfailover/checker.go +++ b/operator/redisfailover/checker.go @@ -5,11 +5,27 @@ import ( "strconv" "time" + corev1 "k8s.io/api/core/v1" + redisfailoverv1 "github.com/freshworks/redis-operator/api/redisfailover/v1" "github.com/freshworks/redis-operator/metrics" ) -// UpdateRedisesPods deletes Redis pods with a stale StatefulSet revision (OnDelete rollout). +// masterResizeTimeout and slaveResizeTimeout bound how long an in-place resize attempt +// (including any headroom eviction) is allowed to stay Deferred before falling back to the +// delete-based rollout. The master gets a longer window because a master restart is costly +// (Sentinel failover); a slave restart is already considered cheap and acceptable today, so +// there's less reason to hold out long before falling back to what would have happened +// anyway. There is no requeue/backoff in this operator (see factory.go's resync), so these +// are evaluated fresh against the resize-started-at annotation on every ~30s reconcile. +const ( + masterResizeTimeout = 6 * time.Minute + slaveResizeTimeout = 2 * time.Minute +) + +// UpdateRedisesPods deletes Redis pods with a stale StatefulSet revision (OnDelete rollout), +// or resizes them in place instead when the pending change is resource-only (see +// k8s.ResizeOnlyAnnotationKey). // DisableMasterRollout when true, only slave pods are rolled out on spec change (OnDelete); master is not deleted until the flag is removed. func (r *RedisFailoverHandler) UpdateRedisesPods(rf *redisfailoverv1.RedisFailover) error { redises, err := r.rfChecker.GetRedisesIPs(rf) @@ -39,6 +55,11 @@ func (r *RedisFailoverHandler) UpdateRedisesPods(rf *redisfailoverv1.RedisFailov return err } + resizeOnly, err := r.rfChecker.GetStatefulSetResizeOnly(rf) + if err != nil { + return err + } + redisesPods, err := r.rfChecker.GetRedisesSlavesPods(rf) if err != nil { return err @@ -51,12 +72,15 @@ func (r *RedisFailoverHandler) UpdateRedisesPods(rf *redisfailoverv1.RedisFailov return err } if revision != ssUR { - //Delete pod and wait next round to check if the new one is synced - err = r.rfHealer.DeletePod(pod, rf) - if err != nil { - return err + if !resizeOnly { + //Delete pod and wait next round to check if the new one is synced + err = r.rfHealer.DeletePod(pod, rf) + if err != nil { + return err + } + return nil } - return nil + return r.attemptPodResize(rf, pod, ssUR, slaveResizeTimeout, false) } } @@ -71,12 +95,129 @@ func (r *RedisFailoverHandler) UpdateRedisesPods(rf *redisfailoverv1.RedisFailov return err } if masterRevision != ssUR { - err = r.rfHealer.DeletePod(master, rf) - if err != nil { + if !resizeOnly { + err = r.rfHealer.DeletePod(master, rf) + if err != nil { + return err + } + return nil + } + return r.attemptPodResize(rf, master, ssUR, masterResizeTimeout, true) + } + } + + return nil +} + +// attemptPodResize is reached only once UpdateRedisesPods has confirmed the pending +// StatefulSet revision change is resource-only (see k8s.ResizeOnlyAnnotationKey). It resizes +// podName (master or slave) in place instead of deleting it, and falls back to the existing +// delete-based rollout if the resize is Infeasible, Deferred with allowEviction false, or +// exceeds timeout. +// For the master, the replication-lag precondition is already enforced by the ready-check at +// the top of UpdateRedisesPods; slaves have no equivalent precondition, since growing a slave +// carries none of the risk that growing the master past what a slave can hold does. +// allowEviction gates whether a Deferred resize may evict other RedisFailovers' co-located +// slave pods to free headroom - true for the master, since a master restart is costly enough +// to justify disrupting another tenant's slave; false for slaves, since a slave restart is +// already cheap and acceptable, and there's no justification for evicting someone else's pod +// just to avoid one - a Deferred slave resize falls straight to delete instead. +func (r *RedisFailoverHandler) attemptPodResize(rf *redisfailoverv1.RedisFailover, podName, ssUR string, timeout time.Duration, allowEviction bool) error { + startedAt, inProgress, err := r.rfChecker.GetResizeState(podName, rf) + if err != nil { + return err + } + + if !inProgress { + if err := r.rfHealer.SetResizeStartedAt(podName, rf); err != nil { + return err + } + if err := r.rfHealer.ResizePod(podName, rf); err != nil { + return err + } + // Give the kubelet a reconcile cycle to act on the request before checking outcome. + return nil + } + + if time.Since(startedAt) > timeout { + r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("pod", podName).Warningf("resize timed out after %s, falling back to delete", timeout) + if err := r.rfHealer.ClearResizeState(podName, rf); err != nil { + return err + } + return r.rfHealer.DeletePod(podName, rf) + } + + found, reason, err := r.rfChecker.GetPodResizeCondition(podName, rf) + if err != nil { + return err + } + + if !found { + // No PodResizePending condition is necessary but not sufficient proof the resize + // applied - it's equally true if the resize call never reached the pod at all (e.g. + // an RBAC rejection on the resize subresource never gets this far in the first + // place). Confirm the pod's actual resources match before declaring success. + matches, err := r.rfChecker.PodResourcesMatchDesired(podName, rf) + if err != nil { + return err + } + if !matches { + r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("pod", podName).Warningf("no resize-pending condition but resources don't match desired spec yet, retrying resize") + return r.rfHealer.ResizePod(podName, rf) + } + // Resize applied. Close the controller-revision-hash bookkeeping gap: a resize never + // updates that label on its own, so without this the next reconcile would see + // this pod's revision != ssUR again and re-enter this whole branch. + if err := r.rfHealer.RelabelPodRevision(podName, rf, ssUR); err != nil { + return err + } + return r.rfHealer.ClearResizeState(podName, rf) + } + + switch reason { + case corev1.PodReasonInfeasible: + r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("pod", podName).Warningf("resize infeasible on current node, falling back to delete") + if err := r.rfHealer.ClearResizeState(podName, rf); err != nil { + return err + } + return r.rfHealer.DeletePod(podName, rf) + + case corev1.PodReasonDeferred: + if !allowEviction { + r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("pod", podName).Warningf("resize deferred and eviction not permitted for this pod, falling back to delete") + if err := r.rfHealer.ClearResizeState(podName, rf); err != nil { return err } + return r.rfHealer.DeletePod(podName, rf) + } + + // Re-issue the resize on every retry, not just the first attempt. ResizePod only ran + // once, when this attempt began; if the CR's resources changed since (e.g. a human + // lowers the ask after seeing it's stuck), the pod's live resize target would + // otherwise stay pinned to whatever was originally requested, and the only way it'd + // ever pick up the new value is the eventual timeout-driven DeletePod fallback. This + // keeps the pod's target in sync with the CR on every reconcile; it's a no-op patch + // if nothing changed. + if err := r.rfHealer.ResizePod(podName, rf); err != nil { + return err + } + + nodeName, err := r.rfChecker.GetPodNode(podName, rf) + if err != nil { + return err + } + requiredCPU, requiredMemory, err := r.rfChecker.ComputeRequiredHeadroom(rf, nodeName, podName) + if err != nil { + return err + } + if requiredCPU.Sign() <= 0 && requiredMemory.Sign() <= 0 { + // Already fits from the node's perspective; let the kubelet's own retry catch up. return nil } + if err := r.rfHealer.FreeResizeHeadroom(rf, nodeName, requiredCPU, requiredMemory); err != nil { + r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("pod", podName).Warningf("could not free enough headroom yet, will keep retrying until timeout: %v", err) + } + return nil } return nil diff --git a/operator/redisfailover/checker_test.go b/operator/redisfailover/checker_test.go index a6f6b35bb..b5c27ee6a 100644 --- a/operator/redisfailover/checker_test.go +++ b/operator/redisfailover/checker_test.go @@ -572,6 +572,7 @@ func TestCheckAndHeal(t *testing.T) { mrfc.On("CheckRedisSlavesReady", "0.0.0.2", rf).Once().Return(true, nil) mrfc.On("CheckRedisSlavesReady", "0.0.0.3", rf).Once().Return(true, nil) mrfc.On("GetStatefulSetUpdateRevision", rf).Once().Return("1", nil) + mrfc.On("GetStatefulSetResizeOnly", rf).Once().Return(false, nil) mrfc.On("GetRedisesSlavesPods", rf).Once().Return([]string{}, nil) if test.redisSetMasterOnAllOK { @@ -626,6 +627,7 @@ func TestCheckAndHeal(t *testing.T) { } mrfc.On("GetRedisesIPs", rf).Twice().Return([]string{master}, nil) mrfc.On("GetStatefulSetUpdateRevision", rf).Once().Return("1", nil) + mrfc.On("GetStatefulSetResizeOnly", rf).Once().Return(false, nil) mrfc.On("GetRedisesSlavesPods", rf).Once().Return([]string{}, nil) mrfc.On("GetRedisesMasterPod", rf).Once().Return(master, nil) mrfc.On("GetRedisRevisionHash", master, rf).Once().Return("1", nil) @@ -1157,6 +1159,10 @@ func TestUpdate(t *testing.T) { replicas = append(replicas, "slave3") } mrfc.On("GetStatefulSetUpdateRevision", rf).Once().Return(test.ssVersion, nil) + // Non-resize-only change in this scenario: falls through to the existing + // delete-based rollout for any mismatched pod, same as before the in-place + // resize feature was added. Called exactly once per UpdateRedisesPods call. + mrfc.On("GetStatefulSetResizeOnly", rf).Once().Return(false, nil) mrfc.On("GetRedisesSlavesPods", rf).Once().Return(replicas, nil) for _, pod := range test.pods { diff --git a/operator/redisfailover/service/check.go b/operator/redisfailover/service/check.go index 3d91892c3..dbbef865d 100644 --- a/operator/redisfailover/service/check.go +++ b/operator/redisfailover/service/check.go @@ -8,6 +8,8 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/resource" redisfailoverv1 "github.com/freshworks/redis-operator/api/redisfailover/v1" "github.com/freshworks/redis-operator/log" @@ -40,6 +42,28 @@ type RedisFailoverCheck interface { IsRedisRunning(rFailover *redisfailoverv1.RedisFailover) bool IsSentinelRunning(rFailover *redisfailoverv1.RedisFailover) bool IsClusterRunning(rFailover *redisfailoverv1.RedisFailover) bool + // ComputeRequiredHeadroom returns how much additional CPU and memory must be freed on + // nodeName for rFailover's podName (master or slave) to resize to its desired spec - + // either may be zero/negative if that resource already fits. + ComputeRequiredHeadroom(rFailover *redisfailoverv1.RedisFailover, nodeName, podName string) (requiredCPU, requiredMemory resource.Quantity, err error) + // GetStatefulSetResizeOnly returns whether the StatefulSet's most recent update was a + // resource-only change, i.e. whether pods can be resized in place instead of deleted for + // the currently pending revision change. + GetStatefulSetResizeOnly(rFailover *redisfailoverv1.RedisFailover) (bool, error) + // GetPodNode returns the node name podName is scheduled on. + GetPodNode(podName string, rFailover *redisfailoverv1.RedisFailover) (string, error) + // GetPodResizeCondition reports whether podName currently has a PodResizePending + // condition and, if so, its reason (corev1.PodReasonDeferred or corev1.PodReasonInfeasible). + GetPodResizeCondition(podName string, rFailover *redisfailoverv1.RedisFailover) (found bool, reason string, err error) + // GetResizeState reports when an in-place resize attempt on podName began, if one is + // currently tracked (see resizeStartedAtAnnotationKey). + GetResizeState(podName string, rFailover *redisfailoverv1.RedisFailover) (startedAt time.Time, exists bool, err error) + // PodResourcesMatchDesired reports whether podName's redis container currently has the + // same resources as rFailover.Spec.Redis.Resources. The absence of a PodResizePending + // condition alone doesn't prove a resize actually applied - it's also true when the + // resize call never reached the pod at all (e.g. an RBAC rejection) - so callers must + // check this before treating a resize as successful. + PodResourcesMatchDesired(podName string, rFailover *redisfailoverv1.RedisFailover) (bool, error) } // RedisFailoverChecker is our implementation of RedisFailoverCheck interface @@ -492,6 +516,88 @@ func (r *RedisFailoverChecker) GetStatefulSetUpdateRevision(rFailover *redisfail return ss.Status.UpdateRevision, nil } +// GetStatefulSetResizeOnly returns whether the StatefulSet's most recent update was a +// resource-only change (see k8s.ResizeOnlyAnnotationKey). +func (r *RedisFailoverChecker) GetStatefulSetResizeOnly(rFailover *redisfailoverv1.RedisFailover) (bool, error) { + ss, err := r.k8sService.GetStatefulSet(rFailover.Namespace, GetRedisName(rFailover)) + if err != nil { + return false, err + } + if ss == nil { + return false, errors.New("statefulSet not found") + } + return ss.Annotations[k8s.ResizeOnlyAnnotationKey] == "true", nil +} + +// GetPodNode returns the node name podName is scheduled on. +func (r *RedisFailoverChecker) GetPodNode(podName string, rFailover *redisfailoverv1.RedisFailover) (string, error) { + pod, err := r.k8sService.GetPod(rFailover.Namespace, podName) + if err != nil { + return "", err + } + if pod == nil { + return "", errors.New("pod not found") + } + return pod.Spec.NodeName, nil +} + +// GetPodResizeCondition reports whether podName currently has a PodResizePending condition +// and, if so, its reason. +func (r *RedisFailoverChecker) GetPodResizeCondition(podName string, rFailover *redisfailoverv1.RedisFailover) (bool, string, error) { + pod, err := r.k8sService.GetPod(rFailover.Namespace, podName) + if err != nil { + return false, "", err + } + if pod == nil { + return false, "", errors.New("pod not found") + } + for _, cond := range pod.Status.Conditions { + if cond.Type == corev1.PodResizePending { + return true, cond.Reason, nil + } + } + return false, "", nil +} + +// GetResizeState reports when an in-place resize attempt on podName began, if one is +// currently tracked. +func (r *RedisFailoverChecker) GetResizeState(podName string, rFailover *redisfailoverv1.RedisFailover) (time.Time, bool, error) { + pod, err := r.k8sService.GetPod(rFailover.Namespace, podName) + if err != nil { + return time.Time{}, false, err + } + if pod == nil { + return time.Time{}, false, errors.New("pod not found") + } + raw, ok := pod.Annotations[resizeStartedAtAnnotationKey] + if !ok { + return time.Time{}, false, nil + } + startedAt, err := time.Parse(time.RFC3339, raw) + if err != nil { + return time.Time{}, false, err + } + return startedAt, true, nil +} + +// PodResourcesMatchDesired reports whether podName's redis container currently has the same +// resources as rFailover.Spec.Redis.Resources. +func (r *RedisFailoverChecker) PodResourcesMatchDesired(podName string, rFailover *redisfailoverv1.RedisFailover) (bool, error) { + pod, err := r.k8sService.GetPod(rFailover.Namespace, podName) + if err != nil { + return false, err + } + if pod == nil { + return false, errors.New("pod not found") + } + for _, container := range pod.Spec.Containers { + if container.Name == "redis" { + return apiequality.Semantic.DeepEqual(container.Resources, rFailover.Spec.Redis.Resources), nil + } + } + return false, errors.New("redis container not found in pod") +} + // GetRedisRevisionHash returns the statefulset uid for the pod func (r *RedisFailoverChecker) GetRedisRevisionHash(podName string, rFailover *redisfailoverv1.RedisFailover) (string, error) { pod, err := r.k8sService.GetPod(rFailover.Namespace, podName) diff --git a/operator/redisfailover/service/constants.go b/operator/redisfailover/service/constants.go index 557e4bba7..4ea737157 100644 --- a/operator/redisfailover/service/constants.go +++ b/operator/redisfailover/service/constants.go @@ -37,4 +37,15 @@ const ( clusterAutoscalerSafeToEvictAnnotationKey = "cluster-autoscaler.kubernetes.io/safe-to-evict" clusterAutoscalerSafeToEvictAnnotationMaster = "false" clusterAutoscalerSafeToEvictAnnotationSlave = "true" + + // redisFailoverNameLabelKey identifies which RedisFailover a pod belongs to (set by the + // handler on every pod it creates). Used to exclude an endpoint's own slaves from the + // eviction candidate pool when freeing headroom for that same endpoint's master resize. + redisFailoverNameLabelKey = "redisfailovers.databases.spotahome.com/name" + + // resizeStartedAtAnnotationKey records when an in-place master resize attempt began, so + // it can be timed out and fall back to the delete-based rollout. There is no requeue/ + // backoff or Status subresource in this operator, so this state must be tracked this way + // across the periodic reconcile resync rather than in memory. + resizeStartedAtAnnotationKey = "redis-failover.freshworks.com/resize-started-at" ) diff --git a/operator/redisfailover/service/generator.go b/operator/redisfailover/service/generator.go index 66fe480a6..ffb93f4fb 100644 --- a/operator/redisfailover/service/generator.go +++ b/operator/redisfailover/service/generator.go @@ -413,6 +413,10 @@ func generateRedisStatefulSet(rf *redisfailoverv1.RedisFailover, labels map[stri VolumeMounts: volumeMounts, Command: redisCommand, Resources: rf.Spec.Redis.Resources, + ResizePolicy: []corev1.ContainerResizePolicy{ + {ResourceName: corev1.ResourceMemory, RestartPolicy: corev1.NotRequired}, + {ResourceName: corev1.ResourceCPU, RestartPolicy: corev1.NotRequired}, + }, Lifecycle: &corev1.Lifecycle{ PreStop: &corev1.LifecycleHandler{ Exec: &corev1.ExecAction{ diff --git a/operator/redisfailover/service/heal.go b/operator/redisfailover/service/heal.go index d964f1f55..3c021d20a 100644 --- a/operator/redisfailover/service/heal.go +++ b/operator/redisfailover/service/heal.go @@ -6,12 +6,15 @@ import ( "sort" "strconv" "strings" + "time" redisfailoverv1 "github.com/freshworks/redis-operator/api/redisfailover/v1" "github.com/freshworks/redis-operator/log" "github.com/freshworks/redis-operator/service/k8s" "github.com/freshworks/redis-operator/service/redis" + appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" ) // RedisFailoverHeal defines the interface able to fix the problems on the redis failovers @@ -26,6 +29,22 @@ type RedisFailoverHeal interface { SetSentinelCustomConfig(ip string, rFailover *redisfailoverv1.RedisFailover) error SetRedisCustomConfig(ip string, rFailover *redisfailoverv1.RedisFailover) error DeletePod(podName string, rFailover *redisfailoverv1.RedisFailover) error + // ResizePod applies rFailover's current redis resource requests/limits to podName + // in place, via the resize subresource, instead of deleting and recreating it. + ResizePod(podName string, rFailover *redisfailoverv1.RedisFailover) error + // FreeResizeHeadroom evicts co-located slave pods of OTHER RedisFailovers on nodeName + // to cover requiredCPU and requiredMemory together, ahead of resizing rFailover's master or slave + // pod. Either requirement may be zero/negative if that resource already fits. + FreeResizeHeadroom(rFailover *redisfailoverv1.RedisFailover, nodeName string, requiredCPU, requiredMemory resource.Quantity) error + // SetResizeStartedAt records that an in-place resize attempt on podName has begun now. + SetResizeStartedAt(podName string, rFailover *redisfailoverv1.RedisFailover) error + // ClearResizeState removes the in-place resize tracking annotation from podName, once a + // resize has succeeded or been abandoned in favor of the delete-based fallback. + ClearResizeState(podName string, rFailover *redisfailoverv1.RedisFailover) error + // RelabelPodRevision patches podName's controller-revision-hash label to revision. Only + // safe to call once the caller has confirmed the only difference between revisions was + // resources and the resize actually succeeded - see GetStatefulSetResizeOnly. + RelabelPodRevision(podName string, rFailover *redisfailoverv1.RedisFailover, revision string) error } // RedisFailoverHealer is our implementation of RedisFailoverCheck interface @@ -471,3 +490,30 @@ func (r *RedisFailoverHealer) DeletePod(podName string, rFailover *redisfailover r.logger.WithField("redisfailover", rFailover.ObjectMeta.Name).WithField("namespace", rFailover.ObjectMeta.Namespace).Infof("Deleting pods %s...", podName) return r.k8sService.DeletePod(rFailover.Namespace, podName) } + +// ResizePod applies rFailover's current redis resource requests/limits to podName in place. +func (r *RedisFailoverHealer) ResizePod(podName string, rFailover *redisfailoverv1.RedisFailover) error { + r.logger.WithField("redisfailover", rFailover.ObjectMeta.Name).WithField("namespace", rFailover.ObjectMeta.Namespace).Infof("Resizing pod %s...", podName) + return r.k8sService.ResizePod(rFailover.Namespace, podName, "redis", rFailover.Spec.Redis.Resources) +} + +// SetResizeStartedAt records that an in-place resize attempt on podName has begun now. +func (r *RedisFailoverHealer) SetResizeStartedAt(podName string, rFailover *redisfailoverv1.RedisFailover) error { + return r.k8sService.UpdatePodAnnotations(rFailover.Namespace, podName, map[string]string{ + resizeStartedAtAnnotationKey: time.Now().Format(time.RFC3339), + }) +} + +// ClearResizeState removes the in-place resize tracking annotation from podName. +func (r *RedisFailoverHealer) ClearResizeState(podName string, rFailover *redisfailoverv1.RedisFailover) error { + return r.k8sService.RemovePodAnnotation(rFailover.Namespace, podName, resizeStartedAtAnnotationKey) +} + +// RelabelPodRevision patches podName's controller-revision-hash label to revision, closing +// the gap between the StatefulSet's status.updateRevision and the pod's own label that a +// resize (unlike a delete-and-recreate) never updates on its own. +func (r *RedisFailoverHealer) RelabelPodRevision(podName string, rFailover *redisfailoverv1.RedisFailover, revision string) error { + return r.k8sService.UpdatePodLabels(rFailover.Namespace, podName, map[string]string{ + appsv1.ControllerRevisionHashLabelKey: revision, + }) +} diff --git a/operator/redisfailover/service/resize_headroom.go b/operator/redisfailover/service/resize_headroom.go new file mode 100644 index 000000000..1a852aaef --- /dev/null +++ b/operator/redisfailover/service/resize_headroom.go @@ -0,0 +1,272 @@ +package service + +import ( + "fmt" + "sort" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + + redisfailoverv1 "github.com/freshworks/redis-operator/api/redisfailover/v1" +) + +// evictionCandidate is a co-located slave pod (belonging to some other RedisFailover) that +// could be evicted to free CPU and/or memory on a node ahead of resizing a master or slave +// pod in place. +type evictionCandidate struct { + Namespace string + Name string + CPU resource.Quantity + Memory resource.Quantity +} + +// ComputeRequiredHeadroom returns how much additional CPU and memory must be freed on +// nodeName for rFailover's podName (master or slave) to resize to its currently desired spec +// - either may be zero/negative if that resource already fits. podName's own current +// allocation is deliberately excluded from the "everything else on the node" totals, and +// rFailover.Spec.Redis.Resources is used directly as the desired usage (the same value +// applies to every replica, since they share one StatefulSet template) - this avoids having +// to reconcile the pod's spec (which already shows the new values once a resize is +// requested) against its status (which shows what's actually allocated until the resize +// completes). +func (r *RedisFailoverChecker) ComputeRequiredHeadroom(rFailover *redisfailoverv1.RedisFailover, nodeName, podName string) (resource.Quantity, resource.Quantity, error) { + node, err := r.k8sService.GetNode(nodeName) + if err != nil { + return resource.Quantity{}, resource.Quantity{}, err + } + allocatableCPU := node.Status.Allocatable.Cpu() + allocatableMemory := node.Status.Allocatable.Memory() + + podList, err := r.k8sService.ListPodsOnNode(nodeName, "") + if err != nil { + return resource.Quantity{}, resource.Quantity{}, err + } + + otherCPU := resource.Quantity{} + otherMemory := resource.Quantity{} + for _, pod := range podList.Items { + if pod.Name == podName && pod.Namespace == rFailover.Namespace { + continue + } + for _, container := range pod.Spec.Containers { + otherCPU.Add(*container.Resources.Requests.Cpu()) + otherMemory.Add(*container.Resources.Requests.Memory()) + } + } + + requiredCPU := otherCPU.DeepCopy() + requiredCPU.Add(*rFailover.Spec.Redis.Resources.Requests.Cpu()) + requiredCPU.Sub(*allocatableCPU) + + requiredMemory := otherMemory.DeepCopy() + requiredMemory.Add(*rFailover.Spec.Redis.Resources.Requests.Memory()) + requiredMemory.Sub(*allocatableMemory) + + return requiredCPU, requiredMemory, nil +} + +// FreeResizeHeadroom evicts co-located slave pods belonging to OTHER RedisFailovers on +// nodeName - never rFailover's own slaves - selecting the minimal-pod-count, minimal-waste +// subset that covers requiredCPU and requiredMemory together (either constraint is skipped +// if its quantity is zero/negative), to make room for rFailover's master or slave pod to resize in +// place. Returns an error if the requirement(s) cannot be covered even by evicting every +// candidate; callers should treat that the same as an Infeasible resize. +func (r *RedisFailoverHealer) FreeResizeHeadroom(rFailover *redisfailoverv1.RedisFailover, nodeName string, requiredCPU, requiredMemory resource.Quantity) error { + pods, err := r.k8sService.ListPodsOnNode(nodeName, redisRoleLabelKey+"="+redisRoleLabelSlave) + if err != nil { + return err + } + + pool := buildEvictionPool(pods, rFailover.Name) + remainingCPU := requiredCPU.DeepCopy() + remainingMemory := requiredMemory.DeepCopy() + + for remainingCPU.Sign() > 0 || remainingMemory.Sign() > 0 { + chosen := selectEvictionSet(pool, remainingCPU, remainingMemory) + if chosen == nil { + return fmt.Errorf("cannot free %s cpu / %s memory of headroom on node %s: not enough evictable slave pods", requiredCPU.String(), requiredMemory.String(), nodeName) + } + + for _, c := range chosen { + pool = removeCandidate(pool, c) + if err := r.k8sService.EvictPod(c.Namespace, c.Name); err != nil { + r.logger.WithField("namespace", c.Namespace).WithField("pod", c.Name).Warningf("eviction rejected, recomputing remaining candidates: %v", err) + break // pool has shrunk; recompute against the same remaining deficit + } + if remainingCPU.Sign() > 0 { + remainingCPU.Sub(c.CPU) + } + if remainingMemory.Sign() > 0 { + remainingMemory.Sub(c.Memory) + } + } + } + return nil +} + +// buildEvictionPool lists eviction candidates from pods, excluding any pod that belongs to +// excludeRedisFailoverName - an endpoint's own slaves are never candidates for freeing room +// for that same endpoint's master. +func buildEvictionPool(pods *corev1.PodList, excludeRedisFailoverName string) []evictionCandidate { + var pool []evictionCandidate + for _, pod := range pods.Items { + if pod.Labels[redisFailoverNameLabelKey] == excludeRedisFailoverName { + continue + } + cpu := resource.Quantity{} + mem := resource.Quantity{} + for _, c := range pod.Spec.Containers { + cpu.Add(*c.Resources.Requests.Cpu()) + mem.Add(*c.Resources.Requests.Memory()) + } + pool = append(pool, evictionCandidate{Namespace: pod.Namespace, Name: pod.Name, CPU: cpu, Memory: mem}) + } + return pool +} + +func removeCandidate(pool []evictionCandidate, target evictionCandidate) []evictionCandidate { + out := make([]evictionCandidate, 0, len(pool)) + for _, c := range pool { + if c.Namespace == target.Namespace && c.Name == target.Name { + continue + } + out = append(out, c) + } + return out +} + +// qualifies reports whether sumCPU/sumMemory cover requiredCPU/requiredMemory. A requirement +// with a zero/negative quantity is treated as already satisfied - this is what makes +// "only CPU increased", "only memory increased", and "both increased" all fall out of the +// same code path instead of needing separate handling. +func qualifies(sumCPU, sumMemory, requiredCPU, requiredMemory resource.Quantity) bool { + if requiredCPU.Sign() > 0 && sumCPU.Cmp(requiredCPU) < 0 { + return false + } + if requiredMemory.Sign() > 0 && sumMemory.Cmp(requiredMemory) < 0 { + return false + } + return true +} + +// wasteScore combines the two resources' over-eviction into one dimensionless ranking value, +// expressed as a fraction of each requirement rather than raw units - CPU (cores) and memory +// (bytes) can't be summed directly, but "20% more CPU than needed" and "20% more memory than +// needed" can. Only resources with a positive requirement contribute. +func wasteScore(sumCPU, sumMemory, requiredCPU, requiredMemory resource.Quantity) float64 { + var score float64 + if requiredCPU.Sign() > 0 { + score += (sumCPU.AsApproximateFloat64() - requiredCPU.AsApproximateFloat64()) / requiredCPU.AsApproximateFloat64() + } + if requiredMemory.Sign() > 0 { + score += (sumMemory.AsApproximateFloat64() - requiredMemory.AsApproximateFloat64()) / requiredMemory.AsApproximateFloat64() + } + return score +} + +// maxExhaustiveCandidates caps the brute-force subset search. Per-node candidate counts are +// expected to be small (tens, not thousands) where 2^n is trivial; beyond this bound we fall +// back to a greedy selection to avoid combinatorial blowup, trading provable optimality for +// bounded runtime. +const maxExhaustiveCandidates = 24 + +// selectEvictionSet returns the minimal-pod-count, minimal-waste subset of candidates whose +// summed CPU and Memory cover requiredCPU and requiredMemory (each requirement independently +// optional - see qualifies), or nil if no subset (including taking every candidate) does. +func selectEvictionSet(candidates []evictionCandidate, requiredCPU, requiredMemory resource.Quantity) []evictionCandidate { + if len(candidates) > maxExhaustiveCandidates { + return selectEvictionSetGreedy(candidates, requiredCPU, requiredMemory) + } + + n := len(candidates) + var best []evictionCandidate + bestWaste := 0.0 + for size := 1; size <= n; size++ { + found := false + combinations(n, size, func(idx []int) { + sumCPU := resource.Quantity{} + sumMemory := resource.Quantity{} + for _, i := range idx { + sumCPU.Add(candidates[i].CPU) + sumMemory.Add(candidates[i].Memory) + } + if !qualifies(sumCPU, sumMemory, requiredCPU, requiredMemory) { + return + } + waste := wasteScore(sumCPU, sumMemory, requiredCPU, requiredMemory) + if best == nil || waste < bestWaste { + best = pick(candidates, idx) + bestWaste = waste + found = true + } + }) + if found { + return best + } + } + return nil +} + +// selectEvictionSetGreedy is the fallback used above maxExhaustiveCandidates: candidates are +// ranked by their combined contribution toward whichever requirements are still outstanding, +// then taken largest-first until both are covered. Not guaranteed minimal-waste, unlike the +// exhaustive search. +func selectEvictionSetGreedy(candidates []evictionCandidate, requiredCPU, requiredMemory resource.Quantity) []evictionCandidate { + reqCPUf := requiredCPU.AsApproximateFloat64() + reqMemf := requiredMemory.AsApproximateFloat64() + + contribution := func(c evictionCandidate) float64 { + var s float64 + if reqCPUf > 0 { + s += c.CPU.AsApproximateFloat64() / reqCPUf + } + if reqMemf > 0 { + s += c.Memory.AsApproximateFloat64() / reqMemf + } + return s + } + + sorted := append([]evictionCandidate{}, candidates...) + sort.Slice(sorted, func(i, j int) bool { return contribution(sorted[i]) > contribution(sorted[j]) }) + + sumCPU := resource.Quantity{} + sumMemory := resource.Quantity{} + var chosen []evictionCandidate + for _, c := range sorted { + if qualifies(sumCPU, sumMemory, requiredCPU, requiredMemory) { + break + } + chosen = append(chosen, c) + sumCPU.Add(c.CPU) + sumMemory.Add(c.Memory) + } + if qualifies(sumCPU, sumMemory, requiredCPU, requiredMemory) { + return chosen + } + return nil +} + +// combinations calls fn once for every size-length combination of indices in [0,n). +func combinations(n, size int, fn func(idx []int)) { + idx := make([]int, size) + var rec func(start, depth int) + rec = func(start, depth int) { + if depth == size { + fn(append([]int{}, idx...)) + return + } + for i := start; i < n; i++ { + idx[depth] = i + rec(i+1, depth+1) + } + } + rec(0, 0) +} + +func pick(candidates []evictionCandidate, idx []int) []evictionCandidate { + out := make([]evictionCandidate, len(idx)) + for i, j := range idx { + out[i] = candidates[j] + } + return out +} diff --git a/operator/redisfailover/service/resize_headroom_test.go b/operator/redisfailover/service/resize_headroom_test.go new file mode 100644 index 000000000..29ff6a546 --- /dev/null +++ b/operator/redisfailover/service/resize_headroom_test.go @@ -0,0 +1,183 @@ +package service + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func mustQty(s string) resource.Quantity { + return resource.MustParse(s) +} + +func candidateNames(cs []evictionCandidate) []string { + names := make([]string, len(cs)) + for i, c := range cs { + names[i] = c.Name + } + return names +} + +func TestSelectEvictionSet_MemoryOnly(t *testing.T) { + // The worked example from the design: candidates {4, 3, 5, 1} GB memory, required 10 GB + // memory, no CPU requirement. Minimal-pod-count, minimal-waste answer is {5, 4, 1} = 10 + // (waste 0), not {5, 4, 3} = 12 (waste 2), even though both are valid 3-pod covers. + candidates := []evictionCandidate{ + {Namespace: "ns", Name: "four", Memory: mustQty("4Gi")}, + {Namespace: "ns", Name: "three", Memory: mustQty("3Gi")}, + {Namespace: "ns", Name: "five", Memory: mustQty("5Gi")}, + {Namespace: "ns", Name: "one", Memory: mustQty("1Gi")}, + } + + chosen := selectEvictionSet(candidates, mustQty("0"), mustQty("10Gi")) + + assert.Len(t, chosen, 3) + assert.ElementsMatch(t, []string{"five", "four", "one"}, candidateNames(chosen)) +} + +func TestSelectEvictionSet_CPUOnly(t *testing.T) { + // Same shape as the memory worked example, but the requirement is CPU-only (no memory + // requested at all) - proves the CPU dimension drives selection on its own, case 1 from + // the "cpu only / memory only / both" split. + candidates := []evictionCandidate{ + {Namespace: "ns", Name: "four", CPU: mustQty("4")}, + {Namespace: "ns", Name: "three", CPU: mustQty("3")}, + {Namespace: "ns", Name: "five", CPU: mustQty("5")}, + {Namespace: "ns", Name: "one", CPU: mustQty("1")}, + } + + chosen := selectEvictionSet(candidates, mustQty("10"), mustQty("0")) + + assert.Len(t, chosen, 3) + assert.ElementsMatch(t, []string{"five", "four", "one"}, candidateNames(chosen)) +} + +func TestSelectEvictionSet_BothRequired(t *testing.T) { + // Case 3: both CPU and memory are short, and the best per-dimension choice differs - + // proves the subset must satisfy both constraints together, not just one. + // A: cpu=2, mem=1 B: cpu=1, mem=4 C: cpu=3, mem=2 + // required: cpu=3, mem=4 + // No single candidate qualifies. Of the covering pairs, {A,B} (cpu=3,mem=5, waste 0.25) + // beats {B,C} (cpu=4,mem=6, waste 0.833); {A,C} doesn't cover memory at all (mem=3 < 4). + a := evictionCandidate{Namespace: "ns", Name: "a", CPU: mustQty("2"), Memory: mustQty("1Gi")} + b := evictionCandidate{Namespace: "ns", Name: "b", CPU: mustQty("1"), Memory: mustQty("4Gi")} + c := evictionCandidate{Namespace: "ns", Name: "c", CPU: mustQty("3"), Memory: mustQty("2Gi")} + + chosen := selectEvictionSet([]evictionCandidate{a, b, c}, mustQty("3"), mustQty("4Gi")) + + assert.Len(t, chosen, 2) + assert.ElementsMatch(t, []string{"a", "b"}, candidateNames(chosen)) +} + +func TestSelectEvictionSet_NoSingleOrPairCovers(t *testing.T) { + // Sanity check on the size-first search order: with candidates {4,3,5,1} and a + // target of 10, no size-1 or size-2 subset should ever be returned (max pair is 5+4=9). + candidates := []evictionCandidate{ + {Namespace: "ns", Name: "four", Memory: mustQty("4Gi")}, + {Namespace: "ns", Name: "three", Memory: mustQty("3Gi")}, + {Namespace: "ns", Name: "five", Memory: mustQty("5Gi")}, + {Namespace: "ns", Name: "one", Memory: mustQty("1Gi")}, + } + + chosen := selectEvictionSet(candidates, mustQty("0"), mustQty("10Gi")) + assert.GreaterOrEqual(t, len(chosen), 3) +} + +func TestSelectEvictionSet_Infeasible(t *testing.T) { + // Even taking every candidate isn't enough - must return nil, not the best-effort subset. + candidates := []evictionCandidate{ + {Namespace: "ns", Name: "a", Memory: mustQty("1Gi")}, + {Namespace: "ns", Name: "b", Memory: mustQty("2Gi")}, + } + + chosen := selectEvictionSet(candidates, mustQty("0"), mustQty("10Gi")) + assert.Nil(t, chosen) +} + +func TestSelectEvictionSet_InfeasibleOnOneDimensionOnly(t *testing.T) { + // Memory is easily covered, but CPU never is - must still return nil overall, since + // qualifies() requires both. + candidates := []evictionCandidate{ + {Namespace: "ns", Name: "a", CPU: mustQty("100m"), Memory: mustQty("8Gi")}, + {Namespace: "ns", Name: "b", CPU: mustQty("100m"), Memory: mustQty("8Gi")}, + } + + chosen := selectEvictionSet(candidates, mustQty("10"), mustQty("1Gi")) + assert.Nil(t, chosen) +} + +func TestSelectEvictionSet_ExactSingleMatch(t *testing.T) { + candidates := []evictionCandidate{ + {Namespace: "ns", Name: "small", Memory: mustQty("2Gi")}, + {Namespace: "ns", Name: "exact", Memory: mustQty("5Gi")}, + {Namespace: "ns", Name: "big", Memory: mustQty("8Gi")}, + } + + chosen := selectEvictionSet(candidates, mustQty("0"), mustQty("5Gi")) + assert.Len(t, chosen, 1) + assert.Equal(t, "exact", chosen[0].Name) +} + +func TestSelectEvictionSetGreedy_FallsBackAboveExhaustiveCap(t *testing.T) { + // Above maxExhaustiveCandidates, selectEvictionSet must delegate to the greedy + // fallback rather than attempting a combinatorial search. + var candidates []evictionCandidate + for i := 0; i < maxExhaustiveCandidates+1; i++ { + candidates = append(candidates, evictionCandidate{ + Namespace: "ns", + Name: "pod", + Memory: mustQty("1Gi"), + }) + } + + chosen := selectEvictionSet(candidates, mustQty("0"), mustQty("3Gi")) + assert.NotNil(t, chosen) + assert.GreaterOrEqual(t, len(chosen), 3) +} + +func TestBuildEvictionPool_ExcludesSameRedisFailover(t *testing.T) { + pods := &corev1.PodList{ + Items: []corev1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "other-endpoint-slave", + Namespace: "ns", + Labels: map[string]string{redisFailoverNameLabelKey: "other-endpoint"}, + }, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceMemory: mustQty("2Gi")}}, + }}}, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "same-endpoint-slave", + Namespace: "ns", + Labels: map[string]string{redisFailoverNameLabelKey: "target-endpoint"}, + }, + Spec: corev1.PodSpec{Containers: []corev1.Container{{ + Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{corev1.ResourceMemory: mustQty("2Gi")}}, + }}}, + }, + }, + } + + pool := buildEvictionPool(pods, "target-endpoint") + + assert.Len(t, pool, 1) + assert.Equal(t, "other-endpoint-slave", pool[0].Name) +} + +func TestRemoveCandidate(t *testing.T) { + pool := []evictionCandidate{ + {Namespace: "ns", Name: "a", Memory: mustQty("1Gi")}, + {Namespace: "ns", Name: "b", Memory: mustQty("2Gi")}, + } + + remaining := removeCandidate(pool, evictionCandidate{Namespace: "ns", Name: "a"}) + + assert.Len(t, remaining, 1) + assert.Equal(t, "b", remaining[0].Name) +} diff --git a/service/k8s/eviction.go b/service/k8s/eviction.go new file mode 100644 index 000000000..f11789988 --- /dev/null +++ b/service/k8s/eviction.go @@ -0,0 +1,76 @@ +package k8s + +import ( + "context" + + corev1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/client-go/kubernetes" + + "github.com/freshworks/redis-operator/log" + "github.com/freshworks/redis-operator/metrics" +) + +// Eviction knows how to list and evict pods across the cluster. It is used to free +// node headroom (e.g. evicting other RedisFailovers' slave pods co-located on a node) +// ahead of resizing a master pod in place. +type Eviction interface { + // GetNode retrieves a Node, used to read its Status.Allocatable capacity. + GetNode(name string) (*corev1.Node, error) + // ListPodsOnNode lists pods across all namespaces scheduled on nodeName, matching labelSelector. + ListPodsOnNode(nodeName, labelSelector string) (*corev1.PodList, error) + // EvictPod evicts a pod via the eviction subresource, which honors PodDisruptionBudgets + // (unlike a raw delete). + EvictPod(namespace, podName string) error +} + +// EvictionService is the Eviction service implementation using API calls to kubernetes. +type EvictionService struct { + kubeClient kubernetes.Interface + logger log.Logger + metricsRecorder metrics.Recorder +} + +// NewEvictionService returns a new Eviction KubeService. +func NewEvictionService(kubeClient kubernetes.Interface, logger log.Logger, metricsRecorder metrics.Recorder) *EvictionService { + logger = logger.With("service", "k8s.eviction") + return &EvictionService{ + kubeClient: kubeClient, + logger: logger, + metricsRecorder: metricsRecorder, + } +} + +func (e *EvictionService) GetNode(name string) (*corev1.Node, error) { + node, err := e.kubeClient.CoreV1().Nodes().Get(context.TODO(), name, metav1.GetOptions{}) + recordMetrics(metav1.NamespaceAll, "Node", name, "GET", err, e.metricsRecorder) + return node, err +} + +func (e *EvictionService) ListPodsOnNode(nodeName, labelSelector string) (*corev1.PodList, error) { + pods, err := e.kubeClient.CoreV1().Pods(metav1.NamespaceAll).List(context.TODO(), metav1.ListOptions{ + LabelSelector: labelSelector, + FieldSelector: fields.OneTermEqualSelector("spec.nodeName", nodeName).String(), + }) + recordMetrics(metav1.NamespaceAll, "Pod", metrics.NOT_APPLICABLE, "LIST", err, e.metricsRecorder) + return pods, err +} + +func (e *EvictionService) EvictPod(namespace, podName string) error { + eviction := &policyv1.Eviction{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: namespace, + }, + } + err := e.kubeClient.CoreV1().Pods(namespace).EvictV1(context.TODO(), eviction) + recordMetrics(namespace, "Pod", podName, "EVICT", err, e.metricsRecorder) + if err != nil { + e.logger.WithField("namespace", namespace).WithField("pod", podName).Errorf("evict pod failed: %v", err) + return err + } + e.logger.WithField("namespace", namespace).WithField("pod", podName).Infof("pod evicted") + return nil +} diff --git a/service/k8s/k8s.go b/service/k8s/k8s.go index 5059808a0..ddeadd9b7 100644 --- a/service/k8s/k8s.go +++ b/service/k8s/k8s.go @@ -20,6 +20,7 @@ type Services interface { RBAC Deployment StatefulSet + Eviction } type services struct { @@ -32,6 +33,7 @@ type services struct { RBAC Deployment StatefulSet + Eviction } // New returns a new Kubernetes service. @@ -46,5 +48,6 @@ func New(kubecli kubernetes.Interface, crdcli redisfailoverclientset.Interface, RBAC: NewRBACService(kubecli, logger, metricsRecorder), Deployment: NewDeploymentService(kubecli, logger, metricsRecorder), StatefulSet: NewStatefulSetService(kubecli, logger, metricsRecorder), + Eviction: NewEvictionService(kubecli, logger, metricsRecorder), } } diff --git a/service/k8s/pod.go b/service/k8s/pod.go index 1769b48e4..84ac96da8 100644 --- a/service/k8s/pod.go +++ b/service/k8s/pod.go @@ -3,6 +3,7 @@ package k8s import ( "context" "encoding/json" + "fmt" "k8s.io/apimachinery/pkg/types" @@ -27,6 +28,9 @@ type Pod interface { UpdatePodLabels(namespace, podName string, labels map[string]string) error UpdatePodAnnotations(namespace, podName string, annotations map[string]string) error RemovePodAnnotation(namespace, podName string, annotationKey string) error + // ResizePod applies new resource requests/limits to the named container of a running pod + // via the resize subresource, without recreating the pod. + ResizePod(namespace, podName, containerName string, resources corev1.ResourceRequirements) error } // PodService is the pod service implementation using API calls to kubernetes. @@ -187,3 +191,34 @@ func (p *PodService) RemovePodAnnotation(namespace, podName string, annotationKe delete(podAnnotations, annotationKey) }) } + +// ResizePod applies new resource requests/limits to the named container of a running pod +// via the resize subresource (stable since Kubernetes v1.35), without recreating the pod. +func (p *PodService) ResizePod(namespace, podName, containerName string, resources corev1.ResourceRequirements) error { + pod, err := p.GetPod(namespace, podName) + if err != nil { + return err + } + + updated := pod.DeepCopy() + found := false + for i := range updated.Spec.Containers { + if updated.Spec.Containers[i].Name == containerName { + updated.Spec.Containers[i].Resources = resources + found = true + break + } + } + if !found { + return fmt.Errorf("container %q not found in pod %q", containerName, podName) + } + + _, err = p.kubeClient.CoreV1().Pods(namespace).UpdateResize(context.TODO(), podName, updated, metav1.UpdateOptions{}) + recordMetrics(namespace, "Pod", podName, "RESIZE", err, p.metricsRecorder) + if err != nil { + p.logger.Errorf("Resize pod failed, namespace: %s, pod name: %s, error: %v", namespace, podName, err) + return err + } + p.logger.WithField("namespace", namespace).WithField("pod", podName).Debugf("pod resize requested") + return nil +} diff --git a/service/k8s/resize_diff_test.go b/service/k8s/resize_diff_test.go new file mode 100644 index 000000000..65536b058 --- /dev/null +++ b/service/k8s/resize_diff_test.go @@ -0,0 +1,64 @@ +package k8s + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func basePodSpec() *corev1.PodSpec { + return &corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "redis", + Image: "redis:7", + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("2Gi")}, + }, + }, + }, + } +} + +func TestIsResourceOnlyChange_ResourcesOnlyDiffers(t *testing.T) { + oldSpec := basePodSpec() + newSpec := basePodSpec() + newSpec.Containers[0].Resources.Requests[corev1.ResourceMemory] = resource.MustParse("4Gi") + + assert.True(t, isResourceOnlyChange(oldSpec, newSpec)) +} + +func TestIsResourceOnlyChange_NoDiffAtAll(t *testing.T) { + oldSpec := basePodSpec() + newSpec := basePodSpec() + + assert.True(t, isResourceOnlyChange(oldSpec, newSpec)) +} + +func TestIsResourceOnlyChange_ImageAlsoChanged(t *testing.T) { + oldSpec := basePodSpec() + newSpec := basePodSpec() + newSpec.Containers[0].Resources.Requests[corev1.ResourceMemory] = resource.MustParse("4Gi") + newSpec.Containers[0].Image = "redis:8" + + assert.False(t, isResourceOnlyChange(oldSpec, newSpec)) +} + +func TestIsResourceOnlyChange_CPUOnlyDiffers(t *testing.T) { + oldSpec := basePodSpec() + newSpec := basePodSpec() + newSpec.Containers[0].Resources.Requests[corev1.ResourceCPU] = resource.MustParse("500m") + + assert.True(t, isResourceOnlyChange(oldSpec, newSpec)) +} + +func TestIsResourceOnlyChange_EnvVarAlsoChanged(t *testing.T) { + oldSpec := basePodSpec() + newSpec := basePodSpec() + newSpec.Containers[0].Resources.Requests[corev1.ResourceMemory] = resource.MustParse("4Gi") + newSpec.Containers[0].Env = []corev1.EnvVar{{Name: "FOO", Value: "bar"}} + + assert.False(t, isResourceOnlyChange(oldSpec, newSpec)) +} diff --git a/service/k8s/statefulset.go b/service/k8s/statefulset.go index 1900855f7..34836d1c6 100644 --- a/service/k8s/statefulset.go +++ b/service/k8s/statefulset.go @@ -12,6 +12,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -20,6 +21,27 @@ import ( "github.com/freshworks/redis-operator/metrics" ) +// ResizeOnlyAnnotationKey marks a StatefulSet update as changing only container +// resources (CPU/memory) versus the previously stored template. UpdateRedisesPods +// reads this to decide whether the master pod can be resized in place instead of deleted. +const ResizeOnlyAnnotationKey = "redis-failover.freshworks.com/resize-only" + +// isResourceOnlyChange reports whether the only difference between oldSpec and newSpec +// is container resource requests/limits, by neutralizing that one field on copies of both +// and comparing everything else. Uses apiequality.Semantic.DeepEqual rather than +// reflect.DeepEqual because raw reflection can report false differences on resource.Quantity. +func isResourceOnlyChange(oldSpec, newSpec *corev1.PodSpec) bool { + oldCopy := oldSpec.DeepCopy() + newCopy := newSpec.DeepCopy() + for i := range oldCopy.Containers { + oldCopy.Containers[i].Resources = corev1.ResourceRequirements{} + } + for i := range newCopy.Containers { + newCopy.Containers[i].Resources = corev1.ResourceRequirements{} + } + return apiequality.Semantic.DeepEqual(oldCopy, newCopy) +} + // StatefulSet the StatefulSet service that knows how to interact with k8s to manage them type StatefulSet interface { GetStatefulSet(namespace, name string) (*appsv1.StatefulSet, error) @@ -170,6 +192,25 @@ func (s *StatefulSetService) CreateOrUpdateStatefulSet(namespace string, statefu } // set stored.volumeClaimTemplates statefulSet.Spec.VolumeClaimTemplates = storedStatefulSet.Spec.VolumeClaimTemplates + + // Dry-run the update first so the resource-only diff compares two objects that have both + // been through API server defaulting. storedStatefulSet came from a Get() and already has + // implicit defaults (terminationMessagePath, port protocol, etc.) filled in; statefulSet is + // a freshly-built, never-submitted Go struct that doesn't. Diffing them directly would show + // spurious differences from that defaulting gap alone, regardless of what actually changed. + dryRunResult, err := s.kubeClient.AppsV1().StatefulSets(namespace).Update(context.TODO(), statefulSet, metav1.UpdateOptions{ + DryRun: []string{metav1.DryRunAll}, + }) + if err != nil { + return err + } + + resourceOnlyChange := isResourceOnlyChange(&storedStatefulSet.Spec.Template.Spec, &dryRunResult.Spec.Template.Spec) + if statefulSet.Annotations == nil { + statefulSet.Annotations = map[string]string{} + } + statefulSet.Annotations[ResizeOnlyAnnotationKey] = strconv.FormatBool(resourceOnlyChange) + statefulSet.Annotations = util.MergeAnnotations(storedStatefulSet.Annotations, statefulSet.Annotations) return s.UpdateStatefulSet(namespace, statefulSet) } diff --git a/service/k8s/statefulset_test.go b/service/k8s/statefulset_test.go index 84590dae7..87171f030 100644 --- a/service/k8s/statefulset_test.go +++ b/service/k8s/statefulset_test.go @@ -87,10 +87,23 @@ func TestStatefulSetServiceGetCreateOrUpdate(t *testing.T) { getStatefulSetResult: testStatefulSet, errorOnGet: nil, errorOnCreation: nil, - expActions: []kubetesting.Action{ - newStatefulSetGetAction(testns, testStatefulSet.ObjectMeta.Name), - newStatefulSetUpdateAction(testns, testStatefulSet), - }, + expActions: func() []kubetesting.Action { + // The resource-only diff check dry-runs the update first (pre-annotation + // object state, DryRun: All), then performs the real update (with the + // resize-only annotation set, based on comparing the dry-run result). + preAnnotation := testStatefulSet.DeepCopy() + postAnnotation := testStatefulSet.DeepCopy() + postAnnotation.Annotations = map[string]string{k8s.ResizeOnlyAnnotationKey: "true"} + + dryRunAction := newStatefulSetUpdateAction(testns, preAnnotation) + dryRunAction.UpdateOptions = metav1.UpdateOptions{DryRun: []string{metav1.DryRunAll}} + + return []kubetesting.Action{ + newStatefulSetGetAction(testns, testStatefulSet.ObjectMeta.Name), + dryRunAction, + newStatefulSetUpdateAction(testns, postAnnotation), + } + }(), expErr: false, }, } @@ -107,6 +120,9 @@ func TestStatefulSetServiceGetCreateOrUpdate(t *testing.T) { mcli.AddReactor("create", "statefulsets", func(action kubetesting.Action) (bool, runtime.Object, error) { return true, nil, test.errorOnCreation }) + mcli.AddReactor("update", "statefulsets", func(action kubetesting.Action) (bool, runtime.Object, error) { + return true, testStatefulSet, nil + }) service := k8s.NewStatefulSetService(mcli, log.Dummy, metrics.Dummy) err := service.CreateOrUpdateStatefulSet(testns, test.statefulSet)