Skip to content

Commit

Permalink
more deep slaves checking when updated
Browse files Browse the repository at this point in the history
  • Loading branch information
Chus committed Nov 26, 2019
1 parent 0a22fed commit 694251f
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 54 deletions.
2 changes: 1 addition & 1 deletion Gopkg.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion charts/redisoperator/Chart.yaml
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
apiVersion: v1
description: A Helm chart for the Spotahome Redis Operator
name: redisoperator
version: 3.1.0
version: 3.0.0
4 changes: 2 additions & 2 deletions mocks/operator/redisfailover/service/RedisFailoverCheck.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions mocks/service/redis/Client.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 10 additions & 7 deletions operator/redisfailover/checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@ func (r *RedisFailoverHandler) UpdateRedisesPods(rf *redisfailoverv1.RedisFailov
return err
}

// No perform updates when nodes are syncing.
masterIP, err := r.rfChecker.GetMasterIP(rf)
// No perform updates when nodes are syncing, still not connected, etc.
for _, rp := range redises {
sync, err := r.rfChecker.CheckRedisSyncing(rp, rf)
if err != nil {
return err
}
if sync {
return nil
if rp != masterIP {
ready, err := r.rfChecker.CheckRedisSlavesReady(rp, rf)
if err != nil {
return err
}
if !ready {
return nil
}
}
}

Expand Down
70 changes: 36 additions & 34 deletions operator/redisfailover/checker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,14 @@ func TestCheckAndHeal(t *testing.T) {
expErr = true
}
if !expErr && continueTests {
mrfc.On("GetMasterIP", rf).Once().Return(master, nil)
mrfc.On("GetMasterIP", rf).Twice().Return(master, nil)
if test.slavesOK {
mrfc.On("CheckAllSlavesFromMaster", master, rf).Once().Return(nil)
} else {
mrfc.On("CheckAllSlavesFromMaster", master, rf).Once().Return(errors.New(""))
mrfh.On("SetMasterOnAll", master, rf).Once().Return(nil)
}
mrfc.On("GetRedisesIPs", rf).Twice().Return([]string{master}, nil)
mrfc.On("CheckRedisSyncing", master, rf).Once().Return(false, nil)
mrfc.On("GetStatefulSetUpdateRevision", rf).Once().Return("1", nil)
mrfc.On("GetRedisesSlavesPods", rf).Once().Return([]string{}, nil)
mrfc.On("GetRedisesMasterPod", rf).Once().Return(master, nil)
Expand Down Expand Up @@ -214,16 +213,15 @@ func TestCheckAndHeal(t *testing.T) {

func TestUpdate(t *testing.T) {
type podStatus struct {
pod corev1.Pod
syncing bool
master bool
pod corev1.Pod
ready bool
master bool
}
tests := []struct {
name string
pods []podStatus
ssVersion string
errExpected error
syncing bool
}{
{
name: "all ok, no change needed",
Expand All @@ -240,8 +238,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.0",
},
},
master: false,
syncing: false,
master: false,
ready: true,
},
{
pod: corev1.Pod{
Expand All @@ -255,8 +253,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.1",
},
},
master: false,
syncing: false,
master: false,
ready: true,
},
{
pod: corev1.Pod{
Expand All @@ -270,8 +268,8 @@ func TestUpdate(t *testing.T) {
PodIP: "1.1.1.1",
},
},
master: true,
syncing: false,
master: true,
ready: true,
},
},
ssVersion: "10",
Expand All @@ -292,8 +290,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.0",
},
},
master: false,
syncing: false,
master: false,
ready: true,
},
{
pod: corev1.Pod{
Expand All @@ -307,8 +305,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.1",
},
},
master: false,
syncing: true,
master: false,
ready: false,
},
{
pod: corev1.Pod{
Expand All @@ -322,8 +320,8 @@ func TestUpdate(t *testing.T) {
PodIP: "1.1.1.1",
},
},
master: true,
syncing: false,
master: true,
ready: true,
},
},
ssVersion: "10",
Expand All @@ -344,8 +342,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.0",
},
},
master: false,
syncing: false,
master: false,
ready: true,
},
{
pod: corev1.Pod{
Expand All @@ -359,8 +357,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.1",
},
},
master: false,
syncing: true,
master: false,
ready: true,
},
{
pod: corev1.Pod{
Expand All @@ -374,8 +372,8 @@ func TestUpdate(t *testing.T) {
PodIP: "1.1.1.1",
},
},
master: true,
syncing: false,
master: true,
ready: true,
},
},
ssVersion: "1",
Expand All @@ -396,8 +394,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.0",
},
},
master: false,
syncing: false,
master: false,
ready: true,
},
{
pod: corev1.Pod{
Expand All @@ -411,8 +409,8 @@ func TestUpdate(t *testing.T) {
PodIP: "0.0.0.1",
},
},
master: false,
syncing: true,
master: false,
ready: true,
},
{
pod: corev1.Pod{
Expand All @@ -426,8 +424,8 @@ func TestUpdate(t *testing.T) {
PodIP: "1.1.1.1",
},
},
master: true,
syncing: false,
master: true,
ready: true,
},
},
ssVersion: "10",
Expand All @@ -445,12 +443,15 @@ func TestUpdate(t *testing.T) {

mrfc := &mRFService.RedisFailoverCheck{}
mrfc.On("GetRedisesIPs", rf).Once().Return([]string{"0.0.0.0", "0.0.0.1", "1.1.1.1"}, nil)
mrfc.On("GetMasterIP", rf).Once().Return("1.1.1.1", nil)

next := true

for _, pod := range test.pods {
mrfc.On("CheckRedisSyncing", pod.pod.Status.PodIP, rf).Once().Return(pod.syncing, nil)
if pod.syncing {

if !pod.master {
mrfc.On("CheckRedisSlavesReady", pod.pod.Status.PodIP, rf).Once().Return(pod.ready, nil)
}
if !pod.ready {
next = false
break
}
Expand All @@ -464,7 +465,7 @@ func TestUpdate(t *testing.T) {
for _, pod := range test.pods {
mrfc.On("GetRedisRevisionHash", pod.pod.ObjectMeta.Name, rf).Once().Return(pod.pod.ObjectMeta.Labels[appsv1.ControllerRevisionHashLabelKey], nil)
if pod.pod.ObjectMeta.Labels[appsv1.ControllerRevisionHashLabelKey] != test.ssVersion {
mrfh.On("DeletePod", mock.Anything, mock.Anything)
mrfh.On("DeletePod", pod.pod.ObjectMeta.Name, rf).Once().Return(nil)
if pod.master == false {
next = false
break
Expand All @@ -473,6 +474,7 @@ func TestUpdate(t *testing.T) {
}
if next {
mrfc.On("GetRedisesMasterPod", rf).Once().Return("master", nil)

}
}

Expand Down
8 changes: 4 additions & 4 deletions operator/redisfailover/service/check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type RedisFailoverCheck interface {
GetRedisesMasterPod(rFailover *redisfailoverv1.RedisFailover) (string, error)
GetStatefulSetUpdateRevision(rFailover *redisfailoverv1.RedisFailover) (string, error)
GetRedisRevisionHash(podName string, rFailover *redisfailoverv1.RedisFailover) (string, error)
CheckRedisSyncing(slaveIP string, rFailover *redisfailoverv1.RedisFailover) (bool, error)
CheckRedisSlavesReady(slaveIP string, rFailover *redisfailoverv1.RedisFailover) (bool, error)
}

// RedisFailoverChecker is our implementation of RedisFailoverCheck interface
Expand Down Expand Up @@ -325,12 +325,12 @@ func (r *RedisFailoverChecker) GetRedisRevisionHash(podName string, rFailover *r
return val, nil
}

// CheckRedisSyncing returns true if the slave is still syncing
func (r *RedisFailoverChecker) CheckRedisSyncing(ip string, rFailover *redisfailoverv1.RedisFailover) (bool, error) {
// CheckRedisSlavesReady returns true if the slave is ready (sync, connected, etc)
func (r *RedisFailoverChecker) CheckRedisSlavesReady(ip string, rFailover *redisfailoverv1.RedisFailover) (bool, error) {
password, err := k8s.GetRedisPassword(r.k8sService, rFailover)
if err != nil {
return false, err
}

return r.redisClient.IsSyncing(ip, password)
return r.redisClient.SlaveIsReady(ip, password)
}
13 changes: 10 additions & 3 deletions service/redis/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Client interface {
GetSentinelMonitor(ip string) (string, error)
SetCustomSentinelConfig(ip string, configs []string) error
SetCustomRedisConfig(ip string, configs []string, password string) error
IsSyncing(ip, password string) (bool, error)
SlaveIsReady(ip, password string) (bool, error)
}

type client struct {
Expand All @@ -41,6 +41,8 @@ const (
redisMasterHostREString = "master_host:([0-9.]+)"
redisRoleMaster = "role:master"
redisSyncing = "master_sync_in_progress:1"
redisMasterSillPending = "master_host:127.0.0.1"
redisLinkUp = "master_link_status:up"
redisPort = "6379"
sentinelPort = "26379"
masterName = "mymaster"
Expand Down Expand Up @@ -304,7 +306,7 @@ func (c *client) getConfigParameters(config string) (parameter string, value str
return s[0], strings.Join(s[1:], " "), nil
}

func (c *client) IsSyncing(ip, password string) (bool, error) {
func (c *client) SlaveIsReady(ip, password string) (bool, error) {
options := &rediscli.Options{
Addr: fmt.Sprintf("%s:%s", ip, redisPort),
Password: password,
Expand All @@ -316,5 +318,10 @@ func (c *client) IsSyncing(ip, password string) (bool, error) {
if err != nil {
return false, err
}
return strings.Contains(info, redisSyncing), nil

ok := !strings.Contains(info, redisSyncing) &&
!strings.Contains(info, redisMasterSillPending) &&
strings.Contains(info, redisLinkUp)

return ok, nil
}

0 comments on commit 694251f

Please sign in to comment.