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 CNI to v0.6.0 #51250

Merged
merged 4 commits into from Oct 16, 2017

Conversation

@dixudx
Member

dixudx commented Aug 24, 2017

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #49480

Special notes for your reviewer:
/assign @luxas @bboreham @feiskyer

Release note:

bump CNI to v0.6.0
@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Aug 24, 2017

@dixudx: GitHub didn't allow me to assign the following users: bboreham.

Note that only kubernetes members can be assigned.

In response to this:

What this PR does / why we need it:

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #49480

Special notes for your reviewer:
/assign @luxas @bboreham @feiskyer

Release note:

bump CNI to v0.6.0

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.

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Aug 24, 2017

Hi @dixudx. 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.

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. I understand the commands that are listed here.

@dnardo

This comment has been minimized.

Contributor

dnardo commented Aug 24, 2017

@freehan

This comment has been minimized.

Member

freehan commented Aug 24, 2017

/ok-to-test

@danehans

This comment has been minimized.

danehans commented Aug 24, 2017

/area ipv6

@freehan

This comment has been minimized.

Member

freehan commented Aug 25, 2017

Is https://github.com/containernetworking/cni/releases/download/ reliable enough to take bombardment for every k8s node bootstrap?

@dixudx

This comment has been minimized.

Member

dixudx commented Aug 25, 2017

Is https://github.com/containernetworking/cni/releases/download/ reliable enough to take bombardment for every k8s node bootstrap?

Aha, this is also a critical issue. But the package is just about 17MB. It won't occupy too much bandwidth. It is easily downloaded in several seconds.

.PHONY: all build push clean
all: push
cni-tars/$(CNI_TARBALL):
mkdir -p cni-tars/
cd cni-tars/ && curl -sSLO --retry 5 https://storage.googleapis.com/kubernetes-release/network-plugins/${CNI_TARBALL}
cd cni-tars/ && curl -sSLO --retry 5 https://github.com/containernetworking/cni/releases/download/${CNI_VERSION}/${CNI_TARBALL}

This comment has been minimized.

@squeed

squeed Aug 25, 2017

Contributor

You almost certainly want to download the plugins release instead. See https://github.com/containernetworking/plugins/releases

@squeed

This comment has been minimized.

Contributor

squeed commented Aug 25, 2017

Watch out, the CNI plugins (e.g. bridge, ptp) moved to a new repository. You need to download from https://github.com/containernetworking/plugins/releases

@freehan

This comment has been minimized.

Member

freehan commented Aug 25, 2017

@danehans Do you need this in 1.8? or can you live with it in HEAD (after 1.8 deadline)?

I asked this because the problems in libcni and cni plugins usually take much longer to expose. And they are a pain to fix. (fix push to cni, revendor cni, cherry-pick, cut k8s release). Merge it after 1.8 leaves us more room to soak it.

@luxas

This comment has been minimized.

Member

luxas commented Aug 25, 2017

@danehans

This comment has been minimized.

danehans commented Oct 9, 2017

@freehan this PR is needed for IPv6 support in 1.9.

@dixudx can you rebase and get the tests to pass?

/assign @thockin

@thockin thockin assigned freehan and dnardo and unassigned thockin Oct 9, 2017

@k8s-merge-robot k8s-merge-robot merged commit 855551d into kubernetes:master Oct 16, 2017

13 of 14 checks passed

pull-kubernetes-e2e-kubeadm-gce Parent Job Status Changed: Job triggered.
Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation dixudx authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Jenkins job succeeded.
Details
pull-kubernetes-e2e-gce-bazel Job succeeded.
Details
pull-kubernetes-e2e-gce-etcd3 Jenkins job succeeded.
Details
pull-kubernetes-e2e-gce-gpu Job succeeded.
Details
pull-kubernetes-e2e-kops-aws Jenkins job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Jenkins job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-unit Jenkins job succeeded.
Details
pull-kubernetes-verify Jenkins job succeeded.
Details

@dixudx dixudx deleted the dixudx:bump_cni_v0.6.0 branch Oct 16, 2017

@@ -24,10 +24,10 @@ ARCH?=amd64
CACHEBUST?=1
BASEIMAGE=gcr.io/google-containers/debian-base-$(ARCH):0.2
CNI_RELEASE=0799f5732f2a11b329d9e3d51b9c8f2e3759f2ff
CNI_VERSION=v0.6.0

This comment has been minimized.

@ixdy

ixdy Oct 19, 2017

Member

we should have bumped the debian-hyperkube-base tag here.

This comment has been minimized.

@dixudx

dixudx Oct 20, 2017

Member

Will do in another PR.

@ixdy

This comment has been minimized.

Member

ixdy commented Oct 19, 2017

Who built/pushed the new CNI tarballs to gs://kubernetes-release/network-plugins? We're missing s390x and ppc64le.

@dixudx

This comment has been minimized.

Member

dixudx commented Oct 20, 2017

@freehan Please help uploading those missing files as well. Thanks.

@ixdy

This comment has been minimized.

Member

ixdy commented Oct 20, 2017

I realized the tarballs are just copied from https://github.com/containernetworking/plugins/releases, so I copied over the ppc64le and s390x ones too.

@bowei

This comment has been minimized.

Member

bowei commented Oct 20, 2017

@ixdy can you also copy the .sha files?

@freehan

This comment has been minimized.

Member

freehan commented Oct 20, 2017

I just uploaded the sha files. Thanks!

