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

Add redis ssl #540

Merged
merged 4 commits into from
Jan 28, 2020
Merged

Add redis ssl #540

merged 4 commits into from
Jan 28, 2020

Conversation

inuyasha82
Copy link
Contributor

@inuyasha82 inuyasha82 commented Jan 15, 2020

Hi guys,

i'm working on adding the SSL connection for redis (see issue #240).
So far i managed to get the redis-scaler to connect to a redis server using ssl.

But i need some help to finish this work.
What is happening at the moment is that the scaler is correctly checking the queue, getting the length, scaling down to 0, and eventually create a pod when. the watched list is being populated, the problem is that no matter how many items are in the queue, it always create only one pod (so it's not scaling up)

The interesting thing is that apparently it is happening only with the ssl queue. When using my code on a redis server without ssl enabled, it is correctly scaling up and down.

What could be the reason?

Do i need to change the code somewhere else?

As you can see in this pull request i'm printing out some debug InformatIon, i checked that the list length reported is correct.

with this change if the user add in the redis-scaler configuration the
following field:

```
    enableTLS: true
```

it will let the redis-scaler to connect to SSL hosts (it will add the
tls configuration to the client connection options, setting
InsecureSkipVerify to true)
@msftclas
Copy link

msftclas commented Jan 15, 2020

CLA assistant check
All CLA requirements met.

@inuyasha82 inuyasha82 changed the title wip: Add redis ssl Add redis ssl Jan 24, 2020
@inuyasha82
Copy link
Contributor Author

So found the reason of why it was not scaling to more than one, is explained on issue #240, shortly i was deploying only the custom keda image, and not the custom metricserver, once deployed both custom keda and metricserver it is correctly scaling up and down.

@marchmallow
Copy link
Contributor

@ahmelsayed does this look good to go in or want us to make further changes? We depend upon using TLS to deploy keda in our production environment

@ahmelsayed
Copy link
Contributor

Sorry for the delay in getting back, sorry for not helping with the issue you're seeing, but looks like you're able to figure out :)

The change looks good to me. I'll merge it in a minute.

@ahmelsayed ahmelsayed merged commit 6876da4 into kedacore:master Jan 28, 2020
@tomkerkhove
Copy link
Member

@inuyasha82 Would you mind sending a PR to update the docs for this please?

https://github.com/kedacore/keda-docs

@inuyasha82
Copy link
Contributor Author

@tomkerkhove sure, i'll do it today!

@tomkerkhove
Copy link
Member

Awesome, thanks!

@inuyasha82
Copy link
Contributor Author

@tomkerkhove created: kedacore/keda-docs#73

@marchmallow marchmallow deleted the add_redis_ssl branch March 30, 2020 19:54
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 this pull request may close these issues.

6 participants