From 38c704a5b68fe4fb1eba38b7bce2154546cce79f Mon Sep 17 00:00:00 2001 From: upodroid Date: Sat, 25 Oct 2025 13:26:52 +0300 Subject: [PATCH] fix missing project id and delete deprecated LegacyIsMasterNode function --- .../measurement/common/scheduler_latency.go | 2 +- .../gatherers/container_resource_gatherer.go | 2 +- clusterloader2/pkg/prometheus/prometheus.go | 2 +- clusterloader2/pkg/provider/gce.go | 21 ++++--- clusterloader2/pkg/util/cluster.go | 45 ++++----------- clusterloader2/pkg/util/cluster_test.go | 55 ++++++++----------- 6 files changed, 50 insertions(+), 77 deletions(-) diff --git a/clusterloader2/pkg/measurement/common/scheduler_latency.go b/clusterloader2/pkg/measurement/common/scheduler_latency.go index ace95a5592..de82c31bd0 100644 --- a/clusterloader2/pkg/measurement/common/scheduler_latency.go +++ b/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 } } diff --git a/clusterloader2/pkg/measurement/util/gatherers/container_resource_gatherer.go b/clusterloader2/pkg/measurement/util/gatherers/container_resource_gatherer.go index c7e787d9ac..2997418959 100644 --- a/clusterloader2/pkg/measurement/util/gatherers/container_resource_gatherer.go +++ b/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) } } diff --git a/clusterloader2/pkg/prometheus/prometheus.go b/clusterloader2/pkg/prometheus/prometheus.go index 7eb0918b62..adb22e589d 100644 --- a/clusterloader2/pkg/prometheus/prometheus.go +++ b/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) diff --git a/clusterloader2/pkg/provider/gce.go b/clusterloader2/pkg/provider/gce.go index 7d90f385e7..a27437f13b 100644 --- a/clusterloader2/pkg/provider/gce.go +++ b/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 diff --git a/clusterloader2/pkg/util/cluster.go b/clusterloader2/pkg/util/cluster.go index f3ebdbbf74..ae6b4546bb 100644 --- a/clusterloader2/pkg/util/cluster.go +++ b/clusterloader2/pkg/util/cluster.go @@ -18,7 +18,6 @@ package util import ( "fmt" - "strings" corev1 "k8s.io/api/core/v1" clientset "k8s.io/client-go/kubernetes" @@ -26,8 +25,7 @@ import ( "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 @@ -35,12 +33,12 @@ const keyControlPlaneNodeLabel = "node-role.kubernetes.io/control-plane" 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 } } diff --git a/clusterloader2/pkg/util/cluster_test.go b/clusterloader2/pkg/util/cluster_test.go index fc641091e0..290ddd968e 100644 --- a/clusterloader2/pkg/util/cluster_test.go +++ b/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)