Generate valid container id in fake docker client.

pkg/kubelet/dockershim/docker_container_test.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 	containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
@@ -242,23 +243,20 @@ func TestContainerCreationConflict(t *testing.T) {
 			expectFields: 6,
 		},
 		"random create error": {
-			createError:  randomError,
+			createError: randomError,
-			expectError:  randomError,
+			expectError: randomError,
-			expectCalls:  []string{"create"},
+			expectCalls: []string{"create"},
-			expectFields: 1,
 		},
 		"conflict create error with successful remove": {
-			createError:  conflictError,
+			createError: conflictError,
-			expectError:  conflictError,
+			expectError: conflictError,
-			expectCalls:  []string{"create", "remove"},
+			expectCalls: []string{"create", "remove"},
-			expectFields: 1,
 		},
 		"conflict create error with random remove error": {
-			createError:  conflictError,
+			createError: conflictError,
-			removeError:  randomError,
+			removeError: randomError,
-			expectError:  conflictError,
+			expectError: conflictError,
-			expectCalls:  []string{"create", "remove"},
+			expectCalls: []string{"create", "remove"},
-			expectFields: 1,
 		},
 		"conflict create error with no such container remove error": {
 			createError:  conflictError,
@@ -276,9 +274,13 @@ func TestContainerCreationConflict(t *testing.T) {
 		if test.removeError != nil {
 			fDocker.InjectError("remove", test.removeError)
 		}
-		name, err := ds.CreateContainer(sandboxId, config, sConfig)
+		id, err := ds.CreateContainer(sandboxId, config, sConfig)
-		assert.Equal(t, test.expectError, err)
+		require.Equal(t, test.expectError, err)
 		assert.NoError(t, fDocker.AssertCalls(test.expectCalls))
-		assert.Len(t, strings.Split(name, nameDelimiter), test.expectFields)
+		if err == nil {
+			c, err := fDocker.InspectContainer(id)
+			assert.NoError(t, err)
+			assert.Len(t, strings.Split(c.Name, nameDelimiter), test.expectFields)
+		}
 	}
 }

pkg/kubelet/dockershim/docker_sandbox_test.go

@@ -26,6 +26,7 @@ import (
 
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
+	"k8s.io/kubernetes/pkg/kubelet/dockertools"
 	"k8s.io/kubernetes/pkg/kubelet/network"
 	"k8s.io/kubernetes/pkg/kubelet/types"
 )
@@ -157,7 +158,7 @@ func TestNetworkPluginInvocation(t *testing.T) {
 		map[string]string{"label": name},
 		map[string]string{"annotation": ns},
 	)
-	cID := kubecontainer.ContainerID{Type: runtimeName, ID: fmt.Sprintf("/%v", makeSandboxName(c))}
+	cID := kubecontainer.ContainerID{Type: runtimeName, ID: dockertools.GetFakeContainerID(fmt.Sprintf("/%v", makeSandboxName(c)))}
 
 	mockPlugin.EXPECT().Name().Return("mockNetworkPlugin").AnyTimes()
 	setup := mockPlugin.EXPECT().SetUpPod(ns, name, cID)
@@ -195,7 +196,7 @@ func TestHostNetworkPluginInvocation(t *testing.T) {
 			},
 		},
 	}
-	cID := kubecontainer.ContainerID{Type: runtimeName, ID: fmt.Sprintf("/%v", makeSandboxName(c))}
+	cID := kubecontainer.ContainerID{Type: runtimeName, ID: dockertools.GetFakeContainerID(fmt.Sprintf("/%v", makeSandboxName(c)))}
 
 	// No calls to network plugin are expected
 	_, err := ds.RunPodSandbox(c)

pkg/kubelet/dockertools/docker_manager_linux_test.go

