Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add quota support for PVC with VolumeAttributesClass #121895

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
+200 −7
Draft

Add quota support for PVC with VolumeAttributesClass #121895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
608fa1d
Add quota support for PVC with VolumeAttributesClass
carlory Nov 15, 2023
22b351b
add e2e test
carlory Dec 21, 2023
921a9aa
add feature label into test
carlory Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 36 additions & 1 deletion pkg/quota/v1/evaluator/core/persistent_volume_claims.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -64,6 +65,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 <volume-attributes-class>.volumeattributesclass.storage.k8s.io/<resource>.
// 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"))
Expand Down Expand Up @@ -126,7 +145,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
}
Expand All @@ -138,6 +157,16 @@ func (p *pvcEvaluator) MatchingResources(items []corev1.ResourceName) []corev1.R
break
}
}
// match pvc resources scoped by volume attributes class (<volume-attributes-class-name>.volumeattributesclass.storage.k8s.io/<resource>)
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
}
Expand Down Expand Up @@ -169,6 +198,12 @@ func (p *pvcEvaluator) Usage(item runtime.Object) (corev1.ResourceList, error) {
}
}

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))
}

return result, nil
}

Expand Down
82 changes: 76 additions & 6 deletions pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -87,11 +88,18 @@ 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

validClaimByEmptyVolumeAttributesClass := validClaimByStorageClass.DeepCopy()
validClaimByEmptyVolumeAttributesClass.Spec.VolumeAttributesClassName = ptr.To("")

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,
Expand Down Expand Up @@ -152,10 +160,34 @@ 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": {
carlory marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down Expand Up @@ -185,8 +217,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{
Expand All @@ -195,6 +228,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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we also add "gold.volumeattributesclass.storage.k8s.io/requests.storage"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xing-yang It is not supported. discussed in #118863 (comment)

},

want: []corev1.ResourceName{
Expand All @@ -203,7 +237,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{
Expand All @@ -214,10 +250,44 @@ func TestPersistentVolumeClaimEvaluatorMatchingResources(t *testing.T) {
},
want: []corev1.ResourceName{},
},
// TODO: remove this test case after featuregate VolumeAttributesClass is GAed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we create issues to track TODOs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tracked in #122694

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be useful to add a link to that here.

"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)
}
Expand Down
73 changes: 73 additions & 0 deletions test/e2e/apimachinery/resource_quota.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,15 @@ 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"
"k8s.io/kubernetes/test/utils/crd"
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"
Expand Down Expand Up @@ -1219,6 +1221,77 @@ var _ = SIGDescribe("ResourceQuota", func() {
})
})

var _ = SIGDescribe("ResourceQuota", feature.VolumeAttributesClass, framework.WithFeatureGate(features.VolumeAttributesClass), func() {
carlory marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
15 changes: 15 additions & 0 deletions test/e2e/feature/feature.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

waiting for @msau42 feedback.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kubernetes/test-infra#31717 was merged.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section looks great! Thanks a lot.

// - 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"))

Expand Down