refactoring

service/controller.go

@@ -2575,17 +2575,15 @@ func (s *service) ControllerGetCapabilities(
 }
 
 func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey string) (zone string, err error) {
-	if zoneLabelKey == "" {
-		return "", nil
-	}
-
+	// get labels for this service, s
 	labels, err := GetNodeLabels(ctx, s)
 	if err != nil {
 		return "", err
 	}
 
 	Log.Infof("Listing labels: %v", labels)
 
+	// get the zone name from the labels
 	if val, ok := labels[zoneLabelKey]; ok {
 		Log.Infof("probing zoneLabel %s, zone value: %s", zoneLabelKey, val)
 		return val, nil
@@ -2596,23 +2594,27 @@ func (s *service) getZoneFromZoneLabelKey(ctx context.Context, zoneLabelKey stri
 
 // systemProbeAll will iterate through all arrays in service.opts.arrays and probe them. If failed, it logs
 // the failed system name
-func (s *service) systemProbeAll(ctx context.Context, zoneLabelKey string) error {
+func (s *service) systemProbeAll(ctx context.Context) error {
 	// probe all arrays
 	Log.Infoln("Probing all associated arrays")
 	allArrayFail := true
 	errMap := make(map[string]error)
+	zoneName := ""
+	var err error
 
-	zoneName, err := s.getZoneFromZoneLabelKey(ctx, zoneLabelKey)
-	if err != nil {
-		return err
+	if s.opts.zoneLabelKey != "" && s.isNodeMode() {
+		zoneName, err = s.getZoneFromZoneLabelKey(ctx, s.opts.zoneLabelKey)
+		if err != nil {
+			return err
+		}
 	}
 
 	for _, array := range s.opts.arrays {
 		// If zone information is available, use it to probe the array
-		if strings.EqualFold(s.mode, "node") && array.AvailabilityZone != nil && array.AvailabilityZone.Name != ZoneName(zoneName) {
+		if s.isNodeMode() && !array.isInZone(zoneName) {
 			// Driver node containers should not probe arrays that exist outside their assigned zone
 			// Driver controller container should probe all arrays
-			Log.Warnf("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zoneName)
+			Log.Infof("array %s zone %s does not match %s, not pinging this array\n", array.SystemID, array.AvailabilityZone.Name, zoneName)
 			continue
 		}
 

service/identity.go

@@ -81,7 +81,7 @@ func (s *service) Probe(
 
 	if !strings.EqualFold(s.mode, "node") {
 		Log.Debug("systemProbe")
-		if err := s.systemProbeAll(ctx, ""); err != nil {
+		if err := s.systemProbeAll(ctx); err != nil {
 			Log.Printf("error in systemProbeAll: %s", err.Error())
 			return nil, err
 		}
@@ -107,7 +107,7 @@ func (s *service) ProbeController(ctx context.Context,
 ) {
 	if !strings.EqualFold(s.mode, "node") {
 		Log.Debug("systemProbe")
-		if err := s.systemProbeAll(ctx, ""); err != nil {
+		if err := s.systemProbeAll(ctx); err != nil {
 			Log.Printf("error in systemProbeAll: %s", err.Error())
 			return nil, err
 		}

service/service.go

@@ -666,7 +666,7 @@ func (s *service) getIPAddressByInterface(interfaceName string, networkInterface
 }
 
 func (s *service) checkNFS(ctx context.Context, systemID string) (bool, error) {
-	err := s.systemProbeAll(ctx, s.opts.zoneLabelKey)
+	err := s.systemProbeAll(ctx)
 	if err != nil {
 		return false, err
 	}
@@ -714,18 +714,18 @@ func (s *service) doProbe(ctx context.Context) error {
 	px.Lock()
 	defer px.Unlock()
 
-	if !strings.EqualFold(s.mode, "node") {
+	if !s.isNodeMode() {
 		Log.Info("[doProbe] controllerProbe")
-		if err := s.systemProbeAll(ctx, ""); err != nil {
+		if err := s.systemProbeAll(ctx); err != nil {
 			return err
 		}
 	}
 
 	// Do a node probe
-	if !strings.EqualFold(s.mode, "controller") {
+	if !s.isControllerMode() {
 		// Probe all systems managed by driver
 		Log.Info("[doProbe] nodeProbe")
-		if err := s.systemProbeAll(ctx, s.opts.zoneLabelKey); err != nil {
+		if err := s.systemProbeAll(ctx); err != nil {
 			return err
 		}
 
@@ -1866,7 +1866,10 @@ func (s *service) SetPodZoneLabel(ctx context.Context, zoneLabel map[string]stri
 	podName := ""
 	for _, pod := range pods.Items {
 		if pod.Spec.NodeName == s.opts.KubeNodeName && pod.Labels["app"] != "" {
-			podName = pod.Name
+			// only add labels to node pods. Controller pod is not restricted to a zone
+			if strings.Contains(pod.Name, "node") {
+				podName = pod.Name
+			}
 		}
 	}
 
@@ -1944,3 +1947,18 @@ func getZoneKeyLabelFromSecret(arrays map[string]*ArrayConnectionData) (string,
 
 	return zoneKeyLabel, nil
 }
+
+// isControllerMode returns true if the mode property of service s is set to "node", false otherwise.
+func (s *service) isNodeMode() bool {
+	return strings.EqualFold(s.mode, "node")
+}
+
+// isControllerMode returns true if the mode property of service s is set to "controller", false otherwise.
+func (s *service) isControllerMode() bool {
+	return strings.EqualFold(s.mode, "controller")
+}
+
+// isInZone returns true if the array is configured for use in the provided zoneName, false otherwise.
+func (array *ArrayConnectionData) isInZone(zoneName string) bool {
+	return array.AvailabilityZone != nil && array.AvailabilityZone.Name == ZoneName(zoneName)
+}

service/service_test.go

@@ -88,7 +88,7 @@ func Test_service_SetPodZoneLabel(t *testing.T) {
 
 	const validZoneName = "zoneA"
 	const validZoneLabelKey = "topology.kubernetes.io/zone"
-	const validAppName = "test-app"
+	const validAppName = "test-node-pod"
 	const validAppLabelKey = "app"
 	const validNodeName = "kube-node-name"
 	validAppLabels := map[string]string{validAppLabelKey: validAppName}
@@ -206,3 +206,77 @@ func Test_service_SetPodZoneLabel(t *testing.T) {
 		})
 	}
 }
+
+func TestArrayConnectionData_isInZone(t *testing.T) {
+	type fields struct {
+		SystemID                  string
+		Username                  string
+		Password                  string
+		Endpoint                  string
+		SkipCertificateValidation bool
+		Insecure                  bool
+		IsDefault                 bool
+		AllSystemNames            string
+		NasName                   string
+		AvailabilityZone          *AvailabilityZone
+	}
+	type args struct {
+		zoneName string
+	}
+	tests := map[string]struct {
+		fields fields
+		args   args
+		want   bool
+	}{
+		"array is in the zone": {
+			want: true,
+			fields: fields{
+				AvailabilityZone: &AvailabilityZone{
+					LabelKey: "topology.kubernetes.io/zone",
+					Name:     "zoneA",
+				},
+			},
+			args: args{
+				zoneName: "zoneA",
+			},
+		},
+		"availability zone is not used": {
+			want:   false,
+			fields: fields{},
+			args: args{
+				zoneName: "zoneA",
+			},
+		},
+		"zone names do not match": {
+			want: false,
+			fields: fields{
+				AvailabilityZone: &AvailabilityZone{
+					LabelKey: "topology.kubernetes.io/zone",
+					Name:     "zoneA",
+				},
+			},
+			args: args{
+				zoneName: "zoneB",
+			},
+		},
+	}
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			array := &ArrayConnectionData{
+				SystemID:                  tt.fields.SystemID,
+				Username:                  tt.fields.Username,
+				Password:                  tt.fields.Password,
+				Endpoint:                  tt.fields.Endpoint,
+				SkipCertificateValidation: tt.fields.SkipCertificateValidation,
+				Insecure:                  tt.fields.Insecure,
+				IsDefault:                 tt.fields.IsDefault,
+				AllSystemNames:            tt.fields.AllSystemNames,
+				NasName:                   tt.fields.NasName,
+				AvailabilityZone:          tt.fields.AvailabilityZone,
+			}
+			if got := array.isInZone(tt.args.zoneName); got != tt.want {
+				t.Errorf("ArrayConnectionData.isInZone() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

service/step_defs_test.go

@@ -4313,7 +4313,7 @@ func (f *feature) iUseConfig(filename string) error {
 		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.service.systemProbeAll(context.Background())
 	f.adminClient = f.service.adminClients[arrayID]
 	if f.adminClient == nil {
 		return fmt.Errorf("adminClient nil")