Only reject quota admission if status is missing relevant usage

This commit is contained in:
Jordan Liggitt 2019-03-13 20:53:16 -07:00
parent 27cd2be49f
commit bef996d0a4
2 changed files with 28 additions and 5 deletions

View File

@ -1098,10 +1098,12 @@ func TestAdmitBestEffortQuotaLimitIgnoresBurstable(t *testing.T) {
func TestHasUsageStats(t *testing.T) { func TestHasUsageStats(t *testing.T) {
testCases := map[string]struct { testCases := map[string]struct {
a corev1.ResourceQuota a corev1.ResourceQuota
relevant []corev1.ResourceName
expected bool expected bool
}{ }{
"empty": { "empty": {
a: corev1.ResourceQuota{Status: corev1.ResourceQuotaStatus{Hard: corev1.ResourceList{}}}, a: corev1.ResourceQuota{Status: corev1.ResourceQuotaStatus{Hard: corev1.ResourceList{}}},
relevant: []corev1.ResourceName{corev1.ResourceMemory},
expected: true, expected: true,
}, },
"hard-only": { "hard-only": {
@ -1113,6 +1115,7 @@ func TestHasUsageStats(t *testing.T) {
Used: corev1.ResourceList{}, Used: corev1.ResourceList{},
}, },
}, },
relevant: []corev1.ResourceName{corev1.ResourceMemory},
expected: false, expected: false,
}, },
"hard-used": { "hard-used": {
@ -1126,11 +1129,27 @@ func TestHasUsageStats(t *testing.T) {
}, },
}, },
}, },
relevant: []corev1.ResourceName{corev1.ResourceMemory},
expected: true,
},
"hard-used-relevant": {
a: corev1.ResourceQuota{
Status: corev1.ResourceQuotaStatus{
Hard: corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("1Gi"),
corev1.ResourcePods: resource.MustParse("1"),
},
Used: corev1.ResourceList{
corev1.ResourceMemory: resource.MustParse("500Mi"),
},
},
},
relevant: []corev1.ResourceName{corev1.ResourceMemory},
expected: true, expected: true,
}, },
} }
for testName, testCase := range testCases { for testName, testCase := range testCases {
if result := hasUsageStats(&testCase.a); result != testCase.expected { if result := hasUsageStats(&testCase.a, testCase.relevant); result != testCase.expected {
t.Errorf("%s expected: %v, actual: %v", testName, testCase.expected, result) t.Errorf("%s expected: %v, actual: %v", testName, testCase.expected, result)
} }
} }

View File

@ -460,8 +460,8 @@ func CheckRequest(quotas []corev1.ResourceQuota, a admission.Attributes, evaluat
if err := evaluator.Constraints(restrictedResources, inputObject); err != nil { if err := evaluator.Constraints(restrictedResources, inputObject); err != nil {
return nil, admission.NewForbidden(a, fmt.Errorf("failed quota: %s: %v", resourceQuota.Name, err)) return nil, admission.NewForbidden(a, fmt.Errorf("failed quota: %s: %v", resourceQuota.Name, err))
} }
if !hasUsageStats(&resourceQuota) { if !hasUsageStats(&resourceQuota, restrictedResources) {
return nil, admission.NewForbidden(a, fmt.Errorf("status unknown for quota: %s", resourceQuota.Name)) return nil, admission.NewForbidden(a, fmt.Errorf("status unknown for quota: %s, resources: %s", resourceQuota.Name, prettyPrintResourceNames(restrictedResources)))
} }
interestingQuotaIndexes = append(interestingQuotaIndexes, i) interestingQuotaIndexes = append(interestingQuotaIndexes, i)
localRestrictedResourcesSet := quota.ToSet(restrictedResources) localRestrictedResourcesSet := quota.ToSet(restrictedResources)
@ -702,9 +702,13 @@ func prettyPrintResourceNames(a []corev1.ResourceName) string {
return strings.Join(values, ",") return strings.Join(values, ",")
} }
// hasUsageStats returns true if for each hard constraint there is a value for its current usage // hasUsageStats returns true if for each hard constraint in interestingResources there is a value for its current usage
func hasUsageStats(resourceQuota *corev1.ResourceQuota) bool { func hasUsageStats(resourceQuota *corev1.ResourceQuota, interestingResources []corev1.ResourceName) bool {
interestingSet := quota.ToSet(interestingResources)
for resourceName := range resourceQuota.Status.Hard { for resourceName := range resourceQuota.Status.Hard {
if !interestingSet.Has(string(resourceName)) {
continue
}
if _, found := resourceQuota.Status.Used[resourceName]; !found { if _, found := resourceQuota.Status.Used[resourceName]; !found {
return false return false
} }