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.

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

how redis container readiness scripts coordinate with redis auth #218

Closed
Jacklli opened this issue Jan 7, 2020 · 11 comments · Fixed by #235
Closed

how redis container readiness scripts coordinate with redis auth #218

Jacklli opened this issue Jan 7, 2020 · 11 comments · Fixed by #235

Comments

@Jacklli
Copy link

Jacklli commented Jan 7, 2020

Expected behaviour

What do you want to achieve?

Actual behaviour

What is happening? Are all the pieces created? Can you access to the service?

exception!!!
when set password for redis, the redis container readiness script aborted with 'unexpected' status!!!

Steps to reproduce the behaviour

Describe step by step what you've have done to get to this point

Environment

How are the pieces configured?

  • Redis Operator version
  • Kubernetes version
  • Kubernetes configuration used (eg: Is RBAC active?)

Logs

Please, add the debugging logs. In order to be able to gather them, add -debug flag when running the operator.

@Jacklli
Copy link
Author

Jacklli commented Jan 7, 2020

The rfr-redisfailover pods status are always 'READY 0/1'

image

image

@Jacklli
Copy link
Author

Jacklli commented Jan 7, 2020

there's no auth logic in keep alive script /redis-readiness/ready.sh

`ROLE="role"
ROLE_MASTER="role:master"
ROLE_SLAVE="role:slave"
IN_SYNC="master_sync_in_progress:1"
NO_MASTER="master_host:127.0.0.1"

check_master(){
exit 0
}

check_slave(){
in_sync=$(redis-cli info replication | grep $IN_SYNC | tr -d "\r" | tr -d "\n")
no_master=$(redis-cli info replication | grep $NO_MASTER | tr -d "\r" | tr -d "\n")

       if [ -z "$in_sync" ] && [ -z "$no_master" ]; then
               exit 0
       fi

       exit 1

}

role=$(redis-cli info replication | grep $ROLE | tr -d "\r" | tr -d "\n")

case $role in
$ROLE_MASTER)
check_master
;;
$ROLE_SLAVE)
check_slave
;;
*)
echo "unespected"
exit 1
esac
`

@Jacklli Jacklli changed the title how redis container scripts coordinate with redis auth how redis container readiness scripts coordinate with redis auth Jan 8, 2020
@Jacklli
Copy link
Author

Jacklli commented Jan 8, 2020

it's hard-coded in operator/redisfailover/service/generator.go

`func generateRedisReadinessConfigMap(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) *corev1.ConfigMap {
name := GetRedisReadinessName(rf)
namespace := rf.Namespace

    labels = util.MergeLabels(labels, generateSelectorLabels(redisRoleName, rf.Name))
    readinessContent := `ROLE="role"

ROLE_MASTER="role:master"
ROLE_SLAVE="role:slave"
IN_SYNC="master_sync_in_progress:1"
NO_MASTER="master_host:127.0.0.1"

check_master(){
exit 0
}
......
`

@Jacklli
Copy link
Author

Jacklli commented Jan 8, 2020

the change below may fix this problem.

operator/redisfailover/service/generator.go

func generateRedisReadinessConfigMap(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference, password string) *corev1.ConfigMap {
name := GetRedisReadinessName(rf)
namespace := rf.Namespace

    labels = util.MergeLabels(labels, generateSelectorLabels(redisRoleName, rf.Name))
    if string != "" {
            readinessContent := `ROLE="role"

ROLE_MASTER="role:master"
ROLE_SLAVE="role:slave"
IN_SYNC="master_sync_in_progress:1"
NO_MASTER="master_host:127.0.0.1"

check_master(){
exit 0
}

check_slave(){
in_sync=$(redis-cli -a + password + info replication | grep $IN_SYNC | tr -d "\r" | tr -d "\n")
no_master=$(redis-cli -a + password + info replication | grep $NO_MASTER | tr -d "\r" | tr -d "\n")

       if [ -z "$in_sync" ] && [ -z "$no_master" ]; then
               exit 0
       fi

       exit 1

}

role=$(redis-cli -a + password + info replication | grep $ROLE | tr -d "\r" | tr -d "\n")

case $role in
$ROLE_MASTER)
check_master
;;
$ROLE_SLAVE)
check_slave
;;
*)
echo "unespected"
exit 1
esac`

    } else {
            readinessContent := `ROLE="role"

ROLE_MASTER="role:master"
ROLE_SLAVE="role:slave"
IN_SYNC="master_sync_in_progress:1"
NO_MASTER="master_host:127.0.0.1"

check_master(){
exit 0
}

check_slave(){
in_sync=$(redis-cli info replication | grep $IN_SYNC | tr -d "\r" | tr -d "\n")
no_master=$(redis-cli info replication | grep $NO_MASTER | tr -d "\r" | tr -d "\n")

       if [ -z "$in_sync" ] && [ -z "$no_master" ]; then
               exit 0
       fi

       exit 1

}

role=$(redis-cli info replication | grep $ROLE | tr -d "\r" | tr -d "\n")

case $role in
$ROLE_MASTER)
check_master
;;
$ROLE_SLAVE)
check_slave
;;
*)
echo "unespected"
exit 1
esac`
}

operator/redisfailover/service/client.go

// EnsureRedisReadinessConfigMap makes sure the redis configmap with shutdown script exists
func (r *RedisFailoverKubeClient) EnsureRedisReadinessConfigMap(rf *redisfailoverv1.RedisFailover, labels map[string]string, ownerRefs []metav1.OwnerReference) error {
password, err := k8s.GetRedisPassword(r.K8SService, rf)
if err != nil {
return err
}

    cm := generateRedisReadinessConfigMap(rf, labels, ownerRefs, password)
    return r.K8SService.CreateOrUpdateConfigMap(rf.Namespace, cm)

}

@Jacklli
Copy link
Author

Jacklli commented Jan 10, 2020

@hoffoo hoffoo sir, could you please have a look at this problem?

@zedtux
Copy link

zedtux commented Feb 5, 2020

Thank you @Jacklli for your investigation, I'm facing the exact same issue as yours.

While @teamon opened a PR which fixed this issue, have you found any workaround?

@Jacklli
Copy link
Author

Jacklli commented Feb 5, 2020

i am just waiting for this pr pulled by @teamon, thanks a lot. i don't find any workaround yet.

@zedtux
Copy link

zedtux commented Feb 5, 2020

Okay, thank you for your quick response :)

@teamon
Copy link
Contributor

teamon commented Feb 5, 2020

You can use teamon/redis-operator:fix-readiness-auth image in the meantime.

@zedtux
Copy link

zedtux commented Feb 5, 2020

For both redis and sentinel, right?

@teamon
Copy link
Contributor

teamon commented Feb 5, 2020

For the operator itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants