Merge pull request kubernetes#30145 from markturansky/limit_range_pvc

Automatic merge from submit-queue

Add PVC storage to LimitRange

This PR adds the ability to add a LimitRange to a namespace that enforces min/max on `pvc.Spec.Resources.Requests["storage"]`.

@derekwaynecarr @abhgupta @kubernetes/sig-storage 

Examples forthcoming. 

```release-note
pvc.Spec.Resources.Requests min and max can be enforced with a LimitRange of type "PersistentVolumeClaim" in the namespace
```

Author: Kubernetes Submit Queue

docs/design/admission_control_limit_range.md

@@ -47,6 +47,7 @@ as part of admission control.
 4. Ability to specify default resource limits for a container
 5. Ability to specify default resource requests for a container
 6. Ability to enforce a ratio between request and limit for a resource.
+7. Ability to enforce min/max storage requests for persistent volume claims
 
 ## Data Model
 
@@ -209,6 +210,23 @@ Across all containers in pod, the following must hold true
 | Max | Limit (required) <= Max |
 | LimitRequestRatio | LimitRequestRatio <= ( Limit (required, non-zero) / Request (non-zero) ) |
 
+**Type: PersistentVolumeClaim**
+
+Supported Resources:
+
+1. storage
+
+Supported Constraints:
+
+Across all claims in a namespace, the following must hold true:
+
+| Constraint | Behavior |
+| ---------- | -------- |
+| Min | Min >= Request (required) |
+| Max | Max <= Request (required) |
+
+Supported Defaults: None. Storage is a required field in `PersistentVolumeClaim`, so defaults are not applied at this time.
+
 ## Run-time configuration
 
 The default ```LimitRange``` that is applied via Salt configuration will be

pkg/api/helpers.go

@@ -123,6 +123,7 @@ func IsStandardContainerResourceName(str string) bool {
 var standardLimitRangeTypes = sets.NewString(
 	string(LimitTypePod),
 	string(LimitTypeContainer),
+	string(LimitTypePersistentVolumeClaim),
 )
 
 // IsStandardLimitRangeType returns true if the type is Pod or Container

pkg/api/types.go

@@ -2684,6 +2684,8 @@ const (
 	LimitTypePod LimitType = "Pod"
 	// Limit that applies to all containers in a namespace
 	LimitTypeContainer LimitType = "Container"
+	// Limit that applies to all persistent volume claims in a namespace
+	LimitTypePersistentVolumeClaim LimitType = "PersistentVolumeClaim"
 )
 
 // LimitRangeItem defines a min/max usage limit for any resource that matches on kind

pkg/api/v1/types.go

@@ -3138,6 +3138,8 @@ const (
 	LimitTypePod LimitType = "Pod"
 	// Limit that applies to all containers in a namespace
 	LimitTypeContainer LimitType = "Container"
+	// Limit that applies to all persistent volume claims in a namespace
+	LimitTypePersistentVolumeClaim LimitType = "PersistentVolumeClaim"
 )
 
 // LimitRangeItem defines a min/max usage limit for any resource that matches on kind.

pkg/api/validation/validation.go

@@ -2942,6 +2942,17 @@ func ValidateLimitRange(limitRange *api.LimitRange) field.ErrorList {
 			}
 		}
 
+		if limit.Type == api.LimitTypePersistentVolumeClaim {
+			_, minQuantityFound := limit.Min[api.ResourceStorage]
+			_, maxQuantityFound := limit.Max[api.ResourceStorage]
+			if !minQuantityFound {
+				allErrs = append(allErrs, field.Required(idxPath.Child("min"), "minimum storage value is required"))
+			}
+			if !maxQuantityFound {
+				allErrs = append(allErrs, field.Required(idxPath.Child("max"), "maximum storage value is required"))
+			}
+		}
+
 		for k, q := range limit.MaxLimitRequestRatio {
 			allErrs = append(allErrs, validateLimitRangeResourceName(limit.Type, string(k), idxPath.Child("maxLimitRequestRatio").Key(string(k)))...)
 			keys.Insert(string(k))

pkg/api/validation/validation_test.go

@@ -6542,6 +6542,11 @@ func TestValidateLimitRange(t *testing.T) {
 						DefaultRequest:       getResourceList("10m", "200Mi"),
 						MaxLimitRequestRatio: getResourceList("10", ""),
 					},
+					{
+						Type: api.LimitTypePersistentVolumeClaim,
+						Max:  getStorageResourceList("10Gi"),
+						Min:  getStorageResourceList("5Gi"),
+					},
 				},
 			},
 		},
@@ -6752,6 +6757,40 @@ func TestValidateLimitRange(t *testing.T) {
 			}},
 			"must be a standard limit type or fully qualified",
 		},
+		"invalid missing required min field": {
+			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: api.LimitRangeSpec{
+				Limits: []api.LimitRangeItem{
+					{
+						Type: api.LimitTypePersistentVolumeClaim,
+						Max:  getStorageResourceList("10000T"),
+					},
+				},
+			}},
+			"minimum storage value is required",
+		},
+		"invalid missing required max field": {
+			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: api.LimitRangeSpec{
+				Limits: []api.LimitRangeItem{
+					{
+						Type: api.LimitTypePersistentVolumeClaim,
+						Min:  getStorageResourceList("10000T"),
+					},
+				},
+			}},
+			"maximum storage value is required",
+		},
+		"invalid min greater than max": {
+			api.LimitRange{ObjectMeta: api.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: api.LimitRangeSpec{
+				Limits: []api.LimitRangeItem{
+					{
+						Type: api.LimitTypePersistentVolumeClaim,
+						Min:  getStorageResourceList("10Gi"),
+						Max:  getStorageResourceList("1Gi"),
+					},
+				},
+			}},
+			"min value 10Gi is greater than max value 1Gi",
+		},
 	}
 
 	for k, v := range errorCases {

plugin/pkg/admission/limitranger/admission.go

@@ -397,25 +397,55 @@ func (d *DefaultLimitRangerActions) Limit(limitRange *api.LimitRange, resourceNa
 	switch resourceName {
 	case "pods":
 		return PodLimitFunc(limitRange, obj.(*api.Pod))
+	case "persistentvolumeclaims":
+		return PersistentVolumeClaimLimitFunc(limitRange, obj.(*api.PersistentVolumeClaim))
 	}
 	return nil
 }
 
-// SupportsAttributes ignores all calls that do not deal with pod resources since that is
-// all this supports now.  Also ignores any call that has a subresource defined.
+// SupportsAttributes ignores all calls that do not deal with pod resources or storage requests (PVCs).
+// Also ignores any call that has a subresource defined.
 func (d *DefaultLimitRangerActions) SupportsAttributes(a admission.Attributes) bool {
 	if a.GetSubresource() != "" {
 		return false
 	}
 
-	return a.GetKind().GroupKind() == api.Kind("Pod")
+	return a.GetKind().GroupKind() == api.Kind("Pod") || a.GetKind().GroupKind() == api.Kind("PersistentVolumeClaim")
 }
 
 // SupportsLimit always returns true.
 func (d *DefaultLimitRangerActions) SupportsLimit(limitRange *api.LimitRange) bool {
 	return true
 }
 
+// PersistentVolumeClaimLimitFunc enforces storage limits for PVCs.
+// Users request storage via pvc.Spec.Resources.Requests.  Min/Max is enforced by an admin with LimitRange.
+// Claims will not be modified with default values because storage is a required part of pvc.Spec.
+// All storage enforced values *only* apply to pvc.Spec.Resources.Requests.
+func PersistentVolumeClaimLimitFunc(limitRange *api.LimitRange, pvc *api.PersistentVolumeClaim) error {
+	var errs []error
+	for i := range limitRange.Spec.Limits {
+		limit := limitRange.Spec.Limits[i]
+		limitType := limit.Type
+		if limitType == api.LimitTypePersistentVolumeClaim {
+			for k, v := range limit.Min {
+				// normal usage of minConstraint. pvc.Spec.Resources.Limits is not recognized as user input
+				if err := minConstraint(limitType, k, v, pvc.Spec.Resources.Requests, api.ResourceList{}); err != nil {
+					errs = append(errs, err)
+				}
+			}
+			for k, v := range limit.Max {
+				// reverse usage of maxConstraint. We want to enforce the max of the LimitRange against what
+				// the user requested.
+				if err := maxConstraint(limitType, k, v, api.ResourceList{}, pvc.Spec.Resources.Requests); err != nil {
+					errs = append(errs, err)
+				}
+			}
+		}
+	}
+	return utilerrors.NewAggregate(errs)
+}
+
 // PodLimitFunc enforces resource requirements enumerated by the pod against
 // the specified LimitRange.  The pod may be modified to apply default resource
 // requirements if not specified, and enumerated on the LimitRange