Skip to content

Commit

Permalink
Merge pull request kubernetes#146 from enxebre/fix-1828731
Browse files Browse the repository at this point in the history
Bug 1828731: UPSTREAM: <carry>: Convert the mem value consistently with other providers
  • Loading branch information
openshift-merge-robot authored Apr 29, 2020
2 parents f6a1456 + feac275 commit be87f46
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 15 deletions.
15 changes: 8 additions & 7 deletions cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/autoscaler/cluster-autoscaler/utils/units"
)

const (
Expand Down Expand Up @@ -185,16 +186,16 @@ func parseCPUCapacity(annotations map[string]string) (resource.Quantity, error)
}

func parseMemoryCapacity(annotations map[string]string) (resource.Quantity, error) {
// the value for the memoryKey is expected to have the unit type included,
// eg "1024Mi". if only a number is present, we add the suffix "Mi".
// The value for the memoryKey is expected to be an integer representing Mebibytes. e.g. "1024".
// https://www.iec.ch/si/binary.htm
val, exists := annotations[memoryKey]
if exists && val != "" {
// TODO remove this check once we ensured that the providers are using the correct values
if _, err := strconv.Atoi(val); err == nil {
// value is a number, we will append "Mi" as the unit type
val = fmt.Sprintf("%sMi", val)
valInt, err := strconv.ParseInt(val, 10, 0)
if err != nil {
return zeroQuantity.DeepCopy(), fmt.Errorf("value %q from annotation %q expected to be an integer: %v", val, memoryKey, err)
}
return resource.ParseQuantity(val)
// Convert from Mebibytes to bytes
return *resource.NewQuantity(valInt*units.MiB, resource.DecimalSI), nil
}
return zeroQuantity.DeepCopy(), nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"k8s.io/apimachinery/pkg/api/resource"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/autoscaler/cluster-autoscaler/utils/units"
)

const (
Expand Down Expand Up @@ -513,20 +514,20 @@ func TestParseMemoryCapacity(t *testing.T) {
expectedQuantity: zeroQuantity.DeepCopy(),
expectedError: true,
}, {
description: "valid quantity without unit type",
description: "valid quantity",
annotations: map[string]string{memoryKey: "456"},
expectedError: false,
expectedQuantity: resource.MustParse("456Mi"),
expectedQuantity: *resource.NewQuantity(456*units.MiB, resource.DecimalSI),
}, {
description: "valid quantity with unit type (Mi)",
description: "quantity with unit type (Mi)",
annotations: map[string]string{memoryKey: "456Mi"},
expectedError: false,
expectedQuantity: resource.MustParse("456Mi"),
expectedError: true,
expectedQuantity: zeroQuantity.DeepCopy(),
}, {
description: "valid quantity with unit type (Gi)",
description: "quantity with unit type (Gi)",
annotations: map[string]string{memoryKey: "8Gi"},
expectedError: false,
expectedQuantity: resource.MustParse("8Gi"),
expectedError: true,
expectedQuantity: zeroQuantity.DeepCopy(),
}} {
t.Run(tc.description, func(t *testing.T) {
got, err := parseMemoryCapacity(tc.annotations)
Expand Down

0 comments on commit be87f46

Please sign in to comment.