Update containerd-shim-runhcs-v1 tests (microsoft#1783)

Update shim tests to match current shim behavior.
Run shim tests in GitHub CI.

Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
(cherry picked from commit 55f8c42)
Signed-off-by: Kirtana Ashok <kiashok@microsoft.com>

.github/workflows/ci.yml

@@ -200,7 +200,7 @@ jobs:
 
       - name: Run guest code unit tests
         run: gotestsum --format standard-verbose --debug -- -mod=mod -gcflags=all=-d=checkptr ./internal/guest/...
-      
+
       - name: Build gcs Testing Binary
         run: go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional ./gcs
         working-directory: test
@@ -229,10 +229,17 @@ jobs:
       - name: Test rego policy interpreter
         run: gotestsum --format standard-verbose --debug -- -mod=mod -gcflags=all=-d=checkptr ./internal/regopolicyinterpreter
 
+      - name: 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
+          }
+          gotestsum --format standard-verbose --debug -- -mod=mod -tags functional -gcflags=all=-d=checkptr ./...
+        working-directory: test/containerd-shim-runhcs-v1
+
       # build testing binaries
-      - name: Build containerd-shim-runhcs-v1 Testing Binary
-        run: go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional ./containerd-shim-runhcs-v1
-        working-directory: test
       - name: Build cri-containerd Testing Binary
         run: go test -mod=mod -gcflags=all=-d=checkptr -c -tags functional ./cri-containerd
         working-directory: test

test/containerd-shim-runhcs-v1/delete_test.go

@@ -4,22 +4,22 @@
 package main
 
 import (
-	"os"
 	"testing"
 	"time"
 
 	"github.com/containerd/containerd/runtime/v2/task"
 	"github.com/gogo/protobuf/proto"
 )
 
-func verifyDeleteCommandSuccess(t *testing.T, stdout, stderr string, runerr error, begin, end time.Time) {
+func verifyDeleteCommandSuccess(t *testing.T, stdout, stderr string, runErr error, begin, end time.Time) {
 	t.Helper()
-	if runerr != nil {
-		t.Fatalf("expected `delete` command success got err: %v", runerr)
+	if runErr != nil {
+		t.Fatalf("expected `delete` command success got err: %v", runErr)
 	}
 	if stdout == "" {
-		t.Fatalf("expected `delete` command stdout to be non-empty, stderr: %v", stderr)
+		t.Fatalf("expected `delete` command stdout to be non-empty, stdout: %v", stdout)
 	}
+	// don't check stderr, since logs will be printed to it
 	var resp task.DeleteResponse
 	if err := proto.Unmarshal([]byte(stdout), &resp); err != nil {
 		t.Fatalf("failed to unmarshal stdout to DeleteResponse with err: '%v", err)
@@ -30,9 +30,6 @@ func verifyDeleteCommandSuccess(t *testing.T, stdout, stderr string, runerr erro
 	if begin.After(resp.ExitedAt) || end.Before(resp.ExitedAt) {
 		t.Fatalf("DeleteResponse.ExitedAt should be between, %v and %v, got: %v", begin, end, resp.ExitedAt)
 	}
-	if stderr != "" {
-		t.Fatalf("expected `delete` command stderr to be empty got: %s", stderr)
-	}
 }
 
 func Test_Delete_No_Bundle_Arg(t *testing.T) {
@@ -72,6 +69,9 @@ func Test_Delete_No_Bundle_Path(t *testing.T) {
 }
 
 func Test_Delete_HcsSystem_NotFound(t *testing.T) {
+	// `delete` no longer removes bundle, but still create a directory regardless
+	//
+	// https://github.com/microsoft/hcsshim/commit/450cdb150a74aa594d7fe63bb0b3a2a37f5dd782
 	dir := t.TempDir()
 
 	before := time.Now()
@@ -90,7 +90,4 @@ func Test_Delete_HcsSystem_NotFound(t *testing.T) {
 		t,
 		stdout, stderr, err,
 		before, after)
-	if _, err := os.Stat(dir); err == nil || !os.IsNotExist(err) {
-		t.Fatalf("expected the bundle dir to be cleaned up. Got err: %v", err)
-	}
 }

test/containerd-shim-runhcs-v1/global_command_test.go

@@ -31,19 +31,19 @@ func runGlobalCommand(t *testing.T, args []string) (string, string, error) {
 	return outb.String(), errb.String(), err
 }
 
-func verifyGlobalCommandSuccess(t *testing.T, expectedStdout, stdout, expectedStderr, stderr string, runerr error) {
+func verifyGlobalCommandSuccess(t *testing.T, expectedStdout, stdout, expectedStderr, stderr string, runErr error) {
 	t.Helper()
-	if runerr != nil {
-		t.Fatalf("expected no error got stdout: '%s', stderr: '%s', err: '%v'", stdout, stderr, runerr)
+	if runErr != nil {
+		t.Fatalf("expected no error got stdout: '%s', stderr: '%s', err: '%v'", stdout, stderr, runErr)
 	}
 
 	verifyGlobalCommandOut(t, expectedStdout, stdout, expectedStderr, stderr)
 }
 
-func verifyGlobalCommandFailure(t *testing.T, expectedStdout, stdout, expectedStderr, stderr string, runerr error) {
+func verifyGlobalCommandFailure(t *testing.T, expectedStdout, stdout, expectedStderr, stderr string, runErr error) {
 	t.Helper()
-	if runerr == nil || runerr.Error() != "exit status 1" {
-		t.Fatalf("expected error: 'exit status 1', got: '%v'", runerr)
+	if runErr == nil || runErr.Error() != "exit status 1" {
+		t.Fatalf("expected error: 'exit status 1', got: '%v'", runErr)
 	}
 
 	verifyGlobalCommandOut(t, expectedStdout, stdout, expectedStderr, stderr)
@@ -54,14 +54,14 @@ func verifyGlobalCommandOut(t *testing.T, expectedStdout, stdout, expectedStderr
 	// stdout verify
 	if expectedStdout == "" && expectedStdout != stdout {
 		t.Fatalf("expected stdout empty got: %s", stdout)
-	} else if !strings.HasPrefix(stdout, expectedStdout) {
+	} else if !strings.Contains(stdout, expectedStdout) {
 		t.Fatalf("expected stdout to begin with: %s, got: %s", expectedStdout, stdout)
 	}
 
 	// stderr verify
 	if expectedStderr == "" && expectedStderr != stderr {
 		t.Fatalf("expected stderr empty got: %s", stderr)
-	} else if !strings.HasPrefix(stderr, expectedStderr) {
+	} else if !strings.Contains(stderr, expectedStderr) {
 		t.Fatalf("expected stderr to begin with: %s, got: %s", expectedStderr, stderr)
 	}
 }
@@ -70,11 +70,7 @@ func Test_Global_Command_No_Namespace(t *testing.T) {
 	stdout, stderr, err := runGlobalCommand(
 		t,
 		[]string{})
-	verifyGlobalCommandFailure(
-		t,
-		"namespace is required\n", stdout,
-		"namespace is required\n", stderr,
-		err)
+	verifyGlobalCommandFailure(t, "", stdout, "namespace is required\n", stderr, err)
 }
 
 func Test_Global_Command_No_Address(t *testing.T) {
@@ -83,11 +79,7 @@ func Test_Global_Command_No_Address(t *testing.T) {
 		[]string{
 			"--namespace", t.Name(),
 		})
-	verifyGlobalCommandFailure(
-		t,
-		"address is required\n", stdout,
-		"address is required\n", stderr,
-		err)
+	verifyGlobalCommandFailure(t, "", stdout, "address is required\n", stderr, err)
 }
 
 func Test_Global_Command_No_PublishBinary(t *testing.T) {
@@ -97,11 +89,7 @@ func Test_Global_Command_No_PublishBinary(t *testing.T) {
 			"--namespace", t.Name(),
 			"--address", t.Name(),
 		})
-	verifyGlobalCommandFailure(
-		t,
-		"publish-binary is required\n", stdout,
-		"publish-binary is required\n", stderr,
-		err)
+	verifyGlobalCommandFailure(t, "", stdout, "publish-binary is required\n", stderr, err)
 }
 
 func Test_Global_Command_No_ID(t *testing.T) {
@@ -112,11 +100,7 @@ func Test_Global_Command_No_ID(t *testing.T) {
 			"--address", t.Name(),
 			"--publish-binary", t.Name(),
 		})
-	verifyGlobalCommandFailure(
-		t,
-		"id is required\n", stdout,
-		"id is required\n", stderr,
-		err)
+	verifyGlobalCommandFailure(t, "", stdout, "id is required\n", stderr, err)
 }
 
 func Test_Global_Command_No_Command(t *testing.T) {

test/containerd-shim-runhcs-v1/start_test.go

@@ -13,6 +13,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/Microsoft/go-winio"
 	"github.com/Microsoft/hcsshim/pkg/annotations"
@@ -52,7 +53,17 @@ func createStartCommandWithID(t *testing.T, id string) (*exec.Cmd, *bytes.Buffer
 
 func cleanupTestBundle(t *testing.T, dir string) {
 	t.Helper()
-	err := os.RemoveAll(dir)
+	var err error
+	for i := 0; i < 2; i++ {
+		// sporadic access-denies errors if trying to delete bundle (namely panic.log) before OS realizes
+		// shim exited and releases dile handle
+		if err = os.RemoveAll(dir); err == nil {
+			// does not os.RemoveAll does not if path doesn't exist
+			return
+		}
+		time.Sleep(time.Millisecond)
+	}
+
 	if err != nil {
 		t.Errorf("failed removing test bundle with: %v", err)
 	}
@@ -101,7 +112,7 @@ func verifyStartCommandSuccess(t *testing.T, expectedNamespace, expectedID strin
 
 	cl.Close()
 	c.Close()
-	if err != nil && !strings.HasPrefix(err.Error(), "ttrpc: client shutting down: ttrpc: closed") {
+	if err != nil && !strings.HasPrefix(err.Error(), "ttrpc: closed") {
 		t.Fatalf("failed to shutdown shim with: %v", err)
 	}
 }