Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Bump dependencies to support k8s 1.18 #2796

Merged
merged 1 commit into from
May 25, 2020
Merged

Bump dependencies to support k8s 1.18 #2796

merged 1 commit into from
May 25, 2020

Conversation

mszostok
Copy link
Contributor

@mszostok mszostok commented May 10, 2020 •
edited
Loading

Description

Fixes #2793

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 10, 2020
@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 10, 2020
@mszostok mszostok changed the title K8s 1.18 WIP K8s 1.18 May 10, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2020
@mszostok mszostok force-pushed the k8s-1.18 branch 3 times, most recently from 3547b38 to ff12c96 Compare May 10, 2020 14:31
@mszostok
Copy link
Contributor Author

mszostok commented May 10, 2020 •
edited
Loading

unit test failed: https://prow.k8s.io/log?id=1259491213271109633&job=pull-service-catalog-test-unit

# k8s.io/apimachinery/pkg/util/errors
../../../../pkg/mod/k8s.io/apimachinery@v0.18.2/pkg/util/errors/errors.go:99:10: undefined: errors.Is
note: module requires Go 1.13

waitng for kubernetes/test-infra#17567 to be merged

@mszostok mszostok force-pushed the k8s-1.18 branch 6 times, most recently from 6984a2a to da0ed65 Compare May 10, 2020 16:08
@jberkhahn
Copy link
Contributor

/retest

@mszostok mszostok mentioned this pull request May 12, 2020
Closed
@jberkhahn
Copy link
Contributor

/retest

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 14, 2020
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 15, 2020
@mszostok mszostok mentioned this pull request May 15, 2020
Closed
@mszostok mszostok force-pushed the k8s-1.18 branch 6 times, most recently from b89dd4b to e326163 Compare May 15, 2020 12:47
@mszostok mszostok force-pushed the k8s-1.18 branch 2 times, most recently from 157087c to 81cd73d Compare May 18, 2020 10:01
MHBauer
MHBauer approved these changes May 19, 2020
View reviewed changes
Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

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

Irritatingly large and excruciating to review.

Some concerns (that would exist prior to this PR).

/lgtm
allowing others to review.
/hold

Makefile Outdated

export GOFLAGS=-mod=vendor
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these should probably add on to whatever ${GOFLAGS} is already defined and available in the environment.

contrib/hack/ci/run-e2e-tests.sh Outdated
@@ -85,7 +87,7 @@ test::prepare_data() {

test::execute() {
shout "- Executing e2e test..."
pushd ${REPO_ROOT_DIR}/test/e2e/
pushd "${REPO_ROOT_DIR}/test/e2e/"
Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch

contrib/hack/ci/run-migration-tests.sh Outdated
@@ -47,6 +47,8 @@ export TB_NAMESPACE="test-broker"

DUMP_CLUSTER_INFO="${DUMP_CLUSTER_INFO:-false}"

GOFLAGS=-mod=vendor
Copy link
Contributor

Choose a reason for hiding this comment

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

why no export like the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 👍

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 19, 2020
@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 19, 2020
@mszostok mszostok mentioned this pull request May 19, 2020
Closed
@mszostok
Copy link
Contributor Author

Thanks @MHBauer for the review, I will apply fixes.

FYI: I was also waiting until this pr: kubernetes/test-infra#17622 will be merged. Now we will have 3 pipelines to test if the current implementation works fine with 3 latest K8s versions, currently 1.16, 1.17, and 1.18. I tested that manually but IMO having pipelines for that is the best option to do not have any regression and also prove that this PR does not break anything.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 20, 2020
@k8s-ci-robot k8s-ci-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 20, 2020
@mszostok mszostok changed the title WIP K8s 1.18 Bump dependencies to support k8s 1.18 May 20, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 20, 2020
@mszostok
Copy link
Contributor Author

/test pull-service-catalog-test-integration

@piotrmiskiewicz
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2020
@jberkhahn
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jberkhahn, mszostok

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

@mszostok
Copy link
Contributor Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 25, 2020
@mszostok
Copy link
Contributor Author

/test pull-service-catalog-test-integration

1 similar comment
@mszostok
Copy link
Contributor Author

/test pull-service-catalog-test-integration

@k8s-ci-robot k8s-ci-robot merged commit 56681c6 into kubernetes-retired:master May 25, 2020
@mszostok mszostok deleted the k8s-1.18 branch May 25, 2020 12:19
@mszostok mszostok mentioned this pull request May 25, 2020
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MHBauer MHBauer MHBauer approved these changes

@jberkhahn jberkhahn Awaiting requested review from jberkhahn

Assignees

@MHBauer MHBauer

@piotrmiskiewicz piotrmiskiewicz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm Indicates that a PR is ready to be merged. 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.

Register broker not working on kubernetes 1.18
5 participants
@mszostok @jberkhahn @piotrmiskiewicz @k8s-ci-robot @MHBauer