From f3262bc01570e441bfaf37aca31b35f0cd71264b Mon Sep 17 00:00:00 2001 From: Drew Walters Date: Mon, 30 Mar 2020 15:43:56 +0000 Subject: [PATCH] Retrieve actual virtual media ID and types The "GetVirtualID" method is stubbed for our Redifsh implementation and always returns the same values. This change uses the Redfish client to retrieve the correct virtual media ID and type. Closes #141 Change-Id: I319b89c13349e7b2f5bf6eb8ced97c949e5f44b5 Signed-off-by: Drew Walters --- pkg/remote/redfish/redfish.go | 2 +- pkg/remote/redfish/redfish_test.go | 42 +++++++++++++++++++- pkg/remote/redfish/utils.go | 32 ++++++++++++---- pkg/remote/redfish/utils_test.go | 61 ++++++++++++++++++++++++++++++ testutil/redfish/mock_helpers.go | 49 ++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 10 deletions(-) create mode 100644 testutil/redfish/mock_helpers.go diff --git a/pkg/remote/redfish/redfish.go b/pkg/remote/redfish/redfish.go index a20ab81db..d72582f24 100644 --- a/pkg/remote/redfish/redfish.go +++ b/pkg/remote/redfish/redfish.go @@ -54,7 +54,7 @@ func (cfg RemoteDirect) DoRemoteDirect() error { alog.Debugf("Ephemeral node managerID: '%s'", managerID) /* Get manager's Cd or DVD virtual media ID */ - vMediaID, vMediaType, err := GetVirtualMediaID() + vMediaID, vMediaType, err := GetVirtualMediaID(cfg.Context, cfg.RedfishAPI, managerID) if err != nil { return err } diff --git a/pkg/remote/redfish/redfish_test.go b/pkg/remote/redfish/redfish_test.go index 29914122a..aaa42afa0 100644 --- a/pkg/remote/redfish/redfish_test.go +++ b/pkg/remote/redfish/redfish_test.go @@ -15,6 +15,7 @@ import ( redfishClient "opendev.org/airship/go-redfish/client" . "opendev.org/airship/airshipctl/pkg/remote/redfish" + testutil "opendev.org/airship/airshipctl/testutil/redfish" ) const ( @@ -37,6 +38,14 @@ func TestRedfishRemoteDirectNormal(t *testing.T) { m.On("GetSystem", ctx, systemID).Times(1). Return(getTestSystem(), httpResp, nil) + + // GetVirtualMediaID() mocks + m.On("ListManagerVirtualMedia", ctx, "manager-1").Times(1). + Return(testutil.GetMediaCollection([]string{"Cd", "Floppy"}), httpResp, nil) + + m.On("GetManagerVirtualMedia", ctx, "manager-1", "Cd").Times(1). + Return(testutil.GetVirtualMedia([]string{"CD"}), httpResp, nil) + m.On("InsertVirtualMedia", ctx, "manager-1", "Cd", mock.Anything). Return(redfishClient.RedfishError{}, httpResp, nil) @@ -142,11 +151,19 @@ func TestRedfishRemoteDirectInvalidIsoPath(t *testing.T) { localRDCfg.IsoPath = "bogus/path/to.iso" realErr := redfishClient.GenericOpenAPIError{} - httpResp := &http.Response{StatusCode: 500} m.On("GetSystem", ctx, systemID). Times(1). Return(getTestSystem(), nil, nil) + httpSuccResp := &http.Response{StatusCode: 200} + // GetVirtualMediaID() mocks + m.On("ListManagerVirtualMedia", ctx, "manager-1").Times(1). + Return(testutil.GetMediaCollection([]string{"Cd", "Floppy"}), httpSuccResp, nil) + + m.On("GetManagerVirtualMedia", ctx, "manager-1", "Cd").Times(1). + Return(testutil.GetVirtualMedia([]string{"CD"}), httpSuccResp, nil) + + httpResp := &http.Response{StatusCode: 500} m.On("InsertVirtualMedia", ctx, "manager-1", "Cd", mock.Anything). Return(redfishClient.RedfishError{}, httpResp, realErr) @@ -176,6 +193,14 @@ func TestRedfishRemoteDirectCdDvdNotAvailableInBootSources(t *testing.T) { m.On("GetSystem", ctx, systemID). Return(invalidSystem, nil, nil) + // GetVirtualMediaID() mocks + httpResp := &http.Response{StatusCode: 200} + m.On("ListManagerVirtualMedia", ctx, "manager-1").Times(1). + Return(testutil.GetMediaCollection([]string{"Cd", "Floppy"}), httpResp, nil) + + m.On("GetManagerVirtualMedia", ctx, "manager-1", "Cd").Times(1). + Return(testutil.GetVirtualMedia([]string{"CD"}), httpResp, nil) + m.On("InsertVirtualMedia", ctx, "manager-1", "Cd", mock.Anything). Return(redfishClient.RedfishError{}, nil, nil) @@ -199,9 +224,17 @@ func TestRedfishRemoteDirectSetSystemBootSourceFailed(t *testing.T) { systemID := computerSystemID httpSuccResp := &http.Response{StatusCode: 200} + m.On("GetSystem", ctx, systemID). Return(getTestSystem(), httpSuccResp, nil) + // GetVirtualMediaID() mocks + m.On("ListManagerVirtualMedia", ctx, "manager-1").Times(1). + Return(testutil.GetMediaCollection([]string{"Cd", "Floppy"}), httpSuccResp, nil) + + m.On("GetManagerVirtualMedia", ctx, "manager-1", "Cd").Times(1). + Return(testutil.GetVirtualMedia([]string{"CD"}), httpSuccResp, nil) + m.On("InsertVirtualMedia", ctx, "manager-1", "Cd", mock.Anything). Return(redfishClient.RedfishError{}, httpSuccResp, nil) @@ -233,6 +266,13 @@ func TestRedfishRemoteDirectSystemRebootFailed(t *testing.T) { m.On("GetSystem", ctx, systemID). Return(getTestSystem(), httpSuccResp, nil) + // GetVirtualMediaID() mocks + m.On("ListManagerVirtualMedia", ctx, "manager-1").Times(1). + Return(testutil.GetMediaCollection([]string{"Cd", "Floppy"}), httpSuccResp, nil) + + m.On("GetManagerVirtualMedia", ctx, "manager-1", "Cd").Times(1). + Return(testutil.GetVirtualMedia([]string{"CD"}), httpSuccResp, nil) + m.On("InsertVirtualMedia", ctx, mock.Anything, mock.Anything, mock.Anything). Return(redfishClient.RedfishError{}, httpSuccResp, nil) diff --git a/pkg/remote/redfish/utils.go b/pkg/remote/redfish/utils.go index 696a24e83..ae42f2bed 100644 --- a/pkg/remote/redfish/utils.go +++ b/pkg/remote/redfish/utils.go @@ -46,14 +46,30 @@ func IsIDInList(idRefList []redfishClient.IdRef, id string) bool { return false } -// This function walks through the list of manager's virtual media resources -// and gets the ID of virtualmedia which has type "CD" or "DVD" -func GetVirtualMediaID() (string, string, error) { - // TODO: Sushy emulator has a bug which sends 'virtualMedia.inserted' field as - // string instead of a boolean which causes the redfish client to fail - // while unmarshalling this field. - // Insert logic here after the bug is fixed in sushy-emulator. - return "Cd", "CD", nil +// GetVirtualMediaID retrieves the ID of a Redfish virtual media resource if it supports type "CD" or "DVD". +func GetVirtualMediaID(ctx context.Context, api redfishApi.RedfishAPI, managerID string) (string, string, error) { + mediaCollection, httpResp, err := api.ListManagerVirtualMedia(ctx, managerID) + if err = ScreenRedfishError(httpResp, err); err != nil { + return "", "", err + } + + for _, mediaURI := range mediaCollection.Members { + // Retrieve the virtual media ID from the request URI + mediaID := GetResourceIDFromURL(mediaURI.OdataId) + + vMedia, httpResp, err := api.GetManagerVirtualMedia(ctx, managerID, mediaID) + if err = ScreenRedfishError(httpResp, err); err != nil { + return "", "", err + } + + for _, mediaType := range vMedia.MediaTypes { + if mediaType == "CD" || mediaType == "DVD" { + return mediaID, mediaType, nil + } + } + } + + return "", "", ErrRedfishClient{Message: "Unable to find virtual media with type CD or DVD"} } // This function walks through the bootsources of a system and sets the bootsource diff --git a/pkg/remote/redfish/utils_test.go b/pkg/remote/redfish/utils_test.go index c9268f3d4..a4eb75db5 100644 --- a/pkg/remote/redfish/utils_test.go +++ b/pkg/remote/redfish/utils_test.go @@ -9,6 +9,8 @@ import ( redfishMocks "opendev.org/airship/go-redfish/api/mocks" redfishClient "opendev.org/airship/go-redfish/client" + + testutil "opendev.org/airship/airshipctl/testutil/redfish" ) const ( @@ -90,6 +92,65 @@ func TestRedfishUtilIsIdInList(t *testing.T) { assert.False(t, res) } +func TestGetVirtualMediaID(t *testing.T) { + m := &redfishMocks.RedfishAPI{} + defer m.AssertExpectations(t) + + ctx := context.Background() + managerID := "manager-090102" + httpResp := &http.Response{StatusCode: 200} + + m.On("ListManagerVirtualMedia", ctx, managerID).Times(1). + Return(testutil.GetMediaCollection([]string{"Floppy", "Cd"}), httpResp, nil) + + m.On("GetManagerVirtualMedia", ctx, managerID, "Floppy").Times(1). + Return(testutil.GetVirtualMedia([]string{"Floppy", "USBStick"}), httpResp, nil) + + m.On("GetManagerVirtualMedia", ctx, managerID, "Cd").Times(1). + Return(testutil.GetVirtualMedia([]string{"CD"}), httpResp, nil) + + mediaID, mediaType, err := GetVirtualMediaID(ctx, m, managerID) + assert.Equal(t, mediaID, "Cd") + assert.Equal(t, mediaType, "CD") + assert.NoError(t, err) +} + +func TestGetVirtualMediaIDNoMedia(t *testing.T) { + m := &redfishMocks.RedfishAPI{} + defer m.AssertExpectations(t) + + ctx := context.Background() + managerID := "manager-090102" + httpResp := &http.Response{StatusCode: 200} + + m.On("ListManagerVirtualMedia", ctx, managerID).Times(1).Return(redfishClient.Collection{}, httpResp, nil) + + mediaID, mediaType, err := GetVirtualMediaID(ctx, m, managerID) + assert.Empty(t, mediaID) + assert.Empty(t, mediaType) + assert.Error(t, err) +} + +func TestGetVirtualMediaIDUnacceptableMediaTypes(t *testing.T) { + m := &redfishMocks.RedfishAPI{} + defer m.AssertExpectations(t) + + ctx := context.Background() + managerID := "manager-090102" + httpResp := &http.Response{StatusCode: 200} + + m.On("ListManagerVirtualMedia", ctx, managerID).Times(1). + Return(testutil.GetMediaCollection([]string{"Floppy"}), httpResp, nil) + + m.On("GetManagerVirtualMedia", ctx, managerID, "Floppy").Times(1). + Return(testutil.GetVirtualMedia([]string{"Floppy", "USBStick"}), httpResp, nil) + + mediaID, mediaType, err := GetVirtualMediaID(ctx, m, managerID) + assert.Empty(t, mediaID) + assert.Empty(t, mediaType) + assert.Error(t, err) +} + func TestRedfishUtilRebootSystemOK(t *testing.T) { m := &redfishMocks.RedfishAPI{} defer m.AssertExpectations(t) diff --git a/testutil/redfish/mock_helpers.go b/testutil/redfish/mock_helpers.go new file mode 100644 index 000000000..f6c5ba142 --- /dev/null +++ b/testutil/redfish/mock_helpers.go @@ -0,0 +1,49 @@ +// 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 +// +// http://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 redfish + +import ( + "fmt" + + redfishClient "opendev.org/airship/go-redfish/client" +) + +// GetMediaCollection builds a collection of media IDs returned by the "ListManagerVirtualMedia" function. +func GetMediaCollection(refs []string) redfishClient.Collection { + uri := "/redfish/v1/Managers/7832-09/VirtualMedia" + ids := []redfishClient.IdRef{} + + for _, r := range refs { + id := redfishClient.IdRef{} + id.OdataId = fmt.Sprintf("%s/%s", uri, r) + ids = append(ids, id) + } + + c := redfishClient.Collection{Members: ids} + + return c +} + +// GetVirtualMedia builds an array of virtual media resources returned by the "GetManagerVirtualMedia" function. +func GetVirtualMedia(types []string) redfishClient.VirtualMedia { + vMedia := redfishClient.VirtualMedia{} + + mediaTypes := []string{} + for _, t := range types { + mediaTypes = append(mediaTypes, t) + } + + vMedia.MediaTypes = mediaTypes + + return vMedia +}