@@ -118,13 +118,7 @@ func TestSeccompIsUnconfinedByDefaultWithDockerV110(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar"}))
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 	if err != nil {
@@ -157,13 +151,7 @@ func TestUnconfinedSeccompProfileWithDockerV110(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar4"}))
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo4_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar4\\.[a-f0-9]+_foo4_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 	if err != nil {
@@ -192,13 +180,7 @@ func TestDefaultSeccompProfileWithDockerV110(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar1"}))
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo1_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar1\\.[a-f0-9]+_foo1_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 	if err != nil {
@@ -228,13 +210,7 @@ func TestSeccompContainerAnnotationTrumpsPod(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar2"}))
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo2_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar2\\.[a-f0-9]+_foo2_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 	if err != nil {
@@ -260,13 +236,7 @@ func TestSecurityOptsAreNilWithDockerV19(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar"}))
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 	if err != nil {
@@ -306,13 +276,7 @@ func TestCreateAppArmorContanier(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "test"}))
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_test\\.[a-f0-9]+_foo_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 
 	// Verify security opts.
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
@@ -384,13 +348,7 @@ func TestSeccompLocalhostProfileIsLoaded(t *testing.T) {
 			"create", "start", "inspect_container",
 		})
 
-		fakeDocker.Lock()
+		assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar2"}))
-		if len(fakeDocker.Created) != 2 ||
-			!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo2_new_", fakeDocker.Created[0]) ||
-			!matchString(t, "/k8s_bar2\\.[a-f0-9]+_foo2_new_", fakeDocker.Created[1]) {
-			t.Errorf("unexpected containers created %v", fakeDocker.Created)
-		}
-		fakeDocker.Unlock()
 
 		newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 		if err != nil {

pkg/kubelet/dockertools/docker_manager_test.go

@@ -639,13 +639,9 @@ func TestSyncPodCreateNetAndContainer(t *testing.T) {
 	if !found {
 		t.Errorf("Custom pod infra container not found: %v", fakeDocker.RunningContainerList)
 	}
-
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
 	fakeDocker.Unlock()
+
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar"}))
 }
 
 func TestSyncPodCreatesNetAndContainerPullsImage(t *testing.T) {
@@ -670,17 +666,12 @@ func TestSyncPodCreatesNetAndContainerPullsImage(t *testing.T) {
 	})
 
 	fakeDocker.Lock()
-
 	if !reflect.DeepEqual(puller.ImagesPulled, []string{"foo/infra_image:v1", "foo/something:v0"}) {
 		t.Errorf("unexpected pulled containers: %v", puller.ImagesPulled)
 	}
-
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
 	fakeDocker.Unlock()
+
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar"}))
 }
 
 func TestSyncPodWithPodInfraCreatesContainer(t *testing.T) {
@@ -703,12 +694,7 @@ func TestSyncPodWithPodInfraCreatesContainer(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByName([]string{"bar"}))
-	if len(fakeDocker.Created) != 1 ||
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 }
 
 func TestSyncPodDeletesWithNoPodInfraContainer(t *testing.T) {
@@ -734,16 +720,7 @@ func TestSyncPodDeletesWithNoPodInfraContainer(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	// A map iteration is used to delete containers, so must not depend on
+	assert.NoError(t, fakeDocker.AssertStopped([]string{"1234"}))
-	// order here.
-	expectedToStop := map[string]bool{
-		"1234": true,
-	}
-	fakeDocker.Lock()
-	if len(fakeDocker.Stopped) != 1 || !expectedToStop[fakeDocker.Stopped[0]] {
-		t.Errorf("Wrong containers were stopped: %v", fakeDocker.Stopped)
-	}
-	fakeDocker.Unlock()
 }
 
 func TestSyncPodDeletesDuplicate(t *testing.T) {
@@ -942,7 +919,7 @@ func TestSyncPodWithRestartPolicy(t *testing.T) {
 		// 'stop' is because the pod infra container is killed when no container is running.
 		verifyCalls(t, fakeDocker, tt.calls)
 
-		if err := fakeDocker.AssertCreated(tt.created); err != nil {
+		if err := fakeDocker.AssertCreatedByName(tt.created); err != nil {
 			t.Errorf("case [%d]: %v", i, err)
 		}
 		if err := fakeDocker.AssertStopped(tt.stopped); err != nil {
@@ -1180,12 +1157,8 @@ func TestSyncPodWithPodInfraCreatesContainerCallsHandler(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByName([]string{"bar"}))
-	if len(fakeDocker.Created) != 1 ||
+
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 	if fakeHTTPClient.url != "http://foo:8080/bar" {
 		t.Errorf("unexpected handler: %q", fakeHTTPClient.url)
 	}
@@ -1225,16 +1198,7 @@ func TestSyncPodEventHandlerFails(t *testing.T) {
 		"stop",
 	})
 
-	if len(fakeDocker.Stopped) != 1 {
+	assert.NoError(t, fakeDocker.AssertStoppedByName([]string{"bar"}))
-		t.Fatalf("Wrong containers were stopped: %v", fakeDocker.Stopped)
-	}
-	dockerName, _, err := ParseDockerName(fakeDocker.Stopped[0])
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if dockerName.ContainerName != "bar" {
-		t.Errorf("Wrong stopped container, expected: bar, get: %q", dockerName.ContainerName)
-	}
 }
 
 type fakeReadWriteCloser struct{}
@@ -1289,13 +1253,8 @@ func TestSyncPodWithTerminationLog(t *testing.T) {
 
 	defer os.Remove(testPodContainerDir)
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar"}))
-	if len(fakeDocker.Created) != 2 ||
+
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 	if err != nil {
 		t.Fatalf("unexpected error %v", err)
@@ -1327,13 +1286,7 @@ func TestSyncPodWithHostNetwork(t *testing.T) {
 		"create", "start", "inspect_container",
 	})
 
-	fakeDocker.Lock()
+	assert.NoError(t, fakeDocker.AssertCreatedByNameWithOrder([]string{"POD", "bar"}))
-	if len(fakeDocker.Created) != 2 ||
-		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
-		!matchString(t, "/k8s_bar\\.[a-f0-9]+_foo_new_", fakeDocker.Created[1]) {
-		t.Errorf("unexpected containers created %v", fakeDocker.Created)
-	}
-	fakeDocker.Unlock()
 
 	newContainer, err := fakeDocker.InspectContainer(fakeDocker.Created[1])
 	if err != nil {

pkg/kubelet/dockertools/fake_docker_client.go

@@ -19,18 +19,20 @@ package dockertools
 import (
 	"encoding/json"
 	"fmt"
+	"hash/fnv"
 	"math/rand"
 	"os"
 	"reflect"
 	"sort"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
 
 	dockertypes "github.com/docker/engine-api/types"
 	dockercontainer "github.com/docker/engine-api/types/container"
-	"k8s.io/client-go/util/clock"
 
+	"k8s.io/client-go/util/clock"
 	"k8s.io/kubernetes/pkg/api/v1"
 )
 
@@ -273,17 +275,39 @@ func (f *FakeDockerClient) AssertCallDetails(calls ...calledDetail) (err error)
 	return
 }
 
-func (f *FakeDockerClient) AssertCreated(created []string) error {
+// idsToNames converts container ids into names. The caller must hold the lock.
+func (f *FakeDockerClient) idsToNames(ids []string) ([]string, error) {
+	names := []string{}
+	for _, id := range ids {
+		dockerName, _, err := ParseDockerName(f.ContainerMap[id].Name)
+		if err != nil {
+			return nil, fmt.Errorf("unexpected error: %v", err)
+		}
+		names = append(names, dockerName.ContainerName)
+	}
+	return names, nil
+}
+
+func (f *FakeDockerClient) AssertCreatedByNameWithOrder(created []string) error {
+	f.Lock()
+	defer f.Unlock()
+	actualCreated, err := f.idsToNames(f.Created)
+	if err != nil {
+		return err
+	}
+	if !reflect.DeepEqual(created, actualCreated) {
+		return fmt.Errorf("expected %#v, got %#v", created, actualCreated)
+	}
+	return nil
+}
+
+func (f *FakeDockerClient) AssertCreatedByName(created []string) error {
 	f.Lock()
 	defer f.Unlock()
 
-	actualCreated := []string{}
+	actualCreated, err := f.idsToNames(f.Created)
-	for _, c := range f.Created {
+	if err != nil {
-		dockerName, _, err := ParseDockerName(c)
+		return err
-		if err != nil {
-			return fmt.Errorf("unexpected error: %v", err)
-		}
-		actualCreated = append(actualCreated, dockerName.ContainerName)
 	}
 	sort.StringSlice(created).Sort()
 	sort.StringSlice(actualCreated).Sort()
@@ -293,13 +317,18 @@ func (f *FakeDockerClient) AssertCreated(created []string) error {
 	return nil
 }
 
-func (f *FakeDockerClient) AssertStarted(started []string) error {
+func (f *FakeDockerClient) AssertStoppedByName(stopped []string) error {
 	f.Lock()
 	defer f.Unlock()
-	sort.StringSlice(started).Sort()
+
-	sort.StringSlice(f.Started).Sort()
+	actualStopped, err := f.idsToNames(f.Stopped)
-	if !reflect.DeepEqual(started, f.Started) {
+	if err != nil {
-		return fmt.Errorf("expected %#v, got %#v", started, f.Started)
+		return err
+	}
+	sort.StringSlice(stopped).Sort()
+	sort.StringSlice(actualStopped).Sort()
+	if !reflect.DeepEqual(stopped, actualStopped) {
+		return fmt.Errorf("expected %#v, got %#v", stopped, actualStopped)
 	}
 	return nil
 }
@@ -307,21 +336,12 @@ func (f *FakeDockerClient) AssertStarted(started []string) error {
 func (f *FakeDockerClient) AssertStopped(stopped []string) error {
 	f.Lock()
 	defer f.Unlock()
+	// Copy stopped to avoid modifying it.
+	actualStopped := append([]string{}, f.Stopped...)
 	sort.StringSlice(stopped).Sort()
-	sort.StringSlice(f.Stopped).Sort()
+	sort.StringSlice(actualStopped).Sort()
-	if !reflect.DeepEqual(stopped, f.Stopped) {
+	if !reflect.DeepEqual(stopped, actualStopped) {
-		return fmt.Errorf("expected %#v, got %#v", stopped, f.Stopped)
+		return fmt.Errorf("expected %#v, got %#v", stopped, actualStopped)
-	}
-	return nil
-}
-
-func (f *FakeDockerClient) AssertRemoved(removed []string) error {
-	f.Lock()
-	defer f.Unlock()
-	sort.StringSlice(removed).Sort()
-	sort.StringSlice(f.Removed).Sort()
-	if !reflect.DeepEqual(removed, f.Removed) {
-		return fmt.Errorf("expected %#v, got %#v", removed, f.Removed)
 	}
 	return nil
 }
@@ -430,6 +450,13 @@ func (f *FakeDockerClient) normalSleep(mean, stdDev, cutOffMillis int) {
 	time.Sleep(delay)
 }
 
+// GetFakeContainerID generates a fake container id from container name with a hash.
+func GetFakeContainerID(name string) string {
+	hash := fnv.New64a()
+	hash.Write([]byte(name))
+	return strconv.FormatUint(hash.Sum64(), 16)
+}
+
 // CreateContainer is a test-spy implementation of DockerInterface.CreateContainer.
 // It adds an entry "create" to the internal method call record.
 func (f *FakeDockerClient) CreateContainer(c dockertypes.ContainerCreateConfig) (*dockertypes.ContainerCreateResponse, error) {
@@ -442,13 +469,13 @@ func (f *FakeDockerClient) CreateContainer(c dockertypes.ContainerCreateConfig)
 	// This is not a very good fake. We'll just add this container's name to the list.
 	// Docker likes to add a '/', so copy that behavior.
 	name := "/" + c.Name
-	id := name
+	id := GetFakeContainerID(name)
-	f.appendContainerTrace("Created", name)
+	f.appendContainerTrace("Created", id)
 	// The newest container should be in front, because we assume so in GetPodStatus()
 	f.RunningContainerList = append([]dockertypes.Container{
-		{ID: name, Names: []string{name}, Image: c.Config.Image, Labels: c.Config.Labels},
+		{ID: id, Names: []string{name}, Image: c.Config.Image, Labels: c.Config.Labels},
 	}, f.RunningContainerList...)
-	f.ContainerMap[name] = convertFakeContainer(&FakeContainer{
+	f.ContainerMap[id] = convertFakeContainer(&FakeContainer{
 		ID: id, Name: name, Config: c.Config, HostConfig: c.HostConfig, CreatedAt: f.Clock.Now()})
 	f.normalSleep(100, 25, 25)
 	return &dockertypes.ContainerCreateResponse{ID: id}, nil