From 3b367f8ab85f6fae648af8a33b469ca33eb53413 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Fri, 20 Mar 2015 13:27:41 +0100 Subject: [PATCH 1/2] Refactor file_test.go --- pkg/kubelet/config/file_test.go | 476 +++++++++++++------------------- 1 file changed, 195 insertions(+), 281 deletions(-) diff --git a/pkg/kubelet/config/file_test.go b/pkg/kubelet/config/file_test.go index eebbcb1576228..d5173816a8db5 100644 --- a/pkg/kubelet/config/file_test.go +++ b/pkg/kubelet/config/file_test.go @@ -32,60 +32,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/types" ) -// TODO(wojtek-t): Most of the test cases are pretty similar and introduce -// the same boilerplate. Refactor them similarly to what is done in http_test.go - -func ExampleManifestAndPod(id string) (v1beta1.ContainerManifest, api.Pod) { - hostname, _ := os.Hostname() - hostname = strings.ToLower(hostname) - - manifest := v1beta1.ContainerManifest{ - Version: "v1beta1", - ID: id, - UUID: types.UID(id), - Containers: []v1beta1.Container{ - { - Name: "c" + id, - Image: "foo", - TerminationMessagePath: "/somepath", - }, - }, - Volumes: []v1beta1.Volume{ - { - Name: "host-dir", - Source: v1beta1.VolumeSource{ - HostDir: &v1beta1.HostPathVolumeSource{"/dir/path"}, - }, - }, - }, - } - expectedPod := api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: id + "-" + hostname, - UID: types.UID(id), - Namespace: kubelet.NamespaceDefault, - SelfLink: "/api/v1beta2/pods/" + id + "-" + hostname + "?namespace=default", - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "c" + id, - Image: "foo", - }, - }, - Volumes: []api.Volume{ - { - Name: "host-dir", - VolumeSource: api.VolumeSource{ - HostPath: &api.HostPathVolumeSource{"/dir/path"}, - }, - }, - }, - }, - } - return manifest, expectedPod -} - func TestExtractFromNonExistentFile(t *testing.T) { ch := make(chan interface{}, 1) c := sourceFile{"/some/fake/file", ch} @@ -127,109 +73,152 @@ func TestReadManifestFromFile(t *testing.T) { hostname, _ := os.Hostname() hostname = strings.ToLower(hostname) - file := writeTestFile(t, os.TempDir(), "test_pod_config", - `{ - "version": "v1beta1", - "uuid": "12345", - "id": "test", - "containers": [{ "name": "image", "image": "test/image", imagePullPolicy: "PullAlways"}] - }`) - defer os.Remove(file.Name()) - - ch := make(chan interface{}) - NewSourceFile(file.Name(), time.Millisecond, ch) - select { - case got := <-ch: - update := got.(kubelet.PodUpdate) - expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: "test-" + hostname, - UID: "12345", - Namespace: kubelet.NamespaceDefault, - SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", - }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, - }) - - if !api.Semantic.DeepDerivative(expected, update) { - t.Fatalf("Expected %#v, Got %#v", expected, update) - } - - case <-time.After(time.Second): - t.Errorf("Expected update, timeout instead") - } -} - -func TestReadManifestFromFileWithoutID(t *testing.T) { - hostname, _ := os.Hostname() - hostname = strings.ToLower(hostname) - - file := writeTestFile(t, os.TempDir(), "test_pod_config", - `{ - "version": "v1beta1", - "uuid": "12345", - "containers": [{ "name": "image", "image": "test/image", imagePullPolicy: "PullAlways"}] - }`) - defer os.Remove(file.Name()) - - ch := make(chan interface{}) - NewSourceFile(file.Name(), time.Millisecond, ch) - select { - case got := <-ch: - update := got.(kubelet.PodUpdate) - expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: "12345-" + hostname, - UID: "12345", - Namespace: kubelet.NamespaceDefault, - SelfLink: "/api/v1beta2/pods/12345-" + hostname + "?namespace=default", - }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, - }) - - if !api.Semantic.DeepDerivative(expected, update) { - t.Fatalf("Expected %#v, Got %#v", expected, update) - } - - case <-time.After(time.Second): - t.Errorf("Expected update, timeout instead") + var testCases = []struct { + desc string + fileContents string + expected kubelet.PodUpdate + }{ + { + desc: "Manifest", + fileContents: `{ + "version": "v1beta1", + "uuid": "12345", + "id": "test", + "containers": [{ "name": "image", "image": "test/image", "imagePullPolicy": "PullAlways"}] + }`, + expected: CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "test-" + hostname, + UID: "12345", + Namespace: kubelet.NamespaceDefault, + SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", + }, + Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + }), + }, + { + desc: "Manifest without ID", + fileContents: `{ + "version": "v1beta1", + "uuid": "12345", + "containers": [{ "name": "image", "image": "test/image", "imagePullPolicy": "PullAlways"}] + }`, + expected: CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "12345-" + hostname, + UID: "12345", + Namespace: kubelet.NamespaceDefault, + SelfLink: "/api/v1beta2/pods/12345-" + hostname + "?namespace=default", + }, + Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + }), + }, + { + desc: "Manifest v1beta2", + fileContents: `{ + "version": "v1beta2", + "uuid": "12345", + "id": "test", + "containers": [{ "name": "image", "image": "test/image", "imagePullPolicy": "PullAlways"}] + }`, + expected: CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "test-" + hostname, + UID: "12345", + Namespace: kubelet.NamespaceDefault, + SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", + }, + Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + }), + }, + { + desc: "Simple pod", + fileContents: `{ + "kind": "Pod", + "apiVersion": "v1beta1", + "uid": "12345", + "id": "test", + "namespace": "mynamespace", + "desiredState": { + "manifest": { + "containers": [{ "name": "image", "image": "test/image" }], + }, + }, + }`, + expected: CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "test-" + hostname, + UID: "12345", + Namespace: "mynamespace", + SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=mynamespace", + }, + Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + }), + }, + { + desc: "Pod without ID", + fileContents: `{ + "kind": "Pod", + "apiversion": "v1beta1", + "uid": "12345", + "desiredState": { + "manifest": { + "containers": [{ "name": "image", "image": "test/image" }], + }, + }, + }`, + expected: CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "12345-" + hostname, + UID: "12345", + Namespace: kubelet.NamespaceDefault, + SelfLink: "/api/v1beta2/pods/12345-" + hostname + "?namespace=default", + }, + Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + }), + }, + { + desc: "Pod v1beta3", + fileContents: `{ + "kind": "Pod", + "apiversion": "v1beta3", + "metadata": { + "uid": "12345", + "name": "test", + }, + "spec": { + "containers": [{ "name": "image", "image": "test/image" }], + }, + }`, + expected: CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: "test-" + hostname, + UID: "12345", + Namespace: kubelet.NamespaceDefault, + SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", + }, + Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + }), + }, } -} - -func TestReadManifestV1Beta2FromFile(t *testing.T) { - hostname, _ := os.Hostname() - hostname = strings.ToLower(hostname) - - file := writeTestFile(t, os.TempDir(), "test_pod_config", - `{ - "version": "v1beta2", - "uuid": "12345", - "id": "test", - "containers": [{ "name": "image", "image": "test/image", imagePullPolicy: "PullAlways"}] - }`) - defer os.Remove(file.Name()) - - ch := make(chan interface{}) - NewSourceFile(file.Name(), time.Millisecond, ch) - select { - case got := <-ch: - update := got.(kubelet.PodUpdate) - expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: "test-" + hostname, - UID: "12345", - Namespace: kubelet.NamespaceDefault, - SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", - }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, - }) - - if !api.Semantic.DeepDerivative(expected, update) { - t.Fatalf("Expected %#v, Got %#v", expected, update) - } - case <-time.After(time.Second): - t.Errorf("Expected update, timeout instead") + for _, testCase := range testCases { + func() { + file := writeTestFile(t, os.TempDir(), "test_pod_config", testCase.fileContents) + defer os.Remove(file.Name()) + + ch := make(chan interface{}) + NewSourceFile(file.Name(), time.Millisecond, ch) + select { + case got := <-ch: + update := got.(kubelet.PodUpdate) + if !api.Semantic.DeepDerivative(testCase.expected, update) { + t.Errorf("%s: Expected %#v, Got %#v", testCase.desc, testCase.expected, update) + } + case <-time.After(time.Second): + t.Errorf("%s: Expected update, timeout instead", testCase.desc) + } + }() } } @@ -256,132 +245,6 @@ func TestReadManifestFromFileWithDefaults(t *testing.T) { } } -func TestReadPodFromFile(t *testing.T) { - hostname, _ := os.Hostname() - hostname = strings.ToLower(hostname) - - file := writeTestFile(t, os.TempDir(), "test_pod_config", - `{ - "kind": "Pod", - "apiVersion": "v1beta1", - "uid": "12345", - "id": "test", - "namespace": "mynamespace", - "desiredState": { - "manifest": { - "containers": [{ "name": "image", "image": "test/image" }], - }, - }, - }`) - defer os.Remove(file.Name()) - - ch := make(chan interface{}) - NewSourceFile(file.Name(), time.Millisecond, ch) - select { - case got := <-ch: - update := got.(kubelet.PodUpdate) - expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: "test-" + hostname, - UID: "12345", - Namespace: "mynamespace", - SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=mynamespace", - }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, - }) - - if !api.Semantic.DeepDerivative(expected, update) { - t.Fatalf("Expected %#v, Got %#v", expected, update) - } - - case <-time.After(time.Second): - t.Errorf("Expected update, timeout instead") - } -} - -func TestReadPodV1Beta3FromFile(t *testing.T) { - hostname, _ := os.Hostname() - hostname = strings.ToLower(hostname) - - file := writeTestFile(t, os.TempDir(), "test_pod_config", - `{ - "kind": "Pod", - "apiversion": "v1beta3", - "metadata": { - "uid": "12345", - "name": "test", - }, - "spec": { - "containers": [{ "name": "image", "image": "test/image" }], - }, - }`) - defer os.Remove(file.Name()) - - ch := make(chan interface{}) - NewSourceFile(file.Name(), time.Millisecond, ch) - select { - case got := <-ch: - update := got.(kubelet.PodUpdate) - expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: "test-" + hostname, - UID: "12345", - Namespace: kubelet.NamespaceDefault, - SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", - }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, - }) - - if !api.Semantic.DeepDerivative(expected, update) { - t.Fatalf("Expected %#v, Got %#v", expected, update) - } - - case <-time.After(time.Second): - t.Errorf("Expected update, timeout instead") - } -} - -func TestReadPodFromFileWithoutID(t *testing.T) { - hostname, _ := os.Hostname() - hostname = strings.ToLower(hostname) - - file := writeTestFile(t, os.TempDir(), "test_pod_config", - `{ - "kind": "Pod", - "apiversion": "v1beta1", - "uid": "12345", - "DesiredState": { - "Manifest": { - "containers": [{ "name": "image", "image": "test/image" }], - }, - }, - }`) - defer os.Remove(file.Name()) - - ch := make(chan interface{}) - NewSourceFile(file.Name(), time.Millisecond, ch) - select { - case got := <-ch: - update := got.(kubelet.PodUpdate) - expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource, api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: "12345-" + hostname, - UID: "12345", - Namespace: kubelet.NamespaceDefault, - SelfLink: "/api/v1beta2/pods/12345-" + hostname + "?namespace=default", - }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, - }) - - if !api.Semantic.DeepDerivative(expected, update) { - t.Fatalf("Expected %#v, Got %#v", expected, update) - } - - case <-time.After(time.Second): - t.Errorf("Expected update, timeout instead") - } -} - func TestExtractFromBadDataFile(t *testing.T) { file := writeTestFile(t, os.TempDir(), "test_pod_config", string([]byte{1, 2, 3})) defer os.Remove(file.Name()) @@ -416,6 +279,57 @@ func TestExtractFromEmptyDir(t *testing.T) { } } +func ExampleManifestAndPod(id string) (v1beta1.ContainerManifest, api.Pod) { + hostname, _ := os.Hostname() + hostname = strings.ToLower(hostname) + + manifest := v1beta1.ContainerManifest{ + Version: "v1beta1", + ID: id, + UUID: types.UID(id), + Containers: []v1beta1.Container{ + { + Name: "c" + id, + Image: "foo", + TerminationMessagePath: "/somepath", + }, + }, + Volumes: []v1beta1.Volume{ + { + Name: "host-dir", + Source: v1beta1.VolumeSource{ + HostDir: &v1beta1.HostPathVolumeSource{"/dir/path"}, + }, + }, + }, + } + expectedPod := api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: id + "-" + hostname, + UID: types.UID(id), + Namespace: kubelet.NamespaceDefault, + SelfLink: "/api/v1beta2/pods/" + id + "-" + hostname + "?namespace=default", + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "c" + id, + Image: "foo", + }, + }, + Volumes: []api.Volume{ + { + Name: "host-dir", + VolumeSource: api.VolumeSource{ + HostPath: &api.HostPathVolumeSource{"/dir/path"}, + }, + }, + }, + }, + } + return manifest, expectedPod +} + func TestExtractFromDir(t *testing.T) { manifest, expectedPod := ExampleManifestAndPod("1") manifest2, expectedPod2 := ExampleManifestAndPod("2") From 9c4ef28b2dba4eca955fbe1c083d0a3ced8400a7 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Fri, 20 Mar 2015 20:08:11 +0100 Subject: [PATCH 2/2] Comments --- pkg/kubelet/config/file_test.go | 71 ++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 9 deletions(-) diff --git a/pkg/kubelet/config/file_test.go b/pkg/kubelet/config/file_test.go index d5173816a8db5..44d1f9279f3e5 100644 --- a/pkg/kubelet/config/file_test.go +++ b/pkg/kubelet/config/file_test.go @@ -69,7 +69,7 @@ func writeTestFile(t *testing.T, dir, name string, contents string) *os.File { return file } -func TestReadManifestFromFile(t *testing.T) { +func TestReadFromFile(t *testing.T) { hostname, _ := os.Hostname() hostname = strings.ToLower(hostname) @@ -93,7 +93,15 @@ func TestReadManifestFromFile(t *testing.T) { Namespace: kubelet.NamespaceDefault, SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{ + Name: "image", + Image: "test/image", + TerminationMessagePath: "/dev/termination-log", + ImagePullPolicy: "Always"}}, + }, }), }, { @@ -110,7 +118,15 @@ func TestReadManifestFromFile(t *testing.T) { Namespace: kubelet.NamespaceDefault, SelfLink: "/api/v1beta2/pods/12345-" + hostname + "?namespace=default", }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{ + Name: "image", + Image: "test/image", + TerminationMessagePath: "/dev/termination-log", + ImagePullPolicy: "Always"}}, + }, }), }, { @@ -128,7 +144,15 @@ func TestReadManifestFromFile(t *testing.T) { Namespace: kubelet.NamespaceDefault, SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{ + Name: "image", + Image: "test/image", + TerminationMessagePath: "/dev/termination-log", + ImagePullPolicy: "Always"}}, + }, }), }, { @@ -152,7 +176,15 @@ func TestReadManifestFromFile(t *testing.T) { Namespace: "mynamespace", SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=mynamespace", }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{ + Name: "image", + Image: "test/image", + TerminationMessagePath: "/dev/termination-log", + ImagePullPolicy: "IfNotPresent"}}, + }, }), }, { @@ -174,7 +206,15 @@ func TestReadManifestFromFile(t *testing.T) { Namespace: kubelet.NamespaceDefault, SelfLink: "/api/v1beta2/pods/12345-" + hostname + "?namespace=default", }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{ + Name: "image", + Image: "test/image", + TerminationMessagePath: "/dev/termination-log", + ImagePullPolicy: "IfNotPresent"}}, + }, }), }, { @@ -197,7 +237,15 @@ func TestReadManifestFromFile(t *testing.T) { Namespace: kubelet.NamespaceDefault, SelfLink: "/api/v1beta2/pods/test-" + hostname + "?namespace=default", }, - Spec: api.PodSpec{Containers: []api.Container{{Image: "test/image"}}}, + Spec: api.PodSpec{ + RestartPolicy: api.RestartPolicyAlways, + DNSPolicy: api.DNSClusterFirst, + Containers: []api.Container{{ + Name: "image", + Image: "test/image", + TerminationMessagePath: "/dev/termination-log", + ImagePullPolicy: "IfNotPresent"}}, + }, }), }, } @@ -212,7 +260,12 @@ func TestReadManifestFromFile(t *testing.T) { select { case got := <-ch: update := got.(kubelet.PodUpdate) - if !api.Semantic.DeepDerivative(testCase.expected, update) { + for _, pod := range update.Pods { + if errs := validation.ValidatePod(&pod); len(errs) > 0 { + t.Errorf("%s: Invalid pod %#v, %#v", testCase.desc, pod, errs) + } + } + if !api.Semantic.DeepEqual(testCase.expected, update) { t.Errorf("%s: Expected %#v, Got %#v", testCase.desc, testCase.expected, update) } case <-time.After(time.Second): @@ -274,7 +327,7 @@ func TestExtractFromEmptyDir(t *testing.T) { update := (<-ch).(kubelet.PodUpdate) expected := CreatePodUpdate(kubelet.SET, kubelet.FileSource) - if !api.Semantic.DeepDerivative(expected, update) { + if !api.Semantic.DeepEqual(expected, update) { t.Errorf("Expected %#v, Got %#v", expected, update) } }