Skip to content

[GEP-28] gardenadm bootstrap: Deploy machine-controller-manager #12152

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
May 27, 2025
Merged

[GEP-28] gardenadm bootstrap: Deploy machine-controller-manager #12152

merged 8 commits into from
May 27, 2025

Conversation

timebertt
Copy link
Member

@timebertt timebertt commented May 21, 2025 •
edited
Loading

How to categorize this PR?

/area ipcei
/kind enhancement

What this PR does / why we need it:

This PR adds the machine-controller-manager deployment to gardenadm bootstrap.

Which issue(s) this PR fixes:
Part of #2906

Special notes for your reviewer:

/cc @rfranzke @ScheererJ

For now, mcm is configured with --target-kubeconfig="", i.e., it points to the bootstrap cluster (kind), as it does not support managing machines without a target cluster, yet: gardener/machine-controller-manager#994
Once this is supported, we can switch to --target-kubeconfig=none to entirely disable its interaction with the target cluster.
I still implemented this PR first, because it will offer a good setup for developing/testing the needed mcm changes together with the next step (deploying the Worker).

Release note:

The `machinecontrollermanager.ProviderSidecarContainer` now expects additional `shoot` and `controlPlaneNamespace` params. In `controlplane` webhooks, extensions can use `GardenContext.GetCluster` to retrieve the shoot from the `Cluster` object.

@gardener-prow gardener-prow bot requested review from rfranzke and ScheererJ May 21, 2025 06:03
@gardener-prow gardener-prow bot added area/ipcei IPCEI (Important Project of Common European Interest) kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 21, 2025
@timebertt timebertt mentioned this pull request May 21, 2025
Open
@ScheererJ
Copy link
Member

/assign

@timebertt timebertt force-pushed the gardenadm-bootstrap-mcm branch from b1ea386 to 289c86c Compare May 21, 2025 14:39
@rfranzke
Copy link
Member

/assign

@timebertt
Copy link
Member Author

I just rebased this PR and updated the last commit message/TODO after discussing it with @oliver-goetz and @rfranzke. It's only a temporary commit to unblock this PR (workaround for the broken CSR approval in autonomous shoots) and can be reverted/dropped once the bug is fixed (@oliver-goetz is already on it).

ScheererJ
ScheererJ reviewed May 21, 2025
View reviewed changes
Copy link
Member

@ScheererJ ScheererJ 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 driving gardenadm bootstrap forward.

rfranzke
rfranzke reviewed May 22, 2025
View reviewed changes
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2025
@timebertt timebertt force-pushed the gardenadm-bootstrap-mcm branch from 289c86c to 8d67e8b Compare May 22, 2025 14:43
@gardener-prow gardener-prow bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 22, 2025
@timebertt timebertt force-pushed the gardenadm-bootstrap-mcm branch from 04cfecc to 31e7c38 Compare May 23, 2025 15:11
@timebertt
Copy link
Member Author

I pushed another commit to drop the distinction between scenarios. For now, the machine CRDs are not deployed in gardenadm init. This allowed me to drop the workaround for #12169 in 8d67e8b.
There are corresponding TODOs in the umbrella issue for this distinction and for deploying the ControlPlane resource.

With this, the PR is ready for another round of reviews.
/hold cancel

@gardener-prow gardener-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 23, 2025
@timebertt timebertt force-pushed the gardenadm-bootstrap-mcm branch from 31e7c38 to 3a964ca Compare May 24, 2025 15:32
@ScheererJ
Copy link
Member

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 26, 2025
@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 26, 2025

LGTM label has been added.

Git tree hash: 6e2e3d0fcb62b145535a6eb20db418e89550a5f9

@timebertt timebertt requested a review from rfranzke May 26, 2025 11:18
@timebertt timebertt mentioned this pull request May 26, 2025
Merged
rfranzke
rfranzke reviewed May 27, 2025
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/approve

@gardener-prow Gardener Prow
Copy link
Contributor

gardener-prow bot commented May 27, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 27, 2025
@gardener-prow gardener-prow bot merged commit ecf9c35 into gardener:master May 27, 2025
19 checks passed
@timebertt timebertt deleted the gardenadm-bootstrap-mcm branch May 27, 2025 10:13
@timebertt timebertt mentioned this pull request May 28, 2025
Merged
@timebertt timebertt mentioned this pull request Aug 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfranzke rfranzke rfranzke left review comments

@ScheererJ ScheererJ ScheererJ left review comments

Assignees

@rfranzke rfranzke

@ScheererJ ScheererJ

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/ipcei IPCEI (Important Project of Common European Interest) cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@timebertt @ScheererJ @rfranzke