UPSTREAM: 3124: openshift: Use quantities for memory capacity differe…

…nces

This allows developers to better interpet how the calculations are being
done by leaving the values as "Quantities". For example, the max
difference is now a string converted to a quantity which will be easier
to reason about and update if needed in the future.
Also adds tests that match real values from a real set of nodes that
would be expected to be the same

cluster-autoscaler/processors/nodegroupset/compare_nodegroups.go

@@ -31,9 +31,11 @@ const (
 	// MaxFreeDifferenceRatio describes how free resources (allocatable - daemon and system pods)
 	// can differ between groups in the same NodeGroupSet
 	MaxFreeDifferenceRatio = 0.05
-	// MaxMemoryDifferenceInKiloBytes describes how much memory
-	// capacity can differ but still be considered equal.
-	MaxMemoryDifferenceInKiloBytes = 256000
+)
+
+var (
+	// MaxMemoryDifference describes how much memory capacity can differ but still be considered equal.
+	MaxMemoryDifference = resource.MustParse("256Mi")
 )
 
 // BasicIgnoredLabels define a set of basic labels that should be ignored when comparing the similarity
@@ -124,8 +126,10 @@ func IsCloudProviderNodeInfoSimilar(n1, n2 *schedulernodeinfo.NodeInfo, ignoredL
 		switch kind {
 		case apiv1.ResourceMemory:
 			// For memory capacity we allow a small tolerance
-			memoryDifference := math.Abs(float64(qtyList[0].Value()) - float64(qtyList[1].Value()))
-			if memoryDifference > MaxMemoryDifferenceInKiloBytes {
+			difference := absSub(qtyList[0], qtyList[1])
+
+			// Cmp compares two quantities and returns -1 for less than, 0 for equal, 1 for greater than
+			if difference.Cmp(MaxMemoryDifference) > 0 {
 				return false
 			}
 		default:
@@ -152,3 +156,16 @@ func IsCloudProviderNodeInfoSimilar(n1, n2 *schedulernodeinfo.NodeInfo, ignoredL
 
 	return true
 }
+
+// absSub returns the absolute value of a - b
+func absSub(a, b resource.Quantity) *resource.Quantity {
+	q := &resource.Quantity{}
+	a.DeepCopyInto(q)
+	q.Sub(b)
+
+	if q.Sign() == -1 {
+		q.Neg()
+	}
+
+	return q
+}

cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test.go

@@ -97,16 +97,38 @@ func TestNodesSimilarVariousRequirementsAndPods(t *testing.T) {
 }
 
 func TestNodesSimilarVariousMemoryRequirements(t *testing.T) {
-	n1 := BuildTestNode("node1", 1000, MaxMemoryDifferenceInKiloBytes)
+	n1 := BuildTestNode("node1", 1000, MaxMemoryDifference.Value())
 
 	// Different memory capacity within tolerance
-	n2 := BuildTestNode("node2", 1000, MaxMemoryDifferenceInKiloBytes)
-	n2.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(2*MaxMemoryDifferenceInKiloBytes, resource.DecimalSI)
+	n2 := BuildTestNode("node2", 1000, MaxMemoryDifference.Value())
+	n2.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(2*MaxMemoryDifference.Value(), resource.DecimalSI)
 	checkNodesSimilar(t, n1, n2, IsNodeInfoSimilar, true)
 
 	// Different memory capacity exceeds tolerance
-	n3 := BuildTestNode("node3", 1000, MaxMemoryDifferenceInKiloBytes)
-	n3.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(2*MaxMemoryDifferenceInKiloBytes+1, resource.DecimalSI)
+	n3 := BuildTestNode("node3", 1000, MaxMemoryDifference.Value())
+	n3.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(2*MaxMemoryDifference.Value()+1, resource.DecimalSI)
+	checkNodesSimilar(t, n1, n3, IsNodeInfoSimilar, false)
+}
+
+func TestNodesSimilarVariousLargeMemoryRequirements(t *testing.T) {
+	// Use realistic memory capacity (taken from real nodes)
+	// 15944120 KB ~= 16GiB (m5.xLarge)
+	q1 := resource.MustParse("16116152Ki")
+	q2 := resource.MustParse("15944120Ki")
+
+	n1 := BuildTestNode("node1", 1000, q1.Value())
+
+	// Different memory capacity within tolerance
+	// Value taken from another m5.xLarge in a different zone
+	n2 := BuildTestNode("node2", 1000, q2.Value())
+	checkNodesSimilar(t, n1, n2, IsNodeInfoSimilar, true)
+
+	// Different memory capacity exceeds tolerance
+	// Same value as q1 but with MaxMemoryDifference added twice
+	q3 := resource.MustParse("16116152Ki")
+	q3.Add(MaxMemoryDifference)
+	q3.Add(MaxMemoryDifference)
+	n3 := BuildTestNode("node3", 1000, q3.Value())
 	checkNodesSimilar(t, n1, n3, IsNodeInfoSimilar, false)
 }