diff --git a/pkg/operator/ceph/cluster/osd/osd_test.go b/pkg/operator/ceph/cluster/osd/osd_test.go index bcaa024cf..2e8bedf16 100644 --- a/pkg/operator/ceph/cluster/osd/osd_test.go +++ b/pkg/operator/ceph/cluster/osd/osd_test.go @@ -570,18 +570,18 @@ func TestDetectCrushLocation(t *testing.T) { // update the location with valid topology labels nodeLabels = map[string]string{ - "failure-domain.beta.kubernetes.io/region": "region1", - "failure-domain.beta.kubernetes.io/zone": "zone1", - "topology.rook.io/rack": "rack1", - "topology.rook.io/row": "row1", + "topology.kubernetes.io/region": "region1", + "topology.kubernetes.io/zone": "zone", + "topology.rook.io/rack": "rack1", + "topology.rook.io/row": "row1", } - + // sorted in alphabetical order expected := []string{ "host=foo", "rack=rack1", "region=region1", "row=row1", - "zone=zone1", + "zone=zone", } updateLocationWithNodeLabels(&location, nodeLabels) diff --git a/pkg/operator/ceph/cluster/osd/topology/topology.go b/pkg/operator/ceph/cluster/osd/topology/topology.go index c1128b951..ab25bc173 100644 --- a/pkg/operator/ceph/cluster/osd/topology/topology.go +++ b/pkg/operator/ceph/cluster/osd/topology/topology.go @@ -43,6 +43,7 @@ var ( const ( topologyLabelPrefix = "topology.rook.io/" + labelHostname = "kubernetes.io/hostname" ) // ExtractTopologyFromLabels extracts rook topology from labels and returns a map from topology type to value @@ -56,6 +57,33 @@ func ExtractOSDTopologyFromLabels(labels map[string]string) (map[string]string, return topology, topologyAffinity } +func rookTopologyLabelsOrdered() []string { + topologyLabelsOrdered := []string{} + for i := len(CRUSHTopologyLabels) - 1; i >= 0; i-- { + label := CRUSHTopologyLabels[i] + topologyLabelsOrdered = append(topologyLabelsOrdered, topologyLabelPrefix+label) + } + return topologyLabelsOrdered +} + +func allKubernetesTopologyLabelsOrdered() []string { + return append( + append([]string{corev1.LabelTopologyRegion, + corev1.LabelTopologyZone}, + rookTopologyLabelsOrdered()...), + labelHostname, // host is the lowest level in the crush map hierarchy + ) +} + +func kubernetesTopologyLabelToCRUSHLabel(label string) string { + crushLabel := strings.Split(label, "/") + if crushLabel[len(crushLabel)-1] == "hostname" { + // kubernetes uses "kubernetes.io/hostname" whereas CRUSH uses "host" + return "host" + } + return crushLabel[len(crushLabel)-1] +} + // ExtractTopologyFromLabels extracts rook topology from labels and returns a map from topology type to value func extractTopologyFromLabels(labels map[string]string) (map[string]string, string) { topology := make(map[string]string) @@ -63,59 +91,34 @@ func extractTopologyFromLabels(labels map[string]string) (map[string]string, str // The topology affinity for the osd is the lowest topology label found in the hierarchy, // not including the host name var topologyAffinity string - - // check for the region k8s topology label that was deprecated in 1.17 - const regionLabel = "region" - region, ok := labels[corev1.LabelZoneRegion] - if ok { - topology[regionLabel] = region - topologyAffinity = formatTopologyAffinity(corev1.LabelZoneRegion, region) - } - - // check for the region k8s topology label that is GA in 1.17. - region, ok = labels[corev1.LabelZoneRegionStable] - if ok { - topology[regionLabel] = region - topologyAffinity = formatTopologyAffinity(corev1.LabelZoneRegionStable, region) - } - - // check for the zone k8s topology label that was deprecated in 1.17 - const zoneLabel = "zone" - zone, ok := labels[corev1.LabelZoneFailureDomain] - if ok { - topology[zoneLabel] = zone - topologyAffinity = formatTopologyAffinity(corev1.LabelZoneFailureDomain, zone) - } - - // check for the zone k8s topology label that is GA in 1.17. - zone, ok = labels[corev1.LabelZoneFailureDomainStable] - if ok { - topology[zoneLabel] = zone - topologyAffinity = formatTopologyAffinity(corev1.LabelZoneFailureDomainStable, zone) - } - - // ignore the region k8s topology label when it has the same value as the k8s zone label - if topology[regionLabel] == topology[zoneLabel] { - delete(topology, regionLabel) - } - - // get host - host, ok := labels[corev1.LabelHostname] - if ok { - topology["host"] = host - } + allKubernetesTopologyLabels := allKubernetesTopologyLabelsOrdered() // get the labels for the CRUSH map hierarchy - // iterate in reverse order so that the last topology found will be the lowest level in the hierarchy + // iterate in a way so the last topology found will be the lowest level in the hierarchy // for the topology affinity - for i := len(CRUSHTopologyLabels) - 1; i >= 0; i-- { - topologyID := CRUSHTopologyLabels[i] - label := topologyLabelPrefix + topologyID + for _, label := range allKubernetesTopologyLabels { + topologyID := kubernetesTopologyLabelToCRUSHLabel(label) if value, ok := labels[label]; ok { topology[topologyID] = value - topologyAffinity = formatTopologyAffinity(label, value) + if topologyID != "host" { + topologyAffinity = formatTopologyAffinity(label, value) + } } } + // iterate in lowest to highest order as the lowest level should be sustained and higher level duplicate + // should be removed + duplicateTopology := make(map[string]int) + for i := len(allKubernetesTopologyLabels) - 1; i >= 0; i-- { + topologyLabel := allKubernetesTopologyLabels[i] + if value, ok := labels[topologyLabel]; ok { + if _, ok := duplicateTopology[value]; ok { + delete(topology, kubernetesTopologyLabelToCRUSHLabel(topologyLabel)) + } else { + duplicateTopology[value] = 1 + } + } + } + return topology, topologyAffinity } diff --git a/pkg/operator/ceph/cluster/osd/topology/topology_test.go b/pkg/operator/ceph/cluster/osd/topology/topology_test.go index 720414f15..33a5c4cdd 100644 --- a/pkg/operator/ceph/cluster/osd/topology/topology_test.go +++ b/pkg/operator/ceph/cluster/osd/topology/topology_test.go @@ -42,22 +42,26 @@ func TestOrderedCRUSHLabels(t *testing.T) { func TestCleanTopologyLabels(t *testing.T) { // load all the expected labels nodeLabels := map[string]string{ - corev1.LabelZoneRegionStable: "r.region", - corev1.LabelZoneFailureDomainStable: "z.zone", - "kubernetes.io/hostname": "host.name", - "topology.rook.io/rack": "r.rack", - "topology.rook.io/row": "r.row", - "topology.rook.io/datacenter": "d.datacenter", - } + corev1.LabelZoneRegionStable: "r.region", + "kubernetes.io/hostname": "host.name", + "topology.rook.io/rack": "r.rack", + "topology.rook.io/row": "r.row", + "topology.rook.io/datacenter": "d.datacenter", + "topology.rook.io/room": "test", + "topology.rook.io/chassis": "test", + "topology.rook.io/pod": "test"} topology, affinity := ExtractOSDTopologyFromLabels(nodeLabels) assert.Equal(t, 6, len(topology)) assert.Equal(t, "r-region", topology["region"]) - assert.Equal(t, "z-zone", topology["zone"]) assert.Equal(t, "host-name", topology["host"]) assert.Equal(t, "r-rack", topology["rack"]) assert.Equal(t, "r-row", topology["row"]) assert.Equal(t, "d-datacenter", topology["datacenter"]) - assert.Equal(t, "topology.rook.io/rack=r.rack", affinity) + assert.Equal(t, "topology.rook.io/chassis=test", affinity) + assert.Equal(t, "test", topology["chassis"]) + assert.Equal(t, "", topology["pod"]) + assert.Equal(t, "", topology["room"]) + } func TestTopologyLabels(t *testing.T) { @@ -86,47 +90,21 @@ func TestTopologyLabels(t *testing.T) { // load all the expected labels nodeLabels = map[string]string{ - corev1.LabelZoneRegionStable: "r1", - corev1.LabelZoneFailureDomainStable: "z1", - "kubernetes.io/hostname": "myhost", - "topology.rook.io/rack": "rack1", - "topology.rook.io/row": "row1", - "topology.rook.io/datacenter": "d1", + corev1.LabelZoneRegionStable: "r1", + "kubernetes.io/hostname": "myhost", + "topology.rook.io/rack": "rack1", + "topology.rook.io/row": "row1", + "topology.rook.io/datacenter": "d1", } topology, affinity = extractTopologyFromLabels(nodeLabels) - assert.Equal(t, 6, len(topology)) + assert.Equal(t, 5, len(topology)) assert.Equal(t, "r1", topology["region"]) - assert.Equal(t, "z1", topology["zone"]) assert.Equal(t, "myhost", topology["host"]) assert.Equal(t, "rack1", topology["rack"]) assert.Equal(t, "row1", topology["row"]) assert.Equal(t, "d1", topology["datacenter"]) assert.Equal(t, "topology.rook.io/rack=rack1", affinity) - // ensure deprecated k8s labels are loaded - nodeLabels = map[string]string{ - corev1.LabelZoneRegion: "r1", - corev1.LabelZoneFailureDomain: "z1", - } - topology, affinity = extractTopologyFromLabels(nodeLabels) - assert.Equal(t, 2, len(topology)) - assert.Equal(t, "r1", topology["region"]) - assert.Equal(t, "z1", topology["zone"]) - assert.Equal(t, "failure-domain.beta.kubernetes.io/zone=z1", affinity) - - // ensure deprecated k8s labels are overridden - nodeLabels = map[string]string{ - corev1.LabelZoneRegionStable: "r1", - corev1.LabelZoneFailureDomainStable: "z1", - corev1.LabelZoneRegion: "oldregion", - corev1.LabelZoneFailureDomain: "oldzone", - } - topology, affinity = extractTopologyFromLabels(nodeLabels) - assert.Equal(t, 2, len(topology)) - assert.Equal(t, "r1", topology["region"]) - assert.Equal(t, "z1", topology["zone"]) - assert.Equal(t, "topology.kubernetes.io/zone=z1", affinity) - // invalid labels under topology.rook.io return an error nodeLabels = map[string]string{ "topology.rook.io/row/bad": "r1", @@ -134,16 +112,6 @@ func TestTopologyLabels(t *testing.T) { topology, affinity = extractTopologyFromLabels(nodeLabels) assert.Equal(t, 0, len(topology)) assert.Equal(t, "", affinity) - - // ignore the region k8s topology label when it has the same value as the k8s zone label - nodeLabels = map[string]string{ - corev1.LabelZoneRegionStable: "zone", - corev1.LabelZoneFailureDomainStable: "zone", - } - topology, affinity = extractTopologyFromLabels(nodeLabels) - assert.Equal(t, 1, len(topology)) - assert.Equal(t, "zone", topology["zone"]) - assert.Equal(t, "topology.kubernetes.io/zone=zone", affinity) } func TestGetDefaultTopologyLabels(t *testing.T) {