-
Notifications
You must be signed in to change notification settings - Fork 284
🐛 Bump CAPI version to fix cert-manager URL #1169
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
🐛 Bump CAPI version to fix cert-manager URL #1169
Conversation
|
✔️ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready! 🔨 Explore the source changes: 41c1d6d 🔍 Inspect the deploy log: https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/62288b5e3ef0fd000860bb3a 😎 Browse the preview: https://deploy-preview-1169--kubernetes-sigs-cluster-api-openstack.netlify.app |
|
Welcome @lentzi90! |
|
Hi @lentzi90. 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/test-infra repository. |
74c4dcf to
b74fb5f
Compare
|
/ok-to-test |
b74fb5f to
46093c6
Compare
|
Original fix was this one: https://github.com/kubernetes-sigs/cluster-api/pull/6050/files Fix on capo main was this one: #1136 So I was wrong with the requirement to bump |
Yeah I realized when the test failed that I needed to also update CAPI, not just the test. I guess this is because the error is coming from clusterctl, which is in CAPI (not part of the test package). Hoping it will pass this time 🤞 |
|
I think they should be in sync so lets see how the test goes now 👍 Forgot to add prior to this: thank you for tackling this and the contribution 🎉 |
|
Hmm now I'm a bit lost. Still the same error. 🤔 |
|
See #1133 which I kind of abandoned when it seemed we weren't going to use it. You'll need to bump the versions in |
46093c6 to
f43d67e
Compare
|
Although you might need to bump them to 1.0.5 rather than 1.0.4, which my older patch did. Note that my PR wasn't working yet. |
f43d67e to
acf98d3
Compare
|
Thanks @mdbooth ! I realized there was a |
|
Yes, the old test did still ues the old version, visible in the logs via: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_cluster-api-provider-openstack/1169/pull-cluster-api-provider-openstack-e2e-test/1501477514122366976#1:build-log.txt%3A1485: 🤞 |
acf98d3 to
41c1d6d
Compare
|
Missed some references in the Makefile also. |
|
Green tests 🎉 /lgtm |
|
We should probably fix OWNERS in the 0.5 branch if we're going to keep maintaining it. /assign @jichenjc |
seanschneeweiss
left a comment
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.
/lgtm
|
/hold cancel |
|
lending an approve until the other PR is merged :) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lentzi90, mdbooth, 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 |
do you mind add corresponding approvers through PR then we can update it ? Thanks @mdbooth |
|
@sbueringer thanks :) |
What this PR does / why we need it:
The e2e tests are broken for release-0.5. Probably due to the organization change for cert-manager. By bumping the CAPI version this should be fixed.
Ref. slack discussion.
Special notes for your reviewer:
TODOs:
/hold