From 0cf1068c5f7c8f1994539b3cf0d3a54be776d263 Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Tue, 17 Mar 2020 15:35:02 +0000 Subject: [PATCH] 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 --- pkg/remote/redfish/errors.go | 15 ++++++++------- pkg/remote/redfish/redfish.go | 2 +- pkg/remote/redfish/redfish_test.go | 12 ++++++------ pkg/remote/redfish/utils.go | 5 +++-- pkg/remote/redfish/utils_test.go | 8 ++++---- 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/pkg/remote/redfish/errors.go b/pkg/remote/redfish/errors.go index bd405959a..a10ff86e7 100644 --- a/pkg/remote/redfish/errors.go +++ b/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 { - e := &ClientError{} - e.Message = fmt.Sprintf(format, v...) - return e +func (e ErrRedfishClient) Error() string { + return fmt.Sprintf("redfish client encountered an error: %s", e.Message) } +// 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 } diff --git a/pkg/remote/redfish/redfish.go b/pkg/remote/redfish/redfish.go index 0d38cc936..b3092953a 100644 --- a/pkg/remote/redfish/redfish.go +++ b/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) diff --git a/pkg/remote/redfish/redfish_test.go b/pkg/remote/redfish/redfish_test.go index 364068765..fa3a6218f 100644 --- a/pkg/remote/redfish/redfish_test.go +++ b/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) } diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index 0993a1738..ad554bfc3 100644 --- a/pkg/remote/redfish/utils.go +++ b/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. diff --git a/pkg/remote/redfish/utils_test.go b/pkg/remote/redfish/utils_test.go index e5efab361..a02a6fb86 100644 --- a/pkg/remote/redfish/utils_test.go +++ b/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) {