rkt: Fix GetPods(), refactor tests for GetPods().

Fix GetPods() so that the container hash is fetched from the
annotations in pod manifest's app list instead of image manifest.

pkg/kubelet/rkt/rkt.go

@@ -845,35 +845,22 @@ func (r *Runtime) convertRktPod(rktpod rktapi.Pod) (*kubecontainer.Pod, error) {
 		return nil, fmt.Errorf("couldn't parse pod creation timestamp: %v", err)
 	}
 
-	var state kubecontainer.ContainerState
-	switch rktpod.State {
-	case rktapi.PodState_POD_STATE_RUNNING:
-		state = kubecontainer.ContainerStateRunning
-	case rktapi.PodState_POD_STATE_ABORTED_PREPARE, rktapi.PodState_POD_STATE_EXITED,
-		rktapi.PodState_POD_STATE_DELETING, rktapi.PodState_POD_STATE_GARBAGE:
-		state = kubecontainer.ContainerStateExited
-	default:
-		state = kubecontainer.ContainerStateUnknown
-	}
-
 	kubepod := &kubecontainer.Pod{
 		ID:        types.UID(podUID),
 		Name:      podName,
 		Namespace: podNamespace,
 	}
-	for _, app := range rktpod.Apps {
+
-		manifest := &appcschema.ImageManifest{}
+	for i, app := range rktpod.Apps {
-		err := json.Unmarshal(app.Image.Manifest, manifest)
+		// The order of the apps is determined by the rkt pod manifest.
-		if err != nil {
+		// TODO(yifan): Let the server to unmarshal the annotations? https://github.com/coreos/rkt/issues/1872
-			return nil, err
+		hashStr, ok := manifest.Apps[i].Annotations.Get(k8sRktContainerHashAnno)
-		}
-		containerHashString, ok := manifest.Annotations.Get(k8sRktContainerHashAnno)
 		if !ok {
-			return nil, fmt.Errorf("app is missing annotation %s", k8sRktContainerHashAnno)
+			return nil, fmt.Errorf("app %q is missing annotation %s", app.Name, k8sRktContainerHashAnno)
 		}
-		containerHash, err := strconv.ParseUint(containerHashString, 10, 64)
+		containerHash, err := strconv.ParseUint(hashStr, 10, 64)
 		if err != nil {
-			return nil, fmt.Errorf("couldn't parse container's hash: %v", err)
+			return nil, fmt.Errorf("couldn't parse container's hash %q: %v", hashStr, err)
 		}
 
 		kubepod.Containers = append(kubepod.Containers, &kubecontainer.Container{
@@ -882,7 +869,7 @@ func (r *Runtime) convertRktPod(rktpod rktapi.Pod) (*kubecontainer.Pod, error) {
 			Image:   app.Image.Name,
 			Hash:    containerHash,
 			Created: podCreated,
-			State:   state,
+			State:   appStateToContainerState(app.State),
 		})
 	}
 

pkg/kubelet/rkt/rkt_test.go

@@ -27,9 +27,121 @@ import (
 	rktapi "github.com/coreos/rkt/api/v1alpha"
 	"github.com/stretchr/testify/assert"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
-	"k8s.io/kubernetes/pkg/types"
 )
 
+func mustMarshalPodManifest(man *appcschema.PodManifest) []byte {
+	manblob, err := json.Marshal(man)
+	if err != nil {
+		panic(err)
+	}
+	return manblob
+}
+
+func mustMarshalImageManifest(man *appcschema.ImageManifest) []byte {
+	manblob, err := json.Marshal(man)
+	if err != nil {
+		panic(err)
+	}
+	return manblob
+}
+
+func mustRktHash(hash string) *appctypes.Hash {
+	h, err := appctypes.NewHash(hash)
+	if err != nil {
+		panic(err)
+	}
+	return h
+}
+
+func makeRktPod(rktPodState rktapi.PodState,
+	rktPodID, podUID, podName, podNamespace,
+	podIP, podCreationTs, podRestartCount string,
+	appNames, imgIDs, imgNames, containerHashes []string,
+	appStates []rktapi.AppState) *rktapi.Pod {
+
+	podManifest := &appcschema.PodManifest{
+		ACKind:    appcschema.PodManifestKind,
+		ACVersion: appcschema.AppContainerVersion,
+		Annotations: appctypes.Annotations{
+			appctypes.Annotation{
+				Name:  *appctypes.MustACIdentifier(k8sRktKubeletAnno),
+				Value: k8sRktKubeletAnnoValue,
+			},
+			appctypes.Annotation{
+				Name:  *appctypes.MustACIdentifier(k8sRktUIDAnno),
+				Value: podUID,
+			},
+			appctypes.Annotation{
+				Name:  *appctypes.MustACIdentifier(k8sRktNameAnno),
+				Value: podName,
+			},
+			appctypes.Annotation{
+				Name:  *appctypes.MustACIdentifier(k8sRktNamespaceAnno),
+				Value: podNamespace,
+			},
+			appctypes.Annotation{
+				Name:  *appctypes.MustACIdentifier(k8sRktCreationTimeAnno),
+				Value: podCreationTs,
+			},
+			appctypes.Annotation{
+				Name:  *appctypes.MustACIdentifier(k8sRktRestartCountAnno),
+				Value: podRestartCount,
+			},
+		},
+	}
+
+	appNum := len(appNames)
+	if appNum != len(imgNames) ||
+		appNum != len(imgIDs) ||
+		appNum != len(containerHashes) ||
+		appNum != len(appStates) {
+		panic("inconsistent app number")
+	}
+
+	apps := make([]*rktapi.App, appNum)
+	for i := range appNames {
+		apps[i] = &rktapi.App{
+			Name:  appNames[i],
+			State: appStates[i],
+			Image: &rktapi.Image{
+				Id:   imgIDs[i],
+				Name: imgNames[i],
+				Manifest: mustMarshalImageManifest(
+					&appcschema.ImageManifest{
+						ACKind:    appcschema.ImageManifestKind,
+						ACVersion: appcschema.AppContainerVersion,
+						Name:      *appctypes.MustACIdentifier(imgNames[i]),
+						Annotations: appctypes.Annotations{
+							appctypes.Annotation{
+								Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
+								Value: containerHashes[i],
+							},
+						},
+					},
+				),
+			},
+		}
+		podManifest.Apps = append(podManifest.Apps, appcschema.RuntimeApp{
+			Name:  *appctypes.MustACName(appNames[i]),
+			Image: appcschema.RuntimeImage{ID: *mustRktHash("sha512-foo")},
+			Annotations: appctypes.Annotations{
+				appctypes.Annotation{
+					Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
+					Value: containerHashes[i],
+				},
+			},
+		})
+	}
+
+	return &rktapi.Pod{
+		Id:       rktPodID,
+		State:    rktPodState,
+		Networks: []*rktapi.Network{{Name: defaultNetworkName, Ipv4: podIP}},
+		Apps:     apps,
+		Manifest: mustMarshalPodManifest(podManifest),
+	}
+}
+
 func TestCheckVersion(t *testing.T) {
 	fr := newFakeRktInterface()
 	fs := newFakeSystemd()
@@ -210,190 +322,133 @@ func TestGetPods(t *testing.T) {
 	r := &Runtime{apisvc: fr, systemd: fs}
 
 	tests := []struct {
-		k8sUID        types.UID
+		pods   []*rktapi.Pod
-		k8sName       string
+		result []*kubecontainer.Pod
-		k8sNamespace  string
-		k8sCreation   int64
-		k8sRestart    int
-		k8sContHashes []uint64
-		rktPodState   rktapi.PodState
-		pods          []*rktapi.Pod
 	}{
+		// No pods.
 		{},
+		// One pod.
 		{
-			k8sUID:        types.UID("0"),
+			[]*rktapi.Pod{
-			k8sName:       "guestbook",
+				makeRktPod(rktapi.PodState_POD_STATE_RUNNING,
-			k8sNamespace:  "default",
+					"uuid-4002", "42", "guestbook", "default",
-			k8sCreation:   10000000000,
+					"10.10.10.42", "100000", "7",
-			k8sRestart:    1,
+					[]string{"app-1", "app-2"},
-			k8sContHashes: []uint64{2353434678},
+					[]string{"img-id-1", "img-id-2"},
-			rktPodState:   rktapi.PodState_POD_STATE_RUNNING,
+					[]string{"img-name-1", "img-name-2"},
-			pods: []*rktapi.Pod{
+					[]string{"1001", "1002"},
+					[]rktapi.AppState{rktapi.AppState_APP_STATE_RUNNING, rktapi.AppState_APP_STATE_EXITED},
+				),
+			},
+			[]*kubecontainer.Pod{
 				{
-					State: rktapi.PodState_POD_STATE_RUNNING,
+					ID:        "42",
-					Apps: []*rktapi.App{
+					Name:      "guestbook",
+					Namespace: "default",
+					Containers: []*kubecontainer.Container{
 						{
-							Name: "test",
+							ID:      kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"),
-							Image: &rktapi.Image{
+							Name:    "app-1",
-								Name: "test",
+							Image:   "img-name-1",
-								Manifest: mustMarshalImageManifest(
+							Hash:    1001,
-									&appcschema.ImageManifest{
+							Created: 100000,
-										ACKind:    appcschema.ImageManifestKind,
+							State:   "running",
-										ACVersion: appcschema.AppContainerVersion,
+						},
-										Name:      *appctypes.MustACIdentifier("test"),
+						{
-										Annotations: appctypes.Annotations{
+							ID:      kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"),
-											appctypes.Annotation{
+							Name:    "app-2",
-												Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
+							Image:   "img-name-2",
-												Value: "2353434678",
+							Hash:    1002,
-											},
+							Created: 100000,
-										},
+							State:   "exited",
-									},
-								),
-							},
 						},
 					},
-					Manifest: mustMarshalPodManifest(
-						&appcschema.PodManifest{
-							ACKind:    appcschema.PodManifestKind,
-							ACVersion: appcschema.AppContainerVersion,
-							Annotations: appctypes.Annotations{
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktKubeletAnno),
-									Value: k8sRktKubeletAnnoValue,
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktUIDAnno),
-									Value: "0",
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktNameAnno),
-									Value: "guestbook",
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktNamespaceAnno),
-									Value: "default",
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktCreationTimeAnno),
-									Value: "10000000000",
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktRestartCountAnno),
-									Value: "1",
-								},
-							},
-						},
-					),
 				},
 			},
 		},
