Functional test housekeeping

Remove unused/legacy functional test flags/environment variables.

Unify [TestMain] control flow, so there is only one exit call, and
`defer` is used to run cleanup after the tests are run.

Standardize UVM `default[L|W]COWOptions` to accept a context, and add
context parameter to `namespacedContext`

Remove all build tags aside from `functional`, since features are used
to select the tests to run. This standardizes the functional tests with
the cri-containerd tests, even though the feature names themselves are
different.

Add `test/pkg/uvm.CreateWCOW` function to mirror `CreateLCOW`, and add
`Create` and `CreateAndStart` functions that pick LCOW or WCOW based on
the options provided.

Have uVM scratch and image layers be created under a dedicated and
persisted folder within `%TEMP%` that is excluded from Windows defender.
(The folder will be removed during OS restart, regardless of contents.)

Remove copied OCI spec options from `test/internal/oci`, add new
options for creating HostProcess containers.

Add a `internal\sync.OnceValue`(`Ctx`) function that mirrors
`sync.OnceValues` (introduced in go1.21) to have a type-safe `Once`
function.

Check that required privileges are held (only once) when unpacking
Windows layers.

Fix LCOW tests in `lcow_test.go` that were setting `KernelDirect`
without also updating `KernelFile`.

Add `util.Context` function to create context that times out before test
timeout, to help with timing issues and allow time for cleanup and
logging.

Rename `cri_util` to `criutil`, since underscores are frowned upon in
package names.
Add a `test` prefix to `github.com/Microsoft/hcsshim/test/pkg/*` and
`github.com/Microsoft/hcsshim/test/internal/*` imports to be consistent
across all `test/functional/*` files.

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

.github/workflows/ci.yml

@@ -4,9 +4,15 @@ on:
   - pull_request
 
 env:
-  GO_VERSION: "1.21.x"
+  GO_VERSION: "oldstable"
+
+  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"
-  GOTESTCMD: "gotestsum --format standard-verbose --debug --"
+
+  GOTESTSUM_CMD: "gotestsum --format standard-verbose --debug --"
+  GOTESTSUM_CMD_RAW: "gotestsum --format standard-verbose --debug --raw-command -- go tool test2json -t"
 
 jobs:
   lint:
@@ -280,19 +286,19 @@ jobs:
         run: go install gotest.tools/gotestsum@${{ env.GOTESTSUM_VERSION }}
 
       - name: Test standard security policy
-        run: ${{ env.GOTESTCMD }} -timeout=30m -gcflags=all=-d=checkptr ./pkg/securitypolicy/...
+        run: ${{ env.GOTESTSUM_CMD }} -timeout=30m -gcflags=all=-d=checkptr ./pkg/securitypolicy/...
 
       - name: Test rego security policy
-        run: ${{ env.GOTESTCMD }} -tags=rego -timeout=30m -gcflags=all=-d=checkptr ./pkg/securitypolicy/...
+        run: ${{ env.GOTESTSUM_CMD }} -tags=rego -timeout=30m -gcflags=all=-d=checkptr ./pkg/securitypolicy/...
 
       - name: Test rego policy interpreter
-        run: ${{ env.GOTESTCMD }} -gcflags=all=-d=checkptr ./internal/regopolicyinterpreter/...
+        run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr ./internal/regopolicyinterpreter/...
 
       - name: Run guest code unit tests
-        run: ${{ env.GOTESTCMD }} -gcflags=all=-d=checkptr ./internal/guest/...
+        run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr ./internal/guest/...
 
       - name: Build gcs Testing Binary
-        run: go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional ./gcs
+        run: ${{ env.GO_BUILD_TEST_CMD }} ./gcs
         working-directory: test
 
   test-windows:
@@ -321,34 +327,45 @@ jobs:
 
       # run tests
       - name: Test repo
-        run: ${{ env.GOTESTCMD }} -gcflags=all=-d=checkptr -tags admin ./...
+        run: ${{ env.GOTESTSUM_CMD }} -gcflags=all=-d=checkptr -tags admin -timeout=20m ./...
 
       - name: Run non-functional tests
-        run: ${{ env.GOTESTCMD }} -mod=mod -gcflags=all=-d=checkptr ./internal/... ./pkg/...
+        run: ${{ env.GOTESTSUM_CMD }} -mod=mod -gcflags=all=-d=checkptr ./internal/... ./pkg/...
         working-directory: test
 
-      - name: Run containerd-shim-runhcs-v1 tests
+      - name: Build and run containerd-shim-runhcs-v1 tests
         shell: powershell
         run: |
-          powershell {
-            cd '../..'
-            go build -trimpath -o './test/containerd-shim-runhcs-v1' ./cmd/containerd-shim-runhcs-v1
+          pwsh {
+            cd '..'
+            ${{ env.GO_BUILD_CMD }} -o ./test ./cmd/containerd-shim-runhcs-v1 2>&1
+          }
+          if ( $LASTEXITCODE ) {
+            Write-Output '::error::Could not build containerd-shim-runhcs-v1.exe'
+            exit $LASTEXITCODE
           }
-          ${{ env.GOTESTCMD }} -mod=mod -tags functional -gcflags=all=-d=checkptr ./...
-        working-directory: test/containerd-shim-runhcs-v1
+
+          ${{ env.GO_BUILD_TEST_CMD }} ./containerd-shim-runhcs-v1
+          if ( $LASTEXITCODE ) {
+            Write-Output '::error::Could not build containerd-shim-runhcs-v1.test.exe'
+            exit $LASTEXITCODE
+          }
+
+          ${{ env.GOTESTSUM_CMD_RAW }} ./containerd-shim-runhcs-v1.test.exe '-test.v'
+        working-directory: test
 
       # build testing binaries
       - name: Build cri-containerd Testing Binary
-        run: go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional ./cri-containerd
+        run: ${{ env.GO_BUILD_TEST_CMD }} ./cri-containerd
         working-directory: test
       - name: Build functional Testing Binary
-        run: go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional ./functional
+        run: ${{ env.GO_BUILD_TEST_CMD }} ./functional
         working-directory: test
       - name: Build runhcs Testing Binary
-        run: go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional ./runhcs
+        run: ${{ env.GO_BUILD_TEST_CMD }} ./runhcs
         working-directory: test
       - name: Build logging-driver Binary
-        run: go build -mod=mod -o sample-logging-driver.exe ./cri-containerd/helpers/log.go
+        run: ${{ env.GO_BUILD_CMD }} -mod=mod -o sample-logging-driver.exe ./cri-containerd/helpers/log.go
         working-directory: test
 
       - uses: actions/upload-artifact@v4
@@ -485,7 +502,7 @@ jobs:
         working-directory: src/github.com/Microsoft/hcsshim
         shell: powershell
         run: |
-          go build -mod vendor -o "${{ github.workspace }}/src/github.com/containerd/containerd/bin/containerd-shim-runhcs-v1.exe" .\cmd\containerd-shim-runhcs-v1
+          ${{ env.GO_BUILD_CMD }} -mod vendor -o "${{ github.workspace }}/src/github.com/containerd/containerd/bin/containerd-shim-runhcs-v1.exe" .\cmd\containerd-shim-runhcs-v1
 
       - name: Install gotestsum
         run: go install gotest.tools/gotestsum@${{ env.GOTESTSUM_VERSION }}
@@ -603,22 +620,35 @@ jobs:
             $LASTEXITCODE = 0
           }
 
-      - run: go build ./cmd/containerd-shim-runhcs-v1
-      - run: go build ./cmd/runhcs
-      - run: go build ./cmd/tar2ext4
-      - run: go build ./cmd/wclayer
-      - run: go build ./cmd/device-util
-      - run: go build ./cmd/ncproxy
-      - run: go build ./cmd/dmverity-vhd
-      - run: go build ./cmd/dmverity-vhd
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/containerd-shim-runhcs-v1
+        name: Build containerd-shim-runhcs-v1.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/runhcs
+        name: Build runhcs.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/tar2ext4
+        name: Build tar2ext4.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/wclayer
+        name: Build wclayer.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/device-util
+        name: Build device-util.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/ncproxy
+        name: Build ncproxy.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/dmverity-vhd
+        name: Build dmverity-vhd.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./cmd/dmverity-vhd
+        name: Build dmverity-vhd
         env:
           GOOS: linux
           GOARCH: amd64
-      - run: go build ./internal/tools/grantvmgroupaccess
-      - run: go build ./internal/tools/networkagent
-      - run: go build ./internal/tools/securitypolicy
-      - run: go build ./internal/tools/uvmboot
-      - run: go build ./internal/tools/zapdir
+      - run: ${{ env.GO_BUILD_CMD }} ./internal/tools/grantvmgroupaccess
+        name: Build grantvmgroupaccess.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./internal/tools/networkagent
+        name: Build networkagent.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./internal/tools/securitypolicy
+        name: Build securitypolicy.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./internal/tools/uvmboot
+        name: Build uvmboot.exe
+      - run: ${{ env.GO_BUILD_CMD }} ./internal/tools/zapdir
+        name: Build zapdir.exe
 
       - uses: actions/upload-artifact@v4
         if: ${{ github.event_name == 'pull_request' }}

internal/cmd/cmd.go

@@ -212,7 +212,6 @@ func (c *Cmd) Start() error {
 	// Start relaying process IO.
 	stdin, stdout, stderr := p.Stdio()
 	if c.Stdin != nil {
-		c.Log.Info("coping stdin")
 		// Do not make stdin part of the error group because there is no way for
 		// us or the caller to reliably unblock the c.Stdin read when the
 		// process exits.

internal/log/context.go

@@ -19,13 +19,13 @@ var (
 	// Instead, use `L.With*` or `L.Dup()`. Or `G(context.Background())`.
 	L = logrus.NewEntry(logrus.StandardLogger())
 
-	// G is an alias for GetEntry
+	// G is an alias for GetEntry.
 	G = GetEntry
 
-	// S is an alias for SetEntry
+	// S is an alias for SetEntry.
 	S = SetEntry
 
-	// U is an alias for UpdateContext
+	// U is an alias for UpdateContext.
 	U = UpdateContext
 )
 
@@ -82,7 +82,7 @@ func UpdateContext(ctx context.Context) context.Context {
 // WithContext returns a context that contains the provided log entry.
 // The entry can be extracted with `GetEntry` (`G`)
 //
-// The entry in the context is a copy of `entry` (generated by `entry.WithContext`)
+// The entry in the context is a copy of `entry` (generated by `entry.WithContext`).
 func WithContext(ctx context.Context, entry *logrus.Entry) (context.Context, *logrus.Entry) {
 	// regardless of the order, entry.Context != GetEntry(ctx)
 	// here, the returned entry will reference the supplied context

internal/sync/once.go

@@ -0,0 +1,41 @@
+package sync
+
+import (
+	"context"
+	"sync"
+)
+
+// TODO (go1.21): use pkg.go.dev/sync#OnceValues
+
+// OnceValue is a wrapper around [sync.Once] that runs f only once and
+// returns both a value (of type T) and an error.
+func OnceValue[T any](f func() (T, error)) func() (T, error) {
+	var (
+		once sync.Once
+		v    T
+		err  error
+	)
+
+	return func() (T, error) {
+		once.Do(func() {
+			v, err = f()
+		})
+		return v, err
+	}
+}
+
+// NewOnce is similar to [OnceValue], but allows passing a context to f.
+func OnceValueCtx[T any](f func(ctx context.Context) (T, error)) func(context.Context) (T, error) {
+	var (
+		once sync.Once
+		v    T
+		err  error
+	)
+
+	return func(ctx context.Context) (T, error) {
+		once.Do(func() {
+			v, err = f(ctx)
+		})
+		return v, err
+	}
+}

pkg/cimfs/cim_test.go

@@ -146,5 +146,4 @@ func TestCimReadWrite(t *testing.T) {
 			}
 		}
 	}
-
 }

test/functional/lcow_bench_test.go

@@ -11,26 +11,26 @@ import (
 
 	"github.com/Microsoft/hcsshim/test/internal/util"
 	"github.com/Microsoft/hcsshim/test/pkg/require"
-	"github.com/Microsoft/hcsshim/test/pkg/uvm"
+	testuvm "github.com/Microsoft/hcsshim/test/pkg/uvm"
 )
 
 func BenchmarkLCOW_UVM(b *testing.B) {
-	requireFeatures(b, featureLCOW)
+	requireFeatures(b, featureLCOW, featureUVM)
 	require.Build(b, osversion.RS5)
 
-	pCtx := context.Background()
+	pCtx := util.Context(context.Background(), b)
 
 	b.Run("Create", func(b *testing.B) {
 		b.StopTimer()
 		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
 			ctx, cancel := context.WithTimeout(pCtx, benchmarkIterationTimeout)
 
-			opts := defaultLCOWOptions(b)
+			opts := defaultLCOWOptions(ctx, b)
 			opts.ID += util.RandNameSuffix(i)
 
 			b.StartTimer()
-			_, cleanup := uvm.CreateLCOW(ctx, b, opts)
+			_, cleanup := testuvm.CreateLCOW(ctx, b, opts)
 			b.StopTimer()
 
 			cleanup(ctx)
@@ -44,9 +44,9 @@ func BenchmarkLCOW_UVM(b *testing.B) {
 		for i := 0; i < b.N; i++ {
 			ctx, cancel := context.WithTimeout(pCtx, benchmarkIterationTimeout)
 
-			opts := defaultLCOWOptions(b)
+			opts := defaultLCOWOptions(ctx, b)
 			opts.ID += util.RandNameSuffix(i)
-			vm, cleanup := uvm.CreateLCOW(ctx, b, opts)
+			vm, cleanup := testuvm.CreateLCOW(ctx, b, opts)
 
 			b.StartTimer()
 			if err := vm.Start(ctx); err != nil {
@@ -65,13 +65,13 @@ func BenchmarkLCOW_UVM(b *testing.B) {
 		for i := 0; i < b.N; i++ {
 			ctx, cancel := context.WithTimeout(pCtx, benchmarkIterationTimeout)
 
-			opts := defaultLCOWOptions(b)
+			opts := defaultLCOWOptions(ctx, b)
 			opts.ID += util.RandNameSuffix(i)
-			vm, cleanup := uvm.CreateLCOW(ctx, b, opts)
-			uvm.Start(ctx, b, vm)
+			vm, cleanup := testuvm.CreateLCOW(ctx, b, opts)
+			testuvm.Start(ctx, b, vm)
 
 			b.StartTimer()
-			uvm.Kill(ctx, b, vm)
+			testuvm.Kill(ctx, b, vm)
 			if err := vm.WaitCtx(ctx); err != nil {
 				b.Fatalf("could not kill uvm %q: %v", vm.ID(), err)
 			}
@@ -88,13 +88,13 @@ func BenchmarkLCOW_UVM(b *testing.B) {
 		for i := 0; i < b.N; i++ {
 			ctx, cancel := context.WithTimeout(pCtx, benchmarkIterationTimeout)
 
-			opts := defaultLCOWOptions(b)
+			opts := defaultLCOWOptions(ctx, b)
 			opts.ID += util.RandNameSuffix(i)
-			vm, cleanup := uvm.CreateLCOW(ctx, b, opts)
-			uvm.Start(ctx, b, vm)
+			vm, cleanup := testuvm.CreateLCOW(ctx, b, opts)
+			testuvm.Start(ctx, b, vm)
 
 			b.StartTimer()
-			if err := vm.Close(); err != nil {
+			if err := vm.CloseCtx(ctx); err != nil {
 				b.Fatalf("could not kill uvm %q: %v", vm.ID(), err)
 			}
 			b.StopTimer()