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

@@ -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 {
+	for _, test := range testCases {
-		if actual := toRuntimeAPIContainerState(test.input); actual != test.expected {
+		actual := toRuntimeAPIContainerState(test.input)
-			t.Errorf("Test[%d]: expected %q, got %q", i, test.expected, actual)
+		assert.Equal(t, test.expected, actual)
-		}
 	}
 }

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() {

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() {

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) {
+// A helper to create a basic config.
-	ds, fakeDocker := newTestDockerSevice()
+func makeSandboxConfig(name, namespace, uid string, attempt uint32) *runtimeApi.PodSandboxConfig {
-	name := "FOO"
+	return &runtimeApi.PodSandboxConfig{
-	namespace := "BAR"
-	uid := "1"
-	config := &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 {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error: %v", err)
+	assert.NoError(t, fakeDocker.AssertStarted([]string{id}))
-	}
-	if err := fakeDocker.AssertStarted([]string{id}); err != nil {
-		t.Errorf("%v", err)
-	}
 
 	// 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 {
+	assert.NoError(t, err)
-		t.Errorf("Unexpected error: %v", err)
+	assert.Len(t, containers, 1)
-	}
+	assert.Equal(t, id, containers[0].ID)
-	if len(containers) != 1 {
+}
-		t.Errorf("More than one running containers: %+v", containers)
+
-	}
+// TestListSandboxes creates several sandboxes and then list them to check
-	if containers[0].ID != id {
+// whether the correct metadatas, states, and labels are returned.
-		t.Errorf("Expected id %q, got %v", id, containers[0].ID)
+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)
 }

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)