From 029b2d219784dacdb9b70b57673140a4159e81ad Mon Sep 17 00:00:00 2001 From: Kenichi Omichi Date: Mon, 6 Jul 2020 23:28:20 +0000 Subject: [PATCH] Remove DeprecatedMightBeMasterNode() This removes DeprecatedGetMasterAndWorkerNodes() usage from vsphere e2e test as deprecated function cleanup. Then all callers of DeprecatedMightBeMasterNode() have been removed. So this removes DeprecatedMightBeMasterNode() itself also. --- test/e2e/framework/node/BUILD | 1 - test/e2e/framework/node/resource.go | 22 --------- .../vsphere/vsphere_volume_vsan_policy.go | 35 ++++++++++---- test/e2e/system/BUILD | 19 -------- test/e2e/system/system_utils.go | 39 --------------- test/e2e/system/system_utils_test.go | 47 ------------------- 6 files changed, 27 insertions(+), 136 deletions(-) delete mode 100644 test/e2e/system/system_utils.go delete mode 100644 test/e2e/system/system_utils_test.go diff --git a/test/e2e/framework/node/BUILD b/test/e2e/framework/node/BUILD index 634d0eb3c4f..3c27ac6b6d0 100644 --- a/test/e2e/framework/node/BUILD +++ b/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", diff --git a/test/e2e/framework/node/resource.go b/test/e2e/framework/node/resource.go index 25f2435a9a7..f5e08fd1c0d 100644 --- a/test/e2e/framework/node/resource.go +++ b/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 { diff --git a/test/e2e/storage/vsphere/vsphere_volume_vsan_policy.go b/test/e2e/storage/vsphere/vsphere_volume_vsan_policy.go index ee2bef7ec84..05bbc883aa9 100644 --- a/test/e2e/storage/vsphere/vsphere_volume_vsan_policy.go +++ b/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") +} diff --git a/test/e2e/system/BUILD b/test/e2e/system/BUILD index 7d4478f92e2..6df04e38cd7 100644 --- a/test/e2e/system/BUILD +++ b/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(["**"]), diff --git a/test/e2e/system/system_utils.go b/test/e2e/system/system_utils.go deleted file mode 100644 index b845ae8463a..00000000000 --- a/test/e2e/system/system_utils.go +++ /dev/null @@ -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 -} diff --git a/test/e2e/system/system_utils_test.go b/test/e2e/system/system_utils_test.go deleted file mode 100644 index 944e684bfa7..00000000000 --- a/test/e2e/system/system_utils_test.go +++ /dev/null @@ -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) - } - } -}