From 2817df848266ba4ce6e9cab6d59c12cc3eeaa596 Mon Sep 17 00:00:00 2001 From: Dmitry Ukov Date: Mon, 16 Mar 2020 14:14:04 +0400 Subject: [PATCH] Ensure node power state ON/OFF for Reset command Change-Id: Icc6b18b4694a17ab2f8e0bf1112e41a1d18a5b00 Relates-To: #54 Depends-On: https://review.opendev.org/713622 --- go.mod | 4 ++-- go.sum | 10 ++++---- pkg/remote/redfish/errors.go | 8 +++++++ pkg/remote/redfish/redfish_test.go | 10 +++++++- pkg/remote/redfish/utils.go | 37 ++++++++++++++++++++++++++---- pkg/remote/redfish/utils_test.go | 31 +++++++++++++++++++++++-- 6 files changed, 86 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index 55a297349..d4c81fba3 100644 --- a/go.mod +++ b/go.mod @@ -36,8 +36,8 @@ require ( k8s.io/cli-runtime v0.0.0-20191114110141-0a35778df828 k8s.io/client-go v11.0.0+incompatible k8s.io/kubectl v0.0.0-20191114113550-6123e1c827f7 - opendev.org/airship/go-redfish v0.0.0-20200110185254-3ab47e28bae8 - opendev.org/airship/go-redfish/client v0.0.0-20200110185254-3ab47e28bae8 + opendev.org/airship/go-redfish v0.0.0-20200318103738-db034d1d753a + opendev.org/airship/go-redfish/client v0.0.0-20200318103738-db034d1d753a sigs.k8s.io/kustomize/v3 v3.2.0 sigs.k8s.io/yaml v1.1.0 ) diff --git a/go.sum b/go.sum index 248f15d4c..d6803ea22 100644 --- a/go.sum +++ b/go.sum @@ -35,6 +35,7 @@ github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7 h1:uSoVVbwJiQipAclBb github.com/alcortesm/tgz v0.0.0-20161220082320-9c5fe88206d7/go.mod h1:6zEj6s6u/ghQa61ZWa/C2Aw3RkjiTBOix7dkqa1VLIs= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 h1:kFOfPq6dUM1hTo4JG6LR5AXSUEsOjtdm0kw0FtQtMJA= github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239/go.mod h1:2FmKhYUyUczH0OGQWaF5ceTx0UBShxjsH6f8oGKYe2c= +github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY= github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs= @@ -308,7 +309,6 @@ golang.org/x/net v0.0.0-20191204025024-5ee1b9f4859a h1:+HHJiFUXVOIS9mr1ThqkQD1N8 golang.org/x/net v0.0.0-20191204025024-5ee1b9f4859a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= -golang.org/x/oauth2 v0.0.0-20190402181905-9f3314589c9a/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45 h1:SVwTIAaPC2U/AvvLNZ2a7OVsmBpC8L5BlwK1whH3hm0= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -431,10 +431,10 @@ modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk= modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k= modernc.org/strutil v1.0.0/go.mod h1:lstksw84oURvj9y3tn8lGvRxyRC1S2+g5uuIzNfIOBs= modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I= -opendev.org/airship/go-redfish v0.0.0-20200110185254-3ab47e28bae8 h1:wIvgHcZH/l7H/eTIjZXGfqurtarlaAv2CtnO/8xYCis= -opendev.org/airship/go-redfish v0.0.0-20200110185254-3ab47e28bae8/go.mod h1:FEjYcb3bYBWGpQIqtvVM0NrT5eyjlCOCj5JNf4lI+6s= -opendev.org/airship/go-redfish/client v0.0.0-20200110185254-3ab47e28bae8 h1:ZDtcVfkn6EzA6Kr5I7C3EuN8qsuByeMPII9TnTx1zXQ= -opendev.org/airship/go-redfish/client v0.0.0-20200110185254-3ab47e28bae8/go.mod h1:LPq6GmtxEl3Pwf0Ora40EOm46Hz425T9kjsOfG5HKmQ= +opendev.org/airship/go-redfish v0.0.0-20200318103738-db034d1d753a h1:4ggAMTwpfu/w3ZXOIJ9tfYF37JIYn+eNCA4O10NduZ0= +opendev.org/airship/go-redfish v0.0.0-20200318103738-db034d1d753a/go.mod h1:FEjYcb3bYBWGpQIqtvVM0NrT5eyjlCOCj5JNf4lI+6s= +opendev.org/airship/go-redfish/client v0.0.0-20200318103738-db034d1d753a h1:S1dmsP5Cc6OQjAd6OgIKMcNPBiGjh5TDbijVjNE/VGU= +opendev.org/airship/go-redfish/client v0.0.0-20200318103738-db034d1d753a/go.mod h1:s0hwuUpBsRXOrhN0NR+fNVivXGyWgHKpqtyq7qYjpew= sigs.k8s.io/kustomize v2.0.3+incompatible h1:JUufWFNlI44MdtnjUqVnvh29rR37PQFzPbLXqhyOyX0= sigs.k8s.io/kustomize v2.0.3+incompatible/go.mod h1:MkjgH3RdOWrievjo6c9T245dYlB5QeXV4WCbnt/PEpU= sigs.k8s.io/kustomize/v3 v3.2.0 h1:EKcEubO29vCbigcMoNynfyZH+ANWkML2UHWibt1Do7o= diff --git a/pkg/remote/redfish/errors.go b/pkg/remote/redfish/errors.go index ae58249e5..f962c05ad 100644 --- a/pkg/remote/redfish/errors.go +++ b/pkg/remote/redfish/errors.go @@ -29,6 +29,14 @@ func (e ErrRedfishMissingConfig) Error() string { return "missing configuration: " + e.What } +// ErrOperationRetriesExceeded raised if number of operation retries exceeded +type ErrOperationRetriesExceeded struct { +} + +func (e ErrOperationRetriesExceeded) Error() string { + return "maximum retries exceeded" +} + // 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 diff --git a/pkg/remote/redfish/redfish_test.go b/pkg/remote/redfish/redfish_test.go index 62ee24e11..f7597f2ce 100644 --- a/pkg/remote/redfish/redfish_test.go +++ b/pkg/remote/redfish/redfish_test.go @@ -28,11 +28,13 @@ func TestRedfishRemoteDirectNormal(t *testing.T) { systemID := computerSystemID httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", context.Background(), systemID). + m.On("GetSystem", context.Background(), systemID).Times(1). Return(getTestSystem(), httpResp, nil) m.On("InsertVirtualMedia", context.Background(), "manager-1", "Cd", mock.Anything). Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("GetSystem", context.Background(), systemID).Times(1). + Return(getTestSystem(), httpResp, nil) systemReq := redfishClient.ComputerSystem{ Boot: redfishClient.Boot{ BootSourceOverrideTarget: redfishClient.BOOTSOURCE_CD, @@ -48,11 +50,17 @@ func TestRedfishRemoteDirectNormal(t *testing.T) { Times(1). Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("GetSystem", context.Background(), systemID).Times(1). + Return(redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_OFF}, httpResp, nil) + resetReq.ResetType = redfishClient.RESETTYPE_ON m.On("ResetSystem", context.Background(), systemID, resetReq). Times(1). Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("GetSystem", context.Background(), systemID).Times(1). + Return(redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_ON}, httpResp, nil) + rDCfg := getDefaultRedfishRemoteDirectObj(t, m) err := rDCfg.DoRemoteDirect() diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index ad554bfc3..696a24e83 100644 --- a/pkg/remote/redfish/utils.go +++ b/pkg/remote/redfish/utils.go @@ -15,6 +15,7 @@ import ( const ( SystemRebootDelay = 2 * time.Second + SystemActionRetries = 30 RedfishURLSchemeSeparator = "+" ) @@ -83,19 +84,47 @@ func SetSystemBootSourceForMediaType(ctx context.Context, // Reboots a system by force shutoff and turning on. func RebootSystem(ctx context.Context, api redfishApi.RedfishAPI, systemID string) error { + waitForPowerState := func(desiredState redfishClient.PowerState) error { + // Check if number of retries is defined in context + totalRetries, ok := ctx.Value("numRetries").(int) + if !ok { + totalRetries = SystemActionRetries + } + + for retry := 0; retry <= totalRetries; retry++ { + system, httpResp, err := api.GetSystem(ctx, systemID) + if err = ScreenRedfishError(httpResp, err); err != nil { + return err + } + if system.PowerState == desiredState { + return nil + } + time.Sleep(SystemRebootDelay) + } + return ErrOperationRetriesExceeded{} + } + resetReq := redfishClient.ResetRequestBody{} + + // Send PowerOff request resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF _, httpResp, err := api.ResetSystem(ctx, systemID, resetReq) if err = ScreenRedfishError(httpResp, err); err != nil { return err } + // Check that node is powered off + if err = waitForPowerState(redfishClient.POWERSTATE_OFF); err != nil { + return err + } - time.Sleep(SystemRebootDelay) - + // Send PowerOn request resetReq.ResetType = redfishClient.RESETTYPE_ON _, httpResp, err = api.ResetSystem(ctx, systemID, resetReq) - - return ScreenRedfishError(httpResp, err) + if err = ScreenRedfishError(httpResp, err); err != nil { + return err + } + // Check that node is powered on and return + return waitForPowerState(redfishClient.POWERSTATE_ON) } // Insert the remote virtual media to the given virtual media id. diff --git a/pkg/remote/redfish/utils_test.go b/pkg/remote/redfish/utils_test.go index e1953e08c..c9268f3d4 100644 --- a/pkg/remote/redfish/utils_test.go +++ b/pkg/remote/redfish/utils_test.go @@ -94,17 +94,24 @@ func TestRedfishUtilRebootSystemOK(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) + httpResp := &http.Response{StatusCode: 200} ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF m.On("ResetSystem", ctx, systemID, resetReq). Times(1). - Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil) + Return(redfishClient.RedfishError{}, httpResp, nil) + + m.On("GetSystem", ctx, systemID).Times(1). + Return(redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_OFF}, httpResp, nil) resetReq.ResetType = redfishClient.RESETTYPE_ON m.On("ResetSystem", ctx, systemID, resetReq). Times(1). - Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil) + Return(redfishClient.RedfishError{}, httpResp, nil) + + m.On("GetSystem", ctx, systemID).Times(1). + Return(redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_ON}, httpResp, nil) err := RebootSystem(ctx, m, systemID) assert.NoError(t, err) @@ -156,6 +163,9 @@ func TestRedfishUtilRebootSystemTurningOnError(t *testing.T) { Times(1). Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil) + m.On("GetSystem", ctx, systemID).Times(1). + Return(redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_OFF}, &http.Response{StatusCode: 200}, nil) + resetOnReq := redfishClient.ResetRequestBody{} resetOnReq.ResetType = redfishClient.RESETTYPE_ON m.On("ResetSystem", ctx, systemID, resetOnReq). @@ -167,3 +177,20 @@ func TestRedfishUtilRebootSystemTurningOnError(t *testing.T) { _, ok := err.(ErrRedfishClient) assert.True(t, ok) } + +func TestRedfishUtilRebootSystemTimeout(t *testing.T) { + m := &redfishMocks.RedfishAPI{} + defer m.AssertExpectations(t) + + ctx := context.WithValue(context.Background(), "numRetries", 1) + resetReq := redfishClient.ResetRequestBody{} + resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF + m.On("ResetSystem", ctx, systemID, resetReq). + Times(1). + Return(redfishClient.RedfishError{}, &http.Response{StatusCode: 200}, nil) + + m.On("GetSystem", ctx, systemID). + Return(redfishClient.ComputerSystem{}, &http.Response{StatusCode: 200}, nil) + err := RebootSystem(ctx, m, systemID) + assert.Equal(t, ErrOperationRetriesExceeded{}, err) +}