From 39c61b0967e766f88a17753d6606776baa99d33c Mon Sep 17 00:00:00 2001 From: huangjiuyuan Date: Tue, 1 Aug 2017 14:28:04 +0800 Subject: [PATCH] adding validations on kubelet starting configurations --- .../apis/kubeletconfig/validation/BUILD | 13 +++ .../kubeletconfig/validation/validation.go | 68 +++++++++++++-- .../validation/validation_test.go | 82 +++++++++++++++++++ .../pkg/util/validation/validation.go | 8 ++ .../pkg/util/validation/validation_test.go | 24 ++++++ 5 files changed, 190 insertions(+), 5 deletions(-) create mode 100644 pkg/kubelet/apis/kubeletconfig/validation/validation_test.go diff --git a/pkg/kubelet/apis/kubeletconfig/validation/BUILD b/pkg/kubelet/apis/kubeletconfig/validation/BUILD index 86889db91be..432212ea3e8 100644 --- a/pkg/kubelet/apis/kubeletconfig/validation/BUILD +++ b/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", + ], +) diff --git a/pkg/kubelet/apis/kubeletconfig/validation/validation.go b/pkg/kubelet/apis/kubeletconfig/validation/validation.go index 519cef7f8c9..f0b243081bb 100644 --- a/pkg/kubelet/apis/kubeletconfig/validation/validation.go +++ b/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", - val, containermanager.NodeAllocatableEnforcementKey, containermanager.SystemReservedEnforcementKey, containermanager.KubeReservedEnforcementKey) + 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)) } } - return nil + return utilerrors.NewAggregate(allErrors) } diff --git a/pkg/kubelet/apis/kubeletconfig/validation/validation_test.go b/pkg/kubelet/apis/kubeletconfig/validation/validation_test.go new file mode 100644 index 00000000000..d9bff809d4d --- /dev/null +++ b/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())) + } +} diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go index 1159a02573c..3bf838fa21b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation.go +++ b/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 ( diff --git a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go b/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go index 061be1a6e65..e930a2fa6f6 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/validation/validation_test.go +++ b/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 {