Add quota support for PVC with VolumeAttributesClass

[carlory]

What type of PR is this?

/kind feature

What this PR does / why we need it:

A cluster admin wants to control costs while giving application developers the freedom to optimize their applications. They set a per VolumeAttributesClass limit to the maximum count of PVCs that can be specified in a cluster ResourceQuota. When an application dev modifies a PVC to request a higher tier of VolumeAttributesClass but there is no quota, the request is rejected. As the ResourceQuota is a cluster-scoped object, only the cluster admin and not application devs can change limits.

An example of defining ResourceQuota for VolumeAttributesClass:

apiVersion: v1
kind: ResourceQuota
metadata:
  name: vacquota
spec:
  hard:
  // Across all persistent volume claims associated with the 
  // <volume-attributes-class-name>, the total number of persistent volume claims 
  // that can exist in the namespace.
<volume-attributes-class-name>.volumeattributesclass.storage.k8s.io/persistentvolumeclaims: "5" 
  ...

Which issue(s) this PR fixes:

Fixes #119299

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Add quota support for PVC with VolumeAttributesClass.  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/persistentvolumeclaims.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

- [KEP]: https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/3751-volume-attributes-class

[carlory]

/cc @sunnylovestiramisu

[carlory]

/sig storage

[carlory]

/test pull-kubernetes-e2e-gce

[jiahuif]

/remove-sig api-machinery

[carlory]

/cc @sunnylovestiramisu @msau42

[carlory]

/test pull-kubernetes-e2e-kind
/test pull-kubernetes-e2e-kind-ipv6

[carlory]

Here's an example how to run e2e on macOS.

Terminal 1: setup local cluster with VolumeAttributesClass enabled.

➜  kubernetes git:(kep-3751-quota) sudo FEATURE_GATES=VolumeAttributesClass=true ./hack/local-up-cluster.sh
Password:
...
Local Kubernetes cluster is running. Press Ctrl-C to shut it down.

Configurations:
  /private/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/local-up-cluster.sh.XXXXXX.Emp1teaU/kube-audit-policy-file
  /private/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/local-up-cluster.sh.XXXXXX.Emp1teaU/kube-scheduler.yaml
  /private/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/local-up-cluster.sh.XXXXXX.Emp1teaU/kube-serviceaccount.key
  /private/var/folders/zz/zyxvpxvq6csfxvn_n0000000000000/T/local-up-cluster.sh.XXXXXX.Emp1teaU/kube_egress_selector_configuration.yaml

Logs:
  /tmp/etcd.log
  /tmp/kube-apiserver.log
  /tmp/kube-controller-manager.log


  /tmp/kube-scheduler.log


To start using your cluster, you can open up another terminal/tab and run:

  export KUBECONFIG=/var/run/kubernetes/admin.kubeconfig
  cluster/kubectl.sh
...

Terminal 2: create a fake node (it's required for e2e tests setup) and run kwok to maintain the fake node.

➜  kubernetes git:(kep-3751-quota) kubectl apply --kubeconfig /var/run/kubernetes/admin.kubeconfig -f - <<EOF
apiVersion: v1
kind: Node
metadata:
  annotations:
    node.alpha.kubernetes.io/ttl: "0"
    kwok.x-k8s.io/node: fake
  labels:
    beta.kubernetes.io/arch: amd64
    beta.kubernetes.io/os: linux
    kubernetes.io/arch: amd64
    kubernetes.io/hostname: kwok-node-0
    kubernetes.io/os: linux
    kubernetes.io/role: agent
    node-role.kubernetes.io/agent: ""
    type: kwok
  name: kwok-node-0
spec:
status:
  allocatable:
    cpu: 32
    memory: 256Gi
    pods: 110
  capacity:
    cpu: 32
    memory: 256Gi
    pods: 110
  nodeInfo:
    architecture: amd64
    bootID: ""
    containerRuntimeVersion: ""
    kernelVersion: ""
    kubeProxyVersion: fake
    kubeletVersion: fake
    machineID: ""
    operatingSystem: linux
    osImage: ""
    systemUUID: ""
  phase: Running
EOF

➜  kubernetes git:(kep-3751-quota) kwok \
  --kubeconfig=/var/run/kubernetes/admin.kubeconfig \
  --manage-all-nodes=false \
  --manage-nodes-with-annotation-selector=kwok.x-k8s.io/node=fake \
  --manage-nodes-with-label-selector= \
  --manage-single-node= \
  --disregard-status-with-annotation-selector=kwok.x-k8s.io/status=custom \
  --disregard-status-with-label-selector= \
  --cidr=10.0.0.1/24 \
  --node-ip=10.0.0.1 \
  --node-lease-duration-seconds=40

Terminal 3: Run e2e tests.

➜  ginkgo --focus "should create a ResourceQuota and capture the life of a persistent volume claim with a volume attributes class" -v test/e2e -- --kubeconfig=/var/run/kubernetes/admin.kubeconfig
  Dec 21 17:40:30.327: INFO: The --provider flag is not set. Continuing as if --provider=skeleton had been used.
  I1221 17:40:30.327668   58333 e2e.go:117] Starting e2e run "ffaf0a46-ce6a-4bf0-b2b7-e8a9c0962114" on Ginkgo node 1
