From 1fa8877c33fc0a4a9744e81ad7634a329c3be899 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 30 Oct 2024 17:47:26 +0100 Subject: [PATCH 1/4] Add unit tests for KCM volume plugin probers --- .../app/plugins_test.go | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 cmd/kube-controller-manager/app/plugins_test.go diff --git a/cmd/kube-controller-manager/app/plugins_test.go b/cmd/kube-controller-manager/app/plugins_test.go new file mode 100644 index 00000000000..9b35aa76a67 --- /dev/null +++ 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) + if err != nil { + t.Fatalf("ProbeAttachableVolumePlugins failed: %s", err) + } + checkPlugins(t, plugins, []string{"kubernetes.io/csi", "kubernetes.io/fc", "kubernetes.io/iscsi", "kubernetes.io/portworx-volume"}) +} + +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/fc", "kubernetes.io/portworx-volume"}) +} + +func TestProbeControllerVolumePlugins(t *testing.T) { + logger, _ := ktesting.NewTestContext(t) + plugins, err := ProbeControllerVolumePlugins(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: "", + } +} From 0ecbdf3622088ad423b1103d5f3c39fb5a52ad1a Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 30 Oct 2024 18:08:26 +0100 Subject: [PATCH 2/4] Remove fc from expandable plugins FibreChannel volume plugin does not implement ExpandableVolumePlugin. --- cmd/kube-controller-manager/app/plugins.go | 1 - cmd/kube-controller-manager/app/plugins_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/kube-controller-manager/app/plugins.go b/cmd/kube-controller-manager/app/plugins.go index 2ae5fc0375b..f87adc251d5 100644 --- a/cmd/kube-controller-manager/app/plugins.go +++ b/cmd/kube-controller-manager/app/plugins.go @@ -72,7 +72,6 @@ func ProbeExpandableVolumePlugins(logger klog.Logger, config persistentvolumecon if err != nil { return allPlugins, err } - allPlugins = append(allPlugins, fc.ProbeVolumePlugins()...) return allPlugins, nil } diff --git a/cmd/kube-controller-manager/app/plugins_test.go b/cmd/kube-controller-manager/app/plugins_test.go index 9b35aa76a67..83f3ba6ee63 100644 --- a/cmd/kube-controller-manager/app/plugins_test.go +++ b/cmd/kube-controller-manager/app/plugins_test.go @@ -53,7 +53,7 @@ func TestProbeExpandableVolumePlugins(t *testing.T) { if err != nil { t.Fatalf("TestProbeExpandableVolumePlugins failed: %s", err) } - checkPlugins(t, plugins, []string{"kubernetes.io/fc", "kubernetes.io/portworx-volume"}) + checkPlugins(t, plugins, []string{"kubernetes.io/portworx-volume"}) } func TestProbeControllerVolumePlugins(t *testing.T) { From cba5a93468043d6956b3826a460b5fb90d0401e1 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 30 Oct 2024 18:10:25 +0100 Subject: [PATCH 3/4] Remove portworx from attachable volume plugins The volume plugin does not implement AttachableVolumePlugin interface. --- cmd/kube-controller-manager/app/plugins.go | 5 ----- cmd/kube-controller-manager/app/plugins_providers.go | 10 +--------- cmd/kube-controller-manager/app/plugins_test.go | 2 +- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/cmd/kube-controller-manager/app/plugins.go b/cmd/kube-controller-manager/app/plugins.go index f87adc251d5..b9f5fe132cf 100644 --- a/cmd/kube-controller-manager/app/plugins.go +++ b/cmd/kube-controller-manager/app/plugins.go @@ -45,12 +45,7 @@ import ( // 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()...) diff --git a/cmd/kube-controller-manager/app/plugins_providers.go b/cmd/kube-controller-manager/app/plugins_providers.go index 587a02dc9bf..cd0f1a1a8d4 100644 --- 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 appendExpandableLegacyProviderVolumes(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 index 83f3ba6ee63..63c4abebade 100644 --- a/cmd/kube-controller-manager/app/plugins_test.go +++ b/cmd/kube-controller-manager/app/plugins_test.go @@ -44,7 +44,7 @@ func TestProbeAttachableVolumePlugins(t *testing.T) { if err != nil { t.Fatalf("ProbeAttachableVolumePlugins failed: %s", err) } - checkPlugins(t, plugins, []string{"kubernetes.io/csi", "kubernetes.io/fc", "kubernetes.io/iscsi", "kubernetes.io/portworx-volume"}) + checkPlugins(t, plugins, []string{"kubernetes.io/csi", "kubernetes.io/fc", "kubernetes.io/iscsi"}) } func TestProbeExpandableVolumePlugins(t *testing.T) { From 9e29f95618c879c02a7d0a0febf2ec389f9b2c08 Mon Sep 17 00:00:00 2001 From: Jan Safranek Date: Wed, 30 Oct 2024 17:23:02 +0100 Subject: [PATCH 4/4] Refactor controller-manager volume plugins Most of the volume plugins were removed from k/k. Refactor how KCM controllers initialize the few leftovers. --- cmd/kube-controller-manager/app/core.go | 4 +- cmd/kube-controller-manager/app/plugins.go | 70 ++++++++++++------- .../app/plugins_providers.go | 2 +- .../app/plugins_test.go | 4 +- 4 files changed, 51 insertions(+), 29 deletions(-) diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index fa8367f58f8..a2b0660627c 100644 --- 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 index b9f5fe132cf..cf87b5b6e6b 100644 --- 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,14 +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) { - allPlugins := []volume.VolumePlugin{} - 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 @@ -61,20 +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 - } - return allPlugins, nil + return probeControllerVolumePlugins(logger, config, func(plugin volume.VolumePlugin) bool { + _, ok := plugin.(volume.ExpandableVolumePlugin) + return ok + }) } -// 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{} +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 +// 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 @@ -107,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 index cd0f1a1a8d4..9293c48f79d 100644 --- 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 appendExpandableLegacyProviderVolumes(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 diff --git a/cmd/kube-controller-manager/app/plugins_test.go b/cmd/kube-controller-manager/app/plugins_test.go index 63c4abebade..219a96d8d14 100644 --- a/cmd/kube-controller-manager/app/plugins_test.go +++ b/cmd/kube-controller-manager/app/plugins_test.go @@ -40,7 +40,7 @@ func checkPlugins(t *testing.T, got []volume.VolumePlugin, expected []string) { func TestProbeAttachableVolumePlugins(t *testing.T) { logger, _ := ktesting.NewTestContext(t) - plugins, err := ProbeAttachableVolumePlugins(logger) + plugins, err := ProbeAttachableVolumePlugins(logger, getConfig()) if err != nil { t.Fatalf("ProbeAttachableVolumePlugins failed: %s", err) } @@ -58,7 +58,7 @@ func TestProbeExpandableVolumePlugins(t *testing.T) { func TestProbeControllerVolumePlugins(t *testing.T) { logger, _ := ktesting.NewTestContext(t) - plugins, err := ProbeControllerVolumePlugins(logger, getConfig()) + plugins, err := ProbeProvisionableRecyclableVolumePlugins(logger, getConfig()) if err != nil { t.Fatalf("ProbeControllerVolumePlugins failed: %s", err) }