github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #124813 from carlory/GenerateBulkVolumeVerifyFunc

Browse Source 
remove BulkVolumeVerifier interface from volume
This commit is contained in:
Kubernetes Prow Robot
2024-05-14 10:01:11 -07:00
committed by GitHub
parent 9f39314eb9 21baa25e1a
commit a6b92838be
23 changed files with 8 additions and 277 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/volume/configmap/configmap.go
View File

@@ -86,10 +86,6 @@ func (plugin *configMapPlugin) SupportsMountOption() bool {
return false
}
func (plugin *configMapPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *configMapPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/csi/csi_plugin.go
View File

@@ -529,10 +529,6 @@ func (p *csiPlugin) SupportsMountOption() bool {
return true
}
func (p *csiPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (p *csiPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
if utilfeature.DefaultFeatureGate.Enabled(features.SELinuxMountReadWriteOncePod) {
driver, err := GetCSIDriverName(spec)

4
pkg/volume/downwardapi/downwardapi.go
View File

@@ -88,10 +88,6 @@ func (plugin *downwardAPIPlugin) SupportsMountOption() bool {
return false
}
func (plugin *downwardAPIPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *downwardAPIPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/emptydir/empty_dir.go
View File

@@ -99,10 +99,6 @@ func (plugin *emptyDirPlugin) SupportsMountOption() bool {
return false
}
func (plugin *emptyDirPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *emptyDirPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/fc/fc.go
View File

@@ -97,10 +97,6 @@ func (plugin *fcPlugin) SupportsMountOption() bool {
return true
}
func (plugin *fcPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *fcPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return true, nil
}

4
pkg/volume/flexvolume/plugin.go
View File

@@ -285,10 +285,6 @@ func (plugin *flexVolumePlugin) unsupported(commands ...string) {
plugin.unsupportedCommands = append(plugin.unsupportedCommands, commands...)
}
func (plugin *flexVolumePlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *flexVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/git_repo/git_repo.go
View File

@@ -85,10 +85,6 @@ func (plugin *gitRepoPlugin) SupportsMountOption() bool {
return false
}
func (plugin *gitRepoPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *gitRepoPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

7
pkg/volume/hostpath/host_path.go
View File

@@ -18,10 +18,11 @@ package hostpath
import (
"fmt"
"k8s.io/klog/v2"
"os"
"regexp"
"k8s.io/klog/v2"
"github.com/opencontainers/selinux/go-selinux"
v1 "k8s.io/api/core/v1"
@@ -107,10 +108,6 @@ func (plugin *hostPathPlugin) SupportsMountOption() bool {
return false
}
func (plugin *hostPathPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *hostPathPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/iscsi/iscsi.go
View File

@@ -90,10 +90,6 @@ func (plugin *iscsiPlugin) SupportsMountOption() bool {
return true
}
func (plugin *iscsiPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *iscsiPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return true, nil
}

4
pkg/volume/local/local.go
View File

@@ -92,10 +92,6 @@ func (plugin *localVolumePlugin) SupportsMountOption() bool {
return true
}
func (plugin *localVolumePlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *localVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/nfs/nfs.go
View File

@@ -101,10 +101,6 @@ func (plugin *nfsPlugin) SupportsMountOption() bool {
return true
}
func (plugin *nfsPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *nfsPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/noop_expandable_plugin.go
View File

@@ -68,10 +68,6 @@ func (n *noopExpandableVolumePluginInstance) SupportsMountOption() bool {
return true
}
func (n *noopExpandableVolumePluginInstance) SupportsBulkVolumeVerification() bool {
return false
}
func (n *noopExpandableVolumePluginInstance) RequiresFSResize() bool {
return true
}

5
pkg/volume/plugins.go
View File

@@ -169,11 +169,6 @@ type VolumePlugin interface {
// user specified mount options will result in error creating persistent volumes
SupportsMountOption() bool
// SupportsBulkVolumeVerification checks if volume plugin type is capable
// of enabling bulk polling of all nodes. This can speed up verification of
// attached volumes by quite a bit, but underlying pluging must support it.
SupportsBulkVolumeVerification() bool
// SupportsSELinuxContextMount returns true if volume plugins supports
// mount -o context=XYZ for a given volume.
SupportsSELinuxContextMount(spec *Spec) (bool, error)

4
pkg/volume/plugins_test.go
View File

@@ -83,10 +83,6 @@ func (plugin *testPlugins) SupportsMountOption() bool {
return false
}
func (plugin *testPlugins) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *testPlugins) SupportsSELinuxContextMount(spec *Spec) (bool, error) {
return false, nil
}

4
pkg/volume/portworx/portworx.go
View File

@@ -228,10 +228,6 @@ func (plugin *portworxVolumePlugin) SupportsMountOption() bool {
return false
}
func (plugin *portworxVolumePlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *portworxVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/projected/projected.go
View File

@@ -103,10 +103,6 @@ func (plugin *projectedPlugin) SupportsMountOption() bool {
return false
}
func (plugin *projectedPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *projectedPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

4
pkg/volume/secret/secret.go
View File

@@ -89,10 +89,6 @@ func (plugin *secretPlugin) SupportsMountOption() bool {
return false
}
func (plugin *secretPlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *secretPlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

12
pkg/volume/testing/testing.go
View File

@@ -290,10 +290,6 @@ func (plugin *FakeVolumePlugin) SupportsMountOption() bool {
return true
}
func (plugin *FakeVolumePlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *FakeVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return plugin.SupportsSELinux, nil
}
@@ -564,10 +560,6 @@ func (f *FakeBasicVolumePlugin) RequiresRemount(spec *volume.Spec) bool {
return f.Plugin.RequiresRemount(spec)
}
func (f *FakeBasicVolumePlugin) SupportsBulkVolumeVerification() bool {
return f.Plugin.SupportsBulkVolumeVerification()
}
func (f *FakeBasicVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return f.Plugin.SupportsSELinuxContextMount(spec)
}
@@ -649,10 +641,6 @@ func (plugin *FakeFileVolumePlugin) SupportsMountOption() bool {
return false
}
func (plugin *FakeFileVolumePlugin) SupportsBulkVolumeVerification() bool {
return false
}
func (plugin *FakeFileVolumePlugin) SupportsSELinuxContextMount(spec *volume.Spec) (bool, error) {
return false, nil
}

10
pkg/volume/util/operationexecutor/fakegenerator.go
View File

@@ -17,9 +17,10 @@ limitations under the License.
package operationexecutor
import (
"k8s.io/klog/v2"
"time"
"k8s.io/klog/v2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/apimachinery/pkg/types"
@@ -95,13 +96,6 @@ func (f *fakeOGCounter) GetCSITranslator() InTreeToCSITranslator {
return csitrans.New()
}
func (f *fakeOGCounter) GenerateBulkVolumeVerifyFunc(
map[types.NodeName][]*volume.Spec,
string,
map[*volume.Spec]v1.UniqueVolumeName, ActualStateOfWorldAttacherUpdater) (volumetypes.GeneratedOperations, error) {
return f.recordFuncCall("GenerateBulkVolumeVerifyFunc"), nil
}
func (f *fakeOGCounter) GenerateExpandVolumeFunc(*v1.PersistentVolumeClaim, *v1.PersistentVolume) (volumetypes.GeneratedOperations, error) {
return f.recordFuncCall("GenerateExpandVolumeFunc"), nil
}

83
pkg/volume/util/operationexecutor/operation_executor.go
View File

@@ -831,89 +831,10 @@ func (oe *operationExecutor) VerifyVolumesAreAttached(
attachedVolumes map[types.NodeName][]AttachedVolume,
actualStateOfWorld ActualStateOfWorldAttacherUpdater) {
// A map of plugin names and nodes on which they exist with volumes they manage
bulkVerifyPluginsByNode := make(map[string]map[types.NodeName][]*volume.Spec)
volumeSpecMapByPlugin := make(map[string]map[*volume.Spec]v1.UniqueVolumeName)
for node, nodeAttachedVolumes := range attachedVolumes {
needIndividualVerifyVolumes := []AttachedVolume{}
for _, volumeAttached := range nodeAttachedVolumes {
if volumeAttached.VolumeSpec == nil {
klog.Errorf("VerifyVolumesAreAttached: nil spec for volume %s", volumeAttached.VolumeName)
continue
}
volumePlugin, err :=
oe.operationGenerator.GetVolumePluginMgr().FindPluginBySpec(volumeAttached.VolumeSpec)
if err != nil {
klog.Errorf(
"VolumesAreAttached.FindPluginBySpec failed for volume %q (spec.Name: %q) on node %q with error: %v",
volumeAttached.VolumeName,
volumeAttached.VolumeSpec.Name(),
volumeAttached.NodeName,
err)
continue
}
if volumePlugin == nil {
// should never happen since FindPluginBySpec always returns error if volumePlugin = nil
klog.Errorf(
"Failed to find volume plugin for volume %q (spec.Name: %q) on node %q",
volumeAttached.VolumeName,
volumeAttached.VolumeSpec.Name(),
volumeAttached.NodeName)
continue
}
pluginName := volumePlugin.GetPluginName()
if volumePlugin.SupportsBulkVolumeVerification() {
pluginNodes, pluginNodesExist := bulkVerifyPluginsByNode[pluginName]
if !pluginNodesExist {
pluginNodes = make(map[types.NodeName][]*volume.Spec)
}
volumeSpecList, nodeExists := pluginNodes[node]
if !nodeExists {
volumeSpecList = []*volume.Spec{}
}
volumeSpecList = append(volumeSpecList, volumeAttached.VolumeSpec)
pluginNodes[node] = volumeSpecList
bulkVerifyPluginsByNode[pluginName] = pluginNodes
volumeSpecMap, mapExists := volumeSpecMapByPlugin[pluginName]
if !mapExists {
volumeSpecMap = make(map[*volume.Spec]v1.UniqueVolumeName)
}
volumeSpecMap[volumeAttached.VolumeSpec] = volumeAttached.VolumeName
volumeSpecMapByPlugin[pluginName] = volumeSpecMap
continue
}
// If node doesn't support Bulk volume polling it is best to poll individually
needIndividualVerifyVolumes = append(needIndividualVerifyVolumes, volumeAttached)
}
nodeError := oe.VerifyVolumesAreAttachedPerNode(needIndividualVerifyVolumes, node, actualStateOfWorld)
nodeError := oe.VerifyVolumesAreAttachedPerNode(nodeAttachedVolumes, node, actualStateOfWorld)
if nodeError != nil {
klog.Errorf("VerifyVolumesAreAttached failed for volumes %v, node %q with error %v", needIndividualVerifyVolumes, node, nodeError)
}
}
for pluginName, pluginNodeVolumes := range bulkVerifyPluginsByNode {
generatedOperations, err := oe.operationGenerator.GenerateBulkVolumeVerifyFunc(
pluginNodeVolumes,
pluginName,
volumeSpecMapByPlugin[pluginName],
actualStateOfWorld)
if err != nil {
klog.Errorf("BulkVerifyVolumes.GenerateBulkVolumeVerifyFunc error bulk verifying volumes for plugin %q with %v", pluginName, err)
}
// Ugly hack to ensure - we don't do parallel bulk polling of same volume plugin
uniquePluginName := v1.UniqueVolumeName(pluginName)
err = oe.pendingOperations.Run(uniquePluginName, "" /* Pod Name */, "" /* nodeName */, generatedOperations)
if err != nil {
klog.Errorf("BulkVerifyVolumes.Run Error bulk volume verification for plugin %q with %v", pluginName, err)
klog.Errorf("VerifyVolumesAreAttached failed for volumes %v, node %q with error %v", nodeAttachedVolumes, node, nodeError)
}
}
}

17
pkg/volume/util/operationexecutor/operation_executor_test.go
View File

@@ -18,11 +18,12 @@ package operationexecutor
import (
"fmt"
"k8s.io/klog/v2"
"strconv"
"testing"
"time"
"k8s.io/klog/v2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -689,20 +690,6 @@ func (fopg *fakeOperationGenerator) GenerateExpandInUseVolumeFunc(volumeToMount
}, nil
}
func (fopg *fakeOperationGenerator) GenerateBulkVolumeVerifyFunc(
pluginNodeVolumes map[types.NodeName][]*volume.Spec,
pluginNane string,
volumeSpecMap map[*volume.Spec]v1.UniqueVolumeName,
actualStateOfWorldAttacherUpdater ActualStateOfWorldAttacherUpdater) (volumetypes.GeneratedOperations, error) {
opFunc := func() volumetypes.OperationContext {
startOperationAndBlock(fopg.ch, fopg.quit)
return volumetypes.NewOperationContext(nil, nil, false)
}
return volumetypes.GeneratedOperations{
OperationFunc: opFunc,
}, nil
}
func (fopg *fakeOperationGenerator) GenerateMapVolumeFunc(waitForAttachTimeout time.Duration, volumeToMount VolumeToMount, actualStateOfWorldMounterUpdater ActualStateOfWorldMounterUpdater) (volumetypes.GeneratedOperations, error) {
opFunc := func() volumetypes.OperationContext {
startOperationAndBlock(fopg.ch, fopg.quit)

83
pkg/volume/util/operationexecutor/operation_generator.go
View File

@@ -150,11 +150,6 @@ type OperationGenerator interface {
// GetCSITranslator returns the CSI Translation Library
GetCSITranslator() InTreeToCSITranslator
GenerateBulkVolumeVerifyFunc(
map[types.NodeName][]*volume.Spec,
string,
map[*volume.Spec]v1.UniqueVolumeName, ActualStateOfWorldAttacherUpdater) (volumetypes.GeneratedOperations, error)
GenerateExpandVolumeFunc(*v1.PersistentVolumeClaim, *v1.PersistentVolume) (volumetypes.GeneratedOperations, error)
GenerateExpandAndRecoverVolumeFunc(*v1.PersistentVolumeClaim, *v1.PersistentVolume, string) (volumetypes.GeneratedOperations, error)
@@ -269,84 +264,6 @@ func (og *operationGenerator) GenerateVolumesAreAttachedFunc(
}, nil
}
func (og *operationGenerator) GenerateBulkVolumeVerifyFunc(
pluginNodeVolumes map[types.NodeName][]*volume.Spec,
pluginName string,
volumeSpecMap map[*volume.Spec]v1.UniqueVolumeName,
actualStateOfWorld ActualStateOfWorldAttacherUpdater) (volumetypes.GeneratedOperations, error) {
// Migration: All inputs already should be translated by caller for this
// function except volumeSpecMap which contains original volume names for
// use with actualStateOfWorld
bulkVolumeVerifyFunc := func() volumetypes.OperationContext {
attachableVolumePlugin, err :=
og.volumePluginMgr.FindAttachablePluginByName(pluginName)
if err != nil || attachableVolumePlugin == nil {
klog.Errorf(
"BulkVerifyVolume.FindAttachablePluginBySpec failed for plugin %q with: %v",
pluginName,
err)
return volumetypes.NewOperationContext(nil, nil, false)
}
volumeAttacher, newAttacherErr := attachableVolumePlugin.NewAttacher()
if newAttacherErr != nil {
klog.Errorf(
"BulkVerifyVolume.NewAttacher failed for getting plugin %q with: %v",
attachableVolumePlugin,
newAttacherErr)
return volumetypes.NewOperationContext(nil, nil, false)
}
bulkVolumeVerifier, ok := volumeAttacher.(volume.BulkVolumeVerifier)
if !ok {
klog.Errorf("BulkVerifyVolume failed to type assert attacher %q", bulkVolumeVerifier)
return volumetypes.NewOperationContext(nil, nil, false)
}
attached, bulkAttachErr := bulkVolumeVerifier.BulkVerifyVolumes(pluginNodeVolumes)
if bulkAttachErr != nil {
klog.Errorf("BulkVerifyVolume.BulkVerifyVolumes Error checking volumes are attached with %v", bulkAttachErr)
return volumetypes.NewOperationContext(nil, nil, false)
}
for nodeName, volumeSpecs := range pluginNodeVolumes {
for _, volumeSpec := range volumeSpecs {
nodeVolumeSpecs, nodeChecked := attached[nodeName]
if !nodeChecked {
klog.V(2).Infof("VerifyVolumesAreAttached.BulkVerifyVolumes failed for node %q and leaving volume %q as attached",
nodeName,
volumeSpec.Name())
continue
}
check := nodeVolumeSpecs[volumeSpec]
if !check {
klog.V(2).Infof("VerifyVolumesAreAttached.BulkVerifyVolumes failed for node %q and volume %q",
nodeName,
volumeSpec.Name())
actualStateOfWorld.MarkVolumeAsDetached(volumeSpecMap[volumeSpec], nodeName)
}
}
}
// It is hard to differentiate migrated status for all volumes for verify_volumes_are_attached
return volumetypes.NewOperationContext(nil, nil, false)
}
return volumetypes.GeneratedOperations{
OperationName: "verify_volumes_are_attached",
OperationFunc: bulkVolumeVerifyFunc,
CompleteFunc: util.OperationCompleteHook(util.GetFullQualifiedPluginNameForVolume(pluginName, nil), "verify_volumes_are_attached"),
EventRecorderFunc: nil, // nil because we do not want to generate event on error
}, nil
}
func (og *operationGenerator) GenerateAttachVolumeFunc(
logger klog.Logger,
volumeToAttach VolumeToAttach,

8
pkg/volume/volume.go
View File

@@ -284,14 +284,6 @@ type DeviceMounter interface {
MountDevice(spec *Spec, devicePath string, deviceMountPath string, deviceMounterArgs DeviceMounterArgs) error
}
type BulkVolumeVerifier interface {
// BulkVerifyVolumes checks whether the list of volumes still attached to the
// clusters in the node. It returns a map which maps from the volume spec to the checking result.
// If an error occurs during check - error should be returned and volume on nodes
// should be assumed as still attached.
BulkVerifyVolumes(volumesByNode map[types.NodeName][]*Spec) (map[types.NodeName]map[*Spec]bool, error)
}
// Detacher can detach a volume from a node.
type Detacher interface {
DeviceUnmounter