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

Deprecate infrastructure ref #542

Merged
merged 1 commit into from
Jan 10, 2025

Conversation

anmazzotti
Copy link
Contributor

@anmazzotti anmazzotti commented Jan 6, 2025

kind/feature
kind/bug

What this PR does / why we need it:

This is a (dirty) attempt of using the v1beta1 newly added RKE2ControlPlane.Spec.MachineTemplate and fixes the API to be used properly with ClusterClass.

Note that this counts as an API change, however this is an attempt of getting away with it without a version bump.
The controlplane controller tries to catch and patch all missed conversions for this reason.

Also note that this PR assumes that v1beta1 RKE2ControlPlane.Spec.MachineTemplate.InfrastructureRef and RKE2ControlPlane.Spec.InfrastructureRef are equal. However since the former is currently ignored, it is not guaranteed that they will match.

All test manifests have been kept the same to validate the quicky code, with the exception of the clusterclass since we want to make usage of the machineTemplate field.

All other examples have been update to the RKE2ControlPlane.Spec.MachineTemplate notation.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #341

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@kkaempf kkaempf added kind/enhancement kind/bug Something isn't working area/clusterclass Issues or PRs related to clusterclass and removed kind/enhancement labels Jan 7, 2025
@anmazzotti anmazzotti force-pushed the deprecate_infrastructure_ref branch 4 times, most recently from d6c1173 to 2f60670 Compare January 7, 2025 14:51
@anmazzotti anmazzotti force-pushed the deprecate_infrastructure_ref branch 5 times, most recently from f41db9c to 384b0dc Compare January 9, 2025 11:02
@anmazzotti anmazzotti marked this pull request as ready for review January 9, 2025 11:57
@anmazzotti anmazzotti requested a review from a team as a code owner January 9, 2025 11:57
@anmazzotti anmazzotti force-pushed the deprecate_infrastructure_ref branch 3 times, most recently from 1d66023 to 4fe19da Compare January 10, 2025 07:04
@anmazzotti anmazzotti enabled auto-merge January 10, 2025 08:10
@anmazzotti anmazzotti self-assigned this Jan 10, 2025
@salasberryfin
Copy link
Contributor

@anmazzotti looks like this needs a rebase

@anmazzotti anmazzotti force-pushed the deprecate_infrastructure_ref branch from 4fe19da to 37dcf28 Compare January 10, 2025 10:01
@anmazzotti anmazzotti force-pushed the deprecate_infrastructure_ref branch 2 times, most recently from 5337d86 to e21a208 Compare January 10, 2025 11:13
Danil-Grigorev
Danil-Grigorev previously approved these changes Jan 10, 2025
Copy link
Contributor

@Danil-Grigorev Danil-Grigorev left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@furkatgofurov7
Copy link
Contributor

@anmazzotti I am afraid this PR needs a rebase

@anmazzotti anmazzotti force-pushed the deprecate_infrastructure_ref branch from e21a208 to 69c3c74 Compare January 10, 2025 12:29
furkatgofurov7
furkatgofurov7 previously approved these changes Jan 10, 2025
Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

LGTM

controlplane/api/v1beta1/rke2controlplane_webhook.go Outdated Show resolved Hide resolved
salasberryfin
salasberryfin previously approved these changes Jan 10, 2025
Danil-Grigorev
Danil-Grigorev previously approved these changes Jan 10, 2025
@anmazzotti anmazzotti merged commit 6c69383 into rancher:main Jan 10, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/clusterclass Issues or PRs related to clusterclass kind/bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Can not create Cluster from Cluster Class without repeating KCP's infrastructureRef
6 participants