Improve containerd client unit tests

containerm/ctr/ctrd_checkpoint.go

@@ -83,7 +83,6 @@ func withCheckpointOpt(checkpoint *containerdtypes.Descriptor) containerd.NewTas
 // getCheckpointDir verifies checkpoint directory for create,remove, list options and checks if checkpoint already exists
 func getCheckpointDir(checkDir, checkpointID, ctrName, ctrID, ctrCheckpointDir string, create bool) (string, error) {
 	var checkpointDir string
-	var err2 error
 	if checkDir != "" {
 		checkpointDir = checkDir
 	} else {
@@ -94,27 +93,27 @@ func getCheckpointDir(checkDir, checkpointID, ctrName, ctrID, ctrCheckpointDir s
 	if create {
 		switch {
 		case err == nil && stat.IsDir():
-			err2 = fmt.Errorf("checkpoint with name %s already exists for container %s", checkpointID, ctrName)
+			return checkpointAbsDir, fmt.Errorf("checkpoint with name %s already exists for container %s", checkpointID, ctrName)
 		case err != nil && os.IsNotExist(err):
-			err2 = os.MkdirAll(checkpointAbsDir, 0700)
+			return checkpointAbsDir, os.MkdirAll(checkpointAbsDir, 0700)
 		case err != nil:
-			err2 = err
+			return checkpointAbsDir, err
 		case err == nil:
-			err2 = fmt.Errorf("%s exists and is not a directory", checkpointAbsDir)
+			return checkpointAbsDir, fmt.Errorf("%s exists and is not a directory", checkpointAbsDir)
 		default:
 			// should never get here
 		}
 	} else {
 		switch {
 		case err != nil:
-			err2 = fmt.Errorf("checkpoint %s does not exist for container %s", checkpointID, ctrName)
+			return checkpointAbsDir, fmt.Errorf("checkpoint %s does not exist for container %s", checkpointID, ctrName)
 		case err == nil && stat.IsDir():
-			err2 = nil
+			return checkpointAbsDir, nil
 		case err == nil:
-			err2 = fmt.Errorf("%s exists and is not a directory", checkpointAbsDir)
+			return checkpointAbsDir, fmt.Errorf("%s exists and is not a directory", checkpointAbsDir)
 		default:
 			// should never get here
 		}
 	}
-	return checkpointAbsDir, err2
+	return checkpointAbsDir, nil
 }

containerm/ctr/ctrd_cio_mgr_internal.go

@@ -19,7 +19,6 @@ package ctr
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"sync"
@@ -36,7 +35,7 @@ func (mgr *cioMgr) newFIFOSet(processID string, withStdin bool, withTerminal boo
 		return nil, err
 	}
 
-	fifoDir, err := ioutil.TempDir(mgr.fifoRootDir, "")
+	fifoDir, err := os.MkdirTemp(mgr.fifoRootDir, "")
 	if err != nil {
 		return nil, err
 	}

containerm/ctr/ctrd_client_internal_test.go

@@ -113,6 +113,56 @@ func TestClientInternalGenerateNewContainerOpts(t *testing.T) {
 	}
 }
 
+func TestConfigureRuncRuntime(t *testing.T) {
+	ctrdClient := &containerdClient{
+		runcRuntime: types.RuntimeTypeV2runcV2,
+	}
+	tests := map[string]struct {
+		container *types.Container
+		runtime   types.Runtime
+	}{
+		"test_RuntimeTypeV1": {
+			container: &types.Container{
+				HostConfig: &types.HostConfig{
+					Runtime: types.RuntimeTypeV1,
+				},
+			},
+			runtime: ctrdClient.runcRuntime,
+		},
+		"RuntimeTypeV2runcV1": {
+			container: &types.Container{
+				HostConfig: &types.HostConfig{
+					Runtime: types.RuntimeTypeV2runcV1,
+				},
+			},
+			runtime: ctrdClient.runcRuntime,
+		},
+		"RuntimeTypeV2runcV2": {
+			container: &types.Container{
+				HostConfig: &types.HostConfig{
+					Runtime: types.RuntimeTypeV2runcV2,
+				},
+			},
+			runtime: ctrdClient.runcRuntime,
+		},
+		"test_default": {
+			container: &types.Container{
+				HostConfig: &types.HostConfig{
+					Runtime: types.RuntimeTypeV2kataV2,
+				},
+			},
+			runtime: types.RuntimeTypeV2kataV2,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			ctrdClient.configureRuncRuntime(test.container)
+			testutil.AssertEqual(t, test.runtime, test.container.HostConfig.Runtime)
+		})
+	}
+}
+
 func TestClientInternalGenerateRemoteOpts(t *testing.T) {
 	const containerImageRef = "some.repo/image:tag"
 	testImageInfo := types.Image{

containerm/ctr/ctrd_client_test.go

@@ -14,6 +14,7 @@ package ctr
 
 import (
 	"context"
+	"io"
 	"reflect"
 	"testing"
 	"time"
@@ -24,7 +25,9 @@ import (
 	"github.com/eclipse-kanto/container-management/containerm/pkg/testutil/matchers"
 	containerdMocks "github.com/eclipse-kanto/container-management/containerm/pkg/testutil/mocks/containerd"
 	ctrdMocks "github.com/eclipse-kanto/container-management/containerm/pkg/testutil/mocks/ctrd"
+	ioMocks "github.com/eclipse-kanto/container-management/containerm/pkg/testutil/mocks/io"
 	loggerMocks "github.com/eclipse-kanto/container-management/containerm/pkg/testutil/mocks/logger"
+	streamsMocks "github.com/eclipse-kanto/container-management/containerm/pkg/testutil/mocks/streams"
 	"github.com/eclipse-kanto/container-management/containerm/streams"
 	"github.com/eclipse-kanto/container-management/containerm/util"
 
@@ -140,9 +143,7 @@ func TestCtrdClientCreateContainer(t *testing.T) {
 
 	for testName, testCase := range tests {
 		t.Run(testName, func(t *testing.T) {
-			expectedError := testCase.mockExec()
-			actualError := testClient.CreateContainer(ctx, testCtr, "")
-			testutil.AssertError(t, expectedError, actualError)
+			testutil.AssertError(t, testCase.mockExec(), testClient.CreateContainer(ctx, testCtr, ""))
 		})
 	}
 }
@@ -555,12 +556,16 @@ func TestAttachContainer(t *testing.T) {
 	defer mockCtrl.Finish()
 
 	mockIoMgr := NewMockcontainerIOManager(mockCtrl)
+	mockReadCloser := ioMocks.NewMockReadCloser(mockCtrl)
+	mockStream := streamsMocks.NewMockStream(mockCtrl)
+	mockIO := NewMockIO(mockCtrl)
+	attachConfig := &streams.AttachConfig{UseStdin: true, Stdin: mockReadCloser}
+	ctx := context.Background()
 
 	testClient := &containerdClient{
 		ioMgr: mockIoMgr,
 	}
 
-	ctx := context.Background()
 	testCtr := &types.Container{
 		ID: "test-id",
 		IOConfig: &types.IOConfig{
@@ -569,13 +574,60 @@ func TestAttachContainer(t *testing.T) {
 		},
 	}
 
-	expectedError := log.NewErrorf("failed to initialise IO for container ID = test-id")
+	tests := map[string]struct {
+		attachConfig *streams.AttachConfig
+		mockExec     func() error
+	}{
+		"test_containerIO_nil": {
+			attachConfig: attachConfig,
+			mockExec: func() error {
+				mockIoMgr.EXPECT().GetIO(testCtr.ID).Return(nil)
+				mockIoMgr.EXPECT().InitIO(testCtr.ID, testCtr.IOConfig.OpenStdin).Return(nil, log.NewError("failed to initialise IO for container ID = test-id"))
+				return log.NewError("failed to initialise IO for container ID = test-id")
+			},
+		},
+		"test_container_stdin_true": {
+			attachConfig: attachConfig,
+			mockExec: func() error {
+				errChan := make(chan error)
+				go func() {
+					errChan <- nil
+					close(errChan)
+				}()
+
+				mockIoMgr.EXPECT().GetIO(testCtr.ID).Return(mockIO)
+				mockReadCloser.EXPECT().Read(gomock.Any()).DoAndReturn(func(p []byte) (int, error) {
+					return -1, io.EOF
+				})
+				mockIO.EXPECT().Stream().Return(mockStream)
+				mockStream.EXPECT().Attach(ctx, gomock.AssignableToTypeOf(attachConfig)).Return(errChan)
+
+				return nil
+			},
+		},
+		"test_container_stdin_false": {
+			attachConfig: &streams.AttachConfig{Stdin: mockReadCloser},
+			mockExec: func() error {
+				errChan := make(chan error)
+				go func() {
+					errChan <- nil
+					close(errChan)
+				}()
 
-	mockIoMgr.EXPECT().GetIO(testCtr.ID).Return(nil)
-	mockIoMgr.EXPECT().InitIO(testCtr.ID, testCtr.IOConfig.OpenStdin).Return(nil, expectedError)
+				mockIoMgr.EXPECT().GetIO(testCtr.ID).Return(mockIO)
+				mockIO.EXPECT().Stream().Return(mockStream)
+				mockStream.EXPECT().Attach(ctx, gomock.AssignableToTypeOf(attachConfig)).Return(errChan)
 
-	actualError := testClient.AttachContainer(ctx, testCtr, &streams.AttachConfig{})
-	testutil.AssertError(t, expectedError, actualError)
+				return nil
+			},
+		},
+	}
+
+	for testName, testCase := range tests {
+		t.Run(testName, func(t *testing.T) {
+			testutil.AssertError(t, testCase.mockExec(), testClient.AttachContainer(ctx, testCtr, testCase.attachConfig))
+		})
+	}
 }
 
 func TestPauseContainer(t *testing.T) {
@@ -633,9 +685,7 @@ func TestPauseContainer(t *testing.T) {
 
 	for testName, testCase := range tests {
 		t.Run(testName, func(t *testing.T) {
-			expectedError := testCase.mockExec(ctx, mockTask)
-			actualError := testClient.PauseContainer(ctx, testCase.arg)
-			testutil.AssertError(t, expectedError, actualError)
+			testutil.AssertError(t, testCase.mockExec(ctx, mockTask), testClient.PauseContainer(ctx, testCase.arg))
 		})
 	}
 }
@@ -695,9 +745,7 @@ func TestUnpauseContainer(t *testing.T) {
 
 	for testName, testCase := range tests {
 		t.Run(testName, func(t *testing.T) {
-			expectedError := testCase.mockExec(ctx, mockTask)
-			actualError := testClient.UnpauseContainer(ctx, testCase.arg)
-			testutil.AssertError(t, expectedError, actualError)
+			testutil.AssertError(t, testCase.mockExec(ctx, mockTask), testClient.UnpauseContainer(ctx, testCase.arg))
 		})
 	}
 }
@@ -886,9 +934,7 @@ func TestRestoreContainer(t *testing.T) {
 
 	for testName, testCase := range tests {
 		t.Run(testName, func(t *testing.T) {
-			expectedError := testCase.mockExec()
-			actualError := testClient.RestoreContainer(ctx, testCtr)
-			testutil.AssertError(t, expectedError, actualError)
+			testutil.AssertError(t, testCase.mockExec(), testClient.RestoreContainer(ctx, testCtr))
 		})
 	}
 }
@@ -923,11 +969,10 @@ func TestSetContainerExitHooks(t *testing.T) {
 
 	testClient.SetContainerExitHooks(arg)
 
-	got := testClient.ctrdCache.containerExitHooks
-	testutil.AssertEqual(t, 1, len(got))
+	testutil.AssertEqual(t, 1, len(testClient.ctrdCache.containerExitHooks))
 
-	if reflect.ValueOf(got[0]).Pointer() != reflect.ValueOf(arg).Pointer() {
-		t.Errorf("SetContainerExitHooks() = %v, want %v", reflect.ValueOf(got[0]), reflect.ValueOf(arg))
+	if reflect.ValueOf(testClient.ctrdCache.containerExitHooks[0]).Pointer() != reflect.ValueOf(arg).Pointer() {
+		t.Errorf("SetContainerExitHooks() = %v, want %v", reflect.ValueOf(testClient.ctrdCache.containerExitHooks[0]), reflect.ValueOf(arg))
 	}
 }
 
@@ -1118,9 +1163,7 @@ func TestCtrdClientUpdateContainer(t *testing.T) {
 				task:      mockTask,
 			}
 
-			expectedErr := testCase.mockExec()
-			actualErr := testClient.UpdateContainer(ctx, testCase.ctr, testCase.resources)
-			testutil.AssertError(t, expectedErr, actualErr)
+			testutil.AssertError(t, testCase.mockExec(), testClient.UpdateContainer(ctx, testCase.ctr, testCase.resources))
 		})
 	}
 }

containerm/ctr/ctrd_container_create_opts_test.go

@@ -0,0 +1,97 @@
+// Copyright (c) 2023 Contributors to the Eclipse Foundation
+//
+// See the NOTICE file(s) distributed with this work for additional
+// information regarding copyright ownership.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+
+package ctr
+
+import (
+	"testing"
+
+	"github.com/eclipse-kanto/container-management/containerm/containers/types"
+	"github.com/eclipse-kanto/container-management/containerm/pkg/testutil"
+
+	"github.com/containerd/containerd"
+	"github.com/containerd/containerd/images"
+)
+
+func TestWithRuntimeOpts(t *testing.T) {
+	tests := map[string]struct {
+		container *types.Container
+	}{
+		"test_runtime_type_v1": {
+			&types.Container{
+				ID:   testCtrID1,
+				Name: testContainerName,
+				HostConfig: &types.HostConfig{
+					Runtime: types.RuntimeTypeV1,
+				},
+			},
+		},
+		"test_runtime_type_v2": {
+			&types.Container{
+				ID:   testCtrID1,
+				Name: testContainerName,
+				HostConfig: &types.HostConfig{
+					Runtime: types.RuntimeTypeV2runcV1,
+				},
+			},
+		},
+		"testing_default": {
+			&types.Container{
+				ID:   testCtrID1,
+				Name: testContainerName,
+				HostConfig: &types.HostConfig{
+					Runtime: "",
+				},
+			},
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			testutil.AssertNotNil(t, WithRuntimeOpts(test.container, ""))
+		})
+	}
+}
+
+func TestWithSpecOpts(t *testing.T) {
+	tests := map[string]struct {
+		container *types.Container
+	}{
+		"test_config": {
+			&types.Container{
+				Config: &types.ContainerConfiguration{
+					Cmd: []string{"test"},
+					Env: []string{"test"},
+				},
+				HostConfig: &types.HostConfig{},
+			},
+		},
+		"test_privileged": {
+			&types.Container{
+				HostConfig: &types.HostConfig{
+					Privileged: true,
+				},
+			},
+		},
+		"test_extra_capabilities": {
+			&types.Container{
+				HostConfig: &types.HostConfig{
+					ExtraCapabilities: []string{"CAP_NET_ADMIN"},
+				},
+			},
+		},
+	}
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			testutil.AssertNotNil(t, WithSpecOpts(test.container, containerd.NewImage(&containerd.Client{}, images.Image{}), "/tmp/test"))
+		})
+	}
+}