From 1ce5005cd6f132c81c4c8f995aaf80c337c72a18 Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Fri, 10 Apr 2020 21:53:53 +0000 Subject: [PATCH] Add baremetal ejectmedia command Early airshipctl usage has identified the need to eject virtual media on-demand using airshipctl in order to prevent booting from stuck media. This change adds a command to eject all virtual media attached to a baremetal node. Change-Id: Id67fa00489093dcb84ce54216e0553fa6a737ea6 Signed-off-by: Drew Walters --- cmd/baremetal/baremetal.go | 3 + cmd/baremetal/baremetal_test.go | 5 + cmd/baremetal/ejectmedia.go | 61 ++++++ .../baremetal-ejectmedia-with-help.golden | 10 + .../baremetal-with-help.golden | 1 + pkg/remote/management.go | 1 + pkg/remote/redfish/client.go | 118 ++++++----- pkg/remote/redfish/client_test.go | 188 +++++++++++++----- testutil/redfishutils/mock_client.go | 14 ++ 9 files changed, 305 insertions(+), 96 deletions(-) create mode 100644 cmd/baremetal/ejectmedia.go create mode 100644 cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden diff --git a/cmd/baremetal/baremetal.go b/cmd/baremetal/baremetal.go index cb9f805d2..797412943 100644 --- a/cmd/baremetal/baremetal.go +++ b/cmd/baremetal/baremetal.go @@ -40,6 +40,9 @@ func NewBaremetalCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Co Short: "Perform actions on baremetal hosts", } + ejectMediaCmd := NewEjectMediaCommand(rootSettings) + cmd.AddCommand(ejectMediaCmd) + isoGenCmd := NewISOGenCommand(rootSettings) cmd.AddCommand(isoGenCmd) diff --git a/cmd/baremetal/baremetal_test.go b/cmd/baremetal/baremetal_test.go index 6c017c114..4df6d4a35 100644 --- a/cmd/baremetal/baremetal_test.go +++ b/cmd/baremetal/baremetal_test.go @@ -28,6 +28,11 @@ func TestBaremetal(t *testing.T) { CmdLine: "-h", Cmd: baremetal.NewBaremetalCommand(nil), }, + { + Name: "baremetal-ejectmedia-with-help", + CmdLine: "-h", + Cmd: baremetal.NewEjectMediaCommand(nil), + }, { Name: "baremetal-isogen-with-help", CmdLine: "-h", diff --git a/cmd/baremetal/ejectmedia.go b/cmd/baremetal/ejectmedia.go new file mode 100644 index 000000000..578bdc9d6 --- /dev/null +++ b/cmd/baremetal/ejectmedia.go @@ -0,0 +1,61 @@ +/* + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package baremetal + +import ( + "fmt" + + "github.com/spf13/cobra" + + "opendev.org/airship/airshipctl/pkg/config" + "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/pkg/remote" +) + +// NewEjectMediaCommand provides a command to eject media attached to a baremetal host. +func NewEjectMediaCommand(rootSettings *environment.AirshipCTLSettings) *cobra.Command { + var labels string + var name string + var phase string + + cmd := &cobra.Command{ + Use: "ejectmedia", + Short: "Eject media attached to a baremetal host", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + m, err := remote.NewManager(rootSettings, phase, remote.ByLabel(labels), remote.ByName(name)) + if err != nil { + return err + } + + for _, host := range m.Hosts { + if err := host.EjectVirtualMedia(host.Context); err != nil { + return err + } + + fmt.Fprintf(cmd.OutOrStdout(), "All media ejected from host %s\n", host.HostName) + } + + return nil + }, + } + + flags := cmd.Flags() + flags.StringVarP(&labels, flagLabel, flagLabelShort, "", flagLabelDescription) + flags.StringVarP(&name, flagName, flagNameShort, "", flagNameDescription) + flags.StringVar(&phase, flagPhase, config.BootstrapPhase, flagPhaseDescription) + + return cmd +} diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden new file mode 100644 index 000000000..39cbcec7b --- /dev/null +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-ejectmedia-with-help.golden @@ -0,0 +1,10 @@ +Eject media attached to a baremetal host + +Usage: + ejectmedia [flags] + +Flags: + -h, --help help for ejectmedia + -l, --labels string Label(s) to filter desired baremetal host documents + -n, --name string Name to filter desired baremetal host document + --phase string airshipctl phase that contains the desired baremetal host document(s) (default "bootstrap") diff --git a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden index 57673503d..7710d630c 100644 --- a/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden +++ b/cmd/baremetal/testdata/TestBaremetalGoldenOutput/baremetal-with-help.golden @@ -4,6 +4,7 @@ Usage: baremetal [command] Available Commands: + ejectmedia Eject media attached to a baremetal host help Help about any command isogen Generate baremetal host ISO image poweroff Shutdown a baremetal host diff --git a/pkg/remote/management.go b/pkg/remote/management.go index 787bdf783..1167ef6b8 100644 --- a/pkg/remote/management.go +++ b/pkg/remote/management.go @@ -27,6 +27,7 @@ import ( // Client is a set of functions that clients created for out-of-band power management and control should implement. The // functions within client are used by power management commands and remote direct functionality. type Client interface { + EjectVirtualMedia(context.Context) error RebootSystem(context.Context) error SystemPowerOff(context.Context) error SystemPowerOn(context.Context) error diff --git a/pkg/remote/redfish/client.go b/pkg/remote/redfish/client.go index b79b8be87..65a45f718 100644 --- a/pkg/remote/redfish/client.go +++ b/pkg/remote/redfish/client.go @@ -45,6 +45,65 @@ func (c *Client) NodeID() string { return c.nodeID } +// 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("numRetries").(int) + if !ok { + totalRetries = systemActionRetries + } + + for retry := 0; retry < totalRetries; retry++ { + vMediaMgr, httpResp, err := c.RedfishAPI.GetManagerVirtualMedia(ctx, managerID, mediaID) + if err = ScreenRedfishError(httpResp, err); err != nil { + return err + } + + if *vMediaMgr.Inserted == false { + log.Debugf("Successfully ejected virtual media.") + return nil + } + } + + return ErrOperationRetriesExceeded{What: fmt.Sprintf("eject media %s", mediaID), Retries: totalRetries} + } + + managerID, err := getManagerID(ctx, c.RedfishAPI, c.nodeID) + if err != nil { + return err + } + + mediaCollection, httpResp, err := c.RedfishAPI.ListManagerVirtualMedia(ctx, managerID) + if err = ScreenRedfishError(httpResp, err); err != nil { + return err + } + + // Walk all virtual media devices and eject if inserted + for _, mediaURI := range mediaCollection.Members { + mediaID := GetResourceIDFromURL(mediaURI.OdataId) + + vMediaMgr, httpResp, err := c.RedfishAPI.GetManagerVirtualMedia(ctx, managerID, mediaID) + if err = ScreenRedfishError(httpResp, err); err != nil { + return err + } + + if *vMediaMgr.Inserted == true { + var emptyBody map[string]interface{} + _, httpResp, err = c.RedfishAPI.EjectVirtualMedia(ctx, managerID, mediaID, emptyBody) + if err = ScreenRedfishError(httpResp, err); err != nil { + return err + } + + if err = waitForEjectMedia(managerID, mediaID); err != nil { + return err + } + } + } + + return nil +} + // RebootSystem power cycles a host by sending a shutdown signal followed by a power on signal. func (c *Client) RebootSystem(ctx context.Context) error { waitForPowerState := func(desiredState redfishClient.PowerState) error { @@ -126,66 +185,27 @@ 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 { - waitForEjectMedia := func(managerID string, vMediaID string) 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++ { - vMediaMgr, httpResp, err := c.RedfishAPI.GetManagerVirtualMedia(ctx, managerID, vMediaID) - if err = ScreenRedfishError(httpResp, err); err != nil { - return err - } - - if *vMediaMgr.Inserted == false { - log.Debugf("Successfully ejected virtual media.") - return nil - } - } - - return ErrOperationRetriesExceeded{What: fmt.Sprintf("eject media %s", vMediaID), Retries: totalRetries} + // Eject all previously-inserted media + if err := c.EjectVirtualMedia(ctx); err != nil { + return err } - log.Debugf("Setting virtual media for node: '%s'", c.nodeID) + // Retrieve the ID of a compatible media type + vMediaID, _, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.nodeID) + if err != nil { + return err + } managerID, err := getManagerID(ctx, c.RedfishAPI, c.nodeID) if err != nil { return err } - log.Debugf("Ephemeral node managerID: '%s'", managerID) - - vMediaID, _, err := GetVirtualMediaID(ctx, c.RedfishAPI, c.nodeID) - if err != nil { - return err - } - - // Eject virtual media if it is already inserted - vMediaMgr, httpResp, err := c.RedfishAPI.GetManagerVirtualMedia(ctx, managerID, vMediaID) - if err = ScreenRedfishError(httpResp, err); err != nil { - return err - } - - if *vMediaMgr.Inserted == true { - log.Debugf("Manager %s media type %s inserted. Attempting to eject.", managerID, vMediaID) - - var emptyBody map[string]interface{} - _, httpResp, err = c.RedfishAPI.EjectVirtualMedia(ctx, managerID, vMediaID, emptyBody) - if err = ScreenRedfishError(httpResp, err); err != nil { - return err - } - - if err = waitForEjectMedia(managerID, vMediaID); err != nil { - return err - } - } - + // Insert media vMediaReq := redfishClient.InsertMediaRequestBody{} vMediaReq.Image = isoPath vMediaReq.Inserted = true - _, httpResp, err = c.RedfishAPI.InsertVirtualMedia(ctx, managerID, vMediaID, vMediaReq) + _, httpResp, err := c.RedfishAPI.InsertVirtualMedia(ctx, managerID, vMediaID, vMediaReq) return ScreenRedfishError(httpResp, err) } diff --git a/pkg/remote/redfish/client_test.go b/pkg/remote/redfish/client_test.go index 7205d5d6d..96aa96e51 100644 --- a/pkg/remote/redfish/client_test.go +++ b/pkg/remote/redfish/client_test.go @@ -69,6 +69,97 @@ func TestNewClientEmptyRedfishURL(t *testing.T) { assert.Error(t, err) } +func TestEjectVirtualMedia(t *testing.T) { + m := &redfishMocks.RedfishAPI{} + defer m.AssertExpectations(t) + + _, client, err := NewClient(redfishURL, false, false, "", "") + assert.NoError(t, err) + + client.nodeID = nodeID + + // Normal retries are 30. Limit them here for test time. + ctx := context.WithValue(context.Background(), "numRetries", 2) + + // Mark CD and DVD test media as inserted + inserted := true + testMediaCD := testutil.GetVirtualMedia([]string{"CD"}) + testMediaCD.Inserted = &inserted + + testMediaDVD := testutil.GetVirtualMedia([]string{"DVD"}) + testMediaDVD.Inserted = &inserted + + httpResp := &http.Response{StatusCode: 200} + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) + m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). + Return(testutil.GetMediaCollection([]string{"Cd", "DVD", "Floppy"}), httpResp, nil) + + // Eject CD + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). + Return(testMediaCD, httpResp, nil) + m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything).Times(1). + Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). + Return(testutil.GetVirtualMedia([]string{"Cd"}), httpResp, nil) + + // Eject DVD and simulate two retries + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "DVD").Times(1). + Return(testMediaDVD, httpResp, nil) + m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "DVD", mock.Anything).Times(1). + Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "DVD").Times(1). + Return(testMediaDVD, httpResp, nil) + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "DVD").Times(1). + Return(testutil.GetVirtualMedia([]string{"DVD"}), httpResp, nil) + + // Floppy is not inserted, so it is not ejected + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Floppy").Times(1). + Return(testutil.GetVirtualMedia([]string{"Floppy"}), httpResp, nil) + + // Replace normal API client with mocked API client + client.RedfishAPI = m + + 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, "", "") + assert.NoError(t, err) + + client.nodeID = nodeID + + ctx := context.WithValue(context.Background(), "numRetries", 1) + + // Mark test media as inserted + inserted := true + testMedia := testutil.GetVirtualMedia([]string{"CD"}) + testMedia.Inserted = &inserted + + httpResp := &http.Response{StatusCode: 200} + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) + m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). + Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). + Return(testMedia, httpResp, nil) + + // Verify retry logic + m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything).Times(1). + 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). + Return(testMedia, httpResp, nil) + + // Replace normal API client with mocked API client + client.RedfishAPI = m + + err = client.EjectVirtualMedia(ctx) + _, ok := err.(ErrOperationRetriesExceeded) + assert.True(t, ok) +} func TestRebootSystem(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -262,7 +353,7 @@ func TestSetBootSourceByTypeBootSourceUnavailable(t *testing.T) { assert.True(t, ok) } -func TestSetVirtualMediaEjectVirtualMedia(t *testing.T) { +func TestSetVirtualMediaEjectExistingMedia(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -281,18 +372,22 @@ func TestSetVirtualMediaEjectVirtualMedia(t *testing.T) { httpResp := &http.Response{StatusCode: 200} m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) + + // Eject Media calls m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). Return(testMedia, httpResp, nil) - m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). - Return(testMedia, httpResp, nil) - - // Verify retry logic m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything).Times(1). Return(redfishClient.RedfishError{}, httpResp, nil) m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). - Return(testutil.GetVirtualMedia([]string{"Cd"}), httpResp, nil) + Return(testutil.GetVirtualMedia([]string{"CD"}), httpResp, nil) + + // Insert media calls + m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). + 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", ctx, testutil.ManagerID, "Cd", mock.Anything).Return( redfishClient.RedfishError{}, httpResp, redfishClient.GenericOpenAPIError{}) @@ -303,6 +398,42 @@ func TestSetVirtualMediaEjectVirtualMedia(t *testing.T) { assert.NoError(t, err) } +func TestSetVirtualMediaEjectExistingMediaFailure(t *testing.T) { + m := &redfishMocks.RedfishAPI{} + defer m.AssertExpectations(t) + + _, client, err := NewClient(redfishURL, false, false, "", "") + assert.NoError(t, err) + + client.nodeID = nodeID + + // Normal retries are 30. Limit them here for test time. + ctx := context.WithValue(context.Background(), "numRetries", 1) + + // Mark test media as inserted + inserted := true + testMedia := testutil.GetVirtualMedia([]string{"CD"}) + testMedia.Inserted = &inserted + + httpResp := &http.Response{StatusCode: 200} + m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) + + // Eject Media calls + m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). + Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). + Return(testMedia, httpResp, nil) + m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything).Times(1). + Return(redfishClient.RedfishError{}, httpResp, nil) + m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). + Return(testMedia, httpResp, nil) + + // Replace normal API client with mocked API client + client.RedfishAPI = m + + err = client.SetVirtualMedia(ctx, isoPath) + assert.Error(t, err) +} func TestSetVirtualMediaGetSystemError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -323,47 +454,6 @@ func TestSetVirtualMediaGetSystemError(t *testing.T) { assert.Error(t, err) } -func TestSetVirtualMediaEjectVirtualMediaRetriesExceeded(t *testing.T) { - m := &redfishMocks.RedfishAPI{} - defer m.AssertExpectations(t) - - _, client, err := NewClient(redfishURL, false, false, "", "") - assert.NoError(t, err) - - client.nodeID = nodeID - - ctx := context.WithValue(context.Background(), "numRetries", 1) - - // Mark test media as inserted - inserted := true - testMedia := testutil.GetVirtualMedia([]string{"CD"}) - testMedia.Inserted = &inserted - - httpResp := &http.Response{StatusCode: 200} - m.On("GetSystem", ctx, client.nodeID).Return(testutil.GetTestSystem(), httpResp, nil) - m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). - Return(testutil.GetMediaCollection([]string{"Cd"}), httpResp, nil) - m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). - Return(testMedia, httpResp, nil) - m.On("GetManagerVirtualMedia", ctx, testutil.ManagerID, "Cd").Times(1). - Return(testMedia, httpResp, nil) - - // Verify retry logic - m.On("EjectVirtualMedia", ctx, testutil.ManagerID, "Cd", mock.Anything).Times(1). - 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). - Return(testMedia, httpResp, nil) - - // Replace normal API client with mocked API client - client.RedfishAPI = m - - err = client.SetVirtualMedia(ctx, isoPath) - _, ok := err.(ErrOperationRetriesExceeded) - assert.True(t, ok) -} - func TestSetVirtualMediaInsertVirtualMediaError(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) @@ -379,6 +469,10 @@ 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) + + // Insert media calls + m.On("ListManagerVirtualMedia", ctx, testutil.ManagerID).Times(1). + 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( diff --git a/testutil/redfishutils/mock_client.go b/testutil/redfishutils/mock_client.go index 544b9edaa..333da719e 100644 --- a/testutil/redfishutils/mock_client.go +++ b/testutil/redfishutils/mock_client.go @@ -40,6 +40,20 @@ func (m *MockClient) NodeID() string { return args.String(0) } +// EjectVirtualMedia provides a stubbed method that can be mocked to test functions that use the +// Redfish client without making any Redfish API calls or requiring the appropriate Redfish client +// settings. +// +// Example usage: +// client := redfishutils.NewClient() +// client.On("EjectVirtualMedia").Return() +// +// err := client.EjectEphemeralVirtualMedia() +func (m *MockClient) EjectVirtualMedia(ctx context.Context) error { + args := m.Called() + return args.Error(0) +} + // RebootSystem provides a stubbed method that can be mocked to test functions that use the Redfish client without // making any Redfish API calls or requiring the appropriate Redfish client settings. //