Update containerd to 06b9cb35161009dcb7123345749fef02f7cea8e0 by mlaventure · Pull Request #34356 · moby/moby

mlaventure · 2017-08-01T16:04:19Z

This also update:

runc to 3f2f8b84a77f73d38244dd690525642a72156c64
runtime-specs to v1.0.0

Signed-off-by: Kenfe-Mickael Laventure [email protected]

diff: containerd/containerd@3addd84...06b9cb3

- What I did

Updated containerd to version 06b9cb35161009dcb7123345749fef02f7cea8e0
Updated runtime-specs to version v1.0.0 to match the one used by containerd
Updated runc to 3f2f8b84a77f73d38244dd690525642a72156c64

- How I did it

Good old vndr ;-)

- How to verify it

containerd --version

- Description for the changelog

Good question 😅

- A picture of a cute animal (not mandatory but encouraged)

🐒

thaJeztah · 2017-08-01T21:25:51Z

@mlaventure panic in CI

16:08:56 --- FAIL: TestLoadProfile (0.00s)
16:08:56 panic: runtime error: invalid memory address or nil pointer dereference [recovered]
16:08:56 	panic: runtime error: invalid memory address or nil pointer dereference
16:08:56 [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1013ec20]

cpuguy83 · 2017-08-03T15:31:47Z

00:01:04.147 # github.com/docker/docker/libcontainerd
00:01:04.147 ..\..\libcontainerd\client_windows.go:103: spec.Platform undefined (type specs.Spec has no field or method Platform)
00:01:04.147 ..\..\libcontainerd\client_windows.go:423: container.ociSpec.Platform undefined (type specs.Spec has no field or method Platform)
00:01:04.148 ..\..\libcontainerd\container_windows.go:85: ctr.ociSpec.Platform undefined (type specs.Spec has no field or method Platform)
00:01:04.148 ..\..\libcontainerd\container_windows.go:93: ctr.ociSpec.Platform undefined (type specs.Spec has no field or method Platform)
00:01:04.148 ..\..\libcontainerd\container_windows.go:248: ctr.ociSpec.Platform undefined (type specs.Spec has no field or method Platform)

mlaventure · 2017-08-03T15:48:13Z

Ah, I forgot windows :)

mlaventure · 2017-08-03T15:48:59Z

If unix test passes, maybe I'll just wait for @darrenstahlmsft to cherry-pick my changes or vice-versa

mlaventure · 2017-08-03T18:10:39Z

Found a bug in runc, I made a PR to fix it: opencontainers/runc#1544

mlaventure · 2017-08-04T15:03:32Z

Ok, so this is waiting on the opencontainers/runc#1544 to be merged, and a decision on whether opencontainers/runc#1450 (comment) is going to be reverted or not. If it's not reverted, I'll probably just fork runc for now.

thaJeztah · 2017-08-05T00:48:44Z

opencontainers/runc#1544 was merged

opencontainers/runc#1548 (reverting opencontainers/runc#1450 (comment)) has two LGTM's (not merged yet)

thaJeztah · 2017-08-05T10:10:01Z

both are merged now

mlaventure · 2017-08-06T18:30:05Z

Waiting for the rc4 to be tagged :)

On Sat, Aug 5, 2017 at 3:11 AM Sebastiaan van Stijn < ***@***.***> wrote: both are merged now — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#34356 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/APUifkI4qbj787vot1OpaPfgcI1Ndccrks5sVD-3gaJpZM4Op8xJ> .

-- Sent from my phone.

tonistiigi · 2017-08-16T00:03:29Z

ping @jhowardmsft

lowenna · 2017-08-16T00:08:26Z

@tonistiigi I haven't got time to review this evening, but will take a look while on the plane to SF in the morning.

I take it back - was waiting for other things to complete in the background. Just some nits really.

