fix missing project id and delete deprecated LegacyIsMasterNode function

Author: upodroid

clusterloader2/pkg/measurement/common/scheduler_latency.go

@@ -99,7 +99,7 @@ func (s *schedulerLatencyMeasurement) Execute(config *measurement.Config) ([]mea
 
 	var masterRegistered = false
 	for _, node := range nodes.Items {
-		if util.LegacyIsMasterNode(&node) || util.IsControlPlaneNode(&node) {
+		if util.IsControlPlaneNode(&node) {
 			masterRegistered = true
 		}
 	}

clusterloader2/pkg/measurement/util/gatherers/container_resource_gatherer.go

@@ -116,7 +116,7 @@ func NewResourceUsageGatherer(c clientset.Interface, host string, port int, prov
 
 		masterNodes := sets.NewString()
 		for _, node := range nodeList.Items {
-			if pkgutil.LegacyIsMasterNode(&node) || pkgutil.IsControlPlaneNode(&node) {
+			if pkgutil.IsControlPlaneNode(&node) {
 				masterNodes.Insert(node.Name)
 			}
 		}

clusterloader2/pkg/prometheus/prometheus.go

@@ -540,7 +540,7 @@ func (pc *Controller) runNodeExporter() error {
 	numMasters := 0
 	for _, node := range nodes {
 		node := node
-		if util.LegacyIsMasterNode(&node) || util.IsControlPlaneNode(&node) {
+		if util.IsControlPlaneNode(&node) {
 			numMasters++
 			g.Go(func() error {
 				f, err := manifestsFS.Open(nodeExporterPod)

clusterloader2/pkg/provider/gce.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"os/exec"
+	"regexp"
 	"strings"
 
 	clientset "k8s.io/client-go/kubernetes"
@@ -83,16 +84,20 @@ func (p *GCEProvider) Metadata(c clientset.Interface) (map[string]string, error)
 
 	var masterInstanceIDs []string
 	for _, node := range nodes {
-		if sshutil.LegacyIsMasterNode(&node) || sshutil.IsControlPlaneNode(&node) {
-			zone, ok := node.Labels["topology.kubernetes.io/zone"]
-			if !ok {
-				// Fallback to old label to make it work for old k8s versions.
-				zone, ok = node.Labels["failure-domain.beta.kubernetes.io/zone"]
-				if !ok {
-					return nil, fmt.Errorf("unknown zone for %q node: no topology-related labels", node.Name)
+		if sshutil.IsControlPlaneNode(&node) {
+			var project, zone string
+			if node.Spec.ProviderID != "" {
+				// providerID is in the format: gce://project-id/zone/instance-name
+				// https://github.com/kubernetes/cloud-provider-gcp/blob/2e539007132d518d1356c0eab9e02ba8dee8a25d/providers/gce/gce_util.go#L259
+				r, _ := regexp.Compile("gce://([^/]+)/([^/]+)/([^/]+)")
+				matches := r.FindStringSubmatch(node.Spec.ProviderID)
+
+				if len(matches) == 4 {
+					project = matches[1]
+					zone = matches[2]
 				}
 			}
-			cmd := exec.Command("gcloud", "compute", "instances", "describe", "--format", "value(id)", "--zone", zone, node.Name)
+			cmd := exec.Command("gcloud", "compute", "instances", "describe", "--format", "value(id)", "--zone", zone, node.Name, "--project", project)
 			out, err := cmd.Output()
 			if err != nil {
 				var stderr string

clusterloader2/pkg/util/cluster.go

@@ -18,29 +18,27 @@ package util
 
 import (
 	"fmt"
-	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/klog/v2"
 	"k8s.io/perf-tests/clusterloader2/pkg/framework/client"
 )
 
-const keyMasterNodeLabel = "node-role.kubernetes.io/master"
-const keyControlPlaneNodeLabel = "node-role.kubernetes.io/control-plane"
+const keyControlPlaneNodeLabelTaint = "node-role.kubernetes.io/control-plane"
 
 // Based on the following docs:
 // https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/#taint-based-evictions
 // https://kubernetes.io/docs/reference/labels-annotations-taints/
 var builtInTaintsKeys = []string{
 	"node.kubernetes.io/not-ready",
 	"node.kubernetes.io/unreachable",
-	"node.kubernetes.io/pid-pressure",
-	"node.kubernetes.io/out-of-disk",
+	"node.kubernetes.io/unschedulable",
 	"node.kubernetes.io/memory-pressure",
 	"node.kubernetes.io/disk-pressure",
 	"node.kubernetes.io/network-unavailable",
-	"node.kubernetes.io/unschedulable",
+	"node.kubernetes.io/pid-pressure",
+	"node.kubernetes.io/out-of-service",
 	"node.cloudprovider.kubernetes.io/uninitialized",
 	"node.cloudprovider.kubernetes.io/shutdown",
 }
@@ -153,7 +151,7 @@ func GetMasterName(c clientset.Interface) (string, error) {
 		return "", err
 	}
 	for i := range nodeList {
-		if LegacyIsMasterNode(&nodeList[i]) || IsControlPlaneNode(&nodeList[i]) {
+		if IsControlPlaneNode(&nodeList[i]) {
 			return nodeList[i].Name, nil
 		}
 	}
@@ -168,7 +166,7 @@ func GetMasterIPs(c clientset.Interface, addressType corev1.NodeAddressType) ([]
 	}
 	var ips []string
 	for i := range nodeList {
-		if LegacyIsMasterNode(&nodeList[i]) || IsControlPlaneNode(&nodeList[i]) {
+		if IsControlPlaneNode(&nodeList[i]) {
 			for _, address := range nodeList[i].Status.Addresses {
 				if address.Type == addressType && address.Address != "" {
 					ips = append(ips, address.Address)
@@ -183,36 +181,15 @@ func GetMasterIPs(c clientset.Interface, addressType corev1.NodeAddressType) ([]
 	return ips, nil
 }
 
-// LegacyIsMasterNode returns true if given node is a registered master according
-// to the logic historically used for this function. This code path is deprecated
-// and the node disruption exclusion label should be used in the future.
-// This code will not be allowed to update to use the node-role label, since
-// node-roles may not be used for feature enablement.
-// DEPRECATED: this will be removed in Kubernetes 1.19
-func LegacyIsMasterNode(node *corev1.Node) bool {
+func IsControlPlaneNode(node *corev1.Node) bool {
 	for key := range node.GetLabels() {
-		if key == keyMasterNodeLabel {
+		if key == keyControlPlaneNodeLabelTaint {
 			return true
 		}
 	}
-
-	// 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.
-	nodeName := node.GetName()
-	if strings.HasSuffix(nodeName, "master") {
-		return true
-	}
-	if len(nodeName) >= 10 {
-		return strings.HasSuffix(nodeName[:len(nodeName)-3], "master-")
-	}
-	return false
-}
-
-func IsControlPlaneNode(node *corev1.Node) bool {
-	for key := range node.GetLabels() {
-		if key == keyControlPlaneNodeLabel {
+	// https://kubernetes.io/docs/reference/labels-annotations-taints/#node-role-kubernetes-io-control-plane-taint
+	for _, taint := range node.Spec.Taints {
+		if taint.Key == keyControlPlaneNodeLabelTaint {
 			return true
 		}
 	}

clusterloader2/pkg/util/cluster_test.go

@@ -24,61 +24,52 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func TestLegacyIsMasterNode(t *testing.T) {
-	testcases := map[string]struct {
-		Name   string
-		Labels map[string]string
-		expect bool
-	}{
-		"node with node label key": {
-			Labels: map[string]string{keyMasterNodeLabel: ""},
-			expect: true,
-		},
-		"node with node label key value": {
-			Labels: map[string]string{keyMasterNodeLabel: "true"},
-			expect: true,
-		},
-		"node without node label": {
-			Labels: map[string]string{},
-			expect: false,
-		},
-	}
-
-	for _, tc := range testcases {
-		node := &corev1.Node{
-			ObjectMeta: metav1.ObjectMeta{Labels: tc.Labels},
-		}
-		result := LegacyIsMasterNode(node)
-		assert.Equal(t, tc.expect, result)
-	}
-}
-
 func TestIsControlPlaneNode(t *testing.T) {
 	testcases := map[string]struct {
 		Name   string
 		Labels map[string]string
+		Taints []corev1.Taint
 		expect bool
 	}{
 		"node with controlplane node-role key": {
-			Labels: map[string]string{keyControlPlaneNodeLabel: ""},
+			Labels: map[string]string{keyControlPlaneNodeLabelTaint: ""},
 			expect: true,
 		},
 		"node with controlplane node-role key and value as true": {
-			Labels: map[string]string{keyControlPlaneNodeLabel: "true"},
+			Labels: map[string]string{keyControlPlaneNodeLabelTaint: "true"},
 			expect: true,
 		},
 		"node with controlplane node-role key and value as false": {
-			Labels: map[string]string{keyControlPlaneNodeLabel: "false"},
+			Labels: map[string]string{keyControlPlaneNodeLabelTaint: "false"},
 			expect: true,
 		},
 		"node without controlplane node-role": {
 			Labels: map[string]string{},
 			expect: false,
 		},
+		"node without controlplane node-role taint": {
+			Taints: []corev1.Taint{
+				{
+					Key:    "some-other-taint",
+					Effect: corev1.TaintEffectNoSchedule,
+				},
+			},
+			expect: false,
+		},
+		"node with controlplane node-role taint": {
+			Taints: []corev1.Taint{
+				{
+					Key:    keyControlPlaneNodeLabelTaint,
+					Effect: corev1.TaintEffectNoSchedule,
+				},
+			},
+			expect: true,
+		},
 	}
 	for _, tc := range testcases {
 		node := &corev1.Node{
 			ObjectMeta: metav1.ObjectMeta{Labels: tc.Labels},
+			Spec:       corev1.NodeSpec{Taints: tc.Taints},
 		}
 		result := IsControlPlaneNode(node)
 		assert.Equal(t, tc.expect, result)