Skip to content

fix: Admission Webhook blocks ScaledObject without metricType with fallback#6702

Merged
wozniakjan merged 7 commits intokedacore:mainfrom
rickbrouwer:issue-6696
Apr 15, 2025
Merged

fix: Admission Webhook blocks ScaledObject without metricType with fallback#6702
wozniakjan merged 7 commits intokedacore:mainfrom
rickbrouwer:issue-6696

Conversation

@rickbrouwer
Copy link
Member

@rickbrouwer rickbrouwer commented Apr 9, 2025

In scaler.go there is a function GetMetricTargetType which helps get the metric target type of the scaler. If no metric type is provided then the AverageValue is used.

In v2.17 is a check in the webhook admission that if at least one trigger is of the type AverageValue, then having fallback is valid. A metricType is optional and often not supplied. As a result, many scaledObjects now give errors when supplying a fallback without metricType.

Adding an extra check if the trigger.MetricType is empty will fix this issue. This check is also in the scaler.go.

Any feedback regarding the chosen solution is okay.

Checklist

Fixes #6696

…llback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
@rickbrouwer rickbrouwer requested a review from a team as a code owner April 9, 2025 11:34
Copy link
Member

@wozniakjan wozniakjan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

could you please also add an e2e test for this case so future KEDA releases are a bit safer? Don't feel pressured, if it's not in your capacity at the moment, I'm happy to approve this as is and add the e2e test coverage later this week.

@wozniakjan
Copy link
Member

wozniakjan commented Apr 9, 2025

/run-e2e fallback|scaling_modifiers
Update: You can check the progress here

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
@rickbrouwer
Copy link
Member Author

thank you!

could you please also add an e2e test for this case so future KEDA releases are a bit safer? Don't feel pressured, if it's not in your capacity at the moment, I'm happy to approve this as is and add the e2e test coverage later this week.

I'll add a unit test first. That also does a good check. I saw that scaledobject_types doesn't have this anyway, so here it is :)

I will create a test case in the e2e of fallback later.

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
@rickbrouwer
Copy link
Member Author

Also added an e2e test

@wozniakjan
Copy link
Member

wozniakjan commented Apr 9, 2025

/run-e2e fallback|scaling_modifiers
Update: You can check the progress here

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
@rickbrouwer
Copy link
Member Author

Added more unit tests for the remaining functions of scaledobject_types

@wozniakjan
Copy link
Member

wozniakjan commented Apr 10, 2025

/run-e2e fallback|scaling_modifiers
Update: You can check the progress here

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
@wozniakjan
Copy link
Member

wozniakjan commented Apr 10, 2025

/run-e2e fallback|scaling_modifiers
Update: You can check the progress here

Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@wozniakjan wozniakjan merged commit 476717e into kedacore:main Apr 15, 2025
21 checks passed
@rickbrouwer rickbrouwer deleted the issue-6696 branch April 15, 2025 13:54
mittalvaibhav1 pushed a commit to mittalvaibhav1/keda that referenced this pull request Apr 26, 2025
…llback (kedacore#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add unit test

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Add e2e test

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update changelog

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Signed-off-by: mittalvaibhav1 <mittalvaibhavandroid@gmail.com>
mittalvaibhav1 pushed a commit to mittalvaibhav1/keda that referenced this pull request Apr 26, 2025
…llback (kedacore#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add unit test

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Add e2e test

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update changelog

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
mittalvaibhav1 pushed a commit to mittalvaibhav1/keda that referenced this pull request Apr 26, 2025
…llback (kedacore#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add unit test

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Add e2e test

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update changelog

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
wozniakjan pushed a commit to wozniakjan/keda that referenced this pull request May 15, 2025
…llback (kedacore#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add unit test

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Add e2e test

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update changelog

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
wozniakjan pushed a commit to wozniakjan/keda that referenced this pull request May 15, 2025
…llback (kedacore#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add unit test

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Add e2e test

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update changelog

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
wozniakjan added a commit that referenced this pull request May 15, 2025
* fix: Admission Webhook blocks ScaledObject without metricType with fallback (#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add unit test

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Add e2e test

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update changelog

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* fix: AWS SQS Queue queueURLFromEnv not working (#6713)

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* fix: Temporal scaler with API Key (#6707)

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* fix: add default Operation in Azure Service Bus scaler (#6731)

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* fix: ScalerCache gets the lock before operate the scalers (#6739)

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* fix: Use pinned version for nginx image (#6737)

* fix: Use pinned version for nginx image

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

* .

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

* fix panic in gcp scaler

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>

---------

Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* Selenium Grid: Update metric name generated without part of empty (#6772)

* Selenium Grid: Update metric name generated without part of empty

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>

* Update CHANGELOG with the PR

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>

---------

Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

* chore: changelog and issue template v2.17.1

Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Jan Wozniak <wozniak.jan@gmail.com>
Signed-off-by: Jorge Turrado <jorge_turrado@hotmail.es>
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
Co-authored-by: rickbrouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Co-authored-by: Jorge Turrado Ferrero <Jorge_turrado@hotmail.es>
Co-authored-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
dpochopsky pushed a commit to dpochopsky/keda that referenced this pull request Sep 12, 2025
…llback (kedacore#6702)

* fix: Admission Webhook blocks ScaledObject without metricType with fallback

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add unit test

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Add e2e test

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>

* Add more unit tests for scaledobject_types

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update changelog

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

* Update

Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>

---------

Signed-off-by: rickbrouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>
Signed-off-by: David Pochopsky <david.pochopsky@united.com>
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.

Admission Webhook blocks ScaledObject with fallback (using AverageValue) 2.17

3 participants