Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Ignore replicaCount when autoscale is enabled #36928

Merged
merged 6 commits into from Mar 11, 2022
Merged

Ignore replicaCount when autoscale is enabled #36928

merged 6 commits into from Mar 11, 2022

Conversation

timonwong
Copy link
Contributor

@timonwong timonwong commented Jan 21, 2022 •
edited

Signed-off-by: Tianpeng Wang tpwang@alauda.io

Please provide a description of this PR:

When autoscale is enabled we should not overwrite replica count, consider following scenario:

  1. Set values.pilot.autoscaleEnabled=true, components.pilot.k8s.replicaCount=1
  2. In istio operator it "caches" the generated manifests (with istiod.replicas=1)
  3. Provides: HPA scales our pilot replicas to 3
  4. Set values.pilot.autoscaleEnabled=false
  5. The generated manifests (with istiod.replicas=1) is same as istio operator "cache", the deployment will not get updated unless istio operator is restarted.

@timonwong timonwong requested a review from a team as a code owner January 21, 2022 06:32
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 21, 2022
@istio-testing
Copy link
Collaborator

Hi @timonwong. Thanks for your PR.

I'm waiting for a istio 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/test-infra repository.

@timonwong
Unset replicaCount when autoscale is enabled
fa062a0 
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@zirain
Copy link
Member

zirain commented Jan 21, 2022

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Jan 21, 2022
@timonwong
fix license header
843ca08
@timonwong
fix tests
0735a0a 
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@timonwong
Copy link
Contributor Author

Should this label "release-notes-none"?

@timonwong timonwong changed the title Unset replicaCount when autoscale is enabled Ignore replicaCount when autoscale is enabled Jan 21, 2022
@zirain
Copy link
Member

zirain commented Jan 22, 2022

@timonwong I prefer to add a release notes

@timonwong
releasenote
efef471 
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@timonwong timonwong requested review from a team as code owners January 22, 2022 00:40
@timonwong
Copy link
Contributor Author

timonwong commented Jan 22, 2022 •
edited

@timonwong I prefer to add a release notes

The release note is now added

@timonwong @ericvn
Update releasenotes/notes/36928.yaml
868c239 
Co-authored-by: Eric Van Norman <ericvn@us.ibm.com>
@jacob-delgado
Copy link
Contributor

This code looks fine to me, but @litong01 and @howardjohn do you have any thoughts?

@timonwong
feature validation refactor into interface
3f0fe8a 
Signed-off-by: Tianpeng Wang <tpwang@alauda.io>
@istio-testing istio-testing merged commit 28c8811 into istio:master Mar 11, 2022
@timonwong timonwong deleted the hpa-replicacount branch March 13, 2022 01:29
aryan16 pushed a commit to aryan16/istio that referenced this pull request Mar 28, 2022
@timonwong @ericvn @aryan16
Ignore replicaCount when autoscale is enabled (istio#36928)
1f004ec 
* Unset replicaCount when autoscale is enabled

Signed-off-by: Tianpeng Wang <tpwang@alauda.io>

* fix license header

* fix tests

Signed-off-by: Tianpeng Wang <tpwang@alauda.io>

* releasenote

Signed-off-by: Tianpeng Wang <tpwang@alauda.io>

* Update releasenotes/notes/36928.yaml

Co-authored-by: Eric Van Norman <ericvn@us.ibm.com>

* feature validation refactor into interface

Signed-off-by: Tianpeng Wang <tpwang@alauda.io>

Co-authored-by: Eric Van Norman <ericvn@us.ibm.com>
@martinstenhoff martinstenhoff mentioned this pull request Aug 3, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@litong01 litong01 litong01 approved these changes

@ericvn ericvn ericvn approved these changes

Assignees
No one assigned
Labels
ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@timonwong @istio-testing @zirain @jacob-delgado @litong01 @ericvn