Use generic zone labels (#360)

samples/secret.yaml

@@ -44,6 +44,9 @@
   #   # name: The name of the container orchestrator's availability zone to which the PowerFlex system
   #   # should be mapped.
   #   name: "zoneA"
+  #   # labelKey: The name of the label used for the availability zone to which the PowerFlex system
+  #   # should be mapped.
+  #   labelKey: "topology.kubernetes.io/zone"
   #   # protectionDomains: A list of the protection domains and their associated pools, defined in
   #   # the PowerFlex system.
   #   # Currently, csi-powerflex only supports one protection domain per zone.

service/controller.go

@@ -219,11 +219,12 @@ func (s *service) CreateVolume(
 	var volumeTopology []*csi.Topology
 	systemSegments := map[string]string{} // topology segments matching requested system for a volume
 
-	// Handle Zone topology, which happens when node is annotated with "Zone" label
+	// Handle Zone topology, which happens when node is annotated with a matching zone label
 	if len(zoneTargetMap) != 0 && accessibility != nil && len(accessibility.GetPreferred()) > 0 {
 		for _, topo := range accessibility.GetPreferred() {
 			for topoLabel, zoneName := range topo.Segments {
-				if strings.HasPrefix(topoLabel, "zone."+Name) {
+				Log.Infof("Zoning based on label %s", s.opts.zoneLabelKey)
+				if strings.HasPrefix(topoLabel, s.opts.zoneLabelKey) {
 					zoneTarget, ok := zoneTargetMap[ZoneName(zoneName)]
 					if !ok {
 						Log.Infof("no zone target for %s", zoneTarget)
@@ -241,7 +242,7 @@ func (s *service) CreateVolume(
 						continue
 					}
 
-					systemSegments["zone."+Name] = zoneName
+					systemSegments[s.opts.zoneLabelKey] = zoneName
 					volumeTopology = append(volumeTopology, &csi.Topology{
 						Segments: systemSegments,
 					})

service/features/array-config/multi_az

@@ -8,6 +8,7 @@
       "systemID": "14dbbf5617523654",
       "zone": {
          "name": "zoneA",
+         "labelKey": "zone.csi-vxflexos.dellemc.com",
          "protectionDomains": [
             {
                "name": "mocksystem",
@@ -27,6 +28,7 @@
       "systemID": "15dbbf5617523655",
       "zone": {
          "name": "zoneB",
+         "labelKey": "zone.csi-vxflexos.dellemc.com",
          "protectionDomains": [
             {
                "name": "mocksystem",

service/features/array-config/multi_az_custom_labels

@@ -0,0 +1,22 @@
+[
+   {
+      "endpoint": "http://127.0.0.1",
+      "username": "username",
+      "password": "password",
+      "insecure": true,
+      "isDefault": true,
+      "systemID": "14dbbf5617523654",
+      "zone": {
+         "name": "zone1",
+         "labelKey": "topology.k8s.io/zone",
+         "protectionDomains": [
+            {
+               "name": "pd",
+               "pools": [
+                  "pool1"
+               ]
+            }
+         ]
+      }
+   }
+]

service/features/service.feature

@@ -1572,12 +1572,10 @@ Feature: VxFlex OS CSI interface
 
   Scenario: Call NodeGetInfo with zone label
     Given a VxFlexOS service
-    And I use config "multi_az"
+    And I use config <config>
     When I call NodeGetInfo with zone labels
     Then a valid NodeGetInfo is returned with node topology
-
-  Scenario: Call NodeGetInfo with zone label with invalid config
-    Given a VxFlexOS service
-    And I use config "invalid_multi_az"
-    When I call NodeGetInfo with zone labels
-    Then a NodeGetInfo is returned without zone system topology
+    Examples:
+      | config             |
+      | "multi_az"         |
+      | "multi_az_custom_labels" |

service/node.go

@@ -758,8 +758,9 @@ func (s *service) NodeGetInfo(
 	// csi-vxflexos.dellemc.com/<systemID>: <provisionerName>
 	Log.Infof("Arrays: %+v", s.opts.arrays)
 	topology := map[string]string{}
-	if zone, ok := labels["zone."+Name]; ok {
-		topology["zone."+Name] = zone
+
+	if zone, ok := labels[s.opts.zoneLabelKey]; ok {
+		topology[s.opts.zoneLabelKey] = zone
 	}
 
 	for _, array := range s.opts.arrays {
@@ -772,7 +773,7 @@ func (s *service) NodeGetInfo(
 			topology[Name+"/"+array.SystemID+"-nfs"] = "true"
 		}
 
-		if zone, ok := topology["zone."+Name]; ok {
+		if zone, ok := topology[s.opts.zoneLabelKey]; ok {
 			if zone == string(array.AvailabilityZone.Name) {
 				// Add only the secret values with the correct zone.
 				nodeID, _ := GetNodeUID(ctx, s)

service/service.go

@@ -130,6 +130,7 @@ type (
 // AvailabilityZone provides a mapping between cluster zones labels and storage systems
 type AvailabilityZone struct {
 	Name              ZoneName           `json:"name"`
+	LabelKey          string             `json:"labelKey"`
 	ProtectionDomains []ProtectionDomain `json:"protectionDomains"`
 }
 
@@ -186,6 +187,7 @@ type Opts struct {
 	IsQuotaEnabled             bool   // allow driver to enable quota limits for NFS volumes
 	ExternalAccess             string // used for adding extra IP/IP range to the NFS export
 	KubeNodeName               string
+	zoneLabelKey               string
 }
 
 type service struct {
@@ -417,6 +419,13 @@ func (s *service) BeforeServe(
 		return err
 	}
 
+	// if custom zoning is being used, find the common label from the array secret
+	opts.zoneLabelKey, err = getZoneKeyLabelFromSecret(opts.arrays)
+	if err != nil {
+		Log.Warnf("unable to get zone key from secret: %s", err.Error())
+		return err
+	}
+
 	if err = s.ProcessMapSecretChange(); err != nil {
 		Log.Warnf("unable to configure dynamic configMap secret change detection : %s", err.Error())
 		return err
@@ -1873,3 +1882,21 @@ func ParseInt64FromContext(ctx context.Context, key string) (int64, error) {
 func lookupEnv(ctx context.Context, key string) (string, bool) {
 	return csictx.LookupEnv(ctx, key)
 }
+
+func getZoneKeyLabelFromSecret(arrays map[string]*ArrayConnectionData) (string, error) {
+	zoneKeyLabel := ""
+
+	for _, array := range arrays {
+		if array.AvailabilityZone != nil {
+			if zoneKeyLabel == "" {
+				// Assumes that the key parameter is not empty
+				zoneKeyLabel = array.AvailabilityZone.LabelKey
+			} else if zoneKeyLabel != array.AvailabilityZone.LabelKey {
+				Log.Warnf("array %s zone key %s does not match %s", array.SystemID, array.AvailabilityZone.LabelKey, zoneKeyLabel)
+				return "", fmt.Errorf("array %s zone key %s does not match %s", array.SystemID, array.AvailabilityZone.LabelKey, zoneKeyLabel)
+			}
+		}
+	}
+
+	return zoneKeyLabel, nil
+}

service/service_unit_test.go

@@ -517,6 +517,74 @@ func TestGetIPAddressByInterface(t *testing.T) {
 	}
 }
 
+func TestGetZoneKeyLabelFromSecret(t *testing.T) {
+	tests := []struct {
+		name          string
+		arrays        map[string]*ArrayConnectionData
+		expectedLabel string
+		expectedErr   error
+	}{
+		{
+			name:          "Empty array connection data",
+			arrays:        map[string]*ArrayConnectionData{},
+			expectedLabel: "",
+			expectedErr:   nil,
+		},
+		{
+			name: "Array connection data with same zone label keys",
+			arrays: map[string]*ArrayConnectionData{
+				"array1": {
+					AvailabilityZone: &AvailabilityZone{
+						Name:     "zone1",
+						LabelKey: "custom-zone.io/area",
+					},
+				},
+				"array2": {
+					AvailabilityZone: &AvailabilityZone{
+						Name:     "zone2",
+						LabelKey: "custom-zone.io/area",
+					},
+				},
+			},
+			expectedLabel: "custom-zone.io/area",
+			expectedErr:   nil,
+		},
+		{
+			name: "Array connection data with different label keys",
+			arrays: map[string]*ArrayConnectionData{
+				"array1": {
+					SystemID: "system-1",
+					AvailabilityZone: &AvailabilityZone{
+						Name:     "zone1",
+						LabelKey: "custom-zone-1.io/area",
+					},
+				},
+				"array2": {
+					SystemID: "system-2",
+					AvailabilityZone: &AvailabilityZone{
+						Name:     "zone2",
+						LabelKey: "custom-zone-2.io/area",
+					},
+				},
+			},
+			expectedLabel: "",
+			expectedErr:   fmt.Errorf("array system-2 zone key custom-zone-2.io/area does not match custom-zone-1.io/area"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			label, err := getZoneKeyLabelFromSecret(tt.arrays)
+			if tt.expectedErr == nil {
+				assert.Nil(t, err)
+			} else {
+				assert.NotNil(t, err)
+			}
+			assert.Equal(t, label, tt.expectedLabel)
+		})
+	}
+}
+
 func TestFindNetworkInterfaceIPs(t *testing.T) {
 	tests := []struct {
 		name            string

service/step_defs_test.go

@@ -4251,6 +4251,10 @@ func (f *feature) iUseConfig(filename string) error {
 		}
 	}
 
+	f.service.opts.zoneLabelKey, err = getZoneKeyLabelFromSecret(f.service.opts.arrays)
+	if err != nil {
+		return fmt.Errorf("get zone key label from secret: %s", err.Error())
+	}
 	fmt.Printf("****************************************************** s.opts.arrays %v\n", f.service.opts.arrays)
 	f.service.systemProbeAll(context.Background())
 	f.adminClient = f.service.adminClients[arrayID]
@@ -4678,7 +4682,7 @@ func (f *feature) iCallPingNASServer(systemID string, name string) error {
 	return nil
 }
 
-func getZoneEnabledRequest() *csi.CreateVolumeRequest {
+func getZoneEnabledRequest(zoneLabelName string) *csi.CreateVolumeRequest {
 	req := new(csi.CreateVolumeRequest)
 	params := make(map[string]string)
 	req.Parameters = params
@@ -4689,12 +4693,12 @@ func getZoneEnabledRequest() *csi.CreateVolumeRequest {
 	topologies := []*csi.Topology{
 		{
 			Segments: map[string]string{
-				"zone.csi-vxflexos.dellemc.com": "zoneA",
+				zoneLabelName: "zoneA",
 			},
 		},
 		{
 			Segments: map[string]string{
-				"zone.csi-vxflexos.dellemc.com": "zoneB",
+				zoneLabelName: "zoneB",
 			},
 		},
 	}
@@ -4705,7 +4709,7 @@ func getZoneEnabledRequest() *csi.CreateVolumeRequest {
 func (f *feature) iCallCreateVolumeWithZones(name string) error {
 	ctx := new(context.Context)
 	if f.createVolumeRequest == nil {
-		req := getZoneEnabledRequest()
+		req := getZoneEnabledRequest(f.service.opts.zoneLabelKey)
 		f.createVolumeRequest = req
 	}
 	req := f.createVolumeRequest
@@ -4724,8 +4728,8 @@ func (f *feature) iCallCreateVolumeWithZones(name string) error {
 	return nil
 }
 
-func mockGetNodeLabelsWithZone(_ context.Context, _ *service) (map[string]string, error) {
-	labels := map[string]string{"zone." + Name: "zoneA"}
+func mockGetNodeLabelsWithZone(_ context.Context, s *service) (map[string]string, error) {
+	labels := map[string]string{s.opts.zoneLabelKey: "zoneA"}
 	return labels, nil
 }
 
@@ -4741,7 +4745,7 @@ func (f *feature) iCallNodeGetInfoWithZoneLabels() error {
 
 func (f *feature) aValidNodeGetInfoIsReturnedWithNodeTopology() error {
 	accessibility := f.nodeGetInfoResponse.GetAccessibleTopology()
-	if _, ok := accessibility.Segments["zone.csi-vxflexos.dellemc.com"]; !ok {
+	if _, ok := accessibility.Segments[f.service.opts.zoneLabelKey]; !ok {
 		return fmt.Errorf("zone not found")
 	}
 
@@ -4751,7 +4755,7 @@ func (f *feature) aValidNodeGetInfoIsReturnedWithNodeTopology() error {
 func (f *feature) aNodeGetInfoIsReturnedWithoutZoneTopology() error {
 	accessibility := f.nodeGetInfoResponse.GetAccessibleTopology()
 	Log.Printf("Node Accessibility %+v", accessibility)
-	if _, ok := accessibility.Segments["zone."+Name]; ok {
+	if _, ok := accessibility.Segments[f.service.opts.zoneLabelKey]; ok {
 		return fmt.Errorf("zone found")
 	}
 	return nil