Add Implementation for WCOW process isolation support. #1264

Random-Liu · 2019-09-05T06:35:11Z

For #1257.

The PR has 5 commits:

Commit 1: Update containerd to latest version, we need Support foreign and encrypted layers in the unpacker. containerd#3616 and Make default platform configurable in the client containerd#3609 for windows support to work;
Commit 2: Open log file with FILE_SHARE_DELETE. This is required for log rotation to work. Kubelet should be able to rename the container log file when the file is opened by containerd. Golang 1.14 is going to support this, and then we can get rid of our own implementation. (See proposal: os: Create/Open/OpenFile() set FILE_SHARE_DELETE on windows golang/go#32088)
Commit 3: Add default platform comparer for WCOW process isolation, which requires matching the major, minor and build version of the OS. (See https://docs.microsoft.com/en-us/virtualization/windowscontainers/deploy-containers/version-compatibility#build-number-new-release-of-windows)
Commit 4: Add actual windows implementation.
Commit 5: Add CRI validation test in the presubmit.

pkg/containerd/opts/spec.go

pkg/containerd/platforms/default_windows_test.go

pkg/netns/netns_windows.go

pkg/server/container_create_windows.go

jterry75 · 2019-09-05T17:55:39Z

pkg/server/container_create_windows.go

+	if username != "" {
+		specOpts = append(specOpts, oci.WithUser(username))
+	}
+	// TODO(windows): Add CredentialSpec support.


microsoft/hcsshim#347 for tracking

jterry75

This is awesome!

jterry75 · 2019-09-05T17:57:19Z

pkg/server/container_create_windows.go

+	}
+	// TODO(windows): Add CredentialSpec support.
+
+	for pKey, pValue := range getPassthroughAnnotations(sandboxConfig.Annotations,


Can you add a TODO for ContainerAnnotations pending: #1260

And I still need to talk with @jiayingz about how we should pass the annotation.

The unit test will catch this anyway, don't worry. :) One good thing about sharing the common unit test in this PR.

This merged want to update?

Already updated. See below.

pkg/server/helpers_windows.go

Random-Liu · 2019-09-05T18:33:09Z

OK. No images meet our requirement in either appveyor or travis.

I'll build a simple framework to use GCE to run the CRI validation test.

Random-Liu · 2019-09-09T23:18:48Z

/test pull-cri-containerd-windows-cri

Random-Liu · 2019-09-09T23:52:17Z

/test pull-cri-containerd-windows-cri

Random-Liu · 2019-09-13T01:16:41Z

/test pull-cri-containerd-windows-cri

Random-Liu · 2019-09-13T05:36:55Z

/test pull-cri-containerd-windows-cri

Random-Liu · 2019-09-13T17:27:11Z

/test pull-cri-containerd-windows-cri

jterry75 · 2019-09-15T14:20:22Z

This is awesome!

Random-Liu · 2019-09-16T17:29:29Z

/test pull-cri-containerd-windows-cri

mikebrow

see questions/comments for first half of review :-)

mikebrow · 2019-09-16T19:15:11Z

pkg/config/config_windows.go

-	return PluginConfig{}
+	return PluginConfig{
+		CniConfig: CniConfig{
+			NetworkPluginBinDir:       "C:\\Program Files\\containerd\\cni\\bin",


should probably have an install directory variable somewhere to cover the /root path of the installed files. This so people can install containerd to a different drive/folder

Make sense.

Changed to filepath.Join(os.Getenv("ProgramFiles"), "containerd", "cni", "bin").

Actually the CNI binaries and configs don't necessarily need to reside in the container install directory, the default seems reasonable enough to me. :)

ok .. just a typical courtesy for 3rd party to have an option... if it gets hosted by msft that would be different. Another question for @jterry75

@mikebrow The CNI directory is configurable. :) People always have an option.

I mean for windows... you don't have to use ProgramFiles..

we can address install options and root paths in some other PR after there is a decision on overall deploy of containerd.

@mikebrow containerd is using ProgramData (previously ProgramFiles) for storing all the data https://github.com/containerd/containerd/blob/master/defaults/defaults_windows.go#L29.

As my understanding, the binaries and configurations should be in ProgramFiles. :) @jterry75 may know more about the convention here.

pkg/config/config_windows.go

pkg/containerd/opts/spec_windows.go

mikebrow · 2019-09-16T19:52:24Z

pkg/containerd/platforms/default_windows_test.go

+			OSVersion:    "10.0.17762.1",
+		},
+	}
+	sort.SliceStable(platforms, func(i, j int) bool {


general question on the overall implementation... is there a statement about 32bit support? will users have to create 64bit containers with 32bit supporting 386 libraries?

I think this is more a question for @jterry75 :)

pkg/server/container_create_test.go

pkg/server/container_create_windows_test.go

pkg/server/helpers_unix.go

pkg/server/helpers_windows.go

Random-Liu · 2019-09-17T23:26:50Z

/test pull-cri-containerd-windows-cri

Random-Liu · 2019-09-17T23:50:43Z

/test pull-cri-containerd-windows-cri

pkg/server/helpers_windows.go

mikebrow

LGTM

Signed-off-by: Lantao Liu <lantaol@google.com>

Random-Liu · 2019-09-18T17:47:28Z

Squashed the last commit.

Apply LGTM based on #1264 (review)

Random-Liu · 2019-09-18T17:47:45Z

/test pull-cri-containerd-windows-cri

jterry75 · 2019-09-18T19:47:14Z

WAHOO!!!

PatrickLang · 2019-09-19T21:32:35Z

What's the proper process to build this? Can someone update the doc and clarify whether you're doing cross or native builds?

Random-Liu · 2019-09-19T21:54:11Z

@PatrickLang Just GOOS=windows make on linux, or make on windows. Filed https://github.com/containerd/cri/issues/new to track that.

daxmc99 · 2019-10-15T20:43:57Z

I think the doc issue mentioned should be #1285

Random-Liu added this to the v1.4 milestone Sep 5, 2019

Random-Liu assigned jterry75, yujuhong and mikebrow Sep 5, 2019

k8s-ci-robot added the size/XXL label Sep 5, 2019

Random-Liu force-pushed the windows-support-2 branch 6 times, most recently from ca19b53 to e41a2fd Compare September 5, 2019 08:19

jterry75 reviewed Sep 5, 2019

View reviewed changes

pkg/containerd/opts/spec.go Show resolved Hide resolved

jterry75 reviewed Sep 5, 2019

View reviewed changes

pkg/containerd/platforms/default_windows_test.go Show resolved Hide resolved

jterry75 reviewed Sep 5, 2019

View reviewed changes

pkg/netns/netns_windows.go Outdated Show resolved Hide resolved

jterry75 reviewed Sep 5, 2019

View reviewed changes

pkg/server/container_create_windows.go Show resolved Hide resolved

jterry75 reviewed Sep 5, 2019

View reviewed changes

Random-Liu force-pushed the windows-support-2 branch from e41a2fd to 66711f6 Compare September 5, 2019 17:55

jterry75 reviewed Sep 5, 2019

View reviewed changes

Random-Liu force-pushed the windows-support-2 branch from 66711f6 to d79a8f3 Compare September 5, 2019 18:08

Random-Liu force-pushed the windows-support-2 branch 4 times, most recently from e261abd to 3a57573 Compare September 9, 2019 23:17

Random-Liu force-pushed the windows-support-2 branch from 3a57573 to c2f4621 Compare September 9, 2019 23:49

Random-Liu force-pushed the windows-support-2 branch 2 times, most recently from 07fad6f to a4ddc01 Compare September 10, 2019 00:13

Random-Liu mentioned this pull request Sep 10, 2019

Add Windows Server Container Support (WCOW with process isolation) #1257

Closed

Random-Liu force-pushed the windows-support-2 branch from f99c407 to de5a901 Compare September 13, 2019 01:16

Random-Liu force-pushed the windows-support-2 branch from de5a901 to 12943e4 Compare September 13, 2019 05:36

Random-Liu force-pushed the windows-support-2 branch from 12943e4 to c77f496 Compare September 13, 2019 17:27

Random-Liu force-pushed the windows-support-2 branch from c77f496 to 945d002 Compare September 16, 2019 17:29

mikebrow reviewed Sep 16, 2019

View reviewed changes

Random-Liu force-pushed the windows-support-2 branch from 3ac2004 to cc45ccd Compare September 17, 2019 23:50

mikebrow reviewed Sep 18, 2019

View reviewed changes

pkg/server/helpers_windows.go Show resolved Hide resolved

mikebrow approved these changes Sep 18, 2019

View reviewed changes

Random-Liu added 4 commits September 18, 2019 10:46

Open/create log file with FILE_SHARE_DELETE on windows

c6cb25c

Signed-off-by: Lantao Liu <lantaol@google.com>

Add windows image platform comparer

bbcf564

Signed-off-by: Lantao Liu <lantaol@google.com>

Add windows implmenetation

dc964de

Signed-off-by: Lantao Liu <lantaol@google.com>

Add CRI Validation test.

7e96aaa

Signed-off-by: Lantao Liu <lantaol@google.com>

Random-Liu force-pushed the windows-support-2 branch from cc45ccd to 7e96aaa Compare September 18, 2019 17:46

Random-Liu added the lgtm label Sep 18, 2019

Random-Liu merged commit 9d60f9c into containerd:master Sep 18, 2019

Random-Liu deleted the windows-support-2 branch September 18, 2019 18:38

TBBle mentioned this pull request Jul 16, 2020

Default windows configuration path is inconsistent containerd/containerd#4392

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Implementation for WCOW process isolation support. #1264

Add Implementation for WCOW process isolation support. #1264

Random-Liu commented Sep 5, 2019

jterry75 Sep 5, 2019

jterry75 left a comment

jterry75 Sep 5, 2019

Random-Liu Sep 5, 2019 •

edited

Loading

jterry75 Sep 16, 2019

Random-Liu Sep 16, 2019 •

edited

Loading

Random-Liu commented Sep 5, 2019

Random-Liu commented Sep 9, 2019

Random-Liu commented Sep 9, 2019

Random-Liu commented Sep 13, 2019

Random-Liu commented Sep 13, 2019

Random-Liu commented Sep 13, 2019

jterry75 commented Sep 15, 2019

Random-Liu commented Sep 16, 2019

mikebrow left a comment

mikebrow Sep 16, 2019

Random-Liu Sep 16, 2019

Random-Liu Sep 17, 2019

mikebrow Sep 17, 2019

Random-Liu Sep 17, 2019

mikebrow Sep 18, 2019

mikebrow Sep 18, 2019

Random-Liu Sep 18, 2019

mikebrow Sep 16, 2019

Random-Liu Sep 17, 2019

Random-Liu commented Sep 17, 2019

Random-Liu commented Sep 17, 2019

mikebrow left a comment

Random-Liu commented Sep 18, 2019

Random-Liu commented Sep 18, 2019

jterry75 commented Sep 18, 2019

PatrickLang commented Sep 19, 2019

Random-Liu commented Sep 19, 2019

daxmc99 commented Oct 15, 2019

Add Implementation for WCOW process isolation support. #1264

Add Implementation for WCOW process isolation support. #1264

Conversation

Random-Liu commented Sep 5, 2019

Choose a reason for hiding this comment

jterry75 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Random-Liu Sep 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Random-Liu Sep 16, 2019 • edited Loading

Choose a reason for hiding this comment

Random-Liu commented Sep 5, 2019

Random-Liu commented Sep 9, 2019

Random-Liu commented Sep 9, 2019

Random-Liu commented Sep 13, 2019

Random-Liu commented Sep 13, 2019

Random-Liu commented Sep 13, 2019

jterry75 commented Sep 15, 2019

Random-Liu commented Sep 16, 2019

mikebrow 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

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

Random-Liu commented Sep 17, 2019

Random-Liu commented Sep 17, 2019

mikebrow left a comment

Choose a reason for hiding this comment

Random-Liu commented Sep 18, 2019

Random-Liu commented Sep 18, 2019

jterry75 commented Sep 18, 2019

PatrickLang commented Sep 19, 2019

Random-Liu commented Sep 19, 2019

daxmc99 commented Oct 15, 2019

Random-Liu Sep 5, 2019 •

edited

Loading

Random-Liu Sep 16, 2019 •

edited

Loading