-
Notifications
You must be signed in to change notification settings - Fork 38.6k
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.
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
Add quota support for PVC with VolumeAttributesClass #121895
base: master
Are you sure you want to change the base?
Conversation
/sig storage |
/test pull-kubernetes-e2e-gce |
/remove-sig api-machinery |
55bf062
to
3f8590e
Compare
/test pull-kubernetes-e2e-kind |
af8174e
to
2253c37
Compare
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 |
/test pull-kubernetes-e2e-kind-alpha-features |
LGTM label has been added. Git tree hash: c57da35c693a4bbe117ddad321e1bf874939c4a4
|
/cc @pohly for e2e test review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me from a SIG Testing perspective.
/cc @xing-yang |
@@ -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", |
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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)
Looks good to me. We need someone to review the quota changes: https://github.com/kubernetes/kubernetes/blob/master/pkg/quota/v1/OWNERS |
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. |
Adding my comment from slack here. I'd like to know things like
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 |
|
/hold
|
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. |
@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:
The extra work here is to add a new scope for volumeattributesclass. |
@sunnylovestiramisu thanks! I re-implement it in #124360 /close |
The Quota scopes has a bug that affects the #124360 PR. The new e2e tests added in #124360 are failing.
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 |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: carlory The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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. |
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:
Which issue(s) this PR fixes:
Fixes #119299
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: