Skip to content

⚠️ Add CAPD v1beta2 types #12226

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
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

⚠️ Add CAPD v1beta2 types #12226

wants to merge 9 commits into from
+9,158 −884

Conversation

sivchari
Copy link
Member

What this PR does / why we need it:

Part of #11947

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 #

/area api

@k8s-ci-robot k8s-ci-robot added area/api Issues or PRs related to the APIs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 16, 2025
@k8s-ci-robot k8s-ci-robot requested review from elmiko and JoelSpeed May 16, 2025 04:33
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 16, 2025
@sivchari sivchari force-pushed the add-capd-v1beta2 branch 3 times, most recently from 460f731 to 7d9165a Compare May 16, 2025 11:05
@sivchari
Copy link
Member Author

/retest

@sivchari sivchari force-pushed the add-capd-v1beta2 branch 3 times, most recently from 1b0013c to b7cd65f Compare May 16, 2025 13:28
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@sivchari sivchari force-pushed the add-capd-v1beta2 branch 2 times, most recently from 6f62bc7 to 4c79b74 Compare May 17, 2025 00:06
@sivchari
Copy link
Member Author

/cc @fabriziopandini @sbueringer @chrischdi

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 23, 2025
@sbueringer
Copy link
Member

Just merged the API move PR

@sivchari sivchari force-pushed the add-capd-v1beta2 branch from 32a61b5 to 920acaa Compare May 26, 2025 10:17
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2025
sbueringer
sbueringer reviewed May 26, 2025
View reviewed changes
test/extension/handlers/topologymutation/handler.go Outdated
@@ -38,8 +38,8 @@ import (
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
runtimehooksv1 "sigs.k8s.io/cluster-api/api/runtime/hooks/v1alpha1"
"sigs.k8s.io/cluster-api/exp/runtime/topologymutation"
infrav1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta1"
infraexpv1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/exp/api/v1beta1"
infrav1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/api/v1beta2"
Copy link
Member

Choose a reason for hiding this comment

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

This should stay on v1beta1 for now (similar the corresponding test file)

Please rebase after #12282 is merged

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2025
@sbueringer
Copy link
Member

@sivchari Sorry can you do another rebase? I'll try to find time to review this soon

@sivchari sivchari force-pushed the add-capd-v1beta2 branch from 587f90a to 70a949c Compare May 28, 2025 11:02
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2025
@sivchari sivchari force-pushed the add-capd-v1beta2 branch from 70a949c to e178b92 Compare May 28, 2025 12:26
@sivchari
Copy link
Member Author

/retest

fabriziopandini
fabriziopandini reviewed May 28, 2025
View reviewed changes
Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

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

@sivchari first of all thanks for this PR, this is really helping in easing some pressure on core maintainers.

Second, kudos! introducing an API version and setting up conversions is not easy, and you did it very well!

test/infrastructure/docker/api/v1beta2/.import-restrictions Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Note: might be worth to align import restrinctions to change introduced by #12302

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I'd deal with it after #12302 is merged.

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

test/infrastructure/docker/api/v1beta2/conversion.go
Comment on lines +19 to +22
func (*DockerCluster) Hub() {}
func (*DockerClusterTemplate) Hub() {}
func (*DockerMachine) Hub() {}
func (*DockerMachineTemplate) Hub() {}
Copy link
Member

Choose a reason for hiding this comment

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

Also DevCluster, DevMachine and corresponding templates needs conversion now

Copy link
Member

Choose a reason for hiding this comment

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

Note:
@sivchari in order to do so you need to implement Hub function in v1beta2, add convert func in v1beta1 (autogenerated conversion fuctions are already there) + tests cases for fuzz conversion test

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I implemented Spoke functions and added test cases for fuzzing.

@sivchari
Copy link
Member Author

/retest

sivchari added 7 commits May 29, 2025 18:36
@sivchari
add CAPD v1beta2
d8862c0 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari
add CAPD exp v1beta2
4f6f57f 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari
regenerate
16a45ea 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari
replace v1beta1 with v1beta2
2c53b10 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari
fix: webhook markers
6e65e9a 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari
re-generate conversion func
3dec3d9 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari
revert api verson to v1
8c53379 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari sivchari force-pushed the add-capd-v1beta2 branch from f8608f3 to 23bdf4e Compare May 29, 2025 10:17
sivchari added 2 commits May 29, 2025 19:37
@sivchari
Add DevCluster, DevMachine and corresponding Template to conversion
deffb5f 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari
restructure import-restrictions
f6269ed 
Signed-off-by: sivchari <shibuuuu5@gmail.com>
@sivchari sivchari force-pushed the add-capd-v1beta2 branch from 23bdf4e to f6269ed Compare May 29, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbueringer sbueringer sbueringer left review comments

@fabriziopandini fabriziopandini fabriziopandini left review comments

@elmiko elmiko Awaiting requested review from elmiko

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

@chrischdi chrischdi Awaiting requested review from chrischdi

Assignees
No one assigned
Labels
area/api Issues or PRs related to the APIs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sivchari @k8s-ci-robot @sbueringer @fabriziopandini