From 4bea2da61c40688b55772f498ecef90ecd6da8ba Mon Sep 17 00:00:00 2001 From: fishu Date: Tue, 10 Dec 2019 17:48:57 +0800 Subject: [PATCH 1/8] feature: ignore slave whose slave-priority = 0 --- pkg/apis/redis/v1beta1/validate.go | 7 ++++++- pkg/client/redis/client.go | 23 ++++++++++++++++------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/pkg/apis/redis/v1beta1/validate.go b/pkg/apis/redis/v1beta1/validate.go index a340162..21d1a8a 100644 --- a/pkg/apis/redis/v1beta1/validate.go +++ b/pkg/apis/redis/v1beta1/validate.go @@ -14,6 +14,8 @@ const ( defaultRedisNumber = 3 defaultSentinelNumber = 3 defaultRedisImage = "redis:5.0.4-alpine" + + defaultSlavePriority = "1" ) var ( @@ -29,7 +31,7 @@ func (r *RedisCluster) Validate() error { if r.Spec.Size == 0 { r.Spec.Size = defaultRedisNumber } else if r.Spec.Size < defaultRedisNumber { - return errors.New("number of redises in spec is less than the minimum") + return errors.New("number of redis in spec is less than the minimum") } if r.Spec.Sentinel.Replicas == 0 { @@ -54,6 +56,9 @@ func (r *RedisCluster) Validate() error { r.Spec.Config = make(map[string]string) } + // https://github.com/ucloud/redis-operator/issues/6 + r.Spec.Config["slave-priority"] = defaultSlavePriority + if !r.Spec.DisablePersistence { enablePersistence(r.Spec.Config) } else { diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index f594826..23bc6b5 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "net" + "reflect" "regexp" "strconv" "strings" @@ -92,17 +93,25 @@ func (c *client) GetNumberSentinelSlavesInMemory(ip string, auth *util.AuthConfi return 0, err } - if err2 := isSentinelReady(info); err2 != nil { - return 0, err2 - } - match := slaveNumberRE.FindStringSubmatch(info) - if len(match) == 0 { - return 0, errors.New("slaves regex not found") + if err = isSentinelReady(info); err != nil { + return 0, err } - nSlaves, err := strconv.Atoi(match[1]) + + cmd := rediscli.NewSliceCmd("sentinel", "slaves", masterName) + rClient.Process(cmd) + slaveInfoBlobs, err := cmd.Result() if err != nil { return 0, err } + nSlaves := len(slaveInfoBlobs) + for _, slaveInfoBlob := range slaveInfoBlobs { + slaveInfo := reflect.ValueOf(slaveInfoBlob) + slavePriority := fmt.Sprintf("%+v", slaveInfo.Index(37)) + if slavePriority == "0" { + nSlaves -= 1 + } + } + return int32(nSlaves), nil } From af94a65ed7dd70c37a5d7e671d834f12ae102566 Mon Sep 17 00:00:00 2001 From: fishu Date: Tue, 10 Dec 2019 18:08:53 +0800 Subject: [PATCH 2/8] add slave num when slavePriority == 1 --- pkg/client/redis/client.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index 23bc6b5..56f2996 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -103,12 +103,12 @@ func (c *client) GetNumberSentinelSlavesInMemory(ip string, auth *util.AuthConfi if err != nil { return 0, err } - nSlaves := len(slaveInfoBlobs) + nSlaves := 0 for _, slaveInfoBlob := range slaveInfoBlobs { slaveInfo := reflect.ValueOf(slaveInfoBlob) slavePriority := fmt.Sprintf("%+v", slaveInfo.Index(37)) - if slavePriority == "0" { - nSlaves -= 1 + if slavePriority == "1" { + nSlaves += 1 } } From 93c01eae9b67a211c6ffbec1c967af3489aacacf Mon Sep 17 00:00:00 2001 From: fishu Date: Mon, 16 Dec 2019 17:10:43 +0800 Subject: [PATCH 3/8] fixed GetNumberSentinelSlavesInMemory --- pkg/client/redis/client.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index 56f2996..0eb0361 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -104,11 +104,18 @@ func (c *client) GetNumberSentinelSlavesInMemory(ip string, auth *util.AuthConfi return 0, err } nSlaves := 0 +OUTER: for _, slaveInfoBlob := range slaveInfoBlobs { slaveInfo := reflect.ValueOf(slaveInfoBlob) - slavePriority := fmt.Sprintf("%+v", slaveInfo.Index(37)) - if slavePriority == "1" { - nSlaves += 1 + for key, value := range slaveInfoBlob.([]interface{}) { + stringValue := value.(string) + if stringValue == "slave-priority" { + slavePriority := fmt.Sprintf("%+v", slaveInfo.Index(key+1)) + if slavePriority == "1" { + nSlaves += 1 + } + continue OUTER + } } } From 42679bced8668bb3569e294fdac791165cdafee3 Mon Sep 17 00:00:00 2001 From: fishu Date: Mon, 16 Dec 2019 18:30:35 +0800 Subject: [PATCH 4/8] fixed: GetNumberSentinelSlavesInMemory failed when the master node becomes a slave node --- pkg/client/redis/client.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index 0eb0361..0c9e4a7 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -109,12 +109,20 @@ OUTER: slaveInfo := reflect.ValueOf(slaveInfoBlob) for key, value := range slaveInfoBlob.([]interface{}) { stringValue := value.(string) + // When the master node becomes a slave node, runid will become empty + if stringValue == "runid" { + runID := fmt.Sprintf("%+v", slaveInfo.Index(key+1)) + if runID == "" { + nSlaves += 1 + continue OUTER + } + } if stringValue == "slave-priority" { slavePriority := fmt.Sprintf("%+v", slaveInfo.Index(key+1)) if slavePriority == "1" { nSlaves += 1 + continue OUTER } - continue OUTER } } } From 82af0a603c148d78eda001ce941c737b19134ead Mon Sep 17 00:00:00 2001 From: fishu Date: Tue, 17 Dec 2019 10:13:27 +0800 Subject: [PATCH 5/8] do not care about slave with slave-priority=0 --- pkg/client/redis/client.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index 0c9e4a7..00681db 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -103,26 +103,18 @@ func (c *client) GetNumberSentinelSlavesInMemory(ip string, auth *util.AuthConfi if err != nil { return 0, err } - nSlaves := 0 + nSlaves := len(slaveInfoBlobs) OUTER: for _, slaveInfoBlob := range slaveInfoBlobs { slaveInfo := reflect.ValueOf(slaveInfoBlob) for key, value := range slaveInfoBlob.([]interface{}) { stringValue := value.(string) - // When the master node becomes a slave node, runid will become empty - if stringValue == "runid" { - runID := fmt.Sprintf("%+v", slaveInfo.Index(key+1)) - if runID == "" { - nSlaves += 1 - continue OUTER - } - } if stringValue == "slave-priority" { slavePriority := fmt.Sprintf("%+v", slaveInfo.Index(key+1)) - if slavePriority == "1" { - nSlaves += 1 - continue OUTER + if slavePriority == "0" { + nSlaves -= 1 } + continue OUTER } } } From 6ab4a8d1f89f938b5ac5f662604da0481d0833d4 Mon Sep 17 00:00:00 2001 From: fishu Date: Tue, 17 Dec 2019 10:49:30 +0800 Subject: [PATCH 6/8] remove nest for loop when GetNumberSentinelSlavesInMemory --- pkg/client/redis/client.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index 00681db..fd2d1f3 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net" - "reflect" "regexp" "strconv" "strings" @@ -104,24 +103,27 @@ func (c *client) GetNumberSentinelSlavesInMemory(ip string, auth *util.AuthConfi return 0, err } nSlaves := len(slaveInfoBlobs) -OUTER: for _, slaveInfoBlob := range slaveInfoBlobs { - slaveInfo := reflect.ValueOf(slaveInfoBlob) - for key, value := range slaveInfoBlob.([]interface{}) { - stringValue := value.(string) - if stringValue == "slave-priority" { - slavePriority := fmt.Sprintf("%+v", slaveInfo.Index(key+1)) - if slavePriority == "0" { - nSlaves -= 1 - } - continue OUTER - } + slavePriority := slaveInfoFieldByName("slave-priority", slaveInfoBlob) + if slavePriority == "0" { + nSlaves -= 1 } } return int32(nSlaves), nil } +func slaveInfoFieldByName(name string, slaveInfoBlob interface{}) string { + slaveInfo := slaveInfoBlob.([]interface{}) + for key, value := range slaveInfo { + stringValue := value.(string) + if stringValue == name { + return slaveInfo[key+1].(string) + } + } + return "" +} + func isSentinelReady(info string) error { matchStatus := sentinelStatusRE.FindStringSubmatch(info) if len(matchStatus) == 0 || matchStatus[1] != "ok" { From 5269ebf823b97787965475c5d59583eb6fe88666 Mon Sep 17 00:00:00 2001 From: fishu Date: Tue, 17 Dec 2019 11:17:27 +0800 Subject: [PATCH 7/8] fixd func slaveInfoFieldByName when slaveInfo have same key and value --- pkg/client/redis/client.go | 9 ++++--- pkg/client/redis/client_test.go | 46 ++++++++++++++++++++++++++++++--- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index fd2d1f3..0ee8655 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -115,11 +115,14 @@ func (c *client) GetNumberSentinelSlavesInMemory(ip string, auth *util.AuthConfi func slaveInfoFieldByName(name string, slaveInfoBlob interface{}) string { slaveInfo := slaveInfoBlob.([]interface{}) - for key, value := range slaveInfo { - stringValue := value.(string) + infoLens := len(slaveInfo) + i := 0 + for i < infoLens { + stringValue := slaveInfo[i].(string) if stringValue == name { - return slaveInfo[key+1].(string) + return slaveInfo[i+1].(string) } + i += 2 } return "" } diff --git a/pkg/client/redis/client_test.go b/pkg/client/redis/client_test.go index 864b2ec..915afc6 100644 --- a/pkg/client/redis/client_test.go +++ b/pkg/client/redis/client_test.go @@ -1,10 +1,10 @@ -package redis_test +package redis import ( - rediscli "github.com/go-redis/redis" - "github.com/ucloud/redis-operator/pkg/client/redis" "strings" "testing" + + rediscli "github.com/go-redis/redis" ) func newClient() *rediscli.Client { @@ -18,7 +18,7 @@ func newClient() *rediscli.Client { func TestGetAllRedisConfig(t *testing.T) { cli := newClient() //var client redis.Client - client := redis.New() + client := New() result, err := client.GetAllRedisConfig(cli) if err != nil { t.Fatal(err) @@ -33,3 +33,41 @@ func TestGetAllRedisConfig(t *testing.T) { } } + +func Test_slaveInfoFieldByName(t *testing.T) { + slaveInfoBlobA := []interface{}{"name", "[xxxxA]:6379", "ip", "xxxxA", "port", "6379", "runid", "6f792839ab551e8dbec58e0eb3b3838d14f19a37", "flags", "slave", "link-pending-commands", "1", "link-refcount", "1", "last-ping-sent", "0", "last-ok-ping-reply", "1055", "last-ping-reply", "1055", "down-after-milliseconds", "5000", "info-refresh", "2074", "role-reported", "slave", "role-reported-time", "2983115", "master-link-down-time", "0", "master-link-status", "ok", "master-host", "xxxxA", "master-port", "6379", "slave-priority", "1", "slave-repl-offset", "124614695"} + slaveInfoBlobB := []interface{}{"name", "[xxxxB]:6371", "ip", "xxxxB", "port", "6371", "runid", "fake_slave_8bb90711-8f37-44e8-b3b2-589af", "flags", "slave", "link-pending-commands", "1", "link-refcount", "1", "last-ping-sent", "0", "last-ok-ping-reply", "1055", "last-ping-reply", "1055", "down-after-milliseconds", "5000", "info-refresh", "2075", "role-reported", "slave", "role-reported-time", "2983114", "master-link-down-time", "0", "master-link-status", "ok", "master-host", "xxxxB", "master-port", "6379", "slave-priority", "0", "slave-repl-offset", "124614695"} + type args struct { + name string + slaveInfoBlob interface{} + } + tests := []struct { + name string + args args + want string + }{ + { + name: "slaveA", + args: args{ + name: "slave-priority", + slaveInfoBlob: slaveInfoBlobA, + }, + want: "1", + }, + { + name: "slaveB", + args: args{ + name: "slave-priority", + slaveInfoBlob: slaveInfoBlobB, + }, + want: "0", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := slaveInfoFieldByName(tt.args.name, tt.args.slaveInfoBlob); got != tt.want { + t.Errorf("slaveInfoFieldByName() = %v, want %v", got, tt.want) + } + }) + } +} From 1bd62d6b56677e68afa3cab859089fc405c3741d Mon Sep 17 00:00:00 2001 From: fishu Date: Tue, 17 Dec 2019 11:42:54 +0800 Subject: [PATCH 8/8] fixed func slaveInfoFieldByName range panic --- pkg/client/redis/client.go | 2 +- pkg/client/redis/client_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/client/redis/client.go b/pkg/client/redis/client.go index 0ee8655..ee5882d 100644 --- a/pkg/client/redis/client.go +++ b/pkg/client/redis/client.go @@ -117,7 +117,7 @@ func slaveInfoFieldByName(name string, slaveInfoBlob interface{}) string { slaveInfo := slaveInfoBlob.([]interface{}) infoLens := len(slaveInfo) i := 0 - for i < infoLens { + for i+1 < infoLens { stringValue := slaveInfo[i].(string) if stringValue == name { return slaveInfo[i+1].(string) diff --git a/pkg/client/redis/client_test.go b/pkg/client/redis/client_test.go index 915afc6..dcb6694 100644 --- a/pkg/client/redis/client_test.go +++ b/pkg/client/redis/client_test.go @@ -37,6 +37,8 @@ func TestGetAllRedisConfig(t *testing.T) { func Test_slaveInfoFieldByName(t *testing.T) { slaveInfoBlobA := []interface{}{"name", "[xxxxA]:6379", "ip", "xxxxA", "port", "6379", "runid", "6f792839ab551e8dbec58e0eb3b3838d14f19a37", "flags", "slave", "link-pending-commands", "1", "link-refcount", "1", "last-ping-sent", "0", "last-ok-ping-reply", "1055", "last-ping-reply", "1055", "down-after-milliseconds", "5000", "info-refresh", "2074", "role-reported", "slave", "role-reported-time", "2983115", "master-link-down-time", "0", "master-link-status", "ok", "master-host", "xxxxA", "master-port", "6379", "slave-priority", "1", "slave-repl-offset", "124614695"} slaveInfoBlobB := []interface{}{"name", "[xxxxB]:6371", "ip", "xxxxB", "port", "6371", "runid", "fake_slave_8bb90711-8f37-44e8-b3b2-589af", "flags", "slave", "link-pending-commands", "1", "link-refcount", "1", "last-ping-sent", "0", "last-ok-ping-reply", "1055", "last-ping-reply", "1055", "down-after-milliseconds", "5000", "info-refresh", "2075", "role-reported", "slave", "role-reported-time", "2983114", "master-link-down-time", "0", "master-link-status", "ok", "master-host", "xxxxB", "master-port", "6379", "slave-priority", "0", "slave-repl-offset", "124614695"} + slaveInfoBlobC := []interface{}{"name", "[xxxxB]:6371", "ip", "slave-priority", "slave-priority", "100"} + slaveInfoBlobD := []interface{}{"name", "[xxxxB]:6371", "ip", "xxxxB", "slave-priority"} type args struct { name string slaveInfoBlob interface{} @@ -62,6 +64,22 @@ func Test_slaveInfoFieldByName(t *testing.T) { }, want: "0", }, + { + name: "slaveC", + args: args{ + name: "slave-priority", + slaveInfoBlob: slaveInfoBlobC, + }, + want: "100", + }, + { + name: "slaveD", + args: args{ + name: "slave-priority", + slaveInfoBlob: slaveInfoBlobD, + }, + want: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {