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

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

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

.github/workflows/ci.yml

@@ -202,7 +202,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
@@ -231,10 +231,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)
  }
 }