From 6234337459c9ad45aada017284bf7416b6392dcd Mon Sep 17 00:00:00 2001 From: yanxuean Date: Mon, 6 Nov 2017 16:47:43 +0800 Subject: [PATCH 1/2] Add truncindex for filter in List and Stat fix #344 Signed-off-by: yanxuean --- integration/container_stats_test.go | 23 +++++++++++++++++++++++ pkg/server/container_list.go | 10 ++++++++++ pkg/server/container_stats_list.go | 10 ++++++++++ pkg/server/sandbox_list.go | 7 +++++++ 4 files changed, 50 insertions(+) diff --git a/integration/container_stats_test.go b/integration/container_stats_test.go index 11991aad9..cc1aee0ce 100644 --- a/integration/container_stats_test.go +++ b/integration/container_stats_test.go @@ -296,6 +296,29 @@ func TestContainerListStatsWithIdSandboxIdFilter(t *testing.T) { testStats(t, s, config) } } + + t.Logf("Fetch container stats for sandbox truncID and container truncID filter ") + for id, config := range containerConfigMap { + require.NoError(t, Eventually(func() (bool, error) { + stats, err = runtimeService.ListContainerStats( + &runtime.ContainerStatsFilter{Id: id[:3], PodSandboxId: sb[:3]}) + if err != nil { + return false, err + } + if len(stats) != 1 { + return false, fmt.Errorf("Unexpected stats length") + } + if stats[0].GetWritableLayer().GetUsedBytes().GetValue() != 0 && + stats[0].GetWritableLayer().GetInodesUsed().GetValue() != 0 { + return true, nil + } + return false, nil + }, time.Second, 30*time.Second)) + t.Logf("Verify container stats for sandbox %q and container %q filter", sb, id) + for _, s := range stats { + testStats(t, s, config) + } + } } // TODO make this as options to use for dead container tests diff --git a/pkg/server/container_list.go b/pkg/server/container_list.go index 215602a35..6fb8ef1a3 100644 --- a/pkg/server/container_list.go +++ b/pkg/server/container_list.go @@ -54,12 +54,22 @@ func toCRIContainer(container containerstore.Container) *runtime.Container { } } +func (c *criContainerdService) normalizeContainerFilter(filter *runtime.ContainerFilter) { + if cntr, err := c.containerStore.Get(filter.GetId()); err == nil { + filter.Id = cntr.ID + } + if sb, err := c.sandboxStore.Get(filter.GetPodSandboxId()); err == nil { + filter.PodSandboxId = sb.ID + } +} + // filterCRIContainers filters CRIContainers. func (c *criContainerdService) filterCRIContainers(containers []*runtime.Container, filter *runtime.ContainerFilter) []*runtime.Container { if filter == nil { return containers } + c.normalizeContainerFilter(filter) filtered := []*runtime.Container{} for _, cntr := range containers { if filter.GetId() != "" && filter.GetId() != cntr.Id { diff --git a/pkg/server/container_stats_list.go b/pkg/server/container_stats_list.go index e4fa86f7c..10a5ece90 100644 --- a/pkg/server/container_stats_list.go +++ b/pkg/server/container_stats_list.go @@ -119,6 +119,15 @@ func (c *criContainerdService) getContainerMetrics( return &cs, nil } +func (c *criContainerdService) normalizeContainerStatsFilter(filter *runtime.ContainerStatsFilter) { + if cntr, err := c.containerStore.Get(filter.GetId()); err == nil { + filter.Id = cntr.ID + } + if sb, err := c.sandboxStore.Get(filter.GetPodSandboxId()); err == nil { + filter.PodSandboxId = sb.ID + } +} + // buildTaskMetricsRequest constructs a tasks.MetricsRequest based on // the information in the stats request and the containerStore func (c *criContainerdService) buildTaskMetricsRequest( @@ -128,6 +137,7 @@ func (c *criContainerdService) buildTaskMetricsRequest( if r.GetFilter() == nil { return req, nil, nil } + c.normalizeContainerStatsFilter(r.GetFilter()) var containers []containerstore.Container for _, cntr := range c.containerStore.List() { if r.GetFilter().GetId() != "" && cntr.ID != r.GetFilter().GetId() { diff --git a/pkg/server/sandbox_list.go b/pkg/server/sandbox_list.go index aca59e2de..c83a40086 100644 --- a/pkg/server/sandbox_list.go +++ b/pkg/server/sandbox_list.go @@ -84,12 +84,19 @@ func toCRISandbox(meta sandboxstore.Metadata, state runtime.PodSandboxState, cre } } +func (c *criContainerdService) normalizePodSandboxFilter(filter *runtime.PodSandboxFilter) { + if sb, err := c.sandboxStore.Get(filter.GetId()); err == nil { + filter.Id = sb.ID + } +} + // filterCRISandboxes filters CRISandboxes. func (c *criContainerdService) filterCRISandboxes(sandboxes []*runtime.PodSandbox, filter *runtime.PodSandboxFilter) []*runtime.PodSandbox { if filter == nil { return sandboxes } + c.normalizePodSandboxFilter(filter) filtered := []*runtime.PodSandbox{} for _, s := range sandboxes { // Filter by id From 12bbbc0edce9d454d751717f3e6129ebaba92e7c Mon Sep 17 00:00:00 2001 From: yanxuean Date: Tue, 7 Nov 2017 14:04:02 +0800 Subject: [PATCH 2/2] add unit test for listcontainer and listpodsandbox Signed-off-by: yanxuean --- integration/container_stats_test.go | 4 +- pkg/server/container_list_test.go | 129 ++++++++++++++++++++++------ pkg/server/sandbox_list_test.go | 82 +++++++++++++++--- 3 files changed, 173 insertions(+), 42 deletions(-) diff --git a/integration/container_stats_test.go b/integration/container_stats_test.go index cc1aee0ce..b486db338 100644 --- a/integration/container_stats_test.go +++ b/integration/container_stats_test.go @@ -283,7 +283,7 @@ func TestContainerListStatsWithIdSandboxIdFilter(t *testing.T) { return false, err } if len(stats) != 1 { - return false, fmt.Errorf("Unexpected stats length") + return false, fmt.Errorf("unexpected stats length") } if stats[0].GetWritableLayer().GetUsedBytes().GetValue() != 0 && stats[0].GetWritableLayer().GetInodesUsed().GetValue() != 0 { @@ -306,7 +306,7 @@ func TestContainerListStatsWithIdSandboxIdFilter(t *testing.T) { return false, err } if len(stats) != 1 { - return false, fmt.Errorf("Unexpected stats length") + return false, fmt.Errorf("unexpected stats length") } if stats[0].GetWritableLayer().GetUsedBytes().GetValue() != 0 && stats[0].GetWritableLayer().GetInodesUsed().GetValue() != 0 { diff --git a/pkg/server/container_list_test.go b/pkg/server/container_list_test.go index 69c0f8142..fbd188543 100644 --- a/pkg/server/container_list_test.go +++ b/pkg/server/container_list_test.go @@ -26,6 +26,7 @@ import ( "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" containerstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/container" + sandboxstore "github.com/kubernetes-incubator/cri-containerd/pkg/store/sandbox" ) func TestToCRIContainer(t *testing.T) { @@ -168,25 +169,40 @@ func (c containerForTest) toContainer() (containerstore.Container, error) { func TestListContainers(t *testing.T) { c := newTestCRIContainerdService() - + sandboxesInStore := []sandboxstore.Sandbox{ + { + Metadata: sandboxstore.Metadata{ + ID: "s-1abcdef1234", + Name: "sandboxname-1", + Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "podname-1"}}, + }, + }, + { + Metadata: sandboxstore.Metadata{ + ID: "s-2abcdef1234", + Name: "sandboxname-2", + Config: &runtime.PodSandboxConfig{Metadata: &runtime.PodSandboxMetadata{Name: "podname-2"}}, + }, + }, + } createdAt := time.Now().UnixNano() startedAt := time.Now().UnixNano() finishedAt := time.Now().UnixNano() containersInStore := []containerForTest{ { metadata: containerstore.Metadata{ - ID: "1", + ID: "c-1container", Name: "name-1", - SandboxID: "s-1", + SandboxID: "s-1abcdef1234", Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-1"}}, }, status: containerstore.Status{CreatedAt: createdAt}, }, { metadata: containerstore.Metadata{ - ID: "2", + ID: "c-2container", Name: "name-2", - SandboxID: "s-1", + SandboxID: "s-1abcdef1234", Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-2"}}, }, status: containerstore.Status{ @@ -196,9 +212,9 @@ func TestListContainers(t *testing.T) { }, { metadata: containerstore.Metadata{ - ID: "3", + ID: "c-3container", Name: "name-3", - SandboxID: "s-1", + SandboxID: "s-1abcdef1234", Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-3"}}, }, status: containerstore.Status{ @@ -209,9 +225,9 @@ func TestListContainers(t *testing.T) { }, { metadata: containerstore.Metadata{ - ID: "4", + ID: "c-4container", Name: "name-4", - SandboxID: "s-2", + SandboxID: "s-2abcdef1234", Config: &runtime.ContainerConfig{Metadata: &runtime.ContainerMetadata{Name: "name-4"}}, }, status: containerstore.Status{ @@ -219,46 +235,105 @@ func TestListContainers(t *testing.T) { }, }, } - filter := &runtime.ContainerFilter{ - PodSandboxId: "s-1", - } - expect := []*runtime.Container{ + + expectedContainers := []*runtime.Container{ { - Id: "1", - PodSandboxId: "s-1", + Id: "c-1container", + PodSandboxId: "s-1abcdef1234", Metadata: &runtime.ContainerMetadata{Name: "name-1"}, State: runtime.ContainerState_CONTAINER_CREATED, CreatedAt: createdAt, }, { - Id: "2", - PodSandboxId: "s-1", + Id: "c-2container", + PodSandboxId: "s-1abcdef1234", Metadata: &runtime.ContainerMetadata{Name: "name-2"}, State: runtime.ContainerState_CONTAINER_RUNNING, CreatedAt: createdAt, }, { - Id: "3", - PodSandboxId: "s-1", + Id: "c-3container", + PodSandboxId: "s-1abcdef1234", Metadata: &runtime.ContainerMetadata{Name: "name-3"}, State: runtime.ContainerState_CONTAINER_EXITED, CreatedAt: createdAt, }, + { + Id: "c-4container", + PodSandboxId: "s-2abcdef1234", + Metadata: &runtime.ContainerMetadata{Name: "name-4"}, + State: runtime.ContainerState_CONTAINER_CREATED, + CreatedAt: createdAt, + }, } - // Inject test metadata + // Inject test sandbox metadata + for _, sb := range sandboxesInStore { + assert.NoError(t, c.sandboxStore.Add(sb)) + } + + // Inject test container metadata for _, cntr := range containersInStore { container, err := cntr.toContainer() assert.NoError(t, err) assert.NoError(t, c.containerStore.Add(container)) } - resp, err := c.ListContainers(context.Background(), &runtime.ListContainersRequest{Filter: filter}) - assert.NoError(t, err) - require.NotNil(t, resp) - containers := resp.GetContainers() - assert.Len(t, containers, len(expect)) - for _, cntr := range expect { - assert.Contains(t, containers, cntr) + for testdesc, testdata := range map[string]struct { + filter *runtime.ContainerFilter + expect []*runtime.Container + }{ + "test without filter": { + filter: &runtime.ContainerFilter{}, + expect: expectedContainers, + }, + "test filter by sandboxid": { + filter: &runtime.ContainerFilter{ + PodSandboxId: "s-1abcdef1234", + }, + expect: expectedContainers[:3], + }, + "test filter by truncated sandboxid": { + filter: &runtime.ContainerFilter{ + PodSandboxId: "s-1", + }, + expect: expectedContainers[:3], + }, + "test filter by containerid": { + filter: &runtime.ContainerFilter{ + Id: "c-1container", + }, + expect: expectedContainers[:1], + }, + "test filter by truncated containerid": { + filter: &runtime.ContainerFilter{ + Id: "c-1", + }, + expect: expectedContainers[:1], + }, + "test filter by containerid and sandboxid": { + filter: &runtime.ContainerFilter{ + Id: "c-1container", + PodSandboxId: "s-1abcdef1234", + }, + expect: expectedContainers[:1], + }, + "test filter by truncated containerid and truncated sandboxid": { + filter: &runtime.ContainerFilter{ + Id: "c-1", + PodSandboxId: "s-1", + }, + expect: expectedContainers[:1], + }, + } { + t.Logf("TestCase: %s", testdesc) + resp, err := c.ListContainers(context.Background(), &runtime.ListContainersRequest{Filter: testdata.filter}) + assert.NoError(t, err) + require.NotNil(t, resp) + containers := resp.GetContainers() + assert.Len(t, containers, len(testdata.expect)) + for _, cntr := range testdata.expect { + assert.Contains(t, containers, cntr) + } } } diff --git a/pkg/server/sandbox_list_test.go b/pkg/server/sandbox_list_test.go index ca0b4b851..46a60627e 100644 --- a/pkg/server/sandbox_list_test.go +++ b/pkg/server/sandbox_list_test.go @@ -59,26 +59,77 @@ func TestToCRISandbox(t *testing.T) { func TestFilterSandboxes(t *testing.T) { c := newTestCRIContainerdService() - - testSandboxes := []*runtime.PodSandbox{ + sandboxes := []struct { + sandbox sandboxstore.Sandbox + state runtime.PodSandboxState + }{ { - Id: "1", - Metadata: &runtime.PodSandboxMetadata{Name: "name-1", Uid: "uid-1", Namespace: "ns-1", Attempt: 1}, - State: runtime.PodSandboxState_SANDBOX_READY, + sandbox: sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: "1abcdef", + Name: "sandboxname-1", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "podname-1", + Uid: "uid-1", + Namespace: "ns-1", + Attempt: 1, + }, + }, + }, + }, + state: runtime.PodSandboxState_SANDBOX_READY, }, { - Id: "2", - Metadata: &runtime.PodSandboxMetadata{Name: "name-2", Uid: "uid-2", Namespace: "ns-2", Attempt: 2}, - State: runtime.PodSandboxState_SANDBOX_NOTREADY, - Labels: map[string]string{"a": "b"}, + sandbox: sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: "2abcdef", + Name: "sandboxname-2", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "podname-2", + Uid: "uid-2", + Namespace: "ns-2", + Attempt: 2, + }, + Labels: map[string]string{"a": "b"}, + }, + }, + }, + state: runtime.PodSandboxState_SANDBOX_NOTREADY, }, { - Id: "3", - Metadata: &runtime.PodSandboxMetadata{Name: "name-2", Uid: "uid-2", Namespace: "ns-2", Attempt: 2}, - State: runtime.PodSandboxState_SANDBOX_READY, - Labels: map[string]string{"c": "d"}, + sandbox: sandboxstore.Sandbox{ + Metadata: sandboxstore.Metadata{ + ID: "3abcdef", + Name: "sandboxname-3", + Config: &runtime.PodSandboxConfig{ + Metadata: &runtime.PodSandboxMetadata{ + Name: "podname-2", + Uid: "uid-2", + Namespace: "ns-2", + Attempt: 2, + }, + Labels: map[string]string{"c": "d"}, + }, + }, + }, + state: runtime.PodSandboxState_SANDBOX_READY, }, } + + // Create PodSandbox + testSandboxes := []*runtime.PodSandbox{} + createdAt := time.Now() + for _, sb := range sandboxes { + testSandboxes = append(testSandboxes, toCRISandbox(sb.sandbox.Metadata, sb.state, createdAt)) + } + + // Inject test sandbox metadata + for _, sb := range sandboxes { + assert.NoError(t, c.sandboxStore.Add(sb.sandbox)) + } + for desc, test := range map[string]struct { filter *runtime.PodSandboxFilter expect []*runtime.PodSandbox @@ -87,6 +138,10 @@ func TestFilterSandboxes(t *testing.T) { expect: testSandboxes, }, "id filter": { + filter: &runtime.PodSandboxFilter{Id: "2abcdef"}, + expect: []*runtime.PodSandbox{testSandboxes[1]}, + }, + "truncid filter": { filter: &runtime.PodSandboxFilter{Id: "2"}, expect: []*runtime.PodSandbox{testSandboxes[1]}, }, @@ -121,6 +176,7 @@ func TestFilterSandboxes(t *testing.T) { expect: []*runtime.PodSandbox{testSandboxes[2]}, }, } { + t.Logf("TestCase: %s", desc) filtered := c.filterCRISandboxes(testSandboxes, test.filter) assert.Equal(t, test.expect, filtered, desc) }