+		// Multiple pods.
 		{
-			k8sUID:        types.UID("1"),
+			[]*rktapi.Pod{
-			k8sName:       "test-pod",
+				makeRktPod(rktapi.PodState_POD_STATE_RUNNING,
-			k8sNamespace:  "default",
+					"uuid-4002", "42", "guestbook", "default",
-			k8sCreation:   10000000001,
+					"10.10.10.42", "100000", "7",
-			k8sRestart:    3,
+					[]string{"app-1", "app-2"},
-			k8sContHashes: []uint64{2353434682, 8732645},
+					[]string{"img-id-1", "img-id-2"},
-			rktPodState:   rktapi.PodState_POD_STATE_EXITED,
+					[]string{"img-name-1", "img-name-2"},
-			pods: []*rktapi.Pod{
+					[]string{"1001", "1002"},
+					[]rktapi.AppState{rktapi.AppState_APP_STATE_RUNNING, rktapi.AppState_APP_STATE_EXITED},
+				),
+				makeRktPod(rktapi.PodState_POD_STATE_EXITED,
+					"uuid-4003", "43", "guestbook", "default",
+					"10.10.10.43", "90000", "7",
+					[]string{"app-11", "app-22"},
+					[]string{"img-id-11", "img-id-22"},
+					[]string{"img-name-11", "img-name-22"},
+					[]string{"10011", "10022"},
+					[]rktapi.AppState{rktapi.AppState_APP_STATE_RUNNING, rktapi.AppState_APP_STATE_EXITED},
+				),
+			},
+			[]*kubecontainer.Pod{
 				{
-					State: rktapi.PodState_POD_STATE_EXITED,
+					ID:        "42",
-					Apps: []*rktapi.App{
+					Name:      "guestbook",
+					Namespace: "default",
+					Containers: []*kubecontainer.Container{
 						{
-							Name: "test",
+							ID:      kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"),
-							Image: &rktapi.Image{
+							Name:    "app-1",
-								Name: "test",
+							Image:   "img-name-1",
-								Manifest: mustMarshalImageManifest(
+							Hash:    1001,
-									&appcschema.ImageManifest{
+							Created: 100000,
-										ACKind:    appcschema.ImageManifestKind,
+							State:   "running",
-										ACVersion: appcschema.AppContainerVersion,
-										Name:      *appctypes.MustACIdentifier("test"),
-										Annotations: appctypes.Annotations{
-											appctypes.Annotation{
-												Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
-												Value: "2353434682",
-											},
-										},
-									},
-								),
-							},
 						},
 						{
-							Name: "test2",
+							ID:      kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"),
-							Image: &rktapi.Image{
+							Name:    "app-2",
-								Name: "test2",
+							Image:   "img-name-2",
-								Manifest: mustMarshalImageManifest(
+							Hash:    1002,
-									&appcschema.ImageManifest{
+							Created: 100000,
-										ACKind:    appcschema.ImageManifestKind,
+							State:   "exited",
-										ACVersion: appcschema.AppContainerVersion,
-										Name:      *appctypes.MustACIdentifier("test2"),
-										Annotations: appctypes.Annotations{
-											appctypes.Annotation{
-												Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
-												Value: "8732645",
-											},
-										},
-									},
-								),
-							},
 						},
 					},
-					Manifest: mustMarshalPodManifest(
+				},
-						&appcschema.PodManifest{
+				{
-							ACKind:    appcschema.PodManifestKind,
+					ID:        "43",
-							ACVersion: appcschema.AppContainerVersion,
+					Name:      "guestbook",
-							Annotations: appctypes.Annotations{
+					Namespace: "default",
-								appctypes.Annotation{
+					Containers: []*kubecontainer.Container{
-									Name:  *appctypes.MustACIdentifier(k8sRktKubeletAnno),
+						{
-									Value: k8sRktKubeletAnnoValue,
+							ID:      kubecontainer.BuildContainerID("rkt", "uuid-4003:app-11"),
-								},
+							Name:    "app-11",
-								appctypes.Annotation{
+							Image:   "img-name-11",
-									Name:  *appctypes.MustACIdentifier(k8sRktUIDAnno),
+							Hash:    10011,
-									Value: "1",
+							Created: 90000,
-								},
+							State:   "running",
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktNameAnno),
-									Value: "test-pod",
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktNamespaceAnno),
-									Value: "default",
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktCreationTimeAnno),
-									Value: "10000000001",
-								},
-								appctypes.Annotation{
-									Name:  *appctypes.MustACIdentifier(k8sRktRestartCountAnno),
-									Value: "3",
-								},
-							},
 						},
-					),
+						{
+							ID:      kubecontainer.BuildContainerID("rkt", "uuid-4003:app-22"),
+							Name:    "app-22",
+							Image:   "img-name-22",
+							Hash:    10022,
+							Created: 90000,
+							State:   "exited",
+						},
+					},
 				},
 			},
 		},
 	}
 
 	for i, tt := range tests {
+		testCaseHint := fmt.Sprintf("test case #%d", i)
 		fr.pods = tt.pods
 
 		pods, err := r.GetPods(true)
 		if err != nil {
-			t.Errorf("%v", err)
+			t.Errorf("test case #%d: unexpected error: %v", i, err)
 		}
-		assert.Equal(t, len(pods), len(tt.pods), fmt.Sprintf("test case %d: mismatched number of pods", i))
 
-		for j, pod := range pods {
+		assert.Equal(t, tt.result, pods, testCaseHint)
-			assert.Equal(t, pod.ID, tt.k8sUID, fmt.Sprintf("test case %d: mismatched UIDs", i))
-			assert.Equal(t, pod.Name, tt.k8sName, fmt.Sprintf("test case %d: mismatched Names", i))
-			assert.Equal(t, pod.Namespace, tt.k8sNamespace, fmt.Sprintf("test case %d: mismatched Namespaces", i))
-			assert.Equal(t, len(pod.Containers), len(tt.pods[j].Apps), fmt.Sprintf("test case %d: mismatched number of containers", i))
-			for k, cont := range pod.Containers {
-				assert.Equal(t, cont.Created, tt.k8sCreation, fmt.Sprintf("test case %d: mismatched creation times", i))
-				assert.Equal(t, cont.Hash, tt.k8sContHashes[k], fmt.Sprintf("test case %d: mismatched container hashes", i))
-			}
-		}
 
 		var inspectPodCalls []string
 		for range pods {
@@ -446,119 +501,6 @@ func TestGetPodsFilter(t *testing.T) {
 	}
 }
 
-func mustMarshalPodManifest(man *appcschema.PodManifest) []byte {
-	manblob, err := json.Marshal(man)
-	if err != nil {
-		panic(err)
-	}
-	return manblob
-}
-
-func mustMarshalImageManifest(man *appcschema.ImageManifest) []byte {
-	manblob, err := json.Marshal(man)
-	if err != nil {
-		panic(err)
-	}
-	return manblob
-}
-
-func mustRktHash(hash string) *appctypes.Hash {
-	h, err := appctypes.NewHash(hash)
-	if err != nil {
-		panic(err)
-	}
-	return h
-}
-
-func makeRktPod(rktPodState rktapi.PodState,
-	rktPodID, podUID, podName, podNamespace,
-	podIP, podCreationTs, podRestartCount string,
-	appNames, imgIDs, imgNames, containerHashes []string,
-	appStates []rktapi.AppState) *rktapi.Pod {
-
-	podManifest := &appcschema.PodManifest{
-		ACKind:    appcschema.PodManifestKind,
-		ACVersion: appcschema.AppContainerVersion,
-		Annotations: appctypes.Annotations{
-			appctypes.Annotation{
-				Name:  *appctypes.MustACIdentifier(k8sRktKubeletAnno),
-				Value: k8sRktKubeletAnnoValue,
-			},
-			appctypes.Annotation{
-				Name:  *appctypes.MustACIdentifier(k8sRktUIDAnno),
-				Value: podUID,
-			},
-			appctypes.Annotation{
-				Name:  *appctypes.MustACIdentifier(k8sRktNameAnno),
-				Value: podName,
-			},
-			appctypes.Annotation{
-				Name:  *appctypes.MustACIdentifier(k8sRktNamespaceAnno),
-				Value: podNamespace,
-			},
-			appctypes.Annotation{
-				Name:  *appctypes.MustACIdentifier(k8sRktCreationTimeAnno),
-				Value: podCreationTs,
-			},
-			appctypes.Annotation{
-				Name:  *appctypes.MustACIdentifier(k8sRktRestartCountAnno),
-				Value: podRestartCount,
-			},
-		},
-	}
-
-	appNum := len(appNames)
-	if appNum != len(imgNames) ||
-		appNum != len(imgIDs) ||
-		appNum != len(containerHashes) ||
-		appNum != len(appStates) {
-		panic("inconsistent app number")
-	}
-
-	apps := make([]*rktapi.App, appNum)
-	for i := range appNames {
-		apps[i] = &rktapi.App{
-			Name:  appNames[i],
-			State: appStates[i],
-			Image: &rktapi.Image{
-				Id:   imgIDs[i],
-				Name: imgNames[i],
-				Manifest: mustMarshalImageManifest(
-					&appcschema.ImageManifest{
-						ACKind:    appcschema.ImageManifestKind,
-						ACVersion: appcschema.AppContainerVersion,
-						Name:      *appctypes.MustACIdentifier(imgNames[i]),
-						Annotations: appctypes.Annotations{
-							appctypes.Annotation{
-								Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
-								Value: containerHashes[i],
-							},
-						},
-					},
-				),
-			},
-		}
-		podManifest.Apps = append(podManifest.Apps, appcschema.RuntimeApp{
-			Name:  *appctypes.MustACName(appNames[i]),
-			Image: appcschema.RuntimeImage{ID: *mustRktHash("sha512-foo")},
-			Annotations: appctypes.Annotations{
-				appctypes.Annotation{
-					Name:  *appctypes.MustACIdentifier(k8sRktContainerHashAnno),
-					Value: containerHashes[i],
-				},
-			},
-		})
-	}
-
-	return &rktapi.Pod{
-		Id:       rktPodID,
-		State:    rktPodState,
-		Networks: []*rktapi.Network{{Name: defaultNetworkName, Ipv4: podIP}},
-		Apps:     apps,
-		Manifest: mustMarshalPodManifest(podManifest),
-	}
-}
-
 func TestGetPodStatus(t *testing.T) {
 	fr := newFakeRktInterface()
 	fs := newFakeSystemd()
@@ -703,6 +645,7 @@ func TestGetPodStatus(t *testing.T) {
 		if err != nil {
 			t.Errorf("test case #%d: unexpected error: %v", i, err)
 		}
+
 		assert.Equal(t, tt.result, status, testCaseHint)
 
 		var inspectPodCalls []string