-
Notifications
You must be signed in to change notification settings - Fork 862
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
hpa-controller supports custom metrics #3673
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #3673 +/- ##
==========================================
- Coverage 56.64% 56.61% -0.03%
==========================================
Files 221 221
Lines 20838 20831 -7
==========================================
- Hits 11803 11794 -9
Misses 8413 8413
- Partials 622 624 +2
Flags with carried forward coverage won't be shown. Click here to find out more. |
8270653
to
7212a5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a reminder, #3638 has been merged.
Test results can refer to karmada-io/website#378. |
/cc @jwcesign |
Could you please review this as well, @chaunceyjiang? |
7212a5c
to
cad8ac6
Compare
Do you have any pictures of the testing result? Refer to: karmada-io/website#378 |
/lgtm cc @chaunceyjiang @RainbowMango for checking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -678,6 +679,8 @@ func (c *FederatedHPAController) validateAndParseSelector(hpa *autoscalingv1alph | |||
|
|||
// Computes the desired number of replicas for a specific hpa and metric specification, | |||
// returning the metric status and a proposed condition to be set on the HPA object. | |||
// | |||
//nolint:gocyclo | |||
func (c *FederatedHPAController) computeReplicasForMetric(ctx context.Context, hpa *autoscalingv1alpha1.FederatedHPA, spec autoscalingv2.MetricSpec, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many parameters on this method, please open an issue to optimize it and remove nolint:gocyclo
.
cad8ac6
to
4105143
Compare
Signed-off-by: Poor12 <shentiecheng@huawei.com>
4105143
to
eeab59c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
/hold
for #3673 (comment).
Please unhold after the issue is created.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
What type of PR is this?
/kind feature
What this PR does / why we need it:
hpa-controller supports custom metrics, depends on #3638.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
None