Skip to content
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 cert-manager to v1.15.0 #10727

Conversation

troy0820
Copy link
Member

@troy0820 troy0820 commented Jun 5, 2024

What this PR does / why we need it:
Updates cert-manager to v1.15.0

Which issue(s) this PR fixes: #10726

/area clusterctl

@k8s-ci-robot k8s-ci-robot added area/clusterctl Issues or PRs related to clusterctl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 5, 2024
@chrischdi
Copy link
Member

/test help

@k8s-ci-robot
Copy link
Contributor

@chrischdi: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

  • /test pull-cluster-api-build-main
  • /test pull-cluster-api-e2e-blocking-main
  • /test pull-cluster-api-e2e-conformance-ci-latest-main
  • /test pull-cluster-api-e2e-conformance-main
  • /test pull-cluster-api-e2e-main
  • /test pull-cluster-api-e2e-mink8s-main
  • /test pull-cluster-api-e2e-upgrade-1-30-1-31-main
  • /test pull-cluster-api-test-main
  • /test pull-cluster-api-test-mink8s-main
  • /test pull-cluster-api-verify-main

The following commands are available to trigger optional jobs:

  • /test pull-cluster-api-apidiff-main

Use /test all to run the following jobs that were automatically triggered:

  • pull-cluster-api-apidiff-main
  • pull-cluster-api-build-main
  • pull-cluster-api-e2e-blocking-main
  • pull-cluster-api-test-main
  • pull-cluster-api-verify-main

In response to this:

/test help

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.

@chrischdi
Copy link
Member

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-30-1-31-main

@troy0820 troy0820 force-pushed the troy0820/update-cert-manager-1.15.0 branch from a865360 to 3ba551f Compare June 5, 2024 16:03
Comment on lines 433 to 434
ms := ms

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe we will backport this but if we do, this may need to be there if 1.7.x and 1.6.x use anything less than go1.22.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was done to pass the linting:
internal/controllers/machinedeployment/mdutil/util.go:433:3: The copy of the 'for' variable "ms" can be deleted (Go 1.22+) (copyloopvar)

Copy link
Member Author

Choose a reason for hiding this comment

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

If there is a need to backport this, I can do this off the branch where it will not impact the linting rules at that branch go version (e.g. go1.21)

Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why this comes up at this (unrelated to this file) pull request 🤔

It should not be part of this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I'll open a PR to fix that separately.

Copy link
Member

Choose a reason for hiding this comment

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

xref: #10731

Copy link
Member Author

Choose a reason for hiding this comment

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

My concern with the removal of this to fix the linting, we will not have that loopvar in go1.21 & go1.20 (on the backports 1.6 & 1.7) where it may be necessary to have that within the scope of the loop (which 1.22 fixes).

Even rebasing with the xref #10731 will still carry that in to the older releases which may impact how that works. @chrischdi

Copy link
Member

Choose a reason for hiding this comment

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

With merging #10731 and rebasing, this PR won't have the change to internal/controllers/machinedeployment/mdutil/util.go anymore and the cherry-picks should just go well. If there's still something missing then, then we have to do a manual cherry-pick I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been rebased with main so I don't carry the linting change to the backports. Thanks @chrischdi

Copy link
Member

@chrischdi chrischdi 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 tackling this, lgtm for me, but not adding it because of the util.go thing.

We definetly want to backport it to v1.7 and v1.6.

@chrischdi
Copy link
Member

/cherry-pick release-1.7

@k8s-infra-cherrypick-robot

@chrischdi: once the present PR merges, I will cherry-pick it on top of release-1.7 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.7

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.

@chrischdi
Copy link
Member

/cherry-pick release-1.6

@k8s-infra-cherrypick-robot

@chrischdi: once the present PR merges, I will cherry-pick it on top of release-1.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.6

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.

@troy0820
Copy link
Member Author

troy0820 commented Jun 6, 2024

Thanks for tackling this, lgtm for me, but not adding it because of the util.go thing.

We definetly want to backport it to v1.7 and v1.6.

I can add it back but linting will fail, would the override be something we are willing to do? @chrischdi

@chrischdi
Copy link
Member

Thanks for tackling this, lgtm for me, but not adding it because of the util.go thing.
We definetly want to backport it to v1.7 and v1.6.

I can add it back but linting will fail, would the override be something we are willing to do? @chrischdi

We should wait for #10731 to merge.

Signed-off-by: Troy Connor <troy0820@users.noreply.github.com>
@troy0820 troy0820 force-pushed the troy0820/update-cert-manager-1.15.0 branch from 3ba551f to ea1dfcc Compare June 6, 2024 16:23
@troy0820 troy0820 requested a review from chrischdi June 6, 2024 16:23
@troy0820
Copy link
Member Author

troy0820 commented Jun 6, 2024

Thanks for tackling this, lgtm for me, but not adding it because of the util.go thing.
We definetly want to backport it to v1.7 and v1.6.

I can add it back but linting will fail, would the override be something we are willing to do? @chrischdi

We should wait for #10731 to merge.

Merged and rebased with main to not carry the linting change in the backport.

Copy link
Member

@chrischdi chrischdi left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: dcf92cff199eb44f4f9ec1a6d71e5758c4547d0e

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi

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 /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 Jun 6, 2024
@chrischdi
Copy link
Member

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-30-1-31-main

just to make sure

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Jun 6, 2024

@troy0820: The following test 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-apidiff-main ea1dfcc link false /test pull-cluster-api-apidiff-main

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.

@k8s-ci-robot k8s-ci-robot merged commit 1dccc9a into kubernetes-sigs:main Jun 6, 2024
25 of 26 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.8 milestone Jun 6, 2024
@k8s-infra-cherrypick-robot

@chrischdi: new pull request created: #10732

In response to this:

/cherry-pick release-1.7

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-infra-cherrypick-robot

@chrischdi: new pull request created: #10733

In response to this:

/cherry-pick release-1.6

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.

@sbueringer
Copy link
Member

Just fyi. I'm not concerned about having to do a manual cherry-pick (e.g. to not backport linter fixes)

@sbueringer
Copy link
Member

/lgtm

(also double checked the release notes, all good)

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. area/clusterctl Issues or PRs related to clusterctl 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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump cert-manager to v1.15.0
5 participants