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 knative/pkg to release-1.6 #4928

Merged
merged 3 commits into from
Aug 10, 2022

Conversation

vdemeester
Copy link
Member

@vdemeester vdemeester commented Jun 2, 2022

Changes

This also updates it in ./hack/update-deps.sh to be inline with what
we fetch (before this PR, the VERSION there is not the one we depend
on / vendor).

Signed-off-by: Vincent Demeester [email protected]

/kind misc

This will bring up the minimum version for Kubernetes to 1.22, so we will need to bump the version of k8s we provision with boskos.

/cc @imjasonh @afrittoli @jerop @abayer

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in
    (if there are no user facing changes, use release note "NONE")

Release Notes

Bump knative/pkg dependency to 1.15.
action required: this will bring up the minimum version for Kubernetes to 1.22

@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/misc Categorizes issue or PR as a miscellaneuous one. labels Jun 2, 2022
@tekton-robot tekton-robot requested a review from dibyom June 2, 2022 08:15
@tekton-robot tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 2, 2022
@tekton-robot tekton-robot requested a review from dlorenc June 2, 2022 08:15
@vdemeester
Copy link
Member Author

/cc @imjasonh @mattmoor

@tekton-robot tekton-robot requested review from imjasonh and mattmoor June 2, 2022 08:16
@vdemeester
Copy link
Member Author

/retest

@tekton-robot tekton-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 2, 2022
@abayer
Copy link
Contributor

abayer commented Jun 2, 2022

Once we get boskos provisioning 1.22 and the various other clusters upgraded, this should be fine. =)

@vdemeester
Copy link
Member Author

Once we get boskos provisioning 1.22 and the various other clusters upgraded, this should be fine. =)

I need to find out how we bump the version boskos provides.. 😅

@vdemeester vdemeester closed this Jun 2, 2022
@vdemeester vdemeester reopened this Jun 2, 2022
@vdemeester
Copy link
Member Author

arg… wrong button 🤦🏼

@vdemeester
Copy link
Member Author

/retest

@vdemeester
Copy link
Member Author

vdemeester commented Jul 15, 2022

@abayer @pritidesai @afrittoli Should we wait for 0.38 to be released to bump this ?
(imo, I think so)

go.mod Outdated
@@ -27,7 +27,7 @@ require (
k8s.io/code-generator v0.23.5
k8s.io/klog v1.0.0
k8s.io/kube-openapi v0.0.0-20220124234850-424119656bbf
knative.dev/pkg v0.0.0-20220329144915-0a1ec2e0d46c
knative.dev/pkg v0.0.0-20220524202603-19adf798efb8
Copy link
Member

Choose a reason for hiding this comment

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

Should update this to @main again, and fix changeset.Get calls accordingly. This will require us to build with Go 1.18+

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Sorry you're right, good point. We should wait for the next knative release that includes the changeset.Get changes, and bump to use that.

Copy link
Member

Choose a reason for hiding this comment

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

This could also be two changes: one to bump to 1.5, and a future one (which I can do if you want) to bump to 1.6 and include changeset.Get goodness.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I am fine with both, depends on when this one gets merged 😝

Copy link
Member

Choose a reason for hiding this comment

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

FWIW knative/pkg release-1.6 is out: https://github.com/knative/pkg/tree/release-1.6

1.7 should include the changeset changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yay I will update to 1.6 then 😇

@abayer
Copy link
Contributor

abayer commented Jul 22, 2022

We're still waiting for 0.38.0, fwiw. =)

@vdemeester
Copy link
Member Author

We're still waiting for 0.38.0, fwiw. =)

didn't we already decide which commit to use for 0.38.0 (and even created a release branch from it) ? 😅

This also updates it in `./hack/update-deps.sh` to be inline with what
we fetch (before this PR, the VERSION there is not the one we depend
on / vendor).

Signed-off-by: Vincent Demeester <[email protected]>
This updates all references of Kubernetes 1.21 as necessary,
including the command for setting up K8s cluster.

Signed-off-by: Vincent Demeester <[email protected]>
@vdemeester vdemeester changed the title Bump knative/pkg to release-1.5 Bump knative/pkg to release-1.6 Jul 26, 2022
Signed-off-by: Vincent Demeester <[email protected]>
@vdemeester
Copy link
Member Author

/retest

@abayer
Copy link
Contributor

abayer commented Jul 26, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 26, 2022
@vdemeester
Copy link
Member Author

/cc @afrittoli @lbernick

@lbernick
Copy link
Member

lbernick commented Aug 3, 2022

I don't have much context here:

  • what was the reason to wait until 0.38?
  • have concerns about bumping the min k8s version been resolved?
  • do we need knative 1.7 to pick up the changeset changes Jason mentioned or is 1.6 sufficient?

@vdemeester
Copy link
Member Author

  • what was the reason to wait until 0.38?

Bumping min kubernetes version was the reason to wait for 0.38 👼🏼

  • have concerns about bumping the min k8s version been resolved?

Yes, the CI is green now 😛

  • do we need knative 1.7 to pick up the changeset changes Jason mentioned or is 1.6 sufficient?

Nope, 1.6 is sufficient 👼🏼

@lbernick
Copy link
Member

lbernick commented Aug 4, 2022

Do we have any developer docs on bumping our min k8s version? It seems backwards incompatible even if it's not exactly part of the api so just wondering if we have any policies around this.

@afrittoli
Copy link
Member

afrittoli commented Aug 9, 2022

Do we have any developer docs on bumping our min k8s version? It seems backwards incompatible even if it's not exactly part of the api so just wondering if we have any policies around this.

It's part of the release notes and of the top level README.
We should have a more formal statement about k8s min version changes.
Let's check with the knative team if there is a policy on their side that we can piggy back on.

Table of k8s version: https://github.com/tektoncd/pipeline#required-kubernetes-version
We currently document min version only, once we have k8s matrix test in place we could document max version (as long as we test older branches for that).

It would be good to have a process to trace k8s deprecations.

@afrittoli afrittoli added this to the Pipelines v0.39 milestone Aug 9, 2022
@abayer
Copy link
Contributor

abayer commented Aug 10, 2022

cc @tektoncd/core-maintainers - does anyone object to merging this?

EDIT: And if not, could someone approve? =)

@lbernick
Copy link
Member

@XinruZhang opened #5300 to discuss our general process for k8s bumps (thanks Xinru!) and I don't think there are any immediately relevant concerns for this one specifically.

/approve

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 10, 2022
@tekton-robot tekton-robot merged commit 94a1ba6 into tektoncd:main Aug 10, 2022
@vdemeester vdemeester deleted the bump-knative-pkg branch August 18, 2022 13:36
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. kind/misc Categorizes issue or PR as a miscellaneuous one. lgtm Indicates that a PR is ready to be merged. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. 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.

6 participants