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 the version of kubernetes-cni to 0.6.0 #71629

Merged
merged 1 commit into from Dec 4, 2018

Conversation

@mauilion
Copy link
Contributor

mauilion commented Dec 1, 2018

This will resolve the kubernetes-anywhere e2e test.
In commit: #71540
I bumped the required version of kubernetes-cni to 0.6.0 but didn't
start packaging it. This resolve that.

Signed-off-by: Duffie Cooley dcooley@heptio.com

What type of PR is this?
/kind failing-test

What this PR does / why we need it:
This bumps the version of kubernetes-cni that we package to 0.6.0

Special notes for your reviewer:
This is related to: https://storage.googleapis.com/kubernetes-jenkins/logs/ci-kubernetes-e2e-kubeadm-gce-master/1018/artifacts/e2e-1018-1ad8f-master/serial-1.log

Does this PR introduce a user-facing change?:

Bumps version of kubernetes-cni to 0.6.0
Bump the version of kubernetes-cni to 0.6.0
This will resolve the kubernetes-anywhere e2e test.
In commit: #71540
I bumped the required version of kubernetes-cni to 0.6.0 but didn't
start packaging it. This resolve that.

Signed-off-by: Duffie Cooley <dcooley@heptio.com>
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 1, 2018

Hi @mauilion. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@mauilion

This comment has been minimized.

Copy link
Contributor Author

mauilion commented Dec 1, 2018

@mauilion

This comment has been minimized.

Copy link
Contributor Author

mauilion commented Dec 1, 2018

@mauilion

This comment has been minimized.

Copy link
Contributor Author

mauilion commented Dec 1, 2018

/sig cluster-lifecycle
/priority important-soon

@fabriziopandini

This comment has been minimized.

Copy link
Contributor

fabriziopandini commented Dec 1, 2018

the change seems safe to me but I'll defer to @AishSundar or @timothysc because it will fix kubeadm gce on master (which is not blocking for release) but for doing this it touches Bazel build

@AishSundar

This comment has been minimized.

Copy link
Contributor

AishSundar commented Dec 1, 2018

@mauilion will this apply to/affect CI jobs in 1.13 branch? If so lets please hold this until EOD Monday 12/3 so that we can get 1.13 out without any CI signal disruption. Let me know if there's any urgency for this to go in before that.

/hold

@mauilion

This comment has been minimized.

Copy link
Contributor Author

mauilion commented Dec 1, 2018

It won't. This just addresses master iiuc. It will fix the failing k/anywhere test tho.

@cblecker

This comment has been minimized.

Copy link
Member

cblecker commented Dec 3, 2018

/ok-to-test

@cblecker

This comment has been minimized.

Copy link
Member

cblecker commented Dec 3, 2018

/retest

@timothysc

This comment has been minimized.

Copy link
Member

timothysc commented Dec 3, 2018

/hold cancel

@AishSundar - this is for master/1.14 only

@timothysc

This comment has been minimized.

Copy link
Member

timothysc commented Dec 3, 2018

@mauilion - how can we verify this fixes. I'm missing how 0.5.1 was installed on k8s-anywhere.

@@ -92,7 +92,7 @@ grep ^STABLE_BUILD_SCM_REVISION bazel-out/stable-status.txt \
genrule(
name = "cni_package_version",
outs = ["cni_version"],
cmd = "echo 0.5.1 >$@",
cmd = "echo 0.6.0 >$@",

This comment has been minimized.

@timothysc

timothysc Dec 3, 2018

Member

This means we have multiple locations for CNI version.

Can we bake it into a single variable?

This comment has been minimized.

@mauilion

mauilion Dec 3, 2018

Author Contributor

Per our conversation. This is fine to merge as it stands @timothysc but we need to capture the requirement that the dependent version and the built version are locked to the same var.

This comment has been minimized.

@ixdy

ixdy Dec 3, 2018

Member

yes, this is what we do with the CRI version:

CRI_TOOLS_VERSION = "1.12.0"

we should do something similar here.

This comment has been minimized.

@ixdy

ixdy Dec 3, 2018

Member

though then also we should probably use that variable in the WORKSPACE:

http_file(
name = "kubernetes_cni",
downloaded_file_path = "kubernetes_cni.tgz",
sha256 = "f04339a21b8edf76d415e7f17b620e63b8f37a76b2f706671587ab6464411f2d",
urls = mirror("https://storage.googleapis.com/kubernetes-release/network-plugins/cni-plugins-amd64-v0.6.0.tgz"),
)

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Dec 3, 2018

CNI bits are obtained here:

urls = mirror("https://storage.googleapis.com/kubernetes-release/network-plugins/cni-plugins-amd64-v0.6.0.tgz"),

AFAICT, it's been 0.6.0 for a long time, with the package version not matching ...

Not sure what the best option to keep this in sync is, but we should get this in to unbreak the builds

/cc @ixdy

@timothysc

This comment has been minimized.

Copy link
Member

timothysc commented Dec 3, 2018

AFAICT, it's been 0.6.0 for a long time, with the package version not matching ...

It's pretty clear from the logs that it was not doing that, but was running 0.5.1

@timothysc
Copy link
Member

timothysc left a comment

/lgtm
/approve

I'm going to unblock this to try and get master green and open some tracking issues to fix some of these issues.

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 3, 2018

@mauilion mauilion closed this Dec 3, 2018

@mauilion

This comment has been minimized.

Copy link
Contributor Author

mauilion commented Dec 3, 2018

AFAICT, it's been 0.6.0 for a long time, with the package version not matching ...

It's pretty clear from the logs that it was not doing that, but was running 0.5.1

What's happening here is even more messed up tho. Basically bazel would pull version 0.6.0 (because of the WORKSPACE config) and then "label" it 0.5.1 (because of the k/k/build/BUILD configuration)

So when you install the resulting rpm it is holding version 0.6.0 but labeled 0.5.1...

@mauilion mauilion reopened this Dec 3, 2018

@cblecker

This comment has been minimized.

Copy link
Member

cblecker commented Dec 3, 2018

@mauilion Can you take a look at @ixdy's recommended changes above?

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Dec 3, 2018

@cblecker discussing this further, It'd be helpful to go ahead and correct the version which is a tiny correctness fix and then follow-up with managing it better, @ixdy and I discussed this and I can guarantee one of us three of us files a follow-up promptly.

The current state is quite broken - the debs fail to install which is breaking cluster-up in some CI.

@ixdy

This comment has been minimized.

Copy link
Member

ixdy commented Dec 3, 2018

/approve

@cblecker

This comment has been minimized.

Copy link
Member

cblecker commented Dec 3, 2018

SGTM 👍

/lgtm
/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 3, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, ixdy, mauilion, timothysc

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

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Dec 4, 2018

/retest

@k8s-ci-robot k8s-ci-robot merged commit 8f7405e into kubernetes:master Dec 4, 2018

18 checks passed

cla/linuxfoundation mauilion authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Dec 4, 2018

Thanks! will look into a follow-up now 👍

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Dec 4, 2018

follow up here: #71686

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment