From 3e7e5efbc6a5c52203074e428e6017e11effbc2c Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Fri, 27 Aug 2021 13:10:49 -0700 Subject: [PATCH 1/2] Rework how working directory for job containers Instead of taking the working directory as is, change to joining the working directory requested with where the sandbox volume is located. It's expected that the default behavior would be to treat all paths as relative to the volume as this would be equivalent to a normal Windows Server Containers behavior. For example: A working directory of C:\ would become C:\C\12345678\ A working directory of C:\work\dir would become C:\C\12345678\work\dir Signed-off-by: Daniel Canter --- internal/jobcontainers/jobcontainer.go | 42 ++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/internal/jobcontainers/jobcontainer.go b/internal/jobcontainers/jobcontainer.go index 86a98e26c2..6702b673c1 100644 --- a/internal/jobcontainers/jobcontainer.go +++ b/internal/jobcontainers/jobcontainer.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "regexp" "strings" "sync" @@ -176,11 +177,38 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_ // Replace any occurences of the sandbox mount point env variable in the commandline. // %CONTAINER_SANDBOX_MOUNTPOINT%\mybinary.exe -> C:\C\123456789\mybinary.exe - commandLine := c.replaceWithMountPoint(conf.CommandLine) + commandLine, _ := c.replaceWithMountPoint(conf.CommandLine) + + removeDriveLetter := func(name string) string { + // If just the letter and colon (C:) then replace with a single backslash. Else just trim the drive letter and leave the rest of the + // path. + if len(name) == 2 && name[1] == ':' { + name = "\\" + } else if len(name) > 2 && name[1] == ':' { + name = name[2:] + } + return name + } workDir := c.sandboxMount if conf.WorkingDirectory != "" { - workDir = c.replaceWithMountPoint(conf.WorkingDirectory) + var changed bool + // For now, we join the working directory requested with where the sandbox volume is located. It's expected that the default behavior + // would be to treat all paths as relative to the volume. + // + // For example: + // A working directory of C:\ would become C:\C\12345678\ + // A working directory of C:\work\dir would become C:\C\12345678\work\dir + // + // The below calls replaceWithMountPoint to replace any occurrences of the environment variable that points to where the container image + // volume is mounted. + workDir, changed = c.replaceWithMountPoint(conf.WorkingDirectory) + // If the working directory was changed, that means the user supplied %CONTAINER_SANDBOX_MOUNT_POINT%\\my\dir or something similar. + // In that case there's nothing left to do, as we don't want to join it with the mount point again.. If it *wasn't* changed, then we + // need to join it with the mount point, as it's some normal path. + if !changed { + workDir = filepath.Join(c.sandboxMount, removeDriveLetter(workDir)) + } } // Reassign commandline here in case it needed to be quoted. For example if "foo bar baz" was supplied, and @@ -575,8 +603,10 @@ func systemProcessInformation() ([]*winapi.SYSTEM_PROCESS_INFORMATION, error) { return procInfos, nil } -// Takes a string and replaces any occurences of CONTAINER_SANDBOX_MOUNT_POINT with where the containers volume is mounted. -func (c *JobContainer) replaceWithMountPoint(str string) string { - str = strings.ReplaceAll(str, "%"+sandboxMountPointEnvVar+"%", c.sandboxMount[:len(c.sandboxMount)-1]) - return strings.ReplaceAll(str, "$env:"+sandboxMountPointEnvVar, c.sandboxMount[:len(c.sandboxMount)-1]) +// Takes a string and replaces any occurences of CONTAINER_SANDBOX_MOUNT_POINT with where the containers volume is mounted, as well as returning +// if the string actually contained the environment variable. +func (c *JobContainer) replaceWithMountPoint(str string) (string, bool) { + newStr := strings.ReplaceAll(str, "%"+sandboxMountPointEnvVar+"%", c.sandboxMount[:len(c.sandboxMount)-1]) + newStr = strings.ReplaceAll(newStr, "$env:"+sandboxMountPointEnvVar, c.sandboxMount[:len(c.sandboxMount)-1]) + return newStr, str != newStr } From 9518addeda73352f3209ff248142e5bb2531aacb Mon Sep 17 00:00:00 2001 From: Daniel Canter Date: Mon, 13 Sep 2021 16:03:51 -0700 Subject: [PATCH 2/2] Add test cases for the working directory functionality This change adds a couple tests to make sure that the working directory functions as expected. Also some very small adjustments on the dockerfiles for the other tests (which really didn't need to be changed, but makes it more explicit). Signed-off-by: Daniel Canter --- test/cri-containerd/jobcontainer_test.go | 106 ++++++++++++++++-- test/cri-containerd/main.go | 19 ++-- .../jobcontainer_createvhd/Dockerfile | 2 +- .../test-images/jobcontainer_etw/Dockerfile | 2 +- .../test-images/jobcontainer_hns/Dockerfile | 8 +- .../jobcontainer_workdir/Dockerfile | 15 +++ .../test-images/jobcontainer_workdir/main.go | 15 +++ 7 files changed, 141 insertions(+), 26 deletions(-) create mode 100644 test/cri-containerd/test-images/jobcontainer_workdir/Dockerfile create mode 100644 test/cri-containerd/test-images/jobcontainer_workdir/main.go diff --git a/test/cri-containerd/jobcontainer_test.go b/test/cri-containerd/jobcontainer_test.go index ec1f2daeef..ad1db8d06b 100644 --- a/test/cri-containerd/jobcontainer_test.go +++ b/test/cri-containerd/jobcontainer_test.go @@ -154,7 +154,7 @@ func Test_RunContainer_HNS_JobContainer_WCOW(t *testing.T) { networkName := fmt.Sprintf("JobContainer-Network-%s", podID) containerRequest.Config.Command = []string{ - "go/src/hns/hns.exe", + "hns.exe", networkName, } @@ -207,7 +207,7 @@ func Test_RunContainer_VHD_JobContainer_WCOW(t *testing.T) { vhdPath := filepath.Join(dir, "test.vhdx") containerRequest.Config.Command = []string{ - "go/src/vhd/vhd.exe", + "vhd.exe", vhdPath, } @@ -266,7 +266,7 @@ func Test_RunContainer_ETW_JobContainer_WCOW(t *testing.T) { dumpFile = filepath.Join(dir, "output.xml") ) containerRequest.Config.Command = []string{ - "go/src/etw/etw.exe", + "etw.exe", networkName, etlFile, dumpFile, @@ -368,10 +368,8 @@ func Test_RunContainer_JobContainer_VolumeMount(t *testing.T) { name string containerName string requiredFeatures []string - runtimeHandler string sandboxImage string containerImage string - cmd []string exec []string mounts []*runtime.Mount } @@ -381,10 +379,8 @@ func Test_RunContainer_JobContainer_VolumeMount(t *testing.T) { name: "JobContainer_VolumeMount_DriveLetter", containerName: t.Name() + "-Container-DriveLetter", requiredFeatures: []string{featureWCOWProcess, featureHostProcess}, - runtimeHandler: wcowProcessRuntimeHandler, sandboxImage: imageWindowsNanoserver, containerImage: imageWindowsNanoserver, - cmd: []string{"ping", "-t", "127.0.0.1"}, mounts: mountDriveLetter, exec: []string{"cmd", "/c", "dir", "%CONTAINER_SANDBOX_MOUNT_POINT%\\path\\in\\container\\tmpfile"}, }, @@ -392,10 +388,8 @@ func Test_RunContainer_JobContainer_VolumeMount(t *testing.T) { name: "JobContainer_VolumeMount_NoDriveLetter", containerName: t.Name() + "-Container-NoDriveLetter", requiredFeatures: []string{featureWCOWProcess, featureHostProcess}, - runtimeHandler: wcowProcessRuntimeHandler, sandboxImage: imageWindowsNanoserver, containerImage: imageWindowsNanoserver, - cmd: []string{"ping", "-t", "127.0.0.1"}, mounts: mountNoDriveLetter, exec: []string{"cmd", "/c", "dir", "%CONTAINER_SANDBOX_MOUNT_POINT%\\path\\in\\container\\tmpfile"}, }, @@ -403,10 +397,8 @@ func Test_RunContainer_JobContainer_VolumeMount(t *testing.T) { name: "JobContainer_VolumeMount_SingleFile", containerName: t.Name() + "-Container-SingleFile", requiredFeatures: []string{featureWCOWProcess, featureHostProcess}, - runtimeHandler: wcowProcessRuntimeHandler, sandboxImage: imageWindowsNanoserver, containerImage: imageWindowsNanoserver, - cmd: []string{"ping", "-t", "127.0.0.1"}, mounts: mountSingleFile, exec: []string{"cmd", "/c", "type", "%CONTAINER_SANDBOX_MOUNT_POINT%\\path\\in\\container\\testfile"}, }, @@ -445,3 +437,95 @@ func Test_RunContainer_JobContainer_VolumeMount(t *testing.T) { }) } } + +func Test_RunContainer_WorkingDirectory_JobContainer_WCOW(t *testing.T) { + client := newTestRuntimeClient(t) + + type config struct { + name string + containerName string + workDir string + requiredFeatures []string + sandboxImage string + containerImage string + cmd []string + } + + tests := []config{ + { + name: "JobContainer_WorkDir_DriveLetter", + workDir: "C:\\go\\", + requiredFeatures: []string{featureWCOWProcess, featureHostProcess}, + sandboxImage: imageWindowsNanoserver, + containerImage: imageJobContainerWorkDir, + cmd: []string{"src\\workdir\\workdir.exe"}, + }, + { + name: "JobContainer_WorkDir_NoDriveLetter", + workDir: "/go", + requiredFeatures: []string{featureWCOWProcess, featureHostProcess}, + sandboxImage: imageWindowsNanoserver, + containerImage: imageJobContainerWorkDir, + cmd: []string{"src/workdir/workdir.exe"}, + }, + { + name: "JobContainer_WorkDir_Default", // Just use the workdir from the image, which is C:\\go\\src\\workdir + requiredFeatures: []string{featureWCOWProcess, featureHostProcess}, + sandboxImage: imageWindowsNanoserver, + containerImage: imageJobContainerWorkDir, + cmd: []string{"workdir.exe"}, + }, + { + name: "JobContainer_WorkDir_EnvVar", // Test that putting the envvar in the workdir functions. + workDir: "$env:CONTAINER_SANDBOX_MOUNT_POINT\\go\\src\\workdir\\", + requiredFeatures: []string{featureWCOWProcess, featureHostProcess}, + sandboxImage: imageWindowsNanoserver, + containerImage: imageJobContainerWorkDir, + cmd: []string{"workdir.exe"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + requireFeatures(t, test.requiredFeatures...) + + requiredImages := []string{test.sandboxImage, test.containerImage} + pullRequiredImages(t, requiredImages) + + podctx := context.Background() + sandboxRequest := getJobContainerPodRequestWCOW(t) + + podID := runPodSandbox(t, client, podctx, sandboxRequest) + defer removePodSandbox(t, client, podctx, podID) + defer stopPodSandbox(t, client, podctx, podID) + + containerRequest := &runtime.CreateContainerRequest{ + Config: &runtime.ContainerConfig{ + Metadata: &runtime.ContainerMetadata{ + Name: test.name, + }, + Image: &runtime.ImageSpec{ + Image: test.containerImage, + }, + Command: test.cmd, + WorkingDir: test.workDir, + Annotations: map[string]string{ + oci.AnnotationHostProcessContainer: "true", + oci.AnnotationHostProcessInheritUser: "true", + }, + Windows: &runtime.WindowsContainerConfig{}, + }, + PodSandboxId: podID, + SandboxConfig: sandboxRequest.Config, + } + + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + + containerID := createContainer(t, client, ctx, containerRequest) + defer removeContainer(t, client, ctx, containerID) + startContainer(t, client, ctx, containerID) + defer stopContainer(t, client, ctx, containerID) + }) + } +} diff --git a/test/cri-containerd/main.go b/test/cri-containerd/main.go index 60abfdb353..d723f52d49 100644 --- a/test/cri-containerd/main.go +++ b/test/cri-containerd/main.go @@ -45,15 +45,16 @@ const ( testVMServiceAddress = "C:\\ContainerPlat\\vmservice.sock" testVMServiceBinary = "C:\\Containerplat\\vmservice.exe" - lcowRuntimeHandler = "runhcs-lcow" - imageLcowK8sPause = "k8s.gcr.io/pause:3.1" - imageLcowAlpine = "docker.io/library/alpine:latest" - imageLcowCosmos = "cosmosarno/spark-master:2.4.1_2019-04-18_8e864ce" - imageJobContainerHNS = "cplatpublic.azurecr.io/jobcontainer_hns:latest" - imageJobContainerETW = "cplatpublic.azurecr.io/jobcontainer_etw:latest" - imageJobContainerVHD = "cplatpublic.azurecr.io/jobcontainer_vhd:latest" - alpineAspNet = "mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine3.11" - alpineAspnetUpgrade = "mcr.microsoft.com/dotnet/core/aspnet:3.1.2-alpine3.11" + lcowRuntimeHandler = "runhcs-lcow" + imageLcowK8sPause = "k8s.gcr.io/pause:3.1" + imageLcowAlpine = "docker.io/library/alpine:latest" + imageLcowCosmos = "cosmosarno/spark-master:2.4.1_2019-04-18_8e864ce" + imageJobContainerHNS = "cplatpublic.azurecr.io/jobcontainer_hns:latest" + imageJobContainerETW = "cplatpublic.azurecr.io/jobcontainer_etw:latest" + imageJobContainerVHD = "cplatpublic.azurecr.io/jobcontainer_vhd:latest" + imageJobContainerWorkDir = "cplatpublic.azurecr.io/jobcontainer_workdir:latest" + alpineAspNet = "mcr.microsoft.com/dotnet/core/aspnet:3.1-alpine3.11" + alpineAspnetUpgrade = "mcr.microsoft.com/dotnet/core/aspnet:3.1.2-alpine3.11" // Default account name for use with GMSA related tests. This will not be // present/you will not have access to the account on your machine unless // your environment is configured properly. diff --git a/test/cri-containerd/test-images/jobcontainer_createvhd/Dockerfile b/test/cri-containerd/test-images/jobcontainer_createvhd/Dockerfile index 189ab2dd8d..21bc806c7b 100644 --- a/test/cri-containerd/test-images/jobcontainer_createvhd/Dockerfile +++ b/test/cri-containerd/test-images/jobcontainer_createvhd/Dockerfile @@ -4,7 +4,7 @@ FROM golang:1.15.10-nanoserver-1809 # Get administrator privileges USER containeradministrator -WORKDIR /go/src/vhd +WORKDIR C:\\go\\src\\vhd COPY main.go . RUN go get -d -v ./... diff --git a/test/cri-containerd/test-images/jobcontainer_etw/Dockerfile b/test/cri-containerd/test-images/jobcontainer_etw/Dockerfile index 8d860ac273..97976860a7 100644 --- a/test/cri-containerd/test-images/jobcontainer_etw/Dockerfile +++ b/test/cri-containerd/test-images/jobcontainer_etw/Dockerfile @@ -4,7 +4,7 @@ FROM golang:1.15.10-nanoserver-1809 # Get administrator privileges USER containeradministrator -WORKDIR /go/src/etw +WORKDIR C:\\go\\src\\etw COPY . . RUN go get -d -v ./... diff --git a/test/cri-containerd/test-images/jobcontainer_hns/Dockerfile b/test/cri-containerd/test-images/jobcontainer_hns/Dockerfile index 3c0f8b990d..4dbe8f441c 100644 --- a/test/cri-containerd/test-images/jobcontainer_hns/Dockerfile +++ b/test/cri-containerd/test-images/jobcontainer_hns/Dockerfile @@ -1,6 +1,6 @@ -# This dockerfile builds a super barebones container image that includes a binary to do a single HNS operation to -# validate that we can actually talk to HNS in a job container. As this is a huge usecase for job containers this is paramount -# to test. The binary in the image will NOT function if this image is used for a normal windows container, both process and hypervisor isolated. +# This dockerfile builds a super barebones container image that includes a binary to do a single HNS operation to +# validate that we can actually talk to HNS in a job container. As this is a huge usecase for job containers this is paramount +# to test. The binary in the image will NOT function if this image is used for a normal windows container, both process and hypervisor isolated. # Irrelevant what image version we use for job containers as there's no container <-> host OS version restraint. FROM golang:1.15.10-nanoserver-1809 @@ -8,7 +8,7 @@ FROM golang:1.15.10-nanoserver-1809 # Get administrator privileges USER containeradministrator -WORKDIR /go/src/hns +WORKDIR C:\\go\\src\\hns COPY main.go . RUN go get -d -v ./... diff --git a/test/cri-containerd/test-images/jobcontainer_workdir/Dockerfile b/test/cri-containerd/test-images/jobcontainer_workdir/Dockerfile new file mode 100644 index 0000000000..7a23de790f --- /dev/null +++ b/test/cri-containerd/test-images/jobcontainer_workdir/Dockerfile @@ -0,0 +1,15 @@ +# This dockerfile builds a super barebones container image that includes a binary to do a single HNS operation to +# validate that we can actually talk to HNS in a job container. As this is a huge usecase for job containers this is paramount +# to test. The binary in the image will NOT function if this image is used for a normal windows container, both process and hypervisor isolated. + +# Irrelevant what image version we use for job containers as there's no container <-> host OS version restraint. +FROM golang:1.15.10-nanoserver-1809 + +# Get administrator privileges +USER containeradministrator + +WORKDIR C:\\go\\src\\workdir +COPY main.go . + +RUN go get -d -v ./... +RUN go build -mod=mod \ No newline at end of file diff --git a/test/cri-containerd/test-images/jobcontainer_workdir/main.go b/test/cri-containerd/test-images/jobcontainer_workdir/main.go new file mode 100644 index 0000000000..545ab86146 --- /dev/null +++ b/test/cri-containerd/test-images/jobcontainer_workdir/main.go @@ -0,0 +1,15 @@ +package main + +import ( + "fmt" + "time" +) + +// The contents of the program are irrelevant. This binary will be placed in an image that is used for testing the working directory behavior +// for job containers. So as long as the binary is launched is all that's being tested. +func main() { + for { + fmt.Println("Hello world") + time.Sleep(time.Second * 5) + } +}