Merge pull request kubernetes#123970 from carlory/rm-volumelimit-interface

remove VolumePluginWithAttachLimits interface

Author: k8s-ci-robot

pkg/kubelet/kubelet_node_status.go

@@ -738,8 +738,6 @@ func (kl *Kubelet) defaultNodeStatusFuncs() []func(context.Context, *v1.Node) er
 		nodestatus.GoRuntime(),
 		nodestatus.RuntimeHandlers(kl.runtimeState.runtimeHandlers),
 	)
-	// Volume limits
-	setters = append(setters, nodestatus.VolumeLimits(kl.volumePluginMgr.ListVolumePluginWithLimits))
 
 	setters = append(setters,
 		nodestatus.MemoryPressureCondition(kl.clock.Now, kl.evictionManager.IsUnderMemoryPressure, kl.recordNodeStatusEvent),

pkg/kubelet/nodestatus/setters.go

@@ -43,7 +43,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/cm"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/events"
-	"k8s.io/kubernetes/pkg/volume"
 	netutils "k8s.io/utils/net"
 
 	"k8s.io/klog/v2"
@@ -779,30 +778,3 @@ func VolumesInUse(syncedFunc func() bool, // typically Kubelet.volumeManager.Rec
 		return nil
 	}
 }
-
-// VolumeLimits returns a Setter that updates the volume limits on the node.
-func VolumeLimits(volumePluginListFunc func() []volume.VolumePluginWithAttachLimits, // typically Kubelet.volumePluginMgr.ListVolumePluginWithLimits
-) Setter {
-	return func(ctx context.Context, node *v1.Node) error {
-		if node.Status.Capacity == nil {
-			node.Status.Capacity = v1.ResourceList{}
-		}
-		if node.Status.Allocatable == nil {
-			node.Status.Allocatable = v1.ResourceList{}
-		}
-
-		pluginWithLimits := volumePluginListFunc()
-		for _, volumePlugin := range pluginWithLimits {
-			attachLimits, err := volumePlugin.GetVolumeLimits()
-			if err != nil {
-				klog.V(4).InfoS("Skipping volume limits for volume plugin", "plugin", volumePlugin.GetPluginName())
-				continue
-			}
-			for limitKey, value := range attachLimits {
-				node.Status.Capacity[v1.ResourceName(limitKey)] = *resource.NewQuantity(value, resource.DecimalSI)
-				node.Status.Allocatable[v1.ResourceName(limitKey)] = *resource.NewQuantity(value, resource.DecimalSI)
-			}
-		}
-		return nil
-	}
-}

pkg/kubelet/nodestatus/setters_test.go

@@ -48,8 +48,6 @@ import (
 	kubecontainertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
 	"k8s.io/kubernetes/pkg/kubelet/events"
 	"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
-	"k8s.io/kubernetes/pkg/volume"
-	volumetest "k8s.io/kubernetes/pkg/volume/testing"
 	netutils "k8s.io/utils/net"
 )
 
@@ -2076,70 +2074,6 @@ func TestVolumesInUse(t *testing.T) {
 	}
 }
 
