diff --git a/test/e2e/framework/node/resource.go b/test/e2e/framework/node/resource.go index c5fddeba776..adf1cbaf9e1 100644 --- a/test/e2e/framework/node/resource.go +++ b/test/e2e/framework/node/resource.go @@ -19,7 +19,7 @@ package node import ( "fmt" "net" - "regexp" + "strings" "time" v1 "k8s.io/api/core/v1" @@ -384,12 +384,12 @@ func GetMasterAndWorkerNodes(c clientset.Interface) (sets.String, *v1.NodeList, // IsNodeUntainted tests whether a fake pod can be scheduled on "node", given its current taints. // TODO: need to discuss wether to return bool and error type func IsNodeUntainted(node *v1.Node) bool { - return isNodeUntaintedWhitelist(node, nil) + return isNodeUntaintedWithNonblocking(node, "") } -// isNodeUntaintedWhitelist tests whether a fake pod can be scheduled on "node" -// but allows for taints matching the regexp of whitelisted values. -func isNodeUntaintedWhitelist(node *v1.Node, ignoreTaints *regexp.Regexp) bool { +// isNodeUntaintedWithNonblocking tests whether a fake pod can be scheduled on "node" +// but allows for taints in the list of non-blocking taints. +func isNodeUntaintedWithNonblocking(node *v1.Node, nonblockingTaints string) bool { fakePod := &v1.Pod{ TypeMeta: metav1.TypeMeta{ Kind: "Pod", @@ -411,11 +411,19 @@ func isNodeUntaintedWhitelist(node *v1.Node, ignoreTaints *regexp.Regexp) bool { nodeInfo := schedulernodeinfo.NewNodeInfo() - if ignoreTaints != nil { + // Simple lookup for nonblocking taints based on comma-delimited list. + nonblockingTaintsMap := map[string]struct{}{} + for _, t := range strings.Split(nonblockingTaints, ",") { + if strings.TrimSpace(t) != "" { + nonblockingTaintsMap[strings.TrimSpace(t)] = struct{}{} + } + } + + if len(nonblockingTaintsMap) > 0 { nodeCopy := node.DeepCopy() nodeCopy.Spec.Taints = []v1.Taint{} for _, v := range node.Spec.Taints { - if !ignoreTaints.MatchString(v.Key) { + if _, isNonblockingTaint := nonblockingTaintsMap[v.Key]; !isNonblockingTaint { nodeCopy.Spec.Taints = append(nodeCopy.Spec.Taints, v) } } @@ -452,17 +460,27 @@ func IsNodeReady(node *v1.Node) bool { return nodeReady && networkReady } -// hasWhitelistedTaint returns true if the node contains at least +// hasNonblockingTaint returns true if the node contains at least // one taint with a key matching the regexp. -func hasWhitelistedTaint(node *v1.Node, whitelist *regexp.Regexp) bool { - if whitelist == nil { +func hasNonblockingTaint(node *v1.Node, nonblockingTaints string) bool { + if node == nil { return false } - for _, v := range node.Spec.Taints { - if whitelist.MatchString(v.Key) { + + // Simple lookup for nonblocking taints based on comma-delimited list. + nonblockingTaintsMap := map[string]struct{}{} + for _, t := range strings.Split(nonblockingTaints, ",") { + if strings.TrimSpace(t) != "" { + nonblockingTaintsMap[strings.TrimSpace(t)] = struct{}{} + } + } + + for _, taint := range node.Spec.Taints { + if _, hasNonblockingTaint := nonblockingTaintsMap[taint.Key]; hasNonblockingTaint { return true } } + return false } diff --git a/test/e2e/framework/node/wait.go b/test/e2e/framework/node/wait.go index b4882f52b6a..0d4c9c8d74e 100644 --- a/test/e2e/framework/node/wait.go +++ b/test/e2e/framework/node/wait.go @@ -209,7 +209,7 @@ func checkWaitListSchedulableNodes(c clientset.Interface) (*v1.NodeList, error) // CheckReadyForTests returns a method usable in polling methods which will check that the nodes are // in a testable state based on schedulability. -func CheckReadyForTests(c clientset.Interface, whitelistedTaints *regexp.Regexp, allowedNotReadyNodes, largeClusterThreshold int) func() (bool, error) { +func CheckReadyForTests(c clientset.Interface, nonblockingTaints string, allowedNotReadyNodes, largeClusterThreshold int) func() (bool, error) { attempt := 0 var notSchedulable []*v1.Node return func() (bool, error) { @@ -229,7 +229,7 @@ func CheckReadyForTests(c clientset.Interface, whitelistedTaints *regexp.Regexp, } for i := range nodes.Items { node := &nodes.Items[i] - if !readyForTests(node, whitelistedTaints) { + if !readyForTests(node, nonblockingTaints) { notSchedulable = append(notSchedulable, node) } } @@ -245,16 +245,12 @@ func CheckReadyForTests(c clientset.Interface, whitelistedTaints *regexp.Regexp, if len(nodes.Items) < largeClusterThreshold || attempt%10 == 0 { e2elog.Logf("Unschedulable nodes:") for i := range notSchedulable { - whitelist := "" - if whitelistedTaints != nil { - whitelist = whitelistedTaints.String() - } - e2elog.Logf("-> %s Ready=%t Network=%t Taints=%v WhitelistedTaints:%v", + e2elog.Logf("-> %s Ready=%t Network=%t Taints=%v NonblockingTaints:%v", notSchedulable[i].Name, IsConditionSetAsExpectedSilent(notSchedulable[i], v1.NodeReady, true), IsConditionSetAsExpectedSilent(notSchedulable[i], v1.NodeNetworkUnavailable, false), notSchedulable[i].Spec.Taints, - whitelist, + nonblockingTaints, ) } @@ -267,13 +263,13 @@ func CheckReadyForTests(c clientset.Interface, whitelistedTaints *regexp.Regexp, // readyForTests determines whether or not we should continue waiting for the nodes // to enter a testable state. By default this means it is schedulable, NodeReady, and untainted. -// Nodes with taints matching the whitelistedTaintRegexp are permitted to have that taint and +// Nodes with taints nonblocking taints are permitted to have that taint and // also have their node.Spec.Unschedulable field ignored for the purposes of this function. -func readyForTests(node *v1.Node, whitelistedTaints *regexp.Regexp) bool { - if hasWhitelistedTaint(node, whitelistedTaints) { - // If the node has one of the whitelisted taints; just check that it is ready +func readyForTests(node *v1.Node, nonblockingTaints string) bool { + if hasNonblockingTaint(node, nonblockingTaints) { + // If the node has one of the nonblockingTaints taints; just check that it is ready // and don't require node.Spec.Unschedulable to be set either way. - if !IsNodeReady(node) || !isNodeUntaintedWhitelist(node, whitelistedTaints) { + if !IsNodeReady(node) || !isNodeUntaintedWithNonblocking(node, nonblockingTaints) { return false } } else { diff --git a/test/e2e/framework/node/wait_test.go b/test/e2e/framework/node/wait_test.go index 9e2089963e5..f039017a049 100644 --- a/test/e2e/framework/node/wait_test.go +++ b/test/e2e/framework/node/wait_test.go @@ -18,7 +18,6 @@ package node import ( "errors" - "regexp" "testing" v1 "k8s.io/api/core/v1" @@ -50,7 +49,7 @@ func TestCheckReadyForTests(t *testing.T) { tcs := []struct { desc string - whitelist *regexp.Regexp + nonblockingTaints string allowedNotReadyNodes int nodes []v1.Node nodeListErr error @@ -64,8 +63,8 @@ func TestCheckReadyForTests(t *testing.T) { }, expected: true, }, { - desc: "Default value for whitelist regexp tolerates master taint", - whitelist: regexp.MustCompile(`^node-role\.kubernetes\.io/master$`), + desc: "Default value for nonblocking taints tolerates master taint", + nonblockingTaints: `node-role.kubernetes.io/master`, nodes: []v1.Node{ fromVanillaNode(func(n *v1.Node) { n.Spec.Taints = []v1.Taint{{Key: labelNodeRoleMaster, Effect: v1.TaintEffectNoSchedule}} @@ -73,8 +72,8 @@ func TestCheckReadyForTests(t *testing.T) { }, expected: true, }, { - desc: "Tainted node should fail if effect is TaintEffectNoExecute", - whitelist: regexp.MustCompile("bar"), + desc: "Tainted node should fail if effect is TaintEffectNoExecute", + nonblockingTaints: "bar", nodes: []v1.Node{ fromVanillaNode(func(n *v1.Node) { n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoExecute}} @@ -82,7 +81,7 @@ func TestCheckReadyForTests(t *testing.T) { expected: false, }, { desc: "Tainted node can be allowed via allowedNotReadyNodes", - whitelist: regexp.MustCompile("bar"), + nonblockingTaints: "bar", allowedNotReadyNodes: 1, nodes: []v1.Node{ fromVanillaNode(func(n *v1.Node) { @@ -116,8 +115,8 @@ func TestCheckReadyForTests(t *testing.T) { }, expected: true, }, { - desc: "Multi-node, single blocking node allowed via whitelisted taint", - whitelist: regexp.MustCompile("foo"), + desc: "Multi-node, single blocking node allowed via nonblocking taint", + nonblockingTaints: "foo", nodes: []v1.Node{ fromVanillaNode(func(n *v1.Node) {}), fromVanillaNode(func(n *v1.Node) { @@ -125,6 +124,32 @@ func TestCheckReadyForTests(t *testing.T) { }), }, expected: true, + }, { + desc: "Multi-node, both blocking nodes allowed via separate nonblocking taints", + nonblockingTaints: "foo,bar", + nodes: []v1.Node{ + fromVanillaNode(func(n *v1.Node) {}), + fromVanillaNode(func(n *v1.Node) { + n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}} + }), + fromVanillaNode(func(n *v1.Node) { + n.Spec.Taints = []v1.Taint{{Key: "bar", Effect: v1.TaintEffectNoSchedule}} + }), + }, + expected: true, + }, { + desc: "Multi-node, one blocking node allowed via nonblocking taints still blocked", + nonblockingTaints: "foo,notbar", + nodes: []v1.Node{ + fromVanillaNode(func(n *v1.Node) {}), + fromVanillaNode(func(n *v1.Node) { + n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}} + }), + fromVanillaNode(func(n *v1.Node) { + n.Spec.Taints = []v1.Taint{{Key: "bar", Effect: v1.TaintEffectNoSchedule}} + }), + }, + expected: false, }, { desc: "Errors from node list are reported", nodeListErr: errors.New("Forced error"), @@ -147,7 +172,7 @@ func TestCheckReadyForTests(t *testing.T) { nodeList := &v1.NodeList{Items: tc.nodes} return true, nodeList, tc.nodeListErr }) - checkFunc := CheckReadyForTests(c, tc.whitelist, tc.allowedNotReadyNodes, testLargeClusterThreshold) + checkFunc := CheckReadyForTests(c, tc.nonblockingTaints, tc.allowedNotReadyNodes, testLargeClusterThreshold) out, err := checkFunc() if out != tc.expected { t.Errorf("Expected %v but got %v", tc.expected, out) @@ -177,10 +202,10 @@ func TestReadyForTests(t *testing.T) { } _ = fromVanillaNode tcs := []struct { - desc string - node *v1.Node - whitelist *regexp.Regexp - expected bool + desc string + node *v1.Node + nonblockingTaints string + expected bool }{ { desc: "Vanilla node should pass", @@ -188,42 +213,38 @@ func TestReadyForTests(t *testing.T) { }), expected: true, }, { - desc: "Vanilla node should pass with non-applicable whitelist", - whitelist: regexp.MustCompile("foo"), + desc: "Vanilla node should pass with non-applicable nonblocking taint", + nonblockingTaints: "foo", node: fromVanillaNode(func(n *v1.Node) { }), expected: true, }, { - desc: "Tainted node should pass if effect is TaintEffectPreferNoSchedule", - whitelist: regexp.MustCompile("bar"), + desc: "Tainted node should pass if effect is TaintEffectPreferNoSchedule", node: fromVanillaNode(func(n *v1.Node) { n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectPreferNoSchedule}} }), expected: true, }, { - desc: "Tainted node should fail if effect is TaintEffectNoExecute", - whitelist: regexp.MustCompile("bar"), + desc: "Tainted node should fail if effect is TaintEffectNoExecute", node: fromVanillaNode(func(n *v1.Node) { n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoExecute}} }), expected: false, }, { - desc: "Tainted node should fail", - whitelist: regexp.MustCompile("bar"), + desc: "Tainted node should fail", node: fromVanillaNode(func(n *v1.Node) { n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}} }), expected: false, }, { - desc: "Tainted node should pass if whitelisted", - whitelist: regexp.MustCompile("foo"), + desc: "Tainted node should pass if nonblocking", + nonblockingTaints: "foo", node: fromVanillaNode(func(n *v1.Node) { n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}} }), expected: true, }, { - desc: "Node with network not ready fails", - whitelist: regexp.MustCompile("foo"), + desc: "Node with network not ready fails", node: fromVanillaNode(func(n *v1.Node) { n.Status.Conditions = append(n.Status.Conditions, v1.NodeCondition{Type: v1.NodeNetworkUnavailable, Status: v1.ConditionTrue}, @@ -231,8 +252,7 @@ func TestReadyForTests(t *testing.T) { }), expected: false, }, { - desc: "Node fails unless NodeReady status", - whitelist: regexp.MustCompile("foo"), + desc: "Node fails unless NodeReady status", node: fromVanillaNode(func(n *v1.Node) { n.Status.Conditions = []v1.NodeCondition{} }), @@ -242,7 +262,7 @@ func TestReadyForTests(t *testing.T) { for _, tc := range tcs { t.Run(tc.desc, func(t *testing.T) { - out := readyForTests(tc.node, tc.whitelist) + out := readyForTests(tc.node, tc.nonblockingTaints) if out != tc.expected { t.Errorf("Expected %v but got %v", tc.expected, out) } diff --git a/test/e2e/framework/test_context.go b/test/e2e/framework/test_context.go index 2cd7fd2a7d7..688a72de346 100644 --- a/test/e2e/framework/test_context.go +++ b/test/e2e/framework/test_context.go @@ -21,7 +21,6 @@ import ( "fmt" "io/ioutil" "os" - "regexp" "sort" "strings" "time" @@ -168,11 +167,8 @@ type TestContextType struct { // The Default IP Family of the cluster ("ipv4" or "ipv6") IPFamily string - // WhitelistedTaint is the string given by the user to specify taints which should not stop the test framework from running tests. - WhitelistedTaint string - - // WhitelistedTaintRegexp is the *regexp.Regexp parsed from the WhitelistedTaint. - WhitelistedTaintRegexp *regexp.Regexp + // NonblockingTaints is the comma-delimeted string given by the user to specify taints which should not stop the test framework from running tests. + NonblockingTaints string } // NodeKillerConfig describes configuration of NodeKiller -- a utility to @@ -290,7 +286,7 @@ func RegisterCommonFlags(flags *flag.FlagSet) { flags.StringVar(&TestContext.ImageServiceEndpoint, "image-service-endpoint", "", "The image service endpoint of cluster VM instances.") flags.StringVar(&TestContext.DockershimCheckpointDir, "dockershim-checkpoint-dir", "/var/lib/dockershim/sandbox", "The directory for dockershim to store sandbox checkpoints.") flags.StringVar(&TestContext.KubernetesAnywherePath, "kubernetes-anywhere-path", "/workspace/k8s.io/kubernetes-anywhere", "Which directory kubernetes-anywhere is installed to.") - flags.StringVar(&TestContext.WhitelistedTaint, "whitelist-taint-regexp", `^node-role\.kubernetes\.io/master$`, "Nodes with taints which match this regexp will not block the test framework from starting tests.") + flags.StringVar(&TestContext.NonblockingTaints, "non-blocking-taints", `node-role.kubernetes.io/master`, "Nodes with taints in this comma-delimited list will not block the test framework from starting tests.") flags.BoolVar(&TestContext.ListImages, "list-images", false, "If true, will show list of images used for runnning tests.") } @@ -432,12 +428,7 @@ func AfterReadingAllFlags(t *TestContextType) { t.AllowedNotReadyNodes = t.CloudConfig.NumNodes / 100 } - if len(TestContext.WhitelistedTaint) > 0 { - TestContext.WhitelistedTaintRegexp = regexp.MustCompile(TestContext.WhitelistedTaint) - klog.Infof("Tolerating taints matching regexp %q when considering if nodes are ready", TestContext.WhitelistedTaintRegexp.String()) - } else { - klog.Info("No taints will be tolerated when considering if nodes are ready") - } + klog.Infof("Tolerating taints %q when considering if nodes are ready", TestContext.NonblockingTaints) // Make sure that all test runs have a valid TestContext.CloudConfig.Provider. // TODO: whether and how long this code is needed is getting discussed diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 086d7571a98..c6f734fd645 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1914,7 +1914,7 @@ func WaitForAllNodesSchedulable(c clientset.Interface, timeout time.Duration) er return wait.PollImmediate( 30*time.Second, timeout, - e2enode.CheckReadyForTests(c, TestContext.WhitelistedTaintRegexp, TestContext.AllowedNotReadyNodes, largeClusterThreshold), + e2enode.CheckReadyForTests(c, TestContext.NonblockingTaints, TestContext.AllowedNotReadyNodes, largeClusterThreshold), ) }