Bump the version of kubernetes-cni to 0.6.0 #71629

mauilion · 2018-12-01T07:25:45Z

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

This will resolve the kubernetes-anywhere e2e test. In commit: kubernetes#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 · 2018-12-01T07:25:53Z

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 · 2018-12-01T07:27:25Z

@fabriziopandini @neolit123 @timothysc

mauilion · 2018-12-01T07:32:42Z

For context: https://kubernetes.slack.com/archives/C13J86Z63/p1543642853032600

mauilion · 2018-12-01T07:33:56Z

/sig cluster-lifecycle
/priority important-soon

fabriziopandini · 2018-12-01T09:25:36Z

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 · 2018-12-01T14:44:16Z

@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 · 2018-12-01T20:40:23Z

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

cblecker · 2018-12-03T01:41:37Z

/ok-to-test

cblecker · 2018-12-03T05:27:43Z

/retest

timothysc · 2018-12-03T16:39:18Z

/hold cancel

@AishSundar - this is for master/1.14 only

timothysc · 2018-12-03T16:50:37Z

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

timothysc · 2018-12-03T16:51:32Z

build/BUILD

@@ -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 means we have multiple locations for CNI version.

Can we bake it into a single variable?

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.

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

kubernetes/build/workspace.bzl

Line 15 in 8c0542d

CRI_TOOLS_VERSION = "1.12.0"

we should do something similar here.

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

kubernetes/build/root/WORKSPACE

Lines 57 to 62 in 8c0542d

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 · 2018-12-03T19:28:22Z

CNI bits are obtained here:

kubernetes/build/root/WORKSPACE

Line 61 in 8c0542d

    
           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 · 2018-12-03T20:39:21Z

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

/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.

mauilion · 2018-12-03T20:56:36Z

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...

cblecker · 2018-12-03T21:09:18Z

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

BenTheElder · 2018-12-03T21:29:18Z

@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 · 2018-12-03T21:35:55Z

/approve

cblecker · 2018-12-03T22:55:14Z

SGTM 👍

/lgtm
/approve

k8s-ci-robot · 2018-12-03T22:55:39Z

[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:

~~build/OWNERS~~ [cblecker,ixdy]

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

BenTheElder · 2018-12-04T00:48:13Z

/retest

BenTheElder · 2018-12-04T02:23:35Z

Thanks! will look into a follow-up now 👍

BenTheElder · 2018-12-04T04:36:22Z

follow up here: #71686

Bump the version of kubernetes-cni to 0.6.0

8e08cbc

This will resolve the kubernetes-anywhere e2e test. In commit: kubernetes#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 requested review from cblecker and ixdy December 1, 2018 07:28

mauilion mentioned this pull request Dec 1, 2018

Fixes broken e2e test after adding conntrack. kubernetes-retired/kubernetes-anywhere#576

Merged

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 1, 2018

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 3, 2018

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 3, 2018

timothysc reviewed Dec 3, 2018

View reviewed changes

BenTheElder mentioned this pull request Dec 3, 2018

add v1alpha2 config version kubernetes-sigs/kind#143

Merged

k8s-ci-robot assigned timothysc Dec 3, 2018

timothysc approved these changes Dec 3, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2018

mauilion closed this Dec 3, 2018

mauilion reopened this Dec 3, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 3, 2018

k8s-ci-robot assigned cblecker Dec 3, 2018

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

BenTheElder mentioned this pull request Dec 4, 2018

add CNI_VERSION to build/workspace.bzl, use it to dedupe version #71686

Merged

mauilion mentioned this pull request Mar 15, 2019

REQUEST: New membership for @mauilion kubernetes/org#609

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump the version of kubernetes-cni to 0.6.0 #71629

Bump the version of kubernetes-cni to 0.6.0 #71629

mauilion commented Dec 1, 2018

k8s-ci-robot commented Dec 1, 2018

mauilion commented Dec 1, 2018 •

edited

mauilion commented Dec 1, 2018

mauilion commented Dec 1, 2018 •

edited

fabriziopandini commented Dec 1, 2018

AishSundar commented Dec 1, 2018 •

edited

mauilion commented Dec 1, 2018

cblecker commented Dec 3, 2018

cblecker commented Dec 3, 2018

timothysc commented Dec 3, 2018

timothysc commented Dec 3, 2018

timothysc Dec 3, 2018

mauilion Dec 3, 2018

ixdy Dec 3, 2018

ixdy Dec 3, 2018

BenTheElder commented Dec 3, 2018

timothysc commented Dec 3, 2018

timothysc left a comment •

edited

mauilion commented Dec 3, 2018

cblecker commented Dec 3, 2018

BenTheElder commented Dec 3, 2018 •

edited

ixdy commented Dec 3, 2018

cblecker commented Dec 3, 2018

k8s-ci-robot commented Dec 3, 2018

BenTheElder commented Dec 4, 2018

BenTheElder commented Dec 4, 2018

BenTheElder commented Dec 4, 2018

	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"),
	)

Bump the version of kubernetes-cni to 0.6.0 #71629

Bump the version of kubernetes-cni to 0.6.0 #71629

Conversation

mauilion commented Dec 1, 2018

k8s-ci-robot commented Dec 1, 2018

mauilion commented Dec 1, 2018 • edited

mauilion commented Dec 1, 2018

mauilion commented Dec 1, 2018 • edited

fabriziopandini commented Dec 1, 2018

AishSundar commented Dec 1, 2018 • edited

mauilion commented Dec 1, 2018

cblecker commented Dec 3, 2018

cblecker commented Dec 3, 2018

timothysc commented Dec 3, 2018

timothysc commented Dec 3, 2018

timothysc Dec 3, 2018

Choose a reason for hiding this comment

mauilion Dec 3, 2018

Choose a reason for hiding this comment

ixdy Dec 3, 2018

Choose a reason for hiding this comment

ixdy Dec 3, 2018

Choose a reason for hiding this comment

BenTheElder commented Dec 3, 2018

timothysc commented Dec 3, 2018

timothysc left a comment • edited

Choose a reason for hiding this comment

mauilion commented Dec 3, 2018

cblecker commented Dec 3, 2018

BenTheElder commented Dec 3, 2018 • edited

ixdy commented Dec 3, 2018

cblecker commented Dec 3, 2018

k8s-ci-robot commented Dec 3, 2018

BenTheElder commented Dec 4, 2018

BenTheElder commented Dec 4, 2018

BenTheElder commented Dec 4, 2018

mauilion commented Dec 1, 2018 •

edited

mauilion commented Dec 1, 2018 •

edited

AishSundar commented Dec 1, 2018 •

edited

timothysc left a comment •

edited

BenTheElder commented Dec 3, 2018 •

edited