Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Exit from NewController() for PersistentVolumeController when InitPlugins() failed #42981

Merged
merged 1 commit into from
Apr 12, 2017
Merged

Exit from NewController() for PersistentVolumeController when InitPlugins() failed #42981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion cmd/kube-controller-manager/app/controllermanager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
23 changes: 16 additions & 7 deletions pkg/controller/volume/persistentvolume/framework_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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{})
Expand All @@ -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)
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/controller/volume/persistentvolume/provision_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions pkg/controller/volume/persistentvolume/pv_controller_base.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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{
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions pkg/controller/volume/persistentvolume/pv_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 4 additions & 1 deletion test/integration/volume/persistent_volumes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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 {
Expand Down