From 9684763568b5c8f4412d3d7e754463185ae93439 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Fri, 12 Nov 2021 11:05:32 -0500 Subject: [PATCH] Implement changes for quota calculation for pvcs --- .../core/persistent_volume_claims.go | 57 +++++++++++++---- .../core/persistent_volume_claims_test.go | 63 ++++++++++++++++--- 2 files changed, 100 insertions(+), 20 deletions(-) diff --git a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go index 879c6c75566..cbeb373fd02 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go @@ -159,25 +159,50 @@ func (p *pvcEvaluator) Usage(item runtime.Object) (corev1.ResourceList, error) { result[storageClassClaim] = *(resource.NewQuantity(1, resource.DecimalSI)) } - // charge for storage - if request, found := pvc.Spec.Resources.Requests[corev1.ResourceStorage]; found { - roundedRequest := request.DeepCopy() - if !roundedRequest.RoundUp(0) { - // Ensure storage requests are counted as whole byte values, to pass resourcequota validation. - // See http://issue.k8s.io/94313 - request = roundedRequest - } - - result[corev1.ResourceRequestsStorage] = request + requestedStorage := p.getStorageUsage(pvc) + if requestedStorage != nil { + result[corev1.ResourceRequestsStorage] = *requestedStorage // charge usage to the storage class (if present) if len(storageClassRef) > 0 { storageClassStorage := corev1.ResourceName(storageClassRef + storageClassSuffix + string(corev1.ResourceRequestsStorage)) - result[storageClassStorage] = request + result[storageClassStorage] = *requestedStorage } } + return result, nil } +func (p *pvcEvaluator) getStorageUsage(pvc *corev1.PersistentVolumeClaim) *resource.Quantity { + var result *resource.Quantity + roundUpFunc := func(i *resource.Quantity) *resource.Quantity { + roundedRequest := i.DeepCopy() + if !roundedRequest.RoundUp(0) { + // Ensure storage requests are counted as whole byte values, to pass resourcequota validation. + // See http://issue.k8s.io/94313 + return &roundedRequest + } + return i + } + + if userRequest, ok := pvc.Spec.Resources.Requests[corev1.ResourceStorage]; ok { + result = roundUpFunc(&userRequest) + } + + if utilfeature.DefaultFeatureGate.Enabled(k8sfeatures.RecoverVolumeExpansionFailure) && result != nil { + if len(pvc.Status.AllocatedResources) == 0 { + return result + } + + // if AllocatedResources is set and is greater than user request, we should use it. + if allocatedRequest, ok := pvc.Status.AllocatedResources[corev1.ResourceStorage]; ok { + if allocatedRequest.Cmp(*result) > 0 { + result = roundUpFunc(&allocatedRequest) + } + } + } + return result +} + // UsageStats calculates aggregate usage for the object. func (p *pvcEvaluator) UsageStats(options quota.UsageStatsOptions) (quota.UsageStats, error) { return generic.CalculateUsageStats(options, p.listFuncByNamespace, generic.MatchesNoScopeFunc, p.Usage) @@ -200,3 +225,13 @@ func toExternalPersistentVolumeClaimOrError(obj runtime.Object) (*corev1.Persist } return pvc, nil } + +// RequiresQuotaReplenish enables quota monitoring for PVCs. +func RequiresQuotaReplenish(pvc, oldPVC *corev1.PersistentVolumeClaim) bool { + if utilfeature.DefaultFeatureGate.Enabled(k8sfeatures.RecoverVolumeExpansionFailure) { + if oldPVC.Status.AllocatedResources.Storage() != pvc.Status.AllocatedResources.Storage() { + return true + } + } + return false +} diff --git a/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go b/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go index 244af605921..57a931244df 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go @@ -25,7 +25,11 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" quota "k8s.io/apiserver/pkg/quota/v1" "k8s.io/apiserver/pkg/quota/v1/generic" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core" + "k8s.io/kubernetes/pkg/features" ) func testVolumeClaim(name string, namespace string, spec api.PersistentVolumeClaimSpec) *api.PersistentVolumeClaim { @@ -85,8 +89,9 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { evaluator := NewPersistentVolumeClaimEvaluator(nil) testCases := map[string]struct { - pvc *api.PersistentVolumeClaim - usage corev1.ResourceList + pvc *api.PersistentVolumeClaim + usage corev1.ResourceList + enableRecoverFromExpansion bool }{ "pvc-usage": { pvc: validClaim, @@ -95,6 +100,7 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { corev1.ResourcePersistentVolumeClaims: resource.MustParse("1"), generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), }, + enableRecoverFromExpansion: true, }, "pvc-usage-by-class": { pvc: validClaimByStorageClass, @@ -105,6 +111,7 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { V1ResourceByStorageClass(classGold, corev1.ResourcePersistentVolumeClaims): resource.MustParse("1"), generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), }, + enableRecoverFromExpansion: true, }, "pvc-usage-rounded": { @@ -114,6 +121,7 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { corev1.ResourcePersistentVolumeClaims: resource.MustParse("1"), generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), }, + enableRecoverFromExpansion: true, }, "pvc-usage-by-class-rounded": { pvc: validClaimByStorageClassWithNonIntegerStorage, @@ -124,15 +132,52 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { V1ResourceByStorageClass(classGold, corev1.ResourcePersistentVolumeClaims): resource.MustParse("1"), generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), }, + enableRecoverFromExpansion: true, + }, + "pvc-usage-higher-allocated-resource": { + pvc: getPVCWithAllocatedResource("5G", "10G"), + usage: corev1.ResourceList{ + corev1.ResourceRequestsStorage: resource.MustParse("10G"), + corev1.ResourcePersistentVolumeClaims: resource.MustParse("1"), + generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), + }, + enableRecoverFromExpansion: true, + }, + "pvc-usage-lower-allocated-resource": { + pvc: getPVCWithAllocatedResource("10G", "5G"), + usage: corev1.ResourceList{ + corev1.ResourceRequestsStorage: resource.MustParse("10G"), + corev1.ResourcePersistentVolumeClaims: resource.MustParse("1"), + generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), + }, + enableRecoverFromExpansion: true, }, } for testName, testCase := range testCases { - actual, err := evaluator.Usage(testCase.pvc) - if err != nil { - t.Errorf("%s unexpected error: %v", testName, err) - } - if !quota.Equals(testCase.usage, actual) { - t.Errorf("%s expected:\n%v\n, actual:\n%v", testName, testCase.usage, actual) - } + t.Run(testName, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, testCase.enableRecoverFromExpansion)() + actual, err := evaluator.Usage(testCase.pvc) + if err != nil { + t.Errorf("%s unexpected error: %v", testName, err) + } + if !quota.Equals(testCase.usage, actual) { + t.Errorf("%s expected:\n%v\n, actual:\n%v", testName, testCase.usage, actual) + } + }) + } } + +func getPVCWithAllocatedResource(pvcSize, allocatedSize string) *api.PersistentVolumeClaim { + validPVCWithAllocatedResources := testVolumeClaim("foo", "ns", api.PersistentVolumeClaimSpec{ + Resources: api.ResourceRequirements{ + Requests: api.ResourceList{ + core.ResourceStorage: resource.MustParse(pvcSize), + }, + }, + }) + validPVCWithAllocatedResources.Status.AllocatedResources = api.ResourceList{ + api.ResourceName(api.ResourceStorage): resource.MustParse(allocatedSize), + } + return validPVCWithAllocatedResources +}