From 824768d6042038463356e06aacd5de9c8f510164 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Mon, 21 Sep 2015 16:30:02 -0700 Subject: [PATCH] consolidate a bunch of FakePodControl testclients into a shared one --- pkg/controller/controller_utils.go | 52 ++++++++++- pkg/controller/daemon/controller_test.go | 55 ++---------- pkg/controller/job/controller_test.go | 71 ++++----------- .../replication_controller_test.go | 90 ++++++------------- 4 files changed, 100 insertions(+), 168 deletions(-) diff --git a/pkg/controller/controller_utils.go b/pkg/controller/controller_utils.go index 9b97146fa2..1b330643c2 100644 --- a/pkg/controller/controller_utils.go +++ b/pkg/controller/controller_utils.go @@ -18,9 +18,9 @@ package controller import ( "fmt" - "time" - + "sync" "sync/atomic" + "time" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" @@ -226,6 +226,8 @@ type RealPodControl struct { Recorder record.EventRecorder } +var _ PodControlInterface = &RealPodControl{} + func getPodsLabelSet(template *api.PodTemplateSpec) labels.Set { desiredLabels := make(labels.Set) for k, v := range template.Labels { @@ -312,6 +314,52 @@ func (r RealPodControl) DeletePod(namespace, podID string) error { return r.KubeClient.Pods(namespace).Delete(podID, nil) } +type FakePodControl struct { + sync.Mutex + Templates []api.PodTemplateSpec + DeletePodName []string + Err error +} + +var _ PodControlInterface = &FakePodControl{} + +func (f *FakePodControl) CreatePods(namespace string, spec *api.PodTemplateSpec, object runtime.Object) error { + f.Lock() + defer f.Unlock() + if f.Err != nil { + return f.Err + } + f.Templates = append(f.Templates, *spec) + return nil +} + +func (f *FakePodControl) CreatePodsOnNode(nodeName, namespace string, template *api.PodTemplateSpec, object runtime.Object) error { + f.Lock() + defer f.Unlock() + if f.Err != nil { + return f.Err + } + f.Templates = append(f.Templates, *template) + return nil +} + +func (f *FakePodControl) DeletePod(namespace string, podName string) error { + f.Lock() + defer f.Unlock() + if f.Err != nil { + return f.Err + } + f.DeletePodName = append(f.DeletePodName, podName) + return nil +} + +func (f *FakePodControl) Clear() { + f.Lock() + defer f.Unlock() + f.DeletePodName = []string{} + f.Templates = []api.PodTemplateSpec{} +} + // ActivePods type allows custom sorting of pods so a controller can pick the best ones to delete. type ActivePods []*api.Pod diff --git a/pkg/controller/daemon/controller_test.go b/pkg/controller/daemon/controller_test.go index 9e458bda42..fb9abeadd2 100644 --- a/pkg/controller/daemon/controller_test.go +++ b/pkg/controller/daemon/controller_test.go @@ -18,7 +18,6 @@ package daemon import ( "fmt" - "sync" "testing" "k8s.io/kubernetes/pkg/api" @@ -28,7 +27,6 @@ import ( "k8s.io/kubernetes/pkg/client/cache" client "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/controller" - "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/securitycontext" ) @@ -39,47 +37,10 @@ var ( simpleNodeLabel2 = map[string]string{"color": "red", "speed": "fast"} ) -type FakePodControl struct { - podSpec []api.PodTemplateSpec - deletePodName []string - lock sync.Mutex - err error -} - func init() { api.ForTesting_ReferencesAllowBlankSelfLinks = true } -func (f *FakePodControl) CreatePods(namespace string, spec *api.PodTemplateSpec, object runtime.Object) error { - return nil -} - -func (f *FakePodControl) CreatePodsOnNode(nodeName, namespace string, spec *api.PodTemplateSpec, object runtime.Object) error { - f.lock.Lock() - defer f.lock.Unlock() - if f.err != nil { - return f.err - } - f.podSpec = append(f.podSpec, *spec) - return nil -} - -func (f *FakePodControl) DeletePod(namespace string, podName string) error { - f.lock.Lock() - defer f.lock.Unlock() - if f.err != nil { - return f.err - } - f.deletePodName = append(f.deletePodName, podName) - return nil -} -func (f *FakePodControl) clear() { - f.lock.Lock() - defer f.lock.Unlock() - f.deletePodName = []string{} - f.podSpec = []api.PodTemplateSpec{} -} - func newDaemonSet(name string) *experimental.DaemonSet { return &experimental.DaemonSet{ TypeMeta: unversioned.TypeMeta{APIVersion: testapi.Experimental.Version()}, @@ -157,24 +118,24 @@ func addPods(podStore cache.Store, nodeName string, label map[string]string, num } } -func newTestController() (*DaemonSetsController, *FakePodControl) { +func newTestController() (*DaemonSetsController, *controller.FakePodControl) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.GroupAndVersion()}) manager := NewDaemonSetsController(client) - podControl := &FakePodControl{} + podControl := &controller.FakePodControl{} manager.podControl = podControl return manager, podControl } -func validateSyncDaemonSets(t *testing.T, fakePodControl *FakePodControl, expectedCreates, expectedDeletes int) { - if len(fakePodControl.podSpec) != expectedCreates { - t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", expectedCreates, len(fakePodControl.podSpec)) +func validateSyncDaemonSets(t *testing.T, fakePodControl *controller.FakePodControl, expectedCreates, expectedDeletes int) { + if len(fakePodControl.Templates) != expectedCreates { + t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", expectedCreates, len(fakePodControl.Templates)) } - if len(fakePodControl.deletePodName) != expectedDeletes { - t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", expectedDeletes, len(fakePodControl.deletePodName)) + if len(fakePodControl.DeletePodName) != expectedDeletes { + t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", expectedDeletes, len(fakePodControl.DeletePodName)) } } -func syncAndValidateDaemonSets(t *testing.T, manager *DaemonSetsController, ds *experimental.DaemonSet, podControl *FakePodControl, expectedCreates, expectedDeletes int) { +func syncAndValidateDaemonSets(t *testing.T, manager *DaemonSetsController, ds *experimental.DaemonSet, podControl *controller.FakePodControl, expectedCreates, expectedDeletes int) { key, err := controller.KeyFunc(ds) if err != nil { t.Errorf("Could not get key for daemon.") diff --git a/pkg/controller/job/controller_test.go b/pkg/controller/job/controller_test.go index 94c6725d87..14cb560d9b 100644 --- a/pkg/controller/job/controller_test.go +++ b/pkg/controller/job/controller_test.go @@ -18,7 +18,6 @@ package job import ( "fmt" - "sync" "testing" "time" @@ -29,7 +28,6 @@ import ( client "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/client/unversioned/testclient" "k8s.io/kubernetes/pkg/controller" - "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util" "k8s.io/kubernetes/pkg/watch" ) @@ -42,43 +40,6 @@ const controllerTimeout = 500 * time.Millisecond var alwaysReady = func() bool { return true } -type FakePodControl struct { - podSpec []api.PodTemplateSpec - deletePodName []string - lock sync.Mutex - err error -} - -func (f *FakePodControl) CreatePods(namespace string, spec *api.PodTemplateSpec, object runtime.Object) error { - f.lock.Lock() - defer f.lock.Unlock() - if f.err != nil { - return f.err - } - f.podSpec = append(f.podSpec, *spec) - return nil -} - -func (f *FakePodControl) CreatePodsOnNode(nodeName, namespace string, template *api.PodTemplateSpec, object runtime.Object) error { - return nil -} - -func (f *FakePodControl) DeletePod(namespace string, podName string) error { - f.lock.Lock() - defer f.lock.Unlock() - if f.err != nil { - return f.err - } - f.deletePodName = append(f.deletePodName, podName) - return nil -} -func (f *FakePodControl) clear() { - f.lock.Lock() - defer f.lock.Unlock() - f.deletePodName = []string{} - f.podSpec = []api.PodTemplateSpec{} -} - func newJob(parallelism, completions int) *experimental.Job { return &experimental.Job{ ObjectMeta: api.ObjectMeta{ @@ -207,7 +168,7 @@ func TestControllerSyncJob(t *testing.T) { // job manager setup client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.GroupAndVersion()}) manager := NewJobController(client) - fakePodControl := FakePodControl{err: tc.podControllerError} + fakePodControl := controller.FakePodControl{Err: tc.podControllerError} manager.podControl = &fakePodControl manager.podStoreSynced = alwaysReady var actual *experimental.Job @@ -236,11 +197,11 @@ func TestControllerSyncJob(t *testing.T) { } // validate created/deleted pods - if len(fakePodControl.podSpec) != tc.expectedCreations { - t.Errorf("%s: unexpected number of creates. Expected %d, saw %d\n", name, tc.expectedCreations, len(fakePodControl.podSpec)) + if len(fakePodControl.Templates) != tc.expectedCreations { + t.Errorf("%s: unexpected number of creates. Expected %d, saw %d\n", name, tc.expectedCreations, len(fakePodControl.Templates)) } - if len(fakePodControl.deletePodName) != tc.expectedDeletions { - t.Errorf("%s: unexpected number of deletes. Expected %d, saw %d\n", name, tc.expectedDeletions, len(fakePodControl.deletePodName)) + if len(fakePodControl.DeletePodName) != tc.expectedDeletions { + t.Errorf("%s: unexpected number of deletes. Expected %d, saw %d\n", name, tc.expectedDeletions, len(fakePodControl.DeletePodName)) } // validate status if actual.Status.Active != tc.expectedActive { @@ -271,7 +232,7 @@ func TestControllerSyncJob(t *testing.T) { func TestSyncJobDeleted(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.GroupAndVersion()}) manager := NewJobController(client) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl manager.podStoreSynced = alwaysReady manager.updateHandler = func(job *experimental.Job) error { return nil } @@ -280,18 +241,18 @@ func TestSyncJobDeleted(t *testing.T) { if err != nil { t.Errorf("Unexpected error when syncing jobs %v", err) } - if len(fakePodControl.podSpec) != 0 { - t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", 0, len(fakePodControl.podSpec)) + if len(fakePodControl.Templates) != 0 { + t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", 0, len(fakePodControl.Templates)) } - if len(fakePodControl.deletePodName) != 0 { - t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", 0, len(fakePodControl.deletePodName)) + if len(fakePodControl.DeletePodName) != 0 { + t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", 0, len(fakePodControl.DeletePodName)) } } func TestSyncJobUpdateRequeue(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.GroupAndVersion()}) manager := NewJobController(client) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl manager.podStoreSynced = alwaysReady manager.updateHandler = func(job *experimental.Job) error { return fmt.Errorf("Fake error") } @@ -401,7 +362,7 @@ func (fe FakeJobExpectations) SatisfiedExpectations(controllerKey string) bool { func TestSyncJobExpectations(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.GroupAndVersion()}) manager := NewJobController(client) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl manager.podStoreSynced = alwaysReady manager.updateHandler = func(job *experimental.Job) error { return nil } @@ -420,11 +381,11 @@ func TestSyncJobExpectations(t *testing.T) { }, } manager.syncJob(getKey(job, t)) - if len(fakePodControl.podSpec) != 0 { - t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", 0, len(fakePodControl.podSpec)) + if len(fakePodControl.Templates) != 0 { + t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", 0, len(fakePodControl.Templates)) } - if len(fakePodControl.deletePodName) != 0 { - t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", 0, len(fakePodControl.deletePodName)) + if len(fakePodControl.DeletePodName) != 0 { + t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", 0, len(fakePodControl.DeletePodName)) } } diff --git a/pkg/controller/replication/replication_controller_test.go b/pkg/controller/replication/replication_controller_test.go index a11edc5453..9174688c15 100644 --- a/pkg/controller/replication/replication_controller_test.go +++ b/pkg/controller/replication/replication_controller_test.go @@ -20,7 +20,6 @@ import ( "fmt" "math/rand" "net/http/httptest" - "sync" "testing" "time" @@ -38,49 +37,12 @@ import ( "k8s.io/kubernetes/pkg/watch" ) -type FakePodControl struct { - controllerSpec []api.PodTemplateSpec - deletePodName []string - lock sync.Mutex - err error -} - var alwaysReady = func() bool { return true } func init() { api.ForTesting_ReferencesAllowBlankSelfLinks = true } -func (f *FakePodControl) CreatePods(namespace string, spec *api.PodTemplateSpec, object runtime.Object) error { - f.lock.Lock() - defer f.lock.Unlock() - if f.err != nil { - return f.err - } - f.controllerSpec = append(f.controllerSpec, *spec) - return nil -} - -func (f *FakePodControl) CreatePodsOnNode(nodeName, namespace string, template *api.PodTemplateSpec, object runtime.Object) error { - return nil -} - -func (f *FakePodControl) DeletePod(namespace string, podName string) error { - f.lock.Lock() - defer f.lock.Unlock() - if f.err != nil { - return f.err - } - f.deletePodName = append(f.deletePodName, podName) - return nil -} -func (f *FakePodControl) clear() { - f.lock.Lock() - defer f.lock.Unlock() - f.deletePodName = []string{} - f.controllerSpec = []api.PodTemplateSpec{} -} - func getKey(rc *api.ReplicationController, t *testing.T) string { if key, err := controller.KeyFunc(rc); err != nil { t.Errorf("Unexpected error getting key for rc %v: %v", rc.Name, err) @@ -152,12 +114,12 @@ func newPodList(store cache.Store, count int, status api.PodPhase, rc *api.Repli } } -func validateSyncReplication(t *testing.T, fakePodControl *FakePodControl, expectedCreates, expectedDeletes int) { - if len(fakePodControl.controllerSpec) != expectedCreates { - t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", expectedCreates, len(fakePodControl.controllerSpec)) +func validateSyncReplication(t *testing.T, fakePodControl *controller.FakePodControl, expectedCreates, expectedDeletes int) { + if len(fakePodControl.Templates) != expectedCreates { + t.Errorf("Unexpected number of creates. Expected %d, saw %d\n", expectedCreates, len(fakePodControl.Templates)) } - if len(fakePodControl.deletePodName) != expectedDeletes { - t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", expectedDeletes, len(fakePodControl.deletePodName)) + if len(fakePodControl.DeletePodName) != expectedDeletes { + t.Errorf("Unexpected number of deletes. Expected %d, saw %d\n", expectedDeletes, len(fakePodControl.DeletePodName)) } } @@ -172,7 +134,7 @@ type serverResponse struct { func TestSyncReplicationControllerDoesNothing(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.Version()}) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager := NewReplicationManager(client, BurstReplicas) manager.podStoreSynced = alwaysReady @@ -188,7 +150,7 @@ func TestSyncReplicationControllerDoesNothing(t *testing.T) { func TestSyncReplicationControllerDeletes(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.Version()}) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager := NewReplicationManager(client, BurstReplicas) manager.podStoreSynced = alwaysReady manager.podControl = &fakePodControl @@ -204,7 +166,7 @@ func TestSyncReplicationControllerDeletes(t *testing.T) { func TestDeleteFinalStateUnknown(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.Version()}) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager := NewReplicationManager(client, BurstReplicas) manager.podStoreSynced = alwaysReady manager.podControl = &fakePodControl @@ -241,12 +203,12 @@ func TestSyncReplicationControllerCreates(t *testing.T) { manager.podStoreSynced = alwaysReady // A controller with 2 replicas and no pods in the store, 2 creates expected - controller := newReplicationController(2) - manager.rcStore.Store.Add(controller) + rc := newReplicationController(2) + manager.rcStore.Store.Add(rc) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl - manager.syncReplicationController(getKey(controller, t)) + manager.syncReplicationController(getKey(rc, t)) validateSyncReplication(t, &fakePodControl, 2, 0) } @@ -269,7 +231,7 @@ func TestStatusUpdatesWithoutReplicasChange(t *testing.T) { rc.Status = api.ReplicationControllerStatus{Replicas: activePods} newPodList(manager.podStore.Store, activePods, api.PodRunning, rc) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl manager.syncReplicationController(getKey(rc, t)) @@ -316,7 +278,7 @@ func TestControllerUpdateReplicas(t *testing.T) { response := runtime.EncodeOrDie(testapi.Default.Codec(), &api.ReplicationController{}) fakeHandler.ResponseBody = response - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl manager.syncReplicationController(getKey(rc, t)) @@ -340,7 +302,7 @@ func TestSyncReplicationControllerDormancy(t *testing.T) { defer testServer.Close() client := client.NewOrDie(&client.Config{Host: testServer.URL, Version: testapi.Default.Version()}) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager := NewReplicationManager(client, BurstReplicas) manager.podStoreSynced = alwaysReady manager.podControl = &fakePodControl @@ -356,7 +318,7 @@ func TestSyncReplicationControllerDormancy(t *testing.T) { // Expectations prevents replicas but not an update on status controllerSpec.Status.Replicas = 0 - fakePodControl.clear() + fakePodControl.Clear() manager.syncReplicationController(getKey(controllerSpec, t)) validateSyncReplication(t, &fakePodControl, 0, 0) @@ -370,14 +332,14 @@ func TestSyncReplicationControllerDormancy(t *testing.T) { // fakePodControl error will prevent this, leaving expectations at 0, 0 manager.expectations.CreationObserved(rcKey) controllerSpec.Status.Replicas = 1 - fakePodControl.clear() - fakePodControl.err = fmt.Errorf("Fake Error") + fakePodControl.Clear() + fakePodControl.Err = fmt.Errorf("Fake Error") manager.syncReplicationController(getKey(controllerSpec, t)) validateSyncReplication(t, &fakePodControl, 0, 0) // This replica should not need a Lowering of expectations, since the previous create failed - fakePodControl.err = nil + fakePodControl.Err = nil manager.syncReplicationController(getKey(controllerSpec, t)) validateSyncReplication(t, &fakePodControl, 1, 0) @@ -610,7 +572,7 @@ func TestControllerUpdateRequeue(t *testing.T) { rc.Status = api.ReplicationControllerStatus{Replicas: 2} newPodList(manager.podStore.Store, 1, api.PodRunning, rc) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl manager.syncReplicationController(getKey(rc, t)) @@ -682,7 +644,7 @@ func TestControllerUpdateStatusWithFailure(t *testing.T) { func doTestControllerBurstReplicas(t *testing.T, burstReplicas, numReplicas int) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.Version()}) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager := NewReplicationManager(client, burstReplicas) manager.podStoreSynced = alwaysReady manager.podControl = &fakePodControl @@ -754,7 +716,7 @@ func doTestControllerBurstReplicas(t *testing.T, burstReplicas, numReplicas int) } // Check that the rc didn't take any action for all the above pods - fakePodControl.clear() + fakePodControl.Clear() manager.syncReplicationController(getKey(controllerSpec, t)) validateSyncReplication(t, &fakePodControl, 0, 0) @@ -802,7 +764,7 @@ func (fe FakeRCExpectations) SatisfiedExpectations(controllerKey string) bool { // and checking expectations. func TestRCSyncExpectations(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.Version()}) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager := NewReplicationManager(client, 2) manager.podStoreSynced = alwaysReady manager.podControl = &fakePodControl @@ -833,13 +795,13 @@ func TestDeleteControllerAndExpectations(t *testing.T) { rc := newReplicationController(1) manager.rcStore.Store.Add(rc) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager.podControl = &fakePodControl // This should set expectations for the rc manager.syncReplicationController(getKey(rc, t)) validateSyncReplication(t, &fakePodControl, 1, 0) - fakePodControl.clear() + fakePodControl.Clear() // Get the RC key rcKey, err := controller.KeyFunc(rc) @@ -869,7 +831,7 @@ func TestDeleteControllerAndExpectations(t *testing.T) { func TestRCManagerNotReady(t *testing.T) { client := client.NewOrDie(&client.Config{Host: "", Version: testapi.Default.Version()}) - fakePodControl := FakePodControl{} + fakePodControl := controller.FakePodControl{} manager := NewReplicationManager(client, 2) manager.podControl = &fakePodControl manager.podStoreSynced = func() bool { return false }