From 608fa1d7a9301aaa12175c3028ff203e58634519 Mon Sep 17 00:00:00 2001 From: carlory Date: Wed, 15 Nov 2023 13:59:40 +0800 Subject: [PATCH 1/3] Add quota support for PVC with VolumeAttributesClass --- .../core/persistent_volume_claims.go | 35 +++++++++- .../core/persistent_volume_claims_test.go | 67 +++++++++++++++++-- 2 files changed, 95 insertions(+), 7 deletions(-) diff --git a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go index 059dd6d154412..1728a7f620a5a 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go @@ -64,6 +64,24 @@ func V1ResourceByStorageClass(storageClass string, resourceName corev1.ResourceN return corev1.ResourceName(string(storageClass + storageClassSuffix + string(resourceName))) } +// pvcVACResources are the set of static resources managed by quota associated with pvcs. +// for each resource in this list, it may be refined dynamically based on volume attributes class. +var pvcVACResources = []corev1.ResourceName{ + corev1.ResourcePersistentVolumeClaims, +} + +// volumeAttributesClassSuffix is the suffix to the qualified portion of volume attributes class resource name. +// For example, if you want to quota storage by a volume attributes class, you would have a declaration +// that follows .volumeattributesclass.storage.k8s.io/. +// For example: +// * bronze.volumeattributesclass.storage.k8s.io/persistentvolumeclaims: "5" +const volumeAttributesClassSuffix string = ".volumeattributesclass.storage.k8s.io/" + +// V1ResourceByVolumeAttributesClass returns a quota resource name by a volume attributes class. +func V1ResourceByVolumeAttributesClass(vacName string, resourceName corev1.ResourceName) corev1.ResourceName { + return corev1.ResourceName(string(vacName + volumeAttributesClassSuffix + string(resourceName))) +} + // NewPersistentVolumeClaimEvaluator returns an evaluator that can evaluate persistent volume claims func NewPersistentVolumeClaimEvaluator(f quota.ListerForResourceFunc) quota.Evaluator { listFuncByNamespace := generic.ListResourceUsingListerFunc(f, corev1.SchemeGroupVersion.WithResource("persistentvolumeclaims")) @@ -126,7 +144,7 @@ func (p *pvcEvaluator) MatchingResources(items []corev1.ResourceName) []corev1.R continue } // match pvc resources - if quota.Contains(pvcResources, item) { + if quota.Contains(pvcResources, item) || quota.Contains(pvcVACResources, item) { result = append(result, item) continue } @@ -138,6 +156,16 @@ func (p *pvcEvaluator) MatchingResources(items []corev1.ResourceName) []corev1.R break } } + // match pvc resources scoped by volume attributes class (.volumeattributesclass.storage.k8s.io/) + if utilfeature.DefaultFeatureGate.Enabled(k8sfeatures.VolumeAttributesClass) { + for _, resource := range pvcVACResources { + byVolumeAttributesClass := volumeAttributesClassSuffix + string(resource) + if strings.HasSuffix(string(item), byVolumeAttributesClass) { + result = append(result, item) + break + } + } + } } return result } @@ -169,6 +197,11 @@ func (p *pvcEvaluator) Usage(item runtime.Object) (corev1.ResourceList, error) { } } + if utilfeature.DefaultFeatureGate.Enabled(k8sfeatures.VolumeAttributesClass) && pvc.Spec.VolumeAttributesClassName != nil { + volumeAttributesClassClaim := corev1.ResourceName(*pvc.Spec.VolumeAttributesClassName + volumeAttributesClassSuffix + string(corev1.ResourcePersistentVolumeClaims)) + result[volumeAttributesClassClaim] = *(resource.NewQuantity(1, resource.DecimalSI)) + } + return result, nil } 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 187c1c78660e2..875bba3a1caaf 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go @@ -87,11 +87,15 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { validClaimByStorageClassWithNonIntegerStorage := validClaimByStorageClass.DeepCopy() validClaimByStorageClassWithNonIntegerStorage.Spec.Resources.Requests[core.ResourceName(core.ResourceStorage)] = resource.MustParse("1001m") + validClaimByVolumeAttributesClass := validClaimByStorageClass.DeepCopy() + validClaimByVolumeAttributesClass.Spec.VolumeAttributesClassName = &classGold + evaluator := NewPersistentVolumeClaimEvaluator(nil) testCases := map[string]struct { - pvc *core.PersistentVolumeClaim - usage corev1.ResourceList - enableRecoverFromExpansion bool + pvc *core.PersistentVolumeClaim + usage corev1.ResourceList + enableRecoverFromExpansion bool + enableVolumeAttributesClass bool }{ "pvc-usage": { pvc: validClaim, @@ -152,10 +156,23 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { }, enableRecoverFromExpansion: true, }, + "pvc-usage-by-volume-attributes-class": { + pvc: validClaimByVolumeAttributesClass, + usage: corev1.ResourceList{ + corev1.ResourceRequestsStorage: resource.MustParse("10Gi"), + corev1.ResourcePersistentVolumeClaims: resource.MustParse("1"), + V1ResourceByStorageClass(classGold, corev1.ResourceRequestsStorage): resource.MustParse("10Gi"), + V1ResourceByStorageClass(classGold, corev1.ResourcePersistentVolumeClaims): resource.MustParse("1"), + V1ResourceByVolumeAttributesClass(classGold, corev1.ResourcePersistentVolumeClaims): resource.MustParse("1"), + generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), + }, + enableVolumeAttributesClass: true, + }, } for testName, testCase := range testCases { t.Run(testName, func(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.RecoverVolumeExpansionFailure, testCase.enableRecoverFromExpansion)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeAttributesClass, testCase.enableVolumeAttributesClass)() actual, err := evaluator.Usage(testCase.pvc) if err != nil { t.Errorf("%s unexpected error: %v", testName, err) @@ -185,8 +202,9 @@ func getPVCWithAllocatedResource(pvcSize, allocatedSize string) *core.Persistent func TestPersistentVolumeClaimEvaluatorMatchingResources(t *testing.T) { evaluator := NewPersistentVolumeClaimEvaluator(nil) testCases := map[string]struct { - items []corev1.ResourceName - want []corev1.ResourceName + items []corev1.ResourceName + want []corev1.ResourceName + enableVolumeAttributesClass bool }{ "supported-resources": { items: []corev1.ResourceName{ @@ -195,6 +213,7 @@ func TestPersistentVolumeClaimEvaluatorMatchingResources(t *testing.T) { "persistentvolumeclaims", "gold.storageclass.storage.k8s.io/requests.storage", "gold.storageclass.storage.k8s.io/persistentvolumeclaims", + "gold.volumeattributesclass.storage.k8s.io/persistentvolumeclaims", }, want: []corev1.ResourceName{ @@ -203,7 +222,9 @@ func TestPersistentVolumeClaimEvaluatorMatchingResources(t *testing.T) { "persistentvolumeclaims", "gold.storageclass.storage.k8s.io/requests.storage", "gold.storageclass.storage.k8s.io/persistentvolumeclaims", + "gold.volumeattributesclass.storage.k8s.io/persistentvolumeclaims", }, + enableVolumeAttributesClass: true, }, "unsupported-resources": { items: []corev1.ResourceName{ @@ -214,10 +235,44 @@ func TestPersistentVolumeClaimEvaluatorMatchingResources(t *testing.T) { }, want: []corev1.ResourceName{}, }, + // TODO: remove this test case after featuregate VolumeAttributesClass is GAed + "supported-resources-on-featuregate-off": { + items: []corev1.ResourceName{ + "count/persistentvolumeclaims", + "requests.storage", + "persistentvolumeclaims", + "gold.storageclass.storage.k8s.io/requests.storage", + "gold.storageclass.storage.k8s.io/persistentvolumeclaims", + "gold.volumeattributesclass.storage.k8s.io/persistentvolumeclaims", + }, + + want: []corev1.ResourceName{ + "count/persistentvolumeclaims", + "requests.storage", + "persistentvolumeclaims", + "gold.storageclass.storage.k8s.io/requests.storage", + "gold.storageclass.storage.k8s.io/persistentvolumeclaims", + }, + }, + // TODO: remove this test case after featuregate VolumeAttributesClass is GAed + "unsupported-resources-on-featuregate-on": { + items: []corev1.ResourceName{ + "storage", + "ephemeral-storage", + "bronze.storageclass.storage.k8s.io/storage", + "gold.storage.k8s.io/requests.storage", + "gold.volumeattributesclass.storage.k8s.io/persistentvolumeclaims", + }, + want: []corev1.ResourceName{ + "gold.volumeattributesclass.storage.k8s.io/persistentvolumeclaims", + }, + enableVolumeAttributesClass: true, + }, } for testName, testCase := range testCases { - actual := evaluator.MatchingResources(testCase.items) + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.VolumeAttributesClass, testCase.enableVolumeAttributesClass)() + actual := evaluator.MatchingResources(testCase.items) if !reflect.DeepEqual(testCase.want, actual) { t.Errorf("%s expected:\n%v\n, actual:\n%v", testName, testCase.want, actual) } From 22b351ba77b9a184a4147f231351bba35ab5cf22 Mon Sep 17 00:00:00 2001 From: carlory Date: Thu, 21 Dec 2023 13:07:42 +0800 Subject: [PATCH 2/3] add e2e test --- .../core/persistent_volume_claims.go | 6 +- .../core/persistent_volume_claims_test.go | 15 ++++ test/e2e/apimachinery/resource_quota.go | 73 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go index 1728a7f620a5a..e28a1b22d742f 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go @@ -32,6 +32,7 @@ import ( api "k8s.io/kubernetes/pkg/apis/core" k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1" k8sfeatures "k8s.io/kubernetes/pkg/features" + "k8s.io/utils/ptr" ) // the name used for object count quota @@ -197,8 +198,9 @@ func (p *pvcEvaluator) Usage(item runtime.Object) (corev1.ResourceList, error) { } } - if utilfeature.DefaultFeatureGate.Enabled(k8sfeatures.VolumeAttributesClass) && pvc.Spec.VolumeAttributesClassName != nil { - volumeAttributesClassClaim := corev1.ResourceName(*pvc.Spec.VolumeAttributesClassName + volumeAttributesClassSuffix + string(corev1.ResourcePersistentVolumeClaims)) + vacName := ptr.Deref(pvc.Spec.VolumeAttributesClassName, "") + if utilfeature.DefaultFeatureGate.Enabled(k8sfeatures.VolumeAttributesClass) && vacName != "" { + volumeAttributesClassClaim := corev1.ResourceName(vacName + volumeAttributesClassSuffix + string(corev1.ResourcePersistentVolumeClaims)) result[volumeAttributesClassClaim] = *(resource.NewQuantity(1, resource.DecimalSI)) } 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 875bba3a1caaf..1fa50f0d91535 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go @@ -30,6 +30,7 @@ import ( featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/features" + "k8s.io/utils/ptr" ) func testVolumeClaim(name string, namespace string, spec core.PersistentVolumeClaimSpec) *core.PersistentVolumeClaim { @@ -90,6 +91,9 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { validClaimByVolumeAttributesClass := validClaimByStorageClass.DeepCopy() validClaimByVolumeAttributesClass.Spec.VolumeAttributesClassName = &classGold + validClaimByEmptyVolumeAttributesClass := validClaimByStorageClass.DeepCopy() + validClaimByEmptyVolumeAttributesClass.Spec.VolumeAttributesClassName = ptr.To("") + evaluator := NewPersistentVolumeClaimEvaluator(nil) testCases := map[string]struct { pvc *core.PersistentVolumeClaim @@ -156,6 +160,17 @@ func TestPersistentVolumeClaimEvaluatorUsage(t *testing.T) { }, enableRecoverFromExpansion: true, }, + "pvc-usage-by-empty-volume-attributes-class": { + pvc: validClaimByEmptyVolumeAttributesClass, + usage: corev1.ResourceList{ + corev1.ResourceRequestsStorage: resource.MustParse("10Gi"), + corev1.ResourcePersistentVolumeClaims: resource.MustParse("1"), + V1ResourceByStorageClass(classGold, corev1.ResourceRequestsStorage): resource.MustParse("10Gi"), + V1ResourceByStorageClass(classGold, corev1.ResourcePersistentVolumeClaims): resource.MustParse("1"), + generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "persistentvolumeclaims"}): resource.MustParse("1"), + }, + enableVolumeAttributesClass: true, + }, "pvc-usage-by-volume-attributes-class": { pvc: validClaimByVolumeAttributesClass, usage: corev1.ResourceList{ diff --git a/test/e2e/apimachinery/resource_quota.go b/test/e2e/apimachinery/resource_quota.go index d02bb2ae70623..a534d75cf64b5 100644 --- a/test/e2e/apimachinery/resource_quota.go +++ b/test/e2e/apimachinery/resource_quota.go @@ -45,6 +45,7 @@ import ( "k8s.io/client-go/tools/cache" watchtools "k8s.io/client-go/tools/watch" "k8s.io/client-go/util/retry" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/quota/v1/evaluator/core" "k8s.io/kubernetes/test/e2e/feature" "k8s.io/kubernetes/test/e2e/framework" @@ -52,6 +53,7 @@ import ( imageutils "k8s.io/kubernetes/test/utils/image" admissionapi "k8s.io/pod-security-admission/api" "k8s.io/utils/pointer" + "k8s.io/utils/ptr" "github.com/onsi/ginkgo/v2" "github.com/onsi/gomega" @@ -1219,6 +1221,77 @@ var _ = SIGDescribe("ResourceQuota", func() { }) }) +var _ = SIGDescribe("ResourceQuota", framework.WithFeatureGate(features.VolumeAttributesClass), func() { + f := framework.NewDefaultFramework("volume-attributes-class") + f.NamespacePodSecurityLevel = admissionapi.LevelBaseline + + /* + Release: v1.30 + Testname: ResourceQuota, object count quota, volumeAttributesClass + Description: Create a ResourceQuota. Creation MUST be successful and its ResourceQuotaStatus MUST match to expected used and total allowed resource quota count within namespace. + Create PersistentVolumeClaim (PVC) with specified volumeAttributesClass and storage capacity of 1G. PVC creation MUST be successful and resource usage count against PVC, volumeAttributesClass and storage object MUST be captured in ResourceQuotaStatus of the ResourceQuota. + Create another PVC with specified volumeAttributesClass and storage capacity of 1G. PVC creation MUST fail due to exceeding the quota. + Delete the PVC. Deletion MUST succeed and resource usage count against PVC, volumeAttributesClass and storage object MUST be released from ResourceQuotaStatus of the ResourceQuota. + */ + ginkgo.It("should create a ResourceQuota and capture the life of a persistent volume claim with a volume attributes class", func(ctx context.Context) { + ginkgo.By("Counting existing ResourceQuota") + c, err := countResourceQuota(ctx, f.ClientSet, f.Namespace.Name) + framework.ExpectNoError(err) + + ginkgo.By("Creating a ResourceQuota") + quotaName := "test-quota" + resourceQuota := newTestResourceQuota(quotaName) + resourceQuota.Spec.Hard[core.V1ResourceByVolumeAttributesClass(classGold, v1.ResourcePersistentVolumeClaims)] = resource.MustParse("1") + _, err = createResourceQuota(ctx, f.ClientSet, f.Namespace.Name, resourceQuota) + framework.ExpectNoError(err) + + ginkgo.By("Ensuring resource quota status is calculated") + usedResources := v1.ResourceList{} + usedResources[v1.ResourceQuotas] = resource.MustParse(strconv.Itoa(c + 1)) + usedResources[v1.ResourcePersistentVolumeClaims] = resource.MustParse("0") + usedResources[v1.ResourceRequestsStorage] = resource.MustParse("0") + usedResources[core.V1ResourceByVolumeAttributesClass(classGold, v1.ResourcePersistentVolumeClaims)] = resource.MustParse("0") + + err = waitForResourceQuota(ctx, f.ClientSet, f.Namespace.Name, quotaName, usedResources) + framework.ExpectNoError(err) + + ginkgo.By("Creating a PersistentVolumeClaim with volume attributes class") + pvc1 := newTestPersistentVolumeClaimForQuota("test-claim-1") + pvc1.Spec.StorageClassName = ptr.To("") + pvc1.Spec.VolumeAttributesClassName = &classGold + pvc1, err = f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Create(ctx, pvc1, metav1.CreateOptions{}) + framework.ExpectNoError(err) + + ginkgo.By("Ensuring resource quota status captures persistent volume claim creation") + usedResources = v1.ResourceList{} + usedResources[v1.ResourcePersistentVolumeClaims] = resource.MustParse("1") + usedResources[v1.ResourceRequestsStorage] = resource.MustParse("1Gi") + usedResources[core.V1ResourceByVolumeAttributesClass(classGold, v1.ResourcePersistentVolumeClaims)] = resource.MustParse("1") + + err = waitForResourceQuota(ctx, f.ClientSet, f.Namespace.Name, quotaName, usedResources) + framework.ExpectNoError(err) + + ginkgo.By("Not allowing a PersistentVolumeClaim to be created that exceeds remaining quota") + pvc2 := newTestPersistentVolumeClaimForQuota("test-claim-2") + pvc2.Spec.StorageClassName = ptr.To("") + pvc2.Spec.VolumeAttributesClassName = &classGold + _, err = f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Create(ctx, pvc2, metav1.CreateOptions{}) + gomega.Expect(err).To(gomega.MatchError(apierrors.IsForbidden, "expect a forbidden error when creating a PVC that exceeds quota")) + + ginkgo.By("Deleting a PersistentVolumeClaim") + err = f.ClientSet.CoreV1().PersistentVolumeClaims(f.Namespace.Name).Delete(ctx, pvc1.Name, metav1.DeleteOptions{}) + framework.ExpectNoError(err) + + ginkgo.By("Ensuring resource quota status released usage") + usedResources[v1.ResourcePersistentVolumeClaims] = resource.MustParse("0") + usedResources[v1.ResourceRequestsStorage] = resource.MustParse("0") + usedResources[core.V1ResourceByVolumeAttributesClass(classGold, v1.ResourcePersistentVolumeClaims)] = resource.MustParse("0") + + err = waitForResourceQuota(ctx, f.ClientSet, f.Namespace.Name, quotaName, usedResources) + framework.ExpectNoError(err) + }) +}) + var _ = SIGDescribe("ResourceQuota", feature.ScopeSelectors, func() { f := framework.NewDefaultFramework("scope-selectors") f.NamespacePodSecurityLevel = admissionapi.LevelBaseline From 921a9aa396e69a34a4309772abcd7c565b535ee7 Mon Sep 17 00:00:00 2001 From: carlory Date: Thu, 25 Jan 2024 11:47:56 +0800 Subject: [PATCH 3/3] add feature label into test --- test/e2e/apimachinery/resource_quota.go | 2 +- test/e2e/feature/feature.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/test/e2e/apimachinery/resource_quota.go b/test/e2e/apimachinery/resource_quota.go index a534d75cf64b5..3f7e2891d12d4 100644 --- a/test/e2e/apimachinery/resource_quota.go +++ b/test/e2e/apimachinery/resource_quota.go @@ -1221,7 +1221,7 @@ var _ = SIGDescribe("ResourceQuota", func() { }) }) -var _ = SIGDescribe("ResourceQuota", framework.WithFeatureGate(features.VolumeAttributesClass), func() { +var _ = SIGDescribe("ResourceQuota", feature.VolumeAttributesClass, framework.WithFeatureGate(features.VolumeAttributesClass), func() { f := framework.NewDefaultFramework("volume-attributes-class") f.NamespacePodSecurityLevel = admissionapi.LevelBaseline diff --git a/test/e2e/feature/feature.go b/test/e2e/feature/feature.go index b100ad67f8e39..8a5000beb26e0 100644 --- a/test/e2e/feature/feature.go +++ b/test/e2e/feature/feature.go @@ -331,6 +331,21 @@ var ( // TODO: document the feature (owning SIG, when to use this feature for a test) ValidatingAdmissionPolicy = framework.WithFeature(framework.ValidFeatures.Add("ValidatingAdmissionPolicy")) + // owning-sig: sig-storage + // kep: https://kep.k8s.io/3751 + // test-infra jobs: + // - pull-kubernetes-e2e-storage-kind-alpha-features (need manual trigger) + // - ci-kubernetes-e2e-storage-kind-alpha-features + // + // When this label is added to a test, it means that the cluster must be created + // with the feature-gate "VolumeAttributesClass=true" and the storage.k8s.io/v1alpha1 + // API version must be enabled. + // + // Once the feature and API version are stable, this label should be removed and + // these tests will be run by default on any cluster. The test-infra job also should + // be updated to not focus on this feature anymore. + VolumeAttributesClass = framework.WithFeature(framework.ValidFeatures.Add("VolumeAttributesClass")) + // TODO: document the feature (owning SIG, when to use this feature for a test) Volumes = framework.WithFeature(framework.ValidFeatures.Add("Volumes"))