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

🌱 MachinePool: automatically bump apiVersions like in our other controllers #7995

Merged

Conversation

sbueringer
Copy link
Member

@sbueringer sbueringer commented Jan 25, 2023

Signed-off-by: Stefan Büringer buringerst@vmware.com

What this PR does / why we need it:
Today the MachinePool controller is our only controller that doesn't automatically bump the apiVersions in references.
This PR makes the controller consistent with our other controllers.

Please note, if infra or bootstrap providers (e.g. CABPK) drop api versions (like v1alpha3/v1alpha4) without this change existing MachinePools might break and manual user intervention is required.

Example:

  1. MachinePool has been created with a reference to v1alpha3.KubeadmConfig
  2. MachinePool controller doesn't bump the ref
  3. Cluster API v1.x drops v1alpha3
  4. MachinePool controller breaks as v1alpha3 doesn't exist anymore
  5. User has to fix the MachinePool manually by upgrading the apiVersion

A comment about ref bumps in other controllers:

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 #6778

controllers

Signed-off-by: Stefan Büringer buringerst@vmware.com
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 25, 2023
@sbueringer
Copy link
Member Author

@fabriziopandini
Copy link
Member

Thanks for addressing this issue!
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 277f7958dc31e018f4a9afa63783403af31cabce

@CecileRobertMichon
Copy link
Contributor

/assign @Jont828 @mboersma

@sbueringer
Copy link
Member Author

sbueringer commented Jan 31, 2023

@killianmuldoon Can you please take a look if you have some time? The prod-code change is basically only to add the ref bump, everything else is adjusting the tests accordingly

Thank you very much!

@sbueringer
Copy link
Member Author

I'll merge given lgtm above.

Please let me know if you have further comments. Happy to open a follow-up PR.

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbueringer

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2023
@sbueringer
Copy link
Member Author

/retest

@k8s-ci-robot k8s-ci-robot merged commit 6f9e69a into kubernetes-sigs:main Feb 3, 2023
@sbueringer sbueringer deleted the pr-fix-apiversion-bump-in-mp branch February 3, 2023 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", 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
Development

Successfully merging this pull request may close these issues.

MachineDeployment refs and ownerRefs are not upgraded after CAPI upgrade with apiVersion bump
7 participants