diff --git a/pkg/apis/storage/types.go b/pkg/apis/storage/types.go index 5f0345d44f3..9a77746a2be 100644 --- a/pkg/apis/storage/types.go +++ b/pkg/apis/storage/types.go @@ -355,6 +355,20 @@ type CSINodeDriver struct { // This can be empty if driver does not support topology. // +optional TopologyKeys []string + + // allocatable represents the volume resources of a node that are available for scheduling. + // +optional + Allocatable *VolumeNodeResources +} + +// VolumeNodeResources is a set of resource limits for scheduling of volumes. +type VolumeNodeResources struct { + // Maximum number of unique volumes managed by the CSI driver that can be used on a node. + // A volume that is both attached and mounted on a node is considered to be used once, not twice. + // The same rule applies for a unique volume that is shared among multiple pods on the same node. + // If this field is nil, then the supported number of volumes on this node is unbounded. + // +optional + Count *int32 } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go index a48b4d87208..1947ca7eb39 100644 --- a/pkg/apis/storage/validation/validation.go +++ b/pkg/apis/storage/validation/validation.go @@ -351,12 +351,25 @@ func validateCSINodeDriverNodeID(nodeID string, fldPath *field.Path) field.Error return allErrs } +// validateCSINodeDriverAllocatable tests if Allocatable in CSINodeDriver has valid volume limits. +func validateCSINodeDriverAllocatable(a *storage.VolumeNodeResources, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if a == nil || a.Count == nil { + return allErrs + } + + allErrs = append(allErrs, apivalidation.ValidateNonnegativeField(int64(*a.Count), fldPath.Child("count"))...) + return allErrs +} + // validateCSINodeDriver tests if CSINodeDriver has valid entries func validateCSINodeDriver(driver storage.CSINodeDriver, driverNamesInSpecs sets.String, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, apivalidation.ValidateCSIDriverName(driver.Name, fldPath.Child("name"))...) allErrs = append(allErrs, validateCSINodeDriverNodeID(driver.NodeID, fldPath.Child("nodeID"))...) + allErrs = append(allErrs, validateCSINodeDriverAllocatable(driver.Allocatable, fldPath.Child("allocatable"))...) // check for duplicate entries for the same driver in specs if driverNamesInSpecs.Has(driver.Name) { diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go index 74e6eddba73..5a9ffd19fd2 100644 --- a/pkg/apis/storage/validation/validation_test.go +++ b/pkg/apis/storage/validation/validation_test.go @@ -28,6 +28,7 @@ import ( api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/features" + utilpointer "k8s.io/utils/pointer" ) var ( @@ -1152,6 +1153,34 @@ func TestCSINodeValidation(t *testing.T) { }, }, }, + { + // Volume limits being zero + ObjectMeta: metav1.ObjectMeta{Name: "foo11"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "io.kubernetes.storage.csi.driver", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(0)}, + }, + }, + }, + }, + { + // Volume limits with positive number + ObjectMeta: metav1.ObjectMeta{Name: "foo11"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "io.kubernetes.storage.csi.driver", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(1)}, + }, + }, + }, + }, { // topology key names with -, _, and dot . ObjectMeta: metav1.ObjectMeta{Name: "foo8"}, @@ -1368,6 +1397,20 @@ func TestCSINodeValidation(t *testing.T) { }, }, }, + { + // Volume limits with negative number + ObjectMeta: metav1.ObjectMeta{Name: "foo11"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "io.kubernetes.storage.csi.driver", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(-1)}, + }, + }, + }, + }, { // topology prefix should be lower case ObjectMeta: metav1.ObjectMeta{Name: "foo14"}, @@ -1409,6 +1452,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { Name: "io.kubernetes.storage.csi.driver-2", NodeID: nodeID, TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(20)}, }, }, }, @@ -1429,6 +1473,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { Name: "io.kubernetes.storage.csi.driver-2", NodeID: nodeID, TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(20)}, }, }, }, @@ -1460,11 +1505,13 @@ func TestCSINodeUpdateValidation(t *testing.T) { Name: "io.kubernetes.storage.csi.driver-2", NodeID: nodeID, TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(20)}, }, { Name: "io.kubernetes.storage.csi.driver-3", NodeID: nodeID, TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(30)}, }, }, }, @@ -1483,6 +1530,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { Name: "io.kubernetes.storage.csi.new-driver", NodeID: nodeID, TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(30)}, }, }, }, @@ -1510,6 +1558,7 @@ func TestCSINodeUpdateValidation(t *testing.T) { Name: "io.kubernetes.storage.csi.driver-2", NodeID: nodeID, TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(20)}, }, }, }, @@ -1521,13 +1570,90 @@ func TestCSINodeUpdateValidation(t *testing.T) { Drivers: []storage.CSINodeDriver{ { Name: "io.kubernetes.storage.csi.driver-1", - NodeID: "nodeB", + NodeID: nodeID, TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, }, { Name: "io.kubernetes.storage.csi.driver-2", NodeID: nodeID, TopologyKeys: []string{"company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(20)}, + }, + }, + }, + }, + { + // invalid change trying to set a previously unset allocatable + ObjectMeta: metav1.ObjectMeta{Name: "foo1"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "io.kubernetes.storage.csi.driver-1", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(10)}, + }, + { + Name: "io.kubernetes.storage.csi.driver-2", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(20)}, + }, + }, + }, + }, + { + // invalid change trying to update allocatable with a different volume limit + ObjectMeta: metav1.ObjectMeta{Name: "foo1"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "io.kubernetes.storage.csi.driver-1", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + }, + { + Name: "io.kubernetes.storage.csi.driver-2", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(21)}, + }, + }, + }, + }, + { + // invalid change trying to update allocatable with an empty volume limit + ObjectMeta: metav1.ObjectMeta{Name: "foo1"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "io.kubernetes.storage.csi.driver-1", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + }, + { + Name: "io.kubernetes.storage.csi.driver-2", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: nil}, + }, + }, + }, + }, + { + // invalid change trying to remove allocatable + ObjectMeta: metav1.ObjectMeta{Name: "foo1"}, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "io.kubernetes.storage.csi.driver-1", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + }, + { + Name: "io.kubernetes.storage.csi.driver-2", + NodeID: nodeID, + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, }, }, }, diff --git a/pkg/registry/storage/csinode/strategy.go b/pkg/registry/storage/csinode/strategy.go index f20e6e57b0e..160f6d5f566 100644 --- a/pkg/registry/storage/csinode/strategy.go +++ b/pkg/registry/storage/csinode/strategy.go @@ -22,9 +22,11 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/apis/storage" "k8s.io/kubernetes/pkg/apis/storage/validation" + "k8s.io/kubernetes/pkg/features" ) // csiNodeStrategy implements behavior for CSINode objects @@ -41,8 +43,14 @@ func (csiNodeStrategy) NamespaceScoped() bool { return false } -// ResetBeforeCreate clears the Status field which is not allowed to be set by end users on creation. +// PrepareForCreate clears fields that are not allowed to be set on creation. func (csiNodeStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { + csiNode := obj.(*storage.CSINode) + if !utilfeature.DefaultFeatureGate.Enabled(features.AttachVolumeLimit) { + for i := range csiNode.Spec.Drivers { + csiNode.Spec.Drivers[i].Allocatable = nil + } + } } func (csiNodeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { @@ -62,8 +70,33 @@ func (csiNodeStrategy) AllowCreateOnUpdate() bool { return false } -// PrepareForUpdate sets the Status fields which is not allowed to be set by an end user updating a CSINode +// PrepareForUpdate sets the driver's Allocatable fields that are not allowed to be set by an end user updating a CSINode. func (csiNodeStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { + newCSINode := obj.(*storage.CSINode) + oldCSINode := old.(*storage.CSINode) + + inUse := getAllocatablesInUse(oldCSINode) + + if !utilfeature.DefaultFeatureGate.Enabled(features.AttachVolumeLimit) { + for i := range newCSINode.Spec.Drivers { + if !inUse[newCSINode.Spec.Drivers[i].Name] { + newCSINode.Spec.Drivers[i].Allocatable = nil + } + } + } +} + +func getAllocatablesInUse(obj *storage.CSINode) map[string]bool { + inUse := make(map[string]bool) + if obj == nil { + return inUse + } + for i := range obj.Spec.Drivers { + if obj.Spec.Drivers[i].Allocatable != nil { + inUse[obj.Spec.Drivers[i].Name] = true + } + } + return inUse } func (csiNodeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { diff --git a/pkg/registry/storage/csinode/strategy_test.go b/pkg/registry/storage/csinode/strategy_test.go index 14d04f6c153..07dee6b73b0 100644 --- a/pkg/registry/storage/csinode/strategy_test.go +++ b/pkg/registry/storage/csinode/strategy_test.go @@ -17,18 +17,24 @@ limitations under the License. package csinode import ( + "reflect" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/apis/storage" + "k8s.io/kubernetes/pkg/features" + utilpointer "k8s.io/utils/pointer" ) -func getValidCSINode(name string) *storage.CSINode { - return &storage.CSINode{ +func TestPrepareForCreate(t *testing.T) { + valid := getValidCSINode("foo") + emptyAllocatable := &storage.CSINode{ ObjectMeta: metav1.ObjectMeta{ - Name: name, + Name: "foo", }, Spec: storage.CSINodeSpec{ Drivers: []storage.CSINodeDriver{ @@ -40,6 +46,171 @@ func getValidCSINode(name string) *storage.CSINode { }, }, } + + volumeLimitsEnabledCases := []struct { + name string + obj *storage.CSINode + expected *storage.CSINode + }{ + { + "empty allocatable", + emptyAllocatable, + emptyAllocatable, + }, + { + "valid allocatable", + valid, + valid, + }, + } + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, true)() + for _, test := range volumeLimitsEnabledCases { + t.Run(test.name, func(t *testing.T) { + testPrepareForCreate(t, test.obj, test.expected) + }) + } + + volumeLimitsDisabledCases := []struct { + name string + obj *storage.CSINode + expected *storage.CSINode + }{ + { + "empty allocatable", + emptyAllocatable, + emptyAllocatable, + }, + { + "drop allocatable", + valid, + emptyAllocatable, + }, + } + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, false)() + for _, test := range volumeLimitsDisabledCases { + t.Run(test.name, func(t *testing.T) { + testPrepareForCreate(t, test.obj, test.expected) + }) + } +} + +func testPrepareForCreate(t *testing.T, obj, expected *storage.CSINode) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ + APIGroup: "storage.k8s.io", + APIVersion: "v1beta1", + Resource: "csinodes", + }) + Strategy.PrepareForCreate(ctx, obj) + if !reflect.DeepEqual(*expected, *obj) { + t.Errorf("Object mismatch! Expected:\n%#v\ngot:\n%#v", *expected, *obj) + } +} + +func TestPrepareForUpdate(t *testing.T) { + valid := getValidCSINode("foo") + differentAllocatable := &storage.CSINode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "valid-driver-name", + NodeID: "valid-node", + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(20)}, + }, + }, + }, + } + emptyAllocatable := &storage.CSINode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "valid-driver-name", + NodeID: "valid-node", + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + }, + }, + }, + } + + volumeLimitsEnabledCases := []struct { + name string + old *storage.CSINode + new *storage.CSINode + expected *storage.CSINode + }{ + { + "allow empty allocatable when it's not set", + emptyAllocatable, + emptyAllocatable, + emptyAllocatable, + }, + { + "allow valid allocatable when it's already set", + valid, + differentAllocatable, + differentAllocatable, + }, + { + "allow valid allocatable when it's not set", + emptyAllocatable, + valid, + valid, + }, + } + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, true)() + for _, test := range volumeLimitsEnabledCases { + t.Run(test.name, func(t *testing.T) { + testPrepareForUpdate(t, test.new, test.old, test.expected) + }) + } + + volumeLimitsDisabledCases := []struct { + name string + old *storage.CSINode + new *storage.CSINode + expected *storage.CSINode + }{ + { + "allow empty allocatable when it's not set", + emptyAllocatable, + emptyAllocatable, + emptyAllocatable, + }, + { + "drop allocatable when it's not set", + emptyAllocatable, + valid, + emptyAllocatable, + }, + } + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.AttachVolumeLimit, false)() + for _, test := range volumeLimitsDisabledCases { + t.Run(test.name, func(t *testing.T) { + testPrepareForUpdate(t, test.new, test.old, test.expected) + }) + } +} + +func testPrepareForUpdate(t *testing.T, obj, old, expected *storage.CSINode) { + ctx := genericapirequest.WithRequestInfo(genericapirequest.NewContext(), &genericapirequest.RequestInfo{ + APIGroup: "storage.k8s.io", + APIVersion: "v1beta1", + Resource: "csinodes", + }) + Strategy.PrepareForUpdate(ctx, obj, old) + if !reflect.DeepEqual(*expected, *obj) { + t.Errorf("Object mismatch! Expected:\n%#v\ngot:\n%#v", *expected, *obj) + } } func TestCSINodeStrategy(t *testing.T) { @@ -87,6 +258,43 @@ func TestCSINodeValidation(t *testing.T) { getValidCSINode("foo"), false, }, + { + "valid csinode with empty allocatable", + &storage.CSINode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "valid-driver-name", + NodeID: "valid-node", + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + }, + }, + }, + }, + false, + }, + { + "valid csinode with missing volume limits", + &storage.CSINode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "valid-driver-name", + NodeID: "valid-node", + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: nil}, + }, + }, + }, + }, + false, + }, { "invalid driver name", &storage.CSINode{ @@ -99,6 +307,7 @@ func TestCSINodeValidation(t *testing.T) { Name: "$csi-driver@", NodeID: "valid-node", TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(10)}, }, }, }, @@ -117,6 +326,26 @@ func TestCSINodeValidation(t *testing.T) { Name: "valid-driver-name", NodeID: "", TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(10)}, + }, + }, + }, + }, + true, + }, + { + "invalid allocatable with negative volumes limit", + &storage.CSINode{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "valid-driver-name", + NodeID: "valid-node", + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(-1)}, }, }, }, @@ -135,6 +364,7 @@ func TestCSINodeValidation(t *testing.T) { Name: "valid-driver-name", NodeID: "valid-node", TopologyKeys: []string{"company.com/zone1", ""}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(10)}, }, }, }, @@ -165,3 +395,21 @@ func TestCSINodeValidation(t *testing.T) { }) } } + +func getValidCSINode(name string) *storage.CSINode { + return &storage.CSINode{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + Spec: storage.CSINodeSpec{ + Drivers: []storage.CSINodeDriver{ + { + Name: "valid-driver-name", + NodeID: "valid-node", + TopologyKeys: []string{"company.com/zone1", "company.com/zone2"}, + Allocatable: &storage.VolumeNodeResources{Count: utilpointer.Int32Ptr(10)}, + }, + }, + }, + } +} diff --git a/staging/src/k8s.io/api/storage/v1beta1/types.go b/staging/src/k8s.io/api/storage/v1beta1/types.go index cca50d82095..762fcfcd001 100644 --- a/staging/src/k8s.io/api/storage/v1beta1/types.go +++ b/staging/src/k8s.io/api/storage/v1beta1/types.go @@ -17,7 +17,7 @@ limitations under the License. package v1beta1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -357,6 +357,20 @@ type CSINodeDriver struct { // This can be empty if driver does not support topology. // +optional TopologyKeys []string `json:"topologyKeys" protobuf:"bytes,3,rep,name=topologyKeys"` + + // allocatable represents the volume resources of a node that are available for scheduling. + // +optional + Allocatable *VolumeNodeResources `json:"allocatable,omitempty" protobuf:"bytes,4,opt,name=allocatable"` +} + +// VolumeNodeResources is a set of resource limits for scheduling of volumes. +type VolumeNodeResources struct { + // Maximum number of unique volumes managed by the CSI driver that can be used on a node. + // A volume that is both attached and mounted on a node is considered to be used once, not twice. + // The same rule applies for a unique volume that is shared among multiple pods on the same node. + // If this field is nil, then the supported number of volumes on this node is unbounded. + // +optional + Count *int32 `json:"count,omitempty" protobuf:"varint,1,opt,name=count"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object