Rework how working directories function for job containers

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
 }

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,32 +379,26 @@ 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"},
 		},
 		{
 			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"},
 		},
 		{
 			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)
+		})
+	}
+}

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.

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 ./...

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 ./...

test/cri-containerd/test-images/jobcontainer_hns/Dockerfile

@@ -1,14 +1,14 @@
-# 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
 
 # Get administrator privileges
 USER containeradministrator
 
-WORKDIR /go/src/hns
+WORKDIR C:\\go\\src\\hns
 COPY main.go .
 
 RUN go get -d -v ./...

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

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)
+	}
+}