karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.12k stars 807 forks source link

Add federatedHpa controller #3589

Closed Poor12 closed 11 months ago

Poor12 commented 11 months ago

What type of PR is this? /kind feature

What this PR does / why we need it: Part of https://github.com/karmada-io/karmada/issues/3379 This PR focus on centralizedHpa controller, the responsibility is:

  1. Get metrics and pods across multiple clusters and calculate desired replicas
  2. Scale resource template based on desiredReplicas

Which issue(s) this PR fixes: Fixes #

Special notes for your reviewer:

  1. Need a Centralized API Definition.
  2. Need to clarify the behavior of Duplicated. When users use duplicated policy to propagate resource templates to multiple clusters, the min/max of hpa means whether the all replicas across clusters or in the single cluster.
  3. Ignore Lint because we want to keep the same between the upstream and centralizedHpa controller.

Does this PR introduce a user-facing change?: None

chaunceyjiang commented 11 months ago

Ignore Lint

What's mean? golang-lint ?

Poor12 commented 11 months ago

Ignore Lint

What's mean? golang-lint ?

Yeah, centralizedHpa controller use part of codes in the native K8s hpa-controller. We do not want to change them right now even if it will not pass golang-lint.

Poor12 commented 11 months ago

/cc @chaunceyjiang @XiShanYongYe-Chang @RainbowMango

chaunceyjiang commented 11 months ago

Yeah, centralizedHpa controller use part of codes in the native K8s hpa-controller. We do not want to change them right now even if it will not pass golang-lint.

🤔

So, in the future, all PRs will not be able to pass the golang-lint check, right?

Poor12 commented 11 months ago

Yeah, centralizedHpa controller use part of codes in the native K8s hpa-controller. We do not want to change them right now even if it will not pass golang-lint.

🤔

So, in the future, all PRs will not be able to pass the golang-lint check, right?

Not exactly, for those long function which is lifted from kubernetes, now we just keep the same. For others, we will deal with the golang-lint.

codecov-commenter commented 11 months ago

Codecov Report

Merging #3589 (3a14648) into master (1e7ab93) will increase coverage by 0.00%. The diff coverage is 56.92%.

:exclamation: 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    #3589    +/-   ##
========================================
  Coverage   55.83%   55.84%            
========================================
  Files         216      220     +4     
  Lines       20129    20595   +466     
========================================
+ Hits        11240    11501   +261     
- Misses       8284     8484   +200     
- Partials      605      610     +5     
Flag Coverage Δ
unittests 55.84% <56.92%> (+<0.01%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/controller-manager/app/options/options.go 0.00% <0.00%> (ø)
pkg/controllers/context/context.go 48.57% <ø> (ø)
pkg/controllers/federatedhpa/metrics/client.go 0.00% <0.00%> (ø)
pkg/util/helper/pod.go 92.00% <0.00%> (-8.00%) :arrow_down:
pkg/util/membercluster_client.go 60.16% <0.00%> (-12.39%) :arrow_down:
pkg/util/lifted/federatedhpa.go 36.95% <36.95%> (ø)
pkg/util/lifted/selectors/bimultimap.go 88.98% <88.98%> (ø)
...kg/controllers/federatedhpa/metrics/utilization.go 100.00% <100.00%> (ø)
...armadactl/cmdinit/karmada/webhook_configuration.go 86.12% <100.00%> (+0.84%) :arrow_up:
pkg/util/fedinformer/transform.go 79.16% <100.00%> (+1.89%) :arrow_up:

... and 1 file with indirect coverage changes

Poor12 commented 11 months ago

Codebase is ready now. For some lifted code, I store it in the pkg/ right now. Later I will move it to lifted for better management.

RainbowMango commented 11 months ago

lint is failing:

pkg/util/helper/federatedhpa_test.go:5: File is not `gci`-ed with --skip-generated -s Standard,Default,Prefix(github.com/karmada-io/karmada) (gci)
    "github.com/stretchr/testify/assert"
pkg/util/helper/federatedhpa_test.go:6: File is not `gci`-ed with --skip-generated -s Standard,Default,Prefix(github.com/karmada-io/karmada) (gci)

pkg/util/helper/federatedhpa_test.go:4: File is not `gofmt`-ed with `-s` (gofmt)
    "testing"
Poor12 commented 11 months ago

/cc @jwcesign @RainbowMango

karmada-bot commented 11 months ago

[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: - ~~[OWNERS](https://github.com/karmada-io/karmada/blob/master/OWNERS)~~ [RainbowMango] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment