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.

Sign up for GitHub

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

Jump to bottom

Bump Knative/K8s dependencies. #784

Merged
merged 1 commit into from
Oct 7, 2020
Merged

Bump Knative/K8s dependencies. #784

merged 1 commit into from
Oct 7, 2020

Conversation

mattmoor
Copy link
Member

@mattmoor mattmoor commented Oct 3, 2020

Changes

This incorporates the breaking changes from K8s 1.18 client libs through triggers.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide for more details.

Release Notes

action required: Users of the generated clients will need to conform to the K8s 1.18 style breaking changes (namely passing context.Context to all calls)

/hold

@tekton-robot tekton-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. labels Oct 3, 2020
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 3, 2020
@mattmoor
Copy link
Member Author

mattmoor commented Oct 3, 2020

This is based on tektoncd/pipeline#3319 so it shouldn't merge until that lands.

@mattmoor mattmoor mentioned this pull request Oct 3, 2020
Merged
4 tasks
@mattmoor
Copy link
Member Author

mattmoor commented Oct 3, 2020 

tekton-triggers-controller-5778bfc4c6-nvbzk    0/1   CrashLoopBackOff   5     5m17s

I'll have to peek at what's up.

@mattmoor mattmoor mentioned this pull request Oct 3, 2020
Merged
4 tasks
@mattmoor
Copy link
Member Author

mattmoor commented Oct 3, 2020

Well, that was easy, there was a redundant klog.InitFlags(). Hopefully that's it 🤞

@mattmoor
Copy link
Member Author

mattmoor commented Oct 6, 2020

/retest

I will try to repro the above failure locally, since it doesn't look like a flake failure mode, but why not let Prow do some of the heavy lifting for me 😉

@mattmoor
Copy link
Member Author

mattmoor commented Oct 6, 2020

Ah golden strings 🙃

@mattmoor
Bump Knative/K8s dependencies.
a720266 
This incorporates the breaking changes from K8s 1.18 client libs through triggers.
@mattmoor
Copy link
Member Author

mattmoor commented Oct 6, 2020

/assign @bobcatfish
/unhold

cc @vdemeester @imjasonh

This should be RFAL. I'm going to update my CLI PR next

@mattmoor
Copy link
Member Author

mattmoor commented Oct 6, 2020

/assign @dibyom

@mattmoor
Copy link
Member Author

mattmoor commented Oct 6, 2020

/unhold

Whoops forgot the hold.

@vdemeester
Copy link
Member

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2020
@mattmoor
Copy link
Member Author

mattmoor commented Oct 7, 2020

🤔 /unhold works in Knative, have y'all not picked up Prow in a while or is this a Knative snowflake 😞

@vdemeester
Copy link
Member

/unhold works in Knative, have y'all not picked up Prow in a while or is this a Knative snowflake

Our prow instance is a bit old too… that might be why it doesn't work here 🙃

dibyom
dibyom reviewed Oct 7, 2020
View reviewed changes
Copy link
Member

@dibyom dibyom left a comment

Choose a reason for hiding this comment

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

/approve
/lgtm

cmd/triggerrun/cmd/root_test.go
@@ -46,7 +46,7 @@ func TestTrigger_Error(t *testing.T) {
buf := new(bytes.Buffer)
err := trigger("../testdata/trigger.yaml", "../testdata/http.txt", "show", buf)

Copy link
Member

Choose a reason for hiding this comment

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

hmmm...we should rewrite this to be less brittle at some point.

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 7, 2020
@tekton-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dibyom

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 Oct 7, 2020
@tekton-robot tekton-robot merged commit cb18793 into tektoncd:master Oct 7, 2020
@mattmoor mattmoor deleted the bump-deps branch October 7, 2020 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dibyom dibyom dibyom left review comments

@bobcatfish bobcatfish Awaiting requested review from bobcatfish

@khrm khrm Awaiting requested review from khrm

Assignees

@bobcatfish bobcatfish

@dibyom dibyom

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mattmoor @vdemeester @tekton-robot @dibyom @bobcatfish