Refactor Redfish ClientError

This change refactors the Redfish ClientError to match the error pattern
used elsewhere in airshipctl.

Change-Id: Ie309ba9ac41e5b618cf2f4c18f1f381d875e9cdb
Signed-off-by: Drew Walters <andrew.walters@att.com>

pkg/remote/redfish/errors.go

@@ -7,16 +7,17 @@ import (
 	aerror "opendev.org/airship/airshipctl/pkg/errors"
 )
 
-type ClientError struct {
+// ErrRedfishClient describes an error encountered by the go-redfish client.
+type ErrRedfishClient struct {
 	aerror.AirshipError
+	Message string
 }
 
-func NewRedfishClientErrorf(format string, v ...interface{}) error {
+func (e ErrRedfishClient) Error() string {
-	e := &ClientError{}
+	return fmt.Sprintf("redfish client encountered an error: %s", e.Message)
-	e.Message = fmt.Sprintf(format, v...)
-	return e
 }
 
+// ErrRedfishMissingConfig describes an error encountered due to a missing configuration option.
 type ErrRedfishMissingConfig struct {
 	What string
 }
@@ -31,10 +32,10 @@ func (e ErrRedfishMissingConfig) Error() string {
 func ScreenRedfishError(httpResp *http.Response, err error) error {
 	if err != nil && httpResp != nil {
 		if httpResp.StatusCode < 200 || httpResp.StatusCode >= 400 {
-			return NewRedfishClientErrorf("%s", err.Error())
+			return ErrRedfishClient{Message: err.Error()}
 		}
 	} else if err != nil {
-		return NewRedfishClientErrorf("%s", err.Error())
+		return ErrRedfishClient{Message: err.Error()}
 	}
 	return nil
 }

pkg/remote/redfish/redfish.go

@@ -39,7 +39,7 @@ func (cfg RemoteDirect) DoRemoteDirect() error {
 	systemID := cfg.EphemeralNodeID
 	system, _, err := cfg.RedfishAPI.GetSystem(cfg.Context, systemID)
 	if err != nil {
-		return NewRedfishClientErrorf("Get System[%s] failed with err: %s", systemID, err.Error())
+		return ErrRedfishClient{Message: fmt.Sprintf("Get System[%s] failed with err: %v", systemID, err)}
 	}
 	alog.Debugf("Ephemeral Node System ID: '%s'", systemID)
 

pkg/remote/redfish/redfish_test.go

@@ -75,7 +75,7 @@ func TestRedfishRemoteDirectInvalidSystemId(t *testing.T) {
 
 	err := localRDCfg.DoRemoteDirect()
 
-	_, ok := err.(*ClientError)
+	_, ok := err.(ErrRedfishClient)
 	assert.True(t, ok)
 }
 
@@ -96,7 +96,7 @@ func TestRedfishRemoteDirectGetSystemNetworkError(t *testing.T) {
 
 	err := rDCfg.DoRemoteDirect()
 
-	_, ok := err.(*ClientError)
+	_, ok := err.(ErrRedfishClient)
 	assert.True(t, ok)
 }
 
@@ -123,7 +123,7 @@ func TestRedfishRemoteDirectInvalidIsoPath(t *testing.T) {
 
 	err := localRDCfg.DoRemoteDirect()
 
-	_, ok := err.(*ClientError)
+	_, ok := err.(ErrRedfishClient)
 	assert.True(t, ok)
 }
 
@@ -148,7 +148,7 @@ func TestRedfishRemoteDirectCdDvdNotAvailableInBootSources(t *testing.T) {
 
 	err := rDCfg.DoRemoteDirect()
 
-	_, ok := err.(*ClientError)
+	_, ok := err.(ErrRedfishClient)
 	assert.True(t, ok)
 }
 
@@ -175,7 +175,7 @@ func TestRedfishRemoteDirectSetSystemBootSourceFailed(t *testing.T) {
 
 	err := rDCfg.DoRemoteDirect()
 
-	_, ok := err.(*ClientError)
+	_, ok := err.(ErrRedfishClient)
 	assert.True(t, ok)
 }
 
@@ -209,7 +209,7 @@ func TestRedfishRemoteDirectSystemRebootFailed(t *testing.T) {
 
 	err := rDCfg.DoRemoteDirect()
 
-	_, ok := err.(*ClientError)
+	_, ok := err.(ErrRedfishClient)
 	assert.True(t, ok)
 }
 

pkg/remote/redfish/utils.go

@@ -2,6 +2,7 @@ package redfish
 
 import (
 	"context"
+	"fmt"
 	"net/url"
 	"strings"
 	"time"
@@ -63,7 +64,7 @@ func SetSystemBootSourceForMediaType(ctx context.Context,
 	/* Check available boot sources for system */
 	system, _, err := api.GetSystem(ctx, systemID)
 	if err != nil {
-		return NewRedfishClientErrorf("Get System[%s] failed with err: %s", systemID, err.Error())
+		return ErrRedfishClient{Message: fmt.Sprintf("Get System[%s] failed with err: %v", systemID, err)}
 	}
 
 	allowableValues := system.Boot.BootSourceOverrideTargetRedfishAllowableValues
@@ -77,7 +78,7 @@ func SetSystemBootSourceForMediaType(ctx context.Context,
 		}
 	}
 
-	return NewRedfishClientErrorf("failed to set system[%s] boot source", systemID)
+	return ErrRedfishClient{Message: fmt.Sprintf("failed to set system[%s] boot source", systemID)}
 }
 
 // Reboots a system by force shutoff and turning on.

pkg/remote/redfish/utils_test.go

@@ -25,7 +25,7 @@ func TestRedfishErrorNonNilErrorWithoutHttpResp(t *testing.T) {
 	realErr := fmt.Errorf("sample error")
 	err := ScreenRedfishError(nil, realErr)
 	assert.Error(t, err)
-	_, ok := err.(*ClientError)
+	_, ok := err.(ErrRedfishClient)
 	assert.True(t, ok)
 }
 
@@ -34,15 +34,15 @@ func TestRedfishErrorNonNilErrorWithHttpRespError(t *testing.T) {
 
 	httpResp := &http.Response{StatusCode: 408}
 	err := ScreenRedfishError(httpResp, realErr)
-	assert.Equal(t, err, NewRedfishClientErrorf(realErr.Error()))
+	assert.Equal(t, err, ErrRedfishClient{Message: realErr.Error()})
 
 	httpResp.StatusCode = 400
 	err = ScreenRedfishError(httpResp, realErr)
-	assert.Equal(t, err, NewRedfishClientErrorf(realErr.Error()))
+	assert.Equal(t, err, ErrRedfishClient{Message: realErr.Error()})
 
 	httpResp.StatusCode = 199
 	err = ScreenRedfishError(httpResp, realErr)
-	assert.Equal(t, err, NewRedfishClientErrorf(realErr.Error()))
+	assert.Equal(t, err, ErrRedfishClient{Message: realErr.Error()})
 }
 
 func TestRedfishErrorNonNilErrorWithHttpRespOK(t *testing.T) {