From e0ef5bfd4044f533f11a2047f5879acf2adbcb55 Mon Sep 17 00:00:00 2001 From: NickrenREN Date: Mon, 13 Mar 2017 15:41:59 +0800 Subject: [PATCH] Exit from NewController() for PersistentVolumeController when InitPlugins() failed just like NewAttachDetachController() does --- .../app/controllermanager.go | 5 +++- .../volume/persistentvolume/framework_test.go | 23 +++++++++++++------ .../volume/persistentvolume/provision_test.go | 5 +++- .../persistentvolume/pv_controller_base.go | 8 ++++--- .../persistentvolume/pv_controller_test.go | 7 ++++-- .../volume/persistent_volumes_test.go | 5 +++- 6 files changed, 38 insertions(+), 15 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 3c019608aa7..13e0456acbe 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -542,7 +542,10 @@ func StartControllers(controllers map[string]InitFunc, s *options.CMServer, root ClassInformer: sharedInformers.Storage().V1beta1().StorageClasses(), EnableDynamicProvisioning: s.VolumeConfiguration.EnableDynamicProvisioning, } - volumeController := persistentvolumecontroller.NewController(params) + volumeController, volumeControllerErr := persistentvolumecontroller.NewController(params) + if volumeControllerErr != nil { + return fmt.Errorf("failed to construct persistentvolume controller: %v", volumeControllerErr) + } go volumeController.Run(stop) time.Sleep(wait.Jitter(s.ControllerStartInterval.Duration, ControllerStartJitter)) } else { diff --git a/pkg/controller/volume/persistentvolume/framework_test.go b/pkg/controller/volume/persistentvolume/framework_test.go index ec744b5ddf7..2f2e51e5b32 100644 --- a/pkg/controller/volume/persistentvolume/framework_test.go +++ b/pkg/controller/volume/persistentvolume/framework_test.go @@ -596,7 +596,7 @@ func newVolumeReactor(client *fake.Clientset, ctrl *PersistentVolumeController, } func alwaysReady() bool { return true } -func newTestController(kubeClient clientset.Interface, informerFactory informers.SharedInformerFactory, enableDynamicProvisioning bool) *PersistentVolumeController { +func newTestController(kubeClient clientset.Interface, informerFactory informers.SharedInformerFactory, enableDynamicProvisioning bool) (*PersistentVolumeController, error) { if informerFactory == nil { informerFactory = informers.NewSharedInformerFactory(kubeClient, controller.NoResyncPeriodFunc()) } @@ -610,13 +610,16 @@ func newTestController(kubeClient clientset.Interface, informerFactory informers EventRecorder: record.NewFakeRecorder(1000), EnableDynamicProvisioning: enableDynamicProvisioning, } - ctrl := NewController(params) + ctrl, err := NewController(params) + if err != nil { + return nil, fmt.Errorf("failed to construct persistentvolume controller: %v", err) + } ctrl.volumeListerSynced = alwaysReady ctrl.claimListerSynced = alwaysReady ctrl.classListerSynced = alwaysReady // Speed up the test ctrl.createProvisionedPVInterval = 5 * time.Millisecond - return ctrl + return ctrl, nil } // newVolume returns a new volume with given attributes @@ -912,7 +915,10 @@ func runSyncTests(t *testing.T, tests []controllerTest, storageClasses []*storag // Initialize the controller client := &fake.Clientset{} - ctrl := newTestController(client, nil, true) + ctrl, err := newTestController(client, nil, true) + if err != nil { + t.Fatalf("Test %q construct persistent volume failed: %v", test.name, err) + } reactor := newVolumeReactor(client, ctrl, nil, nil, test.errors) for _, claim := range test.initialClaims { ctrl.claims.Add(claim) @@ -931,7 +937,7 @@ func runSyncTests(t *testing.T, tests []controllerTest, storageClasses []*storag ctrl.classLister = storagelisters.NewStorageClassLister(indexer) // Run the tested functions - err := test.test(ctrl, reactor, test) + err = test.test(ctrl, reactor, test) if err != nil { t.Errorf("Test %q failed: %v", test.name, err) } @@ -966,7 +972,10 @@ func runMultisyncTests(t *testing.T, tests []controllerTest, storageClasses []*s // Initialize the controller client := &fake.Clientset{} - ctrl := newTestController(client, nil, true) + ctrl, err := newTestController(client, nil, true) + if err != nil { + t.Fatalf("Test %q construct persistent volume failed: %v", test.name, err) + } // Inject classes into controller via a custom lister. indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{}) @@ -986,7 +995,7 @@ func runMultisyncTests(t *testing.T, tests []controllerTest, storageClasses []*s } // Run the tested function - err := test.test(ctrl, reactor, test) + err = test.test(ctrl, reactor, test) if err != nil { t.Errorf("Test %q failed: %v", test.name, err) } diff --git a/pkg/controller/volume/persistentvolume/provision_test.go b/pkg/controller/volume/persistentvolume/provision_test.go index 7e2a7c6d346..0f4fbb8f640 100644 --- a/pkg/controller/volume/persistentvolume/provision_test.go +++ b/pkg/controller/volume/persistentvolume/provision_test.go @@ -391,7 +391,10 @@ func TestProvisionMultiSync(t *testing.T) { // When provisioning is disabled, provisioning a claim should instantly return nil func TestDisablingDynamicProvisioner(t *testing.T) { - ctrl := newTestController(nil, nil, false) + ctrl, err := newTestController(nil, nil, false) + if err != nil { + t.Fatalf("Construct PersistentVolume controller failed: %v", err) + } retVal := ctrl.provisionClaim(nil) if retVal != nil { t.Errorf("Expected nil return but got %v", retVal) diff --git a/pkg/controller/volume/persistentvolume/pv_controller_base.go b/pkg/controller/volume/persistentvolume/pv_controller_base.go index 626847bd413..0296798deb7 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_base.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_base.go @@ -67,7 +67,7 @@ type ControllerParameters struct { } // NewController creates a new PersistentVolume controller -func NewController(p ControllerParameters) *PersistentVolumeController { +func NewController(p ControllerParameters) (*PersistentVolumeController, error) { eventRecorder := p.EventRecorder if eventRecorder == nil { broadcaster := record.NewBroadcaster() @@ -90,7 +90,9 @@ func NewController(p ControllerParameters) *PersistentVolumeController { volumeQueue: workqueue.NewNamed("volumes"), } - controller.volumePluginMgr.InitPlugins(p.VolumePlugins, controller) + if err := controller.volumePluginMgr.InitPlugins(p.VolumePlugins, controller); err != nil { + return nil, fmt.Errorf("Could not initialize volume plugins for PersistentVolume Controller: %v", err) + } p.VolumeInformer.Informer().AddEventHandlerWithResyncPeriod( cache.ResourceEventHandlerFuncs{ @@ -116,7 +118,7 @@ func NewController(p ControllerParameters) *PersistentVolumeController { controller.classLister = p.ClassInformer.Lister() controller.classListerSynced = p.ClassInformer.Informer().HasSynced - return controller + return controller, nil } // initializeCaches fills all controller caches with initial data from etcd in diff --git a/pkg/controller/volume/persistentvolume/pv_controller_test.go b/pkg/controller/volume/persistentvolume/pv_controller_test.go index caa29acfbbf..0c645001777 100644 --- a/pkg/controller/volume/persistentvolume/pv_controller_test.go +++ b/pkg/controller/volume/persistentvolume/pv_controller_test.go @@ -172,7 +172,10 @@ func TestControllerSync(t *testing.T) { client.PrependWatchReactor("storageclasses", core.DefaultWatchReactor(watch.NewFake(), nil)) informers := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) - ctrl := newTestController(client, informers, true) + ctrl, err := newTestController(client, informers, true) + if err != nil { + t.Fatalf("Test %q construct persistent volume failed: %v", test.name, err) + } reactor := newVolumeReactor(client, ctrl, fakeVolumeWatch, fakeClaimWatch, test.errors) for _, claim := range test.initialClaims { @@ -204,7 +207,7 @@ func TestControllerSync(t *testing.T) { glog.V(4).Infof("controller synced, starting test") // Call the tested function - err := test.test(ctrl, reactor, test) + err = test.test(ctrl, reactor, test) if err != nil { t.Errorf("Test %q initial test call failed: %v", test.name, err) } diff --git a/test/integration/volume/persistent_volumes_test.go b/test/integration/volume/persistent_volumes_test.go index de76459c305..170410ed840 100644 --- a/test/integration/volume/persistent_volumes_test.go +++ b/test/integration/volume/persistent_volumes_test.go @@ -1127,7 +1127,7 @@ func createClients(ns *v1.Namespace, t *testing.T, s *httptest.Server, syncPerio plugins := []volume.VolumePlugin{plugin} cloud := &fakecloud.FakeCloud{} informers := informers.NewSharedInformerFactory(testClient, getSyncPeriod(syncPeriod)) - ctrl := persistentvolumecontroller.NewController( + ctrl, err := persistentvolumecontroller.NewController( persistentvolumecontroller.ControllerParameters{ KubeClient: binderClient, SyncPeriod: getSyncPeriod(syncPeriod), @@ -1138,6 +1138,9 @@ func createClients(ns *v1.Namespace, t *testing.T, s *httptest.Server, syncPerio ClassInformer: informers.Storage().V1beta1().StorageClasses(), EnableDynamicProvisioning: true, }) + if err != nil { + t.Fatalf("Failed to construct PersistentVolumes: %v", err) + } watchPV, err := testClient.PersistentVolumes().Watch(metav1.ListOptions{}) if err != nil {