Merge pull request #116 from frobware/fix-node-comparator-v2

Cherry-pick nodeset comparator implementation from upstream

cluster-autoscaler/main.go

@@ -31,7 +31,6 @@ import (
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	cloudBuilder "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/builder"
-	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/openshiftmachineapi"
 	"k8s.io/autoscaler/cluster-autoscaler/config"
 	"k8s.io/autoscaler/cluster-autoscaler/core"
 	"k8s.io/autoscaler/cluster-autoscaler/estimator"
@@ -282,14 +281,6 @@ func buildAutoscaler() (core.Autoscaler, error) {
 		processors.NodeGroupSetProcessor = &nodegroupset.BalancingNodeGroupSetProcessor{
 			Comparator: nodegroupset.IsGkeNodeInfoSimilar}
 
-	}
-	if autoscalingOptions.CloudProviderName == openshiftmachineapi.ProviderName {
-		// We do this because of:
-		// - https://bugzilla.redhat.com/show_bug.cgi?id=1731011
-		// - https://bugzilla.redhat.com/show_bug.cgi?id=1733235
-		processors.NodeGroupSetProcessor = &nodegroupset.BalancingNodeGroupSetProcessor{
-			Comparator: nodegroupset.IsOpenShiftMachineApiNodeInfoSimilar}
-
 	}
 	opts := core.AutoscalerOptions{
 		AutoscalingOptions: autoscalingOptions,

cluster-autoscaler/processors/nodegroupset/compare_nodegroups.go

@@ -31,6 +31,9 @@ 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 = 128000
 )
 
 // NodeInfoComparator is a function that tells if two nodes are from NodeGroups
@@ -76,14 +79,28 @@ func IsNodeInfoSimilar(n1, n2 *schedulernodeinfo.NodeInfo) bool {
 			free[res] = append(free[res], freeRes)
 		}
 	}
-	// For capacity we require exact match.
-	// If this is ever changed, enforcing MaxCoresTotal and MaxMemoryTotal limits
-	// as it is now may no longer work.
-	for _, qtyList := range capacity {
-		if len(qtyList) != 2 || qtyList[0].Cmp(qtyList[1]) != 0 {
+
+	for kind, qtyList := range capacity {
+		if len(qtyList) != 2 {
 			return false
 		}
+		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 {
+				return false
+			}
+		default:
+			// For other capacity types we require exact match.
+			// If this is ever changed, enforcing MaxCoresTotal limits
+			// as it is now may no longer work.
+			if qtyList[0].Cmp(qtyList[1]) != 0 {
+				return false
+			}
+		}
 	}
+
 	// For allocatable and free we allow resource quantities to be within a few % of each other
 	if !compareResourceMapsWithTolerance(allocatable, MaxAllocatableDifferenceRatio) {
 		return false

cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test.go

@@ -96,6 +96,20 @@ func TestNodesSimilarVariousRequirementsAndPods(t *testing.T) {
 	checkNodesSimilarWithPods(t, n1, n4, []*apiv1.Pod{p1}, []*apiv1.Pod{p4}, IsNodeInfoSimilar, true)
 }
 
+func TestNodesSimilarVariousMemoryRequirements(t *testing.T) {
+	n1 := BuildTestNode("node1", 1000, MaxMemoryDifferenceInKiloBytes)
+
+	// Different memory capacity within tolerance
+	n2 := BuildTestNode("node2", 1000, MaxMemoryDifferenceInKiloBytes)
+	n2.Status.Capacity[apiv1.ResourceMemory] = *resource.NewQuantity(2*MaxMemoryDifferenceInKiloBytes, 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)
+	checkNodesSimilar(t, n1, n3, IsNodeInfoSimilar, false)
+}
+
 func TestNodesSimilarVariousLabels(t *testing.T) {
 	n1 := BuildTestNode("node1", 1000, 2000)
 	n1.ObjectMeta.Labels["test-label"] = "test-value"