From 5b7cbf1bc615258251e65b5cce4729d3dc406f17 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Fri, 16 Jun 2017 00:15:16 +0000 Subject: [PATCH] Create/remove sandbox container. Signed-off-by: Lantao Liu --- pkg/server/sandbox_remove.go | 9 ++++ pkg/server/sandbox_remove_test.go | 72 +++++++++++++++++++++---------- pkg/server/sandbox_run.go | 49 +++++++++++++++------ pkg/server/sandbox_run_test.go | 38 ++++++++++------ 4 files changed, 121 insertions(+), 47 deletions(-) diff --git a/pkg/server/sandbox_remove.go b/pkg/server/sandbox_remove.go index 18cd28a37..d490175e2 100644 --- a/pkg/server/sandbox_remove.go +++ b/pkg/server/sandbox_remove.go @@ -19,6 +19,7 @@ package server import ( "fmt" + "github.com/containerd/containerd/api/services/containers" "github.com/containerd/containerd/api/services/execution" "github.com/containerd/containerd/snapshot" "github.com/golang/glog" @@ -82,6 +83,14 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. sandboxRootDir, err) } + // Delete sandbox container. + if _, err := c.containerService.Delete(ctx, &containers.DeleteContainerRequest{ID: id}); err != nil { + if !isContainerdGRPCNotFoundError(err) { + return nil, fmt.Errorf("failed to delete sandbox container %q: %v", id, err) + } + glog.V(5).Infof("Remove called for sandbox container %q that does not exist", id, err) + } + // Remove sandbox metadata from metadata store. Note that once the sandbox // metadata is successfully deleted: // 1) ListPodSandbox will not include this sandbox. diff --git a/pkg/server/sandbox_remove_test.go b/pkg/server/sandbox_remove_test.go index d6cfc994a..378d6fee5 100644 --- a/pkg/server/sandbox_remove_test.go +++ b/pkg/server/sandbox_remove_test.go @@ -18,6 +18,7 @@ import ( "fmt" "testing" + "github.com/containerd/containerd/api/services/containers" snapshotapi "github.com/containerd/containerd/api/services/snapshot" "github.com/containerd/containerd/api/types/mount" "github.com/containerd/containerd/api/types/task" @@ -38,20 +39,22 @@ func TestRemovePodSandbox(t *testing.T) { Name: testName, } for desc, test := range map[string]struct { - sandboxTasks []task.Task - injectMetadata bool - removeSnapshotErr error - injectContainerdErr error - injectFSErr error - expectErr bool - expectRemoved string - expectCalls []string + sandboxTasks []task.Task + injectMetadata bool + removeSnapshotErr error + deleteContainerErr error + taskInfoErr error + injectFSErr error + expectErr bool + expectRemoved string + expectCalls []string }{ "should not return error if sandbox does not exist": { - injectMetadata: false, - removeSnapshotErr: servertesting.SnapshotNotExistError, - expectErr: false, - expectCalls: []string{}, + injectMetadata: false, + removeSnapshotErr: servertesting.SnapshotNotExistError, + deleteContainerErr: servertesting.ContainerNotExistError, + expectErr: false, + expectCalls: []string{}, }, "should not return error if snapshot does not exist": { injectMetadata: true, @@ -65,17 +68,17 @@ func TestRemovePodSandbox(t *testing.T) { expectErr: true, expectCalls: []string{"info"}, }, - "should return error when sandbox container is not deleted": { + "should return error when sandbox container task is not deleted": { injectMetadata: true, sandboxTasks: []task.Task{{ID: testID}}, expectErr: true, expectCalls: []string{"info"}, }, "should return error when arbitrary containerd error is injected": { - injectMetadata: true, - injectContainerdErr: fmt.Errorf("arbitrary error"), - expectErr: true, - expectCalls: []string{"info"}, + injectMetadata: true, + taskInfoErr: fmt.Errorf("arbitrary error"), + expectErr: true, + expectCalls: []string{"info"}, }, "should return error when error fs error is injected": { injectMetadata: true, @@ -84,6 +87,19 @@ func TestRemovePodSandbox(t *testing.T) { expectErr: true, expectCalls: []string{"info"}, }, + "should not return error if sandbox container does not exist": { + injectMetadata: true, + deleteContainerErr: servertesting.ContainerNotExistError, + expectRemoved: getSandboxRootDir(testRootDir, testID), + expectCalls: []string{"info"}, + }, + "should return error if delete sandbox container fails": { + injectMetadata: true, + deleteContainerErr: fmt.Errorf("arbitrary error"), + expectRemoved: getSandboxRootDir(testRootDir, testID), + expectErr: true, + expectCalls: []string{"info"}, + }, "should be able to successfully delete": { injectMetadata: true, expectRemoved: getSandboxRootDir(testRootDir, testID), @@ -92,10 +108,11 @@ func TestRemovePodSandbox(t *testing.T) { } { t.Logf("TestCase %q", desc) c := newTestCRIContainerdService() - fake := c.taskService.(*servertesting.FakeExecutionClient) + fake := c.containerService.(*servertesting.FakeContainersClient) fakeOS := c.os.(*ostesting.FakeOS) + fakeExecutionClient := c.taskService.(*servertesting.FakeExecutionClient) fakeSnapshotClient := WithFakeSnapshotClient(c) - fake.SetFakeTasks(test.sandboxTasks) + fakeExecutionClient.SetFakeTasks(test.sandboxTasks) if test.injectMetadata { c.sandboxNameIndex.Reserve(testName, testID) c.sandboxIDIndex.Add(testID) @@ -112,8 +129,16 @@ func TestRemovePodSandbox(t *testing.T) { } else { fakeSnapshotClient.InjectError("remove", test.removeSnapshotErr) } - if test.injectContainerdErr != nil { - fake.InjectError("info", test.injectContainerdErr) + if test.deleteContainerErr == nil { + _, err := fake.Create(context.Background(), &containers.CreateContainerRequest{ + Container: containers.Container{ID: testID}, + }) + assert.NoError(t, err) + } else { + fake.InjectError("delete", test.deleteContainerErr) + } + if test.taskInfoErr != nil { + fakeExecutionClient.InjectError("info", test.taskInfoErr) } fakeOS.RemoveAllFn = func(path string) error { assert.Equal(t, test.expectRemoved, path) @@ -122,7 +147,7 @@ func TestRemovePodSandbox(t *testing.T) { res, err := c.RemovePodSandbox(context.Background(), &runtime.RemovePodSandboxRequest{ PodSandboxId: testID, }) - assert.Equal(t, test.expectCalls, fake.GetCalledNames()) + assert.Equal(t, test.expectCalls, fakeExecutionClient.GetCalledNames()) if test.expectErr { assert.Error(t, err) assert.Nil(t, res) @@ -141,6 +166,9 @@ func TestRemovePodSandbox(t *testing.T) { mountsResp, err := fakeSnapshotClient.Mounts(context.Background(), &snapshotapi.MountsRequest{Key: testID}) assert.Equal(t, servertesting.SnapshotNotExistError, err, "snapshot should be removed") assert.Nil(t, mountsResp) + getResp, err := fake.Get(context.Background(), &containers.GetContainerRequest{ID: testID}) + assert.Equal(t, servertesting.ContainerNotExistError, err, "containerd container should be removed") + assert.Nil(t, getResp) res, err = c.RemovePodSandbox(context.Background(), &runtime.RemovePodSandboxRequest{ PodSandboxId: testID, }) diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index f621a471d..1099257ae 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -23,8 +23,10 @@ import ( "strings" "time" + "github.com/containerd/containerd/api/services/containers" "github.com/containerd/containerd/api/services/execution" "github.com/containerd/containerd/api/types/mount" + prototypes "github.com/gogo/protobuf/types" "github.com/golang/glog" imagespec "github.com/opencontainers/image-spec/specs-go/v1" runtimespec "github.com/opencontainers/runtime-spec/specs-go" @@ -85,6 +87,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run if err != nil { return nil, fmt.Errorf("failed to get sandbox image %q: %v", defaultSandboxImage, err) } + rootfsMounts, err := c.snapshotService.View(ctx, id, imageMeta.ChainID) if err != nil { return nil, fmt.Errorf("failed to prepare sandbox rootfs %q: %v", imageMeta.ChainID, err) @@ -105,6 +108,39 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run }) } + // Create sandbox container. + spec, err := c.generateSandboxContainerSpec(id, config, imageMeta.Config) + if err != nil { + return nil, fmt.Errorf("failed to generate sandbox container spec: %v", err) + } + rawSpec, err := json.Marshal(spec) + if err != nil { + return nil, fmt.Errorf("failed to marshal oci spec %+v: %v", spec, err) + } + glog.V(4).Infof("Sandbox container spec: %+v", spec) + if _, err = c.containerService.Create(ctx, &containers.CreateContainerRequest{ + Container: containers.Container{ + ID: id, + // TODO(random-liu): Checkpoint metadata into container labels. + Image: imageMeta.ID, + Runtime: defaultRuntime, + Spec: &prototypes.Any{ + TypeUrl: runtimespec.Version, + Value: rawSpec, + }, + RootFS: id, + }, + }); err != nil { + return nil, fmt.Errorf("failed to create containerd container: %v", err) + } + defer func() { + if retErr != nil { + if _, err := c.containerService.Delete(ctx, &containers.DeleteContainerRequest{ID: id}); err != nil { + glog.Errorf("Failed to delete containerd container%q: %v", id, err) + } + } + }() + // Create sandbox container root directory. // Prepare streaming named pipe. sandboxRootDir := getSandboxRootDir(c.rootDir, id) @@ -151,16 +187,6 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run } }() - // Start sandbox container. - spec, err := c.generateSandboxContainerSpec(id, config, imageMeta.Config) - if err != nil { - return nil, fmt.Errorf("failed to generate sandbox container spec: %v", err) - } - _, err = json.Marshal(spec) - if err != nil { - return nil, fmt.Errorf("failed to marshal oci spec %+v: %v", spec, err) - } - glog.V(4).Infof("Sandbox container spec: %+v", spec) createOpts := &execution.CreateRequest{ ContainerID: id, Rootfs: rootfs, @@ -168,8 +194,7 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run Stdout: stdout, Stderr: stderr, } - - // Create sandbox container in containerd. + // Create sandbox task in containerd. glog.V(5).Infof("Create sandbox container (id=%q, name=%q) with options %+v.", id, name, createOpts) createResp, err := c.taskService.Create(ctx, createOpts) diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index c3dedeade..f8ed18929 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -17,11 +17,13 @@ limitations under the License. package server import ( + "encoding/json" "io" "os" "syscall" "testing" + "github.com/containerd/containerd/api/services/containers" "github.com/containerd/containerd/api/services/execution" snapshotapi "github.com/containerd/containerd/api/services/snapshot" imagespec "github.com/opencontainers/image-spec/specs-go/v1" @@ -266,8 +268,9 @@ options timeout:1 } func TestRunPodSandbox(t *testing.T) { - config, imageConfig, _ := getRunPodSandboxTestData() // TODO: declare and test specCheck see below + config, imageConfig, specCheck := getRunPodSandboxTestData() c := newTestCRIContainerdService() + fake := c.containerService.(*servertesting.FakeContainersClient) fakeSnapshotClient := WithFakeSnapshotClient(c) fakeExecutionClient := c.taskService.(*servertesting.FakeExecutionClient) fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) @@ -292,6 +295,7 @@ func TestRunPodSandbox(t *testing.T) { } // Insert sandbox image metadata. assert.NoError(t, c.imageMetadataStore.Create(imageMetadata)) + expectContainersClientCalls := []string{"create"} expectSnapshotClientCalls := []string{"view"} expectExecutionClientCalls := []string{"create", "start"} @@ -316,21 +320,29 @@ func TestRunPodSandbox(t *testing.T) { Parent: testChainID, }, prepareOpts, "prepare request should be correct") - assert.Equal(t, expectExecutionClientCalls, fakeExecutionClient.GetCalledNames(), "expect containerd functions should be called") + assert.Equal(t, expectContainersClientCalls, fake.GetCalledNames(), "expect containers functions should be called") + calls = fake.GetCalledDetails() + createOpts, ok := calls[0].Argument.(*containers.CreateContainerRequest) + assert.True(t, ok, "should create sandbox container") + assert.Equal(t, id, createOpts.Container.ID, "container id should be correct") + assert.Equal(t, testSandboxImage, createOpts.Container.Image, "test image should be correct") + assert.Equal(t, id, createOpts.Container.RootFS, "rootfs should be correct") + spec := &runtimespec.Spec{} + assert.NoError(t, json.Unmarshal(createOpts.Container.Spec.Value, spec)) + t.Logf("oci spec check") + specCheck(t, id, spec) + + assert.Equal(t, expectExecutionClientCalls, fakeExecutionClient.GetCalledNames(), "expect execution functions should be called") calls = fakeExecutionClient.GetCalledDetails() - createOpts := calls[0].Argument.(*execution.CreateRequest) - assert.Equal(t, id, createOpts.ContainerID, "create id should be correct") - assert.Equal(t, stdout, createOpts.Stdout, "stdout pipe should be passed to containerd") - assert.Equal(t, stderr, createOpts.Stderr, "stderr pipe should be passed to containerd") + createTaskOpts := calls[0].Argument.(*execution.CreateRequest) mountsResp, err := fakeSnapshotClient.Mounts(context.Background(), &snapshotapi.MountsRequest{Key: id}) assert.NoError(t, err) - assert.Equal(t, mountsResp.Mounts, createOpts.Rootfs, "rootfs mount should be correct") - - // TODO: Need to create container first.. see Create in containerd/containerd/apsi/services/containers spec is no longer in the create request - //spec := &runtimespec.Spec{} - //assert.NoError(t, json.Unmarshal(createOpts.Spec.Value, spec)) - //t.Logf("oci spec check") - //specCheck(t, id, spec) + assert.Equal(t, &execution.CreateRequest{ + ContainerID: id, + Rootfs: mountsResp.Mounts, + Stdout: stdout, + Stderr: stderr, + }, createTaskOpts, "create options should be correct") startID := calls[1].Argument.(*execution.StartRequest).ContainerID assert.Equal(t, id, startID, "start id should be correct")