@luxas

This comment has been minimized.

Member

luxas commented Oct 20, 2017

Great! Thank you @freehan and @ixdy 👍!

k8s-merge-robot added a commit that referenced this pull request Oct 22, 2017

Merge pull request #54272 from dixudx/bump_debian-hyperkube-base_due_CNI
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

bump debian-hyperkube-base to 0.5 since CNI gets bumped

**What this PR does / why we need it**:
xref [discussion](#51250 (comment)) in #51250

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:
/assign @ixdy @luxas 

**Release note**:

```release-note
None
```

@luxas luxas referenced this pull request Oct 25, 2017

Merged

Fix kubeadm e2e CI build #54573

dims pushed a commit to dims/kubernetes that referenced this pull request Oct 25, 2017

Merge pull request kubernetes#54573 from luxas/luxas-kubeadm-cni-fix
Automatic merge from submit-queue (batch tested with PRs 54545, 54573). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix kubeadm e2e CI build

**What this PR does / why we need it**:

This fixes kubeadm e2e tests; the tarfile was extracted to the wrong directory in kubernetes#51250.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

fixes: kubernetes#54330

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```
@ixdy @pipejakob @kubernetes/sig-cluster-lifecycle-bugs @medinatiger @dims @cmluciano @dixudx

k8s-merge-robot added a commit that referenced this pull request Nov 2, 2017

Merge pull request #54800 from squeed/fix-kubenet-contention
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubenet: yield lock while executing CNI plugin.

The CNI plugin can take up to 3 seconds to execute. CNI plugins can safely be
executed in parallel, so yield the lock to speed up pod creation.

This caused problems with the pod latency tests - previously, CNI plugins executed
in under 20ms. Now they must wait for DAD to finish and addresses to leave
tentative state.

Fixes: #54651

**What this PR does / why we need it**:
After upgrading CNI plugins to v0.6 in #51250, the pod latency tests began failing. This is because the plugins, in order to support IPv6, need to wait for DAD to finish. Because this
delay is while the kubenet lock is held, it significantly slows down the pod creation rate.

**Special notes for your reviewer**:
The CNI plugins also do locking for their critical paths, so it is safe to run them concurrently.

**Release note**:
```release-note
NONE
```
@chrislovecnm

This comment has been minimized.

Member

chrislovecnm commented Dec 17, 2017

I am can d/l

https://storage.googleapis.com/kubernetes-release/network-plugins/cni-plugins-amd64-v0.6.0.tgz.sha1

but I am unable to d/l

https://storage.googleapis.com/kubernetes-release/network-plugins/cni-plugins-amd64-v0.5.0.tgz.sha1

Or the tarball. Can we get 0.4.0 and 0.5.0 uploaded with there SHAs? I am uncertain which versions are for k8s 1.5 and k8s 1.6, as I only have the names with the shas, and the tarball that is staged does not have the version in it.

To be clear I am looking for these two previous files that should have new names and sha files.

https://storage.googleapis.com/kubernetes-release/network-plugins/cni-07a8a28637e97b22eb8dfe710eeae1344f69d16e.tar.gz
https://storage.googleapis.com/kubernetes-release/network-plugins/cni-0799f5732f2a11b329d9e3d51b9c8f2e3759f2ff.tar.gz"

cc @freehan @luxas

Thanks

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Dec 17, 2017

@chrislovecnm: you can't re-open an issue/PR unless you authored it or you are assigned to it.

In response to this:

/reopen

I am can d/l

https://storage.googleapis.com/kubernetes-release/network-plugins/cni-plugins-amd64-v0.6.0.tgz.sha1

but I am unable to d/l

https://storage.googleapis.com/kubernetes-release/network-plugins/cni-plugins-amd64-v0.5.0.tgz.sha1

Or the tarball. Can we get 0.4.0 and 0.5.0 uploaded with there SHAs? I am uncertain which versions are for k8s 1.5 and k8s 1.6, as I only have the names with the shas, and the tarball that is staged does not have the version in it.

To be clear I am looking for these two previous files that should have new names and sha files.

https://storage.googleapis.com/kubernetes-release/network-plugins/cni-07a8a28637e97b22eb8dfe710eeae1344f69d16e.tar.gz
https://storage.googleapis.com/kubernetes-release/network-plugins/cni-0799f5732f2a11b329d9e3d51b9c8f2e3759f2ff.tar.gz"

cc @freehan @luxas

Thanks

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.

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Dec 17, 2017

@chrislovecnm: you can't re-open an issue/PR unless you authored it or you are assigned to it.

In response to this:

/assign
/reopen

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.

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Dec 17, 2017

@chrislovecnm: failed to re-open PR: state cannot be changed. The pull request cannot be reopened.

In response to this:

/reopen

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.

@chrislovecnm

This comment has been minimized.

Member

chrislovecnm commented Dec 18, 2017

/cc @ixdy I am not certain who copied the 0.6.0 tar ball over, but I would love to have the previous two and there sha1 files as well.

@dixudx

This comment has been minimized.

Member

dixudx commented Dec 18, 2017

I am not certain who copied the 0.6.0 tar ball over, but I would love to have the previous two and there sha1 files as well.

@chrislovecnm Please contact @freehan if needed.

@chrislovecnm

This comment has been minimized.

Member

chrislovecnm commented Dec 18, 2017

@dixudx I filed #57341 so we can stop talking on a closed issue

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