Address PR comments:

* Log errors and return new wrapped error struct. * When the node label is missing or empty configure an empty MDM list.
dell · Dec 9, 2024 · 9eca8c2 · 9eca8c2
1 parent 84ce0a2
commit 9eca8c2
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 17 deletions.
diff --git a/service/preinit.go b/service/preinit.go
@@ -75,12 +75,12 @@ func (s *service) PreInit() error {
 
 	arrayConfig, err := arrayConfigurationProviderImpl.GetArrayConfiguration()
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to get array configuration: %v", err)
 	}
 
 	labelKey, err := getLabelKey(arrayConfig)
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to get zone label key: %v", err)
 	}
 
 	var mdmData string
@@ -101,18 +101,21 @@ func (s *service) PreInit() error {
 		Log.Infof("Zone key detected, will configure MDMs for this node, key: %s", labelKey)
 		nodeLabels, err := s.GetNodeLabels(context.Background())
 		if err != nil {
-			return err
+			return fmt.Errorf("unable to get node labels: %v", err)
 		}
-		zone := nodeLabels[labelKey]
 
-		if zone == "" {
-			return fmt.Errorf("No zone found, cannot configure this node")
+		zone, ok := nodeLabels[labelKey]
+
+		if ok && zone == "" {
+			Log.Errorf("node key found but zone is missing, will not configure MDMs for this node, key: %s", labelKey)
 		}
 
-		Log.Infof("Zone found, will configure MDMs for this node, zone: %s", zone)
-		mdmData, err = getMdmList(arrayConfig, labelKey, zone)
-		if err != nil {
-			return err
+		if zone != "" {
+			Log.Infof("zone found, will configure MDMs for this node, zone: %s", zone)
+			mdmData, err = getMdmList(arrayConfig, labelKey, zone)
+			if err != nil {
+				return fmt.Errorf("unable to get MDM list: %v", err)
+			}
 		}
 	}
 

diff --git a/service/preinit_test.go b/service/preinit_test.go
@@ -63,14 +63,14 @@ func TestPreInit(t *testing.T) {
 			name:           "should error on no connection info",
 			connectionInfo: []*ArrayConnectionData{},
 			errorExpected:  true,
-			expectedResult: "array connection data is empty",
+			expectedResult: "unable to get zone label key: array connection data is empty",
 		},
 		{
 			name:            "should handle error getting connection info",
 			connectionInfo:  nil,
 			connectionError: fmt.Errorf("don't care about error text"),
 			errorExpected:   true,
-			expectedResult:  "don't care about error text",
+			expectedResult:  "unable to get array configuration: don't care about error text",
 		},
 		{
 			name: "should error when zone labels different",
@@ -89,7 +89,7 @@ func TestPreInit(t *testing.T) {
 				},
 			},
 			errorExpected:  true,
-			expectedResult: "zone label key is not the same for all arrays",
+			expectedResult: "unable to get zone label key: zone label key is not the same for all arrays",
 		},
 		{
 			name: "should configure all MDMs when no zone label single array",
@@ -126,10 +126,10 @@ func TestPreInit(t *testing.T) {
 				},
 			},
 			errorExpected:  true,
-			expectedResult: "rpc error: code = Internal desc = Unable to fetch the node labels. Error: nodes \"\" not found",
+			expectedResult: "unable to get node labels: rpc error: code = Internal desc = Unable to fetch the node labels. Error: nodes \"\" not found",
 		},
 		{
-			name: "should fail if node label not found for node",
+			name: "should configure empty MDM list if node label not found for node",
 			connectionInfo: []*ArrayConnectionData{
 				{
 					Mdm: "192.168.1.1,192.168.1.2",
@@ -145,8 +145,28 @@ func TestPreInit(t *testing.T) {
 					Labels: map[string]string{"label1": "value1", "label2": "value2"},
 				},
 			},
-			errorExpected:  true,
-			expectedResult: "No zone found, cannot configure this node",
+			errorExpected:  false,
+			expectedResult: "",
+		},
+		{
+			name: "should configure empty MDM list if node label present but value is nil",
+			connectionInfo: []*ArrayConnectionData{
+				{
+					Mdm: "192.168.1.1,192.168.1.2",
+					Zone: ZoneInfo{
+						LabelKey: "key1",
+						Name:     "zone1",
+					},
+				},
+			},
+			nodeInfo: &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:   "",
+					Labels: map[string]string{"key1": "", "label2": "value2"},
+				},
+			},
+			errorExpected:  false,
+			expectedResult: "",
 		},
 		{
 			name: "should configure MDMs for only array matching zone label",