diff --git a/pkg/server/container_start.go b/pkg/server/container_start.go index 1fb10c584..8401c1d3d 100644 --- a/pkg/server/container_start.go +++ b/pkg/server/container_start.go @@ -103,7 +103,7 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me sandboxConfig := sandboxMeta.Config sandboxID := meta.SandboxID // Make sure sandbox is running. - sandboxInfo, err := c.containerService.Info(ctx, &execution.InfoRequest{ContainerID: sandboxID}) + sandboxInfo, err := c.taskService.Info(ctx, &execution.InfoRequest{ContainerID: sandboxID}) if err != nil { return fmt.Errorf("failed to get sandbox container %q info: %v", sandboxID, err) } @@ -115,7 +115,7 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me sandboxPid := sandboxInfo.Task.Pid glog.V(2).Infof("Sandbox container %q is running with pid %d", sandboxID, sandboxPid) - // Generate containerd container create options. + // Generate containerd task create options. imageMeta, err := c.imageMetadataStore.Get(meta.ImageRef) if err != nil { return fmt.Errorf("failed to get container image %q: %v", meta.ImageRef, err) @@ -189,7 +189,7 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me }) } - // Create containerd container. + // Create containerd task. createOpts := &execution.CreateRequest{ ContainerID: id, Rootfs: rootfs, @@ -198,24 +198,24 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me Stderr: stderr, Terminal: config.GetTty(), } - glog.V(5).Infof("Create containerd container (id=%q, name=%q) with options %+v.", + glog.V(5).Infof("Create containerd task (id=%q, name=%q) with options %+v.", id, meta.Name, createOpts) - createResp, err := c.containerService.Create(ctx, createOpts) + createResp, err := c.taskService.Create(ctx, createOpts) if err != nil { - return fmt.Errorf("failed to create containerd container: %v", err) + return fmt.Errorf("failed to create containerd task: %v", err) } defer func() { if retErr != nil { - // Cleanup the containerd container if an error is returned. - if _, err := c.containerService.Delete(ctx, &execution.DeleteRequest{ContainerID: id}); err != nil { - glog.Errorf("Failed to delete containerd container %q: %v", id, err) + // Cleanup the containerd task if an error is returned. + if _, err := c.taskService.Delete(ctx, &execution.DeleteRequest{ContainerID: id}); err != nil { + glog.Errorf("Failed to delete containerd task %q: %v", id, err) } } }() - // Start containerd container. - if _, err := c.containerService.Start(ctx, &execution.StartRequest{ContainerID: id}); err != nil { - return fmt.Errorf("failed to start containerd container %q: %v", id, err) + // Start containerd task. + if _, err := c.taskService.Start(ctx, &execution.StartRequest{ContainerID: id}); err != nil { + return fmt.Errorf("failed to start containerd task %q: %v", id, err) } // Update container start timestamp. diff --git a/pkg/server/container_start_test.go b/pkg/server/container_start_test.go index c6c42b108..67792936c 100644 --- a/pkg/server/container_start_test.go +++ b/pkg/server/container_start_test.go @@ -573,7 +573,7 @@ func TestStartContainer(t *testing.T) { sandboxContainerdContainer: testSandboxContainer, startContainerErr: errors.New("start error"), expectStateChange: true, - // cleanup the containerd container. + // cleanup the containerd task. expectCalls: []string{"info", "create", "start", "delete"}, expectErr: true, }, @@ -588,7 +588,7 @@ func TestStartContainer(t *testing.T) { } { t.Logf("TestCase %q", desc) c := newTestCRIContainerdService() - fake := c.containerService.(*servertesting.FakeExecutionClient) + fake := c.taskService.(*servertesting.FakeExecutionClient) fakeOS := c.os.(*ostesting.FakeOS) fakeSnapshotClient := WithFakeSnapshotClient(c) if test.containerMetadata != nil { @@ -598,7 +598,7 @@ func TestStartContainer(t *testing.T) { assert.NoError(t, c.sandboxStore.Create(*test.sandboxMetadata)) } if test.sandboxContainerdContainer != nil { - fake.SetFakeContainers([]task.Task{*test.sandboxContainerdContainer}) + fake.SetFakeTasks([]task.Task{*test.sandboxContainerdContainer}) } if !test.imageMetadataErr { assert.NoError(t, c.imageMetadataStore.Create(metadata.ImageMetadata{ @@ -650,8 +650,8 @@ func TestStartContainer(t *testing.T) { assert.Equal(t, errorStartReason, meta.Reason) assert.NotEmpty(t, meta.Message) _, err := fake.Info(context.Background(), &execution.InfoRequest{ContainerID: testID}) - assert.True(t, isContainerdContainerNotExistError(err), - "containerd container should be cleaned up after when fail to start") + assert.True(t, isContainerdGRPCNotFoundError(err), + "containerd task should be cleaned up after when fail to start") continue } t.Logf("container state should be running when start successfully") diff --git a/pkg/server/container_stop.go b/pkg/server/container_stop.go index af2bb78b0..f2a27f06b 100644 --- a/pkg/server/container_stop.go +++ b/pkg/server/container_stop.go @@ -69,9 +69,9 @@ func (c *criContainerdService) StopContainer(ctx context.Context, r *runtime.Sto // TODO(random-liu): [P1] Get stop signal from image config. stopSignal := unix.SIGTERM glog.V(2).Infof("Stop container %q with signal %v", id, stopSignal) - _, err = c.containerService.Kill(ctx, &execution.KillRequest{ContainerID: id, Signal: uint32(stopSignal)}) + _, err = c.taskService.Kill(ctx, &execution.KillRequest{ContainerID: id, Signal: uint32(stopSignal)}) if err != nil { - if !isContainerdContainerNotExistError(err) && !isRuncProcessAlreadyFinishedError(err) { + if !isContainerdGRPCNotFoundError(err) && !isRuncProcessAlreadyFinishedError(err) { return nil, fmt.Errorf("failed to stop container %q: %v", id, err) } // Move on to make sure container status is updated. @@ -86,9 +86,9 @@ func (c *criContainerdService) StopContainer(ctx context.Context, r *runtime.Sto // Event handler will Delete the container from containerd after it handles the Exited event. glog.V(2).Infof("Kill container %q", id) - _, err = c.containerService.Kill(ctx, &execution.KillRequest{ContainerID: id, Signal: uint32(unix.SIGKILL)}) + _, err = c.taskService.Kill(ctx, &execution.KillRequest{ContainerID: id, Signal: uint32(unix.SIGKILL)}) if err != nil { - if !isContainerdContainerNotExistError(err) && !isRuncProcessAlreadyFinishedError(err) { + if !isContainerdGRPCNotFoundError(err) && !isRuncProcessAlreadyFinishedError(err) { return nil, fmt.Errorf("failed to kill container %q: %v", id, err) } // Move on to make sure container status is updated. diff --git a/pkg/server/container_stop_test.go b/pkg/server/container_stop_test.go index 554686eaa..294307dea 100644 --- a/pkg/server/container_stop_test.go +++ b/pkg/server/container_stop_test.go @@ -125,7 +125,7 @@ func TestStopContainer(t *testing.T) { expectErr: false, expectCalls: []servertesting.CalledDetail{}, }, - "should not return error if containerd container does not exist": { + "should not return error if containerd task does not exist": { metadata: &testMetadata, containerdContainer: &testContainer, // Since it's hard to inject event during `StopContainer` is running, @@ -133,7 +133,7 @@ func TestStopContainer(t *testing.T) { // status is not updated yet. // We also leverage this behavior to test that when graceful // stop doesn't take effect, container should be SIGKILL-ed. - stopErr: servertesting.ContainerNotExistError, + stopErr: servertesting.TaskNotExistError, expectErr: false, expectCalls: []servertesting.CalledDetail{ { @@ -150,7 +150,7 @@ func TestStopContainer(t *testing.T) { }, }, }, - "should not return error if containerd container process already finished": { + "should not return error if containerd task process already finished": { metadata: &testMetadata, containerdContainer: &testContainer, stopErr: errors.New("os: process already finished"), @@ -182,7 +182,7 @@ func TestStopContainer(t *testing.T) { }, }, }, - "should not return error if containerd container is gracefully stopped": { + "should not return error if containerd task is gracefully stopped": { metadata: &testMetadata, containerdContainer: &testContainer, expectErr: false, @@ -221,15 +221,15 @@ func TestStopContainer(t *testing.T) { c := newTestCRIContainerdService() fake := servertesting.NewFakeExecutionClient().WithEvents() defer fake.Stop() - c.containerService = fake + c.taskService = fake // Inject metadata. if test.metadata != nil { assert.NoError(t, c.containerStore.Create(*test.metadata)) } - // Inject containerd container. + // Inject containerd task. if test.containerdContainer != nil { - fake.SetFakeContainers([]task.Task{*test.containerdContainer}) + fake.SetFakeTasks([]task.Task{*test.containerdContainer}) } if test.stopErr != nil { fake.InjectError("kill", test.stopErr) diff --git a/pkg/server/events.go b/pkg/server/events.go index 30e2dcb5b..665fb2021 100644 --- a/pkg/server/events.go +++ b/pkg/server/events.go @@ -48,7 +48,7 @@ func (c *criContainerdService) startEventMonitor() { } go func() { for { - events, err := c.containerService.Events(context.Background(), &execution.EventsRequest{}) + events, err := c.taskService.Events(context.Background(), &execution.EventsRequest{}) if err != nil { glog.Errorf("Failed to connect to containerd event stream: %v", err) time.Sleep(b.Duration()) @@ -97,8 +97,8 @@ func (c *criContainerdService) handleEvent(e *task.Event) { return } // Delete the container from containerd. - _, err = c.containerService.Delete(context.Background(), &execution.DeleteRequest{ContainerID: e.ID}) - if err != nil && !isContainerdContainerNotExistError(err) { + _, err = c.taskService.Delete(context.Background(), &execution.DeleteRequest{ContainerID: e.ID}) + if err != nil && !isContainerdGRPCNotFoundError(err) { // TODO(random-liu): [P0] Enqueue the event and retry. glog.Errorf("Failed to delete container %q: %v", e.ID, err) return diff --git a/pkg/server/events_test.go b/pkg/server/events_test.go index f239c4f50..ae37fedf6 100644 --- a/pkg/server/events_test.go +++ b/pkg/server/events_test.go @@ -93,7 +93,7 @@ func TestHandleEvent(t *testing.T) { containerdContainer: &testContainerdContainer, expected: &testMetadata, }, - "should not update state when fail to delete containerd container": { + "should not update state when fail to delete containerd task": { event: &testExitEvent, metadata: &testMetadata, containerdContainer: &testContainerdContainer, @@ -112,12 +112,12 @@ func TestHandleEvent(t *testing.T) { containerdContainer: &testContainerdContainer, expected: &testMetadata, }, - "should update state when containerd container is already deleted": { + "should update state when containerd task is already deleted": { event: &testExitEvent, metadata: &testMetadata, expected: &testFinishedMetadata, }, - "should update state when delete containerd container successfully": { + "should update state when delete containerd task successfully": { event: &testExitEvent, metadata: &testMetadata, containerdContainer: &testContainerdContainer, @@ -126,7 +126,7 @@ func TestHandleEvent(t *testing.T) { } { t.Logf("TestCase %q", desc) c := newTestCRIContainerdService() - fake := c.containerService.(*servertesting.FakeExecutionClient) + fake := c.taskService.(*servertesting.FakeExecutionClient) e, err := fake.Events(context.Background(), &execution.EventsRequest{}) assert.NoError(t, err) fakeEvents := e.(*servertesting.EventClient) @@ -139,9 +139,9 @@ func TestHandleEvent(t *testing.T) { // Make sure that original data will not be changed. assert.NoError(t, c.containerStore.Create(*test.metadata)) } - // Inject containerd container. + // Inject containerd task. if test.containerdContainer != nil { - fake.SetFakeContainers([]task.Task{*test.containerdContainer}) + fake.SetFakeTasks([]task.Task{*test.containerdContainer}) } // Inject containerd delete error. if test.containerdErr != nil { diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index 9d6d2b613..1d8867274 100644 --- a/pkg/server/helpers.go +++ b/pkg/server/helpers.go @@ -25,6 +25,7 @@ import ( "strings" "syscall" + containerdmetadata "github.com/containerd/containerd/metadata" "github.com/docker/distribution/reference" "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/truncindex" @@ -33,13 +34,10 @@ import ( imagespec "github.com/opencontainers/image-spec/specs-go/v1" "golang.org/x/net/context" "google.golang.org/grpc" - - containerdmetadata "github.com/containerd/containerd/metadata" - "github.com/containerd/containerd/plugin" + "google.golang.org/grpc/codes" + runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1" "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" - - runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1" ) const ( @@ -219,11 +217,9 @@ func getPIDNamespace(pid uint32) string { return fmt.Sprintf(pidNSFormat, pid) } -// isContainerdContainerNotExistError checks whether a grpc error is containerd -// ErrContainerNotExist error. -// TODO(random-liu): Containerd should expose error better through api. -func isContainerdContainerNotExistError(grpcError error) bool { - return grpc.ErrorDesc(grpcError) == plugin.ErrContainerNotExist.Error() +// isContainerdGRPCNotFoundError checks whether a grpc error is not found error. +func isContainerdGRPCNotFoundError(grpcError error) bool { + return grpc.Code(grpcError) == codes.NotFound } // isRuncProcessAlreadyFinishedError checks whether a grpc error is a process already diff --git a/pkg/server/sandbox_list.go b/pkg/server/sandbox_list.go index 3032e09b3..6a3ddde90 100644 --- a/pkg/server/sandbox_list.go +++ b/pkg/server/sandbox_list.go @@ -45,7 +45,7 @@ func (c *criContainerdService) ListPodSandbox(ctx context.Context, r *runtime.Li return nil, fmt.Errorf("failed to list metadata from sandbox store: %v", err) } - resp, err := c.containerService.List(ctx, &execution.ListRequest{}) + resp, err := c.taskService.List(ctx, &execution.ListRequest{}) if err != nil { return nil, fmt.Errorf("failed to list sandbox containers: %v", err) } diff --git a/pkg/server/sandbox_list_test.go b/pkg/server/sandbox_list_test.go index 4f381c810..c09fd0df5 100644 --- a/pkg/server/sandbox_list_test.go +++ b/pkg/server/sandbox_list_test.go @@ -135,7 +135,7 @@ func TestFilterSandboxes(t *testing.T) { func TestListPodSandbox(t *testing.T) { c := newTestCRIContainerdService() - fake := c.containerService.(*servertesting.FakeExecutionClient) + fake := c.taskService.(*servertesting.FakeExecutionClient) sandboxesInStore := []metadata.SandboxMetadata{ { @@ -197,8 +197,8 @@ func TestListPodSandbox(t *testing.T) { c.sandboxStore.Create(s) } - // Inject fake containerd containers - fake.SetFakeContainers(sandboxesInContainerd) + // Inject fake containerd tasks + fake.SetFakeTasks(sandboxesInContainerd) resp, err := c.ListPodSandbox(context.Background(), &runtime.ListPodSandboxRequest{}) assert.NoError(t, err) diff --git a/pkg/server/sandbox_remove.go b/pkg/server/sandbox_remove.go index acf30a49d..18cd28a37 100644 --- a/pkg/server/sandbox_remove.go +++ b/pkg/server/sandbox_remove.go @@ -56,8 +56,8 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. // Return error if sandbox container is not fully stopped. // TODO(random-liu): [P0] Make sure network is torn down, may need to introduce a state. - _, err = c.containerService.Info(ctx, &execution.InfoRequest{ContainerID: id}) - if err != nil && !isContainerdContainerNotExistError(err) { + _, err = c.taskService.Info(ctx, &execution.InfoRequest{ContainerID: id}) + if err != nil && !isContainerdGRPCNotFoundError(err) { return nil, fmt.Errorf("failed to get sandbox container info for %q: %v", id, err) } if err == nil { diff --git a/pkg/server/sandbox_remove_test.go b/pkg/server/sandbox_remove_test.go index 2cc08542d..d6cfc994a 100644 --- a/pkg/server/sandbox_remove_test.go +++ b/pkg/server/sandbox_remove_test.go @@ -38,7 +38,7 @@ func TestRemovePodSandbox(t *testing.T) { Name: testName, } for desc, test := range map[string]struct { - sandboxContainers []task.Task + sandboxTasks []task.Task injectMetadata bool removeSnapshotErr error injectContainerdErr error @@ -66,10 +66,10 @@ func TestRemovePodSandbox(t *testing.T) { expectCalls: []string{"info"}, }, "should return error when sandbox container is not deleted": { - injectMetadata: true, - sandboxContainers: []task.Task{{ID: testID}}, - expectErr: true, - expectCalls: []string{"info"}, + injectMetadata: true, + sandboxTasks: []task.Task{{ID: testID}}, + expectErr: true, + expectCalls: []string{"info"}, }, "should return error when arbitrary containerd error is injected": { injectMetadata: true, @@ -92,10 +92,10 @@ func TestRemovePodSandbox(t *testing.T) { } { t.Logf("TestCase %q", desc) c := newTestCRIContainerdService() - fake := c.containerService.(*servertesting.FakeExecutionClient) + fake := c.taskService.(*servertesting.FakeExecutionClient) fakeOS := c.os.(*ostesting.FakeOS) fakeSnapshotClient := WithFakeSnapshotClient(c) - fake.SetFakeContainers(test.sandboxContainers) + fake.SetFakeTasks(test.sandboxTasks) if test.injectMetadata { c.sandboxNameIndex.Reserve(testName, testID) c.sandboxIDIndex.Add(testID) diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index 39b6c794a..79539e610 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -172,7 +172,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run // Create sandbox container in containerd. glog.V(5).Infof("Create sandbox container (id=%q, name=%q) with options %+v.", id, name, createOpts) - createResp, err := c.containerService.Create(ctx, createOpts) + createResp, err := c.taskService.Create(ctx, createOpts) if err != nil { return nil, fmt.Errorf("failed to create sandbox container %q: %v", id, err) @@ -180,7 +180,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run defer func() { if retErr != nil { // Cleanup the sandbox container if an error is returned. - if _, err = c.containerService.Delete(ctx, &execution.DeleteRequest{ContainerID: id}); err != nil { + if _, err = c.taskService.Delete(ctx, &execution.DeleteRequest{ContainerID: id}); err != nil { glog.Errorf("Failed to delete sandbox container %q: %v", id, err) } @@ -206,7 +206,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run } // Start sandbox container in containerd. - if _, err := c.containerService.Start(ctx, &execution.StartRequest{ContainerID: id}); err != nil { + if _, err := c.taskService.Start(ctx, &execution.StartRequest{ContainerID: id}); err != nil { return nil, fmt.Errorf("failed to start sandbox container %q: %v", id, err) } diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index becbaa9b7..c3dedeade 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -269,7 +269,7 @@ func TestRunPodSandbox(t *testing.T) { config, imageConfig, _ := getRunPodSandboxTestData() // TODO: declare and test specCheck see below c := newTestCRIContainerdService() fakeSnapshotClient := WithFakeSnapshotClient(c) - fakeExecutionClient := c.containerService.(*servertesting.FakeExecutionClient) + fakeExecutionClient := c.taskService.(*servertesting.FakeExecutionClient) fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) fakeOS := c.os.(*ostesting.FakeOS) var dirs []string diff --git a/pkg/server/sandbox_status.go b/pkg/server/sandbox_status.go index 3511b47f5..309064ae7 100644 --- a/pkg/server/sandbox_status.go +++ b/pkg/server/sandbox_status.go @@ -47,8 +47,8 @@ func (c *criContainerdService) PodSandboxStatus(ctx context.Context, r *runtime. // Use the full sandbox id. id := sandbox.ID - info, err := c.containerService.Info(ctx, &execution.InfoRequest{ContainerID: id}) - if err != nil && !isContainerdContainerNotExistError(err) { + info, err := c.taskService.Info(ctx, &execution.InfoRequest{ContainerID: id}) + if err != nil && !isContainerdGRPCNotFoundError(err) { return nil, fmt.Errorf("failed to get sandbox container info for %q: %v", id, err) } diff --git a/pkg/server/sandbox_status_test.go b/pkg/server/sandbox_status_test.go index a9c1fdb03..950cefd3b 100644 --- a/pkg/server/sandbox_status_test.go +++ b/pkg/server/sandbox_status_test.go @@ -95,15 +95,15 @@ func getSandboxStatusTestData() (*metadata.SandboxMetadata, *runtime.PodSandboxS func TestPodSandboxStatus(t *testing.T) { for desc, test := range map[string]struct { - sandboxContainers []task.Task - injectMetadata bool - injectErr error - injectIP bool - injectCNIErr error - expectState runtime.PodSandboxState - expectErr bool - expectCalls []string - expectedCNICalls []string + sandboxTasks []task.Task + injectMetadata bool + injectErr error + injectIP bool + injectCNIErr error + expectState runtime.PodSandboxState + expectErr bool + expectCalls []string + expectedCNICalls []string }{ "sandbox status without metadata": { injectMetadata: false, @@ -112,7 +112,7 @@ func TestPodSandboxStatus(t *testing.T) { expectedCNICalls: []string{}, }, "sandbox status with running sandbox container": { - sandboxContainers: []task.Task{{ + sandboxTasks: []task.Task{{ ID: sandboxStatusTestID, Pid: 1, Status: task.StatusRunning, @@ -123,7 +123,7 @@ func TestPodSandboxStatus(t *testing.T) { expectedCNICalls: []string{"GetContainerNetworkStatus"}, }, "sandbox status with stopped sandbox container": { - sandboxContainers: []task.Task{{ + sandboxTasks: []task.Task{{ ID: sandboxStatusTestID, Pid: 1, Status: task.StatusStopped, @@ -134,14 +134,14 @@ func TestPodSandboxStatus(t *testing.T) { expectedCNICalls: []string{"GetContainerNetworkStatus"}, }, "sandbox status with non-existing sandbox container": { - sandboxContainers: []task.Task{}, - injectMetadata: true, - expectState: runtime.PodSandboxState_SANDBOX_NOTREADY, - expectCalls: []string{"info"}, - expectedCNICalls: []string{"GetContainerNetworkStatus"}, + sandboxTasks: []task.Task{}, + injectMetadata: true, + expectState: runtime.PodSandboxState_SANDBOX_NOTREADY, + expectCalls: []string{"info"}, + expectedCNICalls: []string{"GetContainerNetworkStatus"}, }, "sandbox status with arbitrary error": { - sandboxContainers: []task.Task{{ + sandboxTasks: []task.Task{{ ID: sandboxStatusTestID, Pid: 1, Status: task.StatusRunning, @@ -154,7 +154,7 @@ func TestPodSandboxStatus(t *testing.T) { expectedCNICalls: []string{}, }, "sandbox status with IP address": { - sandboxContainers: []task.Task{{ + sandboxTasks: []task.Task{{ ID: sandboxStatusTestID, Pid: 1, Status: task.StatusRunning, @@ -166,7 +166,7 @@ func TestPodSandboxStatus(t *testing.T) { expectedCNICalls: []string{"GetContainerNetworkStatus"}, }, "sandbox status with GetContainerNetworkStatus returns error": { - sandboxContainers: []task.Task{{ + sandboxTasks: []task.Task{{ ID: sandboxStatusTestID, Pid: 1, Status: task.StatusRunning, @@ -182,9 +182,9 @@ func TestPodSandboxStatus(t *testing.T) { metadata, expect := getSandboxStatusTestData() expect.Network.Ip = "" c := newTestCRIContainerdService() - fake := c.containerService.(*servertesting.FakeExecutionClient) + fake := c.taskService.(*servertesting.FakeExecutionClient) fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) - fake.SetFakeContainers(test.sandboxContainers) + fake.SetFakeTasks(test.sandboxTasks) if test.injectMetadata { assert.NoError(t, c.sandboxIDIndex.Add(metadata.ID)) assert.NoError(t, c.sandboxStore.Create(*metadata)) diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index 037141964..5d6cf9498 100644 --- a/pkg/server/sandbox_stop.go +++ b/pkg/server/sandbox_stop.go @@ -60,8 +60,8 @@ func (c *criContainerdService) StopPodSandbox(ctx context.Context, r *runtime.St // TODO(random-liu): [P1] Handle sandbox container graceful deletion. // Delete the sandbox container from containerd. - _, err = c.containerService.Delete(ctx, &execution.DeleteRequest{ContainerID: id}) - if err != nil && !isContainerdContainerNotExistError(err) { + _, err = c.taskService.Delete(ctx, &execution.DeleteRequest{ContainerID: id}) + if err != nil && !isContainerdGRPCNotFoundError(err) { return nil, fmt.Errorf("failed to delete sandbox container %q: %v", id, err) } diff --git a/pkg/server/sandbox_stop_test.go b/pkg/server/sandbox_stop_test.go index 81b1868b5..cb361d734 100644 --- a/pkg/server/sandbox_stop_test.go +++ b/pkg/server/sandbox_stop_test.go @@ -21,14 +21,11 @@ import ( "os" "testing" + "github.com/containerd/containerd/api/types/task" "github.com/stretchr/testify/assert" "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" - - "github.com/containerd/containerd/api/types/task" - "github.com/containerd/containerd/plugin" - runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1" "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" @@ -56,14 +53,14 @@ func TestStopPodSandbox(t *testing.T) { } for desc, test := range map[string]struct { - sandboxContainers []task.Task - injectSandbox bool - injectErr error - injectStatErr error - injectCNIErr error - expectErr bool - expectCalls []string - expectedCNICalls []string + sandboxTasks []task.Task + injectSandbox bool + injectErr error + injectStatErr error + injectCNIErr error + expectErr bool + expectCalls []string + expectedCNICalls []string }{ "stop non-existing sandbox": { injectSandbox: false, @@ -72,17 +69,17 @@ func TestStopPodSandbox(t *testing.T) { expectedCNICalls: []string{}, }, "stop sandbox with sandbox container": { - sandboxContainers: []task.Task{testContainer}, - injectSandbox: true, - expectErr: false, - expectCalls: []string{"delete"}, - expectedCNICalls: []string{"TearDownPod"}, + sandboxTasks: []task.Task{testContainer}, + injectSandbox: true, + expectErr: false, + expectCalls: []string{"delete"}, + expectedCNICalls: []string{"TearDownPod"}, }, "stop sandbox with sandbox container not exist error": { - sandboxContainers: []task.Task{}, - injectSandbox: true, + sandboxTasks: []task.Task{}, + injectSandbox: true, // Inject error to make sure fake execution client returns error. - injectErr: grpc.Errorf(codes.Unknown, plugin.ErrContainerNotExist.Error()), + injectErr: servertesting.TaskNotExistError, expectErr: false, expectCalls: []string{"delete"}, expectedCNICalls: []string{"TearDownPod"}, @@ -95,36 +92,36 @@ func TestStopPodSandbox(t *testing.T) { expectedCNICalls: []string{"TearDownPod"}, }, "stop sandbox with Stat returns arbitrary error": { - sandboxContainers: []task.Task{testContainer}, - injectSandbox: true, - expectErr: true, - injectStatErr: errors.New("arbitrary error"), - expectCalls: []string{}, - expectedCNICalls: []string{}, + sandboxTasks: []task.Task{testContainer}, + injectSandbox: true, + expectErr: true, + injectStatErr: errors.New("arbitrary error"), + expectCalls: []string{}, + expectedCNICalls: []string{}, }, "stop sandbox with Stat returns not exist error": { - sandboxContainers: []task.Task{testContainer}, - injectSandbox: true, - expectErr: false, - expectCalls: []string{"delete"}, - injectStatErr: os.ErrNotExist, - expectedCNICalls: []string{}, + sandboxTasks: []task.Task{testContainer}, + injectSandbox: true, + expectErr: false, + expectCalls: []string{"delete"}, + injectStatErr: os.ErrNotExist, + expectedCNICalls: []string{}, }, "stop sandbox with TearDownPod fails": { - sandboxContainers: []task.Task{testContainer}, - injectSandbox: true, - expectErr: true, - expectedCNICalls: []string{"TearDownPod"}, - injectCNIErr: errors.New("arbitrary error"), - expectCalls: []string{}, + sandboxTasks: []task.Task{testContainer}, + injectSandbox: true, + expectErr: true, + expectedCNICalls: []string{"TearDownPod"}, + injectCNIErr: errors.New("arbitrary error"), + expectCalls: []string{}, }, } { t.Logf("TestCase %q", desc) c := newTestCRIContainerdService() - fake := c.containerService.(*servertesting.FakeExecutionClient) + fake := c.taskService.(*servertesting.FakeExecutionClient) fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) fakeOS := c.os.(*ostesting.FakeOS) - fake.SetFakeContainers(test.sandboxContainers) + fake.SetFakeTasks(test.sandboxTasks) if test.injectSandbox { assert.NoError(t, c.sandboxStore.Create(testSandbox)) diff --git a/pkg/server/service.go b/pkg/server/service.go index 3a2d9fc0b..d2b1524b7 100644 --- a/pkg/server/service.go +++ b/pkg/server/service.go @@ -77,8 +77,8 @@ type criContainerdService struct { // containerNameIndex stores all container names and make sure each // name is unique. containerNameIndex *registrar.Registrar - // containerService is containerd tasks client. - containerService execution.TasksClient + // taskService is containerd tasks client. + taskService execution.TasksClient // contentStoreService is the containerd content service client. contentStoreService content.Store // snapshotService is the containerd snapshot service client. @@ -114,7 +114,7 @@ func NewCRIContainerdService(conn *grpc.ClientConn, rootDir, networkPluginBinDir sandboxIDIndex: truncindex.NewTruncIndex(nil), // TODO(random-liu): Add container id index. containerNameIndex: registrar.NewRegistrar(), - containerService: execution.NewTasksClient(conn), + taskService: execution.NewTasksClient(conn), imageStoreService: imagesservice.NewStoreFromClient(imagesapi.NewImagesClient(conn)), contentStoreService: contentservice.NewStoreFromClient(contentapi.NewContentClient(conn)), snapshotService: snapshotservice.NewSnapshotterFromClient(snapshotapi.NewSnapshotClient(conn)), diff --git a/pkg/server/service_test.go b/pkg/server/service_test.go index 0db57ee79..84ee55b76 100644 --- a/pkg/server/service_test.go +++ b/pkg/server/service_test.go @@ -65,7 +65,7 @@ func newTestCRIContainerdService() *criContainerdService { sandboxIDIndex: truncindex.NewTruncIndex(nil), containerStore: metadata.NewContainerStore(store.NewMetadataStore()), containerNameIndex: registrar.NewRegistrar(), - containerService: servertesting.NewFakeExecutionClient(), + taskService: servertesting.NewFakeExecutionClient(), netPlugin: servertesting.NewFakeCNIPlugin(), agentFactory: agentstesting.NewFakeAgentFactory(), } @@ -81,7 +81,7 @@ func WithFakeSnapshotClient(c *criContainerdService) *servertesting.FakeSnapshot // Test all sandbox operations. func TestSandboxOperations(t *testing.T) { c := newTestCRIContainerdService() - fake := c.containerService.(*servertesting.FakeExecutionClient) + fake := c.taskService.(*servertesting.FakeExecutionClient) fakeOS := c.os.(*ostesting.FakeOS) fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) WithFakeSnapshotClient(c) diff --git a/pkg/server/testing/fake_execution_client.go b/pkg/server/testing/fake_execution_client.go index 7a3182529..5e9dfa416 100644 --- a/pkg/server/testing/fake_execution_client.go +++ b/pkg/server/testing/fake_execution_client.go @@ -24,15 +24,14 @@ import ( "github.com/containerd/containerd/api/services/execution" "github.com/containerd/containerd/api/types/task" - "github.com/containerd/containerd/plugin" googleprotobuf "github.com/golang/protobuf/ptypes/empty" "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" ) -// ContainerNotExistError is the fake error returned when container does not exist. -var ContainerNotExistError = grpc.Errorf(codes.Unknown, plugin.ErrContainerNotExist.Error()) +// TaskNotExistError is the fake error returned when task does not exist. +var TaskNotExistError = grpc.Errorf(codes.NotFound, "task does not exist") // CalledDetail is the struct contains called function name and arguments. type CalledDetail struct { @@ -44,7 +43,7 @@ type CalledDetail struct { var _ execution.Tasks_EventsClient = &EventClient{} -// EventClient is a test implementation of execution.ContainerService_EventsClient +// EventClient is a test implementation of execution.Tasks_EventsClient type EventClient struct { Events chan *task.Event grpc.ClientStream @@ -63,11 +62,11 @@ func (cli *EventClient) Recv() (*task.Event, error) { // can be run for testing without requiring a real containerd setup. type FakeExecutionClient struct { sync.Mutex - called []CalledDetail - errors map[string]error - ContainerList map[string]task.Task - eventsQueue chan *task.Event - eventClients []*EventClient + called []CalledDetail + errors map[string]error + TaskList map[string]task.Task + eventsQueue chan *task.Event + eventClients []*EventClient } var _ execution.TasksClient = &FakeExecutionClient{} @@ -75,8 +74,8 @@ var _ execution.TasksClient = &FakeExecutionClient{} // NewFakeExecutionClient creates a FakeExecutionClient func NewFakeExecutionClient() *FakeExecutionClient { return &FakeExecutionClient{ - errors: make(map[string]error), - ContainerList: make(map[string]task.Task), + errors: make(map[string]error), + TaskList: make(map[string]task.Task), } } @@ -183,12 +182,12 @@ func (f *FakeExecutionClient) GetCalledDetails() []CalledDetail { return append([]CalledDetail{}, f.called...) } -// SetFakeContainers injects fake containers. -func (f *FakeExecutionClient) SetFakeContainers(containers []task.Task) { +// SetFakeTasks injects fake tasks. +func (f *FakeExecutionClient) SetFakeTasks(tasks []task.Task) { f.Lock() defer f.Unlock() - for _, c := range containers { - f.ContainerList[c.ID] = c + for _, t := range tasks { + f.TaskList[t.ID] = t } } @@ -200,12 +199,12 @@ func (f *FakeExecutionClient) Create(ctx context.Context, createOpts *execution. if err := f.getError("create"); err != nil { return nil, err } - _, ok := f.ContainerList[createOpts.ContainerID] + _, ok := f.TaskList[createOpts.ContainerID] if ok { - return nil, plugin.ErrContainerExists + return nil, fmt.Errorf("task already exists") } pid := generatePid() - f.ContainerList[createOpts.ContainerID] = task.Task{ + f.TaskList[createOpts.ContainerID] = task.Task{ ContainerID: createOpts.ContainerID, Pid: pid, Status: task.StatusCreated, @@ -229,9 +228,9 @@ func (f *FakeExecutionClient) Start(ctx context.Context, startOpts *execution.St if err := f.getError("start"); err != nil { return nil, err } - c, ok := f.ContainerList[startOpts.ContainerID] + c, ok := f.TaskList[startOpts.ContainerID] if !ok { - return nil, ContainerNotExistError + return nil, TaskNotExistError } f.sendEvent(&task.Event{ ID: c.ID, @@ -241,7 +240,7 @@ func (f *FakeExecutionClient) Start(ctx context.Context, startOpts *execution.St switch c.Status { case task.StatusCreated: c.Status = task.StatusRunning - f.ContainerList[startOpts.ContainerID] = c + f.TaskList[startOpts.ContainerID] = c return &googleprotobuf.Empty{}, nil case task.StatusStopped: return &googleprotobuf.Empty{}, fmt.Errorf("cannot start a container that has stopped") @@ -260,11 +259,11 @@ func (f *FakeExecutionClient) Delete(ctx context.Context, deleteOpts *execution. if err := f.getError("delete"); err != nil { return nil, err } - c, ok := f.ContainerList[deleteOpts.ContainerID] + c, ok := f.TaskList[deleteOpts.ContainerID] if !ok { - return nil, ContainerNotExistError + return nil, TaskNotExistError } - delete(f.ContainerList, deleteOpts.ContainerID) + delete(f.TaskList, deleteOpts.ContainerID) f.sendEvent(&task.Event{ ID: c.ID, Type: task.Event_EXIT, @@ -281,9 +280,9 @@ func (f *FakeExecutionClient) Info(ctx context.Context, infoOpts *execution.Info if err := f.getError("info"); err != nil { return nil, err } - c, ok := f.ContainerList[infoOpts.ContainerID] + c, ok := f.TaskList[infoOpts.ContainerID] if !ok { - return nil, ContainerNotExistError + return nil, TaskNotExistError } return &execution.InfoResponse{Task: &c}, nil } @@ -297,7 +296,7 @@ func (f *FakeExecutionClient) List(ctx context.Context, listOpts *execution.List return nil, err } resp := &execution.ListResponse{} - for _, c := range f.ContainerList { + for _, c := range f.TaskList { resp.Tasks = append(resp.Tasks, &task.Task{ ID: c.ID, Pid: c.Pid, @@ -315,12 +314,12 @@ func (f *FakeExecutionClient) Kill(ctx context.Context, killOpts *execution.Kill if err := f.getError("kill"); err != nil { return nil, err } - c, ok := f.ContainerList[killOpts.ContainerID] + c, ok := f.TaskList[killOpts.ContainerID] if !ok { - return nil, ContainerNotExistError + return nil, TaskNotExistError } c.Status = task.StatusStopped - f.ContainerList[killOpts.ContainerID] = c + f.TaskList[killOpts.ContainerID] = c f.sendEvent(&task.Event{ ID: c.ID, Type: task.Event_EXIT,