From 40684024593cab961fc86e027b5403bcb64fe5ac Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 6 Nov 2020 08:32:34 -0800 Subject: [PATCH 1/3] Change trivial topology labels In these cases the actual label key is incidental. --- pkg/apis/core/validation/validation_test.go | 8 ++-- .../plugins/podtopologyspread/scoring_test.go | 4 +- .../selectorspread/selector_spread_test.go | 2 +- pkg/scheduler/internal/cache/cache_test.go | 4 +- .../internal/cache/node_tree_test.go | 40 +++++++++---------- test/integration/node/lifecycle_test.go | 2 +- .../config/performance-config.yaml | 4 +- .../config/pod-with-node-affinity.yaml | 2 +- .../config/pod-with-pod-affinity.yaml | 2 +- .../scheduler_perf_legacy_test.go | 2 +- 10 files changed, 35 insertions(+), 35 deletions(-) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index d4dec3827ce..415f7c6b527 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -11061,8 +11061,8 @@ func TestValidateServiceCreate(t *testing.T) { tweakSvc: func(s *core.Service) { s.Spec.TopologyKeys = []string{ "kubernetes.io/hostname", - "failure-domain.beta.kubernetes.io/zone", - "failure-domain.beta.kubernetes.io/region", + "topology.kubernetes.io/zone", + "topology.kubernetes.io/region", v1.TopologyKeyAny, } }, @@ -11090,7 +11090,7 @@ func TestValidateServiceCreate(t *testing.T) { s.Spec.TopologyKeys = []string{ "kubernetes.io/hostname", v1.TopologyKeyAny, - "failure-domain.beta.kubernetes.io/zone", + "topology.kubernetes.io/zone", } }, numErrs: 1, @@ -11101,7 +11101,7 @@ func TestValidateServiceCreate(t *testing.T) { s.Spec.TopologyKeys = []string{ "kubernetes.io/hostname", "kubernetes.io/hostname", - "failure-domain.beta.kubernetes.io/zone", + "topology.kubernetes.io/zone", } }, numErrs: 1, diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go index 17f90d5074b..48933d6d1ec 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go @@ -727,7 +727,7 @@ func BenchmarkTestPodTopologySpreadScore(b *testing.B) { { name: "1000nodes/single-constraint-zone", pod: st.MakePod().Name("p").Label("foo", ""). - SpreadConstraint(1, v1.LabelFailureDomainBetaZone, v1.ScheduleAnyway, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, v1.LabelTopologyZone, v1.ScheduleAnyway, st.MakeLabelSelector().Exists("foo").Obj()). Obj(), existingPodsNum: 10000, allNodesNum: 1000, @@ -745,7 +745,7 @@ func BenchmarkTestPodTopologySpreadScore(b *testing.B) { { name: "1000nodes/two-Constraints-zone-node", pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). - SpreadConstraint(1, v1.LabelFailureDomainBetaZone, v1.ScheduleAnyway, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, v1.LabelTopologyZone, v1.ScheduleAnyway, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, v1.LabelHostname, v1.ScheduleAnyway, st.MakeLabelSelector().Exists("bar").Obj()). Obj(), existingPodsNum: 10000, diff --git a/pkg/scheduler/framework/plugins/selectorspread/selector_spread_test.go b/pkg/scheduler/framework/plugins/selectorspread/selector_spread_test.go index dc70262d72c..81f80d63651 100644 --- a/pkg/scheduler/framework/plugins/selectorspread/selector_spread_test.go +++ b/pkg/scheduler/framework/plugins/selectorspread/selector_spread_test.go @@ -441,7 +441,7 @@ func TestZoneSelectorSpreadPriority(t *testing.T) { buildNodeLabels := func(failureDomain string) map[string]string { labels := map[string]string{ - v1.LabelFailureDomainBetaZone: failureDomain, + v1.LabelTopologyZone: failureDomain, } return labels } diff --git a/pkg/scheduler/internal/cache/cache_test.go b/pkg/scheduler/internal/cache/cache_test.go index 4ed77d20b85..272270622e0 100644 --- a/pkg/scheduler/internal/cache/cache_test.go +++ b/pkg/scheduler/internal/cache/cache_test.go @@ -1572,8 +1572,8 @@ func TestSchedulerCache_updateNodeInfoSnapshotList(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("node-%d", i), Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: fmt.Sprintf("region-%d", zone), - v1.LabelFailureDomainBetaZone: fmt.Sprintf("zone-%d", zone), + v1.LabelTopologyRegion: fmt.Sprintf("region-%d", zone), + v1.LabelTopologyZone: fmt.Sprintf("zone-%d", zone), }, }, }) diff --git a/pkg/scheduler/internal/cache/node_tree_test.go b/pkg/scheduler/internal/cache/node_tree_test.go index de5a08376d1..4a4abe9e6c9 100644 --- a/pkg/scheduler/internal/cache/node_tree_test.go +++ b/pkg/scheduler/internal/cache/node_tree_test.go @@ -36,7 +36,7 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-1", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-1", + v1.LabelTopologyRegion: "region-1", }, }, }, @@ -45,7 +45,7 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-2", Labels: map[string]string{ - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -54,8 +54,8 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-3", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-1", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-1", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -64,8 +64,8 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-4", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-1", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-1", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -74,8 +74,8 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-5", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-1", - v1.LabelFailureDomainBetaZone: "zone-3", + v1.LabelTopologyRegion: "region-1", + v1.LabelTopologyZone: "zone-3", }, }, }, @@ -84,8 +84,8 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-6", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-2", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-2", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -94,8 +94,8 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-7", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-2", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-2", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -104,8 +104,8 @@ var allNodes = []*v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node-8", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-2", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-2", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -282,8 +282,8 @@ func TestNodeTree_UpdateNode(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "node-0", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-1", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-1", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -302,8 +302,8 @@ func TestNodeTree_UpdateNode(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "node-0", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-1", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-1", + v1.LabelTopologyZone: "zone-2", }, }, }, @@ -318,8 +318,8 @@ func TestNodeTree_UpdateNode(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "node-new", Labels: map[string]string{ - v1.LabelFailureDomainBetaRegion: "region-1", - v1.LabelFailureDomainBetaZone: "zone-2", + v1.LabelTopologyRegion: "region-1", + v1.LabelTopologyZone: "zone-2", }, }, }, diff --git a/test/integration/node/lifecycle_test.go b/test/integration/node/lifecycle_test.go index cffc3ccf1a1..871ab83bb9e 100644 --- a/test/integration/node/lifecycle_test.go +++ b/test/integration/node/lifecycle_test.go @@ -181,7 +181,7 @@ func TestTaintBasedEvictions(t *testing.T) { nodes = append(nodes, &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("node-%d", i), - Labels: map[string]string{v1.LabelFailureDomainBetaRegion: "region1", v1.LabelFailureDomainBetaZone: "zone1"}, + Labels: map[string]string{v1.LabelTopologyRegion: "region1", v1.LabelTopologyZone: "zone1"}, }, Spec: v1.NodeSpec{}, Status: v1.NodeStatus{ diff --git a/test/integration/scheduler_perf/config/performance-config.yaml b/test/integration/scheduler_perf/config/performance-config.yaml index 5644f93ca33..2278319b0d7 100644 --- a/test/integration/scheduler_perf/config/performance-config.yaml +++ b/test/integration/scheduler_perf/config/performance-config.yaml @@ -171,7 +171,7 @@ countParam: $initNodes nodeTemplatePath: config/node-default.yaml labelNodePrepareStrategy: - labelKey: "failure-domain.beta.kubernetes.io/zone" + labelKey: "topology.kubernetes.io/zone" labelValues: ["zone1"] - opcode: createPods countParam: $initPods @@ -254,7 +254,7 @@ countParam: $initNodes nodeTemplatePath: config/node-default.yaml labelNodePrepareStrategy: - labelKey: "failure-domain.beta.kubernetes.io/zone" + labelKey: "topology.kubernetes.io/zone" labelValues: ["zone1"] - opcode: createPods countParam: $initPods diff --git a/test/integration/scheduler_perf/config/pod-with-node-affinity.yaml b/test/integration/scheduler_perf/config/pod-with-node-affinity.yaml index 813b949981a..352f7e91124 100644 --- a/test/integration/scheduler_perf/config/pod-with-node-affinity.yaml +++ b/test/integration/scheduler_perf/config/pod-with-node-affinity.yaml @@ -8,7 +8,7 @@ spec: requiredDuringSchedulingIgnoredDuringExecution: nodeSelectorTerms: - matchExpressions: - - key: failure-domain.beta.kubernetes.io/zone + - key: topology.kubernetes.io/zone operator: In values: - zone1 diff --git a/test/integration/scheduler_perf/config/pod-with-pod-affinity.yaml b/test/integration/scheduler_perf/config/pod-with-pod-affinity.yaml index 2b60af6822b..f523a1d8036 100644 --- a/test/integration/scheduler_perf/config/pod-with-pod-affinity.yaml +++ b/test/integration/scheduler_perf/config/pod-with-pod-affinity.yaml @@ -11,7 +11,7 @@ spec: - labelSelector: matchLabels: color: blue - topologyKey: failure-domain.beta.kubernetes.io/zone + topologyKey: topology.kubernetes.io/zone namespaces: ["sched-test", "sched-setup"] containers: - image: k8s.gcr.io/pause:3.2 diff --git a/test/integration/scheduler_perf/scheduler_perf_legacy_test.go b/test/integration/scheduler_perf/scheduler_perf_legacy_test.go index 3438ec25fd5..f482034ab50 100644 --- a/test/integration/scheduler_perf/scheduler_perf_legacy_test.go +++ b/test/integration/scheduler_perf/scheduler_perf_legacy_test.go @@ -136,7 +136,7 @@ func BenchmarkSchedulingWaitForFirstConsumerPVs(b *testing.B) { } basePod := makeBasePod() testStrategy := testutils.NewCreatePodWithPersistentVolumeWithFirstConsumerStrategy(gceVolumeFactory, basePod) - nodeStrategy := testutils.NewLabelNodePrepareStrategy(v1.LabelFailureDomainBetaZone, "zone1") + nodeStrategy := testutils.NewLabelNodePrepareStrategy(v1.LabelTopologyZone, "zone1") for _, test := range tests { name := fmt.Sprintf("%vNodes/%vPods", test.nodes, test.existingPods) b.Run(name, func(b *testing.B) { From f63ca48a1f5de2dd9505b3fbb9404b9a97cb2128 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 6 Nov 2020 08:43:00 -0800 Subject: [PATCH 2/3] Clean up comments around old topology labels --- pkg/util/node/node.go | 5 ++-- .../k8s.io/api/core/v1/well_known_labels.go | 23 +++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/pkg/util/node/node.go b/pkg/util/node/node.go index e075ef4a590..15cabbb4fb0 100644 --- a/pkg/util/node/node.go +++ b/pkg/util/node/node.go @@ -187,13 +187,14 @@ func GetZoneKey(node *v1.Node) string { return "" } - // TODO: prefer stable labels for zone in v1.18 + // TODO: "failure-domain.beta..." names are deprecated, but will + // stick around a long time due to existing on old extant objects like PVs. + // Maybe one day we can stop considering them (see #88493). zone, ok := labels[v1.LabelFailureDomainBetaZone] if !ok { zone, _ = labels[v1.LabelTopologyZone] } - // TODO: prefer stable labels for region in v1.18 region, ok := labels[v1.LabelFailureDomainBetaRegion] if !ok { region, _ = labels[v1.LabelTopologyRegion] diff --git a/staging/src/k8s.io/api/core/v1/well_known_labels.go b/staging/src/k8s.io/api/core/v1/well_known_labels.go index a506f17f65b..1b6529bc249 100644 --- a/staging/src/k8s.io/api/core/v1/well_known_labels.go +++ b/staging/src/k8s.io/api/core/v1/well_known_labels.go @@ -19,16 +19,21 @@ package v1 const ( LabelHostname = "kubernetes.io/hostname" - LabelFailureDomainBetaZone = "failure-domain.beta.kubernetes.io/zone" - LabelFailureDomainBetaRegion = "failure-domain.beta.kubernetes.io/region" - LabelTopologyZone = "topology.kubernetes.io/zone" - LabelTopologyRegion = "topology.kubernetes.io/region" + LabelTopologyZone = "topology.kubernetes.io/zone" + LabelTopologyRegion = "topology.kubernetes.io/region" - // Legacy names for compat. - LabelZoneFailureDomain = LabelFailureDomainBetaZone // deprecated, remove after 1.20 - LabelZoneRegion = LabelFailureDomainBetaRegion // deprecated, remove after 1.20 - LabelZoneFailureDomainStable = LabelTopologyZone - LabelZoneRegionStable = LabelTopologyRegion + // These label have been deprecated since 1.17, but will be supported for + // the foreseeable future, to accommodate things like long-lived PVs that + // use them. New users should prefer the "topology.kubernetes.io/*" + // equivalents. + LabelFailureDomainBetaZone = "failure-domain.beta.kubernetes.io/zone" // deprecated + LabelFailureDomainBetaRegion = "failure-domain.beta.kubernetes.io/region" // deprecated + + // Retained for compat when vendored. Do not use these consts in new code. + LabelZoneFailureDomain = LabelFailureDomainBetaZone // deprecated + LabelZoneRegion = LabelFailureDomainBetaRegion // deprecated + LabelZoneFailureDomainStable = LabelTopologyZone // deprecated + LabelZoneRegionStable = LabelTopologyRegion // deprecated LabelInstanceType = "beta.kubernetes.io/instance-type" LabelInstanceTypeStable = "node.kubernetes.io/instance-type" From 3bd337baf487a0ad9d748e7e0362b3d1b5eb5c4d Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 6 Nov 2020 08:47:32 -0800 Subject: [PATCH 3/3] Make tests deal with old and new topology labels --- .../pkg/endpoints/handlers/fieldmanager/node.yaml | 2 ++ test/e2e/common/util.go | 2 ++ test/e2e/network/firewall.go | 2 ++ test/e2e/scheduling/ubernetes_lite.go | 14 +++++++------- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/node.yaml b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/node.yaml index 13a14cf449b..de69de83e63 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/node.yaml +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/fieldmanager/node.yaml @@ -15,6 +15,8 @@ metadata: cloud.google.com/gke-os-distribution: cos failure-domain.beta.kubernetes.io/region: us-central1 failure-domain.beta.kubernetes.io/zone: us-central1-b + topology.kubernetes.io/region: us-central1 + topology.kubernetes.io/zone: us-central1-b kubernetes.io/hostname: node-default-pool-something name: node-default-pool-something resourceVersion: "211582541" diff --git a/test/e2e/common/util.go b/test/e2e/common/util.go index 02e4c2a78bc..d84e91761ad 100644 --- a/test/e2e/common/util.go +++ b/test/e2e/common/util.go @@ -163,6 +163,8 @@ func RestartNodes(c clientset.Interface, nodes []v1.Node) error { zone := framework.TestContext.CloudConfig.Zone if z, ok := node.Labels[v1.LabelFailureDomainBetaZone]; ok { zone = z + } else if z, ok := node.Labels[v1.LabelTopologyZone]; ok { + zone = z } nodeNamesByZone[zone] = append(nodeNamesByZone[zone], node.Name) } diff --git a/test/e2e/network/firewall.go b/test/e2e/network/firewall.go index 3267a6901ea..96bd676ad26 100644 --- a/test/e2e/network/firewall.go +++ b/test/e2e/network/firewall.go @@ -182,6 +182,8 @@ var _ = SIGDescribe("Firewall rule", func() { zone := cloudConfig.Zone if zoneInLabel, ok := nodeList.Items[0].Labels[v1.LabelFailureDomainBetaZone]; ok { zone = zoneInLabel + } else if zoneInLabel, ok := nodeList.Items[0].Labels[v1.LabelTopologyZone]; ok { + zone = zoneInLabel } removedTags := gce.SetInstanceTags(cloudConfig, nodesNames[0], zone, []string{}) defer func() { diff --git a/test/e2e/scheduling/ubernetes_lite.go b/test/e2e/scheduling/ubernetes_lite.go index 0e019571c80..4c5d55a7bdd 100644 --- a/test/e2e/scheduling/ubernetes_lite.go +++ b/test/e2e/scheduling/ubernetes_lite.go @@ -23,7 +23,7 @@ import ( "github.com/onsi/ginkgo" "github.com/onsi/gomega" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/intstr" @@ -120,13 +120,13 @@ func SpreadServiceOrFail(f *framework.Framework, replicaCount int, image string) // Find the name of the zone in which a Node is running func getZoneNameForNode(node v1.Node) (string, error) { - for key, value := range node.Labels { - if key == v1.LabelFailureDomainBetaZone { - return value, nil - } + if z, ok := node.Labels[v1.LabelFailureDomainBetaZone]; ok { + return z, nil + } else if z, ok := node.Labels[v1.LabelTopologyZone]; ok { + return z, nil } - return "", fmt.Errorf("node %s doesn't have zone label %s", - node.Name, v1.LabelFailureDomainBetaZone) + return "", fmt.Errorf("node %s doesn't have zone label %s or %s", + node.Name, v1.LabelFailureDomainBetaZone, v1.LabelTopologyZone) } // Return the number of zones in which we have nodes in this cluster.