From 21dbbe14f28038577f4f8bfa96b74c061a72dfd7 Mon Sep 17 00:00:00 2001 From: Michael Taufen Date: Fri, 9 Feb 2018 14:51:52 -0800 Subject: [PATCH] Ignore 0% and 100% eviction thresholds Primarily, this gives a way to explicitly disable eviction, which is necessary to use omitempty on EvictionHard. See: https://github.com/kubernetes/kubernetes/pull/53833#discussion_r166672137 As justification for this approach, neither 0% nor 100% make sense as eviction thresholds; in the "less-than" case, you can't have less than 0% of a resource and 100% perpetually evicts; in the "greater-than" case (assuming we ever add a resource with this semantic), the reasoning is the reverse (not more than 100%, 0% perpetually evicts). --- .../apis/kubeletconfig/v1alpha1/types.go | 4 +-- pkg/kubelet/eviction/helpers.go | 33 ++++++++++++------- pkg/kubelet/eviction/helpers_test.go | 14 ++++++++ test/e2e_node/eviction_test.go | 6 ++-- 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go b/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go index 4b50e51b1f..596b5f62d8 100644 --- a/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go +++ b/pkg/kubelet/apis/kubeletconfig/v1alpha1/types.go @@ -212,10 +212,10 @@ type KubeletConfiguration struct { SerializeImagePulls *bool `json:"serializeImagePulls"` // Map of signal names to quantities that defines hard eviction thresholds. For example: {"memory.available": "300Mi"}. // +optional - EvictionHard map[string]string `json:"evictionHard"` + EvictionHard map[string]string `json:"evictionHard,omitempty"` // Map of signal names to quantities that defines soft eviction thresholds. For example: {"memory.available": "300Mi"}. // +optional - EvictionSoft map[string]string `json:"evictionSoft"` + EvictionSoft map[string]string `json:"evictionSoft,omitempty"` // Map of signal names to quantities that defines grace periods for each soft eviction signal. For example: {"memory.available": "30s"}. // +optional EvictionSoftGracePeriod map[string]string `json:"evictionSoftGracePeriod"` diff --git a/pkg/kubelet/eviction/helpers.go b/pkg/kubelet/eviction/helpers.go index 0f51de97ed..87c4da7749 100644 --- a/pkg/kubelet/eviction/helpers.go +++ b/pkg/kubelet/eviction/helpers.go @@ -107,7 +107,6 @@ func ParseThresholdConfig(allocatableConfig []string, evictionHard, evictionSoft return nil, err } results = append(results, hardThresholds...) - softThresholds, err := parseThresholdStatements(evictionSoft) if err != nil { return nil, err @@ -151,26 +150,36 @@ func parseThresholdStatements(statements map[string]string) ([]evictionapi.Thres if err != nil { return nil, err } - results = append(results, result) + if result != nil { + results = append(results, *result) + } } return results, nil } -// parseThresholdStatement parses a threshold statement. -func parseThresholdStatement(signal evictionapi.Signal, val string) (evictionapi.Threshold, error) { +// parseThresholdStatement parses a threshold statement and returns a threshold, +// or nil if the threshold should be ignored. +func parseThresholdStatement(signal evictionapi.Signal, val string) (*evictionapi.Threshold, error) { if !validSignal(signal) { - return evictionapi.Threshold{}, fmt.Errorf(unsupportedEvictionSignal, signal) + return nil, fmt.Errorf(unsupportedEvictionSignal, signal) } operator := evictionapi.OpForSignal[signal] if strings.HasSuffix(val, "%") { + // ignore 0% and 100% + if val == "0%" || val == "100%" { + return nil, nil + } percentage, err := parsePercentage(val) if err != nil { - return evictionapi.Threshold{}, err + return nil, err } - if percentage <= 0 { - return evictionapi.Threshold{}, fmt.Errorf("eviction percentage threshold %v must be positive: %s", signal, val) + if percentage < 0 { + return nil, fmt.Errorf("eviction percentage threshold %v must be >= 0%%: %s", signal, val) } - return evictionapi.Threshold{ + if percentage > 100 { + return nil, fmt.Errorf("eviction percentage threshold %v must be <= 100%%: %s", signal, val) + } + return &evictionapi.Threshold{ Signal: signal, Operator: operator, Value: evictionapi.ThresholdValue{ @@ -180,12 +189,12 @@ func parseThresholdStatement(signal evictionapi.Signal, val string) (evictionapi } quantity, err := resource.ParseQuantity(val) if err != nil { - return evictionapi.Threshold{}, err + return nil, err } if quantity.Sign() < 0 || quantity.IsZero() { - return evictionapi.Threshold{}, fmt.Errorf("eviction threshold %v must be positive: %s", signal, &quantity) + return nil, fmt.Errorf("eviction threshold %v must be positive: %s", signal, &quantity) } - return evictionapi.Threshold{ + return &evictionapi.Threshold{ Signal: signal, Operator: operator, Value: evictionapi.ThresholdValue{ diff --git a/pkg/kubelet/eviction/helpers_test.go b/pkg/kubelet/eviction/helpers_test.go index ca78f4583f..168352ff25 100644 --- a/pkg/kubelet/eviction/helpers_test.go +++ b/pkg/kubelet/eviction/helpers_test.go @@ -288,6 +288,20 @@ func TestParseThresholdConfig(t *testing.T) { }, }, }, + "disable via 0%": { + allocatableConfig: []string{}, + evictionHard: map[string]string{"memory.available": "0%"}, + evictionSoft: map[string]string{"memory.available": "0%"}, + expectErr: false, + expectThresholds: []evictionapi.Threshold{}, + }, + "disable via 100%": { + allocatableConfig: []string{}, + evictionHard: map[string]string{"memory.available": "100%"}, + evictionSoft: map[string]string{"memory.available": "100%"}, + expectErr: false, + expectThresholds: []evictionapi.Threshold{}, + }, "invalid-signal": { allocatableConfig: []string{}, evictionHard: map[string]string{"mem.available": "150Mi"}, diff --git a/test/e2e_node/eviction_test.go b/test/e2e_node/eviction_test.go index 1f24b27d9a..c2d9c9a67a 100644 --- a/test/e2e_node/eviction_test.go +++ b/test/e2e_node/eviction_test.go @@ -170,7 +170,8 @@ var _ = framework.KubeDescribe("LocalStorageSoftEviction [Slow] [Serial] [Disrup initialConfig.EvictionMaxPodGracePeriod = 30 initialConfig.EvictionMinimumReclaim = map[string]string{} // Ensure that pods are not evicted because of the eviction-hard threshold - initialConfig.EvictionHard = map[string]string{} + // setting a threshold to 0% disables; non-empty map overrides default value (necessary due to omitempty) + initialConfig.EvictionHard = map[string]string{"memory.available": "0%"} }) runEvictionTest(f, pressureTimeout, expectedNodeCondition, logDiskMetrics, []podEvictSpec{ { @@ -192,7 +193,8 @@ var _ = framework.KubeDescribe("LocalStorageCapacityIsolationEviction [Slow] [Se Context(fmt.Sprintf(testContextFmt, "evictions due to pod local storage violations"), func() { tempSetCurrentKubeletConfig(f, func(initialConfig *kubeletconfig.KubeletConfiguration) { initialConfig.FeatureGates[string(features.LocalStorageCapacityIsolation)] = true - initialConfig.EvictionHard = map[string]string{} + // setting a threshold to 0% disables; non-empty map overrides default value (necessary due to omitempty) + initialConfig.EvictionHard = map[string]string{"memory.available": "0%"} }) sizeLimit := resource.MustParse("100Mi") useOverLimit := 101 /* Mb */