From 2c79c670f186951fec34a4d4b40e0f9cb1913a62 Mon Sep 17 00:00:00 2001 From: Yifan Gu Date: Mon, 14 Dec 2015 17:26:43 -0800 Subject: [PATCH] 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 | 31 +-- pkg/kubelet/rkt/rkt_test.go | 479 ++++++++++++++++-------------------- 2 files changed, 220 insertions(+), 290 deletions(-) diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 62fc0e8ba62..1fe85b5ca5c 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/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{} - err := json.Unmarshal(app.Image.Manifest, manifest) - if err != nil { - return nil, err - } - containerHashString, ok := manifest.Annotations.Get(k8sRktContainerHashAnno) + + for i, app := range rktpod.Apps { + // The order of the apps is determined by the rkt pod manifest. + // TODO(yifan): Let the server to unmarshal the annotations? https://github.com/coreos/rkt/issues/1872 + hashStr, ok := manifest.Apps[i].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), }) } diff --git a/pkg/kubelet/rkt/rkt_test.go b/pkg/kubelet/rkt/rkt_test.go index eee9e927f56..c6be6aefd95 100644 --- a/pkg/kubelet/rkt/rkt_test.go +++ b/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 - k8sName string - k8sNamespace string - k8sCreation int64 - k8sRestart int - k8sContHashes []uint64 - rktPodState rktapi.PodState - pods []*rktapi.Pod + pods []*rktapi.Pod + result []*kubecontainer.Pod }{ + // No pods. {}, + // One pod. { - k8sUID: types.UID("0"), - k8sName: "guestbook", - k8sNamespace: "default", - k8sCreation: 10000000000, - k8sRestart: 1, - k8sContHashes: []uint64{2353434678}, - rktPodState: rktapi.PodState_POD_STATE_RUNNING, - pods: []*rktapi.Pod{ + []*rktapi.Pod{ + makeRktPod(rktapi.PodState_POD_STATE_RUNNING, + "uuid-4002", "42", "guestbook", "default", + "10.10.10.42", "100000", "7", + []string{"app-1", "app-2"}, + []string{"img-id-1", "img-id-2"}, + []string{"img-name-1", "img-name-2"}, + []string{"1001", "1002"}, + []rktapi.AppState{rktapi.AppState_APP_STATE_RUNNING, rktapi.AppState_APP_STATE_EXITED}, + ), + }, + []*kubecontainer.Pod{ { - State: rktapi.PodState_POD_STATE_RUNNING, - Apps: []*rktapi.App{ + ID: "42", + Name: "guestbook", + Namespace: "default", + Containers: []*kubecontainer.Container{ { - Name: "test", - Image: &rktapi.Image{ - Name: "test", - Manifest: mustMarshalImageManifest( - &appcschema.ImageManifest{ - ACKind: appcschema.ImageManifestKind, - ACVersion: appcschema.AppContainerVersion, - Name: *appctypes.MustACIdentifier("test"), - Annotations: appctypes.Annotations{ - appctypes.Annotation{ - Name: *appctypes.MustACIdentifier(k8sRktContainerHashAnno), - Value: "2353434678", - }, - }, - }, - ), - }, + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"), + Name: "app-1", + Image: "img-name-1", + Hash: 1001, + Created: 100000, + State: "running", + }, + { + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"), + Name: "app-2", + Image: "img-name-2", + 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"), - k8sName: "test-pod", - k8sNamespace: "default", - k8sCreation: 10000000001, - k8sRestart: 3, - k8sContHashes: []uint64{2353434682, 8732645}, - rktPodState: rktapi.PodState_POD_STATE_EXITED, - pods: []*rktapi.Pod{ + []*rktapi.Pod{ + makeRktPod(rktapi.PodState_POD_STATE_RUNNING, + "uuid-4002", "42", "guestbook", "default", + "10.10.10.42", "100000", "7", + []string{"app-1", "app-2"}, + []string{"img-id-1", "img-id-2"}, + []string{"img-name-1", "img-name-2"}, + []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, - Apps: []*rktapi.App{ + ID: "42", + Name: "guestbook", + Namespace: "default", + Containers: []*kubecontainer.Container{ { - Name: "test", - Image: &rktapi.Image{ - Name: "test", - Manifest: mustMarshalImageManifest( - &appcschema.ImageManifest{ - ACKind: appcschema.ImageManifestKind, - ACVersion: appcschema.AppContainerVersion, - Name: *appctypes.MustACIdentifier("test"), - Annotations: appctypes.Annotations{ - appctypes.Annotation{ - Name: *appctypes.MustACIdentifier(k8sRktContainerHashAnno), - Value: "2353434682", - }, - }, - }, - ), - }, + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"), + Name: "app-1", + Image: "img-name-1", + Hash: 1001, + Created: 100000, + State: "running", }, { - Name: "test2", - Image: &rktapi.Image{ - Name: "test2", - Manifest: mustMarshalImageManifest( - &appcschema.ImageManifest{ - ACKind: appcschema.ImageManifestKind, - ACVersion: appcschema.AppContainerVersion, - Name: *appctypes.MustACIdentifier("test2"), - Annotations: appctypes.Annotations{ - appctypes.Annotation{ - Name: *appctypes.MustACIdentifier(k8sRktContainerHashAnno), - Value: "8732645", - }, - }, - }, - ), - }, + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"), + Name: "app-2", + Image: "img-name-2", + 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: "1", - }, - 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: "43", + Name: "guestbook", + Namespace: "default", + Containers: []*kubecontainer.Container{ + { + ID: kubecontainer.BuildContainerID("rkt", "uuid-4003:app-11"), + Name: "app-11", + Image: "img-name-11", + Hash: 10011, + Created: 90000, + State: "running", }, - ), + { + 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, 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)) - } - } + assert.Equal(t, tt.result, pods, testCaseHint) 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