From d9cab77143c92be1a931a474462f290f083794dc Mon Sep 17 00:00:00 2001 From: Roi Vazquez Date: Mon, 16 Oct 2023 21:16:00 +0200 Subject: [PATCH] Avoid failover delays in twemproxy reconfig when using master targets It seems that even though the "sentinel master " command returns updated information about the master as soon as a slave is promoted, there is an exeption with the sentinel instance that acts as "failover leader". The failover leader is the instance that actually performs the shard reconfigurations, and in this case, its "sentinel master " command only returns updated information when all the slaves have been reconfigured to point to the new master. This causes delays in twemproxy reconfiguration if this is the instance being used to "discover" the shard. To detect this situation, add a step that checks the master address with the command "sentinel get-master-addr-by-name ". This command always returns updated master information, even if we are querying the leader sentinel instance. --- controllers/sentinel_controller.go | 3 +- examples/backend/redis-v6/redis.yaml | 6 +- examples/backend/redis-v6/sentinel.yaml | 2 +- .../twemproxyconfig/generator_test.go | 56 ++++ pkg/redis/client/fake_client.go | 5 + pkg/redis/client/goredis_client.go | 6 + pkg/redis/client/interface.go | 1 + pkg/redis/server/server.go | 14 + pkg/redis/sharded/discover.go | 1 + pkg/redis/sharded/redis_shard.go | 23 ++ pkg/redis/sharded/redis_shard_test.go | 275 ++++++++++++------ .../sharded/redis_sharded_cluster_test.go | 35 +++ 12 files changed, 338 insertions(+), 89 deletions(-) diff --git a/controllers/sentinel_controller.go b/controllers/sentinel_controller.go index b98a1f12..cb2c6413 100644 --- a/controllers/sentinel_controller.go +++ b/controllers/sentinel_controller.go @@ -175,7 +175,8 @@ func (r *SentinelReconciler) reconcileStatus(ctx context.Context, instance *saas // and rely on reconciles triggered by sentinel events to correct the situation. masterError := &sharded.DiscoveryError_Master_SingleServerFailure{} slaveError := &sharded.DiscoveryError_Slave_SingleServerFailure{} - if errors.As(merr, masterError) || errors.As(merr, slaveError) { + failoverInProgressError := &sharded.DiscoveryError_Slave_FailoverInProgress{} + if errors.As(merr, masterError) || errors.As(merr, slaveError) || errors.As(merr, failoverInProgressError) { log.Error(merr, "DiscoveryError") } diff --git a/examples/backend/redis-v6/redis.yaml b/examples/backend/redis-v6/redis.yaml index 4bf44d8e..c525ba0b 100644 --- a/examples/backend/redis-v6/redis.yaml +++ b/examples/backend/redis-v6/redis.yaml @@ -4,7 +4,7 @@ metadata: name: shard01 spec: image: - tag: 6.2.7-alpine + tag: 6.2.13-alpine --- apiVersion: saas.3scale.net/v1alpha1 kind: RedisShard @@ -12,7 +12,7 @@ metadata: name: shard02 spec: image: - tag: 6.2.7-alpine + tag: 6.2.13-alpine --- apiVersion: saas.3scale.net/v1alpha1 @@ -22,4 +22,4 @@ metadata: spec: slaveCount: 0 image: - tag: 6.2.7-alpine \ No newline at end of file + tag: 6.2.13-alpine \ No newline at end of file diff --git a/examples/backend/redis-v6/sentinel.yaml b/examples/backend/redis-v6/sentinel.yaml index 6b0c2286..fb7f4234 100644 --- a/examples/backend/redis-v6/sentinel.yaml +++ b/examples/backend/redis-v6/sentinel.yaml @@ -5,7 +5,7 @@ metadata: spec: replicas: 3 image: - tag: 6.2.7-debian-11-r17 + tag: 6.2.13-debian-11-r76 config: monitoredShards: # DNS should not be used in production. DNS is diff --git a/pkg/generators/twemproxyconfig/generator_test.go b/pkg/generators/twemproxyconfig/generator_test.go index db8f7eef..b02ec814 100644 --- a/pkg/generators/twemproxyconfig/generator_test.go +++ b/pkg/generators/twemproxyconfig/generator_test.go @@ -92,6 +92,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard0) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, redis_client.FakeResponse{ // cmd: SentinelMaster (shard1) InjectResponse: func() interface{} { @@ -99,6 +106,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard1) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "5000"} + }, + InjectError: func() error { return nil }, + }, ), ), log: logr.Discard(), @@ -258,6 +272,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard0) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, redis_client.FakeResponse{ // cmd: SentinelSlaves (shard0) InjectResponse: func() interface{} { @@ -285,6 +306,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard1) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "5000"} + }, + InjectError: func() error { return nil }, + }, redis_client.FakeResponse{ // cmd: SentinelSlaves (shard1) InjectResponse: func() interface{} { @@ -428,6 +456,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard0) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, redis_client.FakeResponse{ // cmd: SentinelSlaves (shard0) InjectResponse: func() interface{} { @@ -455,6 +490,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard1) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "5000"} + }, + InjectError: func() error { return nil }, + }, redis_client.FakeResponse{ // cmd: SentinelSlaves (shard1) InjectResponse: func() interface{} { @@ -593,6 +635,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard0) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, redis_client.FakeResponse{ // cmd: SentinelMaster (shard1) InjectResponse: func() interface{} { @@ -600,6 +649,13 @@ func TestNewGenerator(t *testing.T) { }, InjectError: func() error { return nil }, }, + redis_client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard1) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "5000"} + }, + InjectError: func() error { return nil }, + }, redis_client.FakeResponse{ // cmd: SentinelSlaves (shard1) InjectResponse: func() interface{} { diff --git a/pkg/redis/client/fake_client.go b/pkg/redis/client/fake_client.go index 14416123..0621a08a 100644 --- a/pkg/redis/client/fake_client.go +++ b/pkg/redis/client/fake_client.go @@ -58,6 +58,11 @@ func (fc *FakeClient) SentinelMaster(ctx context.Context, shard string) (*Sentin return rsp.InjectResponse().(*SentinelMasterCmdResult), rsp.InjectError() } +func (fc *FakeClient) SentinelGetMasterAddrByName(ctx context.Context, shard string) ([]string, error) { + rsp := fc.pop() + return rsp.InjectResponse().([]string), rsp.InjectError() +} + func (fc *FakeClient) SentinelMasters(ctx context.Context) ([]interface{}, error) { rsp := fc.pop() return rsp.InjectResponse().([]interface{}), rsp.InjectError() diff --git a/pkg/redis/client/goredis_client.go b/pkg/redis/client/goredis_client.go index aaf34dc8..864922dd 100644 --- a/pkg/redis/client/goredis_client.go +++ b/pkg/redis/client/goredis_client.go @@ -73,6 +73,12 @@ func (c *GoRedisClient) SentinelMaster(ctx context.Context, shard string) (*Sent return result, err } +func (c *GoRedisClient) SentinelGetMasterAddrByName(ctx context.Context, shard string) ([]string, error) { + + values, err := c.sentinel.GetMasterAddrByName(ctx, shard).Result() + return values, err +} + func (c *GoRedisClient) SentinelMasters(ctx context.Context) ([]interface{}, error) { values, err := c.sentinel.Masters(ctx).Result() diff --git a/pkg/redis/client/interface.go b/pkg/redis/client/interface.go index ec3895d8..0fe445a4 100644 --- a/pkg/redis/client/interface.go +++ b/pkg/redis/client/interface.go @@ -13,6 +13,7 @@ import ( type TestableInterface interface { SentinelMaster(context.Context, string) (*SentinelMasterCmdResult, error) SentinelMasters(context.Context) ([]interface{}, error) + SentinelGetMasterAddrByName(ctx context.Context, shard string) ([]string, error) SentinelSlaves(context.Context, string) ([]interface{}, error) SentinelMonitor(context.Context, string, string, string, int) error SentinelSet(context.Context, string, string, string) error diff --git a/pkg/redis/server/server.go b/pkg/redis/server/server.go index 6c6d691b..79d4d383 100644 --- a/pkg/redis/server/server.go +++ b/pkg/redis/server/server.go @@ -4,6 +4,7 @@ import ( "bufio" "context" "net" + "strconv" "strings" "sync" "time" @@ -109,6 +110,19 @@ func (srv *Server) SentinelMaster(ctx context.Context, shard string) (*client.Se return result, nil } +func (srv *Server) SentinelGetMasterAddrByName(ctx context.Context, shard string) (string, int, error) { + + values, err := srv.client.SentinelGetMasterAddrByName(ctx, shard) + if err != nil { + return "", 0, err + } + port, err := strconv.Atoi(values[1]) + if err != nil { + return "", 0, err + } + return values[0], port, nil +} + func (srv *Server) SentinelMasters(ctx context.Context) ([]client.SentinelMasterCmdResult, error) { values, err := srv.client.SentinelMasters(ctx) diff --git a/pkg/redis/sharded/discover.go b/pkg/redis/sharded/discover.go index 02fc2162..e758a578 100644 --- a/pkg/redis/sharded/discover.go +++ b/pkg/redis/sharded/discover.go @@ -105,5 +105,6 @@ func (srv *RedisServer) Discover(ctx context.Context, opts ...DiscoveryOption) e // Discovery errors type DiscoveryError_Sentinel_Failure struct{ error } type DiscoveryError_Master_SingleServerFailure struct{ error } +type DiscoveryError_Slave_FailoverInProgress struct{ error } type DiscoveryError_Slave_SingleServerFailure struct{ error } type DiscoveryError_UnknownRole_SingleServerFailure struct{ error } diff --git a/pkg/redis/sharded/redis_shard.go b/pkg/redis/sharded/redis_shard.go index 42382b5d..85757f25 100644 --- a/pkg/redis/sharded/redis_shard.go +++ b/pkg/redis/sharded/redis_shard.go @@ -3,7 +3,9 @@ package sharded import ( "context" "fmt" + "net" "sort" + "strconv" "strings" "github.com/3scale/saas-operator/pkg/redis/client" @@ -79,6 +81,22 @@ func (shard *Shard) Discover(ctx context.Context, sentinel *SentinelServer, opti return append(merr, DiscoveryError_Sentinel_Failure{err}) } + // If a failover is currently in progress, only the GetMasterAddrByName command returns the + // updated address of the master. The other commands ('sentinel masters' and 'sentinel master ) + // wait until all the replicas are also reconfigured to start announcing the new IP, which can cause delays + // to reconfigure the clients. + failoverInProgress := false + masterIP, masterPort, err := sentinel.SentinelGetMasterAddrByName(ctx, shard.Name) + if err != nil { + return append(merr, DiscoveryError_Sentinel_Failure{err}) + } + if masterIP != sentinelMasterResult.IP || masterPort != sentinelMasterResult.Port { + failoverInProgress = true + logger.Info("failover in progress", "oldMaster", net.JoinHostPort(sentinelMasterResult.IP, strconv.Itoa(sentinelMasterResult.Port)), "newMaster", net.JoinHostPort(masterIP, strconv.Itoa(masterPort))) + sentinelMasterResult.IP = masterIP + sentinelMasterResult.Port = masterPort + } + // Get the corresponding server or add a new one if not found srv, err := shard.GetServerByID(fmt.Sprintf("%s:%d", sentinelMasterResult.IP, sentinelMasterResult.Port)) if err != nil { @@ -108,6 +126,11 @@ func (shard *Shard) Discover(ctx context.Context, sentinel *SentinelServer, opti return merr.ErrorOrNil() } + // don't discover slaves if a failover is in progress + if failoverInProgress { + return append(merr, DiscoveryError_Slave_FailoverInProgress{fmt.Errorf("refusing to discover slaves while a failover is in progress")}) + } + // discover slaves sentinelSlavesResult, err := sentinel.SentinelSlaves(ctx, shard.Name) if err != nil { diff --git a/pkg/redis/sharded/redis_shard_test.go b/pkg/redis/sharded/redis_shard_test.go index d40ac28a..fb33243f 100644 --- a/pkg/redis/sharded/redis_shard_test.go +++ b/pkg/redis/sharded/redis_shard_test.go @@ -19,6 +19,9 @@ func DiscoveredServersAreEqual(a, b *Shard) (bool, []string) { } for idx := range a.Servers { + if a.Servers[idx].ID() != b.Servers[idx].ID() { + return false, []string{fmt.Sprintf("%s != %s", a.Servers[idx].ID(), b.Servers[idx].ID())} + } if a.Servers[idx].Role != b.Servers[idx].Role { return false, []string{fmt.Sprintf("%s != %s", a.Servers[idx].Role, b.Servers[idx].Role)} } @@ -222,9 +225,9 @@ func TestShard_Discover(t *testing.T) { }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{}}, - {Role: client.Slave, Config: map[string]string{}}, - {Role: client.Slave, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "2000", nil), Role: client.Slave, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "3000", nil), Role: client.Slave, Config: map[string]string{}}, }}, wantErr: false, }, @@ -264,9 +267,9 @@ func TestShard_Discover(t *testing.T) { }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{}}, - {Role: client.Unknown, Config: map[string]string{}}, - {Role: client.Slave, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "2000", nil), Role: client.Unknown, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "3000", nil), Role: client.Slave, Config: map[string]string{}}, }}, wantErr: true, }, @@ -302,6 +305,13 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves InjectResponse: func() interface{} { @@ -327,9 +337,9 @@ func TestShard_Discover(t *testing.T) { }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{"save": ""}}, - {Role: client.Slave, Config: map[string]string{"save": "900 1 300 10", "slave-read-only": "yes"}}, - {Role: client.Slave, Config: map[string]string{"save": "", "slave-read-only": "no"}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{"save": ""}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "2000", nil), Role: client.Slave, Config: map[string]string{"save": "900 1 300 10", "slave-read-only": "yes"}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "3000", nil), Role: client.Slave, Config: map[string]string{"save": "", "slave-read-only": "no"}}, }}, wantErr: false, }, @@ -389,12 +399,19 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, )), options: []DiscoveryOption{SaveConfigDiscoveryOpt, SlaveReadOnlyDiscoveryOpt}, }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Unknown, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Unknown, Config: map[string]string{}}, }}, wantErr: true, }, @@ -419,11 +436,18 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, )), options: []DiscoveryOption{}, }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Unknown, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Unknown, Config: map[string]string{}}, }}, wantErr: true, }, @@ -454,11 +478,18 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, )), options: []DiscoveryOption{}, }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Unknown, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Unknown, Config: map[string]string{}}, }}, wantErr: true, }, @@ -483,6 +514,13 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves InjectResponse: func() interface{} { @@ -495,7 +533,7 @@ func TestShard_Discover(t *testing.T) { }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{}}, }}, wantErr: true, }, @@ -527,6 +565,13 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves InjectResponse: func() interface{} { @@ -552,9 +597,9 @@ func TestShard_Discover(t *testing.T) { }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{"save": ""}}, - {Role: client.Unknown, Config: map[string]string{}}, - {Role: client.Slave, Config: map[string]string{"save": "", "slave-read-only": "no"}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{"save": ""}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "2000", nil), Role: client.Unknown, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "3000", nil), Role: client.Slave, Config: map[string]string{"save": "", "slave-read-only": "no"}}, }}, wantErr: true, }, @@ -585,6 +630,13 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves InjectResponse: func() interface{} { @@ -610,9 +662,9 @@ func TestShard_Discover(t *testing.T) { }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{}}, - {Role: client.Slave, Config: map[string]string{}}, - {Role: client.Unknown, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "2000", nil), Role: client.Slave, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "3000", nil), Role: client.Unknown, Config: map[string]string{}}, }}, wantErr: true, }, @@ -646,6 +698,13 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves InjectResponse: func() interface{} { @@ -671,9 +730,9 @@ func TestShard_Discover(t *testing.T) { }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{"save": ""}}, - {Role: client.Unknown, Config: map[string]string{}}, - {Role: client.Slave, Config: map[string]string{"save": ""}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{"save": ""}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "2000", nil), Role: client.Unknown, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "3000", nil), Role: client.Slave, Config: map[string]string{"save": ""}}, }}, wantErr: true, }, @@ -700,12 +759,60 @@ func TestShard_Discover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, )), options: []DiscoveryOption{OnlyMasterDiscoveryOpt}, }, want: &Shard{Name: "test", Servers: []*RedisServer{ - {Role: client.Master, Config: map[string]string{}}, + {Server: redis.NewServerFromParams("", "127.0.0.1", "1000", nil), Role: client.Master, Config: map[string]string{}}, + }}, + wantErr: false, + }, + { + name: "Sentinel: failover in progress, refuse to discover slaves", + fields: fields{ + Name: "test", + Servers: []*RedisServer{}, + pool: redis.NewServerPool( + redis.NewFakeServerWithFakeClient("127.0.0.1", "1000", + client.NewPredefinedRedisFakeResponse("role-master", nil), + ), + redis.NewFakeServerWithFakeClient("127.0.0.1", "2000", + client.NewPredefinedRedisFakeResponse("role-master", nil), + ), + redis.NewFakeServerWithFakeClient("127.0.0.1", "3000"), + ), + }, + args: args{ + ctx: context.TODO(), + sentinel: NewSentinelServerFromParams(redis.NewFakeServerWithFakeClient("host", "port", + client.FakeResponse{ + // cmd: SentinelMaster + InjectResponse: func() interface{} { + return &client.SentinelMasterCmdResult{Name: "test", IP: "127.0.0.1", Port: 1000, Flags: "master"} + }, + InjectError: func() error { return nil }, + }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "2000"} + }, + InjectError: func() error { return nil }, + }, + )), + options: []DiscoveryOption{OnlyMasterDiscoveryOpt}, + }, + want: &Shard{Name: "test", + Servers: []*RedisServer{ + {Server: redis.NewServerFromParams("", "127.0.0.1", "2000", nil), Role: client.Master, Config: map[string]string{}}, }}, wantErr: false, }, @@ -743,67 +850,67 @@ func TestShard_Init(t *testing.T) { want []string wantErr bool }{ - // { - // name: "All redis servers configured", - // fields: fields{ - // Name: "test", - // Servers: []*RedisServer{ - // NewRedisServerFromParams( - // redis.NewFakeServerWithFakeClient("127.0.0.1", "1000", - // client.FakeResponse{ - // InjectResponse: func() interface{} { - // return []interface{}{"slave", "127.0.0.1"} - // }, - // InjectError: func() error { return nil }, - // }, - // client.FakeResponse{ - // InjectResponse: func() interface{} { return nil }, - // InjectError: func() error { return nil }, - // }, - // client.NewPredefinedRedisFakeResponse("role-master", nil), - // client.NewPredefinedRedisFakeResponse("role-master", nil), - // ), - // client.Unknown, - // map[string]string{}, - // ), - // NewRedisServerFromParams( - // redis.NewFakeServerWithFakeClient("127.0.0.1", "2000", - // client.FakeResponse{ - // InjectResponse: func() interface{} { - // return []interface{}{"slave", "127.0.0.1"} - // }, - // InjectError: func() error { return nil }, - // }, - // client.FakeResponse{ - // InjectResponse: func() interface{} { return nil }, - // InjectError: func() error { return nil }, - // }, - // ), - // client.Unknown, - // map[string]string{}, - // ), - // NewRedisServerFromParams( - // redis.NewFakeServerWithFakeClient("127.0.0.1", "3000", - // client.FakeResponse{ - // InjectResponse: func() interface{} { - // return []interface{}{"slave", "127.0.0.1"} - // }, - // InjectError: func() error { return nil }, - // }, - // client.FakeResponse{ - // InjectResponse: func() interface{} { return nil }, - // InjectError: func() error { return nil }, - // }, - // ), - // client.Unknown, - // map[string]string{}, - // ), - // }, - // }, - // args: args{ctx: context.TODO(), masterHostPort: "127.0.0.1:1000"}, - // want: []string{"127.0.0.1:1000", "127.0.0.1:2000", "127.0.0.1:3000"}, - // wantErr: false, - // }, + { + name: "All redis servers configured", + fields: fields{ + Name: "test", + Servers: []*RedisServer{ + NewRedisServerFromParams( + redis.NewFakeServerWithFakeClient("127.0.0.1", "1000", + client.FakeResponse{ + InjectResponse: func() interface{} { + return []interface{}{"slave", "127.0.0.1"} + }, + InjectError: func() error { return nil }, + }, + client.FakeResponse{ + InjectResponse: func() interface{} { return nil }, + InjectError: func() error { return nil }, + }, + client.NewPredefinedRedisFakeResponse("role-master", nil), + client.NewPredefinedRedisFakeResponse("role-master", nil), + ), + client.Unknown, + map[string]string{}, + ), + NewRedisServerFromParams( + redis.NewFakeServerWithFakeClient("127.0.0.1", "2000", + client.FakeResponse{ + InjectResponse: func() interface{} { + return []interface{}{"slave", "127.0.0.1"} + }, + InjectError: func() error { return nil }, + }, + client.FakeResponse{ + InjectResponse: func() interface{} { return nil }, + InjectError: func() error { return nil }, + }, + ), + client.Unknown, + map[string]string{}, + ), + NewRedisServerFromParams( + redis.NewFakeServerWithFakeClient("127.0.0.1", "3000", + client.FakeResponse{ + InjectResponse: func() interface{} { + return []interface{}{"slave", "127.0.0.1"} + }, + InjectError: func() error { return nil }, + }, + client.FakeResponse{ + InjectResponse: func() interface{} { return nil }, + InjectError: func() error { return nil }, + }, + ), + client.Unknown, + map[string]string{}, + ), + }, + }, + args: args{ctx: context.TODO(), masterHostPort: "127.0.0.1:1000"}, + want: []string{"127.0.0.1:1000", "127.0.0.1:2000", "127.0.0.1:3000"}, + wantErr: false, + }, { name: "No configuration needed", fields: fields{ diff --git a/pkg/redis/sharded/redis_sharded_cluster_test.go b/pkg/redis/sharded/redis_sharded_cluster_test.go index 6a97e930..df6ee268 100644 --- a/pkg/redis/sharded/redis_sharded_cluster_test.go +++ b/pkg/redis/sharded/redis_sharded_cluster_test.go @@ -456,6 +456,13 @@ func TestCluster_SentinelDiscover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard0) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves (shard0) InjectResponse: func() interface{} { @@ -483,6 +490,13 @@ func TestCluster_SentinelDiscover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard1) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "5000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves (shard1) InjectResponse: func() interface{} { @@ -548,6 +562,13 @@ func TestCluster_SentinelDiscover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard0) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves (shard0) InjectResponse: func() interface{} { @@ -575,6 +596,13 @@ func TestCluster_SentinelDiscover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard1) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "5000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves (shard1) InjectResponse: func() interface{} { @@ -674,6 +702,13 @@ func TestCluster_SentinelDiscover(t *testing.T) { }, InjectError: func() error { return nil }, }, + client.FakeResponse{ + // cmd: SentinelGetMasterAddrByName (shard0) + InjectResponse: func() interface{} { + return []string{"127.0.0.1", "1000"} + }, + InjectError: func() error { return nil }, + }, client.FakeResponse{ // cmd: SentinelSlaves (shard0) InjectResponse: func() interface{} {