diff --git a/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go b/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go
index d4e06c6f00710cb6b188ab5f20c365e58845bc15..f2eaa527ebb0e248e86576a90348b45c1c70f0f5 100644
--- a/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go
+++ b/pkg/controller/machinehealthcheck/machinehealthcheck_controller.go
@@ -2,7 +2,11 @@ package machinehealthcheck
import (
"context"
+ "errors"
"fmt"
+ "math"
+ "strconv"
+ "strings"
"time"
"k8s.io/apimachinery/pkg/util/intstr"
@@ -228,7 +232,7 @@ func isAllowedRemediation(mhc *mapiv1.MachineHealthCheck) bool {
if mhc.Spec.MaxUnhealthy == nil {
return true
}
- maxUnhealthy, err := intstr.GetValueFromIntOrPercent(mhc.Spec.MaxUnhealthy, derefInt(mhc.Status.ExpectedMachines), false)
+ maxUnhealthy, err := getValueFromIntOrPercent(mhc.Spec.MaxUnhealthy, derefInt(mhc.Status.ExpectedMachines), false)
if err != nil {
glog.Errorf("%s: error decoding maxUnhealthy, remediation won't be allowed: %v", namespacedName(mhc), err)
return false
@@ -673,3 +677,53 @@ func hasMatchingLabels(machineHealthCheck *mapiv1.MachineHealthCheck, machine *m
}
return true
}
+
+// getValueFromIntOrPercent returns the integer number value based on the
+// percentage of the total or absolute number dependent on the IntOrString given
+//
+// The following code is copied from https://github.com/kubernetes/apimachinery/blob/1a505bc60c6dfb15cb18a8cdbfa01db042156fe2/pkg/util/intstr/intstr.go#L154-L185
+// But fixed so that string values aren't always assumed to be percentages
+// See https://github.com/kubernetes/kubernetes/issues/89082 for details
+func getValueFromIntOrPercent(intOrPercent *intstr.IntOrString, total int, roundUp bool) (int, error) {
+ if intOrPercent == nil {
+ return 0, errors.New("nil value for IntOrString")
+ }
+ value, isPercent, err := getIntOrPercentValue(intOrPercent)
+ if err != nil {
+ return 0, fmt.Errorf("invalid value for IntOrString: %v", err)
+ }
+ if isPercent {
+ if roundUp {
+ value = int(math.Ceil(float64(value) * (float64(total)) / 100))
+ } else {
+ value = int(math.Floor(float64(value) * (float64(total)) / 100))
+ }
+ }
+ return value, nil
+}
+
+// getIntOrPercentValue returns the integer value of the IntOrString and
+// determines if this value is a percentage or absolute number
+//
+// The following code is copied from https://github.com/kubernetes/apimachinery/blob/1a505bc60c6dfb15cb18a8cdbfa01db042156fe2/pkg/util/intstr/intstr.go#L154-L185
+// But fixed so that string values aren't always assumed to be percentages
+// See https://github.com/kubernetes/kubernetes/issues/89082 for details
+func getIntOrPercentValue(intOrStr *intstr.IntOrString) (int, bool, error) {
+ switch intOrStr.Type {
+ case intstr.Int:
+ return intOrStr.IntValue(), false, nil
+ case intstr.String:
+ isPercent := false
+ s := intOrStr.StrVal
+ if strings.Contains(s, "%") {
+ isPercent = true
+ s = strings.Replace(intOrStr.StrVal, "%", "", -1)
+ }
+ v, err := strconv.Atoi(s)
+ if err != nil {
+ return 0, isPercent, fmt.Errorf("invalid value %q: %v", intOrStr.StrVal, err)
+ }
+ return int(v), isPercent, nil
+ }
+ return 0, false, fmt.Errorf("invalid type: neither int nor percentage")
+}
diff --git a/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go b/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go
index 4204066b445a77df44f1fad8a9d58c2261b3334e..96c2dec30bf5c39a7b1a3fdc8ae405a2ab393521 100644
--- a/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go
+++ b/pkg/controller/machinehealthcheck/machinehealthcheck_controller_test.go
@@ -2411,6 +2411,8 @@ func TestIsAllowedRemediation(t *testing.T) {
// short circuit if ever more than 2 out of 5 go unhealthy
maxUnhealthyInt := intstr.FromInt(2)
maxUnhealthyString := intstr.FromString("40%")
+ maxUnhealthyIntInString := intstr.FromString("2")
+ maxUnhealthyMixedString := intstr.FromString("foo%50")
testCases := []struct {
testCase string
@@ -2501,6 +2503,48 @@ func TestIsAllowedRemediation(t *testing.T) {
},
expected: false,
},
+ {
+ testCase: "not above maxUnhealthy (int in string)",
+ mhc: &mapiv1beta1.MachineHealthCheck{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "test",
+ Namespace: namespace,
+ },
+ TypeMeta: metav1.TypeMeta{
+ Kind: "MachineHealthCheck",
+ },
+ Spec: mapiv1beta1.MachineHealthCheckSpec{
+ Selector: metav1.LabelSelector{},
+ MaxUnhealthy: &maxUnhealthyIntInString,
+ },
+ Status: mapiv1beta1.MachineHealthCheckStatus{
+ ExpectedMachines: IntPtr(5),
+ CurrentHealthy: IntPtr(3),
+ },
+ },
+ expected: true,
+ },
+ {
+ testCase: "above maxUnhealthy (int in string)",
+ mhc: &mapiv1beta1.MachineHealthCheck{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "test",
+ Namespace: namespace,
+ },
+ TypeMeta: metav1.TypeMeta{
+ Kind: "MachineHealthCheck",
+ },
+ Spec: mapiv1beta1.MachineHealthCheckSpec{
+ Selector: metav1.LabelSelector{},
+ MaxUnhealthy: &maxUnhealthyIntInString,
+ },
+ Status: mapiv1beta1.MachineHealthCheckStatus{
+ ExpectedMachines: IntPtr(5),
+ CurrentHealthy: IntPtr(2),
+ },
+ },
+ expected: false,
+ },
{
testCase: "nil values",
mhc: &mapiv1beta1.MachineHealthCheck{
@@ -2522,6 +2566,27 @@ func TestIsAllowedRemediation(t *testing.T) {
},
expected: true,
},
+ {
+ testCase: "invalid string value",
+ mhc: &mapiv1beta1.MachineHealthCheck{
+ ObjectMeta: metav1.ObjectMeta{
+ Name: "test",
+ Namespace: namespace,
+ },
+ TypeMeta: metav1.TypeMeta{
+ Kind: "MachineHealthCheck",
+ },
+ Spec: mapiv1beta1.MachineHealthCheckSpec{
+ Selector: metav1.LabelSelector{},
+ MaxUnhealthy: &maxUnhealthyMixedString,
+ },
+ Status: mapiv1beta1.MachineHealthCheckStatus{
+ ExpectedMachines: nil,
+ CurrentHealthy: nil,
+ },
+ },
+ expected: false,
+ },
}
for _, tc := range testCases {
@@ -2533,6 +2598,74 @@ func TestIsAllowedRemediation(t *testing.T) {
}
}
+func TestGetIntOrPercentValue(t *testing.T) {
+ int10 := intstr.FromInt(10)
+ percent20 := intstr.FromString("20%")
+ intInString30 := intstr.FromString("30")
+ invalidStringA := intstr.FromString("a")
+ invalidStringAPercent := intstr.FromString("a%")
+
+ testCases := []struct {
+ name string
+ in *intstr.IntOrString
+ expectedValue int
+ expectedPercent bool
+ expectedError error
+ }{
+ {
+ name: "with a integer",
+ in: &int10,
+ expectedValue: 10,
+ expectedPercent: false,
+ expectedError: nil,
+ },
+ {
+ name: "with a percentage",
+ in: &percent20,
+ expectedValue: 20,
+ expectedPercent: true,
+ expectedError: nil,
+ },
+ {
+ name: "with an int in string",
+ in: &intInString30,
+ expectedValue: 30,
+ expectedPercent: false,
+ expectedError: nil,
+ },
+ {
+ name: "with an 'a' string",
+ in: &invalidStringA,
+ expectedValue: 0,
+ expectedPercent: false,
+ expectedError: fmt.Errorf("invalid value \"a\": strconv.Atoi: parsing \"a\": invalid syntax"),
+ },
+ {
+ name: "with an 'a%' string",
+ in: &invalidStringAPercent,
+ expectedValue: 0,
+ expectedPercent: true,
+ expectedError: fmt.Errorf("invalid value \"a%%\": strconv.Atoi: parsing \"a\": invalid syntax"),
+ },
+ }
+
+ for _, tc := range testCases {
+ t.Run(tc.name, func(t *testing.T) {
+ value, percent, err := getIntOrPercentValue(tc.in)
+ // Check first if one is nil, and the other isn't, otherwise if not nil, do the messages match
+ if (tc.expectedError != nil) != (err != nil) || err != nil && tc.expectedError.Error() != err.Error() {
+ t.Errorf("Case: %s. Got: %v, expected: %v", tc.name, err, tc.expectedError)
+ }
+ if tc.expectedPercent != percent {
+ t.Errorf("Case: %s. Got: %v, expected: %v", tc.name, percent, tc.expectedPercent)
+ }
+ if tc.expectedValue != value {
+ t.Errorf("Case: %s. Got: %v, expected: %v", tc.name, value, tc.expectedValue)
+ }
+ })
+ }
+}
+
func IntPtr(i int) *int {
return &i
}