-
Notifications
You must be signed in to change notification settings - Fork 133
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 <[email protected]>
Co-authored-by: Samarth Deyagond <[email protected]>
Signed-off-by: ialidzhikov <[email protected]>
* 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 <[email protected]>
Co-authored-by: Samarth Deyagond <[email protected]> Co-authored-by: Martin Weindel <[email protected]>
Co-authored-by: Ashwani Kumar <[email protected]> Co-authored-by: Ashwani Kumar <[email protected]>
Co-authored-by: Himanshu Sharma <[email protected]>
Signed-off-by: ialidzhikov <[email protected]>
…adm bootstrap` (#1004) * Add docs for running without target cluster * Handle disabled target cluster in manager/controller/test setup * Adapt `MachineDeployment` controller We skip any interaction with the target cluster by returning an error early if `InPlaceUpdate` is configured when running without a target cluster. `InPlaceUpdate` only works with a target cluster – `Node` metadata needs to be updated for it. Apart from that, there is no interaction with the target cluster in the `MachineDeployment` controller. * Adapt safety controller If running without a target cluster, we can entirely disable the SafetyAPIServer and Node controllers. Also, there is no need to annotate `Nodes` unmanaged by machine-controller-manager. * Adapt machine creation flow During machine creation, we never set the `node` label on the `Machine` to disable all interaction with the target cluster. With this, we skip waiting for the `Node` object to be registered. We also don't create any bootstrap tokens. Lastly, instead of tranisitioning to `Pending`, the `Machine` transitions to `Available` after creating the VM. * Adapt machine deletion flow If running without a target cluster, we skip drain and volume attachment logic and go straight to VM deletion. The `node` label does not exist, so we won't delete the `Node` object. * Extract `constants.TargetKubeconfigDisabledValue` * Move flag validation
aaronfern
left a comment
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 <[email protected]>
|
@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: