Merge pull request #128455 from jsafrane/refactor-kcm-plugins

Refactor KCM volume plugin probe
kubernetes · Oct 31, 2024 · 365b457 · 365b457
2 parents 7a43ede + 9e29f95
commit 365b457
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 41 deletions.
diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go
@@ -270,7 +270,7 @@ func newPersistentVolumeBinderControllerDescriptor() *ControllerDescriptor {
 
 func startPersistentVolumeBinderController(ctx context.Context, controllerContext ControllerContext, controllerName string) (controller.Interface, bool, error) {
 	logger := klog.FromContext(ctx)
-	plugins, err := ProbeControllerVolumePlugins(logger, controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeConfiguration)
+	plugins, err := ProbeProvisionableRecyclableVolumePlugins(logger, controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeConfiguration)
 	if err != nil {
 		return nil, true, fmt.Errorf("failed to probe volume plugins when starting persistentvolume controller: %v", err)
 	}
@@ -307,7 +307,7 @@ func startPersistentVolumeAttachDetachController(ctx context.Context, controller
 	csiNodeInformer := controllerContext.InformerFactory.Storage().V1().CSINodes()
 	csiDriverInformer := controllerContext.InformerFactory.Storage().V1().CSIDrivers()
 
-	plugins, err := ProbeAttachableVolumePlugins(logger)
+	plugins, err := ProbeAttachableVolumePlugins(logger, controllerContext.ComponentConfig.PersistentVolumeBinderController.VolumeConfiguration)
 	if err != nil {
 		return nil, true, fmt.Errorf("failed to probe volume plugins when starting attach/detach controller: %v", err)
 	}

diff --git a/cmd/kube-controller-manager/app/plugins.go b/cmd/kube-controller-manager/app/plugins.go
@@ -24,14 +24,14 @@ import (
 	"fmt"
 
 	"k8s.io/klog/v2"
+	"k8s.io/kubernetes/pkg/volume/csi"
+	"k8s.io/kubernetes/pkg/volume/iscsi"
 
 	// Volume plugins
 	"k8s.io/kubernetes/pkg/volume"
-	"k8s.io/kubernetes/pkg/volume/csi"
 	"k8s.io/kubernetes/pkg/volume/fc"
 	"k8s.io/kubernetes/pkg/volume/flexvolume"
 	"k8s.io/kubernetes/pkg/volume/hostpath"
-	"k8s.io/kubernetes/pkg/volume/iscsi"
 	"k8s.io/kubernetes/pkg/volume/nfs"
 	volumeutil "k8s.io/kubernetes/pkg/volume/util"
 
@@ -42,19 +42,11 @@ import (
 
 // ProbeAttachableVolumePlugins collects all volume plugins for the attach/
 // detach controller.
-// The list of plugins is manually compiled. This code and the plugin
-// initialization code for kubelet really, really need a through refactor.
-func ProbeAttachableVolumePlugins(logger klog.Logger) ([]volume.VolumePlugin, error) {
-	var err error
-	allPlugins := []volume.VolumePlugin{}
-	allPlugins, err = appendAttachableLegacyProviderVolumes(logger, allPlugins, utilfeature.DefaultFeatureGate)
-	if err != nil {
-		return allPlugins, err
-	}
-	allPlugins = append(allPlugins, fc.ProbeVolumePlugins()...)
-	allPlugins = append(allPlugins, iscsi.ProbeVolumePlugins()...)
-	allPlugins = append(allPlugins, csi.ProbeVolumePlugins()...)
-	return allPlugins, nil
+func ProbeAttachableVolumePlugins(logger klog.Logger, config persistentvolumeconfig.VolumeConfiguration) ([]volume.VolumePlugin, error) {
+	return probeControllerVolumePlugins(logger, config, func(plugin volume.VolumePlugin) bool {
+		_, ok := plugin.(volume.AttachableVolumePlugin)
+		return ok
+	})
 }
 
 // GetDynamicPluginProber gets the probers of dynamically discoverable plugins
@@ -66,21 +58,31 @@ func GetDynamicPluginProber(config persistentvolumeconfig.VolumeConfiguration) v
 
 // ProbeExpandableVolumePlugins returns volume plugins which are expandable
 func ProbeExpandableVolumePlugins(logger klog.Logger, config persistentvolumeconfig.VolumeConfiguration) ([]volume.VolumePlugin, error) {
-	var err error
-	allPlugins := []volume.VolumePlugin{}
-	allPlugins, err = appendExpandableLegacyProviderVolumes(logger, allPlugins, utilfeature.DefaultFeatureGate)
-	if err != nil {
-		return allPlugins, err
-	}
-	allPlugins = append(allPlugins, fc.ProbeVolumePlugins()...)
-	return allPlugins, nil
+	return probeControllerVolumePlugins(logger, config, func(plugin volume.VolumePlugin) bool {
+		_, ok := plugin.(volume.ExpandableVolumePlugin)
+		return ok
+	})
+}
+
+func ProbeProvisionableRecyclableVolumePlugins(logger klog.Logger, config persistentvolumeconfig.VolumeConfiguration) ([]volume.VolumePlugin, error) {
+	return probeControllerVolumePlugins(logger, config, func(plugin volume.VolumePlugin) bool {
+		if _, ok := plugin.(volume.ProvisionableVolumePlugin); ok {
+			return true
+		}
+		if _, ok := plugin.(volume.DeletableVolumePlugin); ok {
+			return true
+		}
+		if _, ok := plugin.(volume.RecyclableVolumePlugin); ok {
+			return true
+		}
+		return false
+	})
 }
 
-// ProbeControllerVolumePlugins collects all persistent volume plugins into an
-// easy to use list. Only volume plugins that implement any of
-// provisioner/recycler/deleter interface should be returned.
-func ProbeControllerVolumePlugins(logger klog.Logger, config persistentvolumeconfig.VolumeConfiguration) ([]volume.VolumePlugin, error) {
-	allPlugins := []volume.VolumePlugin{}
+// probeControllerVolumePlugins collects all persistent volume plugins
+// used by KCM controllers into an easy to use list.
+func probeControllerVolumePlugins(logger klog.Logger, config persistentvolumeconfig.VolumeConfiguration, filter func(plugin volume.VolumePlugin) bool) ([]volume.VolumePlugin, error) {
+	var allPlugins []volume.VolumePlugin
 
 	// The list of plugins to probe is decided by this binary, not
 	// by dynamic linking or other "magic".  Plugins will be analyzed and
@@ -113,14 +115,28 @@ func ProbeControllerVolumePlugins(logger klog.Logger, config persistentvolumecon
 		klog.FlushAndExit(klog.ExitFlushTimeout, 1)
 	}
 	allPlugins = append(allPlugins, nfs.ProbeVolumePlugins(nfsConfig)...)
+	allPlugins = append(allPlugins, fc.ProbeVolumePlugins()...)
+	allPlugins = append(allPlugins, iscsi.ProbeVolumePlugins()...)
+	allPlugins = append(allPlugins, csi.ProbeVolumePlugins()...)
 
 	var err error
-	allPlugins, err = appendExpandableLegacyProviderVolumes(logger, allPlugins, utilfeature.DefaultFeatureGate)
+	allPlugins, err = appendLegacyControllerProviders(logger, allPlugins, utilfeature.DefaultFeatureGate)
 	if err != nil {
 		return allPlugins, err
 	}
 
-	return allPlugins, nil
+	var filteredPlugins []volume.VolumePlugin
+	if filter == nil {
+		filteredPlugins = allPlugins
+	} else {
+		for _, plugin := range allPlugins {
+			if filter(plugin) {
+				filteredPlugins = append(filteredPlugins, plugin)
+			}
+		}
+	}
+
+	return filteredPlugins, nil
 }
 
 // AttemptToLoadRecycler tries decoding a pod from a filepath for use as a recycler for a volume.

diff --git a/cmd/kube-controller-manager/app/plugins_providers.go b/cmd/kube-controller-manager/app/plugins_providers.go
@@ -53,7 +53,7 @@ type pluginInfo struct {
 	pluginProbeFunction     probeFn
 }
 
-func appendAttachableLegacyProviderVolumes(logger klog.Logger, allPlugins []volume.VolumePlugin, featureGate featuregate.FeatureGate) ([]volume.VolumePlugin, error) {
+func appendLegacyControllerProviders(logger klog.Logger, allPlugins []volume.VolumePlugin, featureGate featuregate.FeatureGate) ([]volume.VolumePlugin, error) {
 	pluginMigrationStatus := make(map[string]pluginInfo)
 	pluginMigrationStatus[plugins.PortworxVolumePluginName] = pluginInfo{pluginMigrationFeature: features.CSIMigrationPortworx, pluginUnregisterFeature: features.InTreePluginPortworxUnregister, pluginProbeFunction: portworx.ProbeVolumePlugins}
 	var err error
@@ -65,11 +65,3 @@ func appendAttachableLegacyProviderVolumes(logger klog.Logger, allPlugins []volu
 	}
 	return allPlugins, nil
 }
-
-func appendExpandableLegacyProviderVolumes(logger klog.Logger, allPlugins []volume.VolumePlugin, featureGate featuregate.FeatureGate) ([]volume.VolumePlugin, error) {
-	return appendLegacyProviderVolumes(logger, allPlugins, featureGate)
-}
-
-func appendLegacyProviderVolumes(logger klog.Logger, allPlugins []volume.VolumePlugin, featureGate featuregate.FeatureGate) ([]volume.VolumePlugin, error) {
-	return appendAttachableLegacyProviderVolumes(logger, allPlugins, featureGate)
-}
diff --git a/cmd/kube-controller-manager/app/plugins_test.go b/cmd/kube-controller-manager/app/plugins_test.go
@@ -0,0 +1,83 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package app
+
+import (
+	"reflect"
+	"sort"
+	"testing"
+
+	"k8s.io/klog/v2/ktesting"
+	persistentvolumeconfig "k8s.io/kubernetes/pkg/controller/volume/persistentvolume/config"
+	"k8s.io/kubernetes/pkg/volume"
+)
+
+func checkPlugins(t *testing.T, got []volume.VolumePlugin, expected []string) {
+	pluginNames := make([]string, len(got))
+	for i, p := range got {
+		pluginNames[i] = p.GetPluginName()
+	}
+	sort.Strings(pluginNames)
+	sort.Strings(expected)
+	if !reflect.DeepEqual(pluginNames, expected) {
+		t.Errorf("Expected %+v, got %+v", expected, pluginNames)
+	}
+}
+
+func TestProbeAttachableVolumePlugins(t *testing.T) {
+	logger, _ := ktesting.NewTestContext(t)
+	plugins, err := ProbeAttachableVolumePlugins(logger, getConfig())
+	if err != nil {
+		t.Fatalf("ProbeAttachableVolumePlugins failed: %s", err)
+	}
+	checkPlugins(t, plugins, []string{"kubernetes.io/csi", "kubernetes.io/fc", "kubernetes.io/iscsi"})
+}
+
+func TestProbeExpandableVolumePlugins(t *testing.T) {
+	logger, _ := ktesting.NewTestContext(t)
+	plugins, err := ProbeExpandableVolumePlugins(logger, getConfig())
+	if err != nil {
+		t.Fatalf("TestProbeExpandableVolumePlugins failed: %s", err)
+	}
+	checkPlugins(t, plugins, []string{"kubernetes.io/portworx-volume"})
+}
+
+func TestProbeControllerVolumePlugins(t *testing.T) {
+	logger, _ := ktesting.NewTestContext(t)
+	plugins, err := ProbeProvisionableRecyclableVolumePlugins(logger, getConfig())
+	if err != nil {
+		t.Fatalf("ProbeControllerVolumePlugins failed: %s", err)
+	}
+	checkPlugins(t, plugins, []string{"kubernetes.io/host-path", "kubernetes.io/nfs", "kubernetes.io/portworx-volume"})
+}
+
+func getConfig() persistentvolumeconfig.VolumeConfiguration {
+	return persistentvolumeconfig.VolumeConfiguration{
+		EnableHostPathProvisioning: true,
+		EnableDynamicProvisioning:  true,
+		PersistentVolumeRecyclerConfiguration: persistentvolumeconfig.PersistentVolumeRecyclerConfiguration{
+			MaximumRetry:                5,
+			MinimumTimeoutNFS:           30,
+			PodTemplateFilePathNFS:      "",
+			IncrementTimeoutNFS:         10,
+			PodTemplateFilePathHostPath: "",
+			MinimumTimeoutHostPath:      30,
+			IncrementTimeoutHostPath:    10,
+		},
+		FlexVolumePluginDir: "",
+	}
+}