Skip to content

Conversation

@elankath
Copy link
Member

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Machine Class now has VirtualCapacity that maps to WorkerPool's NodeTemplate VirtualCapacity.

ialidzhikov and others added 30 commits January 6, 2022 11:30
* 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: Ashwani Kumar <[email protected]>

Co-authored-by: Ashwani Kumar <[email protected]>
Signed-off-by: ialidzhikov <[email protected]>
timebertt and others added 3 commits June 27, 2025 18:48
…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
@gardener-robot gardener-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 1, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 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 Jul 1, 2025
@ghost ghost 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 Jul 1, 2025
@gardener-robot gardener-robot added kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 1, 2025
@ghost ghost 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 Jul 1, 2025
@gardener-robot gardener-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 1, 2025
@ghost ghost 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 Jul 1, 2025
@gardener-robot gardener-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 1, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 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 Jul 1, 2025
@ghost ghost 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 Jul 1, 2025
Copy link
Member

@aaronfern aaronfern left a 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
// 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 {
Copy link
Member

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

@gardener-robot-ci-1 gardener-robot-ci-1 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 Aug 5, 2025
@gardener-robot gardener-robot added the needs/rebase Needs git rebase label Aug 5, 2025
@gardener-robot
Copy link

@elankath You need rebase this pull request with latest master branch. Please check.

@gardener-robot gardener-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 5, 2025
@elankath elankath closed this by deleting the head repository Aug 5, 2025
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/api-change API change with impact on API users needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/rebase Needs git rebase needs/review Needs review needs/second-opinion Needs second review by someone else reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. status/closed Issue is closed (either delivered or triaged)

Projects

None yet

Development

Successfully merging this pull request may close these issues.