Ensure roundtrip success for Quantities

Author: brejman

pkg/resources/requests.go

@@ -108,15 +108,27 @@ func ResourceQuantity(name corev1.ResourceName, v int64) resource.Quantity {
 	case corev1.ResourceCPU:
 		return *resource.NewMilliQuantity(v, resource.DecimalSI)
 	case corev1.ResourceMemory, corev1.ResourceEphemeralStorage:
-		return *resource.NewQuantity(v, resource.BinarySI)
+		return parseMaybeBinary(v)
 	default:
 		if strings.HasPrefix(string(name), corev1.ResourceHugePagesPrefix) {
-			return *resource.NewQuantity(v, resource.BinarySI)
+			return parseMaybeBinary(v)
 		}
 		return *resource.NewQuantity(v, resource.DecimalSI)
 	}
 }
 
+// Ensures that the resulting Quantity roundtrips.
+// For example, 1000000 will be decimal (1M), while 1048576 will be binary (1Mi)
+func parseMaybeBinary(v int64) resource.Quantity {
+	binary := *resource.NewQuantity(v, resource.BinarySI)
+	final, err := resource.ParseQuantity(binary.String())
+	if err != nil {
+		// Should never happen
+		return binary
+	}
+	return final
+}
+
 func ResourceQuantityString(name corev1.ResourceName, v int64) string {
 	rq := ResourceQuantity(name, v)
 	return rq.String()

pkg/resources/requests_test.go

@@ -17,10 +17,12 @@ limitations under the License.
 package resources
 
 import (
+	"encoding/json"
 	"math"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 )
 
 func TestCountIn(t *testing.T) {
@@ -121,3 +123,76 @@ func TestCountIn(t *testing.T) {
 		})
 	}
 }
+
+func TestResourceQuantityRoundTrips(t *testing.T) {
+	cases := map[string]struct {
+		resource corev1.ResourceName
+		value    int64
+		expected string
+	}{
+		"1": {
+			resource: corev1.ResourceMemory,
+			value:    1,
+			expected: "1",
+		},
+		"1k": {
+			resource: corev1.ResourceMemory,
+			value:    1000,
+			expected: "1k",
+		},
+		"1M": {
+			resource: corev1.ResourceMemory,
+			value:    1000000,
+			expected: "1M",
+		},
+		"1.5M": {
+			resource: corev1.ResourceMemory,
+			value:    1500000,
+			expected: "1500k",
+		},
+		"1G": {
+			resource: corev1.ResourceMemory,
+			value:    1000000000,
+			expected: "1G",
+		},
+		"1Ki": {
+			resource: corev1.ResourceMemory,
+			value:    1024,
+			expected: "1Ki",
+		},
+		"1Mi": {
+			resource: corev1.ResourceMemory,
+			value:    1024 * 1024,
+			expected: "1Mi",
+		},
+		"1.5Mi": {
+			resource: corev1.ResourceMemory,
+			value:    1024 * 1024 * 1.5,
+			expected: "1536Ki",
+		},
+		"1Gi": {
+			resource: corev1.ResourceMemory,
+			value:    1024 * 1024 * 1024,
+			expected: "1Gi",
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			quantity := ResourceQuantity(tc.resource, tc.value)
+			initial := quantity.String()
+
+			if initial != tc.expected {
+				t.Errorf("unexpected result, want=%s, got=%s", tc.expected, initial)
+			}
+
+			serialized, _ := json.Marshal(quantity)
+			var deserialized resource.Quantity
+			err := json.Unmarshal(serialized, &deserialized)
+			roundtrip := deserialized.String()
+
+			if roundtrip != tc.expected {
+				t.Errorf("unexpected result after roundtrip, want=%s, got=%s", tc.expected, roundtrip)
+			}
+		})
+	}
+}