Merge pull request #30907 from deads2k/fix-quota-updates

Automatic merge from submit-queue

only compute delta on non-creating updates

If you're issuing an update that can cause a create, the quota admission charge should be based on the create cost, otherwise you always end up with zero.

@derekwaynecarr ptal, blocker bug.
This commit is contained in:
Kubernetes Submit Queue 2016-08-20 16:16:02 -07:00 committed by GitHub
commit 4145824911
2 changed files with 11 additions and 5 deletions

View File

@ -292,8 +292,8 @@ func TestAdmitHandlesOldObjects(t *testing.T) {
indexer.Add(resourceQuota) indexer.Add(resourceQuota)
// old service was a load balancer, but updated version is a node port. // old service was a load balancer, but updated version is a node port.
oldService := &api.Service{ existingService := &api.Service{
ObjectMeta: api.ObjectMeta{Name: "service", Namespace: "test"}, ObjectMeta: api.ObjectMeta{Name: "service", Namespace: "test", ResourceVersion: "1"},
Spec: api.ServiceSpec{Type: api.ServiceTypeLoadBalancer}, Spec: api.ServiceSpec{Type: api.ServiceTypeLoadBalancer},
} }
newService := &api.Service{ newService := &api.Service{
@ -303,7 +303,7 @@ func TestAdmitHandlesOldObjects(t *testing.T) {
Ports: []api.ServicePort{{Port: 1234}}, Ports: []api.ServicePort{{Port: 1234}},
}, },
} }
err := handler.Admit(admission.NewAttributesRecord(newService, oldService, api.Kind("Service").WithVersion("version"), newService.Namespace, newService.Name, api.Resource("services").WithVersion("version"), "", admission.Update, nil)) err := handler.Admit(admission.NewAttributesRecord(newService, existingService, api.Kind("Service").WithVersion("version"), newService.Namespace, newService.Name, api.Resource("services").WithVersion("version"), "", admission.Update, nil))
if err != nil { if err != nil {
t.Errorf("Unexpected error: %v", err) t.Errorf("Unexpected error: %v", err)
} }

View File

@ -369,8 +369,14 @@ func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.At
if prevItem == nil { if prevItem == nil {
return nil, admission.NewForbidden(a, fmt.Errorf("unable to get previous usage since prior version of object was not found")) return nil, admission.NewForbidden(a, fmt.Errorf("unable to get previous usage since prior version of object was not found"))
} }
prevUsage := evaluator.Usage(prevItem)
deltaUsage = quota.Subtract(deltaUsage, prevUsage) // if we can definitively determine that this is not a case of "create on update",
// then charge based on the delta. Otherwise, bill the maximum
metadata, err := meta.Accessor(prevItem)
if err == nil && len(metadata.GetResourceVersion()) > 0 {
prevUsage := evaluator.Usage(prevItem)
deltaUsage = quota.Subtract(deltaUsage, prevUsage)
}
} }
if quota.IsZero(deltaUsage) { if quota.IsZero(deltaUsage) {
return quotas, nil return quotas, nil