From 578ff3ec343d1f165a06e943a5ec2bd7111aae89 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 26 Jan 2021 14:03:08 -0500 Subject: [PATCH] Move Taint/Toleration helpers to component-helpers repo This is part of the goal for scheduling to remove dependencies on internal packages for the scheduling framework. It also provides these functions in an external location for other components and projects to import. --- pkg/apis/core/v1/helper/helpers.go | 47 ------- pkg/apis/core/v1/helper/helpers_test.go | 120 ----------------- pkg/controller/daemon/BUILD | 2 +- pkg/controller/daemon/daemon_controller.go | 9 +- pkg/kubelet/eviction/BUILD | 1 - pkg/kubelet/eviction/eviction_manager.go | 2 +- .../framework/plugins/nodeunschedulable/BUILD | 2 +- .../nodeunschedulable/node_unschedulable.go | 2 +- .../framework/plugins/tainttoleration/BUILD | 2 +- .../tainttoleration/taint_toleration.go | 2 +- .../scheduling/corev1/helpers.go | 41 ++++++ .../scheduling/corev1/helpers_test.go | 121 ++++++++++++++++++ test/integration/framework/BUILD | 2 +- test/integration/framework/util.go | 5 +- 14 files changed, 177 insertions(+), 181 deletions(-) diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index d60ca300b09..a533b054e68 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -316,53 +316,6 @@ func AddOrUpdateTolerationInPod(pod *v1.Pod, toleration *v1.Toleration) bool { return AddOrUpdateTolerationInPodSpec(&pod.Spec, toleration) } -// TolerationsTolerateTaint checks if taint is tolerated by any of the tolerations. -func TolerationsTolerateTaint(tolerations []v1.Toleration, taint *v1.Taint) bool { - for i := range tolerations { - if tolerations[i].ToleratesTaint(taint) { - return true - } - } - return false -} - -type taintsFilterFunc func(*v1.Taint) bool - -// TolerationsTolerateTaintsWithFilter checks if given tolerations tolerates -// all the taints that apply to the filter in given taint list. -// DEPRECATED: Please use FindMatchingUntoleratedTaint instead. -func TolerationsTolerateTaintsWithFilter(tolerations []v1.Toleration, taints []v1.Taint, applyFilter taintsFilterFunc) bool { - _, isUntolerated := FindMatchingUntoleratedTaint(taints, tolerations, applyFilter) - return !isUntolerated -} - -// FindMatchingUntoleratedTaint checks if the given tolerations tolerates -// all the filtered taints, and returns the first taint without a toleration -func FindMatchingUntoleratedTaint(taints []v1.Taint, tolerations []v1.Toleration, inclusionFilter taintsFilterFunc) (v1.Taint, bool) { - filteredTaints := getFilteredTaints(taints, inclusionFilter) - for _, taint := range filteredTaints { - if !TolerationsTolerateTaint(tolerations, &taint) { - return taint, true - } - } - return v1.Taint{}, false -} - -// getFilteredTaints returns a list of taints satisfying the filter predicate -func getFilteredTaints(taints []v1.Taint, inclusionFilter taintsFilterFunc) []v1.Taint { - if inclusionFilter == nil { - return taints - } - filteredTaints := []v1.Taint{} - for _, taint := range taints { - if !inclusionFilter(&taint) { - continue - } - filteredTaints = append(filteredTaints, taint) - } - return filteredTaints -} - // GetMatchingTolerations returns true and list of Tolerations matching all Taints if all are tolerated, or false otherwise. func GetMatchingTolerations(taints []v1.Taint, tolerations []v1.Toleration) (bool, []v1.Toleration) { if len(taints) == 0 { diff --git a/pkg/apis/core/v1/helper/helpers_test.go b/pkg/apis/core/v1/helper/helpers_test.go index fedff439840..db10d288036 100644 --- a/pkg/apis/core/v1/helper/helpers_test.go +++ b/pkg/apis/core/v1/helper/helpers_test.go @@ -309,126 +309,6 @@ func TestTopologySelectorRequirementsAsSelector(t *testing.T) { } } -func TestTolerationsTolerateTaintsWithFilter(t *testing.T) { - testCases := []struct { - description string - tolerations []v1.Toleration - taints []v1.Taint - applyFilter taintsFilterFunc - expectTolerated bool - }{ - { - description: "empty tolerations tolerate empty taints", - tolerations: []v1.Toleration{}, - taints: []v1.Taint{}, - applyFilter: func(t *v1.Taint) bool { return true }, - expectTolerated: true, - }, - { - description: "non-empty tolerations tolerate empty taints", - tolerations: []v1.Toleration{ - { - Key: "foo", - Operator: "Exists", - Effect: v1.TaintEffectNoSchedule, - }, - }, - taints: []v1.Taint{}, - applyFilter: func(t *v1.Taint) bool { return true }, - expectTolerated: true, - }, - { - description: "tolerations match all taints, expect tolerated", - tolerations: []v1.Toleration{ - { - Key: "foo", - Operator: "Exists", - Effect: v1.TaintEffectNoSchedule, - }, - }, - taints: []v1.Taint{ - { - Key: "foo", - Effect: v1.TaintEffectNoSchedule, - }, - }, - applyFilter: func(t *v1.Taint) bool { return true }, - expectTolerated: true, - }, - { - description: "tolerations don't match taints, but no taints apply to the filter, expect tolerated", - tolerations: []v1.Toleration{ - { - Key: "foo", - Operator: "Exists", - Effect: v1.TaintEffectNoSchedule, - }, - }, - taints: []v1.Taint{ - { - Key: "bar", - Effect: v1.TaintEffectNoSchedule, - }, - }, - applyFilter: func(t *v1.Taint) bool { return false }, - expectTolerated: true, - }, - { - description: "no filterFunc indicated, means all taints apply to the filter, tolerations don't match taints, expect untolerated", - tolerations: []v1.Toleration{ - { - Key: "foo", - Operator: "Exists", - Effect: v1.TaintEffectNoSchedule, - }, - }, - taints: []v1.Taint{ - { - Key: "bar", - Effect: v1.TaintEffectNoSchedule, - }, - }, - applyFilter: nil, - expectTolerated: false, - }, - { - description: "tolerations match taints, expect tolerated", - tolerations: []v1.Toleration{ - { - Key: "foo", - Operator: "Exists", - Effect: v1.TaintEffectNoExecute, - }, - }, - taints: []v1.Taint{ - { - Key: "foo", - Effect: v1.TaintEffectNoExecute, - }, - { - Key: "bar", - Effect: v1.TaintEffectNoSchedule, - }, - }, - applyFilter: func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute }, - expectTolerated: true, - }, - } - - for _, tc := range testCases { - if tc.expectTolerated != TolerationsTolerateTaintsWithFilter(tc.tolerations, tc.taints, tc.applyFilter) { - filteredTaints := []v1.Taint{} - for _, taint := range tc.taints { - if tc.applyFilter != nil && !tc.applyFilter(&taint) { - continue - } - filteredTaints = append(filteredTaints, taint) - } - t.Errorf("[%s] expect tolerations %+v tolerate filtered taints %+v in taints %+v", tc.description, tc.tolerations, filteredTaints, tc.taints) - } - } -} - func TestMatchTopologySelectorTerms(t *testing.T) { type args struct { topologySelectorTerms []v1.TopologySelectorTerm diff --git a/pkg/controller/daemon/BUILD b/pkg/controller/daemon/BUILD index 04961a81e83..a8684c27c01 100644 --- a/pkg/controller/daemon/BUILD +++ b/pkg/controller/daemon/BUILD @@ -16,7 +16,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/controller/daemon", deps = [ "//pkg/api/v1/pod:go_default_library", - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/controller:go_default_library", "//pkg/controller/daemon/util:go_default_library", "//pkg/scheduler/framework/plugins/helper:go_default_library", @@ -46,6 +45,7 @@ go_library( "//staging/src/k8s.io/client-go/util/flowcontrol:go_default_library", "//staging/src/k8s.io/client-go/util/workqueue:go_default_library", "//staging/src/k8s.io/component-base/metrics/prometheus/ratelimiter:go_default_library", + "//staging/src/k8s.io/component-helpers/scheduling/corev1:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", "//vendor/k8s.io/utils/integer:go_default_library", ], diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go index a0fc135156e..90c4ea1aec8 100644 --- a/pkg/controller/daemon/daemon_controller.go +++ b/pkg/controller/daemon/daemon_controller.go @@ -49,8 +49,8 @@ import ( "k8s.io/client-go/util/flowcontrol" "k8s.io/client-go/util/workqueue" "k8s.io/component-base/metrics/prometheus/ratelimiter" + v1helper "k8s.io/component-helpers/scheduling/corev1" podutil "k8s.io/kubernetes/pkg/api/v1/pod" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/daemon/util" pluginhelper "k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper" @@ -1238,10 +1238,10 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *apps. if !fitsTaints { // Scheduled daemon pods should continue running if they tolerate NoExecute taint. - shouldContinueRunning := v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, func(t *v1.Taint) bool { + _, untolerated := v1helper.FindMatchingUntoleratedTaint(taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute }) - return false, shouldContinueRunning, nil + return false, !untolerated, nil } return true, true, nil @@ -1251,9 +1251,10 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *apps. func Predicates(pod *v1.Pod, node *v1.Node, taints []v1.Taint) (fitsNodeName, fitsNodeAffinity, fitsTaints bool) { fitsNodeName = len(pod.Spec.NodeName) == 0 || pod.Spec.NodeName == node.Name fitsNodeAffinity = pluginhelper.PodMatchesNodeSelectorAndAffinityTerms(pod, node) - fitsTaints = v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, func(t *v1.Taint) bool { + _, untolerated := v1helper.FindMatchingUntoleratedTaint(taints, pod.Spec.Tolerations, func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute || t.Effect == v1.TaintEffectNoSchedule }) + fitsTaints = !untolerated return } diff --git a/pkg/kubelet/eviction/BUILD b/pkg/kubelet/eviction/BUILD index a8727ecc7a4..bbbdc36e979 100644 --- a/pkg/kubelet/eviction/BUILD +++ b/pkg/kubelet/eviction/BUILD @@ -49,7 +49,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/kubelet/eviction", deps = [ "//pkg/api/v1/resource:go_default_library", - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/apis/core/v1/helper/qos:go_default_library", "//pkg/features:go_default_library", "//pkg/kubelet/cm:go_default_library", diff --git a/pkg/kubelet/eviction/eviction_manager.go b/pkg/kubelet/eviction/eviction_manager.go index 34c2ba9f081..415e6dd940e 100644 --- a/pkg/kubelet/eviction/eviction_manager.go +++ b/pkg/kubelet/eviction/eviction_manager.go @@ -29,9 +29,9 @@ import ( "k8s.io/apimachinery/pkg/util/clock" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/tools/record" + v1helper "k8s.io/component-helpers/scheduling/corev1" statsapi "k8s.io/kubelet/pkg/apis/stats/v1alpha1" apiv1resource "k8s.io/kubernetes/pkg/api/v1/resource" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" v1qos "k8s.io/kubernetes/pkg/apis/core/v1/helper/qos" "k8s.io/kubernetes/pkg/features" evictionapi "k8s.io/kubernetes/pkg/kubelet/eviction/api" diff --git a/pkg/scheduler/framework/plugins/nodeunschedulable/BUILD b/pkg/scheduler/framework/plugins/nodeunschedulable/BUILD index 64c918b7a23..169ae33bd3a 100644 --- a/pkg/scheduler/framework/plugins/nodeunschedulable/BUILD +++ b/pkg/scheduler/framework/plugins/nodeunschedulable/BUILD @@ -6,10 +6,10 @@ go_library( importpath = "k8s.io/kubernetes/pkg/scheduler/framework/plugins/nodeunschedulable", visibility = ["//visibility:public"], deps = [ - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/scheduler/framework:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/component-helpers/scheduling/corev1:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go index 4b17e2d3edf..6f128d6ab06 100644 --- a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go +++ b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable.go @@ -21,7 +21,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" + v1helper "k8s.io/component-helpers/scheduling/corev1" "k8s.io/kubernetes/pkg/scheduler/framework" ) diff --git a/pkg/scheduler/framework/plugins/tainttoleration/BUILD b/pkg/scheduler/framework/plugins/tainttoleration/BUILD index ff88b1d0cb5..0de152ea44a 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/BUILD +++ b/pkg/scheduler/framework/plugins/tainttoleration/BUILD @@ -6,11 +6,11 @@ go_library( importpath = "k8s.io/kubernetes/pkg/scheduler/framework/plugins/tainttoleration", visibility = ["//visibility:public"], deps = [ - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/scheduler/framework:go_default_library", "//pkg/scheduler/framework/plugins/helper:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/component-helpers/scheduling/corev1:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go index 769fe372a1c..b60e1ea9dc1 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration.go @@ -22,7 +22,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" + v1helper "k8s.io/component-helpers/scheduling/corev1" "k8s.io/kubernetes/pkg/scheduler/framework" pluginhelper "k8s.io/kubernetes/pkg/scheduler/framework/plugins/helper" ) diff --git a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go index a21aebbff4f..23a94bd7bcd 100644 --- a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go +++ b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers.go @@ -58,3 +58,44 @@ func GetAvoidPodsFromNodeAnnotations(annotations map[string]string) (v1.AvoidPod } return avoidPods, nil } + +// TolerationsTolerateTaint checks if taint is tolerated by any of the tolerations. +func TolerationsTolerateTaint(tolerations []v1.Toleration, taint *v1.Taint) bool { + for i := range tolerations { + if tolerations[i].ToleratesTaint(taint) { + return true + } + } + return false +} + +type taintsFilterFunc func(*v1.Taint) bool + +// FindMatchingUntoleratedTaint checks if the given tolerations tolerates +// all the filtered taints, and returns the first taint without a toleration +// Returns true if there is an untolerated taint +// Returns false if all taints are tolerated +func FindMatchingUntoleratedTaint(taints []v1.Taint, tolerations []v1.Toleration, inclusionFilter taintsFilterFunc) (v1.Taint, bool) { + filteredTaints := getFilteredTaints(taints, inclusionFilter) + for _, taint := range filteredTaints { + if !TolerationsTolerateTaint(tolerations, &taint) { + return taint, true + } + } + return v1.Taint{}, false +} + +// getFilteredTaints returns a list of taints satisfying the filter predicate +func getFilteredTaints(taints []v1.Taint, inclusionFilter taintsFilterFunc) []v1.Taint { + if inclusionFilter == nil { + return taints + } + filteredTaints := []v1.Taint{} + for _, taint := range taints { + if !inclusionFilter(&taint) { + continue + } + filteredTaints = append(filteredTaints, taint) + } + return filteredTaints +} diff --git a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go index f82ea7911dd..54679d17e94 100644 --- a/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go +++ b/staging/src/k8s.io/component-helpers/scheduling/corev1/helpers_test.go @@ -642,3 +642,124 @@ func TestGetAvoidPodsFromNode(t *testing.T) { } } } + +func TestFindMatchingUntoleratedTaint(t *testing.T) { + testCases := []struct { + description string + tolerations []v1.Toleration + taints []v1.Taint + applyFilter taintsFilterFunc + expectTolerated bool + }{ + { + description: "empty tolerations tolerate empty taints", + tolerations: []v1.Toleration{}, + taints: []v1.Taint{}, + applyFilter: func(t *v1.Taint) bool { return true }, + expectTolerated: true, + }, + { + description: "non-empty tolerations tolerate empty taints", + tolerations: []v1.Toleration{ + { + Key: "foo", + Operator: "Exists", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{}, + applyFilter: func(t *v1.Taint) bool { return true }, + expectTolerated: true, + }, + { + description: "tolerations match all taints, expect tolerated", + tolerations: []v1.Toleration{ + { + Key: "foo", + Operator: "Exists", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{ + { + Key: "foo", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: func(t *v1.Taint) bool { return true }, + expectTolerated: true, + }, + { + description: "tolerations don't match taints, but no taints apply to the filter, expect tolerated", + tolerations: []v1.Toleration{ + { + Key: "foo", + Operator: "Exists", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{ + { + Key: "bar", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: func(t *v1.Taint) bool { return false }, + expectTolerated: true, + }, + { + description: "no filterFunc indicated, means all taints apply to the filter, tolerations don't match taints, expect untolerated", + tolerations: []v1.Toleration{ + { + Key: "foo", + Operator: "Exists", + Effect: v1.TaintEffectNoSchedule, + }, + }, + taints: []v1.Taint{ + { + Key: "bar", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: nil, + expectTolerated: false, + }, + { + description: "tolerations match taints, expect tolerated", + tolerations: []v1.Toleration{ + { + Key: "foo", + Operator: "Exists", + Effect: v1.TaintEffectNoExecute, + }, + }, + taints: []v1.Taint{ + { + Key: "foo", + Effect: v1.TaintEffectNoExecute, + }, + { + Key: "bar", + Effect: v1.TaintEffectNoSchedule, + }, + }, + applyFilter: func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute }, + expectTolerated: true, + }, + } + + for _, tc := range testCases { + _, untolerated := FindMatchingUntoleratedTaint(tc.taints, tc.tolerations, tc.applyFilter) + if tc.expectTolerated != !untolerated { + filteredTaints := []v1.Taint{} + for _, taint := range tc.taints { + if tc.applyFilter != nil && !tc.applyFilter(&taint) { + continue + } + filteredTaints = append(filteredTaints, taint) + } + t.Errorf("[%s] expect tolerations %+v tolerate filtered taints %+v in taints %+v", tc.description, tc.tolerations, filteredTaints, tc.taints) + } + } +} diff --git a/test/integration/framework/BUILD b/test/integration/framework/BUILD index 486199c747e..cbefe14fb3d 100644 --- a/test/integration/framework/BUILD +++ b/test/integration/framework/BUILD @@ -38,7 +38,6 @@ go_library( "//cmd/kube-apiserver/app:go_default_library", "//cmd/kube-apiserver/app/options:go_default_library", "//pkg/api/legacyscheme:go_default_library", - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/controller/nodelifecycle:go_default_library", "//pkg/controlplane:go_default_library", "//pkg/generated/openapi:go_default_library", @@ -73,6 +72,7 @@ go_library( "//staging/src/k8s.io/client-go/rest:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", "//staging/src/k8s.io/component-base/version:go_default_library", + "//staging/src/k8s.io/component-helpers/scheduling/corev1:go_default_library", "//test/utils:go_default_library", "//vendor/github.com/go-openapi/spec:go_default_library", "//vendor/github.com/google/uuid:go_default_library", diff --git a/test/integration/framework/util.go b/test/integration/framework/util.go index d5ffc81e91c..76c21bda30a 100644 --- a/test/integration/framework/util.go +++ b/test/integration/framework/util.go @@ -31,8 +31,8 @@ import ( "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" + v1helper "k8s.io/component-helpers/scheduling/corev1" "k8s.io/klog/v2" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" nodectlr "k8s.io/kubernetes/pkg/controller/nodelifecycle" ) @@ -266,7 +266,8 @@ func isNodeUntainted(node *v1.Node) bool { n = nodeCopy } - return v1helper.TolerationsTolerateTaintsWithFilter(fakePod.Spec.Tolerations, n.Spec.Taints, func(t *v1.Taint) bool { + _, tolerated := v1helper.FindMatchingUntoleratedTaint(n.Spec.Taints, fakePod.Spec.Tolerations, func(t *v1.Taint) bool { return t.Effect == v1.TaintEffectNoExecute || t.Effect == v1.TaintEffectNoSchedule }) + return tolerated }