Skip to content

Bump releases#553

Merged
irbekrm merged 2 commits intocert-manager:masterfrom
irbekrm:bump_release
Aug 12, 2021
Merged

Bump releases#553
irbekrm merged 2 commits intocert-manager:masterfrom
irbekrm:bump_release

Conversation

@irbekrm
Copy link
Contributor

@irbekrm irbekrm commented Aug 11, 2021

This PR:

  • bumps cert-manager release tags as we just released v1.5
  • run presubmits against Kubernetes v1.22 by default (before it was v1.21)
  • adds a periodic for release next to run tests on Kubernetes v1.22

Signed-off-by: irbekrm irbekrm@gmail.com

Due to cert-manager 1.5 being released

Signed-off-by: irbekrm <irbekrm@gmail.com>
@jetstack-bot jetstack-bot added dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. area/cert-manager Indicates a PR related to cert-manager approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 11, 2021
@irbekrm
Copy link
Contributor Author

irbekrm commented Aug 11, 2021

/release-note-none

@jetstack-bot jetstack-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 11, 2021
wallrj
wallrj previously approved these changes Aug 11, 2021
Copy link
Member

@wallrj wallrj left a comment

Choose a reason for hiding this comment

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

There was a bit more to the PR last you did this: https://github.com/jetstack/testing/pull/515/files

but I think that was because we also added K8S 1.21 testing at the same time.

/approve
/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2021
Copy link
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

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

Do we want to add 1.22 testing for all PRs against master in this change too? I think it'd be valuable; feel free to unhold if we should do that in a different PR! (EDIT: or if we don't want to do that 😁 )

(If we don't change it here, we'll need to change the supported releases page, which I'll keep an eye on)

/hold

@jetstack-bot jetstack-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 11, 2021
@irbekrm
Copy link
Contributor Author

irbekrm commented Aug 11, 2021

Do we want to add 1.22 testing for all PRs against master in this change too? I think it'd be valuable; feel free to unhold if we should do that in a different PR! (EDIT: or if we don't want to do that grin )

I wanted to get cert-manager/cert-manager#4341 merged first

Also adds an e2e test against Kubernetes v1.22 to release-next periodics and bump the version of Kubernetes for upgrade tests and tests that focus on Venafi TPP/Venafi Cloud to v1.22

Signed-off-by: irbekrm <irbekrm@gmail.com>
@jetstack-bot jetstack-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 12, 2021
@irbekrm
Copy link
Contributor Author

irbekrm commented Aug 12, 2021

Do we want to add 1.22 testing for all PRs against master in this change too?

Thanks @SgtCoDFish I've updated PR.

Copy link
Member

@SgtCoDFish SgtCoDFish left a comment

Choose a reason for hiding this comment

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

/lgtm

I think there'd be some value to us running both v1.21 and v1.22 e2e tests on every PR, but I don't think it's required and given the flakiness we've seen from our tests, it might well do more harm than good to test against 2 versions every time 😅

After this is merged, I'll update this page to reflect exactly which versions we test against.

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2021
@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: irbekrm, SgtCoDFish, wallrj

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:
  • OWNERS [SgtCoDFish,irbekrm,wallrj]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@irbekrm
Copy link
Contributor Author

irbekrm commented Aug 12, 2021

I think there'd be some value to us running both v1.21 and v1.22 e2e tests on every PR, but I don't think it's required

Yeah I was thinking about that too. Not strongly convinced either way, but we are also resource limited for test nodes 😿

Maybe I will just unhold this, running both if we decide so would be just a matter of changing two lines, shout if you disagree

/hold cancel

@jetstack-bot jetstack-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 2021
@irbekrm
Copy link
Contributor Author

irbekrm commented Aug 12, 2021

FWIW I don't understand why the PRs against this repo don't get automatically merged.
As I understand it is Prow's tide component that should merge this and looking at its config it seems to be that this is configured against this repo correctly.

@irbekrm
Copy link
Contributor Author

irbekrm commented Aug 12, 2021

Seems like some unhappiness in Tide:

