airship/airshipctl
Code Issues Proposed changes

Merge "[#52] Provide Redfish feedback in RemoteDirect"

Browse Source
This commit is contained in:
Zuul 2020-03-18 05:46:49 +00:00 committed by Gerrit Code Review
commit ae853757bc
3 changed files with 76 additions and 82 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
pkg/remote/redfish/errors.go
View File

@ -1,9 +1,12 @@
package redfish
import (
"encoding/json"
"fmt"
"net/http"
redfishClient "opendev.org/airship/go-redfish/client"
aerror "opendev.org/airship/airshipctl/pkg/errors"
)
@ -26,16 +29,29 @@ func (e ErrRedfishMissingConfig) Error() string {
return "missing configuration: " + e.What
}
// Redfish client return error if response has no body.
// In this case this function further checks the
// HTTP response code.
func ScreenRedfishError(httpResp *http.Response, err error) error {
if err != nil && httpResp != nil {
if httpResp.StatusCode < 200 || httpResp.StatusCode >= 400 {
// ScreenRedfishError provides detailed error checking on a Redfish client response.
func ScreenRedfishError(httpResp *http.Response, clientErr error) error {
// NOTE(drewwalters96): clientErr may not be nil even though the request was successful. The HTTP status code
// has to be verified for success on each request. The Redfish client uses HTTP codes 200 and 204 to indicate
// success.
if httpResp != nil && httpResp.StatusCode != 200 && httpResp.StatusCode != 204 {
if clientErr == nil {
return ErrRedfishClient{Message: http.StatusText(httpResp.StatusCode)}
}
oAPIErr, ok := clientErr.(redfishClient.GenericOpenAPIError)
if !ok {
return ErrRedfishClient{Message: "Unable to decode client error."}
}
var resp redfishClient.RedfishError
if err := json.Unmarshal(oAPIErr.Body(), &resp); err != nil {
// No JSON response included; use generic error text.
return ErrRedfishClient{Message: err.Error()}
}
} else if err != nil {
return ErrRedfishClient{Message: err.Error()}
} else if httpResp == nil {
return ErrRedfishClient{Message: "HTTP request failed. Please try again."}
}
return nil
}

49
pkg/remote/redfish/redfish_test.go
View File

@ -27,10 +27,11 @@ func TestRedfishRemoteDirectNormal(t *testing.T) {
defer m.AssertExpectations(t)
systemID := computerSystemID
httpResp := &http.Response{StatusCode: 200}
m.On("GetSystem", context.Background(), systemID).
Return(getTestSystem(), nil, nil)
Return(getTestSystem(), httpResp, nil)
m.On("InsertVirtualMedia", context.Background(), "manager-1", "Cd", mock.Anything).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, httpResp, nil)
systemReq := redfishClient.ComputerSystem{
Boot: redfishClient.Boot{
@ -39,18 +40,18 @@ func TestRedfishRemoteDirectNormal(t *testing.T) {
}
m.On("SetSystem", context.Background(), systemID, systemReq).
Times(1).
Return(redfishClient.ComputerSystem{}, nil, nil)
Return(redfishClient.ComputerSystem{}, httpResp, nil)
resetReq := redfishClient.ResetRequestBody{}
resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
m.On("ResetSystem", context.Background(), systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, httpResp, nil)
resetReq.ResetType = redfishClient.RESETTYPE_ON
m.On("ResetSystem", context.Background(), systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, httpResp, nil)
rDCfg := getDefaultRedfishRemoteDirectObj(t, m)
@ -85,9 +86,7 @@ func TestRedfishRemoteDirectGetSystemNetworkError(t *testing.T) {
systemID := computerSystemID
realErr := fmt.Errorf("server request timeout")
httpResp := &http.Response{
StatusCode: 408,
}
httpResp := &http.Response{StatusCode: 408}
m.On("GetSystem", context.Background(), systemID).
Times(1).
Return(redfishClient.ComputerSystem{}, httpResp, realErr)
@ -109,11 +108,8 @@ func TestRedfishRemoteDirectInvalidIsoPath(t *testing.T) {
localRDCfg := rDCfg
localRDCfg.IsoPath = "bogus/path/to.iso"
errStr := "invalid remote boot path"
realErr := fmt.Errorf(errStr)
httpResp := &http.Response{
StatusCode: 500,
}
realErr := redfishClient.GenericOpenAPIError{}
httpResp := &http.Response{StatusCode: 500}
m.On("GetSystem", context.Background(), systemID).
Times(1).
Return(getTestSystem(), nil, nil)
@ -157,19 +153,17 @@ func TestRedfishRemoteDirectSetSystemBootSourceFailed(t *testing.T) {
defer m.AssertExpectations(t)
systemID := computerSystemID
httpSuccResp := &http.Response{StatusCode: 200}
m.On("GetSystem", context.Background(), systemID).
Return(getTestSystem(), nil, nil)
Return(getTestSystem(), httpSuccResp, nil)
m.On("InsertVirtualMedia", context.Background(), "manager-1", "Cd", mock.Anything).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, httpSuccResp, nil)
realErr := fmt.Errorf("unauthorized")
httpResp := &http.Response{
StatusCode: 401,
}
m.On("SetSystem", context.Background(), systemID, mock.Anything).
Times(1).
Return(redfishClient.ComputerSystem{}, httpResp, realErr)
Return(redfishClient.ComputerSystem{}, &http.Response{StatusCode: 401},
redfishClient.GenericOpenAPIError{})
rDCfg := getDefaultRedfishRemoteDirectObj(t, m)
@ -184,26 +178,23 @@ func TestRedfishRemoteDirectSystemRebootFailed(t *testing.T) {
defer m.AssertExpectations(t)
systemID := computerSystemID
httpSuccResp := &http.Response{StatusCode: 200}
m.On("GetSystem", context.Background(), systemID).
Return(getTestSystem(), nil, nil)
Return(getTestSystem(), httpSuccResp, nil)
m.On("InsertVirtualMedia", context.Background(), mock.Anything, mock.Anything, mock.Anything).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, httpSuccResp, nil)
m.On("SetSystem", context.Background(), systemID, mock.Anything).
Times(1).
Return(redfishClient.ComputerSystem{}, nil, nil)
Return(redfishClient.ComputerSystem{}, httpSuccResp, nil)
realErr := fmt.Errorf("unauthorized")
httpResp := &http.Response{
StatusCode: 401,
}
resetReq := redfishClient.ResetRequestBody{}
resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
m.On("ResetSystem", context.Background(), systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, httpResp, realErr)
Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 401},
redfishClient.GenericOpenAPIError{})
rDCfg := getDefaultRedfishRemoteDirectObj(t, m)

77
pkg/remote/redfish/utils_test.go
View File

@ -2,7 +2,6 @@ package redfish
import (
"context"
"fmt"
"net/http"
"testing"
@ -17,47 +16,41 @@ const (
)
func TestRedfishErrorNoError(t *testing.T) {
err := ScreenRedfishError(nil, nil)
err := ScreenRedfishError(&http.Response{StatusCode: 200}, nil)
assert.NoError(t, err)
}
func TestRedfishErrorNonNilErrorWithoutHttpResp(t *testing.T) {
realErr := fmt.Errorf("sample error")
err := ScreenRedfishError(nil, realErr)
assert.Error(t, err)
err := ScreenRedfishError(nil, redfishClient.GenericOpenAPIError{})
_, ok := err.(ErrRedfishClient)
assert.True(t, ok)
}
func TestRedfishErrorNonNilErrorWithHttpRespError(t *testing.T) {
realErr := fmt.Errorf("sample error")
respErr := redfishClient.GenericOpenAPIError{}
httpResp := &http.Response{StatusCode: 408}
err := ScreenRedfishError(httpResp, realErr)
assert.Equal(t, err, ErrRedfishClient{Message: realErr.Error()})
err := ScreenRedfishError(&http.Response{StatusCode: 408}, respErr)
_, ok := err.(ErrRedfishClient)
assert.True(t, ok)
httpResp.StatusCode = 400
err = ScreenRedfishError(httpResp, realErr)
assert.Equal(t, err, ErrRedfishClient{Message: realErr.Error()})
err = ScreenRedfishError(&http.Response{StatusCode: 500}, respErr)
_, ok = err.(ErrRedfishClient)
assert.True(t, ok)
httpResp.StatusCode = 199
err = ScreenRedfishError(httpResp, realErr)
assert.Equal(t, err, ErrRedfishClient{Message: realErr.Error()})
err = ScreenRedfishError(&http.Response{StatusCode: 199}, respErr)
_, ok = err.(ErrRedfishClient)
assert.True(t, ok)
}
func TestRedfishErrorNonNilErrorWithHttpRespOK(t *testing.T) {
realErr := fmt.Errorf("sample error")
respErr := redfishClient.GenericOpenAPIError{}
httpResp := &http.Response{StatusCode: 204}
err := ScreenRedfishError(httpResp, realErr)
// NOTE: Redfish client only uses HTTP 200 & HTTP 204 for success.
err := ScreenRedfishError(&http.Response{StatusCode: 204}, respErr)
assert.NoError(t, err)
httpResp.StatusCode = 200
err = ScreenRedfishError(httpResp, realErr)
assert.NoError(t, err)
httpResp.StatusCode = 399
err = ScreenRedfishError(httpResp, realErr)
err = ScreenRedfishError(&http.Response{StatusCode: 200}, respErr)
assert.NoError(t, err)
}
@ -106,12 +99,12 @@ func TestRedfishUtilRebootSystemOK(t *testing.T) {
resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
m.On("ResetSystem", ctx, systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil)
resetReq.ResetType = redfishClient.RESETTYPE_ON
m.On("ResetSystem", ctx, systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil)
err := RebootSystem(ctx, m, systemID)
assert.NoError(t, err)
@ -122,19 +115,17 @@ func TestRedfishUtilRebootSystemForceOffError2(t *testing.T) {
defer m.AssertExpectations(t)
ctx := context.Background()
realErr := fmt.Errorf("unauthorized")
httpResp := &http.Response{
StatusCode: 401,
}
resetReq := redfishClient.ResetRequestBody{}
resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
m.On("ResetSystem", ctx, systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, httpResp, realErr)
Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 401},
redfishClient.GenericOpenAPIError{})
err := RebootSystem(ctx, m, systemID)
assert.Error(t, err)
_, ok := err.(ErrRedfishClient)
assert.True(t, ok)
}
func TestRedfishUtilRebootSystemForceOffError(t *testing.T) {
@ -142,18 +133,16 @@ func TestRedfishUtilRebootSystemForceOffError(t *testing.T) {
defer m.AssertExpectations(t)
ctx := context.Background()
realErr := fmt.Errorf("unauthorized")
httpResp := &http.Response{
StatusCode: 401,
}
resetReq := redfishClient.ResetRequestBody{}
resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
m.On("ResetSystem", ctx, systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, httpResp, realErr)
Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 401},
redfishClient.GenericOpenAPIError{})
err := RebootSystem(ctx, m, systemID)
assert.Error(t, err)
_, ok := err.(ErrRedfishClient)
assert.True(t, ok)
}
func TestRedfishUtilRebootSystemTurningOnError(t *testing.T) {
@ -165,18 +154,16 @@ func TestRedfishUtilRebootSystemTurningOnError(t *testing.T) {
resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF
m.On("ResetSystem", ctx, systemID, resetReq).
Times(1).
Return(redfishClient.RedfishError{}, nil, nil)
Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil)
realErr := fmt.Errorf("unauthorized")
httpResp := &http.Response{
StatusCode: 401,
}
resetOnReq := redfishClient.ResetRequestBody{}
resetOnReq.ResetType = redfishClient.RESETTYPE_ON
m.On("ResetSystem", ctx, systemID, resetOnReq).
Times(1).
Return(redfishClient.RedfishError{}, httpResp, realErr)
Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 401},
redfishClient.GenericOpenAPIError{})
err := RebootSystem(ctx, m, systemID)
assert.Error(t, err)
_, ok := err.(ErrRedfishClient)
assert.True(t, ok)
}