-
Notifications
You must be signed in to change notification settings - Fork 440
Bump CAPI to v1.10.0-rc.0 #5517
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
base: main
Are you sure you want to change the base?
Conversation
Welcome @tamalsaha! |
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 Once the patch is verified, the new status will be reflected by the 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
5000+ changes?! 😱 |
Whatever The real change is the fixes needed for the Validator and Defaulter api breakage by the controller-runtime. |
Thanks for the update! Just an FYI: Cluster API Provider Azure doesn’t vendor its dependencies, we rely on Go modules and our |
Removed the vendor folder. PTAL. Just curious why the vendor folder is not added to the .gitignore. |
|
||
// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type. | ||
func (r *AzureMachineTemplate) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { | ||
func (_ *azureMachineTemplateWebhook) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code on the left looks suspect. Some lines modify the receiver object, some lines modify the input parameter obj. Now, all lines modify the input parameter.
13b70ca
to
d360b1e
Compare
/ok-to-test |
I did not find a historical reason for this, so I cannot really guess either. |
go.mod
Outdated
go 1.23.2 | ||
|
||
toolchain go1.22.12 | ||
toolchain go1.24.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CAPZ's go version updates are driven by CAPI minor updates.
Currently CAPZ main is at CAPI v1.9.x. Once we merge CAPI v1.10.0, we can review this PR (Reference: https://github.com/kubernetes-sigs/cluster-api/blob/release-1.10/go.mod#L3)
/hold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sigs.k8s.io/yaml v1.4.0 // indirect | ||
) | ||
|
||
replace github.com/Azure/azure-service-operator/v2 => github.com/Azure/azure-service-operator/v2 v2.12.1-0.20250328191251-ba00156bcd68 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I don't fully understand this. Is there a specific reason to be pinning github.com/Azure/azure-service-operator/v2 v1.12.0
to a specific SHA v2.12.1-0.20250328191251-ba00156bcd68
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, does updating ASO version also require us to update API Versions at CAPZ, @nojnhuh ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally should not be pinning to untagged commits for ASO because we also pull in their YAML manifests and CRDs from tagged releases. I don't think those are available for every commit. The ones from the closest release might work, but we we don't need to be in the business of verifying those things ourselves. Let's wait to update ASO until they tag a release with the new controller-runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The kubebuilder api breakage also affects the ASO repo. This is the first commit where this was fixed.
Azure/azure-service-operator#4646
Once they have a proper tag that should be used. But there is no tag since ASO pr was merged.
Signed-off-by: Tamal Saha <[email protected]>
@tamalsaha: The following tests failed, say
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. |
What type of PR is this?
What this PR does / why we need it:
As the title suggests, I am updating k/k libs. Requires Azure/azure-service-operator#4646
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:
TODOs:
Release note: