Move the IsMasterNode function to tests and mark it Deprecated

A future change will stop using this signal and instead use a
label selector passed on creation.

pkg/controller/.import-restrictions

@@ -244,7 +244,6 @@
         "k8s.io/kubernetes/pkg/util/mount",
         "k8s.io/kubernetes/pkg/util/node",
         "k8s.io/kubernetes/pkg/util/slice",
-        "k8s.io/kubernetes/pkg/util/system",
         "k8s.io/kubernetes/pkg/util/taints",
         "k8s.io/kubernetes/pkg/volume",
         "k8s.io/kubernetes/pkg/volume/util",

pkg/util/BUILD

@@ -47,7 +47,6 @@ filegroup(
         "//pkg/util/selinux:all-srcs",
         "//pkg/util/slice:all-srcs",
         "//pkg/util/sysctl:all-srcs",
-        "//pkg/util/system:all-srcs",
         "//pkg/util/tail:all-srcs",
         "//pkg/util/taints:all-srcs",
         "//pkg/util/tolerations:all-srcs",

test/e2e/BUILD

@@ -113,6 +113,7 @@ filegroup(
         "//test/e2e/scheduling:all-srcs",
         "//test/e2e/servicecatalog:all-srcs",
         "//test/e2e/storage:all-srcs",
+        "//test/e2e/system:all-srcs",
         "//test/e2e/testing-manifests:all-srcs",
         "//test/e2e/ui:all-srcs",
         "//test/e2e/upgrades:all-srcs",

test/e2e/framework/BUILD

@@ -34,7 +34,6 @@ go_library(
         "//pkg/apis/storage/v1/util:go_default_library",
         "//pkg/client/conditions:go_default_library",
         "//pkg/controller:go_default_library",
-        "//pkg/controller/service:go_default_library",
         "//pkg/features:go_default_library",
         "//pkg/kubelet/apis/config:go_default_library",
         "//pkg/kubelet/events:go_default_library",
@@ -42,7 +41,6 @@ go_library(
         "//pkg/master/ports:go_default_library",
         "//pkg/scheduler/algorithm/predicates:go_default_library",
         "//pkg/scheduler/nodeinfo:go_default_library",
-        "//pkg/util/system:go_default_library",
         "//pkg/util/taints:go_default_library",
         "//pkg/version:go_default_library",
         "//pkg/volume/util:go_default_library",
@@ -96,6 +94,7 @@ go_library(
         "//test/e2e/framework/ssh:go_default_library",
         "//test/e2e/framework/testfiles:go_default_library",
         "//test/e2e/manifest:go_default_library",
+        "//test/e2e/system:go_default_library",
         "//test/utils:go_default_library",
         "//test/utils/image:go_default_library",
         "//vendor/github.com/onsi/ginkgo:go_default_library",

test/e2e/framework/log/logger_test.go

@@ -84,7 +84,7 @@ func TestFailureOutput(t *testing.T) {
 			output:  "INFO: before\nFAIL: hard-coded error\nUnexpected error:\n    <*errors.errorString>: {\n        s: \"an error with a long, useless description\",\n    }\n    an error with a long, useless description\noccurred\nINFO: after\nFAIL: true is never false either\nExpected\n    <bool>: true\nto equal\n    <bool>: false\n",
 			failure: "hard-coded error\nUnexpected error:\n    <*errors.errorString>: {\n        s: \"an error with a long, useless description\",\n    }\n    an error with a long, useless description\noccurred",
 			// TODO: should start with k8s.io/kubernetes/test/e2e/framework/log_test.glob..func1.4()
-			stack: "\tutil.go:1369\nk8s.io/kubernetes/test/e2e/framework.ExpectNoError()\n\tutil.go:1363\nk8s.io/kubernetes/test/e2e/framework/log_test.glob..func1.4()\n\tlogger_test.go:49\nk8s.io/kubernetes/vendor/github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync()\n\tlogger_test.go:65\n",
+			stack: "\tutil.go:1368\nk8s.io/kubernetes/test/e2e/framework.ExpectNoError()\n\tutil.go:1362\nk8s.io/kubernetes/test/e2e/framework/log_test.glob..func1.4()\n\tlogger_test.go:49\nk8s.io/kubernetes/vendor/github.com/onsi/ginkgo/internal/leafnodes.(*runner).runSync()\n\tlogger_test.go:65\n",
 		},
 		testResult{
 			name:    "[Top Level] log fails",

test/e2e/framework/metrics/BUILD

@@ -30,7 +30,6 @@ go_library(
         "//pkg/kubelet/metrics:go_default_library",
         "//pkg/master/ports:go_default_library",
         "//pkg/scheduler/metrics:go_default_library",
-        "//pkg/util/system:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
@@ -38,6 +37,7 @@ go_library(
         "//test/e2e/framework/log:go_default_library",
         "//test/e2e/framework/ssh:go_default_library",
         "//test/e2e/perftype:go_default_library",
+        "//test/e2e/system:go_default_library",
         "//vendor/github.com/onsi/gomega:go_default_library",
         "//vendor/github.com/prometheus/common/expfmt:go_default_library",
         "//vendor/github.com/prometheus/common/model:go_default_library",

test/e2e/framework/metrics/latencies.go

@@ -30,9 +30,9 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/kubernetes/pkg/master/ports"
 	schedulermetric "k8s.io/kubernetes/pkg/scheduler/metrics"
-	"k8s.io/kubernetes/pkg/util/system"
 	e2elog "k8s.io/kubernetes/test/e2e/framework/log"
 	e2essh "k8s.io/kubernetes/test/e2e/framework/ssh"
+	"k8s.io/kubernetes/test/e2e/system"
 
 	"github.com/onsi/gomega"
 
@@ -210,7 +210,7 @@ func sendRestRequestToScheduler(c clientset.Interface, op, provider, cloudMaster
 
 	var masterRegistered = false
 	for _, node := range nodes.Items {
-		if system.IsMasterNode(node.Name) {
+		if system.DeprecatedMightBeMasterNode(node.Name) {
 			masterRegistered = true
 		}
 	}

test/e2e/framework/metrics/metrics_grabber.go

@@ -24,7 +24,7 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	api "k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/master/ports"
-	"k8s.io/kubernetes/pkg/util/system"
+	"k8s.io/kubernetes/test/e2e/system"
 
 	"k8s.io/klog"
 )
@@ -63,7 +63,7 @@ func NewMetricsGrabber(c clientset.Interface, ec clientset.Interface, kubelets b
 		klog.Warning("Can't find any Nodes in the API server to grab metrics from")
 	}
 	for _, node := range nodeList.Items {
-		if system.IsMasterNode(node.Name) {
+		if system.DeprecatedMightBeMasterNode(node.Name) {
 			registeredMaster = true
 			masterName = node.Name
 			break

test/e2e/framework/node/BUILD

@@ -12,7 +12,6 @@ go_library(
         "//pkg/controller/nodelifecycle:go_default_library",
         "//pkg/scheduler/algorithm/predicates:go_default_library",
         "//pkg/scheduler/nodeinfo:go_default_library",
-        "//pkg/util/system:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
@@ -20,6 +19,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//test/e2e/framework/log:go_default_library",
+        "//test/e2e/system:go_default_library",
         "//test/utils:go_default_library",
     ],
 )

test/e2e/framework/node/resource.go

@@ -30,8 +30,8 @@ import (
 	nodectlr "k8s.io/kubernetes/pkg/controller/nodelifecycle"
 	"k8s.io/kubernetes/pkg/scheduler/algorithm/predicates"
 	schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
-	"k8s.io/kubernetes/pkg/util/system"
 	e2elog "k8s.io/kubernetes/test/e2e/framework/log"
+	"k8s.io/kubernetes/test/e2e/system"
 	testutils "k8s.io/kubernetes/test/utils"
 )
 
@@ -371,7 +371,7 @@ func GetMasterAndWorkerNodes(c clientset.Interface) (sets.String, *v1.NodeList,
 		return nil, nil, fmt.Errorf("get nodes error: %s", err)
 	}
 	for _, n := range all.Items {
-		if system.IsMasterNode(n.Name) {
+		if system.DeprecatedMightBeMasterNode(n.Name) {
 			masters.Insert(n.Name)
 		} else if isNodeSchedulable(&n) && isNodeUntainted(&n) {
 			nodes.Items = append(nodes.Items, n)

test/e2e/framework/node/wait.go

@@ -26,8 +26,8 @@ import (
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
-	"k8s.io/kubernetes/pkg/util/system"
 	e2elog "k8s.io/kubernetes/test/e2e/framework/log"
+	"k8s.io/kubernetes/test/e2e/system"
 	testutils "k8s.io/kubernetes/test/utils"
 )
 
@@ -83,7 +83,7 @@ func WaitForTotalHealthy(c clientset.Interface, timeout time.Duration) error {
 		}
 		missingPodsPerNode = make(map[string][]string)
 		for _, node := range nodes.Items {
-			if !system.IsMasterNode(node.Name) {
+			if !system.DeprecatedMightBeMasterNode(node.Name) {
 				for _, requiredPod := range requiredPerNodePods {
 					foundRequired := false
 					for _, presentPod := range systemPodsPerNode[node.Name] {

test/e2e/framework/resource_usage_gatherer.go

@@ -27,14 +27,14 @@ import (
 	"text/tabwriter"
 	"time"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientset "k8s.io/client-go/kubernetes"
-	"k8s.io/kubernetes/pkg/util/system"
 	e2ekubelet "k8s.io/kubernetes/test/e2e/framework/kubelet"
 	e2elog "k8s.io/kubernetes/test/e2e/framework/log"
 	e2emetrics "k8s.io/kubernetes/test/e2e/framework/metrics"
+	"k8s.io/kubernetes/test/e2e/system"
 )
 
 // ResourceConstraint is a struct to hold constraints.
@@ -280,10 +280,10 @@ func NewResourceUsageGatherer(c clientset.Interface, options ResourceGathererOpt
 	}
 	dnsNodes := make(map[string]bool)
 	for _, pod := range pods.Items {
-		if (options.Nodes == MasterNodes) && !system.IsMasterNode(pod.Spec.NodeName) {
+		if (options.Nodes == MasterNodes) && !system.DeprecatedMightBeMasterNode(pod.Spec.NodeName) {
 			continue
 		}
-		if (options.Nodes == MasterAndDNSNodes) && !system.IsMasterNode(pod.Spec.NodeName) && pod.Labels["k8s-app"] != "kube-dns" {
+		if (options.Nodes == MasterAndDNSNodes) && !system.DeprecatedMightBeMasterNode(pod.Spec.NodeName) && pod.Labels["k8s-app"] != "kube-dns" {
 			continue
 		}
 		for _, container := range pod.Status.InitContainerStatuses {
@@ -303,7 +303,7 @@ func NewResourceUsageGatherer(c clientset.Interface, options ResourceGathererOpt
 	}
 
 	for _, node := range nodeList.Items {
-		if options.Nodes == AllNodes || system.IsMasterNode(node.Name) || dnsNodes[node.Name] {
+		if options.Nodes == AllNodes || system.DeprecatedMightBeMasterNode(node.Name) || dnsNodes[node.Name] {
 			g.workerWg.Add(1)
 			g.workers = append(g.workers, resourceGatherWorker{
 				c:                           c,

test/e2e/framework/util.go

@@ -74,7 +74,6 @@ import (
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 	"k8s.io/kubernetes/pkg/client/conditions"
 	"k8s.io/kubernetes/pkg/controller"
-	"k8s.io/kubernetes/pkg/controller/service"
 	"k8s.io/kubernetes/pkg/features"
 	"k8s.io/kubernetes/pkg/master/ports"
 	"k8s.io/kubernetes/pkg/scheduler/algorithm/predicates"
@@ -1974,7 +1973,7 @@ func WaitForAllNodesSchedulable(c clientset.Interface, timeout time.Duration) er
 		}
 		for i := range nodes.Items {
 			node := &nodes.Items[i]
-			if _, hasMasterRoleLabel := node.ObjectMeta.Labels[service.LabelNodeRoleMaster]; hasMasterRoleLabel {
+			if _, hasMasterRoleLabel := node.ObjectMeta.Labels["node-role.kubernetes.io/master"]; hasMasterRoleLabel {
 				// Kops clusters have masters with spec.unscheduable = false and
 				// node-role.kubernetes.io/master NoSchedule taint.
 				// Don't wait for them.

test/e2e/system/BUILD

@@ -1,15 +1,10 @@
-package(default_visibility = ["//visibility:public"])
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
-
-load(
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-    "go_test",
-)
 
 go_library(
     name = "go_default_library",
     srcs = ["system_utils.go"],
-    importpath = "k8s.io/kubernetes/pkg/util/system",
+    importpath = "k8s.io/kubernetes/test/e2e/system",
+    visibility = ["//visibility:public"],
 )
 
 go_test(
@@ -33,4 +28,5 @@ filegroup(
     name = "all-srcs",
     srcs = [":package-srcs"],
     tags = ["automanaged"],
+    visibility = ["//visibility:public"],
 )

test/e2e/system/system_utils.go

@@ -20,9 +20,11 @@ import (
 	"strings"
 )
 
-// IsMasterNode returns true if given node is a registered master.
+// DeprecatedMightBeMasterNode returns true if given node is a registered master.
-// TODO: find a better way of figuring out if given node is a registered master.
+// This code must not be updated to use node role labels, since node role labels
-func IsMasterNode(nodeName string) bool {
+// may not change behavior of the system.
+// DEPRECATED: use a label selector provided by test initialization.
+func DeprecatedMightBeMasterNode(nodeName string) bool {
 	// We are trying to capture "master(-...)?$" regexp.
 	// However, using regexp.MatchString() results even in more than 35%
 	// of all space allocations in ControllerManager spent in this function.

test/e2e/system/system_utils_test.go

@@ -39,7 +39,7 @@ func TestIsMasterNode(t *testing.T) {
 
 	for _, tc := range testCases {
 		node := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: tc.input}}
-		res := IsMasterNode(node.Name)
+		res := DeprecatedMightBeMasterNode(node.Name)
 		if res != tc.result {
 			t.Errorf("case \"%s\": expected %t, got %t", tc.input, tc.result, res)
 		}

test/test_owners.csv

@@ -793,7 +793,6 @@ k8s.io/kubernetes/pkg/util/oom,vishh,0,
 k8s.io/kubernetes/pkg/util/parsers,derekwaynecarr,1,
 k8s.io/kubernetes/pkg/util/procfs,roberthbailey,1,
 k8s.io/kubernetes/pkg/util/slice,quinton-hoole,0,
-k8s.io/kubernetes/pkg/util/system,mwielgus,0,
 k8s.io/kubernetes/pkg/util/tail,zmerlynn,1,
 k8s.io/kubernetes/pkg/util/taints,rrati,0,
 k8s.io/kubernetes/pkg/util/term,davidopp,1,