Merge pull request #4433 from vmarmol/purge

Use Docker name (not ID) to parse Kubernetes components.

pkg/kubelet/dockertools/docker.go

@@ -578,6 +578,7 @@ func BuildDockerName(podUID types.UID, podFullName string, container *api.Contai
 		rand.Uint32())
 }
 
+// TODO(vmarmol): This should probably return an error.
 // Unpacks a container name, returning the pod full name and container name we would have used to
 // construct the docker name. If the docker name isn't the one we created, we may return empty strings.
 func ParseDockerName(name string) (podFullName string, podUID types.UID, containerName string, hash uint64) {

pkg/kubelet/kubelet.go

@@ -395,7 +395,7 @@ func (kl *Kubelet) GarbageCollectContainers() error {
 	}
 	uidToIDMap := map[string][]string{}
 	for _, container := range containers {
-		_, uid, name, _ := dockertools.ParseDockerName(container.ID)
+		_, uid, name, _ := dockertools.ParseDockerName(container.Names[0])
 		uidName := string(uid) + "." + name
 		uidToIDMap[uidName] = append(uidToIDMap[uidName], container.ID)
 	}

pkg/kubelet/kubelet_test.go

@@ -95,6 +95,27 @@ func verifyStringArrayEquals(t *testing.T, actual, expected []string) {
 	}
 }
 
+func verifyStringArrayEqualsAnyOrder(t *testing.T, actual, expected []string) {
+	invalid := len(actual) != len(expected)
+	if !invalid {
+		for _, exp := range expected {
+			found := false
+			for _, act := range actual {
+				if exp == act {
+					found = true
+					break
+				}
+			}
+			if !found {
+				t.Errorf("Expected element %s not found in %#v", exp, actual)
+			}
+		}
+	}
+	if invalid {
+		t.Errorf("Expected: %#v, Actual: %#v", expected, actual)
+	}
+}
+
 func verifyBoolean(t *testing.T, expected, value bool) {
 	if expected != value {
 		t.Errorf("Unexpected boolean.  Expected %t.  Found %t", expected, value)
@@ -1634,6 +1655,7 @@ func TestKubeletGarbageCollection(t *testing.T) {
 		containerDetails map[string]*docker.Container
 		expectedRemoved  []string
 	}{
+		// Remove oldest containers.
 		{
 			containers: []docker.APIContainers{
 				{
@@ -1651,21 +1673,6 @@ func TestKubeletGarbageCollection(t *testing.T) {
 					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
 					ID:    "3876",
 				},
-				{
-					// pod infra container
-					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
-					ID:    "4876",
-				},
-				{
-					// pod infra container
-					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
-					ID:    "5876",
-				},
-				{
-					// pod infra container
-					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
-					ID:    "6876",
-				},
 			},
 			containerDetails: map[string]*docker.Container{
 				"1876": {
@@ -1678,6 +1685,7 @@ func TestKubeletGarbageCollection(t *testing.T) {
 			},
 			expectedRemoved: []string{"1876"},
 		},
+		// Only remove non-running containers.
 		{
 			containers: []docker.APIContainers{
 				{
@@ -1700,21 +1708,6 @@ func TestKubeletGarbageCollection(t *testing.T) {
 					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
 					ID:    "4876",
 				},
-				{
-					// pod infra container
-					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
-					ID:    "5876",
-				},
-				{
-					// pod infra container
-					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
-					ID:    "6876",
-				},
-				{
-					// pod infra container
-					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
-					ID:    "7876",
-				},
 			},
 			containerDetails: map[string]*docker.Container{
 				"1876": {
@@ -1734,6 +1727,7 @@ func TestKubeletGarbageCollection(t *testing.T) {
 			},
 			expectedRemoved: []string{"2876"},
 		},
+		// Less than maxContainerCount doesn't delete any.
 		{
 			containers: []docker.APIContainers{
 				{
@@ -1743,10 +1737,62 @@ func TestKubeletGarbageCollection(t *testing.T) {
 				},
 			},
 		},
+		// maxContainerCount applies per container..
+		{
+			containers: []docker.APIContainers{
+				{
+					// pod infra container
+					Names: []string{"/k8s_POD_foo2.new.test_.beefbeef_40"},
+					ID:    "1706",
+				},
+				{
+					// pod infra container
+					Names: []string{"/k8s_POD_foo2.new.test_.beefbeef_40"},
+					ID:    "2706",
+				},
+				{
+					// pod infra container
+					Names: []string{"/k8s_POD_foo2.new.test_.beefbeef_40"},
+					ID:    "3706",
+				},
+				{
+					// pod infra container
+					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
+					ID:    "1876",
+				},
+				{
+					// pod infra container
+					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
+					ID:    "2876",
+				},
+				{
+					// pod infra container
+					Names: []string{"/k8s_POD_foo.new.test_.deadbeef_42"},
+					ID:    "3876",
+				},
+			},
+			containerDetails: map[string]*docker.Container{
+				"1706": {
+					State: docker.State{
+						Running: false,
+					},
+					ID:      "1706",
+					Created: time.Now(),
+				},
+				"1876": {
+					State: docker.State{
+						Running: false,
+					},
+					ID:      "1876",
+					Created: time.Now(),
+				},
+			},
+			expectedRemoved: []string{"1706", "1876"},
+		},
 	}
 	for _, test := range tests {
 		kubelet, fakeDocker := newTestKubelet(t)
-		kubelet.maxContainerCount = 5
+		kubelet.maxContainerCount = 2
 		fakeDocker.ContainerList = test.containers
 		fakeDocker.ContainerMap = test.containerDetails
 		fakeDocker.Container = &docker.Container{ID: "error", Created: time.Now()}
@@ -1754,9 +1800,7 @@ func TestKubeletGarbageCollection(t *testing.T) {
 		if err != nil {
 			t.Errorf("unexpected error: %v", err)
 		}
-		if !reflect.DeepEqual(fakeDocker.Removed, test.expectedRemoved) {
+		verifyStringArrayEqualsAnyOrder(t, test.expectedRemoved, fakeDocker.Removed)
-			t.Errorf("expected: %v, got: %v", test.expectedRemoved, fakeDocker.Removed)
-		}
 	}
 }