From 4317e6119ae8c1068b8155e6b7e6d0457788c063 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Mon, 5 Jun 2017 20:59:50 +0000 Subject: [PATCH] Remove sandbox truncindex. Signed-off-by: Lantao Liu --- pkg/server/container_create.go | 2 +- pkg/server/container_start.go | 2 +- pkg/server/helpers.go | 24 ----------------- pkg/server/helpers_test.go | 44 ------------------------------- pkg/server/sandbox_list.go | 12 +-------- pkg/server/sandbox_remove.go | 5 +--- pkg/server/sandbox_remove_test.go | 4 --- pkg/server/sandbox_run.go | 10 ------- pkg/server/sandbox_run_test.go | 4 --- pkg/server/sandbox_status.go | 2 +- pkg/server/sandbox_status_test.go | 1 - pkg/server/sandbox_stop.go | 2 +- pkg/server/sandbox_stop_test.go | 2 -- pkg/server/service.go | 23 +++++----------- pkg/server/service_test.go | 2 -- 15 files changed, 13 insertions(+), 126 deletions(-) diff --git a/pkg/server/container_create.go b/pkg/server/container_create.go index 81f3d636b..84709a44f 100644 --- a/pkg/server/container_create.go +++ b/pkg/server/container_create.go @@ -47,7 +47,7 @@ func (c *criContainerdService) CreateContainer(ctx context.Context, r *runtime.C config := r.GetConfig() sandboxConfig := r.GetSandboxConfig() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { return nil, fmt.Errorf("failed to find sandbox id %q: %v", r.GetPodSandboxId(), err) } diff --git a/pkg/server/container_start.go b/pkg/server/container_start.go index b0203cb55..369d0f2b3 100644 --- a/pkg/server/container_start.go +++ b/pkg/server/container_start.go @@ -90,7 +90,7 @@ func (c *criContainerdService) startContainer(ctx context.Context, id string, me }() // Get sandbox config from sandbox store. - sandboxMeta, err := c.getSandbox(meta.SandboxID) + sandboxMeta, err := c.sandboxStore.Get(meta.SandboxID) if err != nil { return fmt.Errorf("sandbox %q not found: %v", meta.SandboxID, err) } diff --git a/pkg/server/helpers.go b/pkg/server/helpers.go index 546bc638b..d790ce31a 100644 --- a/pkg/server/helpers.go +++ b/pkg/server/helpers.go @@ -28,7 +28,6 @@ import ( containerdmetadata "github.com/containerd/containerd/metadata" "github.com/docker/distribution/reference" "github.com/docker/docker/pkg/stringid" - "github.com/docker/docker/pkg/truncindex" imagedigest "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" imagespec "github.com/opencontainers/image-spec/specs-go/v1" @@ -236,29 +235,6 @@ func isRuncProcessAlreadyFinishedError(grpcError error) bool { return strings.Contains(grpc.ErrorDesc(grpcError), "os: process already finished") } -// getSandbox gets the sandbox metadata from the sandbox store. It returns nil without -// error if the sandbox metadata is not found. It also tries to get full sandbox id and -// retry if the sandbox metadata is not found with the initial id. -func (c *criContainerdService) getSandbox(id string) (*metadata.SandboxMetadata, error) { - sandbox, err := c.sandboxStore.Get(id) - if err != nil && !metadata.IsNotExistError(err) { - return nil, fmt.Errorf("sandbox metadata not found: %v", err) - } - if err == nil { - return sandbox, nil - } - // sandbox is not found in metadata store, try to extract full id. - id, indexErr := c.sandboxIDIndex.Get(id) - if indexErr != nil { - if indexErr == truncindex.ErrNotExist { - // Return the original error if sandbox id is not found. - return nil, err - } - return nil, fmt.Errorf("sandbox id not found: %v", err) - } - return c.sandboxStore.Get(id) -} - // criContainerStateToString formats CRI container state to string. func criContainerStateToString(state runtime.ContainerState) string { return runtime.ContainerState_name[int32(state)] diff --git a/pkg/server/helpers_test.go b/pkg/server/helpers_test.go index 868705668..02fd1b7bc 100644 --- a/pkg/server/helpers_test.go +++ b/pkg/server/helpers_test.go @@ -28,7 +28,6 @@ import ( "github.com/stretchr/testify/assert" "golang.org/x/net/context" - "github.com/kubernetes-incubator/cri-containerd/pkg/metadata" ostesting "github.com/kubernetes-incubator/cri-containerd/pkg/os/testing" ) @@ -119,49 +118,6 @@ func TestPrepareStreamingPipesError(t *testing.T) { } } -func TestGetSandbox(t *testing.T) { - c := newTestCRIContainerdService() - testID := "abcdefg" - testSandbox := metadata.SandboxMetadata{ - ID: testID, - Name: "test-name", - } - assert.NoError(t, c.sandboxStore.Create(testSandbox)) - assert.NoError(t, c.sandboxIDIndex.Add(testID)) - - for desc, test := range map[string]struct { - id string - expected *metadata.SandboxMetadata - expectErr bool - }{ - "full id": { - id: testID, - expected: &testSandbox, - expectErr: false, - }, - "partial id": { - id: testID[:3], - expected: &testSandbox, - expectErr: false, - }, - "non-exist id": { - id: "gfedcba", - expected: nil, - expectErr: true, - }, - } { - t.Logf("TestCase %q", desc) - sb, err := c.getSandbox(test.id) - if test.expectErr { - assert.Error(t, err) - assert.True(t, metadata.IsNotExistError(err)) - } else { - assert.NoError(t, err) - } - assert.Equal(t, test.expected, sb) - } -} - func TestNormalizeImageRef(t *testing.T) { for _, test := range []struct { input string diff --git a/pkg/server/sandbox_list.go b/pkg/server/sandbox_list.go index 1ff9e0058..69583e847 100644 --- a/pkg/server/sandbox_list.go +++ b/pkg/server/sandbox_list.go @@ -93,20 +93,10 @@ func (c *criContainerdService) filterCRISandboxes(sandboxes []*runtime.PodSandbo return sandboxes } - var filterID string - if filter.GetId() != "" { - // Handle truncate id. Use original filter if failed to convert. - var err error - filterID, err = c.sandboxIDIndex.Get(filter.GetId()) - if err != nil { - filterID = filter.GetId() - } - } - filtered := []*runtime.PodSandbox{} for _, s := range sandboxes { // Filter by id - if filterID != "" && filterID != s.Id { + if filter.GetId() != "" && filter.GetId() != s.Id { continue } // Filter by state diff --git a/pkg/server/sandbox_remove.go b/pkg/server/sandbox_remove.go index 6d93b2a28..8493e1c7c 100644 --- a/pkg/server/sandbox_remove.go +++ b/pkg/server/sandbox_remove.go @@ -39,7 +39,7 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. } }() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { if !metadata.IsNotExistError(err) { return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %v", @@ -117,9 +117,6 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. return nil, fmt.Errorf("failed to delete sandbox metadata for %q: %v", id, err) } - // Release the sandbox id from id index. - c.sandboxIDIndex.Delete(id) // nolint: errcheck - // Release the sandbox name reserved for the sandbox. c.sandboxNameIndex.ReleaseByKey(id) diff --git a/pkg/server/sandbox_remove_test.go b/pkg/server/sandbox_remove_test.go index 50cff7c6b..71d37690e 100644 --- a/pkg/server/sandbox_remove_test.go +++ b/pkg/server/sandbox_remove_test.go @@ -116,7 +116,6 @@ func TestRemovePodSandbox(t *testing.T) { fakeExecutionClient.SetFakeTasks(test.sandboxTasks) if test.injectMetadata { c.sandboxNameIndex.Reserve(testName, testID) - c.sandboxIDIndex.Add(testID) c.sandboxStore.Create(testMetadata) } if test.removeSnapshotErr == nil { @@ -158,8 +157,6 @@ func TestRemovePodSandbox(t *testing.T) { assert.NotNil(t, res) assert.NoError(t, c.sandboxNameIndex.Reserve(testName, testID), "sandbox name should be released") - _, err = c.sandboxIDIndex.Get(testID) - assert.Error(t, err, "sandbox id should be removed") meta, err := c.sandboxStore.Get(testID) assert.Error(t, err) assert.True(t, metadata.IsNotExistError(err)) @@ -209,7 +206,6 @@ func TestRemoveContainersInSandbox(t *testing.T) { c := newTestCRIContainerdService() WithFakeSnapshotClient(c) assert.NoError(t, c.sandboxNameIndex.Reserve(testName, testID)) - assert.NoError(t, c.sandboxIDIndex.Add(testID)) assert.NoError(t, c.sandboxStore.Create(testMetadata)) for _, cntr := range testContainersMetadata { assert.NoError(t, c.containerNameIndex.Reserve(cntr.Name, cntr.ID)) diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index fe91fdaaa..af1759b81 100644 --- a/pkg/server/sandbox_run.go +++ b/pkg/server/sandbox_run.go @@ -64,16 +64,6 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run c.sandboxNameIndex.ReleaseByName(name) } }() - // Register the sandbox id. - if err := c.sandboxIDIndex.Add(id); err != nil { - return nil, fmt.Errorf("failed to insert sandbox id %q: %v", id, err) - } - defer func() { - // Delete the sandbox id if the function returns with an error. - if retErr != nil { - c.sandboxIDIndex.Delete(id) // nolint: errcheck - } - }() // Create initial sandbox metadata. meta := metadata.SandboxMetadata{ diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index 24c5c6cd9..8417ff55d 100644 --- a/pkg/server/sandbox_run_test.go +++ b/pkg/server/sandbox_run_test.go @@ -362,10 +362,6 @@ func TestRunPodSandbox(t *testing.T) { pid := info.Task.Pid assert.Equal(t, meta.NetNS, getNetworkNamespace(pid), "metadata network namespace should be correct") - gotID, err := c.sandboxIDIndex.Get(id) - assert.NoError(t, err) - assert.Equal(t, id, gotID, "sandbox id should be indexed") - expectedCNICalls := []string{"SetUpPod"} assert.Equal(t, expectedCNICalls, fakeCNIPlugin.GetCalledNames(), "expect SetUpPod should be called") calls = fakeCNIPlugin.GetCalledDetails() diff --git a/pkg/server/sandbox_status.go b/pkg/server/sandbox_status.go index 4e0c45a0b..74a1ea743 100644 --- a/pkg/server/sandbox_status.go +++ b/pkg/server/sandbox_status.go @@ -39,7 +39,7 @@ func (c *criContainerdService) PodSandboxStatus(ctx context.Context, r *runtime. } }() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %v", r.GetPodSandboxId(), err) diff --git a/pkg/server/sandbox_status_test.go b/pkg/server/sandbox_status_test.go index 2cf22fc0c..724eca828 100644 --- a/pkg/server/sandbox_status_test.go +++ b/pkg/server/sandbox_status_test.go @@ -186,7 +186,6 @@ func TestPodSandboxStatus(t *testing.T) { fakeCNIPlugin := c.netPlugin.(*servertesting.FakeCNIPlugin) fake.SetFakeTasks(test.sandboxTasks) if test.injectMetadata { - assert.NoError(t, c.sandboxIDIndex.Add(metadata.ID)) assert.NoError(t, c.sandboxStore.Create(*metadata)) } if test.injectErr != nil { diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index dc56f0f7c..ca8062f10 100644 --- a/pkg/server/sandbox_stop.go +++ b/pkg/server/sandbox_stop.go @@ -38,7 +38,7 @@ func (c *criContainerdService) StopPodSandbox(ctx context.Context, r *runtime.St } }() - sandbox, err := c.getSandbox(r.GetPodSandboxId()) + sandbox, err := c.sandboxStore.Get(r.GetPodSandboxId()) if err != nil { return nil, fmt.Errorf("an error occurred when try to find sandbox %q: %v", r.GetPodSandboxId(), err) diff --git a/pkg/server/sandbox_stop_test.go b/pkg/server/sandbox_stop_test.go index bc8e8777c..21ebc73ee 100644 --- a/pkg/server/sandbox_stop_test.go +++ b/pkg/server/sandbox_stop_test.go @@ -137,7 +137,6 @@ func TestStopPodSandbox(t *testing.T) { if test.injectSandbox { assert.NoError(t, c.sandboxStore.Create(testSandbox)) - c.sandboxIDIndex.Add(testID) } if test.injectErr != nil { fake.InjectError("delete", test.injectErr) @@ -234,7 +233,6 @@ func TestStopContainersInSandbox(t *testing.T) { c.taskService = fake fake.SetFakeTasks(testContainerdContainers) assert.NoError(t, c.sandboxStore.Create(testSandbox)) - assert.NoError(t, c.sandboxIDIndex.Add(testID)) for _, cntr := range testContainers { assert.NoError(t, c.containerStore.Create(cntr)) } diff --git a/pkg/server/service.go b/pkg/server/service.go index 5f9c1bd3b..5b92ced0e 100644 --- a/pkg/server/service.go +++ b/pkg/server/service.go @@ -27,7 +27,6 @@ import ( "github.com/containerd/containerd/images" diffservice "github.com/containerd/containerd/services/diff" "github.com/containerd/containerd/snapshot" - "github.com/docker/docker/pkg/truncindex" "github.com/kubernetes-incubator/cri-o/pkg/ocicni" healthapi "google.golang.org/grpc/health/grpc_health_v1" "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" @@ -65,10 +64,6 @@ type criContainerdService struct { // sandboxNameIndex stores all sandbox names and make sure each name // is unique. sandboxNameIndex *registrar.Registrar - // sandboxIDIndex is trie tree for truncated id indexing, e.g. after an - // id "abcdefg" is added, we could use "abcd" to identify the same thing - // as long as there is no ambiguity. - sandboxIDIndex *truncindex.TruncIndex // containerStore stores all container metadata. containerStore metadata.ContainerStore // containerNameIndex stores all container names and make sure each @@ -109,17 +104,13 @@ func NewCRIContainerdService(containerdEndpoint, rootDir, networkPluginBinDir, n } c := &criContainerdService{ - os: osinterface.RealOS{}, - rootDir: rootDir, - sandboxImage: defaultSandboxImage, - sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), - containerStore: metadata.NewContainerStore(store.NewMetadataStore()), - imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), - // TODO(random-liu): Register sandbox/container id/name for recovered sandbox/container. - // TODO(random-liu): Use the same name and id index for both container and sandbox. - sandboxNameIndex: registrar.NewRegistrar(), - sandboxIDIndex: truncindex.NewTruncIndex(nil), - // TODO(random-liu): Add container id index. + os: osinterface.RealOS{}, + rootDir: rootDir, + sandboxImage: defaultSandboxImage, + sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), + containerStore: metadata.NewContainerStore(store.NewMetadataStore()), + imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), + sandboxNameIndex: registrar.NewRegistrar(), containerNameIndex: registrar.NewRegistrar(), containerService: client.ContainerService(), taskService: client.TaskService(), diff --git a/pkg/server/service_test.go b/pkg/server/service_test.go index 6a55d327b..4c15bfd24 100644 --- a/pkg/server/service_test.go +++ b/pkg/server/service_test.go @@ -23,7 +23,6 @@ import ( "github.com/containerd/containerd/api/services/execution" snapshotservice "github.com/containerd/containerd/services/snapshot" - "github.com/docker/docker/pkg/truncindex" imagespec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -62,7 +61,6 @@ func newTestCRIContainerdService() *criContainerdService { sandboxStore: metadata.NewSandboxStore(store.NewMetadataStore()), imageMetadataStore: metadata.NewImageMetadataStore(store.NewMetadataStore()), sandboxNameIndex: registrar.NewRegistrar(), - sandboxIDIndex: truncindex.NewTruncIndex(nil), containerStore: metadata.NewContainerStore(store.NewMetadataStore()), containerNameIndex: registrar.NewRegistrar(), taskService: servertesting.NewFakeExecutionClient(),