Bump pause container used by kubelet and tests to 3.1 #57517

verb · 2017-12-21T15:53:28Z

This updates the version of the pause container used by the kubelet and
various test utilities to 3.1.

What this PR does / why we need it: The pause container hasn't been rebuilt in quite a while and needs an update to reap zombies (#50865) and for schema2 manifest (#56253).

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #50865, Fixes #56253

Special notes for your reviewer:

Release note:

The kubelet uses a new release 3.1 of the pause container with the Docker runtime. This version will clean up orphaned zombie processes that it inherits.

verb · 2017-12-21T15:54:16Z

/sig node

ixdy

wonder if it's worth updating hack/make-rules/test-cmd-util.sh and hack/testdata/pod-with-precision.json too?

otherwise LGTM

ixdy · 2017-12-28T21:49:59Z

cmd/kubelet/app/options/container_runtime.go

@@ -28,7 +28,7 @@ import (
 const (
 	// When these values are updated, also update test/e2e/framework/util.go


maybe update this comment while you're here, since it's no longer valid? :)

Path updated, technical debt repaid. :)

ixdy · 2017-12-28T21:50:10Z

test/integration/framework/util.go

@@ -41,7 +41,7 @@ const (
 	// When these values are updated, also update cmd/kubelet/app/options/options.go


Path updated.

ixdy · 2017-12-28T21:50:22Z

test/utils/image/manifest.go

@@ -83,7 +83,7 @@ var (
 	NoSnatTestProxy          = ImageConfig{e2eRegistry, "no-snat-test-proxy", "1.0", true}
 	NWayHTTP                 = ImageConfig{e2eRegistry, "n-way-http", "1.0", true}
 	// When these values are updated, also update cmd/kubelet/app/options/options.go


and this one.

Path updated.

luxas

Aren't there more places that need updating in the repo or have we finally gotten into a such good place so that this is a 5 LoC change :)?

This needs a pretty big release note and needs to be pointed out in v1.10 external dependencies as well...

Also, what has changed between 3.0 and 3.1? Any changelog to refer to?

verb · 2018-01-03T16:38:25Z

Aren't there more places that need updating in the repo or have we finally gotten into a such good place so that this is a 5 LoC change :)?

Actually, I think we have. There are lots of other references to the pause container, but in config snippets used in unit tests. These were the only places I saw that were actually using the container, but admittedly it was a little bit needle-in-haystack.

I wouldn't mind updating all of those if someone thinks it is important.

This needs a pretty big release note and needs to be pointed out in v1.10 external dependencies as well...

Oh really? I didn't think anyone would care that much. I suppose some may use cached images. I'll update the description with a release note.

Also, what has changed between 3.0 and 3.1? Any changelog to refer to?

There were two changes to pause.c:

Add SIGCHLD handler to pause container #36853 added a SIGCHLD handler to reap zombies in pods with shared PID namespaces
New release for pause container #56762 added a version flag

The only changelog is the commit history for pause.c, though I've been starting to wonder if pause should be a separate repo since it's versioned separately.

luxas · 2018-01-03T17:30:01Z

I wouldn't mind updating all of those if someone thinks it is important.

Please update all refs to 3.0 you only can see

I'll update the description with a release note.

Great 👍

I didn't think anyone would care that much.

Every change of this sort needs to be visibly advertised, e.g. as people may be in air-gapped envs where they pull the images themselves and need to know the exact image to pull. In that light this is a breaking change. We need for instance to update the kubeadm v1.10 docs with this information, where we do reference the 3.0 pause image.

There were two changes to pause.c:

#36853 added a SIGCHLD handler to reap zombies in pods with shared PID namespaces
#56762 added a version flag

Can you please create a CHANGELOG.md file in the pause directory so it's easier to keep track in source control?

The only changelog is the commit history for pause.c, though I've been starting to wonder if pause should be a separate repo since it's versioned separately.

I think it's fine here, we have lots of other images here that are versioned independently as well

verb · 2018-01-03T18:04:38Z

Updated all existing references of pause 3.0 to pause 3.1 and added a simple CHANGELOG.md. Let me know if you want anything more from the CHANGELOG.

verb · 2018-01-03T19:17:07Z

/retest

luxas

/lgtm

@ixdy WDYT?

ixdy · 2018-01-03T22:44:23Z

build/pause/CHANGELOG.md

@@ -0,0 +1,8 @@
+# 3.1
+
+* The pause container gains a single handler to clean up orphaned zombie processes. ([#36853](https://prs.k8s.io/36853), [@verb](https://github.com/verb))


typo: single->signal?

Whoops, fixed. Thanks for catching that.

ixdy · 2018-01-03T22:46:34Z

LGTM besides typo.

Wonder who's a good approver for this...

This updates the version of the pause container used by the kubelet and various test utilities to 3.1. This also adds a CHANGELOG.md for build/pause

verb · 2018-01-04T10:43:45Z

Looks like the combination of @luxas, @ixdy and @vishh can approve.

/assign @ixdy @vishh
/unassign @Random-Liu @foxish

verb · 2018-01-04T12:11:30Z

Looks like pull-kubernetes-cross is unable to build hyperkube across PRs: https://k8s-gubernator.appspot.com/builds/kubernetes-jenkins/pr-logs/directory/pull-kubernetes-cross

verb · 2018-01-04T14:44:15Z

Issue for cross: #57822

ixdy · 2018-01-04T20:08:27Z

/lgtm
/retest

yujuhong · 2018-01-05T00:17:30Z

cmd/kubelet/app/options/container_runtime.go

@@ -26,9 +26,9 @@ import (
 )

 const (
-	// When these values are updated, also update test/e2e/framework/util.go
+	// When these values are updated, also update test/utils/image/manifest.go


These "please also update" comments in multiple files seem a bit too much to me. Maybe we should clean it up another time

yujuhong · 2018-01-05T00:18:12Z

/lgtm

/cc @Random-Liu, just FYI

k8s-ci-robot · 2018-01-05T00:18:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ixdy, luxas, verb, yujuhong

Associated issue: #50865

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~build/OWNERS~~ [ixdy]
~~cmd/kubeadm/OWNERS~~ [luxas]
~~cmd/kubelet/OWNERS~~ [yujuhong]
~~hack/OWNERS~~ [ixdy]
~~pkg/kubelet/OWNERS~~ [yujuhong]
~~test/OWNERS~~ [ixdy]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

k8s-github-robot · 2018-01-05T00:19:55Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-01-05T01:06:47Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 21, 2017

k8s-github-robot assigned foxish and Random-Liu Dec 21, 2017

k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Dec 21, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 23, 2017

verb force-pushed the paws-bump branch from f478757 to 7985ad9 Compare December 23, 2017 09:56

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 23, 2017

ixdy mentioned this pull request Dec 28, 2017

Use multi-arch pause image for tests #57589

Merged

ixdy reviewed Dec 28, 2017

View reviewed changes

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 29, 2017

luxas approved these changes Dec 30, 2017

View reviewed changes

verb force-pushed the paws-bump branch from 7985ad9 to aafb4e9 Compare January 3, 2018 16:39

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 3, 2018

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Jan 3, 2018

verb force-pushed the paws-bump branch from aafb4e9 to 3da150a Compare January 3, 2018 18:04

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 3, 2018

verb force-pushed the paws-bump branch from 3da150a to d54e769 Compare January 3, 2018 18:09

k8s-ci-robot assigned luxas Jan 3, 2018

luxas approved these changes Jan 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 Jan 3, 2018

ixdy reviewed Jan 3, 2018

View reviewed changes

Update pause container version to 3.1

1ea6970

This updates the version of the pause container used by the kubelet and various test utilities to 3.1. This also adds a CHANGELOG.md for build/pause

verb force-pushed the paws-bump branch from d54e769 to 1ea6970 Compare January 4, 2018 10:38

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 4, 2018

k8s-ci-robot assigned ixdy and vishh and unassigned foxish and Random-Liu Jan 4, 2018

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

yujuhong reviewed Jan 5, 2018

View reviewed changes

k8s-ci-robot assigned yujuhong Jan 5, 2018

k8s-ci-robot requested a review from Random-Liu January 5, 2018 00:18

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

k8s-github-robot merged commit afbbd39 into kubernetes:master Jan 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump pause container used by kubelet and tests to 3.1 #57517

Bump pause container used by kubelet and tests to 3.1 #57517

verb commented Dec 21, 2017 •

edited

Loading

verb commented Dec 21, 2017

ixdy left a comment

ixdy Dec 28, 2017

verb Jan 3, 2018

ixdy Dec 28, 2017

verb Jan 3, 2018

ixdy Dec 28, 2017

verb Jan 3, 2018

luxas left a comment

verb commented Jan 3, 2018

luxas commented Jan 3, 2018

verb commented Jan 3, 2018

verb commented Jan 3, 2018

luxas left a comment

ixdy Jan 3, 2018

verb Jan 4, 2018

ixdy commented Jan 3, 2018

verb commented Jan 4, 2018

verb commented Jan 4, 2018

verb commented Jan 4, 2018

ixdy commented Jan 4, 2018

yujuhong Jan 5, 2018

yujuhong commented Jan 5, 2018 •

edited

Loading

k8s-ci-robot commented Jan 5, 2018

k8s-github-robot commented Jan 5, 2018

k8s-github-robot commented Jan 5, 2018

		@@ -28,7 +28,7 @@ import (
		const (
		// When these values are updated, also update test/e2e/framework/util.go

		@@ -41,7 +41,7 @@ const (
		// When these values are updated, also update cmd/kubelet/app/options/options.go

		@@ -0,0 +1,8 @@
		# 3.1

		* The pause container gains a single handler to clean up orphaned zombie processes. ([#36853](https://prs.k8s.io/36853), [@verb](https://github.com/verb))

Bump pause container used by kubelet and tests to 3.1 #57517

Bump pause container used by kubelet and tests to 3.1 #57517

Conversation

verb commented Dec 21, 2017 • edited Loading

verb commented Dec 21, 2017

ixdy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luxas left a comment

Choose a reason for hiding this comment

verb commented Jan 3, 2018

luxas commented Jan 3, 2018

verb commented Jan 3, 2018

verb commented Jan 3, 2018

luxas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ixdy commented Jan 3, 2018

verb commented Jan 4, 2018

verb commented Jan 4, 2018

verb commented Jan 4, 2018

ixdy commented Jan 4, 2018

Choose a reason for hiding this comment

yujuhong commented Jan 5, 2018 • edited Loading

k8s-ci-robot commented Jan 5, 2018

k8s-github-robot commented Jan 5, 2018

k8s-github-robot commented Jan 5, 2018

verb commented Dec 21, 2017 •

edited

Loading

yujuhong commented Jan 5, 2018 •

edited

Loading