Move from regexp to csv string

This commit is contained in:
John Schnake 2019-09-05 11:48:41 -05:00
parent 8772187680
commit 3c53481d5c
5 changed files with 93 additions and 68 deletions

View File

@ -19,7 +19,7 @@ package node
import ( import (
"fmt" "fmt"
"net" "net"
"regexp" "strings"
"time" "time"
v1 "k8s.io/api/core/v1" 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. // 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 // TODO: need to discuss wether to return bool and error type
func IsNodeUntainted(node *v1.Node) bool { func IsNodeUntainted(node *v1.Node) bool {
return isNodeUntaintedWhitelist(node, nil) return isNodeUntaintedWithNonblocking(node, "")
} }
// isNodeUntaintedWhitelist tests whether a fake pod can be scheduled on "node" // isNodeUntaintedWithNonblocking tests whether a fake pod can be scheduled on "node"
// but allows for taints matching the regexp of whitelisted values. // but allows for taints in the list of non-blocking taints.
func isNodeUntaintedWhitelist(node *v1.Node, ignoreTaints *regexp.Regexp) bool { func isNodeUntaintedWithNonblocking(node *v1.Node, nonblockingTaints string) bool {
fakePod := &v1.Pod{ fakePod := &v1.Pod{
TypeMeta: metav1.TypeMeta{ TypeMeta: metav1.TypeMeta{
Kind: "Pod", Kind: "Pod",
@ -411,11 +411,19 @@ func isNodeUntaintedWhitelist(node *v1.Node, ignoreTaints *regexp.Regexp) bool {
nodeInfo := schedulernodeinfo.NewNodeInfo() 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 := node.DeepCopy()
nodeCopy.Spec.Taints = []v1.Taint{} nodeCopy.Spec.Taints = []v1.Taint{}
for _, v := range node.Spec.Taints { 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) nodeCopy.Spec.Taints = append(nodeCopy.Spec.Taints, v)
} }
} }
@ -452,17 +460,27 @@ func IsNodeReady(node *v1.Node) bool {
return nodeReady && networkReady 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. // one taint with a key matching the regexp.
func hasWhitelistedTaint(node *v1.Node, whitelist *regexp.Regexp) bool { func hasNonblockingTaint(node *v1.Node, nonblockingTaints string) bool {
if whitelist == nil { if node == nil {
return false 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 true
} }
} }
return false return false
} }

View File

@ -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 // CheckReadyForTests returns a method usable in polling methods which will check that the nodes are
// in a testable state based on schedulability. // 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 attempt := 0
var notSchedulable []*v1.Node var notSchedulable []*v1.Node
return func() (bool, error) { return func() (bool, error) {
@ -229,7 +229,7 @@ func CheckReadyForTests(c clientset.Interface, whitelistedTaints *regexp.Regexp,
} }
for i := range nodes.Items { for i := range nodes.Items {
node := &nodes.Items[i] node := &nodes.Items[i]
if !readyForTests(node, whitelistedTaints) { if !readyForTests(node, nonblockingTaints) {
notSchedulable = append(notSchedulable, node) notSchedulable = append(notSchedulable, node)
} }
} }
@ -245,16 +245,12 @@ func CheckReadyForTests(c clientset.Interface, whitelistedTaints *regexp.Regexp,
if len(nodes.Items) < largeClusterThreshold || attempt%10 == 0 { if len(nodes.Items) < largeClusterThreshold || attempt%10 == 0 {
e2elog.Logf("Unschedulable nodes:") e2elog.Logf("Unschedulable nodes:")
for i := range notSchedulable { for i := range notSchedulable {
whitelist := "<nil>" e2elog.Logf("-> %s Ready=%t Network=%t Taints=%v NonblockingTaints:%v",
if whitelistedTaints != nil {
whitelist = whitelistedTaints.String()
}
e2elog.Logf("-> %s Ready=%t Network=%t Taints=%v WhitelistedTaints:%v",
notSchedulable[i].Name, notSchedulable[i].Name,
IsConditionSetAsExpectedSilent(notSchedulable[i], v1.NodeReady, true), IsConditionSetAsExpectedSilent(notSchedulable[i], v1.NodeReady, true),
IsConditionSetAsExpectedSilent(notSchedulable[i], v1.NodeNetworkUnavailable, false), IsConditionSetAsExpectedSilent(notSchedulable[i], v1.NodeNetworkUnavailable, false),
notSchedulable[i].Spec.Taints, 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 // 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. // 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. // also have their node.Spec.Unschedulable field ignored for the purposes of this function.
func readyForTests(node *v1.Node, whitelistedTaints *regexp.Regexp) bool { func readyForTests(node *v1.Node, nonblockingTaints string) bool {
if hasWhitelistedTaint(node, whitelistedTaints) { if hasNonblockingTaint(node, nonblockingTaints) {
// If the node has one of the whitelisted taints; just check that it is ready // 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. // 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 return false
} }
} else { } else {

View File

@ -18,7 +18,6 @@ package node
import ( import (
"errors" "errors"
"regexp"
"testing" "testing"
v1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1"
@ -50,7 +49,7 @@ func TestCheckReadyForTests(t *testing.T) {
tcs := []struct { tcs := []struct {
desc string desc string
whitelist *regexp.Regexp nonblockingTaints string
allowedNotReadyNodes int allowedNotReadyNodes int
nodes []v1.Node nodes []v1.Node
nodeListErr error nodeListErr error
@ -64,8 +63,8 @@ func TestCheckReadyForTests(t *testing.T) {
}, },
expected: true, expected: true,
}, { }, {
desc: "Default value for whitelist regexp tolerates master taint", desc: "Default value for nonblocking taints tolerates master taint",
whitelist: regexp.MustCompile(`^node-role\.kubernetes\.io/master$`), nonblockingTaints: `node-role.kubernetes.io/master`,
nodes: []v1.Node{ nodes: []v1.Node{
fromVanillaNode(func(n *v1.Node) { fromVanillaNode(func(n *v1.Node) {
n.Spec.Taints = []v1.Taint{{Key: labelNodeRoleMaster, Effect: v1.TaintEffectNoSchedule}} n.Spec.Taints = []v1.Taint{{Key: labelNodeRoleMaster, Effect: v1.TaintEffectNoSchedule}}
@ -74,7 +73,7 @@ func TestCheckReadyForTests(t *testing.T) {
expected: true, expected: true,
}, { }, {
desc: "Tainted node should fail if effect is TaintEffectNoExecute", desc: "Tainted node should fail if effect is TaintEffectNoExecute",
whitelist: regexp.MustCompile("bar"), nonblockingTaints: "bar",
nodes: []v1.Node{ nodes: []v1.Node{
fromVanillaNode(func(n *v1.Node) { fromVanillaNode(func(n *v1.Node) {
n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoExecute}} n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoExecute}}
@ -82,7 +81,7 @@ func TestCheckReadyForTests(t *testing.T) {
expected: false, expected: false,
}, { }, {
desc: "Tainted node can be allowed via allowedNotReadyNodes", desc: "Tainted node can be allowed via allowedNotReadyNodes",
whitelist: regexp.MustCompile("bar"), nonblockingTaints: "bar",
allowedNotReadyNodes: 1, allowedNotReadyNodes: 1,
nodes: []v1.Node{ nodes: []v1.Node{
fromVanillaNode(func(n *v1.Node) { fromVanillaNode(func(n *v1.Node) {
@ -116,8 +115,8 @@ func TestCheckReadyForTests(t *testing.T) {
}, },
expected: true, expected: true,
}, { }, {
desc: "Multi-node, single blocking node allowed via whitelisted taint", desc: "Multi-node, single blocking node allowed via nonblocking taint",
whitelist: regexp.MustCompile("foo"), nonblockingTaints: "foo",
nodes: []v1.Node{ nodes: []v1.Node{
fromVanillaNode(func(n *v1.Node) {}), fromVanillaNode(func(n *v1.Node) {}),
fromVanillaNode(func(n *v1.Node) { fromVanillaNode(func(n *v1.Node) {
@ -125,6 +124,32 @@ func TestCheckReadyForTests(t *testing.T) {
}), }),
}, },
expected: true, 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", desc: "Errors from node list are reported",
nodeListErr: errors.New("Forced error"), nodeListErr: errors.New("Forced error"),
@ -147,7 +172,7 @@ func TestCheckReadyForTests(t *testing.T) {
nodeList := &v1.NodeList{Items: tc.nodes} nodeList := &v1.NodeList{Items: tc.nodes}
return true, nodeList, tc.nodeListErr return true, nodeList, tc.nodeListErr
}) })
checkFunc := CheckReadyForTests(c, tc.whitelist, tc.allowedNotReadyNodes, testLargeClusterThreshold) checkFunc := CheckReadyForTests(c, tc.nonblockingTaints, tc.allowedNotReadyNodes, testLargeClusterThreshold)
out, err := checkFunc() out, err := checkFunc()
if out != tc.expected { if out != tc.expected {
t.Errorf("Expected %v but got %v", tc.expected, out) t.Errorf("Expected %v but got %v", tc.expected, out)
@ -179,7 +204,7 @@ func TestReadyForTests(t *testing.T) {
tcs := []struct { tcs := []struct {
desc string desc string
node *v1.Node node *v1.Node
whitelist *regexp.Regexp nonblockingTaints string
expected bool expected bool
}{ }{
{ {
@ -188,42 +213,38 @@ func TestReadyForTests(t *testing.T) {
}), }),
expected: true, expected: true,
}, { }, {
desc: "Vanilla node should pass with non-applicable whitelist", desc: "Vanilla node should pass with non-applicable nonblocking taint",
whitelist: regexp.MustCompile("foo"), nonblockingTaints: "foo",
node: fromVanillaNode(func(n *v1.Node) { node: fromVanillaNode(func(n *v1.Node) {
}), }),
expected: true, expected: true,
}, { }, {
desc: "Tainted node should pass if effect is TaintEffectPreferNoSchedule", desc: "Tainted node should pass if effect is TaintEffectPreferNoSchedule",
whitelist: regexp.MustCompile("bar"),
node: fromVanillaNode(func(n *v1.Node) { node: fromVanillaNode(func(n *v1.Node) {
n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectPreferNoSchedule}} n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectPreferNoSchedule}}
}), }),
expected: true, expected: true,
}, { }, {
desc: "Tainted node should fail if effect is TaintEffectNoExecute", desc: "Tainted node should fail if effect is TaintEffectNoExecute",
whitelist: regexp.MustCompile("bar"),
node: fromVanillaNode(func(n *v1.Node) { node: fromVanillaNode(func(n *v1.Node) {
n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoExecute}} n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoExecute}}
}), }),
expected: false, expected: false,
}, { }, {
desc: "Tainted node should fail", desc: "Tainted node should fail",
whitelist: regexp.MustCompile("bar"),
node: fromVanillaNode(func(n *v1.Node) { node: fromVanillaNode(func(n *v1.Node) {
n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}} n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}}
}), }),
expected: false, expected: false,
}, { }, {
desc: "Tainted node should pass if whitelisted", desc: "Tainted node should pass if nonblocking",
whitelist: regexp.MustCompile("foo"), nonblockingTaints: "foo",
node: fromVanillaNode(func(n *v1.Node) { node: fromVanillaNode(func(n *v1.Node) {
n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}} n.Spec.Taints = []v1.Taint{{Key: "foo", Effect: v1.TaintEffectNoSchedule}}
}), }),
expected: true, expected: true,
}, { }, {
desc: "Node with network not ready fails", desc: "Node with network not ready fails",
whitelist: regexp.MustCompile("foo"),
node: fromVanillaNode(func(n *v1.Node) { node: fromVanillaNode(func(n *v1.Node) {
n.Status.Conditions = append(n.Status.Conditions, n.Status.Conditions = append(n.Status.Conditions,
v1.NodeCondition{Type: v1.NodeNetworkUnavailable, Status: v1.ConditionTrue}, v1.NodeCondition{Type: v1.NodeNetworkUnavailable, Status: v1.ConditionTrue},
@ -232,7 +253,6 @@ func TestReadyForTests(t *testing.T) {
expected: false, expected: false,
}, { }, {
desc: "Node fails unless NodeReady status", desc: "Node fails unless NodeReady status",
whitelist: regexp.MustCompile("foo"),
node: fromVanillaNode(func(n *v1.Node) { node: fromVanillaNode(func(n *v1.Node) {
n.Status.Conditions = []v1.NodeCondition{} n.Status.Conditions = []v1.NodeCondition{}
}), }),
@ -242,7 +262,7 @@ func TestReadyForTests(t *testing.T) {
for _, tc := range tcs { for _, tc := range tcs {
t.Run(tc.desc, func(t *testing.T) { t.Run(tc.desc, func(t *testing.T) {
out := readyForTests(tc.node, tc.whitelist) out := readyForTests(tc.node, tc.nonblockingTaints)
if out != tc.expected { if out != tc.expected {
t.Errorf("Expected %v but got %v", tc.expected, out) t.Errorf("Expected %v but got %v", tc.expected, out)
} }

View File

@ -21,7 +21,6 @@ import (
"fmt" "fmt"
"io/ioutil" "io/ioutil"
"os" "os"
"regexp"
"sort" "sort"
"strings" "strings"
"time" "time"
@ -168,11 +167,8 @@ type TestContextType struct {
// The Default IP Family of the cluster ("ipv4" or "ipv6") // The Default IP Family of the cluster ("ipv4" or "ipv6")
IPFamily string IPFamily string
// WhitelistedTaint is the string given by the user to specify taints which should not stop the test framework from running tests. // NonblockingTaints is the comma-delimeted string given by the user to specify taints which should not stop the test framework from running tests.
WhitelistedTaint string NonblockingTaints string
// WhitelistedTaintRegexp is the *regexp.Regexp parsed from the WhitelistedTaint.
WhitelistedTaintRegexp *regexp.Regexp
} }
// NodeKillerConfig describes configuration of NodeKiller -- a utility to // 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.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.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.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.") 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 t.AllowedNotReadyNodes = t.CloudConfig.NumNodes / 100
} }
if len(TestContext.WhitelistedTaint) > 0 { klog.Infof("Tolerating taints %q when considering if nodes are ready", TestContext.NonblockingTaints)
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")
}
// Make sure that all test runs have a valid TestContext.CloudConfig.Provider. // Make sure that all test runs have a valid TestContext.CloudConfig.Provider.
// TODO: whether and how long this code is needed is getting discussed // TODO: whether and how long this code is needed is getting discussed

View File

@ -1914,7 +1914,7 @@ func WaitForAllNodesSchedulable(c clientset.Interface, timeout time.Duration) er
return wait.PollImmediate( return wait.PollImmediate(
30*time.Second, 30*time.Second,
timeout, timeout,
e2enode.CheckReadyForTests(c, TestContext.WhitelistedTaintRegexp, TestContext.AllowedNotReadyNodes, largeClusterThreshold), e2enode.CheckReadyForTests(c, TestContext.NonblockingTaints, TestContext.AllowedNotReadyNodes, largeClusterThreshold),
) )
} }