Merge pull request #49927 from huangjiuyuan/fix-kubelet-option-validation

Automatic merge from submit-queue (batch tested with PRs 49961, 50005, 50738, 51045, 49927)

adding validations on kubelet starting configurations

**What this PR does / why we need it**:
I found some validations of kubelet starting options were missing when I was creating a custom cluster from scratch. The kubelet does not check invalid configurations on `--cadvisor-port`, `--event-burst`, `--image-gc-high-threshold`, etc. I have added some validations in kubelet like validations in `cmd/kube-apiserver/app/options/validation.go`.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
Adds additional validation for kubelet in `pkg/kubelet/apis/kubeletconfig/validation`.
```

pkg/kubelet/apis/kubeletconfig/validation/BUILD

@@ -5,6 +5,7 @@ licenses(["notice"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
@@ -14,6 +15,8 @@ go_library(
     deps = [
         "//pkg/kubelet/apis/kubeletconfig:go_default_library",
         "//pkg/kubelet/cm:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
     ],
 )
 
@@ -29,3 +32,13 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["validation_test.go"],
+    library = ":go_default_library",
+    deps = [
+        "//pkg/kubelet/apis/kubeletconfig:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
+    ],
+)

pkg/kubelet/apis/kubeletconfig/validation/validation.go

@@ -19,17 +19,75 @@ package validation
 import (
 	"fmt"
 
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	utilvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig"
 	containermanager "k8s.io/kubernetes/pkg/kubelet/cm"
 )
 
 // ValidateKubeletConfiguration validates `kc` and returns an error if it is invalid
 func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration) error {
+	allErrors := []error{}
+
 	if !kc.CgroupsPerQOS && len(kc.EnforceNodeAllocatable) > 0 {
-		return fmt.Errorf("node allocatable enforcement is not supported unless Cgroups Per QOS feature is turned on")
+		allErrors = append(allErrors, fmt.Errorf("EnforceNodeAllocatable (--enforce-node-allocatable) is not supported unless CgroupsPerQOS (--cgroups-per-qos) feature is turned on"))
 	}
 	if kc.SystemCgroups != "" && kc.CgroupRoot == "" {
-		return fmt.Errorf("invalid configuration: system container was specified and cgroup root was not specified")
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: SystemCgroups (--system-cgroups) was specified and CgroupRoot (--cgroup-root) was not specified"))
+	}
+	if kc.CAdvisorPort != 0 && utilvalidation.IsValidPortNum(int(kc.CAdvisorPort)) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: CAdvisorPort (--cadvisor-port) %v must be between 0 and 65535, inclusive", kc.CAdvisorPort))
+	}
+	if kc.EventBurst < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: EventBurst (--event-burst) %v must not be a negative number", kc.EventBurst))
+	}
+	if kc.EventRecordQPS < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: EventRecordQPS (--event-qps) %v must not be a negative number", kc.EventRecordQPS))
+	}
+	if kc.HealthzPort != 0 && utilvalidation.IsValidPortNum(int(kc.HealthzPort)) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: HealthzPort (--healthz-port) %v must be between 1 and 65535, inclusive", kc.HealthzPort))
+	}
+	if utilvalidation.IsInRange(int(kc.ImageGCHighThresholdPercent), 0, 100) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: ImageGCHighThresholdPercent (--image-gc-high-threshold) %v must be between 0 and 100, inclusive", kc.ImageGCHighThresholdPercent))
+	}
+	if utilvalidation.IsInRange(int(kc.ImageGCLowThresholdPercent), 0, 100) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: ImageGCLowThresholdPercent (--image-gc-low-threshold) %v must be between 0 and 100, inclusive", kc.ImageGCLowThresholdPercent))
+	}
+	if utilvalidation.IsInRange(int(kc.IPTablesDropBit), 0, 31) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: IPTablesDropBit (--iptables-drop-bit) %v must be between 0 and 31, inclusive", kc.IPTablesDropBit))
+	}
+	if utilvalidation.IsInRange(int(kc.IPTablesMasqueradeBit), 0, 31) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: IPTablesMasqueradeBit (--iptables-masquerade-bit) %v must be between 0 and 31, inclusive", kc.IPTablesMasqueradeBit))
+	}
+	if kc.KubeAPIBurst < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: KubeAPIBurst (--kube-api-burst) %v must not be a negative number", kc.KubeAPIBurst))
+	}
+	if kc.KubeAPIQPS < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: KubeAPIQPS (--kube-api-qps) %v must not be a negative number", kc.KubeAPIQPS))
+	}
+	if kc.MaxOpenFiles < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: MaxOpenFiles (--max-open-files) %v must not be a negative number", kc.MaxOpenFiles))
+	}
+	if kc.MaxPods < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: MaxPods (--max-pods) %v must not be a negative number", kc.MaxPods))
+	}
+	if utilvalidation.IsInRange(int(kc.OOMScoreAdj), -1000, 1000) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: OOMScoreAdj (--oom-score-adj) %v must be between -1000 and 1000, inclusive", kc.OOMScoreAdj))
+	}
+	if kc.PodsPerCore < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: PodsPerCore (--pods-per-core) %v must not be a negative number", kc.PodsPerCore))
+	}
+	if utilvalidation.IsValidPortNum(int(kc.Port)) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: Port (--port) %v must be between 1 and 65535, inclusive", kc.Port))
+	}
+	if kc.ReadOnlyPort != 0 && utilvalidation.IsValidPortNum(int(kc.ReadOnlyPort)) != nil {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: ReadOnlyPort (--read-only-port) %v must be between 0 and 65535, inclusive", kc.ReadOnlyPort))
+	}
+	if kc.RegistryBurst < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: RegistryBurst (--registry-burst) %v must not be a negative number", kc.RegistryBurst))
+	}
+	if kc.RegistryPullQPS < 0 {
+		allErrors = append(allErrors, fmt.Errorf("Invalid configuration: RegistryPullQPS (--registry-qps) %v must not be a negative number", kc.RegistryPullQPS))
 	}
 	for _, val := range kc.EnforceNodeAllocatable {
 		switch val {
@@ -38,9 +96,9 @@ func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration) error
 		case containermanager.KubeReservedEnforcementKey:
 			continue
 		default:
-			return fmt.Errorf("invalid option %q specified for EnforceNodeAllocatable setting. Valid options are %q, %q or %q",
+			allErrors = append(allErrors, fmt.Errorf("Invalid option %q specified for EnforceNodeAllocatable (--enforce-node-allocatable) setting. Valid options are %q, %q or %q",
-				val, containermanager.NodeAllocatableEnforcementKey, containermanager.SystemReservedEnforcementKey, containermanager.KubeReservedEnforcementKey)
+				val, containermanager.NodeAllocatableEnforcementKey, containermanager.SystemReservedEnforcementKey, containermanager.KubeReservedEnforcementKey))
 		}
 	}
-	return nil
+	return utilerrors.NewAggregate(allErrors)
 }

pkg/kubelet/apis/kubeletconfig/validation/validation_test.go

@@ -0,0 +1,82 @@
+/*
+Copyright 2017 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package validation
+
+import (
+	"testing"
+
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
+	"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig"
+)
+
+func TestValidateKubeletConfiguration(t *testing.T) {
+	successCase := &kubeletconfig.KubeletConfiguration{
+		CgroupsPerQOS:               true,
+		EnforceNodeAllocatable:      []string{"pods"},
+		SystemCgroups:               "",
+		CgroupRoot:                  "",
+		CAdvisorPort:                0,
+		EventBurst:                  10,
+		EventRecordQPS:              5,
+		HealthzPort:                 10248,
+		ImageGCHighThresholdPercent: 85,
+		ImageGCLowThresholdPercent:  80,
+		IPTablesDropBit:             15,
+		IPTablesMasqueradeBit:       14,
+		KubeAPIBurst:                10,
+		KubeAPIQPS:                  5,
+		MaxOpenFiles:                1000000,
+		MaxPods:                     110,
+		OOMScoreAdj:                 -999,
+		PodsPerCore:                 100,
+		Port:                        65535,
+		ReadOnlyPort:                0,
+		RegistryBurst:               10,
+		RegistryPullQPS:             5,
+	}
+	if allErrors := ValidateKubeletConfiguration(successCase); allErrors != nil {
+		t.Errorf("expect no errors got %v", allErrors)
+	}
+
+	errorCase := &kubeletconfig.KubeletConfiguration{
+		CgroupsPerQOS:               false,
+		EnforceNodeAllocatable:      []string{"pods"},
+		SystemCgroups:               "/",
+		CgroupRoot:                  "",
+		CAdvisorPort:                -10,
+		EventBurst:                  -10,
+		EventRecordQPS:              -10,
+		HealthzPort:                 -10,
+		ImageGCHighThresholdPercent: 101,
+		ImageGCLowThresholdPercent:  101,
+		IPTablesDropBit:             -10,
+		IPTablesMasqueradeBit:       -10,
+		KubeAPIBurst:                -10,
+		KubeAPIQPS:                  -10,
+		MaxOpenFiles:                -10,
+		MaxPods:                     -10,
+		OOMScoreAdj:                 -1001,
+		PodsPerCore:                 -10,
+		Port:                        0,
+		ReadOnlyPort:                -10,
+		RegistryBurst:               -10,
+		RegistryPullQPS:             -10,
+	}
+	if allErrors := ValidateKubeletConfiguration(errorCase); len(allErrors.(utilerrors.Aggregate).Errors()) != 20 {
+		t.Errorf("expect 20 errors got %v", len(allErrors.(utilerrors.Aggregate).Errors()))
+	}
+}

staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go

@@ -188,6 +188,14 @@ func IsValidPortNum(port int) []string {
 	return []string{InclusiveRangeError(1, 65535)}
 }
 
+// IsInRange tests that the argument is in an inclusive range.
+func IsInRange(value int, min int, max int) []string {
+	if value >= min && value <= max {
+		return nil
+	}
+	return []string{InclusiveRangeError(min, max)}
+}
+
 // Now in libcontainer UID/GID limits is 0 ~ 1<<31 - 1
 // TODO: once we have a type for UID/GID we should make these that type.
 const (

staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go

@@ -154,6 +154,30 @@ func TestIsValidPortNum(t *testing.T) {
 	}
 }
 
+func TestIsInRange(t *testing.T) {
+	goodValues := []struct {
+		value int
+		min   int
+		max   int
+	}{{1, 0, 10}, {5, 5, 20}, {25, 10, 25}}
+	for _, val := range goodValues {
+		if msgs := IsInRange(val.value, val.min, val.max); len(msgs) > 0 {
+			t.Errorf("expected no errors for %#v, but got %v", val, msgs)
+		}
+	}
+
+	badValues := []struct {
+		value int
+		min   int
+		max   int
+	}{{1, 2, 10}, {5, -4, 2}, {25, 100, 120}}
+	for _, val := range badValues {
+		if msgs := IsInRange(val.value, val.min, val.max); len(msgs) == 0 {
+			t.Errorf("expected errors for %#v", val)
+		}
+	}
+}
+
 func createGroupIDs(ids ...int64) []int64 {
 	var output []int64
 	for _, id := range ids {