mirror of https://github.com/k3s-io/k3s
Merge pull request #24514 from derekwaynecarr/pod_quota_fix
Automatic merge from submit-queue Quota ignores pod compute resources on updates Scenario: 1. define a quota Q that tracks memory and cpu 2. create pod P that uses memory=100Mi, cpu=100m 3. update pod P to use memory=50Mi,cpu=10m Expected Results: Step 3 should fail with validation error. Quota Q should not have changed. Actual Results: Step 3 fails validation, but quota Q is decremented to have memory usage down 50Mi and cpu usage down 40m. This is because the quota was getting updated even though the pod was going to fail validation. Fix: Quota should only support modifying pod compute resources when pods themselves support modifying their compute resources. This also fixes https://github.com/kubernetes/kubernetes/issues/24352 /cc @smarterclayton - this is what we discussed. fyi: @kubernetes/rh-cluster-infrapull/6/head
commit
32121e344b
|
@ -47,7 +47,8 @@ func NewPodEvaluator(kubeClient clientset.Interface) quota.Evaluator {
|
||||||
InternalGroupKind: api.Kind("Pod"),
|
InternalGroupKind: api.Kind("Pod"),
|
||||||
InternalOperationResources: map[admission.Operation][]api.ResourceName{
|
InternalOperationResources: map[admission.Operation][]api.ResourceName{
|
||||||
admission.Create: allResources,
|
admission.Create: allResources,
|
||||||
admission.Update: computeResources,
|
// TODO: the quota system can only charge for deltas on compute resources when pods support updates.
|
||||||
|
// admission.Update: computeResources,
|
||||||
},
|
},
|
||||||
GetFuncByNamespace: func(namespace, name string) (runtime.Object, error) {
|
GetFuncByNamespace: func(namespace, name string) (runtime.Object, error) {
|
||||||
return kubeClient.Core().Pods(namespace).Get(name)
|
return kubeClient.Core().Pods(namespace).Get(name)
|
||||||
|
|
|
@ -331,6 +331,7 @@ var _ = framework.KubeDescribe("ResourceQuota", func() {
|
||||||
pod := newTestPodForQuota(podName, requests, api.ResourceList{})
|
pod := newTestPodForQuota(podName, requests, api.ResourceList{})
|
||||||
pod, err = f.Client.Pods(f.Namespace.Name).Create(pod)
|
pod, err = f.Client.Pods(f.Namespace.Name).Create(pod)
|
||||||
Expect(err).NotTo(HaveOccurred())
|
Expect(err).NotTo(HaveOccurred())
|
||||||
|
podToUpdate := pod
|
||||||
|
|
||||||
By("Ensuring ResourceQuota status captures the pod usage")
|
By("Ensuring ResourceQuota status captures the pod usage")
|
||||||
usedResources[api.ResourceQuotas] = resource.MustParse("1")
|
usedResources[api.ResourceQuotas] = resource.MustParse("1")
|
||||||
|
@ -348,6 +349,19 @@ var _ = framework.KubeDescribe("ResourceQuota", func() {
|
||||||
pod, err = f.Client.Pods(f.Namespace.Name).Create(pod)
|
pod, err = f.Client.Pods(f.Namespace.Name).Create(pod)
|
||||||
Expect(err).To(HaveOccurred())
|
Expect(err).To(HaveOccurred())
|
||||||
|
|
||||||
|
By("Ensuring a pod cannot update its resource requirements")
|
||||||
|
// a pod cannot dynamically update its resource requirements.
|
||||||
|
requests = api.ResourceList{}
|
||||||
|
requests[api.ResourceCPU] = resource.MustParse("100m")
|
||||||
|
requests[api.ResourceMemory] = resource.MustParse("100Mi")
|
||||||
|
podToUpdate.Spec.Containers[0].Resources.Requests = requests
|
||||||
|
_, err = f.Client.Pods(f.Namespace.Name).Update(podToUpdate)
|
||||||
|
Expect(err).To(HaveOccurred())
|
||||||
|
|
||||||
|
By("Ensuring attempts to update pod resource requirements did not change quota usage")
|
||||||
|
err = waitForResourceQuota(f.Client, f.Namespace.Name, quotaName, usedResources)
|
||||||
|
Expect(err).NotTo(HaveOccurred())
|
||||||
|
|
||||||
By("Deleting the pod")
|
By("Deleting the pod")
|
||||||
err = f.Client.Pods(f.Namespace.Name).Delete(podName, api.NewDeleteOptions(0))
|
err = f.Client.Pods(f.Namespace.Name).Delete(podName, api.NewDeleteOptions(0))
|
||||||
Expect(err).NotTo(HaveOccurred())
|
Expect(err).NotTo(HaveOccurred())
|
||||||
|
|
Loading…
Reference in New Issue