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.

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

No scale down for rolling update machineDeployment #160

Conversation

himanshu-kun
Copy link

@himanshu-kun himanshu-kun commented Jan 9, 2023

What this PR does / why we need it:
This PR adds changes to avoid any scale-down (scale-down due to underutilization was already stopped earlier using the scale-down-disabled annotation) like removal of longUnregistered nodes or removal of registered but long not Ready nodes.

Which issue(s) this PR fixes:
Fixes #30

Special notes for your reviewer:
Part of fix as per solution proposed here

Logic relies on the Progressing condition of machineDeployment.

Release note:

- Autoscaler would not scale-down if the node-grp(i.e. machineDeployment) is under rolling update. 
- Logic of fixing up the node grp size for node grp, whose current size not equals expected size for long time , is also disabled.

@himanshu-kun himanshu-kun requested review from unmarshall, rishabh-11 and a team as code owners January 9, 2023 08:57
@gardener-robot
Copy link

@himanshu-kun You need rebase this pull request with latest master branch. Please check.

@gardener-robot gardener-robot added needs/rebase Needs git rebase needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Jan 9, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 9, 2023
@himanshu-kun himanshu-kun force-pushed the no-scale-down-during-rolling-update branch from 29da250 to 4b960b9 Compare January 9, 2023 09:49
@gardener-robot gardener-robot removed the needs/rebase Needs git rebase label Jan 9, 2023
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 9, 2023
@gardener-robot gardener-robot added needs/changes Needs (more) changes size/s Size of pull request is small (see gardener-robot robot/bots/size.py) and removed size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Jan 9, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 10, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Jan 10, 2023
@rishabh-11
Copy link

Thanks for the changes.
/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging needs/changes Needs (more) changes and removed needs/changes Needs (more) changes needs/review Needs review reviewed/lgtm Has approval for merging labels Jan 10, 2023
@rishabh-11 rishabh-11 added the reviewed/lgtm Has approval for merging label Jan 10, 2023
@gardener-robot gardener-robot removed the needs/changes Needs (more) changes label Jan 10, 2023
@himanshu-kun himanshu-kun force-pushed the no-scale-down-during-rolling-update branch from efa357c to c085070 Compare January 23, 2023 10:22
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Jan 23, 2023
@himanshu-kun himanshu-kun self-assigned this Mar 7, 2023
@himanshu-kun himanshu-kun merged commit c322fca into gardener:machine-controller-manager-provider Mar 16, 2023
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Mar 16, 2023
@himanshu-kun himanshu-kun deleted the no-scale-down-during-rolling-update branch March 16, 2023 12:16
@himanshu-kun himanshu-kun added this to the v1.26 milestone Mar 17, 2023
@himanshu-kun himanshu-kun added the needs/cherry-pick Needs to be cherry-picked to older version label Mar 29, 2023
@himanshu-kun himanshu-kun removed their assignment Apr 4, 2023
@himanshu-kun
Copy link
Author

himanshu-kun commented Apr 10, 2023

/needs cherry-pick
on rel-v1.26, 1.25, 1.24 , 1.23 , 1.22 , 1.21 , 1.20

@gardener-robot
Copy link

@himanshu-kun No more than 5 labels permitted, but 13 labels were given.

rishabh-11 pushed a commit that referenced this pull request Apr 10, 2023
…ing update (#209)

* fixNodeGroupSize logic silenced

* no scale-down allowed during rolling update

* updated logs

* removed useless found variable

* addressed review comments and some refactoring
rishabh-11 pushed a commit that referenced this pull request Apr 10, 2023
…ing update (#208)

* fixNodeGroupSize logic silenced

* no scale-down allowed during rolling update

* updated logs

* removed useless found variable

* addressed review comments and some refactoring
rishabh-11 pushed a commit that referenced this pull request Apr 10, 2023
…ing update (#207)

* fixNodeGroupSize logic silenced

* no scale-down allowed during rolling update

* updated logs

* removed useless found variable

* addressed review comments and some refactoring
rishabh-11 pushed a commit that referenced this pull request Apr 10, 2023
…ing update (#206)

* fixNodeGroupSize logic silenced

* no scale-down allowed during rolling update

* updated logs

* removed useless found variable

* addressed review comments and some refactoring
rishabh-11 pushed a commit that referenced this pull request Apr 10, 2023
…ing update (#205)

* fixNodeGroupSize logic silenced

* no scale-down allowed during rolling update

* updated logs

* removed useless found variable

* addressed review comments and some refactoring
@himanshu-kun himanshu-kun removed the needs/cherry-pick Needs to be cherry-picked to older version label Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Avoid fixing nodegroup size
7 participants