From 9c1724f6d88285c5a753203b83cf2078b88fce17 Mon Sep 17 00:00:00 2001 From: Vladislav Kuzmin Date: Wed, 6 May 2020 18:42:34 +0400 Subject: [PATCH] Make remote parameters configurable Remove constants and define these parameteres in config file for remote configuration. Closes: #138 Change-Id: Ib9250b8d39c01ef43a1262b62e4d37215481bc43 --- pkg/config/constants.go | 6 + pkg/config/types.go | 4 + pkg/config/utils.go | 8 +- pkg/remote/management.go | 8 +- pkg/remote/redfish/client.go | 50 ++++---- pkg/remote/redfish/client_test.go | 116 ++++++++++-------- pkg/remote/redfish/utils.go | 13 +- pkg/remote/redfish/vendors/dell/client.go | 7 +- .../redfish/vendors/dell/client_test.go | 16 ++- .../defaults/main.yaml | 2 + .../templates/airshipconfig.j2 | 2 + 11 files changed, 136 insertions(+), 96 deletions(-) diff --git a/pkg/config/constants.go b/pkg/config/constants.go index d616778c7..0fa6ca817 100644 --- a/pkg/config/constants.go +++ b/pkg/config/constants.go @@ -55,3 +55,9 @@ const ( AirshipDefaultIsoURL = "http://localhost:8099/debian-custom.iso" AirshipDefaultRemoteType = redfish.ClientType ) + +// Default values for remote operations +const ( + DefaultSystemActionRetries = 30 + DefaultSystemRebootDelay = 2 +) diff --git a/pkg/config/types.go b/pkg/config/types.go index 0f6aab5fd..769f26be8 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -79,4 +79,8 @@ type ManagementConfiguration struct { // UseProxy indicates whether airshipctl should transmit remote management requests through a proxy server when // one is configured in an environment. UseProxy bool `json:"useproxy,omitempty"` + // Number of attempts to reach host during reboot process and ejecting virtual media + SystemActionRetries int `json:"systemActionRetries,omitempty"` + // Number of seconds to wait after reboot if host isn't available + SystemRebootDelay int `json:"systemRebootDelay,omitempty"` } diff --git a/pkg/config/utils.go b/pkg/config/utils.go index 939f05afa..32d252f0c 100644 --- a/pkg/config/utils.go +++ b/pkg/config/utils.go @@ -56,9 +56,11 @@ func NewConfig() *Config { }, ManagementConfiguration: map[string]*ManagementConfiguration{ AirshipDefaultContext: { - Type: redfish.ClientType, - Insecure: true, - UseProxy: false, + Type: redfish.ClientType, + Insecure: true, + UseProxy: false, + SystemActionRetries: DefaultSystemActionRetries, + SystemRebootDelay: DefaultSystemRebootDelay, }, }, Manifests: map[string]*Manifest{ diff --git a/pkg/remote/management.go b/pkg/remote/management.go index 2c84521d0..970469139 100644 --- a/pkg/remote/management.go +++ b/pkg/remote/management.go @@ -178,7 +178,9 @@ func newBaremetalHost(mgmtCfg config.ManagementConfiguration, mgmtCfg.Insecure, mgmtCfg.UseProxy, username, - password) + password, + mgmtCfg.SystemActionRetries, + mgmtCfg.SystemRebootDelay) if err != nil { return host, err @@ -192,7 +194,9 @@ func newBaremetalHost(mgmtCfg config.ManagementConfiguration, mgmtCfg.Insecure, mgmtCfg.UseProxy, username, - password) + password, + mgmtCfg.SystemActionRetries, + mgmtCfg.SystemRebootDelay) if err != nil { return host, err diff --git a/pkg/remote/redfish/client.go b/pkg/remote/redfish/client.go index 26d3cfd0f..e286a59f6 100644 --- a/pkg/remote/redfish/client.go +++ b/pkg/remote/redfish/client.go @@ -27,23 +27,18 @@ import ( "opendev.org/airship/airshipctl/pkg/remote/power" ) -// contextKey is used by the redfish package as a unique key type in order to prevent collisions -// with context keys in other packages. -type contextKey string - const ( // ClientType is used by other packages as the identifier of the Redfish client. - ClientType string = "redfish" - systemActionRetries = 30 - systemRebootDelay = 30 * time.Second - ctxKeyNumRetries contextKey = "numRetries" + ClientType string = "redfish" ) // Client holds details about a Redfish out-of-band system required for out-of-band management. type Client struct { - nodeID string - RedfishAPI redfishAPI.RedfishAPI - RedfishCFG *redfishClient.Configuration + nodeID string + RedfishAPI redfishAPI.RedfishAPI + RedfishCFG *redfishClient.Configuration + systemActionRetries int + systemRebootDelay int // Sleep is meant to be mocked out for tests Sleep func(d time.Duration) @@ -54,16 +49,20 @@ func (c *Client) NodeID() string { return c.nodeID } +// SystemActionRetries returns number of attempts to reach host during reboot process and ejecting virtual media +func (c *Client) SystemActionRetries() int { + return c.systemActionRetries +} + +// SystemRebootDelay returns number of seconds to wait after reboot if host isn't available +func (c *Client) SystemRebootDelay() int { + return c.systemRebootDelay +} + // EjectVirtualMedia ejects a virtual media device attached to a host. func (c *Client) EjectVirtualMedia(ctx context.Context) error { waitForEjectMedia := func(managerID string, mediaID string) error { - // Check if number of retries is defined in context - totalRetries, ok := ctx.Value(ctxKeyNumRetries).(int) - if !ok { - totalRetries = systemActionRetries - } - - for retry := 0; retry < totalRetries; retry++ { + for retry := 0; retry < c.systemActionRetries; retry++ { vMediaMgr, httpResp, err := c.RedfishAPI.GetManagerVirtualMedia(ctx, managerID, mediaID) if err = ScreenRedfishError(httpResp, err); err != nil { return err @@ -75,7 +74,7 @@ func (c *Client) EjectVirtualMedia(ctx context.Context) error { } } - return ErrOperationRetriesExceeded{What: fmt.Sprintf("eject media %s", mediaID), Retries: totalRetries} + return ErrOperationRetriesExceeded{What: fmt.Sprintf("eject media %s", mediaID), Retries: c.systemActionRetries} } managerID, err := getManagerID(ctx, c.RedfishAPI, c.nodeID) @@ -268,7 +267,9 @@ func NewClient(redfishURL string, insecure bool, useProxy bool, username string, - password string) (context.Context, *Client, error) { + password string, + systemActionRetries int, + systemRebootDelay int) (context.Context, *Client, error) { var ctx context.Context if username != "" && password != "" { ctx = context.WithValue( @@ -323,9 +324,12 @@ func NewClient(redfishURL string, } c := &Client{ - nodeID: systemID, - RedfishAPI: redfishClient.NewAPIClient(cfg).DefaultApi, - RedfishCFG: cfg, + nodeID: systemID, + RedfishAPI: redfishClient.NewAPIClient(cfg).DefaultApi, + RedfishCFG: cfg, + systemActionRetries: systemActionRetries, + systemRebootDelay: systemRebootDelay, + Sleep: func(d time.Duration) { time.Sleep(d) }, diff --git a/pkg/remote/redfish/client_test.go b/pkg/remote/redfish/client_test.go index eae3c6f55..49f6580a1 100644 --- a/pkg/remote/redfish/client_test.go +++ b/pkg/remote/redfish/client_test.go @@ -32,20 +32,31 @@ import ( ) const ( - nodeID = "System.Embedded.1" - isoPath = "https://localhost:8080/debian.iso" - redfishURL = "redfish+https://localhost:2224/Systems/System.Embedded.1" + nodeID = "System.Embedded.1" + isoPath = "https://localhost:8080/debian.iso" + redfishURL = "redfish+https://localhost:2224/Systems/System.Embedded.1" + systemActionRetries = 1 + systemRebootDelay = 0 ) func TestNewClient(t *testing.T) { - _, _, err := NewClient(redfishURL, false, false, "", "") + _, _, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + assert.NoError(t, err) +} + +func TestNewClientDefaultValues(t *testing.T) { + sysActRetr := 111 + sysRebDel := 999 + _, c, err := NewClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel) + assert.Equal(t, c.systemActionRetries, sysActRetr) + assert.Equal(t, c.systemRebootDelay, sysRebDel) assert.NoError(t, err) } func TestNewClientMissingSystemID(t *testing.T) { badURL := "redfish+https://localhost:2224" - _, _, err := NewClient(badURL, false, false, "", "") + _, _, err := NewClient(badURL, false, false, "", "", systemActionRetries, systemRebootDelay) _, ok := err.(ErrRedfishMissingConfig) assert.True(t, ok) } @@ -53,12 +64,12 @@ func TestNewClientMissingSystemID(t *testing.T) { func TestNewClientNoRedfishMarking(t *testing.T) { url := "https://localhost:2224/Systems/System.Embedded.1" - _, _, err := NewClient(url, false, false, "", "") + _, _, err := NewClient(url, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) } func TestNewClientAuth(t *testing.T) { - ctx, _, err := NewClient(redfishURL, false, false, "username", "password") + ctx, _, err := NewClient(redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay) assert.NoError(t, err) cAuth := ctx.Value(redfishClient.ContextBasicAuth) @@ -68,21 +79,19 @@ func TestNewClientAuth(t *testing.T) { func TestNewClientEmptyRedfishURL(t *testing.T) { // Redfish URL cannot be empty when creating a client. - _, _, err := NewClient("", false, false, "", "") + _, _, err := NewClient("", false, false, "", "", systemActionRetries, systemRebootDelay) assert.Error(t, err) } - func TestEjectVirtualMedia(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries+1, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - // Normal retries are 30. Limit them here for test time. - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 2) + ctx := context.Background() // Mark CD and DVD test media as inserted inserted := true @@ -127,16 +136,17 @@ func TestEjectVirtualMedia(t *testing.T) { err = client.EjectVirtualMedia(ctx) assert.NoError(t, err) } + func TestEjectVirtualMediaRetriesExceeded(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() // Mark test media as inserted inserted := true @@ -171,7 +181,7 @@ func TestRebootSystem(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -205,7 +215,7 @@ func TestRebootSystemShutdownError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -231,7 +241,7 @@ func TestRebootSystemStartupError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -268,12 +278,12 @@ func TestRebootSystemTimeout(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -297,7 +307,7 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -319,7 +329,7 @@ func TestSetBootSourceByTypeSetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -346,7 +356,7 @@ func TestSetBootSourceByTypeBootSourceUnavailable(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -378,13 +388,12 @@ func TestSetVirtualMediaEjectExistingMedia(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - // Normal retries are 30. Limit them here for test time. - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() // Mark test media as inserted inserted := true @@ -425,13 +434,12 @@ func TestSetVirtualMediaEjectExistingMediaFailure(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - // Normal retries are 30. Limit them here for test time. - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() // Mark test media as inserted inserted := true @@ -463,7 +471,7 @@ func TestSetVirtualMediaGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -485,7 +493,7 @@ func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID @@ -519,12 +527,12 @@ func TestSystemPowerOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -552,12 +560,12 @@ func TestSystemPowerOffResetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -577,12 +585,12 @@ func TestSystemPowerOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -610,12 +618,12 @@ func TestSystemPowerOnResetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -635,7 +643,7 @@ func TestSystemPowerStatusUnknown(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -658,7 +666,7 @@ func TestSystemPowerStatusOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -680,7 +688,7 @@ func TestSystemPowerStatusOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -702,7 +710,7 @@ func TestSystemPowerStatusPoweringOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -724,7 +732,7 @@ func TestSystemPowerStatusPoweringOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -746,7 +754,7 @@ func TestSystemPowerStatusGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID @@ -766,10 +774,10 @@ func TestWaitForPowerStateGetSystemFailed(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -790,10 +798,10 @@ func TestWaitForPowerStateNoRetries(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -816,10 +824,10 @@ func TestWaitForPowerStateWithRetries(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -847,10 +855,10 @@ func TestWaitForPowerStateRetriesExceeded(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -878,10 +886,10 @@ func TestWaitForPowerStateDifferentPowerState(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "") + _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.WithValue(context.Background(), ctxKeyNumRetries, 1) + ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_ON diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index 0cea15eef..bbb7ba689 100644 --- a/pkg/remote/redfish/utils.go +++ b/pkg/remote/redfish/utils.go @@ -19,6 +19,7 @@ import ( "net/http" "net/url" "strings" + "time" redfishAPI "opendev.org/airship/go-redfish/api" redfishClient "opendev.org/airship/go-redfish/client" @@ -255,13 +256,7 @@ func getBasePath(redfishURL string) (string, error) { func (c Client) waitForPowerState(ctx context.Context, desiredState redfishClient.PowerState) error { log.Debugf("Waiting for node '%s' to reach power state '%s'.", c.nodeID, desiredState) - // Check if number of retries is defined in context - totalRetries, ok := ctx.Value(ctxKeyNumRetries).(int) - if !ok { - totalRetries = systemActionRetries - } - - for retry := 0; retry <= totalRetries; retry++ { + for retry := 0; retry <= c.systemActionRetries; retry++ { system, httpResp, err := c.RedfishAPI.GetSystem(ctx, c.NodeID()) if err = ScreenRedfishError(httpResp, err); err != nil { return err @@ -272,11 +267,11 @@ func (c Client) waitForPowerState(ctx context.Context, desiredState redfishClien return nil } - c.Sleep(systemRebootDelay) + c.Sleep(time.Duration(c.systemRebootDelay)) } return ErrOperationRetriesExceeded{ What: fmt.Sprintf("reach desired power state %s", desiredState), - Retries: totalRetries, + Retries: c.systemActionRetries, } } diff --git a/pkg/remote/redfish/vendors/dell/client.go b/pkg/remote/redfish/vendors/dell/client.go index b4672ef50..43e05a90c 100644 --- a/pkg/remote/redfish/vendors/dell/client.go +++ b/pkg/remote/redfish/vendors/dell/client.go @@ -128,8 +128,11 @@ func NewClient(redfishURL string, insecure bool, useProxy bool, username string, - password string) (context.Context, *Client, error) { - ctx, genericClient, err := redfish.NewClient(redfishURL, insecure, useProxy, username, password) + password string, + systemActionRetries int, + systemRebootDelay int) (context.Context, *Client, error) { + ctx, genericClient, err := redfish.NewClient(redfishURL, insecure, useProxy, username, password, + systemActionRetries, systemRebootDelay) if err != nil { return ctx, nil, err } diff --git a/pkg/remote/redfish/vendors/dell/client_test.go b/pkg/remote/redfish/vendors/dell/client_test.go index 2bbd3bb64..159412734 100644 --- a/pkg/remote/redfish/vendors/dell/client_test.go +++ b/pkg/remote/redfish/vendors/dell/client_test.go @@ -25,19 +25,29 @@ import ( ) const ( - redfishURL = "redfish+https://localhost/Systems/System.Embedded.1" + redfishURL = "redfish+https://localhost/Systems/System.Embedded.1" + systemActionRetries = 0 + systemRebootDelay = 0 ) func TestNewClient(t *testing.T) { - _, _, err := NewClient(redfishURL, false, false, "username", "password") + _, _, err := NewClient(redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay) assert.NoError(t, err) } +func TestNewClientDefaultValues(t *testing.T) { + sysActRetr := 222 + sysRebDel := 555 + _, c, err := NewClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel) + assert.Equal(t, c.SystemActionRetries(), sysActRetr) + assert.Equal(t, c.SystemRebootDelay(), sysRebDel) + assert.NoError(t, err) +} func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "") + ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) // Mock redfish get system request diff --git a/roles/airshipctl-test-configs/defaults/main.yaml b/roles/airshipctl-test-configs/defaults/main.yaml index 23767bdb2..38feb7c77 100644 --- a/roles/airshipctl-test-configs/defaults/main.yaml +++ b/roles/airshipctl-test-configs/defaults/main.yaml @@ -24,6 +24,8 @@ airship_site_name: manifests/site/test-site remote_type: redfish remote_insecure: true remote_proxy: false +system_action_retries: 30 +system_reboot_delay: 2 airship_config_ca_data: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUN5RENDQWJDZ0F3SUJBZ0lCQURBTkJna3Foa2lHOXcwQkFRc0ZBREFWTVJNd0VRWURWUVFERXdwcmRXSmwKY201bGRHVnpNQjRYRFRFNU1USXlOakE0TWpneU5Gb1hEVEk1TVRJeU16QTRNamd5TkZvd0ZURVRNQkVHQTFVRQpBeE1LYTNWaVpYSnVaWFJsY3pDQ0FTSXdEUVlKS29aSWh2Y05BUUVCQlFBRGdnRVBBRENDQVFvQ2dnRUJBTTFSClM0d3lnajNpU0JBZjlCR0JUS1p5VTFwYmdDaGQ2WTdJektaZWRoakM2K3k1ZEJpWm81ZUx6Z2tEc2gzOC9YQ1MKenFPS2V5cE5RcDN5QVlLdmJKSHg3ODZxSFZZNjg1ZDVYVDNaOHNyVVRzVDR5WmNzZHAzV3lHdDM0eXYzNi9BSQoxK1NlUFErdU5JemN6bzNEdWhXR0ZoQjk3VjZwRitFUTBlVWN5bk05c2hkL3AwWVFzWDR1ZlhxaENENVpzZnZUCnBka3UvTWkyWnVGUldUUUtNeGpqczV3Z2RBWnBsNnN0L2ZkbmZwd1Q5cC9WTjRuaXJnMEsxOURTSFFJTHVrU2MKb013bXNBeDJrZmxITWhPazg5S3FpMEloL2cyczRFYTRvWURZemt0Y2JRZ24wd0lqZ2dmdnVzM3pRbEczN2lwYQo4cVRzS2VmVGdkUjhnZkJDNUZNQ0F3RUFBYU1qTUNFd0RnWURWUjBQQVFIL0JBUURBZ0trTUE4R0ExVWRFd0VCCi93UUZNQU1CQWY4d0RRWUpLb1pJaHZjTkFRRUxCUUFEZ2dFQkFJek9BL00xWmRGUElzd2VoWjFuemJ0VFNURG4KRHMyVnhSV0VnclFFYzNSYmV3a1NkbTlBS3MwVGR0ZHdEbnBEL2tRYkNyS2xEeFF3RWg3NFZNSFZYYkFadDdsVwpCSm90T21xdXgxYThKYklDRTljR0FHRzFvS0g5R29jWERZY0JzOTA3ckxIdStpVzFnL0xVdG5hN1dSampqZnBLCnFGelFmOGdJUHZIM09BZ3B1RVVncUx5QU8ya0VnelZwTjZwQVJxSnZVRks2TUQ0YzFmMnlxWGxwNXhrN2dFSnIKUzQ4WmF6d0RmWUVmV3Jrdld1YWdvZ1M2SktvbjVEZ0Z1ZHhINXM2Snl6R3lPVnZ0eG1TY2FvOHNxaCs3UXkybgoyLzFVcU5ZK0hlN0x4d04rYkhwYkIxNUtIMTU5ZHNuS3BRbjRORG1jSTZrVnJ3MDVJMUg5ZGRBbGF0bz0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQo= airship_config_client_cert_data: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUQwRENDQXJnQ0ZFdFBveEZYSjVrVFNWTXQ0OVlqcHBQL3hCYnlNQTBHQ1NxR1NJYjNEUUVCQ3dVQU1CVXgKRXpBUkJnTlZCQU1UQ210MVltVnlibVYwWlhNd0hoY05NakF3TVRJME1Ua3hOVEV3V2hjTk1qa3hNakF5TVRreApOVEV3V2pBME1Sa3dGd1lEVlFRRERCQnJkV0psY201bGRHVnpMV0ZrYldsdU1SY3dGUVlEVlFRS0RBNXplWE4wClpXMDZiV0Z6ZEdWeWN6Q0NBaUl3RFFZSktvWklodmNOQVFFQkJRQURnZ0lQQURDQ0Fnb0NnZ0lCQU1iaFhUUmsKVjZiZXdsUjBhZlpBdTBGYWVsOXRtRThaSFEvaGtaSHhuTjc2bDZUUFltcGJvaDRvRjNGMFFqbzROS1o5NVRuWgo0OWNoV240eFJiZVlPU25EcDBpV0Qzd0pXUlZ5aVFvVUFyYTlNcHVPNkVFU1FpbFVGNXNxc0VXUVdVMjBETStBCkdxK1k0Z2c3eDJ1Q0hTdk1GUmkrNEw5RWlXR2xnRDIvb1hXUm5NWEswNExQajZPb3Vkb2Zid2RmT3J6dTBPVkUKUzR0eGtuS1BCY1BUU3YxMWVaWVhja0JEVjNPbExENEZ3dTB3NTcwcnczNzAraEpYdlZxd3Zjb2RjZjZEL1BXWQowamlnd2ppeUJuZ2dXYW04UVFjd1Nud3o0d05sV3hKOVMyWUJFb1ptdWxVUlFaWVk5ZXRBcEpBdFMzTjlUNlQ2ClovSlJRdEdhZDJmTldTYkxEck5qdU1OTGhBYWRMQnhJUHpBNXZWWk5aalJkdEMwU25pMlFUMTVpSFp4d1RxcjQKakRQQ0pYRXU3KytxcWpQVldUaUZLK3JqcVNhS1pqVWZVaUpHQkJWcm5RZkJENHNtRnNkTjB5cm9tYTZOYzRMNQpKS21RV1NHdmd1aG0zbW5sYjFRaVRZanVyZFJQRFNmdmwrQ0NHbnA1QkkvZ1pwMkF1SHMvNUpKVTJlc1ZvL0xsCkVPdHdSOXdXd3dXcTAvZjhXS3R4bVRrMTUyOUp2dFBGQXQweW1CVjhQbHZlYnVwYmJqeW5pL2xWbTJOYmV6dWUKeCtlMEpNbGtWWnFmYkRSS243SjZZSnJHWW1CUFV0QldoSVkzb1pJVTFEUXI4SUlIbkdmYlZoWlR5ME1IMkFCQQp1dlVQcUtSVk80UGkxRTF4OEE2eWVPeVRDcnB4L0pBazVyR2RBZ01CQUFFd0RRWUpLb1pJaHZjTkFRRUxCUUFECmdnRUJBSWNFM1BxZHZDTVBIMnJzMXJESk9ESHY3QWk4S01PVXZPRi90RjlqR2EvSFBJbkh3RlVFNEltbldQeDYKVUdBMlE1bjFsRDFGQlU0T0M4eElZc3VvS1VQVHk1T0t6SVNMNEZnL0lEcG54STlrTXlmNStMR043aG8rblJmawpCZkpJblVYb0tERW1neHZzSWFGd1h6bGtSTDJzL1lKYUZRRzE1Uis1YzFyckJmd2dJOFA5Tkd6aEM1cXhnSmovCm04K3hPMGhXUmJIYklrQ21NekRib2pCSWhaL00rb3VYR1doei9TakpodXhZTVBnek5MZkFGcy9PMTVaSjd3YXcKZ3ZoSGc3L2E5UzRvUCtEYytPa3VrMkV1MUZjL0E5WHpWMzc5aWhNWW5ub3RQMldWeFZ3b0ZZQUg0NUdQcDZsUApCQmwyNnkxc2JMbjl6aGZYUUJIMVpFN0EwZVE9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K airship_config_client_key_data: LS0tLS1CRUdJTiBSU0EgUFJJVkFURSBLRVktLS0tLQpNSUlKS1FJQkFBS0NBZ0VBeHVGZE5HUlhwdDdDVkhScDlrQzdRVnA2WDIyWVR4a2REK0dSa2ZHYzN2cVhwTTlpCmFsdWlIaWdYY1hSQ09qZzBwbjNsT2RuajF5RmFmakZGdDVnNUtjT25TSllQZkFsWkZYS0pDaFFDdHIweW00N28KUVJKQ0tWUVhteXF3UlpCWlRiUU16NEFhcjVqaUNEdkhhNElkSzh3VkdMN2d2MFNKWWFXQVBiK2hkWkdjeGNyVApncytQbzZpNTJoOXZCMTg2dk83UTVVUkxpM0dTY284Rnc5TksvWFY1bGhkeVFFTlhjNlVzUGdYQzdURG52U3ZECmZ2VDZFbGU5V3JDOXloMXgvb1A4OVpqU09LRENPTElHZUNCWnFieEJCekJLZkRQakEyVmJFbjFMWmdFU2htYTYKVlJGQmxoajE2MENra0MxTGMzMVBwUHBuOGxGQzBacDNaODFaSnNzT3MyTzR3MHVFQnAwc0hFZy9NRG05VmsxbQpORjIwTFJLZUxaQlBYbUlkbkhCT3F2aU1NOElsY1M3djc2cXFNOVZaT0lVcjZ1T3BKb3BtTlI5U0lrWUVGV3VkCkI4RVBpeVlXeDAzVEt1aVpybzF6Z3Zra3FaQlpJYStDNkdiZWFlVnZWQ0pOaU82dDFFOE5KKytYNElJYWVua0UKaitCbW5ZQzRlei9ra2xUWjZ4V2o4dVVRNjNCSDNCYkRCYXJUOS94WXEzR1pPVFhuYjBtKzA4VUMzVEtZRlh3KwpXOTV1Nmx0dVBLZUwrVldiWTF0N081N0g1N1FreVdSVm1wOXNORXFmc25wZ21zWmlZRTlTMEZhRWhqZWhraFRVCk5DdndnZ2VjWjl0V0ZsUExRd2ZZQUVDNjlRK29wRlU3ZytMVVRYSHdEcko0N0pNS3VuSDhrQ1Rtc1owQ0F3RUEKQVFLQ0FnQUJ2U1N3ZVpRZW5HSDhsUXY4SURMQzdvU1ZZd0xxNWlCUDdEdjJsN00wYStKNWlXcWwzV2s4ZEVOSQpOYWtDazAwNmkyMCtwVDROdW5mdEZJYzBoTHN6TjBlMkpjRzY1dVlGZnZ2ZHY3RUtZZnNZU3hhU3d4TWJBMlkxCmNCa2NjcGVsUzBhMVpieFYvck16T1RxVUlRNGFQTzJPU3RUeU55b3dWVjhhcXh0QlNPV2pBUlA2VjlBOHNSUDIKNlVGeVFnM2thdjRla3d0S0M5TW85MEVvcGlkSXNnYy9IYk5kQm5tMFJDUnY0bU1DNmVPTXp0NGx0UVNldG0rcwpaRkUwZkM5cjkwRjE4RUVlUjZHTEYxdGhIMzlKTWFFcjYrc3F6TlZXU1VPVGxNN2M5SE55QTJIcnJudnhVUVNOCmF3SkZWSEFOY1hJSjBqcW9icmR6MTdMbGtIRVFGczNLdjRlcDR3REJKMlF0eisxdUFvY1JoV3ZSaWJxWEQ3THgKVmpPdGRyT1h3ZFQxY2ZrKzZRc1RMWUFKR3ptdDdsY1M2QjNnYzJHWmNJWGwyNVlqTUQ1ZVhpa1dEc3hYWmt1UAorb3MzVGhxeGZIS25ITmxtYk9SSVpDMW92Q1NkSTRWZVpzalk0MUs5K0dNaXdXSk1kektpRkp3NlR2blRSUldTCkxod2EzUTlBVmMvTEg0SC9PbU9qWDc0QTNZSWwrRDFVUHd3VzAvMmw4S3BNM0VWZ21XalJMV1ZIRnBNTGJNSlcKZVZKd3dKUmF3bWZLdHZ6bU9KRHlhTXJJblhqTDMvSE1EaWtwU3JhRzFyTnc1SUozOXJZdEFIUUQ1L1VuZlRkSApLNXVjakVucTdPdDMyR1ozcHJvRTU1ZGFBY0hQbktuOGpYZ1ZKTUQyOWh5cEZvL2ZRUUtDQVFFQStBbjRoSDFFCm9GK3FlcWlvYXR3N2cwaVdQUDNCeklxOEZWbWtsRlZBYVF5U28wU2QxWFBybmErR0RFQVd0cHlsVjF5ZkZkR2oKSHc4YXU5NnpUZnRuNWZCRkQxWG1NTkNZeTcrM293V3ArK1NwYUMvMTYzN1dvb3lLRjBjVFNvcWEzZEVuRUtSSwp4TGF2a0lFUTI3OXRBNFVUK0dVK3pTb0NPUFBNNE1JS3poR0FDczZ1anRySzFNcXpwK0JhYldzRlBuN2J1bStVCkRHSFIrNCtab2tBL1Q2N2luYlRxZUwwVzJCNjRMckFURHpZL3Y4NlRGbW1aallEaHRKR1JIWVZUOU9XSXR0RVkKNnZtUDN0a1dOTWt0R2w4bTFiQ0FHQ1JlcGtycUhxWXNMWG5GQ2ZZSFFtOXNpaGgvM3JFVjZ1MUYxZCt0U3JFMgprU1ZVOHhVWDUwbHFNUUtDQVFFQXpVTjZaS0lRNldkT09FR3ZyMExRL1hVczI0bUczN3lGMjhJUDJEcWFBWWVzCnJza2xTdjdlSU9TZWV3MW1CRHVCRkl2bkZvcTVsRlA3cXhWcEIyWjNNSGlDMVNaclZSZjlQTjdCNGFzcmNyMCsKdDB2S0NXWFFIaTVQQXhucXdYb2E2N0Q1bnkwdnlvV0lVUXAyZEZMdkIwQmp0b3MvajJFaHpJZk5WMm1UOW15bgpWQXZOWEdtZnc4SVJCL1diMGkzQ3c0Wityb1l1dTJkRHo2UUwzUFVvN1hLS3ljZzR1UzU1eksvcWZPc09lYm5mCnpsd3ZqbGxNSitmVFFHNzMrQnpINE5IWGs2akZZQzU4eXBrdXd0cmJmYk1pSkZOWThyV1ptL01Nd1VDWlZDQ3kKeUlxQ3FHQVB6b2kyU05zSEtaTlJqN3ZZQ3dQQVd6TzFidjFGcC9hM0xRS0NBUUVBeG0zTGw4cFROVzF6QjgrWApkRzJkV3FpZU1FcmRXRklBcDUvZ1R4NW9lZUdxQ2QxaDJ4cHlldUtwZlhGaitsRVU0Ty9qQU9TRjk5bndqQzFjCkNsMit2Ni9ZdjZ6N2l6L0ZqUEpoNlpRbGFiT0RaeXMvTkZkelEvVGtvRHluRFRJWE5LOFc3blJRc0ZCcDRWT3YKZGUwTlBBeWhiazBvMFo3eXlqY1lSeEpVN0lnSmhCdldmOGcvRGI3ZnZNUjU4eUR6d0F4aW9pS1RNTmlzMFBBUAplMEtrbzQySUU1eGhHNWhDQjBHRUhTMlZBYzFuY0gzRkk5LzFETVAzVEtwTGltOVlQQW5JdG1CTzYrUWNtYTNYCjJ3QzZDV2ZudkhvSDc4aGd3KzRZbjg1V2QwYjhQN3pJRC9qdHZ3aGNlMzMxeDh4cjJ1Nm5ScUxBd1pzNCs0SjcKYmZkSWNRS0NBUUFDL2JlNzNheTNhZnoyenVZN2ZKTEZEcjhQbCtweU9qSU5LTC9JVzlwQXFYUjN1NUNpamlJNApnbnhZdUxKQzM0Y2JBSXJtaGpEOEcxa3dmZ2hneGpwNFoxa290LzJhYU5ZVTIvNGhScmhFWE1PY01pdUloWVpKCjJrem1jNnM3RklkdDVjOU5aWUFyeUZSYk1mYlY3UnQwbEppZllWb1V3Y3FYUzJkUG5jYzlNUW9qTEdUYXN1TlUKRy9EWmw5ZWtjV3hFSXlLWGNuY2QzZnhiK3p6OUJFbUxaRDduZjlacnhHU2IrZmhGeDdzWFJRRWc1YkQvdHdkbwpFWFcvbTU1YmJEZnhhNzFqZG5NaDJxdVEzRGlWT0ZFNGZMTERxcjlDRWlsaDMySFJNeHJJNGcwWTVRUFFaazMwCnFZTldmbktWUllOTHYrWC9DeGZ6ZkVacGpxRkVPRkVsQW9JQkFRQ0t6R2JGdmx6d1BaUmh4czd2VXYxOXlIUXAKQzFmR3gwb0tpRDFSNWZwWVBrT0VRQWVudEFKRHNyYVRsNy9rSDY5V09VbUQ1T3gxbWpyRFB0a1M4WnhXYlJXeApGYjJLK3JxYzRtcGFacGROV09OTkszK3RNZmsrb0FRcWUySU1JV253NUhmbVpjNE1QY0t0bkZQYlJTTkF0aktwCkQ2aG9oL3BXMmdjRFA0cVpNWVZvRW04MVZYZEZDUGhOYitNYnUvU3gyaFB4U0dXYTVGaTczeEtwWWp5M3BISlQKWFoyY2lHN0VNQ3NKZW9HS2FRdmNCY1kvNGlSRGFoV0hWcmlsSVhJQXJQdXdmVUIybzZCZFR0allHeU5sZ2NmeApxWEt4aXBTaEE2VlNienVnR3pkdEdNeEUyekRHVEkxOXFSQy96OUNEREM1ZTJTQUZqbEJUV0QyUHJjcU4KLS0tLS1FTkQgUlNBIFBSSVZBVEUgS0VZLS0tLS0K diff --git a/roles/airshipctl-test-configs/templates/airshipconfig.j2 b/roles/airshipctl-test-configs/templates/airshipconfig.j2 index df5dcdaa5..06b9dffe9 100644 --- a/roles/airshipctl-test-configs/templates/airshipconfig.j2 +++ b/roles/airshipctl-test-configs/templates/airshipconfig.j2 @@ -19,6 +19,8 @@ managementConfiguration: type: {{ remote_type }} insecure: {{ remote_insecure }} useproxy: {{ remote_proxy }} + systemActionRetries: {{ system_action_retries }} + systemRebootDelay: {{ system_reboot_delay }} clusters: dummycluster: