Skip to content

Commit f917182

Browse files
committed
fix missing project id and delete deprecated LegacyIsMasterNode function
1 parent c51aeb1 commit f917182

File tree

6 files changed

+27
-75
lines changed

6 files changed

+27
-75
lines changed

clusterloader2/pkg/measurement/common/scheduler_latency.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ func (s *schedulerLatencyMeasurement) Execute(config *measurement.Config) ([]mea
9999

100100
var masterRegistered = false
101101
for _, node := range nodes.Items {
102-
if util.LegacyIsMasterNode(&node) || util.IsControlPlaneNode(&node) {
102+
if util.IsControlPlaneNode(&node) {
103103
masterRegistered = true
104104
}
105105
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func NewResourceUsageGatherer(c clientset.Interface, host string, port int, prov
116116

117117
masterNodes := sets.NewString()
118118
for _, node := range nodeList.Items {
119-
if pkgutil.LegacyIsMasterNode(&node) || pkgutil.IsControlPlaneNode(&node) {
119+
if pkgutil.IsControlPlaneNode(&node) {
120120
masterNodes.Insert(node.Name)
121121
}
122122
}

clusterloader2/pkg/prometheus/prometheus.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ func (pc *Controller) runNodeExporter() error {
540540
numMasters := 0
541541
for _, node := range nodes {
542542
node := node
543-
if util.LegacyIsMasterNode(&node) || util.IsControlPlaneNode(&node) {
543+
if util.IsControlPlaneNode(&node) {
544544
numMasters++
545545
g.Go(func() error {
546546
f, err := manifestsFS.Open(nodeExporterPod)

clusterloader2/pkg/provider/gce.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"context"
2121
"fmt"
2222
"os/exec"
23+
"regexp"
2324
"strings"
2425

2526
clientset "k8s.io/client-go/kubernetes"
@@ -83,16 +84,20 @@ func (p *GCEProvider) Metadata(c clientset.Interface) (map[string]string, error)
8384

8485
var masterInstanceIDs []string
8586
for _, node := range nodes {
86-
if sshutil.LegacyIsMasterNode(&node) || sshutil.IsControlPlaneNode(&node) {
87-
zone, ok := node.Labels["topology.kubernetes.io/zone"]
88-
if !ok {
89-
// Fallback to old label to make it work for old k8s versions.
90-
zone, ok = node.Labels["failure-domain.beta.kubernetes.io/zone"]
91-
if !ok {
92-
return nil, fmt.Errorf("unknown zone for %q node: no topology-related labels", node.Name)
87+
if sshutil.IsControlPlaneNode(&node) {
88+
var project, zone string
89+
if node.Spec.ProviderID != "" {
90+
// providerID is in the format: gce://project-id/zone/instance-name
91+
// https://github.com/kubernetes/cloud-provider-gcp/blob/2e539007132d518d1356c0eab9e02ba8dee8a25d/providers/gce/gce_util.go#L259
92+
r, _ := regexp.Compile("gce://([^/]+)/([^/]+)/([^/]+)")
93+
matches := r.FindStringSubmatch(node.Spec.ProviderID)
94+
95+
if len(matches) == 4 {
96+
project = matches[1]
97+
zone = matches[2]
9398
}
9499
}
95-
cmd := exec.Command("gcloud", "compute", "instances", "describe", "--format", "value(id)", "--zone", zone, node.Name)
100+
cmd := exec.Command("gcloud", "compute", "instances", "describe", "--format", "value(id)", "--zone", zone, node.Name, "--project", project)
96101
out, err := cmd.Output()
97102
if err != nil {
98103
var stderr string

clusterloader2/pkg/util/cluster.go

Lines changed: 11 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,29 +18,27 @@ package util
1818

1919
import (
2020
"fmt"
21-
"strings"
2221

2322
corev1 "k8s.io/api/core/v1"
2423
clientset "k8s.io/client-go/kubernetes"
2524
"k8s.io/klog/v2"
2625
"k8s.io/perf-tests/clusterloader2/pkg/framework/client"
2726
)
2827

29-
const keyMasterNodeLabel = "node-role.kubernetes.io/master"
30-
const keyControlPlaneNodeLabel = "node-role.kubernetes.io/control-plane"
28+
const keyControlPlaneNodeLabelTaint = "node-role.kubernetes.io/control-plane"
3129

3230
// Based on the following docs:
3331
// https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/#taint-based-evictions
3432
// https://kubernetes.io/docs/reference/labels-annotations-taints/
3533
var builtInTaintsKeys = []string{
3634
"node.kubernetes.io/not-ready",
3735
"node.kubernetes.io/unreachable",
38-
"node.kubernetes.io/pid-pressure",
39-
"node.kubernetes.io/out-of-disk",
36+
"node.kubernetes.io/unschedulable",
4037
"node.kubernetes.io/memory-pressure",
4138
"node.kubernetes.io/disk-pressure",
4239
"node.kubernetes.io/network-unavailable",
43-
"node.kubernetes.io/unschedulable",
40+
"node.kubernetes.io/pid-pressure",
41+
"node.kubernetes.io/out-of-service",
4442
"node.cloudprovider.kubernetes.io/uninitialized",
4543
"node.cloudprovider.kubernetes.io/shutdown",
4644
}
@@ -153,7 +151,7 @@ func GetMasterName(c clientset.Interface) (string, error) {
153151
return "", err
154152
}
155153
for i := range nodeList {
156-
if LegacyIsMasterNode(&nodeList[i]) || IsControlPlaneNode(&nodeList[i]) {
154+
if IsControlPlaneNode(&nodeList[i]) {
157155
return nodeList[i].Name, nil
158156
}
159157
}
@@ -168,7 +166,7 @@ func GetMasterIPs(c clientset.Interface, addressType corev1.NodeAddressType) ([]
168166
}
169167
var ips []string
170168
for i := range nodeList {
171-
if LegacyIsMasterNode(&nodeList[i]) || IsControlPlaneNode(&nodeList[i]) {
169+
if IsControlPlaneNode(&nodeList[i]) {
172170
for _, address := range nodeList[i].Status.Addresses {
173171
if address.Type == addressType && address.Address != "" {
174172
ips = append(ips, address.Address)
@@ -183,38 +181,16 @@ func GetMasterIPs(c clientset.Interface, addressType corev1.NodeAddressType) ([]
183181
return ips, nil
184182
}
185183

186-
// LegacyIsMasterNode returns true if given node is a registered master according
187-
// to the logic historically used for this function. This code path is deprecated
188-
// and the node disruption exclusion label should be used in the future.
189-
// This code will not be allowed to update to use the node-role label, since
190-
// node-roles may not be used for feature enablement.
191-
// DEPRECATED: this will be removed in Kubernetes 1.19
192-
func LegacyIsMasterNode(node *corev1.Node) bool {
184+
func IsControlPlaneNode(node *corev1.Node) bool {
193185
for key := range node.GetLabels() {
194-
if key == keyMasterNodeLabel {
186+
if key == keyControlPlaneNodeLabelTaint {
195187
return true
196188
}
197189
}
198-
199-
// We are trying to capture "master(-...)?$" regexp.
200-
// However, using regexp.MatchString() results even in more than 35%
201-
// of all space allocations in ControllerManager spent in this function.
202-
// That's why we are trying to be a bit smarter.
203-
nodeName := node.GetName()
204-
if strings.HasSuffix(nodeName, "master") {
205-
return true
206-
}
207-
if len(nodeName) >= 10 {
208-
return strings.HasSuffix(nodeName[:len(nodeName)-3], "master-")
209-
}
210-
return false
211-
}
212-
213-
func IsControlPlaneNode(node *corev1.Node) bool {
214-
for key := range node.GetLabels() {
215-
if key == keyControlPlaneNodeLabel {
190+
// https://kubernetes.io/docs/reference/labels-annotations-taints/#node-role-kubernetes-io-control-plane-taint
191+
for taint := range node.Spec.Taints {
192+
if node.Spec.Taints[taint].Key == keyControlPlaneNodeLabelTaint {
216193
return true
217194
}
218-
}
219195
return false
220196
}

clusterloader2/pkg/util/cluster_test.go

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -24,35 +24,6 @@ import (
2424
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2525
)
2626

27-
func TestLegacyIsMasterNode(t *testing.T) {
28-
testcases := map[string]struct {
29-
Name string
30-
Labels map[string]string
31-
expect bool
32-
}{
33-
"node with node label key": {
34-
Labels: map[string]string{keyMasterNodeLabel: ""},
35-
expect: true,
36-
},
37-
"node with node label key value": {
38-
Labels: map[string]string{keyMasterNodeLabel: "true"},
39-
expect: true,
40-
},
41-
"node without node label": {
42-
Labels: map[string]string{},
43-
expect: false,
44-
},
45-
}
46-
47-
for _, tc := range testcases {
48-
node := &corev1.Node{
49-
ObjectMeta: metav1.ObjectMeta{Labels: tc.Labels},
50-
}
51-
result := LegacyIsMasterNode(node)
52-
assert.Equal(t, tc.expect, result)
53-
}
54-
}
55-
5627
func TestIsControlPlaneNode(t *testing.T) {
5728
testcases := map[string]struct {
5829
Name string

0 commit comments

Comments
 (0)