{"base-sha":"7c81219cd37d6248ee47f0f07856bd1700feb49a","branch":"master","component":"tide","controller":"sync","error":"status code 422 not one of [201], body: {\"message\":\"Validation Failed\",\"errors\":[{\"resource\":\"Status\",\"code\":\"custom\",\"message\":\"This SHA and context has reached the maximum number of statuses.\"}],\"documentation_url\":\"https://docs.github.com/rest/reference/repos#create-a-commit-status\"}","file":"prow/tide/tide.go:1197","func":"k8s.io/test-infra/prow/tide.(*Controller).mergePRs","level":"error","merge-targets":[528,529],"msg":"Unable to set tide context to SUCCESS.","org":"jetstack","pr":529,"repo":"testing","severity":"error","sha":"8fbbfe5e94f828ffcb5a1bda400ad2132be239f0","time":"2021-08-12T14:11:42Z"}
{"action":"MERGE_BATCH","base-sha":"7c81219cd37d6248ee47f0f07856bd1700feb49a","branch":"master","component":"tide","controller":"sync","file":"prow/tide/tide.go:1625","func":"k8s.io/test-infra/prow/tide.(*Controller).syncSubpool","level":"info","msg":"Subpool synced.","org":"jetstack","repo":"testing","severity":"info","targets":[528,529],"time":"2021-08-12T14:11:42Z"}
{"base-sha":"7c81219cd37d6248ee47f0f07856bd1700feb49a","branch":"master","component":"tide","controller":"sync","error":"failed merging [528 529] from batch [528 529]: status code 422 not one of [201], body: {\"message\":\"Validation Failed\",\"errors\":[{\"resource\":\"Status\",\"code\":\"custom\",\"message\":\"This SHA and context has reached the maximum number of statuses.\"}],\"documentation_url\":\"https://docs.github.com/rest/reference/repos#create-a-commit-status\"}","file":"prow/tide/tide.go:425","func":"k8s.io/test-infra/prow/tide.(*Controller).Sync.func2","level":"error","msg":"Error syncing subpool.","org":"jetstack","repo":"testing","severity":"error","time":"2021-08-12T14:11:42Z"}

I see that a similar issue has been fixed a while ago (and we are running a tide version with that fix). Perhaps something to keep in mind and investigate in case PR merging breaks for cert-manager as well.

@irbekrm irbekrm merged commit e67b331 into cert-manager:master Aug 12, 2021
@jetstack-bot
Copy link
Contributor

@irbekrm: Updated the following 3 configmaps:

  • plugins configmap in namespace default at cluster default using the following files:
    • key plugins.yaml using file config/plugins.yaml
  • config configmap in namespace default at cluster default using the following files:
    • key config.yaml using file config/config.yaml
  • job-config configmap in namespace default at cluster default using the following files:
    • key cert-manager-periodics.yaml using file config/jobs/cert-manager/cert-manager-periodics.yaml
    • key cert-manager-presubmits.yaml using file config/jobs/cert-manager/cert-manager-presubmits.yaml
    • key cert-manager-release-next-periodics.yaml using file config/jobs/cert-manager/release-next/cert-manager-release-next-periodics.yaml
    • key cert-manager-release-previous-periodics.yaml using file config/jobs/cert-manager/release-previous/cert-manager-release-previous-periodics.yaml
    • key cert-manager-release-previous-presubmits.yaml using file config/jobs/cert-manager/release-previous/cert-manager-release-previous-presubmits.yaml
Details

In response to this:

This PR:

  • bumps cert-manager release tags as we just released v1.5
  • run presubmits against Kubernetes v1.22 by default (before it was v1.21)
  • adds a periodic for release next to run tests on Kubernetes v1.22

Signed-off-by: irbekrm irbekrm@gmail.com

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.

@irbekrm irbekrm deleted the bump_release branch August 12, 2021 14:25
SgtCoDFish added a commit to SgtCoDFish/cert-manager-website that referenced this pull request Aug 12, 2021
this follows cert-manager/testing#553 being merged

Signed-off-by: Ashley Davis <ashley.davis@jetstack.io>
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/cert-manager Indicates a PR related to cert-manager dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants