Exit from NewController() for PersistentVolumeController when InitPlugins() failed just like NewAttachDetachController() does

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 {

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)
 		}

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)

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

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)
 		}

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 {