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

Cache not to be used to form node-template during scale from zero #119

Conversation

himanshu-kun
Copy link

What this PR does / why we need it:
NodeTemplate is formed for each nodegroup every time main loop of CA executes. This nodeTemplate is req by CA to perform calculation and finally deciding upon which nodegrp to scale-up. NodeTemplate is formed by 3 options for each nodegrp (in order):

  1. from a ready existing node
  2. from a previously ready, now non-existent node
  3. from template creation function implemented for particular provider (in our case we take values from machineClass in this function)

We have a case when a node got created in a sfz nodegrp , but now its not there. Then every other time the cached nodeTemplate is used for that nodegrp. This could be problematic in case when nodeGrp instance type gets updated , because still the nodeTemplate is based on the old instanceType.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
To resolve the above , the nodeTemplate formation from cache has been disabled for nodeGrps with minSize 0 i.e. the ones which would scale from zero. This will cause CA to form nodeTemplate using the machineClass only.
This change is done in the core part of CA which we usually don't touch.
Release note:

NodeTemplate is not formed using cache in case nodegrp has minimum size zero. This is done to keep nodeTemplate updated even when instance type of nodegrp is updated.

@himanshu-kun himanshu-kun requested review from hardikdr and a team as code owners May 2, 2022 09:43
@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels May 2, 2022
@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 May 2, 2022
@himanshu-kun
Copy link
Author

/invite @unmarshall

@gardener-robot gardener-robot requested a review from unmarshall May 2, 2022 09:43
@gardener-robot-ci-1 gardener-robot-ci-1 added 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 May 2, 2022
@gardener-robot
Copy link

@hardikdr, @unmarshall You have pull request review open invite, please check

@himanshu-kun himanshu-kun force-pushed the no-cache-for-sfz-node-groups branch from 66b57ad to 5a4c295 Compare May 9, 2022 08:31
@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 May 9, 2022
@gardener-robot-ci-1 gardener-robot-ci-1 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 May 9, 2022
@himanshu-kun himanshu-kun merged commit 7af967b into gardener:machine-controller-manager-provider May 9, 2022
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label May 9, 2022
@himanshu-kun himanshu-kun deleted the no-cache-for-sfz-node-groups branch May 9, 2022 08:49
himanshu-kun added a commit to himanshu-kun/autoscaler that referenced this pull request Jun 23, 2022
@himanshu-kun himanshu-kun mentioned this pull request Jun 23, 2022
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) needs/review Needs review size/xs Size of pull request is tiny (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.

4 participants