-func TestVolumeLimits(t *testing.T) {
-	const (
-		volumeLimitKey = "attachable-volumes-fake-provider"
-		volumeLimitVal = 16
-	)
-
-	var cases = []struct {
-		desc             string
-		volumePluginList []volume.VolumePluginWithAttachLimits
-		expectNode       *v1.Node
-	}{
-		{
-			desc: "translate limits to capacity and allocatable for plugins that return successfully from GetVolumeLimits",
-			volumePluginList: []volume.VolumePluginWithAttachLimits{
-				&volumetest.FakeVolumePlugin{
-					VolumeLimits: map[string]int64{volumeLimitKey: volumeLimitVal},
-				},
-			},
-			expectNode: &v1.Node{
-				Status: v1.NodeStatus{
-					Capacity: v1.ResourceList{
-						volumeLimitKey: *resource.NewQuantity(volumeLimitVal, resource.DecimalSI),
-					},
-					Allocatable: v1.ResourceList{
-						volumeLimitKey: *resource.NewQuantity(volumeLimitVal, resource.DecimalSI),
-					},
-				},
-			},
-		},
-		{
-			desc: "skip plugins that return errors from GetVolumeLimits",
-			volumePluginList: []volume.VolumePluginWithAttachLimits{
-				&volumetest.FakeVolumePlugin{
-					VolumeLimitsError: fmt.Errorf("foo"),
-				},
-			},
-			expectNode: &v1.Node{},
-		},
-		{
-			desc:       "no plugins",
-			expectNode: &v1.Node{},
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.desc, func(t *testing.T) {
-			ctx := context.Background()
-			volumePluginListFunc := func() []volume.VolumePluginWithAttachLimits {
-				return tc.volumePluginList
-			}
-			// construct setter
-			setter := VolumeLimits(volumePluginListFunc)
-			// call setter on node
-			node := &v1.Node{}
-			if err := setter(ctx, node); err != nil {
-				t.Fatalf("unexpected error: %v", err)
-			}
-			// check expected node
-			assert.True(t, apiequality.Semantic.DeepEqual(tc.expectNode, node),
-				"Diff: %s", cmp.Diff(tc.expectNode, node))
-		})
-	}
-}
-
 func TestDaemonEndpoints(t *testing.T) {
 	for _, test := range []struct {
 		name      string

pkg/volume/plugins.go

@@ -263,32 +263,6 @@ type NodeExpandableVolumePlugin interface {
 	NodeExpand(resizeOptions NodeResizeOptions) (bool, error)
 }
 
-// VolumePluginWithAttachLimits is an extended interface of VolumePlugin that restricts number of
-// volumes that can be attached to a node.
-type VolumePluginWithAttachLimits interface {
-	VolumePlugin
-	// Return maximum number of volumes that can be attached to a node for this plugin.
-	// The key must be same as string returned by VolumeLimitKey function. The returned
-	// map may look like:
-	//     - { "storage-limits-aws-ebs": 39 }
-	//     - { "storage-limits-gce-pd": 10 }
-	// A volume plugin may return error from this function - if it can not be used on a given node or not
-	// applicable in given environment (where environment could be cloudprovider or any other dependency)
-	// For example - calling this function for EBS volume plugin on a GCE node should
-	// result in error.
-	// The returned values are stored in node allocatable property and will be used
-	// by scheduler to determine how many pods with volumes can be scheduled on given node.
-	GetVolumeLimits() (map[string]int64, error)
-	// Return volume limit key string to be used in node capacity constraints
-	// The key must start with prefix storage-limits-. For example:
-	//    - storage-limits-aws-ebs
-	//    - storage-limits-csi-cinder
-	// The key should respect character limit of ResourceName type
-	// This function may be called by kubelet or scheduler to identify node allocatable property
-	// which stores volumes limits.
-	VolumeLimitKey(spec *Spec) string
-}
-
 // BlockVolumePlugin is an extend interface of VolumePlugin and is used for block volumes support.
 type BlockVolumePlugin interface {
 	VolumePlugin
@@ -751,20 +725,6 @@ func (pm *VolumePluginMgr) refreshProbedPlugins() {
 	}
 }
 
-// ListVolumePluginWithLimits returns plugins that have volume limits on nodes
-func (pm *VolumePluginMgr) ListVolumePluginWithLimits() []VolumePluginWithAttachLimits {
-	pm.mutex.RLock()
-	defer pm.mutex.RUnlock()
-
-	matchedPlugins := []VolumePluginWithAttachLimits{}
-	for _, v := range pm.plugins {
-		if plugin, ok := v.(VolumePluginWithAttachLimits); ok {
-			matchedPlugins = append(matchedPlugins, plugin)
-		}
-	}
-	return matchedPlugins
-}
-
 // FindPersistentPluginBySpec looks for a persistent volume plugin that can
 // support a given volume specification.  If no plugin is found, return an
 // error
@@ -779,20 +739,6 @@ func (pm *VolumePluginMgr) FindPersistentPluginBySpec(spec *Spec) (PersistentVol
 	return nil, fmt.Errorf("no persistent volume plugin matched")
 }
 
-// FindVolumePluginWithLimitsBySpec returns volume plugin that has a limit on how many
-// of them can be attached to a node
-func (pm *VolumePluginMgr) FindVolumePluginWithLimitsBySpec(spec *Spec) (VolumePluginWithAttachLimits, error) {
-	volumePlugin, err := pm.FindPluginBySpec(spec)
-	if err != nil {
-		return nil, fmt.Errorf("could not find volume plugin for spec : %#v", spec)
-	}
-
-	if limitedPlugin, ok := volumePlugin.(VolumePluginWithAttachLimits); ok {
-		return limitedPlugin, nil
-	}
-	return nil, fmt.Errorf("no plugin with limits found")
-}
-
 // FindPersistentPluginByName fetches a persistent volume plugin by name.  If
 // no plugin is found, returns error.
 func (pm *VolumePluginMgr) FindPersistentPluginByName(name string) (PersistentVolumePlugin, error) {

pkg/volume/testing/testing.go

@@ -212,7 +212,6 @@ var _ volume.RecyclableVolumePlugin = &FakeVolumePlugin{}
 var _ volume.DeletableVolumePlugin = &FakeVolumePlugin{}
 var _ volume.ProvisionableVolumePlugin = &FakeVolumePlugin{}
 var _ volume.AttachableVolumePlugin = &FakeVolumePlugin{}
-var _ volume.VolumePluginWithAttachLimits = &FakeVolumePlugin{}
 var _ volume.DeviceMountableVolumePlugin = &FakeVolumePlugin{}
 var _ volume.NodeExpandableVolumePlugin = &FakeVolumePlugin{}