Do not render .spec.replicas if Autoscaling is Enabled

[sherifabdlnaby]

Issue

The current chart template will render .spec.replicas even if Autoscaling is enabled. This causes a conflict in ownership of the .spec.replicas field between the HPA and whoever is installing/reconciling the manifests.

The effect of this depends on the tool used to install the manifests:

1. Helm CLI -> the .spec.replicas is reset to the .replicaCount value everytime Helm Install/Update is invoked.
2. AgroCD (or any GitOps tool) -> .spec.replicas is reset on each Sync... which can result into a constant back and forth between ArgoCD and the HPA that technically eliminate the benefits of the HPA.

Description of changes

Do not render .spec.replicas if Autoscaling is Enabled.

Checklist

• Added/modified documentation as required (such as the README.md for modified charts)
• Incremented the chart version in Chart.yaml for the modified chart(s)
• Manually tested. Describe what testing was done in the testing section below
• Make sure the title of the PR is a good description that can go into the release notes

Testing

Checking the Helm Diff between main and this branch. Validate that .spec.replicas is no rendered when Autoscaling is enabled, and is still rendered when Autoscaling is disabled.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[k8s-ci-robot]

Welcome @sherifabdlnaby!

It looks like this is your first PR to kubernetes-sigs/aws-load-balancer-controller 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/aws-load-balancer-controller has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @sherifabdlnaby. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

[zac-nixon]

Could you explain the benefit of this PR? Currently, it doesn't make sense to horizontally scale this controller as it's leader elected. Only one replica can perform work at any given time.

[sherifabdlnaby]

@zac-nixon Yes sure, it's explained in Values.yaml

AWS LBC only has 1 main working pod, other pods are just standby
the purpose of enable hpa is to survive load induced failure by the calls to the aws-load-balancer-webhook-service
since the calls from kube-apiserver are sent round-robin to all replicas, and the failure policy on those webhooks is Fail
if the pods become overloaded and do not respond within the timeout that could block the creation of pods, targetgroupbindings or ingresses

https://github.com/kubernetes-sigs/aws-load-balancer-controller/blob/main/helm/aws-load-balancer-controller/values.yaml#L19-L25

[zac-nixon]

Sorry, I missed that the PR wasn't rebased. Please rebase to add back in
411778c
fab90f4

[sherifabdlnaby]

@zac-nixon Rebased. Please re-review 👍🏻

[zac-nixon]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sherifabdlnaby, zac-nixon

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 [zac-nixon]

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