From 9eca8c28112ff60172577c07b748bfca834f9b74 Mon Sep 17 00:00:00 2001 From: Don Khan Date: Mon, 9 Dec 2024 14:41:02 -0500 Subject: [PATCH] Address PR comments: * Log errors and return new wrapped error struct. * When the node label is missing or empty configure an empty MDM list. --- service/preinit.go | 23 +++++++++++++---------- service/preinit_test.go | 34 +++++++++++++++++++++++++++------- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/service/preinit.go b/service/preinit.go index 4a578e33..c73d7893 100644 --- 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 index 23a9fb07..c7763251 100644 --- 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",