Skip to content

🌱 Bump CAPI to v1.10.2 #5447

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 10 commits into
base: main
Choose a base branch
from
Open

🌱 Bump CAPI to v1.10.2 #5447

wants to merge 10 commits into from

Conversation

tamalsaha
Copy link

@tamalsaha tamalsaha commented Mar 26, 2025 •
edited
Loading

What type of PR is this?
I fixed the validator and defaulter interface broken by controller-runtime.
util/defaulting package is also internal to cluster-api now. I copied that. Not sure what is the right answer on that one.

What this PR does / why we need it:

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 #

Special notes for your reviewer:

Checklist:

  • squashed commits
  • includes documentation
  • includes emoji in title
  • adds unit tests
  • adds or updates e2e tests

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority labels Mar 26, 2025
@k8s-ci-robot
Copy link
Contributor

Welcome @tamalsaha!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-aws 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-aws has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 26, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @tamalsaha. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 26, 2025
@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 fabriziopandini 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

richardcase
richardcase reviewed Apr 4, 2025
View reviewed changes
Copy link
Member

@richardcase richardcase left a comment

Choose a reason for hiding this comment

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

Thanks for this. This is really a CAPI bump and as such could you:

  • Retitle the PR to indicate its a CAPI bump
  • Update the CAPI versions in the e2e tests (in the e2e config files)
  • Ensure that the go versions are updated in things like the devbox config file.

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

@tamalsaha - just tagging you in case you didn't see the feedback.

@nrb nrb linked an issue Apr 14, 2025 that may be closed by this pull request
Remove deprecated admission interfaces from API types #5450
Open
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2025
@tamalsaha tamalsaha changed the title Use k8s 1.32 client libs Bump CAPI to v1.10.0-rc.0 Apr 15, 2025
@tamalsaha tamalsaha changed the title Bump CAPI to v1.10.0-rc.0 🌱 Bump CAPI to v1.10.0-rc.0 Apr 15, 2025
@tamalsaha
Copy link
Author

tamalsaha commented Apr 15, 2025 •
edited
Loading

I am back after kubecon. I have made the changes as I understood. I think the internal/webhook refactoring is better done in a separate pr and preferrably by project maintainers as this will result in a very large pr.

Let me know if any other change is needed to make this ready for merging.

@tamalsaha tamalsaha force-pushed the k132 branch 2 times, most recently from 21f63b9 to d1faab0 Compare April 16, 2025 00:31
@tamalsaha tamalsaha force-pushed the k132 branch 2 times, most recently from 03810a1 to 5860551 Compare April 25, 2025 18:09
@tamalsaha tamalsaha changed the title 🌱 Bump CAPI to v1.10.0-rc.0 🌱 Bump CAPI to v1.10.0 Apr 25, 2025
@tamalsaha tamalsaha force-pushed the k132 branch 2 times, most recently from e4f0ae9 to 6a875b2 Compare April 25, 2025 19:40
@tamalsaha
Copy link
Author

@richardcase , can you please take another look?

@fiunchinho
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Apr 28, 2025
@damdo
Copy link
Member

damdo commented May 8, 2025

/test pull-cluster-api-provider-aws-e2e

@damdo
Copy link
Member

damdo commented May 8, 2025

Yeah so this one EKS e2e failure:

      <errors.aggregate | len:1, cap:1>: 
      MachinePool.cluster.x-k8s.io "eks-nodes-efd528-pool-0" is invalid: spec.template.spec.bootstrap.dataSecretName: Invalid value: "": spec.template.spec.bootstrap.dataSecretName in body should be at least 1 chars long

Is kubernetes-sigs/cluster-api#12164

@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 10, 2025
@damdo
Copy link
Member

damdo commented May 12, 2025

/test pull-cluster-api-provider-aws-e2e

@damdo
Copy link
Member

damdo commented May 12, 2025

@tamalsaha this will need rebasing.

Regarding the outstanding things:

@damdo damdo added this to the v2.9 milestone May 13, 2025
@damdo
Copy link
Member

damdo commented May 20, 2025

@tamalsaha CAPI v1.10.2 is out
https://github.com/kubernetes-sigs/cluster-api/releases/tag/v1.10.2

@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 20, 2025
@tamalsaha tamalsaha changed the title 🌱 Bump CAPI to v1.10.1 🌱 Bump CAPI to v1.10.2 May 20, 2025
@tamalsaha
Copy link
Author

rebased, updated

@damdo
Copy link
Member

damdo commented May 21, 2025

@tamalsaha

@damdo
Copy link
Member

damdo commented May 21, 2025

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-eks

@tamalsaha
Copy link
Author

@tamalsaha

😃 fixed!

@damdo
Copy link
Member

damdo commented May 21, 2025

/test pull-cluster-api-provider-aws-e2e
/test pull-cluster-api-provider-aws-e2e-eks

@k8s-ci-robot
Copy link
Contributor

@tamalsaha: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-cluster-api-provider-aws-e2e-eks a850819 link false /test pull-cluster-api-provider-aws-e2e-eks
pull-cluster-api-provider-aws-e2e a850819 link false /test pull-cluster-api-provider-aws-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

damdo
damdo reviewed May 22, 2025
View reviewed changes
test/e2e/data/e2e_conf.yaml Outdated
EXP_BOOTSTRAP_FORMAT_IGNITION: "true"
EXP_KUBEADM_BOOTSTRAP_FORMAT_IGNITION: "true"
GC_WORKLOAD: "../../data/gcworkload.yaml"
CLASSICELB_TEST_KUBERNETES_VERSION_FROM: "v1.29.9"
CLASSICELB_TEST_KUBERNETES_VERSION_TO: "v1.30.8"
CLASSICELB_TEST_KUBERNETES_VERSION_TO: "v1.31.0"
Copy link
Member

Choose a reason for hiding this comment

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

This is causing an error:

Message: "Forbidden: cannot update Kubernetes version from v1.29.9 to v1.31.0",

Let's not bump this, as this is required to test the v1.29 -> v1.30 upgrade behaviour for the deprecated Classic ELBs.

Suggested change
CLASSICELB_TEST_KUBERNETES_VERSION_TO: "v1.31.0"
CLASSICELB_TEST_KUBERNETES_VERSION_TO: "v1.30.8" # keep this as v1.30.z

@damdo
Copy link
Member

damdo commented May 22, 2025

@tamalsaha Also we need to update all the DumpAllResourcesInput in the repo to pass in the KubeConfigPath as that's now a required field in the input struct.

Context: this error:

  [FAILED] input.KubeConfigPath is required for DumpAllResources

tamalsaha added 10 commits May 23, 2025 02:25
@tamalsaha
Bump CAPI to v1.10.0
b146916 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
Fix e2e tests
06e7d45 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
make generate
abf7b00 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
fix linter warning
ade3035 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
fix tests
cfb2048 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
bump to v1.10.1
0d16b9f 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
Update components
bbbd15b 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
update config
71c0c98 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
capi 1.10.2
a68bfc3 
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha
fix
1519eac 
Signed-off-by: Tamal Saha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardcase richardcase richardcase left review comments

@nrb nrb nrb left review comments

@damdo damdo damdo left review comments

@cnmcavoy cnmcavoy Awaiting requested review from cnmcavoy

@serngawy serngawy Awaiting requested review from serngawy

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Milestone
v2.9
Development

Successfully merging this pull request may close these issues.

Remove deprecated admission interfaces from API types
6 participants
@tamalsaha @k8s-ci-robot @richardcase @fiunchinho @nrb @damdo