Running Suite: Kubernetes e2e suite - /Users/kiki/workspace/golang/src/k8s.io/kubernetes/test/e2e
=================================================================================================
Random Seed: 1703151598 - will randomize all specs

Will run 1 of 7408 specs
------------------------------
[ReportBeforeSuite]
/Users/kiki/workspace/golang/src/k8s.io/kubernetes/test/e2e/e2e_test.go:157
[ReportBeforeSuite] PASSED [0.000 seconds]
------------------------------
[SynchronizedBeforeSuite]
/Users/kiki/workspace/golang/src/k8s.io/kubernetes/test/e2e/e2e.go:77
  Dec 21 17:40:30.417: INFO: >>> kubeConfig: /var/run/kubernetes/admin.kubeconfig
  Dec 21 17:40:30.419: INFO: Waiting up to 30m0s for all (but 0) nodes to be schedulable
  Dec 21 17:40:30.436: INFO: Waiting up to 5m0s for all daemonsets in namespace 'kube-system' to start
  Dec 21 17:40:30.436: INFO: e2e test version: v0.0.0-master+$Format:%H$
  Dec 21 17:40:30.437: INFO: kube-apiserver version: v1.29.0-alpha.3.117+af8174e3caabf3
  Dec 21 17:40:30.437: INFO: >>> kubeConfig: /var/run/kubernetes/admin.kubeconfig
  Dec 21 17:40:30.438: INFO: Cluster IP family: ipv4
[SynchronizedBeforeSuite] PASSED [0.021 seconds]
...
[sig-api-machinery] ResourceQuota [Feature:VolumeAttributesClass] should create a ResourceQuota and capture the life of a persistent volume claim with a volume attributes class [sig-api-machinery, Feature:VolumeAttributesClass]
/Users/kiki/workspace/golang/src/k8s.io/kubernetes/test/e2e/apimachinery/resource_quota.go:1220
  STEP: Creating a kubernetes client @ 12/21/23 17:40:30.532
  Dec 21 17:40:30.532: INFO: >>> kubeConfig: /var/run/kubernetes/admin.kubeconfig
  STEP: Building a namespace api object, basename volume-attributes-class @ 12/21/23 17:40:30.532
  STEP: Waiting for a default service account to be provisioned in namespace @ 12/21/23 17:40:30.56
  STEP: Waiting for kube-root-ca.crt to be provisioned in namespace @ 12/21/23 17:40:30.563
  STEP: Counting existing ResourceQuota @ 12/21/23 17:40:30.565
  STEP: Creating a ResourceQuota @ 12/21/23 17:40:35.573
  STEP: Ensuring resource quota status is calculated @ 12/21/23 17:40:35.584
  STEP: Creating a PersistentVolumeClaim with volume attributes class @ 12/21/23 17:40:37.591
  STEP: Ensuring resource quota status captures persistent volume claim creation @ 12/21/23 17:40:37.619
  STEP: Not allowing a PersistentVolumeClaim to be created that exceeds remaining quota @ 12/21/23 17:40:39.625
  STEP: Deleting a PersistentVolumeClaim @ 12/21/23 17:40:39.63
  STEP: Ensuring resource quota status released usage @ 12/21/23 17:40:39.638
  Dec 21 17:40:41.644: INFO: Waiting up to 7m0s for all (but 0) nodes to be ready
  STEP: Destroying namespace "volume-attributes-class-8151" for this suite. @ 12/21/23 17:40:41.648
• [11.125 seconds]
...
[SynchronizedAfterSuite]
/Users/kiki/workspace/golang/src/k8s.io/kubernetes/test/e2e/e2e.go:88
  Dec 21 17:40:41.673: INFO: Running AfterSuite actions on node 1
[SynchronizedAfterSuite] PASSED [0.000 seconds]
------------------------------
[ReportAfterSuite] Kubernetes e2e suite report
/Users/kiki/workspace/golang/src/k8s.io/kubernetes/test/e2e/e2e_test.go:161
[ReportAfterSuite] PASSED [0.000 seconds]
------------------------------

Ran 1 of 7408 Specs in 11.256 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 7407 Skipped
You're using deprecated Ginkgo functionality:
=============================================
  --ginkgo.slow-spec-threshold is deprecated --slow-spec-threshold has been deprecated and will be removed in a future version of Ginkgo.  This feature has proved to be more noisy than useful.  You can use --poll-progress-after, instead, to get more actionable feedback about potentially slow specs and understand where they might be getting stuck.

To silence deprecations that can be silenced set the following environment variable:
  ACK_GINKGO_DEPRECATIONS=2.13.0

PASS

Ginkgo ran 1 suite in 43.437464292s
Test Suite Passed

[carlory]

/test pull-kubernetes-e2e-kind-alpha-features
/test pull-kubernetes-e2e-kind-alpha-beta-features

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: c57da35c693a4bbe117ddad321e1bf874939c4a4

[carlory]

/assign @deads2k @msau42

[carlory]

/cc @pohly

for e2e test review.

[pohly]

Looks good to me from a SIG Testing perspective.

[carlory]

/cc @xing-yang

[xing-yang]

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

[carlory]

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

[xing-yang]

Looks good to me.

We need someone to review the quota changes: https://github.com/kubernetes/kubernetes/blob/master/pkg/quota/v1/OWNERS

[carlory]

/cc @deads2k @derekwaynecarr

[derekwaynecarr]

The PR looks fine to me. The use case for quota on this concept is similar to quota on storage class. It appears @deads2k wants to discuss more generally in api-machinery meeting. Will defer to him for final approval based on outcome of that discussion.

[deads2k]

Adding my comment from slack here.

I'd like to know things like

1. if we go generic, what would a resourcename look like
2. why choose a resourcename over a scope + scope selector
3. if we created a generic mechanism, would it use resourcename parsing or a different mechanism

At first blush, a scoped count sounds pretty reasonable. But doing so would leverage a different name pattern. The agenda for apimachinery is pretty flexible if you'd like to come and discuss: https://docs.google.com/document/d/1x9RNaaysyO0gXHIr1y50QFbiL1x8OWnk2v3XnrdkT5Y/edit

[carlory]

1. KEP: https://github.com/kubernetes/enhancements/tree/master/keps/sig-storage/3751-volume-attributes-class#administrator-quota-restrictions

   apiVersion: v1
   kind: ResourceQuota
   metadata:
     name: glod-pvcs
   spec:
     hard:
       gold.volumeattributesclass.storage.k8s.io/persistentvolumeclaims: "10"

   why choose a resourcename over a scope + scope selector

   The current implementation is following the existing style in

   kubernetes/pkg/quota/v1/evaluator/core/persistent_volume_claims.go

   Line 49 in 95a6f2e

   // that follows <storage-class>.storageclass.storage.k8s.io/<resource>.

   .

   I don't know the scope + scope selector mechanism is a generic mechanism for all resources. And the scope is only used for pods when I read the document. https://kubernetes.io/docs/concepts/policy/resource-quotas/#quota-scopes. I can not find other resources that use the scope mechanism.

