Merge pull request #1885 from harche/bug_1745919

Bug 1745919: KubeletConfig: Validate kubeReserved and systemReserved for KubeletConfig Signed-off-by: Harshal Patil <harpatil@redhat.com>
openshift · Aug 10, 2020 · d6428e5 · d6428e5
1 parent 6b77b94
commit d6428e5
Show file tree

Hide file tree

Showing 2 changed files with 129 additions and 0 deletions.
diff --git a/pkg/controller/kubelet-config/helpers.go b/pkg/controller/kubelet-config/helpers.go
@@ -12,6 +12,8 @@ import (
 	ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
 	"github.com/vincent-petithory/dataurl"
 	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -121,6 +123,36 @@ func validateUserKubeletConfig(cfg *mcfgv1.KubeletConfig) error {
 		}
 	}
 
+	reservedResources := []v1.ResourceName{v1.ResourceCPU, v1.ResourceMemory, v1.ResourceEphemeralStorage}
+
+	if kcDecoded.KubeReserved != nil && len(kcDecoded.KubeReserved) > 0 {
+		for _, rr := range reservedResources {
+			if val, ok := kcDecoded.KubeReserved[rr.String()]; ok {
+				q, err := resource.ParseQuantity(val)
+				if err != nil {
+					return fmt.Errorf("KubeletConfiguration: invalid value specified for %s reservation in kubeReserved, %s", rr.String(), val)
+				}
+				if q.Sign() == -1 {
+					return fmt.Errorf("KubeletConfiguration: %s reservation value cannot be negative in kubeReserved", rr.String())
+				}
+			}
+		}
+	}
+
+	if kcDecoded.SystemReserved != nil && len(kcDecoded.SystemReserved) > 0 {
+		for _, rr := range reservedResources {
+			if val, ok := kcDecoded.SystemReserved[rr.String()]; ok {
+				q, err := resource.ParseQuantity(val)
+				if err != nil {
+					return fmt.Errorf("KubeletConfiguration: invalid value specified for %s reservation in systemReserved, %s", rr.String(), val)
+				}
+				if q.Sign() == -1 {
+					return fmt.Errorf("KubeletConfiguration: %s reservation value cannot be negative in systemReserved", rr.String())
+				}
+			}
+		}
+	}
+
 	return nil
 }
 

diff --git a/pkg/controller/kubelet-config/kubelet_config_controller_test.go b/pkg/controller/kubelet-config/kubelet_config_controller_test.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/golang/glog"
 
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -480,6 +481,102 @@ func TestKubeletConfigBlacklistedOptions(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "kubeReserved cpu value cannot be negative",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				KubeReserved: map[string]string{
+					v1.ResourceCPU.String(): "-20",
+				},
+			},
+		},
+		{
+			name: "systemReserved cpu value cannot be negative",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				SystemReserved: map[string]string{
+					v1.ResourceCPU.String(): "-20",
+				},
+			},
+		},
+		{
+			name: "kubeReserved memory value cannot be negative",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				KubeReserved: map[string]string{
+					v1.ResourceMemory.String(): "-20M",
+				},
+			},
+		},
+		{
+			name: "systemReserved memory value cannot be negative",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				SystemReserved: map[string]string{
+					v1.ResourceMemory.String(): "-20M",
+				},
+			},
+		},
+		{
+			name: "kubeReserved cpu value fails to parse",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				KubeReserved: map[string]string{
+					v1.ResourceCPU.String(): "Peter Griffin",
+				},
+			},
+		},
+		{
+			name: "systemReserved cpu value fails to parse",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				SystemReserved: map[string]string{
+					v1.ResourceCPU.String(): "Stewie Griffin",
+				},
+			},
+		},
+		{
+			name: "kubeReserved memory value fails to parse",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				KubeReserved: map[string]string{
+					v1.ResourceMemory.String(): "Brian Griffin",
+				},
+			},
+		},
+		{
+			name: "systemReserved memory value fails to parse",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				SystemReserved: map[string]string{
+					v1.ResourceMemory.String(): "Meg Griffin",
+				},
+			},
+		},
+		{
+			name: "kubeReserved ephemeral-storage value fails to parse",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				KubeReserved: map[string]string{
+					v1.ResourceEphemeralStorage.String(): "Lois Griffin",
+				},
+			},
+		},
+		{
+			name: "kubeReserved ephemeral-storage value cannot be negative",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				KubeReserved: map[string]string{
+					v1.ResourceEphemeralStorage.String(): "-20M",
+				},
+			},
+		},
+		{
+			name: "systemReserved ephemeral-storage value fails to parse",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				SystemReserved: map[string]string{
+					v1.ResourceEphemeralStorage.String(): "Glenn Quagmire",
+				},
+			},
+		},
+		{
+			name: "systemReserved ephemeral-storage value cannot be negative",
+			config: &kubeletconfigv1beta1.KubeletConfiguration{
+				SystemReserved: map[string]string{
+					v1.ResourceEphemeralStorage.String(): "-20M",
+				},
+			},
+		},
 	}
 
 	successTests := []struct {