Merge pull request #25161 from derekwaynecarr/format_quota_error

Automatic merge from submit-queue

Sort resources in quota errors to avoid duplicate events

Bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=1333122

Errors describing why a request was rejected to quota would get variable responses (cpu=x,memory=y or memory=x,cpu=y) which caused duplicate events for the same root cause.

/cc @ncdc @jwforres

plugin/pkg/admission/resourcequota/admission.go

@@ -18,6 +18,7 @@ package resourcequota
 
 import (
 	"io"
+	"sort"
 	"strings"
 	"time"
 
@@ -91,10 +92,17 @@ func (q *quotaAdmission) Admit(a admission.Attributes) (err error) {
 }
 
 // prettyPrint formats a resource list for usage in errors
+// it outputs resources sorted in increasing order
 func prettyPrint(item api.ResourceList) string {
 	parts := []string{}
-	for key, value := range item {
-		constraint := string(key) + "=" + value.String()
+	keys := []string{}
+	for key := range item {
+		keys = append(keys, string(key))
+	}
+	sort.Strings(keys)
+	for _, key := range keys {
+		value := item[api.ResourceName(key)]
+		constraint := key + "=" + value.String()
 		parts = append(parts, constraint)
 	}
 	return strings.Join(parts, ",")

plugin/pkg/admission/resourcequota/admission_test.go

@@ -72,6 +72,46 @@ func validPod(name string, numContainers int, resources api.ResourceRequirements
 	return pod
 }
 
+func TestPrettyPrint(t *testing.T) {
+	toResourceList := func(resources map[api.ResourceName]string) api.ResourceList {
+		resourceList := api.ResourceList{}
+		for key, value := range resources {
+			resourceList[key] = resource.MustParse(value)
+		}
+		return resourceList
+	}
+	testCases := []struct {
+		input    api.ResourceList
+		expected string
+	}{
+		{
+			input: toResourceList(map[api.ResourceName]string{
+				api.ResourceCPU: "100m",
+			}),
+			expected: "cpu=100m",
+		},
+		{
+			input: toResourceList(map[api.ResourceName]string{
+				api.ResourcePods:                   "10",
+				api.ResourceServices:               "10",
+				api.ResourceReplicationControllers: "10",
+				api.ResourceServicesNodePorts:      "10",
+				api.ResourceRequestsCPU:            "100m",
+				api.ResourceRequestsMemory:         "100Mi",
+				api.ResourceLimitsCPU:              "100m",
+				api.ResourceLimitsMemory:           "100Mi",
+			}),
+			expected: "limits.cpu=100m,limits.memory=100Mi,pods=10,replicationcontrollers=10,requests.cpu=100m,requests.memory=100Mi,services=10,services.nodeports=10",
+		},
+	}
+	for i, testCase := range testCases {
+		result := prettyPrint(testCase.input)
+		if result != testCase.expected {
+			t.Errorf("Pretty print did not give stable sorted output[%d], expected %v, but got %v", i, testCase.expected, result)
+		}
+	}
+}
+
 // TestAdmissionIgnoresDelete verifies that the admission controller ignores delete operations
 func TestAdmissionIgnoresDelete(t *testing.T) {
 	kubeClient := fake.NewSimpleClientset()