2. Suggestions from sig api-machinery @deads2k, according to comments (If I understand correctly):

   apiVersion: v1
   kind: ResourceQuota
   metadata:
     name: glod-pvcs
   spec:
     hard:
       count/persistentvolumeclaims: "10"
     scopeSelector:
       matchExpressions:
       - operator : In
         scopeName: VolumeAttributesClass # Match persistentvolumesclaims that references the specified volume attributes class.
         values: ["glod"]

   if we created a generic mechanism, would it use resourcename parsing or a different mechanism

   I don't know the generic mechanism when I implement this feature. If we change the implementation, does gold.storageclass.storage.k8s.io/persistentvolumeclaims also need to be changed?

   If so, we need defines some new ResourceQuotaScope types to match persistentvolumesclaims.

   IMO, If scope + scope selector is a generic mechanism for all resources, we can use it. Otherwise, we can use the current implementation.

   cc @sunnylovestiramisu @msau42 @xing-yang What do you think?

[carlory]

/hold

• waiting for a feedback
• Is it a bug? Users create unlimited volumes with special attributes by changing a PVC's vac into a non-existent vac. Those modified PVCs have the same attributes as the original PVCs. So, we should consider the status field of the PVC when computing the quota. cc @sunnylovestiramisu

[sunnylovestiramisu]

Discussed offline, the quota system should be calculating current and the target resources, this is consistent with other existing quota settings.

For VAC, we have 3 fields spec.VACName, status.currentVACName, and status.modityVolumeStatus.targetVACName. So if any one of them is set to a VAC, the PVC needs to be calculated against the quota.

[sunnylovestiramisu]

@carlory Discussed in the API machinery meeting, for the VAC use case, there is not much difference between using the scope or not from the users' perspective. However the scope implementation is better for the API machinery system itself because they have a lot of resources to track quotas. For the new resources they recommend to implement it with the scope:

resource: PVC
scope: field-selected
scopeselectorthing: .spec.volumeattributesclassname=gold

The extra work here is to add a new scope for volumeattributesclass.

[carlory]

@sunnylovestiramisu thanks!

I re-implement it in #124360

/close

[carlory]

The Quota scopes has a bug that affects the #124360 PR.

The new e2e tests added in #124360 are failing.

(base) (⎈|kind-kind:test)➜  kubernetes git:(kep-3751-quota-2) ✗ sudo FEATURE_GATES=VolumeAttributesClass=true ./hack/local-up-cluster.sh
(base) (⎈|kind-kind:test)➜  kubernetes git:(kep-3751-quota-2) ✗ ginkgo --focus "against a pvc with different volume attributes class." -v test/e2e -- --kubeconfig=/var/run/kubernetes/admin.kubeconfig --num-nodes=0 --allowed-not-ready-nodes=-1 --e2e-verify-service-account=false

Summarizing 1 Failure:
  [FAIL] [sig-api-machinery] ResourceQuota [Feature:VolumeAttributesClass] [FeatureGate:VolumeAttributesClass] [Alpha] [It] should verify ResourceQuota's volume attributes class scope (quota set to pvc count: 1) against a pvc with different volume attributes class. [sig-api-machinery, Feature:VolumeAttributesClass, FeatureGate:VolumeAttributesClass, Alpha]
  /Users/kiki/workspace/golang/src/k8s.io/kubernetes/test/e2e/apimachinery/resource_quota.go:1344

Ran 1 of 7200 Specs in 96.243 seconds
FAIL! -- 0 Passed | 1 Failed | 0 Pending | 7199 Skipped

We may have to use this PR to add quota support for PVC with VolumeAttributesClass, so I reopen it.

/cc @sunnylovestiramisu @msau42 @deads2k @derekwaynecarr

/reopen

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: carlory
Once this PR has been reviewed and has the lgtm label, please ask for approval from deads2k. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• pkg/quota/v1/OWNERS
• test/e2e/apimachinery/OWNERS
• test/e2e/feature/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.