add replicas syncer for resources with HPA

[lxtywypc]

What type of PR is this?
/kind feature

What this PR does / why we need it:
As #4057

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmada-controller-manager`: Introduced `hpaReplicasSyncer` controller which syncs workload's replicas from the member cluster to the control plane.  

[codecov-commenter]

Codecov Report

Patch coverage: 40.24% and project coverage change: -0.02% ⚠️

Comparison is base (5c77f45) 53.80% compared to head (5ae8178) 53.79%.
Report is 18 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4072      +/-   ##
==========================================
- Coverage   53.80%   53.79%   -0.02%     
==========================================
  Files         231      232       +1     
  Lines       23013    23144     +131     
==========================================
+ Hits        12383    12451      +68     
- Misses       9957    10015      +58     
- Partials      673      678       +5     

Flag	Coverage Δ
unittests	53.79% <40.24%> (-0.02%)	⬇️

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

Files Changed	Coverage Δ
...pareplicassyncer/hpa_replicas_syncer_controller.go	40.24% <40.24%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[XiShanYongYe-Chang]

/assign

[lxtywypc]

Hi @XiShanYongYe-Chang

I'm not familiar with the chart-lint-test, could you help me find out why it failed? XD

[XiShanYongYe-Chang]

Hi @lxtywypc Can we not add the logic related to webhook first? Can we consider adding a finalizer for HPA?

The controller we added can be set to closed by default, but once the webhook is set, it will always take effect. Therefore, there may be inconsistencies between these two behaviors.

[lxtywypc]

Okay, I will give a try.

[RainbowMango]

I think we can focus on building the controller with this PR (for the finalizer thing, we can do it by a follow-up), which helps keep PR small and easy for review and testing.

[XiShanYongYe-Chang]

Thanks a lot~
I skipped the logic related to webhook when I reviewed it.

[XiShanYongYe-Chang]

Thanks a lot~

[XiShanYongYe-Chang]

Thanks a lot~
Just one little suggestion. Other parts LGTM

[XiShanYongYe-Chang]

Thanks a lot~
/lgtm
/cc @RainbowMango @chaosi-zju

[karmada-bot]

@XiShanYongYe-Chang: GitHub didn't allow me to request PR reviews from the following users: chaosi-zju.

Note that only karmada-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Thanks a lot~
/lgtm
/cc @RainbowMango @chaosi-zju

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chaunceyjiang]

/assign

[XiShanYongYe-Chang]

Thanks @chaunceyjiang
I'm sorry forgot to assign it to you.

[chaunceyjiang]

Can you explain why you are using CurrentReplicas instead of DesiredReplicas?

[RainbowMango]

I think the .status.currentReplicas represents the real replicas of a managed workload, but the .status.desiredReplicas is the planned replicas which might not represent the current situation.

[XiShanYongYe-Chang]

I agree with you! Thanks!
I wrote it in the issue, and I wrote it wrong.
We really should use DesiredReplicas, which is the replicas described by spec in the member cluster:

https://github.com/kubernetes/kubernetes/blob/fd5f36e6a0e1d71d59e59620a4826eec6e6a6c7b/pkg/controller/podautoscaler/horizontal.go#L798

https://github.com/kubernetes/kubernetes/blob/fd5f36e6a0e1d71d59e59620a4826eec6e6a6c7b/pkg/controller/podautoscaler/horizontal.go#L907-L910

https://github.com/kubernetes/kubernetes/blob/fd5f36e6a0e1d71d59e59620a4826eec6e6a6c7b/pkg/controller/podautoscaler/horizontal.go#L1368-L1370

[XiShanYongYe-Chang]

Hi @lxtywypc, can you help update it?

Wait a moment, Let me have a try.

[RainbowMango]

@chaunceyjiang
I guess you mean the .spec.replicas represents desired replicas that match the definition of .status.desiredReplicas in HPA, right?

The difference between the two approaches is when to sync replicas. after and before replica change. It makes more sense to sync desired replicas, it looks like the HPA in the Karmada control plane scales the workload :)

[XiShanYongYe-Chang]

Hi @lxtywypc How do you think?

[chaunceyjiang]

I guess you mean the .spec.replicas represents desired replicas that match the definition of .status.desiredReplicas in HPA, right?

Yes. I mean exactly this.

[chaunceyjiang]

So I prefer to use .status.desiredReplicas because its meaning is consistent with .spec.replicas.

[RainbowMango]

Thanks @chaunceyjiang, it sounds good to me.

Given following task relies on this controller, I suppose we can move this forward, and leave another task for this.
What do you think? @chaunceyjiang @XiShanYongYe-Chang @lxtywypc

[chaunceyjiang]

OK

[chaunceyjiang]

Can you explain why you use CurrentReplicas insterd of DesiredReplicas

[XiShanYongYe-Chang]

Now we have different views on whether to use CurrentReplicas or desiredReplicas, and in my opinion, we need to have a more in-depth discussion of that.

For now, whether to use CurrentReplicas or desiredReplicas does not affect the addition of controllers. The current PR complies with the task description in the issue #4057. Therefore, the issues discussed here can be escalated to the issue description for discussion and analysis.

What do you think? @lxtywypc @chaunceyjiang @RainbowMango

[chaunceyjiang]

Thanks~

/lgtm

[RainbowMango]

/approve

[karmada-bot]

[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 [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment