Add WCOW and vSMB functional tests

Update and un-skip WCOW uVM and container tests (and add WCOW uVM
benchmarks), as well as WCOW vSMB and LCOW boto files tests.

Add WCOW host process tests, including dedicated tests for setting
username, and verifying hostname and volume mounts.

Moved:
 - `lcow_bench_test.go` to `uvm_bench_test.go`
 - `lcow_container_test.go` to `container_test.go`
 - `lcow_test.go` to `lcow_uvm_test.go` and `uvm_test.go`

Fix bug where removing a direct-mapped vSMB share fails.

Run (non-virtualization/uVM) functional tests within CI.

Make sure container specs are created with the default working
directory (`C:\`), similar to how `internal\cmd` works).

Relies on PR: #1974

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>

.github/workflows/ci.yml

@@ -6,7 +6,7 @@ on:
 env:
   GO_VERSION: "oldstable"
 
-  GO_BUILD_CMD: "go build \"-ldflags=-s -w\" -trimpath"
+  GO_BUILD_CMD: 'go build "-ldflags=-s -w" -trimpath'
   GO_BUILD_TEST_CMD: "go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional"
 
   GOTESTSUM_VERSION: "latest"
@@ -325,6 +325,30 @@ jobs:
       - name: Install gotestsum
         run: go install gotest.tools/gotestsum@${{ env.GOTESTSUM_VERSION }}
 
+      # Download PsExec so we can run (functional) tests as 'NT Authority\System'.
+      # Needed for hostprocess tests, as well ensuring backup and restore privileges for
+      # unpacking WCOW images.
+      - name: Install PsExec.exe
+        run: |
+          New-Item -ItemType Directory -Force '${{ github.workspace }}\bin' > $null
+          '${{ github.workspace }}\bin' | Out-File -FilePath $env:GITHUB_PATH -Encoding utf8 -Append
+
+          curl.exe -L --no-progress-meter --fail-with-body -o 'C:\PSTools.zip' `
+            'https://download.sysinternals.com/files/PSTools.zip' 2>&1
+          if ( $LASTEXITCODE ) {
+            Write-Output '::error::Could not download PSTools.zip'
+            exit $LASTEXITCODE
+          }
+
+          tar.exe xf 'C:\PSTools.zip' -C '${{ github.workspace }}\bin' 'PsExec*' 2>&1
+          if ( $LASTEXITCODE ) {
+            Write-Output '::error::Could not extract PsExec.exe'
+            exit $LASTEXITCODE
+          }
+
+          # accept the eula
+          & '${{ github.workspace }}/bin/psexec' -accepteula -nobanner cmd /c "exit 0" 2>$null
+
       # run tests
       - name: Test repo
         run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr -tags admin -timeout=20m ./...
@@ -354,13 +378,42 @@ jobs:
           ${{ env.GOTESTSUM_CMD_RAW }} ./containerd-shim-runhcs-v1.test.exe '-test.v'
         working-directory: test
 
+      - name: Build and run functional testing binary
+        run: |
+          ${{ env.GO_BUILD_TEST_CMD }} ./functional
+          if ( $LASTEXITCODE ) {
+            Write-Output '::error::Could not build functional.test.exe'
+            exit $LASTEXITCODE
+          }
+
+          # PsExec doesn't load GOBIN into path, so resolve gotestsum path
+          $gotestsum = Get-Command -Name 'gotestsum' -CommandType Application -ErrorAction 'Stop' |
+            Select-Object -First 1 -ExpandProperty Source
+          if ( [string]::IsNullOrEmpty($gotestsum) ) {
+            Write-Output '::error::could not find 'gotestsum.exe' path'
+            exit $LASTEXITCODE
+          }
+
+          # Don't run uVM (ie, nested virt) or LCOW integrity tests
+          $cmd = '${{ env.GOTESTSUM_CMD_RAW }} ./functional.test.exe -exclude="LCOW,LCOWIntegrity,uVM" -test.timeout=1h -test.v'
+          $cmd = $cmd -replace 'gotestsum', $gotestsum
+          Write-Host "gotestsum command: $cmd"
+
+          # Apparently, in a GH runner, PsExec always runs noninteractively (even with `-i`)
+          # and doesn't capture or redirect std IO.
+          # So redirect stdout/stderr to a file.
+          psexec -nobanner -w (Get-Location) -s cmd /c "$cmd > out.txt 2>&1"
+          $ec = $LASTEXITCODE
+
+          Get-Content out.txt
+
+          exit $ec
+        working-directory: test
+
       # build testing binaries
       - name: Build cri-containerd Testing Binary
         run: ${{ env.GO_BUILD_TEST_CMD }} ./cri-containerd
         working-directory: test
-      - name: Build functional Testing Binary
-        run: ${{ env.GO_BUILD_TEST_CMD }} ./functional
-        working-directory: test
       - name: Build runhcs Testing Binary
         run: ${{ env.GO_BUILD_TEST_CMD }} ./runhcs
         working-directory: test
@@ -688,7 +741,7 @@ jobs:
 
       - name: Install kmod
         shell: bash
-        run: | 
+        run: |
           sudo apt-get install -y libkmod-dev
 
       - name: Set version info

internal/jobcontainers/jobcontainer.go

@@ -31,11 +31,6 @@ import (
 	"golang.org/x/sys/windows"
 )
 
-var (
-	fileBindingSupport   bool
-	checkBindSupportOnce sync.Once
-)
-
 const (
 	// jobContainerNameFmt is the naming format that job objects for job containers will follow.
 	jobContainerNameFmt = "JobContainer_%s"
@@ -186,15 +181,8 @@ func Create(ctx context.Context, id string, s *specs.Spec, createOpts CreateOpti
 	// show up at beforehand as you would need to know the containers ID before you launched it. Now that the
 	// rootfs location can be static, a user can easily supply C:\hpc\rest\of\path as their work dir and still
 	// supply anything outside of C:\hpc if they want another location on the host.
-	checkBindSupportOnce.Do(func() {
-		bindDLL := `C:\windows\system32\bindfltapi.dll`
-		if _, err := os.Stat(bindDLL); err == nil {
-			fileBindingSupport = true
-		}
-	})
-
 	var closer resources.ResourceCloser
-	if fileBindingSupport {
+	if FileBindingSupported() {
 		closer, err = container.bindSetup(ctx, s, createOpts)
 	} else {
 		closer, err = container.fallbackSetup(ctx, s, createOpts)
@@ -259,7 +247,7 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_
 		// 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, and there's
 		// no bindflt support then we need to join it with the mount point, as it's some normal path.
-		if !changed && !fileBindingSupport {
+		if !changed && !FileBindingSupported() {
 			workDir = filepath.Join(c.rootfsLocation, removeDriveLetter(workDir))
 		}
 	}
@@ -340,7 +328,7 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_
 	// (cmd in this case) after launch can now see C:\<rootfslocation> as it's in the silo. We could
 	// also add a new mode/flag for the shim where it's just a dummy process launcher, so we can invoke
 	// the shim instead of cmd and have more control over things.
-	if fileBindingSupport {
+	if FileBindingSupported() {
 		commandLine = "cmd /c " + commandLine
 	}
 

internal/jobcontainers/storage.go

@@ -5,24 +5,34 @@ package jobcontainers
 import (
 	"context"
 	"fmt"
+	"os"
+	"path/filepath"
+	"sync"
+
+	specs "github.com/opencontainers/runtime-spec/specs-go"
 
 	"github.com/Microsoft/hcsshim/internal/layers"
 	"github.com/Microsoft/hcsshim/internal/log"
 	"github.com/Microsoft/hcsshim/internal/resources"
-	specs "github.com/opencontainers/runtime-spec/specs-go"
 )
 
 // fallbackRootfsFormat is the fallback location for the rootfs if file binding support isn't available.
 // %s will be expanded with the container ID. Trailing backslash required for SetVolumeMountPoint and
-// DeleteVolumeMountPoint
+// DeleteVolumeMountPoint.
 const fallbackRootfsFormat = `C:\hpc\%s\`
 
 // defaultSiloRootfsLocation is the default location the rootfs for the container will show up
 // inside of a given silo. If bind filter support isn't available the rootfs will be
-// C:\hpc\<containerID>
+// C:\hpc\<containerID>.
 const defaultSiloRootfsLocation = `C:\hpc\`
 
-func (c *JobContainer) mountLayers(ctx context.Context, containerID string, s *specs.Spec, wl layers.WCOWLayers, volumeMountPath string) (_ resources.ResourceCloser, err error) {
+func (c *JobContainer) mountLayers(
+	ctx context.Context,
+	containerID string,
+	s *specs.Spec,
+	wl layers.WCOWLayers,
+	volumeMountPath string,
+) (_ resources.ResourceCloser, err error) {
 	if s.Root == nil {
 		s.Root = &specs.Root{}
 	}
@@ -75,3 +85,22 @@ func (c *JobContainer) setupRootfsBinding(root, target string) error {
 	}
 	return nil
 }
+
+var fileBindingSupportedOnce = sync.OnceValues(func() (bool, error) {
+	// TODO: use windows.NewLazySystemDLL("bindfltapi.dll").Load() (or windows.LoadLibraryEx directly)
+
+	root := os.Getenv("SystemRoot")
+	if root == "" {
+		root = `C:\windows` // shouldn't really need this fall back, but ...
+	}
+	bindDLL := filepath.Join(root, `system32\bindfltapi.dll`)
+	if _, err := os.Stat(bindDLL); err != nil {
+		return false, err
+	}
+	return true, nil
+})
+
+func FileBindingSupported() bool {
+	b, _ := fileBindingSupportedOnce()
+	return b
+}

internal/uvm/vsmb.go

@@ -133,7 +133,12 @@ func forceNoDirectMap(path string) (bool, error) {
 	var info winapi.FILE_ID_INFO
 	// We check for any error, rather than just ERROR_INVALID_PARAMETER. It seems better to also
 	// fall back if e.g. some other backing filesystem is used which returns a different error.
-	if err := windows.GetFileInformationByHandleEx(h, winapi.FileIdInfo, (*byte)(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info))); err != nil {
+	if err := windows.GetFileInformationByHandleEx(
+		h,
+		winapi.FileIdInfo,
+		(*byte)(unsafe.Pointer(&info)),
+		uint32(unsafe.Sizeof(info)),
+	); err != nil {
 		return true, nil
 	}
 	return false, nil
@@ -282,6 +287,24 @@ func (uvm *UtilityVM) removeVSMB(ctx context.Context, hostPath string, readOnly,
 		return nil
 	}
 
+	// Cannot remove a directmapped vSMB share without first closing all open handles to the
+	// share files from inside the the uVM (otherwise, the removal would un-map the files from
+	// the uVM's memory and subsequent access's would fail).
+	// Rather than forgetting about the share on the host side, keep it (with refCount == 0)
+	// in case that directory is re-added back for some reason.
+	//
+	// Note: HCS (vmcompute.exe) issues a remove vSMB request to the guest GCS iff:
+	//  - vmwp.exe direct mapped the vSMB share; and
+	//  - the GCS (on its internal bridge) has the PurgeVSmbCachedHandlesSupported capability.
+	// We do not (currently) have the ability to check for either.
+	if !share.options.NoDirectmap {
+		log.G(ctx).WithFields(logrus.Fields{
+			"name": share.name,
+			"path": hostPath,
+		}).Debug("skipping remove of directmapped vSMB share")
+		return nil
+	}
+
 	modification := &hcsschema.ModifySettingRequest{
 		RequestType:  guestrequest.RequestTypeRemove,
 		Settings:     hcsschema.VirtualSmbShare{Name: share.name},