Skip to content

Add NodeNamePolicy to Controlplane configuration#190

Merged
Nuckal777 merged 2 commits intomainfrom
enh/nodename
May 28, 2025
Merged

Add NodeNamePolicy to Controlplane configuration#190
Nuckal777 merged 2 commits intomainfrom
enh/nodename

Conversation

@afritzler
Copy link
Copy Markdown
Member

@afritzler afritzler commented May 22, 2025

Proposed Changes

Extend the Controlplane provider configuration to allow to set the NodeNamePolicy for a Shoot cluster. Supported values are Server. If that field is set, the MCM gets an extra arg to use the Server name as a Node name instead of using the ServerClaim name.

If this field is not set, ServerClaim name will be used (default behaviour).

This solution should be future proof in case we add a ServerClaim.Spec.Hostname field for custom hostnames.

/ref ironcore-dev/machine-controller-manager-provider-ironcore-metal#103

@afritzler afritzler requested a review from a team as a code owner May 22, 2025 12:45
@github-actions github-actions Bot added api-change size/L controllers documentation Improvements or additions to documentation enhancement New feature or request labels May 22, 2025
Extend the `Controlplane` provider configuration to allow to set the
`HostnamePolicy` for a `Shoot` cluster. Supported values are `Server`.
If that field is set, the MCM gets an extra `arg` to use the `Server`
name as a `Node` name instead of using the `ServerClaim` name.

If this field is not set, `ServerClaim` name will be used (default
behaviour).
Copy link
Copy Markdown
Contributor

@Nuckal777 Nuckal777 left a comment

Choose a reason for hiding this comment

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

Looks good in general. One question from the api design perspective. Should the hostnamePolicy field be required and defaulted to Claim? 🤔

@afritzler
Copy link
Copy Markdown
Member Author

I actually might change the flag in the MCM to also use policy instead of a bool. But either way since the claim name is the default that is ok. We might want also to extend the ServerClaim to use a custom hostname in the future - that is why a policy based approach here an in the MCM is better.

@afritzler afritzler changed the title Add HostnamePolicy to Controlplane configuration Add NodeNamePolicy to Controlplane configuration May 23, 2025
ensurer = NewEnsurer(logger, false)
scheme := runtime.NewScheme()
Expect(v1alpha1.AddToScheme(scheme)).To(Succeed())
ensurer = controlplane.NewEnsurer(logr.Discard(), scheme)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fyi, Ginkgo brings a GinkgoLogr, which prints logs in case of test failures.

@Nuckal777 Nuckal777 merged commit 655c44c into main May 28, 2025
9 checks passed
@Nuckal777 Nuckal777 deleted the enh/nodename branch May 28, 2025 09:32
@hardikdr hardikdr added this to Roadmap Jun 26, 2025
@hardikdr hardikdr added the area/gardener-extension Gardener extension integration. label Jun 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change area/gardener-extension Gardener extension integration. controllers documentation Improvements or additions to documentation enhancement New feature or request size/L

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants