Skip to content
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

Improve usability of ControlPlane's registrationMethod with Cluster Classes #343

Closed
anmazzotti opened this issue Jun 3, 2024 · 0 comments · Fixed by #418
Closed

Improve usability of ControlPlane's registrationMethod with Cluster Classes #343

anmazzotti opened this issue Jun 3, 2024 · 0 comments · Fixed by #418
Assignees
Labels
kind/feature New feature or request priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@anmazzotti
Copy link
Contributor

anmazzotti commented Jun 3, 2024

Describe the solution you'd like:
Due to validation of the RKE2ControlPlane.spec.registrationMethod, it is not currently possible to use registrationMethod: "address" in ClusterClasses without providing a valid registrationAddress too.

Even with patching this value afterwards, like:

    - jsonPatches:
      - op: replace
        path: /spec/template/spec/registrationAddress
        valueFrom:
          variable: myRegistrationAddress

This will fail with:

The RKE2ControlPlane "rke2-control-plane" is invalid: spec.registrationAddress: Invalid value: "": registrationAddress must be supplied when using registration method 'address'

As the initial RKE2ControlPlane is created with empty registrationAddress, before patching.

One workaround is to configure the initial registrationMethod to control-plane-endpoint or any other default that does not require validation.

Then I think one patch should replace both values ad the same time:

    - jsonPatches:
      - op: replace
        path: /spec/template/spec/registrationMethod
        value: "address"
      - op: replace
        path: /spec/template/spec/registrationAddress
        valueFrom:
          variable: myRegistrationAddress

I did not test it, but it should technically work.
The drawback of this workaround however, is that initially the RKE2ControlPlane object is created with a different registrationMethod. This could lead to race condition issues where maybe nodes are joining using an unwanted registration method.

It should be fine on the initial cluster creation, assuming the first node ever will be the first control plane node, so there will be nothing to register on yet, and after that (or even before bootstrapping this node), the registrationMethod should be patched to the user defined value.

Could be more tricky however on a control plane rollout or during other upgrade scenarios.

Why do you want this feature:
I want to avoid having to configure a default registrationMethod for the real one to be patched later, if this could lead to problems.

@anmazzotti anmazzotti added kind/feature New feature or request needs-priority Indicates an issue or PR needs a priority assigning to it needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jun 3, 2024
@Danil-Grigorev Danil-Grigorev self-assigned this Aug 13, 2024
@alexander-demicev alexander-demicev added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-priority Indicates an issue or PR needs a priority assigning to it needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants