Support paused-replicas being false
Adds support for the annotation autoscaling.keda.sh/paused-replicas: 'false'
This is particularly useful Argo users where you want to explicitly include the annotation in your git manifest, so that Argo can track it as a managed field and you can see the diff if someone manually modifies it (we do this a lot!).
Note that '' would have also been an option but that felt a little less explicit and potentially a breaking change (although I think if empty string is used the object fails anyways). As long as something can be set, it solves the underlying issue.
Checklist
- [ ] When introducing a new scaler, I agree with the scaling governance policy
- [ ] I have verified that my change is according to the deprecations & breaking changes policy
- [ ] Tests have been added
- [ ] Changelog has been updated and is aligned with our changelog requirements
- [ ] A PR is opened to update our Helm chart (repo) (if applicable, ie. when deployment manifests are modified)
- [ ] A PR is opened to update the documentation on (repo) (if applicable)
- [ ] Commits are signed with Developer Certificate of Origin (DCO - learn more)
Fixes https://github.com/kedacore/keda/issues/7263
Thank you for your contribution! 🙏
Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.
While you are waiting, make sure to:
- Add an entry in our changelog in alphabetical order and link related issue
- Update the documentation, if needed
- Add unit & e2e tests for your changes
- GitHub checks are passing
- Is the DCO check failing? Here is how you can fix DCO issues
Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.
Learn more about our contribution guide.
:white_check_mark: Snyk checks have passed. No issues have been found so far.
| Status | Scanner | Total (0) | ||||
|---|---|---|---|---|---|---|
| :white_check_mark: | Open Source Security | 0 | 0 | 0 | 0 | 0 issues |
:computer: Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
I understand what you're saying, but I personally don't support the word (boolean) false for paused replicas, because that's where the number of replicas expected is.
By allowing false, it might seem like you're also allowing true, and what's the action in that case?
If you have an incident, what value do you use for paused-replicas? Is it always 0? Or can it be a different value?
I understand what you're saying, but I personally don't support the word (boolean)
falsefor paused replicas, because that's where the number of replicas expected is. By allowingfalse, it might seem like you're also allowingtrue, and what's the action in that case?If you have an incident, what value do you use for paused-replicas? Is it always
0? Or can it be a different value?
Yeah I indicated in the issue that false might be better to be explicit versus empty string for example ''. The point is it has to be something that isn't just missing otherwise Argo can't detect drift. I have zero opinion on that so happy for you to just say "make it this" and I will do it :D
Our extension sets the value if we pause to whatever the user decides. Then unsetting just involves removing the value alltogether. The issue then is that you can end up in a state where you dont see that the app is out of sync due to what I described in the description.
The documentation states the following:
Typically, either one or the other is being used given they serve a different purpose/scenario. However, if both paused and paused-replicas are set, KEDA will scale your current workload to the number specified count in paused-replicas and then pause autoscaling.
I almost think we should reverse this. That way, you can keep setting the paused-replicas, but with the additional annotation, indicate whether it should actually be paused or not. If someone changes that and also the paused itself, I think you'll get what you want.
However, this would be a kind of breaking change in behavior for people who both specify that pause should be false, but still want to pause via the paused-replicas.
I'm not sure why that choice was made, but there's probably a good reason :)
For now, if I had to choose a solution, I think I'd go with "". But I'm not making the decision alone; I'm curious to hear the @kedacore/keda-contributors opinions.
I almost think we should reverse this. That way, you can keep setting the paused-replicas, but with the additional annotation, indicate whether it should actually be paused or not. If someone changes that and also the paused itself, I think you'll get what you want.
I would fully agree with this. We actually built our tool at first assuming it worked the way you are proposing - one annotation determines the fixed value, the other determines if that fixed value should be used or not. This would be ideal and would solve it all but as you say would be a breaking change.
As I say, happy to make the change to support empty string instead if that is preferred. Will wait for some feedback.
Yeah, I also prefer empty value over false here
Could you also create a PR in keda-docs so we can have this documented? 🙂
:( this DCO stuff is killing me 😅 - edit - Claude to the rescue :D
Could you also create a PR in keda-docs so we can have this documented? 🙂
@rickbrouwer Not super clear to me where that should be done? Like for an unreleased version as the docs all seem to be version coded. Can you link me to the page I should be editing?
Sure!
https://github.com/kedacore/keda-docs/blob/main/content/docs/2.19/concepts/scaling-deployments.md
/run-e2e pause Update: You can check the progress here
Sure!
https://github.com/kedacore/keda-docs/blob/main/content/docs/2.19/concepts/scaling-deployments.md
@rickbrouwer so i should modify the docs for the existing version? That feels off.
Sure! https://github.com/kedacore/keda-docs/blob/main/content/docs/2.19/concepts/scaling-deployments.md
@rickbrouwer so i should modify the docs for the existing version? That feels off.
We always have the new unreleased version in the docs, so 2.19. It's still to be released, so that's the version you can edit. Feel free to create a PR there and mention me, and I'll check if it went well and give you some tips and feedback if needed.