Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Redis Slave Endpoint to HAProxy Service configuration #35

Closed
wants to merge 1 commit into from
Closed

Add Redis Slave Endpoint to HAProxy Service configuration #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions operator/redisfailover/service/generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,26 @@ func generateHAProxyConfigmap(rf *redisfailoverv1.RedisFailover, labels map[stri
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)

if rf.Spec.BootstrapNode != nil && rf.Spec.BootstrapNode.Port != rf.Spec.Redis.Port.ToString() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth noting that the redis-operator already exposes Services to get at the "master" node, or "slave" nodes of a RedisFailover. I'm not sure I fully understand the cost/benefit equation of doing the same thing with more moving parts via HAProxy.

That said, BootstrapNode.Port is

If you are wanting to migrate off of a pre-existing Redis instance, you can provide a bootstrapNode to your RedisFailover resource spec...The Port that the target Redis address is listening to. Defaults to 6379.

I'm confused, so maybe I'm reading this code wrong. Wouldn't this expose the slave nodes of the Replicating RedisFailover on the Port that the "target"/source Redis instance is exposed on?

IF I've understood this correctly, had you considered the following alternatives that avoid overloading the meaning of the Bootstrap.Port:

  1. Expose the slaves on a static port. To be clear, I'm fan of this idea yet the redis-operator already does this for the metrics exporter(s)12; so it's not that far off script and we could make it more dynamic later if the need arises.
  2. Make the HAPRoxy ports configurable like the redis-operator already does for the Redis and Sentinel nodes.

Footnotes

  1. (https://github.com/powerhome/redis-operator/blob/bd12c9ae881b5b50f34fbb511237b08c35348234/operator/redisfailover/service/generator.go#L451) ↩

  2. (https://github.com/powerhome/redis-operator/blob/632aa3da88ee2a28ccb9fc4f571ddcb556dbd713/operator/redisfailover/service/constants.go#L5) ↩


portInt, _ := strconv.Atoi(rf.Spec.BootstrapNode.Port)
haproxyCfg += fmt.Sprintf(`
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
`,
portInt, rf.Spec.Redis.Replicas, redisName, namespace, port)

}

if rf.Spec.Haproxy.CustomConfig != "" {
haproxyCfg = rf.Spec.Haproxy.CustomConfig
}
Expand Down Expand Up @@ -263,6 +283,18 @@ func generateHAProxyService(rf *redisfailoverv1.RedisFailover, labels map[string
},
}

if rf.Spec.BootstrapNode != nil && rf.Spec.BootstrapNode.Port != rf.Spec.Redis.Port.ToString() {

portInt, _ := strconv.Atoi(rf.Spec.BootstrapNode.Port)
additionalPort := corev1.ServicePort{
Name: "redis-slave",
Port: int32(portInt),
TargetPort: intstr.FromInt(int(portInt)),
Protocol: "TCP",
}
spec.Ports = append(spec.Ports, additionalPort)
}

serviceSettings := rf.Spec.Haproxy.Service
if serviceSettings != nil {
if serviceSettings.ClusterIP != "" {
Expand Down
94 changes: 85 additions & 9 deletions operator/redisfailover/service/generator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package service_test

import (
"fmt"
"strconv"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -1297,12 +1298,15 @@ func TestRedisService(t *testing.T) {
func TestHaproxyService(t *testing.T) {
haproxyName := "redis-haproxy"
portName := "redis-master"
defaultRedisPort := redisfailoverv1.Port(6379)
slaveRedisPortName := "redis-slave"
redisPort := 6379
redisSlavePort := 6389
defaultRedisPort := redisfailoverv1.Port(redisPort)
defaultRedisSlavePort := redisfailoverv1.Port(redisSlavePort)
customClusterIP := "10.1.1.100"
tests := []struct {
name string
rfName string
rfNamespace string
rfBootstrap redisfailoverv1.BootstrapSettings
rfLabels map[string]string
rfAnnotations map[string]string
rfRedisPort redisfailoverv1.Port
Expand Down Expand Up @@ -1374,6 +1378,79 @@ func TestHaproxyService(t *testing.T) {
},
},
},
}, {
name: "with bootstrapSettings without Port",
rfRedisPort: defaultRedisPort,
rfBootstrap: redisfailoverv1.BootstrapSettings{
Host: "redis",
Port: strconv.Itoa(redisPort),
},
expectedService: corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: haproxyName,
Namespace: namespace,
OwnerReferences: []metav1.OwnerReference{
{
Name: "testing",
},
},
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeClusterIP,
Selector: map[string]string{
"app.kubernetes.io/component": "redis",
"redisfailovers.databases.spotahome.com/component": "haproxy",
},
Ports: []corev1.ServicePort{
{
Name: portName,
Port: defaultRedisPort.ToInt32(),
TargetPort: intstr.FromInt(int(defaultRedisPort)),
Protocol: corev1.ProtocolTCP,
},
},
},
},
},
{
name: "with bootstrapSettings and with different Port",
rfRedisPort: defaultRedisPort,
rfBootstrap: redisfailoverv1.BootstrapSettings{
Host: "redis",
Port: defaultRedisSlavePort.ToString(),
},
expectedService: corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: haproxyName,
Namespace: namespace,
OwnerReferences: []metav1.OwnerReference{
{
Name: "testing",
},
},
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeClusterIP,
Selector: map[string]string{
"app.kubernetes.io/component": "redis",
"redisfailovers.databases.spotahome.com/component": "haproxy",
},
Ports: []corev1.ServicePort{
{
Name: portName,
Port: defaultRedisPort.ToInt32(),
TargetPort: intstr.FromInt(int(defaultRedisPort)),
Protocol: corev1.ProtocolTCP,
},
{
Name: slaveRedisPortName,
Port: defaultRedisSlavePort.ToInt32(),
TargetPort: intstr.FromInt(int(defaultRedisSlavePort)),
Protocol: corev1.ProtocolTCP,
},
},
},
},
},
}

Expand All @@ -1383,16 +1460,15 @@ func TestHaproxyService(t *testing.T) {

// Generate a default RedisFailover and attaching the required annotations
rf := generateRF()
if test.rfName != "" {
rf.Name = test.rfName
}
if test.rfNamespace != "" {
rf.Namespace = test.rfNamespace
}

rf.Spec.Redis.ServiceAnnotations = test.rfAnnotations
rf.Spec.Redis.Port = test.rfRedisPort
rf.Spec.Haproxy = &test.haproxy

if test.rfBootstrap.Host != "" {
rf.Spec.BootstrapNode = &test.rfBootstrap
}

generatedService := corev1.Service{}

ms := &mK8SService.Services{}
Expand Down