Merge pull request #92845 from oomichi/remove-DeprecatedMightBeMasterNode

Remove DeprecatedMightBeMasterNode()

test/e2e/framework/node/BUILD

@@ -24,7 +24,6 @@ go_library(
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//staging/src/k8s.io/client-go/util/retry:go_default_library",
         "//test/e2e/framework/log: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/node/resource.go

@@ -41,9 +41,6 @@ import (
 	clientset "k8s.io/client-go/kubernetes"
 	clientretry "k8s.io/client-go/util/retry"
 	e2elog "k8s.io/kubernetes/test/e2e/framework/log"
-
-	// TODO remove the direct dependency for internal k8s.io/kubernetes
-	"k8s.io/kubernetes/test/e2e/system"
 )
 
 const (
@@ -363,25 +360,6 @@ func GetReadyNodesIncludingTainted(c clientset.Interface) (nodes *v1.NodeList, e
 	return nodes, nil
 }
 
-// DeprecatedGetMasterAndWorkerNodes will return a list masters and schedulable worker nodes
-// NOTE: This function has been deprecated because of calling DeprecatedMightBeMasterNode().
-func DeprecatedGetMasterAndWorkerNodes(c clientset.Interface) (sets.String, *v1.NodeList, error) {
-	nodes := &v1.NodeList{}
-	masters := sets.NewString()
-	all, err := c.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{})
-	if err != nil {
-		return nil, nil, fmt.Errorf("get nodes error: %s", err)
-	}
-	for _, n := range all.Items {
-		if system.DeprecatedMightBeMasterNode(n.Name) {
-			masters.Insert(n.Name)
-		} else if isNodeSchedulableWithoutTaints(&n) {
-			nodes.Items = append(nodes.Items, n)
-		}
-	}
-	return masters, nodes, nil
-}
-
 // 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 {

test/e2e/storage/vsphere/vsphere_volume_vsan_policy.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"hash/fnv"
+	"regexp"
 	"time"
 
 	"strings"
@@ -81,7 +82,6 @@ var _ = utils.SIGDescribe("Storage Policy Based Volume Provisioning [Feature:vsp
 		scParameters map[string]string
 		policyName   string
 		tagPolicy    string
-		masterNode   string
 	)
 	ginkgo.BeforeEach(func() {
 		e2eskipper.SkipUnlessProviderIs("vsphere")
@@ -94,10 +94,6 @@ var _ = utils.SIGDescribe("Storage Policy Based Volume Provisioning [Feature:vsp
 		scParameters = make(map[string]string)
 		_, err := e2enode.GetRandomReadySchedulableNode(f.ClientSet)
 		framework.ExpectNoError(err)
-		masternodes, _, err := e2enode.DeprecatedGetMasterAndWorkerNodes(client)
-		framework.ExpectNoError(err)
-		gomega.Expect(masternodes).NotTo(gomega.BeEmpty())
-		masterNode = masternodes.List()[0]
 	})
 
 	// Valid policy.
@@ -211,7 +207,9 @@ var _ = utils.SIGDescribe("Storage Policy Based Volume Provisioning [Feature:vsp
 		scParameters[Datastore] = vsanDatastore
 		framework.Logf("Invoking test for SPBM storage policy: %+v", scParameters)
 		kubernetesClusterName := GetAndExpectStringEnvVar(KubernetesClusterName)
-		invokeStaleDummyVMTestWithStoragePolicy(client, masterNode, namespace, kubernetesClusterName, scParameters)
+		controlPlaneNode, err := getControlPlaneNode(client)
+		framework.ExpectNoError(err)
+		invokeStaleDummyVMTestWithStoragePolicy(client, controlPlaneNode, namespace, kubernetesClusterName, scParameters)
 	})
 
 	ginkgo.It("verify if a SPBM policy is not honored on a non-compatible datastore for dynamically provisioned pvc using storageclass", func() {
@@ -309,7 +307,9 @@ func invokeInvalidPolicyTestNeg(client clientset.Interface, namespace string, sc
 	return fmt.Errorf("Failure message: %+q", eventList.Items[0].Message)
 }
 
-func invokeStaleDummyVMTestWithStoragePolicy(client clientset.Interface, masterNode string, namespace string, clusterName string, scParameters map[string]string) {
+// invokeStaleDummyVMTestWithStoragePolicy assumes control plane node is present on the datacenter specified in the workspace section of vsphere.conf file.
+// With in-tree VCP, when the volume is created using storage policy, shadow (dummy) VM is getting created and deleted to apply SPBM policy on the volume.
+func invokeStaleDummyVMTestWithStoragePolicy(client clientset.Interface, controlPlaneNode string, namespace string, clusterName string, scParameters map[string]string) {
 	ginkgo.By("Creating Storage Class With storage policy params")
 	storageclass, err := client.StorageV1().StorageClasses().Create(context.TODO(), getVSphereStorageClassSpec("storagepolicysc", scParameters, nil, ""), metav1.CreateOptions{})
 	framework.ExpectNoError(err, fmt.Sprintf("Failed to create storage class with err: %v", err))
@@ -336,8 +336,27 @@ func invokeStaleDummyVMTestWithStoragePolicy(client clientset.Interface, masterN
 	fnvHash.Write([]byte(vmName))
 	dummyVMFullName := dummyVMPrefixName + "-" + fmt.Sprint(fnvHash.Sum32())
 	errorMsg := "Dummy VM - " + vmName + " is still present. Failing the test.."
-	nodeInfo := TestContext.NodeMapper.GetNodeInfo(masterNode)
+	nodeInfo := TestContext.NodeMapper.GetNodeInfo(controlPlaneNode)
 	isVMPresentFlag, err := nodeInfo.VSphere.IsVMPresent(dummyVMFullName, nodeInfo.DataCenterRef)
 	framework.ExpectNoError(err)
 	framework.ExpectEqual(isVMPresentFlag, false, errorMsg)
 }
+
+func getControlPlaneNode(client clientset.Interface) (string, error) {
+	regKubeScheduler := regexp.MustCompile("kube-scheduler-.*")
+	regKubeControllerManager := regexp.MustCompile("kube-controller-manager-.*")
+
+	podList, err := client.CoreV1().Pods(metav1.NamespaceSystem).List(context.TODO(), metav1.ListOptions{})
+	if err != nil {
+		return "", err
+	}
+	if len(podList.Items) < 1 {
+		return "", fmt.Errorf("could not find any pods in namespace %s to grab metrics from", metav1.NamespaceSystem)
+	}
+	for _, pod := range podList.Items {
+		if regKubeScheduler.MatchString(pod.Name) || regKubeControllerManager.MatchString(pod.Name) {
+			return pod.Spec.NodeName, nil
+		}
+	}
+	return "", fmt.Errorf("could not find any nodes where control plane pods are running")
+}

test/e2e/system/BUILD

@@ -1,22 +1,3 @@
-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/test/e2e/system",
-    visibility = ["//visibility:public"],
-)
-
-go_test(
-    name = "go_default_test",
-    srcs = ["system_utils_test.go"],
-    embed = [":go_default_library"],
-    deps = [
-        "//staging/src/k8s.io/api/core/v1:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
-    ],
-)
-
 filegroup(
     name = "package-srcs",
     srcs = glob(["**"]),

test/e2e/system/system_utils.go

@@ -1,39 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package system
-
-import (
-	"strings"
-)
-
-// DeprecatedMightBeMasterNode returns true if given node is a registered master.
-// This code must not be updated to use node role labels, since node role labels
-// 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.
-	// That's why we are trying to be a bit smarter.
-	if strings.HasSuffix(nodeName, "master") {
-		return true
-	}
-	if len(nodeName) >= 10 {
-		return strings.HasSuffix(nodeName[:len(nodeName)-3], "master-")
-	}
-	return false
-}

test/e2e/system/system_utils_test.go

@@ -1,47 +0,0 @@
-/*
-Copyright 2016 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package system
-
-import (
-	"testing"
-
-	"k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-)
-
-func TestIsMasterNode(t *testing.T) {
-	testCases := []struct {
-		input  string
-		result bool
-	}{
-		{"foo-master", true},
-		{"foo-master-", false},
-		{"foo-master-a", false},
-		{"foo-master-ab", false},
-		{"foo-master-abc", true},
-		{"foo-master-abdc", false},
-		{"foo-bar", false},
-	}
-
-	for _, tc := range testCases {
-		node := v1.Node{ObjectMeta: metav1.ObjectMeta{Name: tc.input}}
-		res := DeprecatedMightBeMasterNode(node.Name)
-		if res != tc.result {
-			t.Errorf("case \"%s\": expected %t, got %t", tc.input, tc.result, res)
-		}
-	}
-}