Moving heuristic reserved resources calculation from GCE to GKE provider

cluster-autoscaler/cloudprovider/gce/templates.go

@@ -94,24 +94,11 @@ func (t *GceTemplateBuilder) BuildAllocatableFromKubeEnv(capacity apiv1.Resource
 	if quantity, found := reserved[apiv1.ResourceMemory]; found {
 		reserved[apiv1.ResourceMemory] = *resource.NewQuantity(quantity.Value()+kubeletEvictionHardMemory, resource.BinarySI)
 	}
-	return t.getAllocatable(capacity, reserved), nil
+	return t.CalculateAllocatable(capacity, reserved), nil
 }
 
-// BuildAllocatableFromCapacity builds node allocatable based only on node capacity.
-// Calculates reserved as a ratio of capacity. See calculateReserved for more details
-func (t *GceTemplateBuilder) BuildAllocatableFromCapacity(capacity apiv1.ResourceList) apiv1.ResourceList {
-	memoryReserved := memoryReservedMB(capacity.Memory().Value() / bytesPerMB)
-	cpuReserved := cpuReservedMillicores(capacity.Cpu().MilliValue())
-	reserved := apiv1.ResourceList{}
-	reserved[apiv1.ResourceCPU] = *resource.NewMilliQuantity(cpuReserved, resource.DecimalSI)
-	// Duplicating an upstream bug treating MB as MiB (we need to predict the end result accurately).
-	memoryReserved = memoryReserved * 1024 * 1024
-	memoryReserved += kubeletEvictionHardMemory
-	reserved[apiv1.ResourceMemory] = *resource.NewQuantity(memoryReserved, resource.BinarySI)
-	return t.getAllocatable(capacity, reserved)
-}
-
-func (t *GceTemplateBuilder) getAllocatable(capacity, reserved apiv1.ResourceList) apiv1.ResourceList {
+// CalculateAllocatable computes allocatable resources substracting reserved values from corresponding capacity.
+func (t *GceTemplateBuilder) CalculateAllocatable(capacity, reserved apiv1.ResourceList) apiv1.ResourceList {
 	allocatable := apiv1.ResourceList{}
 	for key, value := range capacity {
 		quantity := *value.Copy()
@@ -346,81 +333,3 @@ func buildTaints(kubeEnvTaints map[string]string) ([]apiv1.Taint, error) {
 	}
 	return taints, nil
 }
-
-type allocatableBracket struct {
-	threshold            int64
-	marginalReservedRate float64
-}
-
-func memoryReservedMB(memoryCapacityMB int64) int64 {
-	if memoryCapacityMB <= 1*mbPerGB {
-		// do not set any memory reserved for nodes with less than 1 Gb of capacity
-		return 0
-	}
-	return calculateReserved(memoryCapacityMB, []allocatableBracket{
-		{
-			threshold:            0,
-			marginalReservedRate: 0.25,
-		},
-		{
-			threshold:            4 * mbPerGB,
-			marginalReservedRate: 0.2,
-		},
-		{
-			threshold:            8 * mbPerGB,
-			marginalReservedRate: 0.1,
-		},
-		{
-			threshold:            16 * mbPerGB,
-			marginalReservedRate: 0.06,
-		},
-		{
-			threshold:            128 * mbPerGB,
-			marginalReservedRate: 0.02,
-		},
-	})
-}
-
-func cpuReservedMillicores(cpuCapacityMillicores int64) int64 {
-	return calculateReserved(cpuCapacityMillicores, []allocatableBracket{
-		{
-			threshold:            0,
-			marginalReservedRate: 0.06,
-		},
-		{
-			threshold:            1 * millicoresPerCore,
-			marginalReservedRate: 0.01,
-		},
-		{
-			threshold:            2 * millicoresPerCore,
-			marginalReservedRate: 0.005,
-		},
-		{
-			threshold:            4 * millicoresPerCore,
-			marginalReservedRate: 0.0025,
-		},
-	})
-}
-
-// calculateReserved calculates reserved using capacity and a series of
-// brackets as follows:  the marginalReservedRate applies to all capacity
-// greater than the bracket, but less than the next bracket.  For example, if
-// the first bracket is threshold: 0, rate:0.1, and the second bracket has
-// threshold: 100, rate: 0.4, a capacity of 100 results in a reserved of
-// 100*0.1 = 10, but a capacity of 200 results in a reserved of
-// 10 + (200-100)*.4 = 50.  Using brackets with marginal rates ensures that as
-// capacity increases, reserved always increases, and never decreases.
-func calculateReserved(capacity int64, brackets []allocatableBracket) int64 {
-	var reserved float64
-	for i, bracket := range brackets {
-		c := capacity
-		if i < len(brackets)-1 && brackets[i+1].threshold < capacity {
-			c = brackets[i+1].threshold
-		}
-		additionalReserved := float64(c-bracket.threshold) * bracket.marginalReservedRate
-		if additionalReserved > 0 {
-			reserved += additionalReserved
-		}
-	}
-	return int64(reserved)
-}

cluster-autoscaler/cloudprovider/gce/templates_test.go

@@ -233,39 +233,6 @@ func TestGetAcceleratorCount(t *testing.T) {
 	}
 }
 
-func TestBuildAllocatableFromCapacity(t *testing.T) {
-	type testCase struct {
-		capacityCpu       string
-		capacityMemory    string
-		allocatableCpu    string
-		allocatableMemory string
-		gpuCount          int64
-	}
-	testCases := []testCase{{
-		capacityCpu:    "16000m",
-		capacityMemory: fmt.Sprintf("%v", 1*mbPerGB*bytesPerMB),
-		allocatableCpu: "15890m",
-		// Below threshold for reserving memory
-		allocatableMemory: fmt.Sprintf("%v", 1*mbPerGB*bytesPerMB-kubeletEvictionHardMemory),
-		gpuCount:          1,
-	}, {
-		capacityCpu:    "500m",
-		capacityMemory: fmt.Sprintf("%v", 1.1*mbPerGB*bytesPerMB),
-		allocatableCpu: "470m",
-		// Final 1024*1024 because we're duplicating upstream bug using MB as MiB
-		allocatableMemory: fmt.Sprintf("%v", 1.1*mbPerGB*bytesPerMB-0.25*1.1*mbPerGB*1024*1024-kubeletEvictionHardMemory),
-	}}
-	for _, tc := range testCases {
-		tb := GceTemplateBuilder{}
-		capacity, err := makeResourceList(tc.capacityCpu, tc.capacityMemory, tc.gpuCount)
-		assert.NoError(t, err)
-		expectedAllocatable, err := makeResourceList(tc.allocatableCpu, tc.allocatableMemory, tc.gpuCount)
-		assert.NoError(t, err)
-		allocatable := tb.BuildAllocatableFromCapacity(capacity)
-		assertEqualResourceLists(t, "Allocatable", expectedAllocatable, allocatable)
-	}
-}
-
 func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) {
 	cases := []struct {
 		desc   string
@@ -531,52 +498,6 @@ func TestParseKubeReserved(t *testing.T) {
 	}
 }
 
-func TestCalculateReserved(t *testing.T) {
-	type testCase struct {
-		name             string
-		function         func(capacity int64) int64
-		capacity         int64
-		expectedReserved int64
-	}
-	testCases := []testCase{
-		{
-			name:             "zero memory capacity",
-			function:         memoryReservedMB,
-			capacity:         0,
-			expectedReserved: 0,
-		},
-		{
-			name:             "between memory thresholds",
-			function:         memoryReservedMB,
-			capacity:         2 * mbPerGB,
-			expectedReserved: 500, // 0.5 Gb
-		},
-		{
-			name:             "at a memory threshold boundary",
-			function:         memoryReservedMB,
-			capacity:         8 * mbPerGB,
-			expectedReserved: 1800, // 1.8 Gb
-		},
-		{
-			name:             "exceeds highest memory threshold",
-			function:         memoryReservedMB,
-			capacity:         200 * mbPerGB,
-			expectedReserved: 10760, // 10.8 Gb
-		},
-		{
-			name:             "cpu sanity check",
-			function:         cpuReservedMillicores,
-			capacity:         4 * millicoresPerCore,
-			expectedReserved: 80,
-		},
-	}
-	for _, tc := range testCases {
-		if actualReserved := tc.function(tc.capacity); actualReserved != tc.expectedReserved {
-			t.Errorf("Test case: %s, Got f(%d Mb) = %d.  Want %d", tc.name, tc.capacity, actualReserved, tc.expectedReserved)
-		}
-	}
-}
-
 func makeTaintSet(taints []apiv1.Taint) map[apiv1.Taint]bool {
 	set := make(map[apiv1.Taint]bool)
 	for _, taint := range taints {

cluster-autoscaler/cloudprovider/gke/templates.go

@@ -24,6 +24,7 @@ import (
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/gce"
 
 	apiv1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/autoscaler/cluster-autoscaler/utils/gpu"
 )
@@ -87,6 +88,20 @@ func (t *GkeTemplateBuilder) BuildNodeFromMigSpec(mig *GkeMig, cpu int64, mem in
 	return &node, nil
 }
 
+// BuildAllocatableFromCapacity builds node allocatable based only on node capacity.
+// Calculates reserved as a ratio of capacity. See calculateReserved for more details
+func (t *GkeTemplateBuilder) BuildAllocatableFromCapacity(capacity apiv1.ResourceList) apiv1.ResourceList {
+	memoryReserved := memoryReservedMB(capacity.Memory().Value() / bytesPerMB)
+	cpuReserved := cpuReservedMillicores(capacity.Cpu().MilliValue())
+	reserved := apiv1.ResourceList{}
+	reserved[apiv1.ResourceCPU] = *resource.NewMilliQuantity(cpuReserved, resource.DecimalSI)
+	// Duplicating an upstream bug treating MB as MiB (we need to predict the end result accurately).
+	memoryReserved = memoryReserved * 1024 * 1024
+	memoryReserved += kubeletEvictionHardMemory
+	reserved[apiv1.ResourceMemory] = *resource.NewQuantity(memoryReserved, resource.BinarySI)
+	return t.CalculateAllocatable(capacity, reserved)
+}
+
 func buildLabelsForAutoprovisionedMig(mig *GkeMig, nodeName string) (map[string]string, error) {
 	// GenericLabels
 	labels, err := gce.BuildGenericLabels(mig.GceRef(), mig.Spec().MachineType, nodeName)
@@ -105,3 +120,81 @@ func buildLabelsForAutoprovisionedMig(mig *GkeMig, nodeName string) (map[string]
 	}
 	return labels, nil
 }
+
+type allocatableBracket struct {
+	threshold            int64
+	marginalReservedRate float64
+}
+
+func memoryReservedMB(memoryCapacityMB int64) int64 {
+	if memoryCapacityMB <= 1*mbPerGB {
+		// do not set any memory reserved for nodes with less than 1 Gb of capacity
+		return 0
+	}
+	return calculateReserved(memoryCapacityMB, []allocatableBracket{
+		{
+			threshold:            0,
+			marginalReservedRate: 0.25,
+		},
+		{
+			threshold:            4 * mbPerGB,
+			marginalReservedRate: 0.2,
+		},
+		{
+			threshold:            8 * mbPerGB,
+			marginalReservedRate: 0.1,
+		},
+		{
+			threshold:            16 * mbPerGB,
+			marginalReservedRate: 0.06,
+		},
+		{
+			threshold:            128 * mbPerGB,
+			marginalReservedRate: 0.02,
+		},
+	})
+}
+
+func cpuReservedMillicores(cpuCapacityMillicores int64) int64 {
+	return calculateReserved(cpuCapacityMillicores, []allocatableBracket{
+		{
+			threshold:            0,
+			marginalReservedRate: 0.06,
+		},
+		{
+			threshold:            1 * millicoresPerCore,
+			marginalReservedRate: 0.01,
+		},
+		{
+			threshold:            2 * millicoresPerCore,
+			marginalReservedRate: 0.005,
+		},
+		{
+			threshold:            4 * millicoresPerCore,
+			marginalReservedRate: 0.0025,
+		},
+	})
+}
+
+// calculateReserved calculates reserved using capacity and a series of
+// brackets as follows:  the marginalReservedRate applies to all capacity
+// greater than the bracket, but less than the next bracket.  For example, if
+// the first bracket is threshold: 0, rate:0.1, and the second bracket has
+// threshold: 100, rate: 0.4, a capacity of 100 results in a reserved of
+// 100*0.1 = 10, but a capacity of 200 results in a reserved of
+// 10 + (200-100)*.4 = 50.  Using brackets with marginal rates ensures that as
+// capacity increases, reserved always increases, and never decreases.
+func calculateReserved(capacity int64, brackets []allocatableBracket) int64 {
+	var reserved float64
+	for i, bracket := range brackets {
+		c := capacity
+		if i < len(brackets)-1 && brackets[i+1].threshold < capacity {
+			c = brackets[i+1].threshold
+		}
+		additionalReserved := float64(c-bracket.threshold) * bracket.marginalReservedRate
+		if additionalReserved > 0 {
+			reserved += additionalReserved
+		}
+	}
+	return int64(reserved)
+}

cluster-autoscaler/cloudprovider/gke/templates_test.go

@@ -295,3 +295,49 @@ func makeResourceList(cpu string, memory string, gpu int64) (apiv1.ResourceList,
 func assertEqualResourceLists(t *testing.T, name string, expected, actual apiv1.ResourceList) {
 	assert.True(t, quota.V1Equals(expected, actual), "%q unequal:\nExpected:%v\nActual:%v", name, expected, actual)
 }
+
+func TestCalculateReserved(t *testing.T) {
+	type testCase struct {
+		name             string
+		function         func(capacity int64) int64
+		capacity         int64
+		expectedReserved int64
+	}
+	testCases := []testCase{
+		{
+			name:             "zero memory capacity",
+			function:         memoryReservedMB,
+			capacity:         0,
+			expectedReserved: 0,
+		},
+		{
+			name:             "between memory thresholds",
+			function:         memoryReservedMB,
+			capacity:         2 * mbPerGB,
+			expectedReserved: 500, // 0.5 Gb
+		},
+		{
+			name:             "at a memory threshold boundary",
+			function:         memoryReservedMB,
+			capacity:         8 * mbPerGB,
+			expectedReserved: 1800, // 1.8 Gb
+		},
+		{
+			name:             "exceeds highest memory threshold",
+			function:         memoryReservedMB,
+			capacity:         200 * mbPerGB,
+			expectedReserved: 10760, // 10.8 Gb
+		},
+		{
+			name:             "cpu sanity check",
+			function:         cpuReservedMillicores,
+			capacity:         4 * millicoresPerCore,
+			expectedReserved: 80,
+		},
+	}
+	for _, tc := range testCases {
+		if actualReserved := tc.function(tc.capacity); actualReserved != tc.expectedReserved {
+			t.Errorf("Test case: %s, Got f(%d Mb) = %d.  Want %d", tc.name, tc.capacity, actualReserved, tc.expectedReserved)
+		}
+	}
+}