Merge pull request #106703 from scottnice/patch-1

Fixed skipping valid volume plugins when another plugin has an error

pkg/volume/flexvolume/probe_test.go

@@ -30,8 +30,9 @@ import (
 )
 
 const (
-	pluginDir  = "/flexvolume"
+	pluginDir       = "/flexvolume"
-	driverName = "fake-driver"
+	driverName      = "fake-driver"
+	errorDriverName = "error-driver"
 )
 
 func assertPathSuffix(t *testing.T, dir1 string, dir2 string) {
@@ -185,7 +186,7 @@ func TestEmptyPluginDir(t *testing.T) {
 		pluginDir: pluginDir,
 		watcher:   watcher,
 		fs:        fs,
-		factory:   fakePluginFactory{error: false},
+		factory:   fakePluginFactory{},
 	}
 	prober.Init()
 
@@ -279,7 +280,7 @@ func TestProberError(t *testing.T) {
 		pluginDir: pluginDir,
 		watcher:   watcher,
 		fs:        fs,
-		factory:   fakePluginFactory{error: true},
+		factory:   fakePluginFactory{errorDriver: driverName},
 	}
 	installDriver(driverName, fs)
 	prober.Init()
@@ -288,6 +289,34 @@ func TestProberError(t *testing.T) {
 	assert.Error(t, err)
 }
 
+func TestProberSuccessAndError(t *testing.T) {
+
+	// Arrange
+	fs := utilfs.NewTempFs()
+	watcher := newFakeWatcher()
+	prober := &flexVolumeProber{
+		pluginDir: pluginDir,
+		watcher:   watcher,
+		fs:        fs,
+		factory:   fakePluginFactory{errorDriver: errorDriverName},
+	}
+	installDriver(driverName, fs)
+	prober.Init()
+
+	installDriver(errorDriverName, fs)
+	driverPath := filepath.Join(pluginDir, errorDriverName)
+	watcher.TriggerEvent(fsnotify.Create, filepath.Join(driverPath, errorDriverName))
+
+	// Act
+	events, err := prober.Probe()
+
+	// Assert
+	assert.Equal(t, 1, len(events))
+	assert.Equal(t, volume.ProbeAddOrUpdate, events[0].Op)
+	assert.Equal(t, driverName, events[0].PluginName)
+	assert.Error(t, err)
+}
+
 // Installs a mock driver (an empty file) in the mock fs.
 func installDriver(driverName string, fs utilfs.Filesystem) {
 	driverPath := filepath.Join(pluginDir, driverName)
@@ -307,7 +336,7 @@ func initTestEnvironment(t *testing.T) (
 		pluginDir: pluginDir,
 		watcher:   watcher,
 		fs:        fs,
-		factory:   fakePluginFactory{error: false},
+		factory:   fakePluginFactory{},
 	}
 	driverPath = filepath.Join(pluginDir, driverName)
 	installDriver(driverName, fs)
@@ -320,13 +349,13 @@ func initTestEnvironment(t *testing.T) (
 
 // Fake Flexvolume plugin
 type fakePluginFactory struct {
-	error bool // Indicates whether an error should be returned.
+	errorDriver string // the name of the driver in error
 }
 
 var _ PluginFactory = fakePluginFactory{}
 
 func (m fakePluginFactory) NewFlexVolumePlugin(_, driverName string, _ exec.Interface) (volume.VolumePlugin, error) {
-	if m.error {
+	if driverName == m.errorDriver {
 		return nil, fmt.Errorf("Flexvolume plugin error")
 	}
 	// Dummy Flexvolume plugin. Prober never interacts with the plugin.

pkg/volume/plugins.go

@@ -132,7 +132,7 @@ type NodeResizeOptions struct {
 type DynamicPluginProber interface {
 	Init() error
 
-	// If an error occurs, events are undefined.
+	// aggregates events for successful drivers and errors for failed drivers
 	Probe() (events []ProbeEvent, err error)
 }
 
@@ -746,11 +746,14 @@ func (pm *VolumePluginMgr) logDeprecation(plugin string) {
 // If it is, initialize all probed plugins and replace the cache with them.
 func (pm *VolumePluginMgr) refreshProbedPlugins() {
 	events, err := pm.prober.Probe()
+
 	if err != nil {
 		klog.ErrorS(err, "Error dynamically probing plugins")
-		return // Use cached plugins upon failure.
 	}
 
+	// because the probe function can return a list of valid plugins
+	// even when an error is present we still must add the plugins
+	// or they will be skipped because each event only fires once
 	for _, event := range events {
 		if event.Op == ProbeAddOrUpdate {
 			if err := pm.initProbedPlugin(event.Plugin); err != nil {