From 1163a1d51ed007ff2c3cd6fe548f60fc0b175a24 Mon Sep 17 00:00:00 2001 From: draveness Date: Fri, 13 Sep 2019 11:38:02 +0800 Subject: [PATCH] feat: update taint nodes by condition to GA --- cmd/kube-controller-manager/app/core.go | 1 - .../daemon/daemon_controller_test.go | 4 +- .../node_lifecycle_controller.go | 19 +---- .../node_lifecycle_controller_test.go | 1 - pkg/features/kube_features.go | 3 +- pkg/kubeapiserver/options/plugins.go | 5 +- pkg/kubelet/BUILD | 1 - pkg/kubelet/eviction/eviction_manager.go | 11 ++- pkg/kubelet/kubelet_node_status.go | 12 ++- pkg/kubelet/kubelet_node_status_test.go | 31 ++++--- pkg/kubelet/kubelet_test.go | 14 ---- pkg/scheduler/algorithmprovider/BUILD | 7 +- .../algorithmprovider/defaults/defaults.go | 33 +------- .../defaults/defaults_test.go | 5 +- .../defaults/register_predicates.go | 7 +- .../algorithmprovider/plugins_test.go | 10 +-- .../api/compatibility/compatibility_test.go | 6 +- plugin/pkg/admission/nodetaint/BUILD | 5 -- plugin/pkg/admission/nodetaint/admission.go | 23 ++--- .../pkg/admission/nodetaint/admission_test.go | 31 ------- .../admission/podtolerationrestriction/BUILD | 3 - .../admission_test.go | 5 -- test/integration/daemonset/daemonset_test.go | 4 +- test/integration/scheduler/predicates_test.go | 52 +----------- test/integration/scheduler/scheduler_test.go | 83 ++++++------------- test/integration/scheduler/taint_test.go | 12 +-- test/integration/scheduler/util.go | 1 + 27 files changed, 89 insertions(+), 300 deletions(-) diff --git a/cmd/kube-controller-manager/app/core.go b/cmd/kube-controller-manager/app/core.go index 2646efd233e..07daa9251e9 100644 --- a/cmd/kube-controller-manager/app/core.go +++ b/cmd/kube-controller-manager/app/core.go @@ -179,7 +179,6 @@ func startNodeLifecycleController(ctx ControllerContext) (http.Handler, bool, er ctx.ComponentConfig.NodeLifecycleController.UnhealthyZoneThreshold, ctx.ComponentConfig.NodeLifecycleController.EnableTaintManager, utilfeature.DefaultFeatureGate.Enabled(features.TaintBasedEvictions), - utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition), ) if err != nil { return nil, true, err diff --git a/pkg/controller/daemon/daemon_controller_test.go b/pkg/controller/daemon/daemon_controller_test.go index 09f5d1bbda9..9c60d359cde 100644 --- a/pkg/controller/daemon/daemon_controller_test.go +++ b/pkg/controller/daemon/daemon_controller_test.go @@ -26,7 +26,7 @@ import ( "time" apps "k8s.io/api/apps/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -1804,8 +1804,6 @@ func TestTaintPressureNodeDaemonLaunchesPod(t *testing.T) { } manager.nodeStore.Add(node) - // Enabling critical pod and taint nodes by condition feature gate should create critical pod - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintNodesByCondition, true)() manager.dsStore.Add(ds) syncAndValidateDaemonSets(t, manager, ds, podControl, 1, 0, 0) } diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller.go b/pkg/controller/nodelifecycle/node_lifecycle_controller.go index 3286ca18d7f..66ab5ece80d 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller.go @@ -296,10 +296,6 @@ type Controller struct { // taints instead of evicting Pods itself. useTaintBasedEvictions bool - // if set to true, NodeController will taint Nodes based on its condition for 'NetworkUnavailable', - // 'MemoryPressure', 'PIDPressure' and 'DiskPressure'. - taintNodeByCondition bool - nodeUpdateQueue workqueue.Interface } @@ -320,7 +316,7 @@ func NewNodeLifecycleController( unhealthyZoneThreshold float32, runTaintManager bool, useTaintBasedEvictions bool, - taintNodeByCondition bool) (*Controller, error) { +) (*Controller, error) { if kubeClient == nil { klog.Fatalf("kubeClient is nil when starting Controller") @@ -359,7 +355,6 @@ func NewNodeLifecycleController( unhealthyZoneThreshold: unhealthyZoneThreshold, runTaintManager: runTaintManager, useTaintBasedEvictions: useTaintBasedEvictions && runTaintManager, - taintNodeByCondition: taintNodeByCondition, nodeUpdateQueue: workqueue.NewNamed("node_lifecycle_controller"), } if useTaintBasedEvictions { @@ -469,10 +464,6 @@ func NewNodeLifecycleController( }), }) - if nc.taintNodeByCondition { - klog.Infof("Controller will taint node by condition.") - } - nc.leaseLister = leaseInformer.Lister() if utilfeature.DefaultFeatureGate.Enabled(features.NodeLease) { nc.leaseInformerSynced = leaseInformer.Informer().HasSynced @@ -547,11 +538,9 @@ func (nc *Controller) doNodeProcessingPassWorker() { return } nodeName := obj.(string) - if nc.taintNodeByCondition { - if err := nc.doNoScheduleTaintingPass(nodeName); err != nil { - klog.Errorf("Failed to taint NoSchedule on node <%s>, requeue it: %v", nodeName, err) - // TODO(k82cn): Add nodeName back to the queue - } + if err := nc.doNoScheduleTaintingPass(nodeName); err != nil { + klog.Errorf("Failed to taint NoSchedule on node <%s>, requeue it: %v", nodeName, err) + // TODO(k82cn): Add nodeName back to the queue } // TODO: re-evaluate whether there are any labels that need to be // reconcile in 1.19. Remove this function if it's no longer necessary. diff --git a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go index 9530698c1dd..0022e57ae4d 100644 --- a/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go +++ b/pkg/controller/nodelifecycle/node_lifecycle_controller_test.go @@ -182,7 +182,6 @@ func newNodeLifecycleControllerFromClient( unhealthyZoneThreshold, useTaints, useTaints, - useTaints, ) if err != nil { return nil, err diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 5bd1f2b991d..d7bfe2147cc 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -127,6 +127,7 @@ const ( // owner: @k82cn // beta: v1.12 + // GA: v1.17 // // Taint nodes based on their condition status for 'NetworkUnavailable', // 'MemoryPressure', 'PIDPressure' and 'DiskPressure'. @@ -512,7 +513,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS EphemeralContainers: {Default: false, PreRelease: featuregate.Alpha}, PodShareProcessNamespace: {Default: true, PreRelease: featuregate.Beta}, PodPriority: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 - TaintNodesByCondition: {Default: true, PreRelease: featuregate.Beta}, + TaintNodesByCondition: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 QOSReserved: {Default: false, PreRelease: featuregate.Alpha}, ExpandPersistentVolumes: {Default: true, PreRelease: featuregate.Beta}, ExpandInUsePersistentVolumes: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/kubeapiserver/options/plugins.go b/pkg/kubeapiserver/options/plugins.go index 28d64cd8511..0f3ae7790ba 100644 --- a/pkg/kubeapiserver/options/plugins.go +++ b/pkg/kubeapiserver/options/plugins.go @@ -142,12 +142,9 @@ func DefaultOffAdmissionPlugins() sets.String { resourcequota.PluginName, //ResourceQuota storageobjectinuseprotection.PluginName, //StorageObjectInUseProtection podpriority.PluginName, //PodPriority + nodetaint.PluginName, //TaintNodesByCondition ) - if utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition) { - defaultOnPlugins.Insert(nodetaint.PluginName) //TaintNodesByCondition - } - if utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) { defaultOnPlugins.Insert(runtimeclass.PluginName) //RuntimeClass } diff --git a/pkg/kubelet/BUILD b/pkg/kubelet/BUILD index f2a52599ef1..74580901464 100644 --- a/pkg/kubelet/BUILD +++ b/pkg/kubelet/BUILD @@ -245,7 +245,6 @@ go_test( "//staging/src/k8s.io/client-go/tools/record:go_default_library", "//staging/src/k8s.io/client-go/util/flowcontrol:go_default_library", "//staging/src/k8s.io/client-go/util/testing:go_default_library", - "//staging/src/k8s.io/component-base/featuregate:go_default_library", "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//staging/src/k8s.io/component-base/version:go_default_library", "//vendor/github.com/google/cadvisor/info/v1:go_default_library", diff --git a/pkg/kubelet/eviction/eviction_manager.go b/pkg/kubelet/eviction/eviction_manager.go index 2a7ee0c88bd..1e3fce78e17 100644 --- a/pkg/kubelet/eviction/eviction_manager.go +++ b/pkg/kubelet/eviction/eviction_manager.go @@ -144,13 +144,12 @@ func (m *managerImpl) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAd return lifecycle.PodAdmitResult{Admit: true} } - // When node has memory pressure and TaintNodesByCondition is enabled, check BestEffort Pod's toleration: + // When node has memory pressure, check BestEffort Pod's toleration: // admit it if tolerates memory pressure taint, fail for other tolerations, e.g. DiskPressure. - if utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition) && - v1helper.TolerationsTolerateTaint(attrs.Pod.Spec.Tolerations, &v1.Taint{ - Key: schedulerapi.TaintNodeMemoryPressure, - Effect: v1.TaintEffectNoSchedule, - }) { + if v1helper.TolerationsTolerateTaint(attrs.Pod.Spec.Tolerations, &v1.Taint{ + Key: schedulerapi.TaintNodeMemoryPressure, + Effect: v1.TaintEffectNoSchedule, + }) { return lifecycle.PodAdmitResult{Admit: true} } } diff --git a/pkg/kubelet/kubelet_node_status.go b/pkg/kubelet/kubelet_node_status.go index 67caedc39e8..2014563db87 100644 --- a/pkg/kubelet/kubelet_node_status.go +++ b/pkg/kubelet/kubelet_node_status.go @@ -25,7 +25,7 @@ import ( "strings" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" @@ -246,13 +246,11 @@ func (kl *Kubelet) initialNode() (*v1.Node, error) { Effect: v1.TaintEffectNoSchedule, } - // If TaintNodesByCondition enabled, taint node with TaintNodeUnschedulable when initializing + // Taint node with TaintNodeUnschedulable when initializing // node to avoid race condition; refer to #63897 for more detail. - if utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition) { - if node.Spec.Unschedulable && - !taintutil.TaintExists(nodeTaints, &unschedulableTaint) { - nodeTaints = append(nodeTaints, unschedulableTaint) - } + if node.Spec.Unschedulable && + !taintutil.TaintExists(nodeTaints, &unschedulableTaint) { + nodeTaints = append(nodeTaints, unschedulableTaint) } if kl.externalCloudProvider { diff --git a/pkg/kubelet/kubelet_node_status_test.go b/pkg/kubelet/kubelet_node_status_test.go index ba52719cf4f..9bd6c33ab0b 100644 --- a/pkg/kubelet/kubelet_node_status_test.go +++ b/pkg/kubelet/kubelet_node_status_test.go @@ -48,7 +48,6 @@ import ( "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" core "k8s.io/client-go/testing" - "k8s.io/component-base/featuregate" featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/component-base/version" "k8s.io/kubernetes/pkg/features" @@ -1983,25 +1982,23 @@ func TestRegisterWithApiServerWithTaint(t *testing.T) { // Make node to be unschedulable. kubelet.registerSchedulable = false - forEachFeatureGate(t, []featuregate.Feature{features.TaintNodesByCondition}, func(t *testing.T) { - // Reset kubelet status for each test. - kubelet.registrationCompleted = false + // Reset kubelet status for each test. + kubelet.registrationCompleted = false - // Register node to apiserver. - kubelet.registerWithAPIServer() + // Register node to apiserver. + kubelet.registerWithAPIServer() - // Check the unschedulable taint. - got := gotNode.(*v1.Node) - unschedulableTaint := &v1.Taint{ - Key: schedulerapi.TaintNodeUnschedulable, - Effect: v1.TaintEffectNoSchedule, - } + // Check the unschedulable taint. + got := gotNode.(*v1.Node) + unschedulableTaint := &v1.Taint{ + Key: schedulerapi.TaintNodeUnschedulable, + Effect: v1.TaintEffectNoSchedule, + } - require.Equal(t, - utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition), - taintutil.TaintExists(got.Spec.Taints, unschedulableTaint), - "test unschedulable taint for TaintNodesByCondition") - }) + require.Equal(t, + true, + taintutil.TaintExists(got.Spec.Taints, unschedulableTaint), + "test unschedulable taint for TaintNodesByCondition") } func TestNodeStatusHasChanged(t *testing.T) { diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 80c6dcb73b6..7b92438807e 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -35,12 +35,9 @@ import ( utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/flowcontrol" - "k8s.io/component-base/featuregate" - featuregatetesting "k8s.io/component-base/featuregate/testing" cadvisortest "k8s.io/kubernetes/pkg/kubelet/cadvisor/testing" "k8s.io/kubernetes/pkg/kubelet/cm" "k8s.io/kubernetes/pkg/kubelet/config" @@ -2031,17 +2028,6 @@ func runVolumeManager(kubelet *Kubelet) chan struct{} { return stopCh } -func forEachFeatureGate(t *testing.T, fs []featuregate.Feature, tf func(t *testing.T)) { - for _, fg := range fs { - for _, f := range []bool{true, false} { - func() { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, fg, f)() - t.Run(fmt.Sprintf("%v(%t)", fg, f), tf) - }() - } - } -} - // Sort pods by UID. type podsByUID []*v1.Pod diff --git a/pkg/scheduler/algorithmprovider/BUILD b/pkg/scheduler/algorithmprovider/BUILD index b0fce90c5c3..71ad334411a 100644 --- a/pkg/scheduler/algorithmprovider/BUILD +++ b/pkg/scheduler/algorithmprovider/BUILD @@ -17,12 +17,7 @@ go_test( name = "go_default_test", srcs = ["plugins_test.go"], embed = [":go_default_library"], - deps = [ - "//pkg/features:go_default_library", - "//pkg/scheduler:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", - "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", - ], + deps = ["//pkg/scheduler:go_default_library"], ) filegroup( diff --git a/pkg/scheduler/algorithmprovider/defaults/defaults.go b/pkg/scheduler/algorithmprovider/defaults/defaults.go index 564af712002..a4c343abf65 100644 --- a/pkg/scheduler/algorithmprovider/defaults/defaults.go +++ b/pkg/scheduler/algorithmprovider/defaults/defaults.go @@ -47,12 +47,9 @@ func defaultPredicates() sets.String { predicates.MatchInterPodAffinityPred, predicates.NoDiskConflictPred, predicates.GeneralPred, - predicates.CheckNodeMemoryPressurePred, - predicates.CheckNodeDiskPressurePred, - predicates.CheckNodePIDPressurePred, - predicates.CheckNodeConditionPred, predicates.PodToleratesNodeTaintsPred, predicates.CheckVolumeBindingPred, + predicates.CheckNodeUnschedulablePred, ) } @@ -62,34 +59,6 @@ func defaultPredicates() sets.String { // of a feature gate temporarily. func ApplyFeatureGates() (restore func()) { snapshot := scheduler.RegisteredPredicatesAndPrioritiesSnapshot() - if utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition) { - // Remove "CheckNodeCondition", "CheckNodeMemoryPressure", "CheckNodePIDPressure" - // and "CheckNodeDiskPressure" predicates - scheduler.RemoveFitPredicate(predicates.CheckNodeConditionPred) - scheduler.RemoveFitPredicate(predicates.CheckNodeMemoryPressurePred) - scheduler.RemoveFitPredicate(predicates.CheckNodeDiskPressurePred) - scheduler.RemoveFitPredicate(predicates.CheckNodePIDPressurePred) - // Remove key "CheckNodeCondition", "CheckNodeMemoryPressure", "CheckNodePIDPressure" and "CheckNodeDiskPressure" - // from ALL algorithm provider - // The key will be removed from all providers which in algorithmProviderMap[] - // if you just want remove specific provider, call func RemovePredicateKeyFromAlgoProvider() - scheduler.RemovePredicateKeyFromAlgorithmProviderMap(predicates.CheckNodeConditionPred) - scheduler.RemovePredicateKeyFromAlgorithmProviderMap(predicates.CheckNodeMemoryPressurePred) - scheduler.RemovePredicateKeyFromAlgorithmProviderMap(predicates.CheckNodeDiskPressurePred) - scheduler.RemovePredicateKeyFromAlgorithmProviderMap(predicates.CheckNodePIDPressurePred) - - // Fit is determined based on whether a pod can tolerate all of the node's taints - scheduler.RegisterMandatoryFitPredicate(predicates.PodToleratesNodeTaintsPred, predicates.PodToleratesNodeTaints) - // Fit is determined based on whether a pod can tolerate unschedulable of node - scheduler.RegisterMandatoryFitPredicate(predicates.CheckNodeUnschedulablePred, predicates.CheckNodeUnschedulablePredicate) - // Insert Key "PodToleratesNodeTaints" and "CheckNodeUnschedulable" To All Algorithm Provider - // The key will insert to all providers which in algorithmProviderMap[] - // if you just want insert to specific provider, call func InsertPredicateKeyToAlgoProvider() - scheduler.InsertPredicateKeyToAlgorithmProviderMap(predicates.PodToleratesNodeTaintsPred) - scheduler.InsertPredicateKeyToAlgorithmProviderMap(predicates.CheckNodeUnschedulablePred) - - klog.Infof("TaintNodesByCondition is enabled, PodToleratesNodeTaints predicate is mandatory") - } // Only register EvenPodsSpread predicate & priority if the feature is enabled if utilfeature.DefaultFeatureGate.Enabled(features.EvenPodsSpread) { diff --git a/pkg/scheduler/algorithmprovider/defaults/defaults_test.go b/pkg/scheduler/algorithmprovider/defaults/defaults_test.go index 48696ba0264..3feaf3a8cb1 100644 --- a/pkg/scheduler/algorithmprovider/defaults/defaults_test.go +++ b/pkg/scheduler/algorithmprovider/defaults/defaults_test.go @@ -78,12 +78,9 @@ func TestDefaultPredicates(t *testing.T) { predicates.MatchInterPodAffinityPred, predicates.NoDiskConflictPred, predicates.GeneralPred, - predicates.CheckNodeMemoryPressurePred, - predicates.CheckNodeDiskPressurePred, - predicates.CheckNodePIDPressurePred, - predicates.CheckNodeConditionPred, predicates.PodToleratesNodeTaintsPred, predicates.CheckVolumeBindingPred, + predicates.CheckNodeUnschedulablePred, ) if expected := defaultPredicates(); !result.Equal(expected) { diff --git a/pkg/scheduler/algorithmprovider/defaults/register_predicates.go b/pkg/scheduler/algorithmprovider/defaults/register_predicates.go index a83da76d8c9..265b8fbaade 100644 --- a/pkg/scheduler/algorithmprovider/defaults/register_predicates.go +++ b/pkg/scheduler/algorithmprovider/defaults/register_predicates.go @@ -113,10 +113,13 @@ func init() { scheduler.RegisterFitPredicate(predicates.CheckNodePIDPressurePred, predicates.CheckNodePIDPressurePredicate) // Fit is determined by node conditions: not ready, network unavailable or out of disk. - scheduler.RegisterMandatoryFitPredicate(predicates.CheckNodeConditionPred, predicates.CheckNodeConditionPredicate) + scheduler.RegisterFitPredicate(predicates.CheckNodeConditionPred, predicates.CheckNodeConditionPredicate) // Fit is determined based on whether a pod can tolerate all of the node's taints - scheduler.RegisterFitPredicate(predicates.PodToleratesNodeTaintsPred, predicates.PodToleratesNodeTaints) + scheduler.RegisterMandatoryFitPredicate(predicates.PodToleratesNodeTaintsPred, predicates.PodToleratesNodeTaints) + + // Fit is determined based on whether a pod can tolerate unschedulable of node + scheduler.RegisterMandatoryFitPredicate(predicates.CheckNodeUnschedulablePred, predicates.CheckNodeUnschedulablePredicate) // Fit is determined by volume topology requirements. scheduler.RegisterFitPredicateFactory( diff --git a/pkg/scheduler/algorithmprovider/plugins_test.go b/pkg/scheduler/algorithmprovider/plugins_test.go index 9255a900801..71f1da44eca 100644 --- a/pkg/scheduler/algorithmprovider/plugins_test.go +++ b/pkg/scheduler/algorithmprovider/plugins_test.go @@ -20,9 +20,6 @@ import ( "fmt" "testing" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" - "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/scheduler" ) @@ -81,8 +78,8 @@ func TestApplyFeatureGates(t *testing.T) { t.Fatalf("Error retrieving provider: %v", err) } - if !p.FitPredicateKeys.Has("CheckNodeCondition") { - t.Fatalf("Failed to find predicate: 'CheckNodeCondition'") + if p.FitPredicateKeys.Has("CheckNodeCondition") { + t.Fatalf("Unexpected predicate: 'CheckNodeCondition'") } if !p.FitPredicateKeys.Has("PodToleratesNodeTaints") { @@ -91,9 +88,6 @@ func TestApplyFeatureGates(t *testing.T) { }) } - // Apply features for algorithm providers. - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintNodesByCondition, true)() - defer ApplyFeatureGates()() for _, pn := range algorithmProviderNames { diff --git a/pkg/scheduler/api/compatibility/compatibility_test.go b/pkg/scheduler/api/compatibility/compatibility_test.go index 8b2a9c258cb..cb0dcc77ac5 100644 --- a/pkg/scheduler/api/compatibility/compatibility_test.go +++ b/pkg/scheduler/api/compatibility/compatibility_test.go @@ -65,6 +65,7 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { {Name: "NodeName"}, {Name: "NodePorts"}, {Name: "NodeAffinity"}, + {Name: "TaintToleration"}, }, }, }, @@ -104,6 +105,7 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { {Name: "NodeAffinity"}, {Name: "NodeResources"}, {Name: "VolumeRestrictions"}, + {Name: "TaintToleration"}, }, }, }, @@ -150,6 +152,7 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { {Name: "NodeAffinity"}, {Name: "NodeResources"}, {Name: "VolumeRestrictions"}, + {Name: "TaintToleration"}, }, }, }, @@ -205,6 +208,7 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { {Name: "NodeAffinity"}, {Name: "NodeResources"}, {Name: "VolumeRestrictions"}, + {Name: "TaintToleration"}, {Name: "VolumeZone"}, }, "ScorePlugin": { @@ -1207,7 +1211,7 @@ func TestCompatibility_v1_Scheduler(t *testing.T) { registeredPriorities := sets.NewString(scheduler.ListRegisteredPriorityFunctions()...) seenPredicates := sets.NewString() seenPriorities := sets.NewString() - mandatoryPredicates := sets.NewString("CheckNodeCondition") + mandatoryPredicates := sets.NewString("CheckNodeUnschedulable") generalPredicateFilters := []string{"NodeResources", "NodeName", "NodePorts", "NodeAffinity"} filterToPredicateMap := map[string]string{ "TaintToleration": "PodToleratesNodeTaints", diff --git a/plugin/pkg/admission/nodetaint/BUILD b/plugin/pkg/admission/nodetaint/BUILD index c01864b455d..147e1dff2ee 100644 --- a/plugin/pkg/admission/nodetaint/BUILD +++ b/plugin/pkg/admission/nodetaint/BUILD @@ -7,10 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/apis/core:go_default_library", - "//pkg/features:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", - "//staging/src/k8s.io/component-base/featuregate:go_default_library", ], ) @@ -20,12 +17,10 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/core:go_default_library", - "//pkg/features:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library", - "//staging/src/k8s.io/component-base/featuregate:go_default_library", ], ) diff --git a/plugin/pkg/admission/nodetaint/admission.go b/plugin/pkg/admission/nodetaint/admission.go index 84a1e0f4224..b23c91f25a0 100644 --- a/plugin/pkg/admission/nodetaint/admission.go +++ b/plugin/pkg/admission/nodetaint/admission.go @@ -22,10 +22,7 @@ import ( "io" "k8s.io/apiserver/pkg/admission" - utilfeature "k8s.io/apiserver/pkg/util/feature" - "k8s.io/component-base/featuregate" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" ) const ( @@ -46,16 +43,13 @@ func Register(plugins *admission.Plugins) { // This plugin identifies requests from nodes func NewPlugin() *Plugin { return &Plugin{ - Handler: admission.NewHandler(admission.Create), - features: utilfeature.DefaultFeatureGate, + Handler: admission.NewHandler(admission.Create), } } // Plugin holds state for and implements the admission plugin. type Plugin struct { *admission.Handler - // allows overriding for testing - features featuregate.FeatureGate } var ( @@ -68,11 +62,6 @@ var ( // Admit is the main function that checks node identity and adds taints as needed. func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { - // If TaintNodesByCondition is not enabled, we don't need to do anything. - if !p.features.Enabled(features.TaintNodesByCondition) { - return nil - } - // Our job is just to taint nodes. if a.GetResource().GroupResource() != nodeResource || a.GetSubresource() != "" { return nil @@ -83,11 +72,11 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission. return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject())) } - // Taint node with NotReady taint at creation if TaintNodesByCondition is - // enabled. This is needed to make sure that nodes are added to the cluster - // with the NotReady taint. Otherwise, a new node may receive the taint with - // some delay causing pods to be scheduled on a not-ready node. - // Node controller will remove the taint when the node becomes ready. + // Taint node with NotReady taint at creation. This is needed to make sure + // that nodes are added to the cluster with the NotReady taint. Otherwise, + // a new node may receive the taint with some delay causing pods to be + // scheduled on a not-ready node. Node controller will remove the taint + // when the node becomes ready. addNotReadyTaint(node) return nil } diff --git a/plugin/pkg/admission/nodetaint/admission_test.go b/plugin/pkg/admission/nodetaint/admission_test.go index ec2ca7299ba..208946f2465 100644 --- a/plugin/pkg/admission/nodetaint/admission_test.go +++ b/plugin/pkg/admission/nodetaint/admission_test.go @@ -26,25 +26,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/authentication/user" - "k8s.io/component-base/featuregate" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" ) -var ( - enableTaintNodesByCondition = featuregate.NewFeatureGate() - disableTaintNodesByCondition = featuregate.NewFeatureGate() -) - -func init() { - if err := enableTaintNodesByCondition.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TaintNodesByCondition: {Default: true}}); err != nil { - panic(err) - } - if err := disableTaintNodesByCondition.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TaintNodesByCondition: {Default: false}}); err != nil { - panic(err) - } -} - func Test_nodeTaints(t *testing.T) { var ( mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}} @@ -63,7 +47,6 @@ func Test_nodeTaints(t *testing.T) { name string node api.Node oldNode api.Node - features featuregate.FeatureGate operation admission.Operation options runtime.Object expectedTaints []api.Taint @@ -71,23 +54,13 @@ func Test_nodeTaints(t *testing.T) { { name: "notReady taint is added on creation", node: myNodeObj, - features: enableTaintNodesByCondition, operation: admission.Create, options: &metav1.CreateOptions{}, expectedTaints: []api.Taint{notReadyTaint}, }, - { - name: "NotReady taint is not added when TaintNodesByCondition is disabled", - node: myNodeObj, - features: disableTaintNodesByCondition, - operation: admission.Create, - options: &metav1.CreateOptions{}, - expectedTaints: nil, - }, { name: "already tainted node is not tainted again", node: myTaintedNodeObj, - features: enableTaintNodesByCondition, operation: admission.Create, options: &metav1.CreateOptions{}, expectedTaints: []api.Taint{notReadyTaint}, @@ -95,7 +68,6 @@ func Test_nodeTaints(t *testing.T) { { name: "NotReady taint is added to an unready node as well", node: myUnreadyNodeObj, - features: enableTaintNodesByCondition, operation: admission.Create, options: &metav1.CreateOptions{}, expectedTaints: []api.Taint{notReadyTaint}, @@ -105,9 +77,6 @@ func Test_nodeTaints(t *testing.T) { t.Run(tt.name, func(t *testing.T) { attributes := admission.NewAttributesRecord(&tt.node, &tt.oldNode, nodeKind, myNodeObj.Namespace, myNodeObj.Name, resource, "", tt.operation, tt.options, false, mynode) c := NewPlugin() - if tt.features != nil { - c.features = tt.features - } err := c.Admit(context.TODO(), attributes, nil) if err != nil { t.Errorf("nodePlugin.Admit() error = %v", err) diff --git a/plugin/pkg/admission/podtolerationrestriction/BUILD b/plugin/pkg/admission/podtolerationrestriction/BUILD index 98aa60a1b35..3b57483d30b 100644 --- a/plugin/pkg/admission/podtolerationrestriction/BUILD +++ b/plugin/pkg/admission/podtolerationrestriction/BUILD @@ -12,7 +12,6 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/core:go_default_library", - "//pkg/features:go_default_library", "//pkg/scheduler/api:go_default_library", "//plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", @@ -21,11 +20,9 @@ go_test( "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/testing:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", - "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], ) diff --git a/plugin/pkg/admission/podtolerationrestriction/admission_test.go b/plugin/pkg/admission/podtolerationrestriction/admission_test.go index fcf9c545eba..66c972cb64d 100644 --- a/plugin/pkg/admission/podtolerationrestriction/admission_test.go +++ b/plugin/pkg/admission/podtolerationrestriction/admission_test.go @@ -29,13 +29,10 @@ import ( "k8s.io/apiserver/pkg/admission" genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer" admissiontesting "k8s.io/apiserver/pkg/admission/testing" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/fake" - featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" schedulerapi "k8s.io/kubernetes/pkg/scheduler/api" pluginapi "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction" ) @@ -87,8 +84,6 @@ func TestPodAdmission(t *testing.T) { }, } - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintNodesByCondition, true)() - tests := []struct { pod *api.Pod defaultClusterTolerations []api.Toleration diff --git a/test/integration/daemonset/daemonset_test.go b/test/integration/daemonset/daemonset_test.go index 8b4e7c5d347..5dffcfbe871 100644 --- a/test/integration/daemonset/daemonset_test.go +++ b/test/integration/daemonset/daemonset_test.go @@ -966,10 +966,8 @@ func TestTaintedNode(t *testing.T) { } // TestUnschedulableNodeDaemonDoesLaunchPod tests that the DaemonSet Pods can still be scheduled -// to the Unschedulable nodes when TaintNodesByCondition are enabled. +// to the Unschedulable nodes. func TestUnschedulableNodeDaemonDoesLaunchPod(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintNodesByCondition, true)() - forEachFeatureGate(t, func(t *testing.T) { forEachStrategy(t, func(t *testing.T, strategy *apps.DaemonSetUpdateStrategy) { server, closeFn, dc, informers, clientset := setup(t) diff --git a/test/integration/scheduler/predicates_test.go b/test/integration/scheduler/predicates_test.go index b568b787b95..67b3e28c7d1 100644 --- a/test/integration/scheduler/predicates_test.go +++ b/test/integration/scheduler/predicates_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" @@ -877,56 +877,6 @@ func TestInterPodAffinity(t *testing.T) { } } -// TestNodePIDPressure verifies that scheduler's CheckNodePIDPressurePredicate predicate -// functions works correctly. -func TestNodePIDPressure(t *testing.T) { - context := initTest(t, "node-pid-pressure") - defer cleanupTest(t, context) - // Add a node. - node, err := createNode(context.clientSet, "testnode", nil) - if err != nil { - t.Fatalf("Cannot create node: %v", err) - } - - cs := context.clientSet - - // Adds PID pressure condition to the node. - node.Status.Conditions = []v1.NodeCondition{ - { - Type: v1.NodePIDPressure, - Status: v1.ConditionTrue, - }, - } - - // Update node condition. - err = updateNodeStatus(context.clientSet, node) - if err != nil { - t.Fatalf("Cannot update node: %v", err) - } - - // Create test pod. - testPod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: "pidpressure-fake-name"}, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - {Name: "container", Image: imageutils.GetPauseImageName()}, - }, - }, - } - - testPod, err = cs.CoreV1().Pods(context.ns.Name).Create(testPod) - if err != nil { - t.Fatalf("Test Failed: error: %v, while creating pod", err) - } - - err = waitForPodUnschedulable(cs, testPod) - if err != nil { - t.Errorf("Test Failed: error, %v, while waiting for scheduled", err) - } - - cleanupPods(cs, t, []*v1.Pod{testPod}) -} - // TestEvenPodsSpreadPredicate verifies that EvenPodsSpread predicate functions well. func TestEvenPodsSpreadPredicate(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.EvenPodsSpread, true)() diff --git a/test/integration/scheduler/scheduler_test.go b/test/integration/scheduler/scheduler_test.go index bd9280cbdb9..10d37bbe32c 100644 --- a/test/integration/scheduler/scheduler_test.go +++ b/test/integration/scheduler/scheduler_test.go @@ -110,7 +110,7 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { ] }`, expectedPredicates: sets.NewString( - "CheckNodeCondition", // mandatory predicate + "CheckNodeUnschedulable", // mandatory predicate "PredicateOne", "PredicateTwo", ), @@ -118,6 +118,11 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { "PriorityOne", "PriorityTwo", ), + expectedPlugins: map[string][]kubeschedulerconfig.Plugin{ + "FilterPlugin": { + {Name: "TaintToleration"}, + }, + }, }, { policy: `{ @@ -125,10 +130,7 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { "apiVersion" : "v1" }`, expectedPredicates: sets.NewString( - "CheckNodeCondition", // mandatory predicate - "CheckNodeDiskPressure", - "CheckNodeMemoryPressure", - "CheckNodePIDPressure", + "CheckNodeUnschedulable", // mandatory predicate "MaxAzureDiskVolumeCount", "MaxEBSVolumeCount", "MaxGCEPDVolumeCount", @@ -168,9 +170,14 @@ func TestSchedulerCreationFromConfigMap(t *testing.T) { "priorities" : [] }`, expectedPredicates: sets.NewString( - "CheckNodeCondition", // mandatory predicate + "CheckNodeUnschedulable", // mandatory predicate ), expectedPrioritizers: sets.NewString(), + expectedPlugins: map[string][]kubeschedulerconfig.Plugin{ + "FilterPlugin": { + {Name: "TaintToleration"}, + }, + }, }, { policy: `apiVersion: v1 @@ -185,7 +192,7 @@ priorities: weight: 5 `, expectedPredicates: sets.NewString( - "CheckNodeCondition", // mandatory predicate + "CheckNodeUnschedulable", // mandatory predicate "PredicateOne", "PredicateTwo", ), @@ -193,16 +200,18 @@ priorities: "PriorityOne", "PriorityTwo", ), + expectedPlugins: map[string][]kubeschedulerconfig.Plugin{ + "FilterPlugin": { + {Name: "TaintToleration"}, + }, + }, }, { policy: `apiVersion: v1 kind: Policy `, expectedPredicates: sets.NewString( - "CheckNodeCondition", // mandatory predicate - "CheckNodeDiskPressure", - "CheckNodeMemoryPressure", - "CheckNodePIDPressure", + "CheckNodeUnschedulable", // mandatory predicate "MaxAzureDiskVolumeCount", "MaxEBSVolumeCount", "MaxGCEPDVolumeCount", @@ -241,9 +250,14 @@ predicates: [] priorities: [] `, expectedPredicates: sets.NewString( - "CheckNodeCondition", // mandatory predicate + "CheckNodeUnschedulable", // mandatory predicate ), expectedPrioritizers: sets.NewString(), + expectedPlugins: map[string][]kubeschedulerconfig.Plugin{ + "FilterPlugin": { + {Name: "TaintToleration"}, + }, + }, }, } { // Add a ConfigMap object. @@ -362,12 +376,6 @@ func TestUnschedulableNodes(t *testing.T) { Reason: fmt.Sprintf("schedulable condition"), LastHeartbeatTime: metav1.Time{Time: time.Now()}, } - badCondition := v1.NodeCondition{ - Type: v1.NodeReady, - Status: v1.ConditionUnknown, - Reason: fmt.Sprintf("unschedulable condition"), - LastHeartbeatTime: metav1.Time{Time: time.Now()}, - } // Create a new schedulable node, since we're first going to apply // the unschedulable condition and verify that pods aren't scheduled. node := &v1.Node{ @@ -426,43 +434,6 @@ func TestUnschedulableNodes(t *testing.T) { } }, }, - // Test node.Status.Conditions=ConditionTrue/Unknown - { - makeUnSchedulable: func(t *testing.T, n *v1.Node, nodeLister corelisters.NodeLister, c clientset.Interface) { - n.Status = v1.NodeStatus{ - Capacity: v1.ResourceList{ - v1.ResourcePods: *resource.NewQuantity(32, resource.DecimalSI), - }, - Conditions: []v1.NodeCondition{badCondition}, - } - if _, err = c.CoreV1().Nodes().UpdateStatus(n); err != nil { - t.Fatalf("Failed to update node with bad status condition: %v", err) - } - err = waitForReflection(t, nodeLister, nodeKey, func(node interface{}) bool { - return node != nil && node.(*v1.Node).Status.Conditions[0].Status == v1.ConditionUnknown - }) - if err != nil { - t.Fatalf("Failed to observe reflected update for status condition update: %v", err) - } - }, - makeSchedulable: func(t *testing.T, n *v1.Node, nodeLister corelisters.NodeLister, c clientset.Interface) { - n.Status = v1.NodeStatus{ - Capacity: v1.ResourceList{ - v1.ResourcePods: *resource.NewQuantity(32, resource.DecimalSI), - }, - Conditions: []v1.NodeCondition{goodCondition}, - } - if _, err = c.CoreV1().Nodes().UpdateStatus(n); err != nil { - t.Fatalf("Failed to update node with healthy status condition: %v", err) - } - err = waitForReflection(t, nodeLister, nodeKey, func(node interface{}) bool { - return node != nil && node.(*v1.Node).Status.Conditions[0].Status == v1.ConditionTrue - }) - if err != nil { - t.Fatalf("Failed to observe reflected update for status condition update: %v", err) - } - }, - }, } for i, mod := range nodeModifications { @@ -484,7 +455,7 @@ func TestUnschedulableNodes(t *testing.T) { // There are no schedulable nodes - the pod shouldn't be scheduled. err = waitForPodToScheduleWithTimeout(context.clientSet, myPod, 2*time.Second) if err == nil { - t.Errorf("Pod scheduled successfully on unschedulable nodes") + t.Errorf("Test %d: Pod scheduled successfully on unschedulable nodes", i) } if err != wait.ErrWaitTimeout { t.Errorf("Test %d: failed while trying to confirm the pod does not get scheduled on the node: %v", i, err) diff --git a/test/integration/scheduler/taint_test.go b/test/integration/scheduler/taint_test.go index 39590419012..222612a5bf0 100644 --- a/test/integration/scheduler/taint_test.go +++ b/test/integration/scheduler/taint_test.go @@ -67,9 +67,6 @@ func newPod(nsName, name string, req, limit v1.ResourceList) *v1.Pod { // TestTaintNodeByCondition tests related cases for TaintNodeByCondition feature. func TestTaintNodeByCondition(t *testing.T) { - // Enable TaintNodeByCondition - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.TaintNodesByCondition, true)() - // Build PodToleration Admission. admission := podtolerationrestriction.NewPodTolerationsPlugin(&pluginapi.Configuration{}) @@ -110,7 +107,6 @@ func TestTaintNodeByCondition(t *testing.T) { 100, // Unhealthy zone threshold true, // Run taint manager true, // Use taint based evictions - true, // Enabled TaintNodeByCondition feature ) if err != nil { t.Errorf("Failed to create node controller: %v", err) @@ -539,7 +535,12 @@ func TestTaintNodeByCondition(t *testing.T) { t.Errorf("Failed to create node, err: %v", err) } if err := waitForNodeTaints(cs, node, test.expectedTaints); err != nil { - t.Errorf("Failed to taint node <%s>, err: %v", node.Name, err) + node, err = cs.CoreV1().Nodes().Get(node.Name, metav1.GetOptions{}) + if err != nil { + t.Errorf("Failed to get node <%s>", node.Name) + } + + t.Errorf("Failed to taint node <%s>, expected: %v, got: %v, err: %v", node.Name, test.expectedTaints, node.Spec.Taints, err) } var pods []*v1.Pod @@ -689,7 +690,6 @@ func TestTaintBasedEvictions(t *testing.T) { 0.55, // Unhealthy zone threshold true, // Run taint manager true, // Use taint based evictions - true, // Enabled TaintNodeByCondition feature ) if err != nil { t.Errorf("Failed to create node controller: %v", err) diff --git a/test/integration/scheduler/util.go b/test/integration/scheduler/util.go index bd2467acfb2..2ad9f304123 100644 --- a/test/integration/scheduler/util.go +++ b/test/integration/scheduler/util.go @@ -212,6 +212,7 @@ func initTestSchedulerWithOptions( context.informerFactory.WaitForCacheSync(context.scheduler.StopEverything) go context.scheduler.Run(context.ctx) + return context }