refactor: remove unused scaledObjectMetricSpecs variable#7292
Merged
JorTurFer merged 2 commits intoDec 8, 2025
Merged
Conversation
Signed-off-by: u-kai <[email protected]>
|
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:
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. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Signed-off-by: u-kai <[email protected]>
u-kai
marked this pull request as ready for review
December 3, 2025 22:47
Member
|
/run-e2e internal |
rickbrouwer
approved these changes
Dec 8, 2025
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the getScaledObjectMetricSpecs function in the HPA controller by removing an unnecessary intermediate variable scaledObjectMetricSpecs that was redundantly storing and manipulating the same data as metricSpecs. The refactoring simplifies the code by directly using metricSpecs throughout the function.
Key changes:
- Eliminated the redundant
scaledObjectMetricSpecsvariable and all operations on it - Updated all references to work directly with the
metricSpecsvariable returned from the scaler cache - Added a CHANGELOG entry documenting the change
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| controllers/keda/hpa.go | Removed unused scaledObjectMetricSpecs variable and refactored to use metricSpecs directly throughout the function |
| CHANGELOG.md | Added entry documenting the removal of the unnecessary variable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -270,7 +270,6 @@ func (r *ScaledObjectReconciler) deleteHPA(ctx context.Context, logger logr.Logg | |||
|
|
|||
| // getScaledObjectMetricSpecs returns MetricSpec for HPA, generater from Triggers defitinion in ScaledObject | |||
There was a problem hiding this comment.
Typo in comment: "generater" should be "generated" and "defitinion" should be "definition".
Suggested change
| // getScaledObjectMetricSpecs returns MetricSpec for HPA, generater from Triggers defitinion in ScaledObject | |
| // getScaledObjectMetricSpecs returns MetricSpec for HPA, generated from Triggers definition in ScaledObject |
|
|
||
| - **General**: Fix nil reference panic when transfer-hpa-ownership is set but no hpa name is provided ([#7254](https://github.com/kedacore/keda/issues/7254)) | ||
| - **General**: Fix race condition in paused-replicas annotation causing ScaledObject to get stuck ([#7231](https://github.com/kedacore/keda/issues/7231)) | ||
| - **General**: Remove unnecessary scaledObjectMetricSpecs variable in HPA ([#7292](https://github.com/kedacore/keda/pull/7292)) |
There was a problem hiding this comment.
The CHANGELOG entry categorizes this as a fix, but this is a refactoring/code cleanup that removes an unnecessary variable. This should be in the "Improvements" section rather than under fixes, as it doesn't fix a bug or incorrect behavior.
JorTurFer
pushed a commit
to JorTurFer/keda
that referenced
this pull request
Dec 8, 2025
* refactor: remove unused scaledObjectMetricSpecs variable Signed-off-by: u-kai <[email protected]> * update CHANGELOG.md Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]>
JorTurFer
pushed a commit
to JorTurFer/keda
that referenced
this pull request
Dec 8, 2025
* refactor: remove unused scaledObjectMetricSpecs variable Signed-off-by: u-kai <[email protected]> * update CHANGELOG.md Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]>
JorTurFer
pushed a commit
to JorTurFer/keda
that referenced
this pull request
Dec 8, 2025
* refactor: remove unused scaledObjectMetricSpecs variable Signed-off-by: u-kai <[email protected]> * update CHANGELOG.md Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]> Signed-off-by: Jorge Turrado <[email protected]>
JorTurFer
added a commit
that referenced
this pull request
Dec 8, 2025
* fix: Correct parse error ActiveMQ (#7245) Signed-off-by: Rick Brouwer <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * fix: metricUnavailableValue parameter not working in Datadog scaler (#7241) * fix: metricUnavailableValue parameter not working in Datadog scaler The UseFiller flag was not being set correctly when metricUnavailableValue was configured. This fix distinguishes between 'not configured' and 'explicitly set to 0' by checking TriggerMetadata directly. Changes: - Set UseFiller in validateAPIMetadata() when metricUnavailableValue exists - Set UseFiller in validateClusterAgentMetadata() when metricUnavailableValue exists - Remove UseFiller logic from Validate() (responsibility moved to validate functions) - Update tests to verify UseFiller behavior with various values including 0 This allows users to explicitly set metricUnavailableValue to 0 and have it work as a fallback value, while still erroring when not configured. Fixes #7238 Signed-off-by: Hiroki Matsui <[email protected]> * test: cover both API and ClusterAgent modes in UseFiller test Updated TestDatadogMetadataValidateUseFiller to test both validateAPIMetadata() and validateClusterAgentMetadata() code paths. This ensures that the UseFiller flag is correctly set in both integration modes. Test cases now cover: - API mode: 5 test cases (not configured, 0, positive, negative, decimal) - Cluster Agent mode: 5 test cases (same variations) Signed-off-by: Hiroki Matsui <[email protected]> * refactor: use pointer type for FillValue to avoid TriggerMetadata access Changed FillValue from float64 to *float64 to distinguish between 'not configured' (nil) and 'explicitly set to any value including 0'. This addresses reviewer feedback about avoiding direct TriggerMetadata access and improves type safety and refactoring resistance. Changes: - FillValue type changed from float64 to *float64 with optional tag - validateAPIMetadata checks nil instead of TriggerMetadata map - validateClusterAgentMetadata checks nil instead of TriggerMetadata map - Dereference FillValue when returning fallback value (2 locations) - Update tests to handle pointer type with proper nil checks Signed-off-by: Hiroki Matsui <[email protected]> --------- Signed-off-by: Hiroki Matsui <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * Fix ScaledObject pause behavior when HPA doesn't exist (#7233) When a ScaledObject has the paused annotation set before the HPA is created, the controller would fall through and create the HPA, ignoring the pause annotation. The fix writes the paused status to etcd immediately before stopping the scale loop or deleting the HPA. This prevents race conditions where concurrent reconciles triggered by HPA deletion would not see the paused status and perform redundant operations. The key insight is to establish the paused state in etcd BEFORE any operations that trigger new reconciles, ensuring subsequent reconciles see the paused status and exit early. This solution follows the approach suggested by @rickbrouwer. Fixes #7231 Signed-off-by: nusmql <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * fix: use TriggerError when all ScaledJob triggers fail (#7205) Signed-off-by: Rick Brouwer <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * Fix transfer-hpa-ownership panic when hpa name not provided (#7260) * chore: renormalize line endings Signed-off-by: James Williams <[email protected]> * fix: nil pointer when transfer-hpa-ownership is true but hpa name not specified (#7254) Signed-off-by: James Williams <[email protected]> * update changelog Signed-off-by: James Williams <[email protected]> * revert vendor changes Signed-off-by: James Williams <[email protected]> --------- Signed-off-by: James Williams <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * fix: restore HPA behavior when paused-scale-in/out annotation is deleted (#7291) When paused-scale-in or paused-scale-out annotation is deleted (not set to "false") and the corresponding selectPolicy (scaleDown.selectPolicy or scaleUp.selectPolicy) is not explicitly set in the ScaledObject spec, the HPA's SelectPolicy remains stuck at "Disabled" instead of being restored. This occurs even if other behavior fields like policies or stabilizationWindowSeconds are defined - only an explicit selectPolicy value triggers the update. Root cause: DeepDerivative treats nil as "unset" and considers it a subset of any value, so DeepDerivative(nil, Disabled) returns true, preventing the HPA update. Fix: Add explicit DeepEqual check for Behavior field, following the existing pattern used for Metrics length check. test: add e2e test for paused-scale-in annotation removal Signed-off-by: Dima Shevchuk <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * refactor: remove unused scaledObjectMetricSpecs variable (#7292) * refactor: remove unused scaledObjectMetricSpecs variable Signed-off-by: u-kai <[email protected]> * update CHANGELOG.md Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * fix: handle requestScaleLoop error in ScaledObject controller (#7273) * fix: handle requestScaleLoop error in ScaledObject controller Signed-off-by: u-kai <[email protected]> * chore: update CHANGELOG for PR #7273 Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]> Signed-off-by: Jorge Turrado Ferrero <[email protected]> Co-authored-by: Jorge Turrado Ferrero <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * bump actions and go version (#7295) * bump actions and go version Signed-off-by: Jorge Turrado <[email protected]> * bump deps Signed-off-by: Jorge Turrado <[email protected]> * update pkgs Signed-off-by: Jorge Turrado <[email protected]> * update tools Signed-off-by: Jorge Turrado <[email protected]> * . Signed-off-by: Jorge Turrado <[email protected]> * fix test Signed-off-by: Jorge Turrado <[email protected]> * fix lint Signed-off-by: Jorge Turrado <[email protected]> * update setup-go to use go.mod version Signed-off-by: Jorge Turrado <[email protected]> * add nolint to exclude pulsar issues Signed-off-by: Jorge Turrado <[email protected]> * fix devenv Signed-off-by: Jorge Turrado <[email protected]> * fix codeql Signed-off-by: Jorge Turrado <[email protected]> * fix splunk test Signed-off-by: Jorge Turrado <[email protected]> * include job in links Signed-off-by: Jorge Turrado <[email protected]> * update to ubuntu-slim some runners Signed-off-by: Jorge Turrado <[email protected]> * Update apis/keda/v1alpha1/scaledobject_webhook_test.go Co-authored-by: Copilot <[email protected]> Signed-off-by: Jorge Turrado Ferrero <[email protected]> * Update .github/workflows/scorecards.yml Co-authored-by: Jan Wozniak <[email protected]> Signed-off-by: Jorge Turrado Ferrero <[email protected]> --------- Signed-off-by: Jorge Turrado <[email protected]> Signed-off-by: Jorge Turrado Ferrero <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Jan Wozniak <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> * update changelog Signed-off-by: Jorge Turrado <[email protected]> --------- Signed-off-by: Rick Brouwer <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> Signed-off-by: Hiroki Matsui <[email protected]> Signed-off-by: nusmql <[email protected]> Signed-off-by: James Williams <[email protected]> Signed-off-by: Dima Shevchuk <[email protected]> Signed-off-by: u-kai <[email protected]> Signed-off-by: Jorge Turrado Ferrero <[email protected]> Signed-off-by: Jorge Turrado <[email protected]> Co-authored-by: Rick Brouwer <[email protected]> Co-authored-by: Matchan <[email protected]> Co-authored-by: nusmql <[email protected]> Co-authored-by: James Williams <[email protected]> Co-authored-by: Dima Shevchuk <[email protected]> Co-authored-by: Kai Udo <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Jan Wozniak <[email protected]>
alt-dima
pushed a commit
to alt-dima/keda
that referenced
this pull request
Dec 13, 2025
* refactor: remove unused scaledObjectMetricSpecs variable Signed-off-by: u-kai <[email protected]> * update CHANGELOG.md Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]> Signed-off-by: Dmitriy Altuhov <[email protected]>
tangobango5
pushed a commit
to tangobango5/keda
that referenced
this pull request
Dec 22, 2025
* refactor: remove unused scaledObjectMetricSpecs variable Signed-off-by: u-kai <[email protected]> * update CHANGELOG.md Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]>
tangobango5
pushed a commit
to tangobango5/keda
that referenced
this pull request
Feb 13, 2026
* refactor: remove unused scaledObjectMetricSpecs variable Signed-off-by: u-kai <[email protected]> * update CHANGELOG.md Signed-off-by: u-kai <[email protected]> --------- Signed-off-by: u-kai <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Provide a description of what has been changed
Checklist
Fixes #
None
Relates to #
None