diff --git a/CHANGELOG.md b/CHANGELOG.md index 15e35e36b..6c4a76df0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ Also check this project's [releases](https://github.com/powerhome/redis-operator ## Unreleased +## [v3.0.0] - 2024-02-26 + +### Removed + +- [Remove HAProxy for redis with role:slave as unnecessary and potentially dangerous](https://github.com/powerhome/redis-operator/pull/50) + +Action required: + +If your application is using the `rfrs-haproxy-[redisfailvover-name]` service you'll need to use the `rfrs-[redis-failover-name]` service which bypassess HAProxy altogether. + ## [v2.1.0] - 2024-02-26 ### Fixed diff --git a/Makefile b/Makefile index e2600f8dc..f8db8c32b 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -VERSION := v2.1.0 +VERSION := v3.0.0 # Name of this service/application SERVICE_NAME := redis-operator diff --git a/mocks/operator/redisfailover/service/RedisFailoverClient.go b/mocks/operator/redisfailover/service/RedisFailoverClient.go index e12b989fe..3cb98ac38 100644 --- a/mocks/operator/redisfailover/service/RedisFailoverClient.go +++ b/mocks/operator/redisfailover/service/RedisFailoverClient.go @@ -14,6 +14,20 @@ type RedisFailoverClient struct { mock.Mock } +// DestroyOrphanedRedisSlaveHaProxy provides a mock function with given fields: rFailover +func (_m *RedisFailoverClient) DestroyOrphanedRedisSlaveHaProxy(rFailover *v1.RedisFailover) error { + ret := _m.Called(rFailover) + + var r0 error + if rf, ok := ret.Get(0).(func(*v1.RedisFailover) error); ok { + r0 = rf(rFailover) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // DestroySentinelResources provides a mock function with given fields: rFailover func (_m *RedisFailoverClient) DestroySentinelResources(rFailover *v1.RedisFailover) error { ret := _m.Called(rFailover) @@ -84,48 +98,6 @@ func (_m *RedisFailoverClient) EnsureHAProxyRedisMasterService(rFailover *v1.Red return r0 } -// EnsureHAProxyRedisSlaveConfigmap provides a mock function with given fields: rFailover, labels, ownerRefs -func (_m *RedisFailoverClient) EnsureHAProxyRedisSlaveConfigmap(rFailover *v1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { - ret := _m.Called(rFailover, labels, ownerRefs) - - var r0 error - if rf, ok := ret.Get(0).(func(*v1.RedisFailover, map[string]string, []metav1.OwnerReference) error); ok { - r0 = rf(rFailover, labels, ownerRefs) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// EnsureHAProxyRedisSlaveDeployment provides a mock function with given fields: rFailover, labels, ownerRefs -func (_m *RedisFailoverClient) EnsureHAProxyRedisSlaveDeployment(rFailover *v1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { - ret := _m.Called(rFailover, labels, ownerRefs) - - var r0 error - if rf, ok := ret.Get(0).(func(*v1.RedisFailover, map[string]string, []metav1.OwnerReference) error); ok { - r0 = rf(rFailover, labels, ownerRefs) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// EnsureHAProxyRedisSlaveService provides a mock function with given fields: rFailover, labels, ownerRefs -func (_m *RedisFailoverClient) EnsureHAProxyRedisSlaveService(rFailover *v1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { - ret := _m.Called(rFailover, labels, ownerRefs) - - var r0 error - if rf, ok := ret.Get(0).(func(*v1.RedisFailover, map[string]string, []metav1.OwnerReference) error); ok { - r0 = rf(rFailover, labels, ownerRefs) - } else { - r0 = ret.Error(0) - } - - return r0 -} - // EnsureNotPresentRedisService provides a mock function with given fields: rFailover func (_m *RedisFailoverClient) EnsureNotPresentRedisService(rFailover *v1.RedisFailover) error { ret := _m.Called(rFailover) diff --git a/operator/redisfailover/ensurer.go b/operator/redisfailover/ensurer.go index 33870a3f2..48ff08d26 100644 --- a/operator/redisfailover/ensurer.go +++ b/operator/redisfailover/ensurer.go @@ -46,18 +46,9 @@ func (w *RedisFailoverHandler) Ensure(rf *redisfailoverv1.RedisFailover, labels return err } - if err := w.rfService.EnsureHAProxyRedisSlaveService(rf, labels, or); err != nil { + if err := w.rfService.DestroyOrphanedRedisSlaveHaProxy(rf); err != nil { return err } - - if err := w.rfService.EnsureHAProxyRedisSlaveConfigmap(rf, labels, or); err != nil { - return err - } - - if err := w.rfService.EnsureHAProxyRedisSlaveDeployment(rf, labels, or); err != nil { - return err - } - } if err := w.rfService.EnsureRedisMasterService(rf, labels, or); err != nil { diff --git a/operator/redisfailover/ensurer_test.go b/operator/redisfailover/ensurer_test.go index 2a8b11ac8..24331d75d 100644 --- a/operator/redisfailover/ensurer_test.go +++ b/operator/redisfailover/ensurer_test.go @@ -139,9 +139,7 @@ func TestEnsure(t *testing.T) { mrfs.On("EnsureHAProxyRedisMasterConfigmap", rf, mock.Anything, mock.Anything).Once().Return(nil) mrfs.On("EnsureHAProxyRedisMasterDeployment", rf, mock.Anything, mock.Anything).Once().Return(nil) - mrfs.On("EnsureHAProxyRedisSlaveService", rf, mock.Anything, mock.Anything).Once().Return(nil) - mrfs.On("EnsureHAProxyRedisSlaveConfigmap", rf, mock.Anything, mock.Anything).Once().Return(nil) - mrfs.On("EnsureHAProxyRedisSlaveDeployment", rf, mock.Anything, mock.Anything).Once().Return(nil) + mrfs.On("DestroyOrphanedRedisSlaveHaProxy", rf, mock.Anything, mock.Anything).Once().Return(nil) } mrfs.On("EnsureRedisMasterService", rf, mock.Anything, mock.Anything).Once().Return(nil) diff --git a/operator/redisfailover/service/client.go b/operator/redisfailover/service/client.go index fef338439..72d587dfa 100644 --- a/operator/redisfailover/service/client.go +++ b/operator/redisfailover/service/client.go @@ -32,14 +32,11 @@ type RedisFailoverClient interface { EnsureRedisConfigMap(rFailover *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error EnsureNotPresentRedisService(rFailover *redisfailoverv1.RedisFailover) error - EnsureHAProxyRedisSlaveService(rFailover *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error - EnsureHAProxyRedisSlaveConfigmap(rFailover *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error - EnsureHAProxyRedisSlaveDeployment(rFailover *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error - DestroySentinelResources(rFailover *redisfailoverv1.RedisFailover) error UpdateStatus(rFailover *redisfailoverv1.RedisFailover) (*redisfailoverv1.RedisFailover, error) DestroydOrphanedRedisNetworkPolicy(rFailover *redisfailoverv1.RedisFailover) error + DestroyOrphanedRedisSlaveHaProxy(rFailover *redisfailoverv1.RedisFailover) error } // RedisFailoverKubeClient implements the required methods to talk with kubernetes @@ -129,31 +126,6 @@ func (r *RedisFailoverKubeClient) EnsureHAProxyRedisMasterDeployment(rf *redisfa return err } -// EnsureHAProxyRedisSlaveService makes sure the HAProxy service exists -func (r *RedisFailoverKubeClient) EnsureHAProxyRedisSlaveService(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { - svc := generateHAProxyRedisSlaveService(rf, labels, ownerRefs) - err := r.K8SService.CreateOrUpdateService(rf.Namespace, svc) - r.setEnsureOperationMetrics(svc.Namespace, svc.Name, "EnsureHAProxyRedisMasterService", rf.Name, err) - return err -} - -// EnsureHAProxyRedisSlaveConfigmap makes sure the HAProxy configmap exists -func (r *RedisFailoverKubeClient) EnsureHAProxyRedisSlaveConfigmap(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { - svc := generateHAProxyRedisSlaveConfigmap(rf, labels, ownerRefs) - err := r.K8SService.CreateOrUpdateConfigMap(rf.Namespace, svc) - r.setEnsureOperationMetrics(svc.Namespace, svc.Name, "EnsureHAProxyRedisMasterConfigmap", rf.Name, err) - return err -} - -// EnsureHAProxyRedisSlaveDeployment makes sure the deployment exists in the desired state -func (r *RedisFailoverKubeClient) EnsureHAProxyRedisSlaveDeployment(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { - d := generateHAProxyRedisSlaveDeployment(rf, labels, ownerRefs) - err := r.K8SService.CreateOrUpdateDeployment(rf.Namespace, d) - - r.setEnsureOperationMetrics(d.Namespace, d.Name, "EnsureHAProxyRedisMasterDeployment", rf.Name, err) - return err -} - // EnsureSentinelService makes sure the sentinel service exists func (r *RedisFailoverKubeClient) EnsureSentinelService(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { svc := generateSentinelService(rf, labels, ownerRefs) @@ -230,6 +202,50 @@ func (r *RedisFailoverKubeClient) DestroydOrphanedRedisNetworkPolicy(rf *redisfa return err } +func (r *RedisFailoverKubeClient) DestroyOrphanedRedisSlaveHaProxy(rf *redisfailoverv1.RedisFailover) error { + + // Helper function to handle the deletion of resources + deleteResource := func(namespace, name string, getter func(namespace, name string) (interface{}, error), deleter func(namespace, name string) error) error { + _, err := getter(namespace, name) + if err != nil { + if errors.IsNotFound(err) { + return nil + } + return err + } + return deleter(namespace, name) + } + + resourceTypes := map[string]struct { + getter func(namespace, name string) (interface{}, error) + deleter func(namespace, name string) error + }{ + "service": { + getter: func(namespace, name string) (interface{}, error) { return r.K8SService.GetService(namespace, name) }, + deleter: r.K8SService.DeleteService, + }, + "configmap": { + getter: func(namespace, name string) (interface{}, error) { return r.K8SService.GetConfigMap(namespace, name) }, + deleter: r.K8SService.DeleteConfigMap, + }, + "deployment": { + getter: func(namespace, name string) (interface{}, error) { return r.K8SService.GetDeployment(namespace, name) }, + deleter: r.K8SService.DeleteDeployment, + }, + } + + name := GetHaproxySlaveName(rf) + + for _, resType := range []string{"service", "configmap", "deployment"} { + resource := resourceTypes[resType] + if err := deleteResource(rf.Namespace, name, resource.getter, resource.deleter); err != nil { + return err + } + } + + return nil +} + // EnsureRedisStatefulset makes sure the redis statefulset exists in the desired state func (r *RedisFailoverKubeClient) EnsureRedisStatefulset(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error { if !rf.Spec.Redis.DisablePodDisruptionBudget { diff --git a/operator/redisfailover/service/generator.go b/operator/redisfailover/service/generator.go index cb15228e2..1a1f05729 100644 --- a/operator/redisfailover/service/generator.go +++ b/operator/redisfailover/service/generator.go @@ -273,189 +273,6 @@ func generateHAProxyRedisMasterService(rf *redisfailoverv1.RedisFailover, labels } } -func generateHAProxyRedisSlaveDeployment(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) *appsv1.Deployment { - - name := GetHaproxySlaveName(rf) - - namespace := rf.Namespace - - labels = util.MergeLabels(labels, generateSelectorLabels("haproxy", rf.Name), generateRedisSlaveRoleLabel()) - - selectorLabels := util.MergeLabels( - labels, - generateComponentLabel("haproxy"), - ) - - volumeMounts := []corev1.VolumeMount{ - { - Name: "config", - MountPath: "/usr/local/etc/haproxy/haproxy.cfg", - SubPath: "haproxy.cfg", - ReadOnly: true, - }, - } - - volumes := []corev1.Volume{ - { - Name: "config", - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: name, - }, - }, - }, - }, - } - - sd := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: labels, - OwnerReferences: ownerRefs, - }, - Spec: appsv1.DeploymentSpec{ - Replicas: &rf.Spec.Haproxy.Replicas, - Selector: &metav1.LabelSelector{ - MatchLabels: selectorLabels, - }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: selectorLabels, - }, - Spec: corev1.PodSpec{ - Containers: []corev1.Container{ - { - Name: "haproxy", - Image: rf.Spec.Haproxy.Image, - Ports: []corev1.ContainerPort{ - { - ContainerPort: rf.Spec.Redis.Port.ToInt32(), - }, - }, - VolumeMounts: volumeMounts, - Resources: rf.Spec.Haproxy.Resources, - }, - }, - Volumes: volumes, - RestartPolicy: "Always", - }, - }, - }, - } - - if rf.Spec.Haproxy.Affinity != nil { - sd.Spec.Template.Spec.Affinity = rf.Spec.Haproxy.Affinity - } - - return sd -} - -func generateHAProxyRedisSlaveConfigmap(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) *corev1.ConfigMap { - name := GetHaproxySlaveName(rf) - redisName := rf.GenerateName("redis") - - namespace := rf.Namespace - - labels = util.MergeLabels(labels, generateSelectorLabels("haproxy", rf.Name), generateRedisSlaveRoleLabel()) - - port := rf.Spec.Redis.Port - haproxyCfg := fmt.Sprintf(`global - daemon - maxconn 5000 - - defaults - mode tcp - timeout connect 5000ms - timeout client 50000ms - timeout server 50000ms - timeout check 5000ms - - frontend http - bind :8080 - default_backend stats - - backend stats - mode http - stats enable - stats uri / - stats refresh 1s - stats show-legends - stats admin if TRUE - - resolvers k8s - parse-resolv-conf - hold other 10s - hold refused 10s - hold nx 10 - hold timeout 10s - hold valid 10s - hold obsolete 10s - - frontend redis-slave - bind *:%d - default_backend redis-slave - - backend redis-slave - mode tcp - balance first - option tcp-check - tcp-check send info\ replication\r\n - tcp-check expect string role:slave - server-template redis %d _redis._tcp.%s.%s.svc.cluster.local:%d check inter 1s resolvers k8s init-addr none -`, port, rf.Spec.Redis.Replicas, redisName, namespace, port) - - return &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: labels, - OwnerReferences: ownerRefs, - }, - Data: map[string]string{ - "haproxy.cfg": haproxyCfg, - }, - } -} - -func generateHAProxyRedisSlaveService(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) *corev1.Service { - name := GetHaproxySlaveName(rf) - - namespace := rf.Namespace - redisTargetPort := intstr.FromInt(int(rf.Spec.Redis.Port)) - - selectorLabels := util.MergeLabels(labels, generateSelectorLabels("haproxy", rf.Name), generateRedisSlaveRoleLabel()) - - selectorLabels = util.MergeLabels( - selectorLabels, - generateComponentLabel("haproxy"), - labels) - - spec := corev1.ServiceSpec{ - Selector: selectorLabels, - Type: "ClusterIP", - Ports: []corev1.ServicePort{ - { - Name: "redis-slave", - Port: rf.Spec.Redis.Port.ToInt32(), - TargetPort: redisTargetPort, - Protocol: "TCP", - }, - }, - } - - return &corev1.Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: labels, - OwnerReferences: ownerRefs, - }, - Spec: spec, - } -} - func generateSentinelNetworkPolicy(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) *np.NetworkPolicy { name := GetSentinelNetworkPolicyName(rf) namespace := rf.Namespace