From 789e69715d0531f34190650e1476ee76febd3a1f Mon Sep 17 00:00:00 2001 From: Oded Viner Date: Sun, 12 Jan 2025 14:51:21 +0200 Subject: [PATCH] Adjust Node Label Conditions Based on Full Label Name on strechcluster Signed-off-by: Oded Viner --- api/v1/topologymap.go | 27 ++++++++++++++----- .../storagecluster/cephcluster_test.go | 23 ++++++++-------- controllers/storagecluster/placement_test.go | 10 ++++--- .../ocs-operator/api/v4/v1/topologymap.go | 27 ++++++++++++++----- .../ocs-operator/api/v4/v1/topologymap.go | 27 ++++++++++++++----- 5 files changed, 78 insertions(+), 36 deletions(-) diff --git a/api/v1/topologymap.go b/api/v1/topologymap.go index 63c008d2f5..14584e942c 100644 --- a/api/v1/topologymap.go +++ b/api/v1/topologymap.go @@ -17,7 +17,7 @@ limitations under the License. package v1 import ( - "strings" + corev1 "k8s.io/api/core/v1" ) // NewNodeTopologyMap returns an initialized NodeTopologyMap @@ -61,18 +61,31 @@ func (m *NodeTopologyMap) Add(topologyKey string, value string) { m.Labels[topologyKey] = append(m.Labels[topologyKey], value) } -// GetKeyValues returns a node label matching the topologyKey and all values -// for that label across all storage nodes +// GetKeyValues returns a node label matching the supported topologyKey and all values +// for that label across all storage nodes. func (m *NodeTopologyMap) GetKeyValues(topologyKey string) (string, []string) { values := []string{} + // Supported failure domain labels + supportedLabels := map[string]string{ + "rack": "topology.rook.io/rack", + "hostname": corev1.LabelHostname, + "zone": corev1.LabelZoneFailureDomainStable, + } + + // Get the specific label based on the topologyKey + expectedLabel, exists := supportedLabels[topologyKey] + if !exists { + return "", values // Return empty if the topologyKey is unsupported + } + + // Match the expected label and fetch the values for label, labelValues := range m.Labels { - if strings.Contains(label, topologyKey) { - topologyKey = label + if label == expectedLabel { values = labelValues - break + return label, values } } - return topologyKey, values + return "", values // Return empty if no match is found } diff --git a/controllers/storagecluster/cephcluster_test.go b/controllers/storagecluster/cephcluster_test.go index 3833cb9913..6996e0eeb5 100644 --- a/controllers/storagecluster/cephcluster_test.go +++ b/controllers/storagecluster/cephcluster_test.go @@ -489,7 +489,7 @@ func TestStorageClassDeviceSetCreation(t *testing.T) { sc1.Spec.StorageDeviceSets = mockDeviceSets sc1.Status.NodeTopologies = &ocsv1.NodeTopologyMap{ Labels: map[string]ocsv1.TopologyLabelValues{ - zoneTopologyLabel: []string{ + corev1.LabelZoneFailureDomainStable: []string{ "zone1", "zone2", }, @@ -501,7 +501,7 @@ func TestStorageClassDeviceSetCreation(t *testing.T) { sc2.Spec.StorageDeviceSets = mockDeviceSets sc2.Status.NodeTopologies = &ocsv1.NodeTopologyMap{ Labels: map[string]ocsv1.TopologyLabelValues{ - zoneTopologyLabel: []string{ + corev1.LabelZoneFailureDomainStable: []string{ "zone1", "zone2", "zone3", @@ -514,7 +514,7 @@ func TestStorageClassDeviceSetCreation(t *testing.T) { sc3.Spec.StorageDeviceSets = mockDeviceSets sc3.Status.NodeTopologies = &ocsv1.NodeTopologyMap{ Labels: map[string]ocsv1.TopologyLabelValues{ - zoneTopologyLabel: []string{ + corev1.LabelZoneFailureDomainStable: []string{ "zone1", "zone2", "zone3", @@ -572,6 +572,7 @@ func TestStorageClassDeviceSetCreation(t *testing.T) { for _, c := range cases { t.Logf("Case: %s\n", c.label) + setFailureDomain(c.sc) actual := newStorageClassDeviceSets(c.sc) assert.Equal(t, defaults.DeviceSetReplica, len(actual)) deviceSet := c.sc.Spec.StorageDeviceSets[0] @@ -607,8 +608,8 @@ func TestStorageClassDeviceSetCreation(t *testing.T) { if c.topologyKey == "rack" { assert.Equal(t, defaults.RackTopologyKey, topologyKey) - } else if len(c.sc.Status.NodeTopologies.Labels[zoneTopologyLabel]) >= defaults.DeviceSetReplica { - assert.Equal(t, zoneTopologyLabel, topologyKey) + } else if len(c.sc.Status.NodeTopologies.Labels[corev1.LabelZoneFailureDomainStable]) >= defaults.DeviceSetReplica { + assert.Equal(t, corev1.LabelZoneFailureDomainStable, topologyKey) } else { assert.Equal(t, hostnameLabel, topologyKey) } @@ -765,7 +766,7 @@ func TestStorageClassDeviceSetCreationForArbiter(t *testing.T) { sc1.Spec.StorageDeviceSets = getMockDeviceSets("mock", 1, 4, true) sc1.Status.NodeTopologies = &ocsv1.NodeTopologyMap{ Labels: map[string]ocsv1.TopologyLabelValues{ - zoneTopologyLabel: []string{ + corev1.LabelZoneFailureDomainStable: []string{ "zone1", "zone2", }, @@ -773,7 +774,7 @@ func TestStorageClassDeviceSetCreationForArbiter(t *testing.T) { ArbiterLocation: "zone3", } sc1.Status.FailureDomain = "zone" - sc1.Status.FailureDomainKey = zoneTopologyLabel + sc1.Status.FailureDomainKey = corev1.LabelZoneFailureDomainStable sc1.Status.FailureDomainValues = []string{"zone1", "zone2"} sc2 := &ocsv1.StorageCluster{} @@ -783,11 +784,11 @@ func TestStorageClassDeviceSetCreationForArbiter(t *testing.T) { } sc2.Spec.StorageDeviceSets = getMockDeviceSets("mock", 1, 6, true) sc2.Status.NodeTopologies = &ocsv1.NodeTopologyMap{ - Labels: map[string]ocsv1.TopologyLabelValues{zoneTopologyLabel: []string{"zone1", "zone2"}}, + Labels: map[string]ocsv1.TopologyLabelValues{corev1.LabelZoneFailureDomainStable: []string{"zone1", "zone2"}}, ArbiterLocation: "zone3", } sc2.Status.FailureDomain = "zone" - sc2.Status.FailureDomainKey = zoneTopologyLabel + sc2.Status.FailureDomainKey = corev1.LabelZoneFailureDomainStable sc2.Status.FailureDomainValues = []string{"zone1", "zone2"} cases := []struct { @@ -798,12 +799,12 @@ func TestStorageClassDeviceSetCreationForArbiter(t *testing.T) { { label: "case 1", sc: sc1, - topologyKey: zoneTopologyLabel, + topologyKey: corev1.LabelZoneFailureDomainStable, }, { label: "case 2", sc: sc2, - topologyKey: zoneTopologyLabel, + topologyKey: corev1.LabelZoneFailureDomainStable, }, } diff --git a/controllers/storagecluster/placement_test.go b/controllers/storagecluster/placement_test.go index d5737c6f71..92e6e9a813 100644 --- a/controllers/storagecluster/placement_test.go +++ b/controllers/storagecluster/placement_test.go @@ -381,7 +381,7 @@ func TestGetPlacement(t *testing.T) { }, }, }, - TopologyKey: zoneTopologyLabel, + TopologyKey: corev1.LabelZoneFailureDomainStable, }, }, }, @@ -402,7 +402,7 @@ func TestGetPlacement(t *testing.T) { }, }, }, - TopologyKey: zoneTopologyLabel, + TopologyKey: corev1.LabelZoneFailureDomainStable, }, }, }, @@ -410,7 +410,7 @@ func TestGetPlacement(t *testing.T) { }, topologyMap: &ocsv1.NodeTopologyMap{ Labels: map[string]ocsv1.TopologyLabelValues{ - zoneTopologyLabel: []string{ + corev1.LabelZoneFailureDomainStable: []string{ "zone1", "zone2", "zone3", @@ -497,7 +497,9 @@ func TestGetPlacement(t *testing.T) { sc.Spec.Placement = c.placements sc.Spec.LabelSelector = c.labelSelector sc.Status.NodeTopologies = c.topologyMap - + if c.topologyMap != nil { + setFailureDomain(sc) + } expectedPlacement := c.expectedPlacements["all"] actualPlacement = getPlacement(sc, "all") assert.Equal(t, expectedPlacement, actualPlacement, c.label) diff --git a/metrics/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go b/metrics/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go index 63c008d2f5..14584e942c 100644 --- a/metrics/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go +++ b/metrics/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go @@ -17,7 +17,7 @@ limitations under the License. package v1 import ( - "strings" + corev1 "k8s.io/api/core/v1" ) // NewNodeTopologyMap returns an initialized NodeTopologyMap @@ -61,18 +61,31 @@ func (m *NodeTopologyMap) Add(topologyKey string, value string) { m.Labels[topologyKey] = append(m.Labels[topologyKey], value) } -// GetKeyValues returns a node label matching the topologyKey and all values -// for that label across all storage nodes +// GetKeyValues returns a node label matching the supported topologyKey and all values +// for that label across all storage nodes. func (m *NodeTopologyMap) GetKeyValues(topologyKey string) (string, []string) { values := []string{} + // Supported failure domain labels + supportedLabels := map[string]string{ + "rack": "topology.rook.io/rack", + "hostname": corev1.LabelHostname, + "zone": corev1.LabelZoneFailureDomainStable, + } + + // Get the specific label based on the topologyKey + expectedLabel, exists := supportedLabels[topologyKey] + if !exists { + return "", values // Return empty if the topologyKey is unsupported + } + + // Match the expected label and fetch the values for label, labelValues := range m.Labels { - if strings.Contains(label, topologyKey) { - topologyKey = label + if label == expectedLabel { values = labelValues - break + return label, values } } - return topologyKey, values + return "", values // Return empty if no match is found } diff --git a/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go b/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go index 63c008d2f5..14584e942c 100644 --- a/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go +++ b/vendor/github.com/red-hat-storage/ocs-operator/api/v4/v1/topologymap.go @@ -17,7 +17,7 @@ limitations under the License. package v1 import ( - "strings" + corev1 "k8s.io/api/core/v1" ) // NewNodeTopologyMap returns an initialized NodeTopologyMap @@ -61,18 +61,31 @@ func (m *NodeTopologyMap) Add(topologyKey string, value string) { m.Labels[topologyKey] = append(m.Labels[topologyKey], value) } -// GetKeyValues returns a node label matching the topologyKey and all values -// for that label across all storage nodes +// GetKeyValues returns a node label matching the supported topologyKey and all values +// for that label across all storage nodes. func (m *NodeTopologyMap) GetKeyValues(topologyKey string) (string, []string) { values := []string{} + // Supported failure domain labels + supportedLabels := map[string]string{ + "rack": "topology.rook.io/rack", + "hostname": corev1.LabelHostname, + "zone": corev1.LabelZoneFailureDomainStable, + } + + // Get the specific label based on the topologyKey + expectedLabel, exists := supportedLabels[topologyKey] + if !exists { + return "", values // Return empty if the topologyKey is unsupported + } + + // Match the expected label and fetch the values for label, labelValues := range m.Labels { - if strings.Contains(label, topologyKey) { - topologyKey = label + if label == expectedLabel { values = labelValues - break + return label, values } } - return topologyKey, values + return "", values // Return empty if no match is found }