Skip to content

🌱 Bump Cluster API to v1.5.0-rc.0#186

Merged
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
Fedosin:capi15
Jul 21, 2023
Merged

🌱 Bump Cluster API to v1.5.0-rc.0#186
k8s-ci-robot merged 1 commit into
kubernetes-sigs:mainfrom
Fedosin:capi15

Conversation

@Fedosin

@Fedosin Fedosin commented Jul 10, 2023

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
This PR update CAPI operator to the latest Cluster API version.

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

@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 Jul 10, 2023
@Fedosin

Fedosin commented Jul 10, 2023

Copy link
Copy Markdown
Contributor Author

/hold

waiting for the new version to be released.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 10, 2023
@Fedosin Fedosin force-pushed the capi15 branch 4 times, most recently from e05ea81 to 8b612ab Compare July 10, 2023 22:37

@furkatgofurov7 furkatgofurov7 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Fedosin thanks for the PR, overall looks good to me, only one missing update I noticed based on the migration doc https://main.cluster-api.sigs.k8s.io/developer/providers/migrations/v1.4-to-v1.5.html#go-version and few more suggestions inline:

/retitle 🌱 Bump Cluster API to v1.5.0-beta.1

Comment thread go.mod Outdated
Comment thread go.mod
Comment thread internal/webhook/bootstrapprovider_webhook.go Outdated
Comment on lines -103 to -109
{
GroupVersion: "cert-manager.io/v1",
APIResources: []metav1.APIResource{
{Kind: "Certificate", Namespaced: true},
{Kind: "Issuer", Namespaced: true},
},
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why are we removing this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are 2 reasons for that:

  1. In clusterctl they explicitly declare that this function should not return cert-manager objects. It's better to keep things consistent with cluster-api.
  2. After upgrading to controller-runtime v0.15.0 some tests fail because they need cert-manager CRDs installed (I still don't understand why it works in the previous versions). I don't want to add cert-manager to the operator dependencies, so I just decided to remove the section.

Comment thread api/v1alpha1/provider_types.go Outdated
@k8s-ci-robot k8s-ci-robot changed the title 🌱 Bump Cluster API to v1.5 🌱 Bump Cluster API to v1.5.0-beta.1 Jul 11, 2023
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 12, 2023
@Fedosin

Fedosin commented Jul 12, 2023

Copy link
Copy Markdown
Contributor Author

I updated container images for e2e tests to make them work with golang 1.20: kubernetes/test-infra#30077

Comment thread Dockerfile Outdated
Comment thread Makefile Outdated
Comment thread cmd/main.go Outdated
Comment thread hack/ensure-go.sh Outdated
@furkatgofurov7

Copy link
Copy Markdown
Member

/retest

@furkatgofurov7

furkatgofurov7 commented Jul 13, 2023

Copy link
Copy Markdown
Member

waiting for the new version to be released.

We can always merge the rc bump and give the CI time to run longer with it to see if there are problems, so that whenever actual 1.5.0 comes out, it is just a matter of updating go.mod, nothing more.

@furkatgofurov7

Copy link
Copy Markdown
Member

/retitle 🌱 Bump Cluster API to v1.5.0-rc.0

@k8s-ci-robot k8s-ci-robot changed the title 🌱 Bump Cluster API to v1.5.0-beta.1 🌱 Bump Cluster API to v1.5.0-rc.0 Jul 13, 2023
@Fedosin

Fedosin commented Jul 13, 2023

Copy link
Copy Markdown
Contributor Author

@furkatgofurov7 I'm okay to merge #187 first and then I'll update my PR.

@furkatgofurov7

Copy link
Copy Markdown
Member

@furkatgofurov7 I'm okay to merge #187 first and then I'll update my PR.

@Fedosin sure, it should be ready to merge now, PTAL

@furkatgofurov7

Copy link
Copy Markdown
Member

@Fedosin #187 is merged, please rebase when you get a time, thanks!

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 13, 2023
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 14, 2023

@furkatgofurov7 furkatgofurov7 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for working on this @Fedosin!

/approve

@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: furkatgofurov7

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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 Jul 14, 2023

@alexander-demicev alexander-demicev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2023
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

LGTM label has been added.

DetailsGit tree hash: d2fdc8b097eec310ad623578af7077072fa9544c

@furkatgofurov7

Copy link
Copy Markdown
Member

/hold cancel

We will bump to v1.5 CAPI once out (fairly straightforward then this one), this we can take in now.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 21, 2023
@k8s-ci-robot k8s-ci-robot merged commit 1b9007d into kubernetes-sigs:main Jul 21, 2023
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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CAPI v1.5.0-beta.0 has been released and is ready for testing

4 participants