lowenna · 2017-08-16T00:15:48Z

 		// TODO @jhowardmsft LCOW Support. Modify this check when running in dual-mode
 		if system.LCOWSupported() && img.OS == "linux" {
-			daemon.createSpecLinuxFields(c, &s)
+			createSpecLinuxFields(c, &s)


This function is a shell at the moment, but will shortly need daemon. I'd probably keep that in for now

lowenna · 2017-08-16T00:21:17Z

-	osName := spec.Platform.OS
-	if osName == "windows" {
+
+	// spec.Linux must be nill for Windows containers, but spec.Windows will be filled in regardless of container platform.


lowenna · 2017-08-16T00:22:50Z

+	// spec.Linux must be nill for Windows containers, but spec.Windows will be filled in regardless of container platform.
+	// This is a temporary workaround due to LCOW requiring layer folder paths, which are stored under spec.Windows.
+	// TODO: @darrenstahlmsft fix this once the OCI spec is updated to support layer folder paths for LCOW
+	isWindows := spec.Linux == nil


Not sure we need this variable if it's only accessed once (could be in the if itself). Nit though.

lowenna · 2017-08-16T00:24:28Z

+	}
+
+	// We must have least one layer in the spec
+	if spec.Windows.LayerFolders == nil || len(spec.Windows.LayerFolders) < 1 {


lowenna · 2017-08-16T00:27:04Z

-			}
-			break
-		}
+	if container.ociSpec.Windows.HyperV == nil {


Hmmm, this will break LCOW pause/resume

Actually, I plead temporary insanity. Ignore that comment. It's fine as is! I misread == as !=.

lowenna · 2017-08-16T00:27:27Z

-			break
-		}
+
+	if container.ociSpec.Windows.HyperV == nil {


Same here for LCOW

And same, ignore above comment too.

lowenna · 2017-08-16T00:29:37Z

-	// Linux containers requires the raw OCI spec passed through HCS and onwards to GCS for the utility VM.
-	if ctr.ociSpec.Platform.OS == "linux" {
+	// LCOW requires the raw OCI spec passed through HCS and onwards to GCS for the utility VM.
+	if system.LCOWSupported() && !ctr.isWindows {


Drop system.LCOWSupported(). It'll never get this far if Linux and LCOW isn't enabled.

mlaventure · 2017-08-16T00:56:53Z

@darrenstahlmsft do you mind addressing @jhowardmsft comments? I can pick the changes from your branch once done.

darstahl · 2017-08-16T07:12:11Z

Yep, I'll fix those as soon as I can.

darstahl · 2017-08-21T19:02:41Z

Fixed nits with darstahl@5c6a3c0

This also update: - runc to 3f2f8b84a77f73d38244dd690525642a72156c64 - runtime-specs to v1.0.0 Signed-off-by: Kenfe-Mickael Laventure <[email protected]>

darstahl · 2017-08-21T21:53:50Z

@mlaventure I don't think the nits fixed in darstahl@5c6a3c0 got picked up 👼

mlaventure · 2017-08-21T22:17:08Z

@darrenstahlmsft in which branch is it? I cherry-picked the #33336 PR commits

EDIT: I guess I can just clone your whole repo 👼

Signed-off-by: Darren Stahl <[email protected]> Signed-off-by: Kenfe-Mickael Laventure <[email protected]>

mlaventure · 2017-08-21T22:20:07Z

@darrenstahlmsft done, thanks for the ping!

darstahl · 2017-08-21T22:28:55Z

Sorry, I pushed it on a branch based on this PR, attempting to make it easier to take. Maybe I should've just continued working in the other branch 👼

The docker update to use this version of runc can be found in the PR moby/moby#34356.

mlaventure requested review from dnephin and tianon as code owners August 1, 2017 16:04

GordonTheTurtle added the status/0-triage label Aug 1, 2017

mlaventure mentioned this pull request Aug 1, 2017

[Needs carry for Linux fixes] Windows: Update to latest OCI runtime spec #33336

Closed

thaJeztah added status/2-code-review status/failing-ci Indicates that the PR in its current state fails the test suite and removed status/0-triage labels Aug 1, 2017

darstahl mentioned this pull request Aug 1, 2017

[Needs-Carry] Update opencontainers/runtime-spec to v1.0.0 #34357

Closed

mlaventure force-pushed the update-containerd branch 3 times, most recently from fe8229e to 2f5e3b4 Compare August 3, 2017 14:59

mlaventure requested a review from cpuguy83 as a code owner August 3, 2017 14:59

mlaventure changed the title ~~Update containerd to 08888ce31d3eaca228526d2722b55b8a41a891e1~~ [WIP] Update containerd to 08888ce31d3eaca228526d2722b55b8a41a891e1 Aug 3, 2017

mlaventure added the rebuild/* label Aug 3, 2017

mlaventure force-pushed the update-containerd branch from 2f5e3b4 to 3a926ff Compare August 3, 2017 15:16

mlaventure force-pushed the update-containerd branch from 1e3fe63 to 4696ff8 Compare August 3, 2017 18:11

mlaventure removed the rebuild/* label Aug 3, 2017

mlaventure force-pushed the update-containerd branch 2 times, most recently from 6e9eaef to 21228fa Compare August 3, 2017 21:36

mlaventure force-pushed the update-containerd branch 2 times, most recently from fd8cb42 to cb03625 Compare August 4, 2017 15:33

yongtang added the rebuild/z label Aug 15, 2017

GordonTheTurtle removed the rebuild/z label Aug 15, 2017

GordonTheTurtle assigned tonistiigi Aug 16, 2017

lowenna reviewed Aug 16, 2017

View reviewed changes

Update containerd to 06b9cb35161009dcb7123345749fef02f7cea8e0

45d85c9

This also update: - runc to 3f2f8b84a77f73d38244dd690525642a72156c64 - runtime-specs to v1.0.0 Signed-off-by: Kenfe-Mickael Laventure <[email protected]>

mlaventure force-pushed the update-containerd branch from a8dea25 to d699e37 Compare August 21, 2017 19:06

Update Windows and LCOW to use v1.0.0 runtime-spec

7c29103

Signed-off-by: Darren Stahl <[email protected]> Signed-off-by: Kenfe-Mickael Laventure <[email protected]>

mlaventure force-pushed the update-containerd branch from d699e37 to 7c29103 Compare August 21, 2017 22:19

lowenna approved these changes Aug 24, 2017

View reviewed changes

lowenna merged commit 285bc99 into moby:master Aug 24, 2017

mlaventure deleted the update-containerd branch August 24, 2017 21:41

beweedon added a commit to microsoft/opengcs that referenced this pull request Aug 25, 2017

Update runc version to version used by docker

5836ae6

The docker update to use this version of runc can be found in the PR moby/moby#34356.

beweedon mentioned this pull request Aug 25, 2017

Update runc version to version used by docker microsoft/opengcs#121

Merged

beweedon added a commit to microsoft/opengcs that referenced this pull request Aug 25, 2017

Update runc version to version used by docker

a080817

The docker update to use this version of runc can be found in the PR moby/moby#34356.

thaJeztah mentioned this pull request Feb 6, 2018

Daemon: passdown the --oom-kill-disable option to containerd #36201

Merged

Conversation

mlaventure commented Aug 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Aug 1, 2017

Uh oh!

cpuguy83 commented Aug 3, 2017

Uh oh!

mlaventure commented Aug 3, 2017

Uh oh!

mlaventure commented Aug 3, 2017

Uh oh!

mlaventure commented Aug 3, 2017

Uh oh!

mlaventure commented Aug 4, 2017

Uh oh!

thaJeztah commented Aug 5, 2017

Uh oh!

thaJeztah commented Aug 5, 2017

Uh oh!

mlaventure commented Aug 6, 2017 via email

Uh oh!

tonistiigi commented Aug 16, 2017

Uh oh!

lowenna commented Aug 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlaventure commented Aug 16, 2017

Uh oh!

darstahl commented Aug 16, 2017

Uh oh!

darstahl commented Aug 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

darstahl commented Aug 21, 2017

Uh oh!

mlaventure commented Aug 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlaventure commented Aug 21, 2017

Uh oh!

darstahl commented Aug 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mlaventure commented Aug 1, 2017 •

edited

Loading

lowenna commented Aug 16, 2017 •

edited

Loading

darstahl commented Aug 21, 2017 •

edited

Loading

mlaventure commented Aug 21, 2017 •

edited

Loading