From 55f8c428a2d9b6f0aeced9d5dc497095160817d6 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Tue, 16 May 2023 18:11:54 -0400 Subject: [PATCH] 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 --- .github/workflows/ci.yml | 15 +++++-- test/containerd-shim-runhcs-v1/delete_test.go | 19 ++++----- .../global_command_test.go | 40 ++++++------------- test/containerd-shim-runhcs-v1/start_test.go | 15 ++++++- 4 files changed, 44 insertions(+), 45 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8e5c08c5f9..b422bc1f72 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/test/containerd-shim-runhcs-v1/delete_test.go b/test/containerd-shim-runhcs-v1/delete_test.go index 3c1bdbd9ba..5453702a60 100644 --- a/test/containerd-shim-runhcs-v1/delete_test.go +++ b/test/containerd-shim-runhcs-v1/delete_test.go @@ -4,7 +4,6 @@ package main import ( - "os" "testing" "time" @@ -12,14 +11,15 @@ import ( "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) - } } diff --git a/test/containerd-shim-runhcs-v1/global_command_test.go b/test/containerd-shim-runhcs-v1/global_command_test.go index 8778cf0a03..0de82e503a 100644 --- a/test/containerd-shim-runhcs-v1/global_command_test.go +++ b/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) { diff --git a/test/containerd-shim-runhcs-v1/start_test.go b/test/containerd-shim-runhcs-v1/start_test.go index 9b46bf27ef..26b6734a5c 100644 --- a/test/containerd-shim-runhcs-v1/start_test.go +++ b/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) } }