Merge "Ensure node power state ON/OFF for Reset command"

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 v0.0.0-20200318103738-db034d1d753a
-	opendev.org/airship/go-redfish/client v0.0.0-20200110185254-3ab47e28bae8
+	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
 )

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-20200318103738-db034d1d753a h1:4ggAMTwpfu/w3ZXOIJ9tfYF37JIYn+eNCA4O10NduZ0=
-opendev.org/airship/go-redfish v0.0.0-20200110185254-3ab47e28bae8/go.mod h1:FEjYcb3bYBWGpQIqtvVM0NrT5eyjlCOCj5JNf4lI+6s=
+opendev.org/airship/go-redfish v0.0.0-20200318103738-db034d1d753a/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-20200318103738-db034d1d753a h1:S1dmsP5Cc6OQjAd6OgIKMcNPBiGjh5TDbijVjNE/VGU=
-opendev.org/airship/go-redfish/client v0.0.0-20200110185254-3ab47e28bae8/go.mod h1:LPq6GmtxEl3Pwf0Ora40EOm46Hz425T9kjsOfG5HKmQ=
+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=

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

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()

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)
-
+	if err = ScreenRedfishError(httpResp, err); err != nil {
-	return ScreenRedfishError(httpResp, err)
+		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.

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)
+}