-
Notifications
You must be signed in to change notification settings - Fork 130
Added VirtualCapacity to MachineClass NodeTemplate #998
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
Conversation
Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>
Co-authored-by: Samarth Deyagond <samarth.deyagond@sap.com>
Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>
* improve documentation * added make rule to generate api docs
* Fixed broken links * Fixed additional broken links
…ollout Scale down annotation value corrected
Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>
Co-authored-by: Samarth Deyagond <sdeyagond@microsoft.com> Co-authored-by: Martin Weindel <martin.weindel@sap.com>
Co-authored-by: Ashwani Kumar <ash.kumar@sap.com> Co-authored-by: Ashwani Kumar <ash.kumar@sap.com>
Co-authored-by: Himanshu Sharma <79965161+himanshu-kun@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @elankath!
A few comments from me
// Capacity contains subfields to track all node resources required to scale nodegroup from zero | ||
Capacity corev1.ResourceList | ||
|
||
// VirtualCapacity represents the expected Node 'virtual' capacity ie comprising virtual extended resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
// VirtualCapacity represents the expected Node 'virtual' capacity ie comprising virtual extended resources. | |
// VirtualCapacity represents the expected Node 'virtual' capacity i.e comprising virtual extended resources. |
virtualCapacityChanged = SyncVirtualCapacity(machineClass.NodeTemplate.VirtualCapacity, nodeCopy, lastAppliedVirtualCapacity) | ||
} | ||
|
||
if !annotationsChanged && !labelsChanged && !taintsChanged && !virtualCapacityChanged { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will have to consider initializedNodeAnnotation
in this check here
As otherwise we could prematurely return and miss adding node.machine.sapcloud.io/last-applied-anno-labels-taints
annotation to the node
Co-authored-by: Aaron Francis Fernandes <79958509+aaronfern@users.noreply.github.com>
@elankath You need rebase this pull request with latest master branch. Please check. |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note: