From 690d8d88c1f0fd647304e3c4b7d5fa6a4a9fe8bc Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Fri, 23 Oct 2020 17:30:06 -0500 Subject: [PATCH] Remove context from BMH object. This is first step in refactoring remote package to support future inventory interface, also relates to remote-direct executor, as it simplifies usage of Host objects Relates-To: #362 Change-Id: Ief2fece134a465916ce461960cd7d16c9e237eb7 Relates-To: #362 --- cmd/baremetal/baremetal.go | 12 +- pkg/remote/management.go | 9 +- pkg/remote/redfish/client.go | 44 +++--- pkg/remote/redfish/client_test.go | 135 +++++++++--------- pkg/remote/redfish/vendors/dell/client.go | 8 +- .../redfish/vendors/dell/client_test.go | 8 +- pkg/remote/remote_direct.go | 12 +- pkg/remote/remote_direct_test.go | 34 +++-- testutil/redfishutils/mock_client.go | 21 +-- 9 files changed, 147 insertions(+), 136 deletions(-) diff --git a/cmd/baremetal/baremetal.go b/cmd/baremetal/baremetal.go index e64c49319..29d085fa0 100644 --- a/cmd/baremetal/baremetal.go +++ b/cmd/baremetal/baremetal.go @@ -15,6 +15,7 @@ package baremetal import ( + "context" "fmt" "io" @@ -104,28 +105,29 @@ func selectAction(m *remote.Manager, cfg *config.Config, action Action, writer i return ephemeralHost.DoRemoteDirect(cfg) } + ctx := context.Background() for _, host := range m.Hosts { switch action { case ejectAction: - if err := host.EjectVirtualMedia(host.Context); err != nil { + if err := host.EjectVirtualMedia(ctx); err != nil { return err } fmt.Fprintf(writer, "All media ejected from host '%s'.\n", host.HostName) case powerOffAction: - if err := host.SystemPowerOff(host.Context); err != nil { + if err := host.SystemPowerOff(ctx); err != nil { return err } fmt.Fprintf(writer, "Powered off host '%s'.\n", host.HostName) case powerOnAction: - if err := host.SystemPowerOn(host.Context); err != nil { + if err := host.SystemPowerOn(ctx); err != nil { return err } fmt.Fprintf(writer, "Powered on host '%s'.\n", host.HostName) case powerStatusAction: - powerStatus, err := host.SystemPowerStatus(host.Context) + powerStatus, err := host.SystemPowerStatus(ctx) if err != nil { return err } @@ -133,7 +135,7 @@ func selectAction(m *remote.Manager, cfg *config.Config, action Action, writer i fmt.Fprintf(writer, "Host '%s' has power status: '%s'\n", host.HostName, powerStatus) case rebootAction: - if err := host.RebootSystem(host.Context); err != nil { + if err := host.RebootSystem(ctx); err != nil { return err } diff --git a/pkg/remote/management.go b/pkg/remote/management.go index 7bf839959..480720d91 100644 --- a/pkg/remote/management.go +++ b/pkg/remote/management.go @@ -56,7 +56,6 @@ type Manager struct { // actions an out-of-band client can perform. Once instantiated, actions can be performed on a baremetal host. type baremetalHost struct { Client - Context context.Context BMCAddress string HostName string username string @@ -218,7 +217,7 @@ func newBaremetalHost(mgmtCfg config.ManagementConfiguration, switch mgmtCfg.Type { case redfish.ClientType: log.Debug("Remote type: Redfish") - ctx, client, err := redfish.NewClient( + client, err := redfish.NewClient( address, mgmtCfg.Insecure, mgmtCfg.UseProxy, @@ -231,10 +230,10 @@ func newBaremetalHost(mgmtCfg config.ManagementConfiguration, return host, err } - host = baremetalHost{client, ctx, address, hostDoc.GetName(), username, password} + host = baremetalHost{client, address, hostDoc.GetName(), username, password} case redfishdell.ClientType: log.Debug("Remote type: Redfish for Integrated Dell Remote Access Controller (iDrac) systems") - ctx, client, err := redfishdell.NewClient( + client, err := redfishdell.NewClient( address, mgmtCfg.Insecure, mgmtCfg.UseProxy, @@ -247,7 +246,7 @@ func newBaremetalHost(mgmtCfg config.ManagementConfiguration, return host, err } - host = baremetalHost{client, ctx, address, hostDoc.GetName(), username, password} + host = baremetalHost{client, address, hostDoc.GetName(), username, password} default: return host, ErrUnknownManagementType{Type: mgmtCfg.Type} } diff --git a/pkg/remote/redfish/client.go b/pkg/remote/redfish/client.go index e286a59f6..8ce4d3ea9 100644 --- a/pkg/remote/redfish/client.go +++ b/pkg/remote/redfish/client.go @@ -35,6 +35,8 @@ const ( // Client holds details about a Redfish out-of-band system required for out-of-band management. type Client struct { nodeID string + username string + password string RedfishAPI redfishAPI.RedfishAPI RedfishCFG *redfishClient.Configuration systemActionRetries int @@ -61,6 +63,7 @@ func (c *Client) SystemRebootDelay() int { // EjectVirtualMedia ejects a virtual media device attached to a host. func (c *Client) EjectVirtualMedia(ctx context.Context) error { + ctx = c.setAuth(ctx) waitForEjectMedia := func(managerID string, mediaID string) error { for retry := 0; retry < c.systemActionRetries; retry++ { vMediaMgr, httpResp, err := c.RedfishAPI.GetManagerVirtualMedia(ctx, managerID, mediaID) @@ -117,6 +120,7 @@ func (c *Client) EjectVirtualMedia(ctx context.Context) error { // RebootSystem power cycles a host by sending a shutdown signal followed by a power on signal. func (c *Client) RebootSystem(ctx context.Context) error { log.Debugf("Rebooting node '%s': powering off.", c.nodeID) + ctx = c.setAuth(ctx) resetReq := redfishClient.ResetRequestBody{} // Send PowerOff request @@ -149,6 +153,7 @@ func (c *Client) RebootSystem(ctx context.Context) error { // SetBootSourceByType sets the boot source of the ephemeral node to one that's compatible with the boot // source type. func (c *Client) SetBootSourceByType(ctx context.Context) error { + ctx = c.setAuth(ctx) _, vMediaType, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.nodeID) if err != nil { return err @@ -184,6 +189,7 @@ func (c *Client) SetBootSourceByType(ctx context.Context) error { // SetVirtualMedia injects a virtual media device to an established virtual media ID. This assumes that isoPath is // accessible to the redfish server and virtualMedia device is either of type CD or DVD. func (c *Client) SetVirtualMedia(ctx context.Context, isoPath string) error { + ctx = c.setAuth(ctx) log.Debugf("Inserting virtual media '%s'.", isoPath) // Eject all previously-inserted media if err := c.EjectVirtualMedia(ctx); err != nil { @@ -217,6 +223,7 @@ func (c *Client) SetVirtualMedia(ctx context.Context, isoPath string) error { // SystemPowerOff shuts down a host. func (c *Client) SystemPowerOff(ctx context.Context) error { + ctx = c.setAuth(ctx) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -230,6 +237,7 @@ func (c *Client) SystemPowerOff(ctx context.Context) error { // SystemPowerOn powers on a host. func (c *Client) SystemPowerOn(ctx context.Context) error { + ctx = c.setAuth(ctx) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_ON @@ -243,6 +251,7 @@ func (c *Client) SystemPowerOn(ctx context.Context) error { // SystemPowerStatus retrieves the power status of a host as a human-readable string. func (c *Client) SystemPowerStatus(ctx context.Context) (power.Status, error) { + ctx = c.setAuth(ctx) computerSystem, httpResp, err := c.RedfishAPI.GetSystem(ctx, c.nodeID) if err = ScreenRedfishError(httpResp, err); err != nil { return power.StatusUnknown, err @@ -262,6 +271,18 @@ func (c *Client) SystemPowerStatus(ctx context.Context) (power.Status, error) { } } +func (c *Client) setAuth(ctx context.Context) context.Context { + authValue := redfishClient.BasicAuth{UserName: c.username, Password: c.password} + if ctx.Value(redfishClient.ContextBasicAuth) == authValue { + return ctx + } + return context.WithValue( + ctx, + redfishClient.ContextBasicAuth, + redfishClient.BasicAuth{UserName: c.username, Password: c.password}, + ) +} + // NewClient returns a client with the capability to make Redfish requests. func NewClient(redfishURL string, insecure bool, @@ -269,25 +290,14 @@ func NewClient(redfishURL string, username string, password string, systemActionRetries int, - systemRebootDelay int) (context.Context, *Client, error) { - var ctx context.Context - if username != "" && password != "" { - ctx = context.WithValue( - context.Background(), - redfishClient.ContextBasicAuth, - redfishClient.BasicAuth{UserName: username, Password: password}, - ) - } else { - ctx = context.Background() - } - + systemRebootDelay int) (*Client, error) { if redfishURL == "" { - return ctx, nil, ErrRedfishMissingConfig{What: "Redfish URL"} + return nil, ErrRedfishMissingConfig{What: "Redfish URL"} } basePath, err := getBasePath(redfishURL) if err != nil { - return ctx, nil, err + return nil, err } cfg := &redfishClient.Configuration{ @@ -320,7 +330,7 @@ func NewClient(redfishURL string, // Retrieve system ID from end of Redfish URL systemID := GetResourceIDFromURL(redfishURL) if len(systemID) == 0 { - return ctx, nil, ErrRedfishMissingConfig{What: "management URL system ID"} + return nil, ErrRedfishMissingConfig{What: "management URL system ID"} } c := &Client{ @@ -329,11 +339,13 @@ func NewClient(redfishURL string, RedfishCFG: cfg, systemActionRetries: systemActionRetries, systemRebootDelay: systemRebootDelay, + password: password, + username: username, Sleep: func(d time.Duration) { time.Sleep(d) }, } - return ctx, c, nil + return c, nil } diff --git a/pkg/remote/redfish/client_test.go b/pkg/remote/redfish/client_test.go index 57d5d8407..06ebc09cc 100644 --- a/pkg/remote/redfish/client_test.go +++ b/pkg/remote/redfish/client_test.go @@ -40,14 +40,15 @@ const ( ) func TestNewClient(t *testing.T) { - _, _, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + c, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) + assert.NotNil(t, c) } func TestNewClientDefaultValues(t *testing.T) { sysActRetr := 111 sysRebDel := 999 - _, c, err := NewClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel) + c, err := NewClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel) assert.Equal(t, c.systemActionRetries, sysActRetr) assert.Equal(t, c.systemRebootDelay, sysRebDel) assert.NoError(t, err) @@ -56,7 +57,7 @@ func TestNewClientDefaultValues(t *testing.T) { func TestNewClientMissingSystemID(t *testing.T) { badURL := "redfish+https://localhost:2224" - _, _, err := NewClient(badURL, false, false, "", "", systemActionRetries, systemRebootDelay) + _, err := NewClient(badURL, false, false, "", "", systemActionRetries, systemRebootDelay) _, ok := err.(ErrRedfishMissingConfig) assert.True(t, ok) } @@ -64,34 +65,24 @@ func TestNewClientMissingSystemID(t *testing.T) { func TestNewClientNoRedfishMarking(t *testing.T) { url := "https://localhost:2224/Systems/System.Embedded.1" - _, _, err := NewClient(url, false, false, "", "", systemActionRetries, systemRebootDelay) + _, err := NewClient(url, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) } -func TestNewClientAuth(t *testing.T) { - ctx, _, err := NewClient(redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay) - assert.NoError(t, err) - - cAuth := ctx.Value(redfishClient.ContextBasicAuth) - auth := redfishClient.BasicAuth{UserName: "username", Password: "password"} - assert.Equal(t, cAuth, auth) -} - func TestNewClientEmptyRedfishURL(t *testing.T) { // Redfish URL cannot be empty when creating a client. - _, _, err := NewClient("", false, false, "", "", systemActionRetries, systemRebootDelay) + _, 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, "", "", systemActionRetries+1, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries+1, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - - ctx := context.Background() + ctx := client.setAuth(context.Background()) // Mark CD and DVD test media as inserted inserted := true @@ -102,7 +93,7 @@ func TestEjectVirtualMedia(t *testing.T) { testMediaDVD.Inserted = &inserted httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil).Times(1) m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). Return(testutil.GetMediaCollection([]string{"Cd", "DVD", "Floppy"}), httpResp, nil) @@ -141,12 +132,12 @@ func TestEjectVirtualMediaRetriesExceeded(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - ctx := context.Background() + ctx := client.setAuth(context.Background()) // Mark test media as inserted inserted := true @@ -155,17 +146,17 @@ func TestEjectVirtualMediaRetriesExceeded(t *testing.T) { httpResp := &http.Response{StatusCode: 200} m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) - m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). + m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID). Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) - m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd"). Return(testMedia, httpResp, nil) // Verify retry logic - m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything).Times(1). + m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything). Return(redfishClient.RedfishError{}, httpResp, nil) // Media still inserted on retry. Since retries are 1, this causes failure. - m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd"). Return(testMedia, httpResp, nil) // Replace normal API client with mocked API client @@ -181,10 +172,11 @@ func TestRebootSystem(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) // Mock redfish shutdown and status requests resetReq := redfishClient.ResetRequestBody{} @@ -215,10 +207,11 @@ func TestRebootSystemShutdownError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -241,10 +234,11 @@ func TestRebootSystemStartupError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -278,12 +272,12 @@ func TestRebootSystemTimeout(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) - ctx := context.Background() resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -307,10 +301,11 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) // Mock redfish get system request m.On("GetSystem", ctx, client.NodeID()).Times(1).Return(redfishClient.ComputerSystem{}, @@ -329,10 +324,11 @@ func TestSetBootSourceByTypeSetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) httpResp := &http.Response{StatusCode: 200} m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) @@ -356,9 +352,10 @@ func TestSetBootSourceByTypeBootSourceUnavailable(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) + ctx := client.setAuth(context.Background()) client.nodeID = nodeID invalidSystem := testutil.GetTestSystem() @@ -388,12 +385,12 @@ func TestSetVirtualMediaEjectExistingMedia(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - ctx := context.Background() + ctx := client.setAuth(context.Background()) // Mark test media as inserted inserted := true @@ -434,12 +431,12 @@ func TestSetVirtualMediaEjectExistingMediaFailure(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) client.nodeID = nodeID - ctx := context.Background() + ctx := client.setAuth(context.Background()) // Mark test media as inserted inserted := true @@ -471,9 +468,10 @@ func TestSetVirtualMediaGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) + ctx := client.setAuth(context.Background()) client.nodeID = nodeID // Mock redfish get system request @@ -493,9 +491,10 @@ func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) + ctx := client.setAuth(context.Background()) client.nodeID = nodeID httpResp := &http.Response{StatusCode: 200} @@ -510,7 +509,7 @@ func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) { Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). Return(testutil.GetVirtualMedia([]string{"CD"}), httpResp, nil) - m.On("InsertVirtualMedia", context.Background(), testutil.ManagerID, "Cd", mock.Anything).Return( + m.On("InsertVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything).Return( redfishClient.RedfishError{}, &http.Response{StatusCode: 500}, redfishClient.GenericOpenAPIError{}) // Replace normal API client with mocked API client @@ -527,12 +526,11 @@ func TestSystemPowerOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - - ctx := context.Background() + ctx := client.setAuth(context.Background()) m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -560,12 +558,11 @@ func TestSystemPowerOffResetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - - ctx := context.Background() + ctx := client.setAuth(context.Background()) m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -585,12 +582,11 @@ func TestSystemPowerOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - - ctx := context.Background() + ctx := client.setAuth(context.Background()) m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -618,12 +614,11 @@ func TestSystemPowerOnResetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID - - ctx := context.Background() + ctx := client.setAuth(context.Background()) m.On("ResetSystem", ctx, client.nodeID, mock.Anything).Return( redfishClient.RedfishError{}, @@ -643,10 +638,11 @@ func TestSystemPowerStatusUnknown(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) var unknownState redfishClient.PowerState = "unknown" m.On("GetSystem", ctx, client.nodeID).Return( @@ -666,9 +662,10 @@ func TestSystemPowerStatusOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) + ctx := client.setAuth(context.Background()) client.nodeID = nodeID m.On("GetSystem", ctx, client.nodeID).Return( @@ -688,10 +685,11 @@ func TestSystemPowerStatusOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) m.On("GetSystem", ctx, client.nodeID).Return( redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_OFF}, @@ -710,10 +708,11 @@ func TestSystemPowerStatusPoweringOn(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) m.On("GetSystem", ctx, client.nodeID).Return( redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_POWERING_ON}, @@ -732,10 +731,11 @@ func TestSystemPowerStatusPoweringOff(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) m.On("GetSystem", ctx, client.nodeID).Return( redfishClient.ComputerSystem{PowerState: redfishClient.POWERSTATE_POWERING_OFF}, @@ -754,10 +754,11 @@ func TestSystemPowerStatusGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) require.NoError(t, err) client.nodeID = nodeID + ctx := client.setAuth(context.Background()) m.On("GetSystem", ctx, client.nodeID).Return( redfishClient.ComputerSystem{}, @@ -774,10 +775,10 @@ func TestWaitForPowerStateGetSystemFailed(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.Background() + ctx := client.setAuth(context.Background()) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -798,10 +799,10 @@ func TestWaitForPowerStateNoRetries(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.Background() + ctx := client.setAuth(context.Background()) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -824,10 +825,10 @@ func TestWaitForPowerStateWithRetries(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.Background() + ctx := client.setAuth(context.Background()) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -855,10 +856,10 @@ func TestWaitForPowerStateRetriesExceeded(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.Background() + ctx := client.setAuth(context.Background()) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_OFF @@ -886,10 +887,10 @@ func TestWaitForPowerStateDifferentPowerState(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - _, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) - ctx := context.Background() + ctx := client.setAuth(context.Background()) resetReq := redfishClient.ResetRequestBody{} resetReq.ResetType = redfishClient.RESETTYPE_FORCE_ON diff --git a/pkg/remote/redfish/vendors/dell/client.go b/pkg/remote/redfish/vendors/dell/client.go index 43e05a90c..8a1d2878a 100644 --- a/pkg/remote/redfish/vendors/dell/client.go +++ b/pkg/remote/redfish/vendors/dell/client.go @@ -130,14 +130,14 @@ func NewClient(redfishURL string, username string, password string, systemActionRetries int, - systemRebootDelay int) (context.Context, *Client, error) { - ctx, genericClient, err := redfish.NewClient(redfishURL, insecure, useProxy, username, password, + systemRebootDelay int) (*Client, error) { + genericClient, err := redfish.NewClient(redfishURL, insecure, useProxy, username, password, systemActionRetries, systemRebootDelay) if err != nil { - return ctx, nil, err + return nil, err } c := &Client{*genericClient, genericClient.RedfishAPI, genericClient.RedfishCFG} - return ctx, c, nil + return c, nil } diff --git a/pkg/remote/redfish/vendors/dell/client_test.go b/pkg/remote/redfish/vendors/dell/client_test.go index 159412734..f3aaa83a5 100644 --- a/pkg/remote/redfish/vendors/dell/client_test.go +++ b/pkg/remote/redfish/vendors/dell/client_test.go @@ -15,6 +15,7 @@ package dell import ( + "context" "net/http" "testing" @@ -31,14 +32,14 @@ const ( ) func TestNewClient(t *testing.T) { - _, _, err := NewClient(redfishURL, false, false, "username", "password", systemActionRetries, systemRebootDelay) + _, 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) + c, err := NewClient(redfishURL, false, false, "", "", sysActRetr, sysRebDel) assert.Equal(t, c.SystemActionRetries(), sysActRetr) assert.Equal(t, c.SystemRebootDelay(), sysRebDel) assert.NoError(t, err) @@ -47,8 +48,9 @@ func TestSetBootSourceByTypeGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) - ctx, client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) + client, err := NewClient(redfishURL, false, false, "", "", systemActionRetries, systemRebootDelay) assert.NoError(t, err) + ctx := context.Background() // Mock redfish get system request m.On("GetSystem", ctx, client.NodeID()).Times(1).Return(redfishClient.ComputerSystem{}, diff --git a/pkg/remote/remote_direct.go b/pkg/remote/remote_direct.go index 59ca87a0d..4a972a9fe 100644 --- a/pkg/remote/remote_direct.go +++ b/pkg/remote/remote_direct.go @@ -15,6 +15,8 @@ package remote import ( + "context" + api "opendev.org/airship/airshipctl/pkg/api/v1alpha1" "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/document" @@ -65,7 +67,7 @@ func (b baremetalHost) DoRemoteDirect(cfg *config.Config) error { log.Debugf("Bootstrapping ephemeral host '%s' with ID '%s' and BMC Address '%s'.", b.HostName, b.NodeID(), b.BMCAddress) - powerStatus, err := b.SystemPowerStatus(b.Context) + powerStatus, err := b.SystemPowerStatus(context.Background()) if err != nil { return err } @@ -73,7 +75,7 @@ func (b baremetalHost) DoRemoteDirect(cfg *config.Config) error { // Power on node if it is off if powerStatus != power.StatusOn { log.Debugf("Ephemeral node has power status '%s'. Attempting to power on.", powerStatus.String()) - if err = b.SystemPowerOn(b.Context); err != nil { + if err = b.SystemPowerOn(context.Background()); err != nil { return err } } @@ -83,17 +85,17 @@ func (b baremetalHost) DoRemoteDirect(cfg *config.Config) error { return ErrMissingOption{What: "isoURL"} } - err = b.SetVirtualMedia(b.Context, remoteDirectConfiguration.IsoURL) + err = b.SetVirtualMedia(context.Background(), remoteDirectConfiguration.IsoURL) if err != nil { return err } - err = b.SetBootSourceByType(b.Context) + err = b.SetBootSourceByType(context.Background()) if err != nil { return err } - err = b.RebootSystem(b.Context) + err = b.RebootSystem(context.Background()) if err != nil { return err } diff --git a/pkg/remote/remote_direct_test.go b/pkg/remote/remote_direct_test.go index 3a7c0943c..287a4d4ec 100644 --- a/pkg/remote/remote_direct_test.go +++ b/pkg/remote/remote_direct_test.go @@ -15,6 +15,7 @@ package remote import ( + "context" "fmt" "testing" @@ -34,12 +35,11 @@ const ( ) func TestDoRemoteDirectMissingConfigOpts(t *testing.T) { - ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) ephemeralHost := baremetalHost{ rMock, - ctx, redfishURL, "doc-name", username, @@ -56,15 +56,16 @@ func TestDoRemoteDirectMissingConfigOpts(t *testing.T) { } func TestDoRemoteDirectMissingISOURL(t *testing.T) { - ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) + ctx := context.Background() + rMock.On("NodeID").Times(1).Return(systemID) rMock.On("SystemPowerStatus", ctx).Times(1).Return(power.StatusOn, nil) ephemeralHost := baremetalHost{ rMock, - ctx, redfishURL, "doc-name", username, @@ -79,9 +80,11 @@ func TestDoRemoteDirectMissingISOURL(t *testing.T) { } func TestDoRemoteDirectRedfish(t *testing.T) { - ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) + ctx := context.Background() + rMock.On("NodeID").Times(1).Return(systemID) rMock.On("SystemPowerStatus", ctx).Times(1).Return(power.StatusOn, nil) rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(nil) @@ -91,7 +94,6 @@ func TestDoRemoteDirectRedfish(t *testing.T) { ephemeralHost := baremetalHost{ rMock, - ctx, redfishURL, "doc-name", username, @@ -104,9 +106,11 @@ func TestDoRemoteDirectRedfish(t *testing.T) { } func TestDoRemoteDirectRedfishNodePoweredOff(t *testing.T) { - ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) + ctx := context.Background() + rMock.On("NodeID").Times(1).Return(systemID) rMock.On("SystemPowerStatus", ctx).Times(1).Return(power.StatusOff, nil) rMock.On("SystemPowerOn", ctx).Times(1).Return(nil) @@ -117,7 +121,6 @@ func TestDoRemoteDirectRedfishNodePoweredOff(t *testing.T) { ephemeralHost := baremetalHost{ rMock, - ctx, redfishURL, "doc-name", username, @@ -130,9 +133,11 @@ func TestDoRemoteDirectRedfishNodePoweredOff(t *testing.T) { } func TestDoRemoteDirectRedfishVirtualMediaError(t *testing.T) { - ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) + ctx := context.Background() + expectedErr := redfish.ErrRedfishClient{Message: "Unable to set virtual media."} rMock.On("NodeID").Times(1).Return(systemID) @@ -144,7 +149,6 @@ func TestDoRemoteDirectRedfishVirtualMediaError(t *testing.T) { ephemeralHost := baremetalHost{ rMock, - ctx, redfishURL, "doc-name", username, @@ -159,9 +163,11 @@ func TestDoRemoteDirectRedfishVirtualMediaError(t *testing.T) { } func TestDoRemoteDirectRedfishBootSourceError(t *testing.T) { - ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) + ctx := context.Background() + rMock.On("NodeID").Times(1).Return(systemID) rMock.On("SystemPowerStatus", ctx).Times(1).Return(power.StatusOn, nil) rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(nil) @@ -173,7 +179,6 @@ func TestDoRemoteDirectRedfishBootSourceError(t *testing.T) { ephemeralHost := baremetalHost{ rMock, - ctx, redfishURL, "doc-name", username, @@ -188,9 +193,11 @@ func TestDoRemoteDirectRedfishBootSourceError(t *testing.T) { } func TestDoRemoteDirectRedfishRebootError(t *testing.T) { - ctx, rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) + rMock, err := redfishutils.NewClient(redfishURL, false, false, username, password) assert.NoError(t, err) + ctx := context.Background() + rMock.On("NodeID").Times(1).Return(systemID) rMock.On("SystemPowerStatus", ctx).Times(1).Return(power.StatusOn, nil) rMock.On("SetVirtualMedia", ctx, isoURL).Times(1).Return(nil) @@ -203,7 +210,6 @@ func TestDoRemoteDirectRedfishRebootError(t *testing.T) { ephemeralHost := baremetalHost{ rMock, - ctx, redfishURL, "doc-name", username, diff --git a/testutil/redfishutils/mock_client.go b/testutil/redfishutils/mock_client.go index ac5829288..b7d67c079 100644 --- a/testutil/redfishutils/mock_client.go +++ b/testutil/redfishutils/mock_client.go @@ -16,7 +16,6 @@ import ( "context" "github.com/stretchr/testify/mock" - redfishClient "opendev.org/airship/go-redfish/client" "opendev.org/airship/airshipctl/pkg/remote/power" "opendev.org/airship/airshipctl/pkg/remote/redfish" @@ -141,29 +140,17 @@ func (m *MockClient) SystemPowerStatus(ctx context.Context) (power.Status, error // NewClient returns a mocked Redfish client in order to test functions that use the Redfish client without making any // Redfish API calls. func NewClient(redfishURL string, insecure bool, useProxy bool, username string, - password string) (context.Context, *MockClient, error) { - var ctx context.Context - if username != "" && password != "" { - ctx = context.WithValue( - context.Background(), - redfishClient.ContextBasicAuth, - redfishClient.BasicAuth{UserName: username, Password: password}, - ) - } else { - ctx = context.Background() - } - + password string) (*MockClient, error) { if redfishURL == "" { - return ctx, nil, redfish.ErrRedfishMissingConfig{What: "Redfish URL"} + return nil, redfish.ErrRedfishMissingConfig{What: "Redfish URL"} } // Retrieve system ID from end of Redfish URL systemID := redfish.GetResourceIDFromURL(redfishURL) if len(systemID) == 0 { - return ctx, nil, redfish.ErrRedfishMissingConfig{What: "management URL system ID"} + return nil, redfish.ErrRedfishMissingConfig{What: "management URL system ID"} } m := &MockClient{nodeID: systemID} - - return ctx, m, nil + return m, nil }