From a1833d19473673bf9d569d709a3d0bd490d36e90 Mon Sep 17 00:00:00 2001 From: Yu-Ju Hong Date: Fri, 26 Aug 2016 16:15:06 -0700 Subject: [PATCH] dockershim: bug fixes and more unit tests Fixing the name triming and other small bugs. Added sandbox listing unit tests. --- pkg/kubelet/dockershim/convert_test.go | 9 ++- pkg/kubelet/dockershim/docker_container.go | 3 +- pkg/kubelet/dockershim/docker_sandbox.go | 3 +- pkg/kubelet/dockershim/docker_sandbox_test.go | 73 +++++++++++++------ pkg/kubelet/dockershim/helpers.go | 3 + 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/pkg/kubelet/dockershim/convert_test.go b/pkg/kubelet/dockershim/convert_test.go index 57f614bec0b..c6e428ba235 100644 --- a/pkg/kubelet/dockershim/convert_test.go +++ b/pkg/kubelet/dockershim/convert_test.go @@ -19,6 +19,8 @@ package dockershim import ( "testing" + "github.com/stretchr/testify/assert" + runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" ) @@ -33,9 +35,8 @@ func TestConvertDockerStatusToRuntimeAPIState(t *testing.T) { {input: "Random string", expected: runtimeApi.ContainerState_UNKNOWN}, } - for i, test := range testCases { - if actual := toRuntimeAPIContainerState(test.input); actual != test.expected { - t.Errorf("Test[%d]: expected %q, got %q", i, test.expected, actual) - } + for _, test := range testCases { + actual := toRuntimeAPIContainerState(test.input) + assert.Equal(t, test.expected, actual) } } diff --git a/pkg/kubelet/dockershim/docker_container.go b/pkg/kubelet/dockershim/docker_container.go index a9f9d5526ff..075785e0a1e 100644 --- a/pkg/kubelet/dockershim/docker_container.go +++ b/pkg/kubelet/dockershim/docker_container.go @@ -62,7 +62,8 @@ func (ds *dockerService) ListContainers(filter *runtimeApi.ContainerFilter) ([]* } // Convert docker to runtime api containers. result := []*runtimeApi.Container{} - for _, c := range containers { + for i := range containers { + c := containers[i] if len(filter.GetName()) > 0 { _, _, _, containerName, _, err := parseContainerName(c.Names[0]) if err != nil || containerName != filter.GetName() { diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index 9aad2e693cd..986c3b72d93 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/pkg/kubelet/dockershim/docker_sandbox.go @@ -182,7 +182,8 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeApi.PodSandboxFilter) ([] // Convert docker containers to runtime api sandboxes. result := []*runtimeApi.PodSandbox{} - for _, c := range containers { + for i := range containers { + c := containers[i] if len(filter.GetName()) > 0 { sandboxName, _, _, _, err := parseSandboxName(c.Names[0]) if err != nil || sandboxName != filter.GetName() { diff --git a/pkg/kubelet/dockershim/docker_sandbox_test.go b/pkg/kubelet/dockershim/docker_sandbox_test.go index 82f0c94741e..72df1a38efc 100644 --- a/pkg/kubelet/dockershim/docker_sandbox_test.go +++ b/pkg/kubelet/dockershim/docker_sandbox_test.go @@ -17,43 +17,74 @@ limitations under the License. package dockershim import ( + "fmt" "testing" dockertypes "github.com/docker/engine-api/types" + "github.com/stretchr/testify/assert" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" ) -func TestCreateSandbox(t *testing.T) { - ds, fakeDocker := newTestDockerSevice() - name := "FOO" - namespace := "BAR" - uid := "1" - config := &runtimeApi.PodSandboxConfig{ +// A helper to create a basic config. +func makeSandboxConfig(name, namespace, uid string, attempt uint32) *runtimeApi.PodSandboxConfig { + return &runtimeApi.PodSandboxConfig{ Metadata: &runtimeApi.PodSandboxMetadata{ Name: &name, Namespace: &namespace, Uid: &uid, + Attempt: &attempt, }, } +} + +// TestRunSandbox tests that RunSandbox creates and starts a container +// acting a the sandbox for the pod. +func TestRunSandbox(t *testing.T) { + ds, fakeDocker := newTestDockerSevice() + config := makeSandboxConfig("foo", "bar", "1", 0) id, err := ds.RunPodSandbox(config) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if err := fakeDocker.AssertStarted([]string{id}); err != nil { - t.Errorf("%v", err) - } + assert.NoError(t, err) + assert.NoError(t, fakeDocker.AssertStarted([]string{id})) // List running containers and verify that there is one (and only one) // running container that we just created. containers, err := fakeDocker.ListContainers(dockertypes.ContainerListOptions{All: false}) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if len(containers) != 1 { - t.Errorf("More than one running containers: %+v", containers) - } - if containers[0].ID != id { - t.Errorf("Expected id %q, got %v", id, containers[0].ID) - } + assert.NoError(t, err) + assert.Len(t, containers, 1) + assert.Equal(t, id, containers[0].ID) +} + +// TestListSandboxes creates several sandboxes and then list them to check +// whether the correct metadatas, states, and labels are returned. +func TestListSandboxes(t *testing.T) { + ds, _ := newTestDockerSevice() + name, namespace := "foo", "bar" + configs := []*runtimeApi.PodSandboxConfig{} + for i := 0; i < 3; i++ { + c := makeSandboxConfig(fmt.Sprintf("%s%d", name, i), + fmt.Sprintf("%s%d", namespace, i), fmt.Sprintf("%d", i), 0) + configs = append(configs, c) + } + + expected := []*runtimeApi.PodSandbox{} + state := runtimeApi.PodSandBoxState_READY + var createdAt int64 = 0 + for i := range configs { + id, err := ds.RunPodSandbox(configs[i]) + assert.NoError(t, err) + // Prepend to the expected list because ListPodSandbox returns + // the most recent sandbox first. + expected = append([]*runtimeApi.PodSandbox{{ + Metadata: configs[i].Metadata, + Id: &id, + State: &state, + Labels: map[string]string{containerTypeLabelKey: containerTypeLabelSandbox}, + CreatedAt: &createdAt, + }}, expected...) + } + sandboxes, err := ds.ListPodSandbox(nil) + assert.NoError(t, err) + assert.Len(t, sandboxes, len(expected)) + assert.Equal(t, expected, sandboxes) } diff --git a/pkg/kubelet/dockershim/helpers.go b/pkg/kubelet/dockershim/helpers.go index a75e216608e..dd30318de27 100644 --- a/pkg/kubelet/dockershim/helpers.go +++ b/pkg/kubelet/dockershim/helpers.go @@ -206,6 +206,9 @@ func buildContainerName(sandboxConfig *runtimeApi.PodSandboxConfig, containerCon // parseContainerName unpacks a container name, returning the pod name, namespace, UID, // container name and attempt. func parseContainerName(name string) (podName, podNamespace, podUID, containerName string, attempt uint32, err error) { + // Docker adds a "/" prefix to names. so trim it. + name = strings.TrimPrefix(name, "/") + parts := strings.Split(name, "_") if len(parts) == 0 || parts[0] != kubePrefix { err = fmt.Errorf("failed to parse container name %q into parts", name)