From e815e5bab97a894d998be0e1469d25c620dd655a Mon Sep 17 00:00:00 2001 From: deads2k Date: Mon, 29 Aug 2016 13:52:44 -0400 Subject: [PATCH] UPSTREAM: 31627: make deep copy of quota objects before mutations --- .../pkg/admission/resourcequota/controller.go | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/vendor/k8s.io/kubernetes/plugin/pkg/admission/resourcequota/controller.go b/vendor/k8s.io/kubernetes/plugin/pkg/admission/resourcequota/controller.go index 06051571c8a4..ddf37807b1a8 100644 --- a/vendor/k8s.io/kubernetes/plugin/pkg/admission/resourcequota/controller.go +++ b/vendor/k8s.io/kubernetes/plugin/pkg/admission/resourcequota/controller.go @@ -194,8 +194,11 @@ func (e *quotaEvaluator) checkAttributes(ns string, admissionAttributes []*admis // documents for these waiters have already been evaluated. Step 1, will mark all the ones that should already have succeeded. func (e *quotaEvaluator) checkQuotas(quotas []api.ResourceQuota, admissionAttributes []*admissionWaiter, remainingRetries int) { // yet another copy to compare against originals to see if we actually have deltas - originalQuotas := make([]api.ResourceQuota, len(quotas), len(quotas)) - copy(originalQuotas, quotas) + originalQuotas, err := copyQuotas(quotas) + if err != nil { + utilruntime.HandleError(err) + return + } atLeastOneChanged := false for i := range admissionAttributes { @@ -210,7 +213,7 @@ func (e *quotaEvaluator) checkQuotas(quotas []api.ResourceQuota, admissionAttrib // that means that no quota docs applied, so it can get a pass atLeastOneChangeForThisWaiter := false for j := range newQuotas { - if !quota.Equals(originalQuotas[j].Status.Used, newQuotas[j].Status.Used) { + if !quota.Equals(quotas[j].Status.Used, newQuotas[j].Status.Used) { atLeastOneChanged = true atLeastOneChangeForThisWaiter = true break @@ -220,6 +223,7 @@ func (e *quotaEvaluator) checkQuotas(quotas []api.ResourceQuota, admissionAttrib if !atLeastOneChangeForThisWaiter { admissionAttribute.result = nil } + quotas = newQuotas } @@ -236,6 +240,7 @@ func (e *quotaEvaluator) checkQuotas(quotas []api.ResourceQuota, admissionAttrib var lastErr error for i := range quotas { newQuota := quotas[i] + // if this quota didn't have its status changed, skip it if quota.Equals(originalQuotas[i].Status.Used, newQuota.Status.Used) { continue @@ -297,6 +302,19 @@ func (e *quotaEvaluator) checkQuotas(quotas []api.ResourceQuota, admissionAttrib e.checkQuotas(quotasToCheck, admissionAttributes, remainingRetries-1) } +func copyQuotas(in []api.ResourceQuota) ([]api.ResourceQuota, error) { + out := make([]api.ResourceQuota, 0, len(in)) + for _, quota := range in { + copied, err := api.Scheme.Copy("a) + if err != nil { + return nil, err + } + out = append(out, *copied.(*api.ResourceQuota)) + } + + return out, nil +} + // checkRequest verifies that the request does not exceed any quota constraint. it returns back a copy of quotas not yet persisted // that capture what the usage would be if the request succeeded. It return an error if the is insufficient quota to satisfy the request func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.Attributes) ([]api.ResourceQuota, error) { @@ -329,8 +347,7 @@ func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.At hardResources := quota.ResourceNames(resourceQuota.Status.Hard) evaluatorResources := evaluator.MatchesResources() requiredResources := quota.Intersection(hardResources, evaluatorResources) - err := evaluator.Constraints(requiredResources, inputObject) - if err != nil { + if err := evaluator.Constraints(requiredResources, inputObject); err != nil { return nil, admission.NewForbidden(a, fmt.Errorf("Failed quota: %s: %v", resourceQuota.Name, err)) } if !hasUsageStats(&resourceQuota) { @@ -376,8 +393,13 @@ func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.At return quotas, nil } + outQuotas, err := copyQuotas(quotas) + if err != nil { + return nil, err + } + for _, index := range interestingQuotaIndexes { - resourceQuota := quotas[index] + resourceQuota := outQuotas[index] hardResources := quota.ResourceNames(resourceQuota.Status.Hard) requestedUsage := quota.Mask(deltaUsage, hardResources) @@ -397,10 +419,10 @@ func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.At } // update to the new usage number - quotas[index].Status.Used = newUsage + outQuotas[index].Status.Used = newUsage } - return quotas, nil + return outQuotas, nil } func (e *quotaEvaluator) Evaluate(a admission.Attributes) error {