From 5622adc5fa87ceda7cfc2c18cf6585fe5424d4c9 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 11:17:04 +0100 Subject: [PATCH 01/11] redfish/GetBiosconfiguration: tests and fixtures moved under redfishwrapper package --- internal/redfishwrapper/bios_test.go | 94 +++++++++++++++++++ .../redfishwrapper/fixtures}/dell/bios.json | 0 .../fixtures/dell/serviceroot.json | 80 ++++++++++++++++ .../fixtures}/dell/system.embedded.1.json | 0 .../redfishwrapper/fixtures/dell/systems.json | 13 +++ providers/redfish/bios_test.go | 57 ----------- 6 files changed, 187 insertions(+), 57 deletions(-) create mode 100644 internal/redfishwrapper/bios_test.go rename {providers/redfish/fixtures/v1 => internal/redfishwrapper/fixtures}/dell/bios.json (100%) create mode 100644 internal/redfishwrapper/fixtures/dell/serviceroot.json rename {providers/redfish/fixtures/v1 => internal/redfishwrapper/fixtures}/dell/system.embedded.1.json (100%) create mode 100644 internal/redfishwrapper/fixtures/dell/systems.json delete mode 100644 providers/redfish/bios_test.go diff --git a/internal/redfishwrapper/bios_test.go b/internal/redfishwrapper/bios_test.go new file mode 100644 index 00000000..643ebda2 --- /dev/null +++ b/internal/redfishwrapper/bios_test.go @@ -0,0 +1,94 @@ +package redfishwrapper + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "testing" + + "github.com/stretchr/testify/assert" +) + +func biosConfigFromFixture(t *testing.T) map[string]string { + t.Helper() + + fixturePath := fixturesDir + "/dell/bios.json" + fh, err := os.Open(fixturePath) + if err != nil { + t.Fatalf("%s, failed to open fixture: %s", err.Error(), fixturePath) + } + + defer fh.Close() + + b, err := io.ReadAll(fh) + if err != nil { + t.Fatalf("%s, failed to read fixture: %s", err.Error(), fixturePath) + } + + var bios map[string]any + err = json.Unmarshal([]byte(b), &bios) + if err != nil { + t.Fatalf("%s, failed to unmarshal fixture: %s", err.Error(), fixturePath) + } + + expectedBiosConfig := make(map[string]string) + for k, v := range bios["Attributes"].(map[string]any) { + expectedBiosConfig[k] = fmt.Sprintf("%v", v) + } + + return expectedBiosConfig +} + +func TestGetBiosConfiguration(t *testing.T) { + tests := []struct { + testName string + hfunc map[string]func(http.ResponseWriter, *http.Request) + expectedBiosConfig map[string]string + }{ + { + "GetBiosConfiguration", + map[string]func(http.ResponseWriter, *http.Request){ + "/redfish/v1/": endpointFunc(t, "/dell/serviceroot.json"), + "/redfish/v1/Systems": endpointFunc(t, "/dell/systems.json"), + "/redfish/v1/Systems/System.Embedded.1": endpointFunc(t, "/dell/system.embedded.1.json"), + "/redfish/v1/Systems/System.Embedded.1/Bios": endpointFunc(t, "/dell/bios.json"), + }, + biosConfigFromFixture(t), + }, + } + + for _, tc := range tests { + t.Run(tc.testName, func(t *testing.T) { + mux := http.NewServeMux() + handleFunc := tc.hfunc + for endpoint, handler := range handleFunc { + mux.HandleFunc(endpoint, handler) + } + + server := httptest.NewTLSServer(mux) + defer server.Close() + + parsedURL, err := url.Parse(server.URL) + if err != nil { + t.Fatal(err) + } + + ctx := context.Background() + client := NewClient(parsedURL.Hostname(), parsedURL.Port(), "", "", WithBasicAuthEnabled(true)) + + err = client.Open(ctx) + if err != nil { + t.Fatal(err) + } + + biosConfig, err := client.GetBiosConfiguration(ctx) + assert.Nil(t, err) + assert.Equal(t, tc.expectedBiosConfig, biosConfig) + }) + } +} diff --git a/providers/redfish/fixtures/v1/dell/bios.json b/internal/redfishwrapper/fixtures/dell/bios.json similarity index 100% rename from providers/redfish/fixtures/v1/dell/bios.json rename to internal/redfishwrapper/fixtures/dell/bios.json diff --git a/internal/redfishwrapper/fixtures/dell/serviceroot.json b/internal/redfishwrapper/fixtures/dell/serviceroot.json new file mode 100644 index 00000000..4bd38c6f --- /dev/null +++ b/internal/redfishwrapper/fixtures/dell/serviceroot.json @@ -0,0 +1,80 @@ +{ + "@odata.context": "/redfish/v1/$metadata#ServiceRoot.ServiceRoot", + "@odata.id": "/redfish/v1", + "@odata.type": "#ServiceRoot.v1_6_0.ServiceRoot", + "AccountService": { + "@odata.id": "/redfish/v1/AccountService" + }, + "CertificateService": { + "@odata.id": "/redfish/v1/CertificateService" + }, + "Chassis": { + "@odata.id": "/redfish/v1/Chassis" + }, + "Description": "Root Service", + "EventService": { + "@odata.id": "/redfish/v1/EventService" + }, + "Fabrics": { + "@odata.id": "/redfish/v1/Fabrics" + }, + "Id": "RootService", + "JobService": { + "@odata.id": "/redfish/v1/JobService" + }, + "JsonSchemas": { + "@odata.id": "/redfish/v1/JsonSchemas" + }, + "Links": { + "Sessions": { + "@odata.id": "/redfish/v1/SessionService/Sessions" + } + }, + "Managers": { + "@odata.id": "/redfish/v1/Managers" + }, + "Name": "Root Service", + "Oem": { + "Dell": { + "@odata.context": "/redfish/v1/$metadata#DellServiceRoot.DellServiceRoot", + "@odata.type": "#DellServiceRoot.v1_0_0.DellServiceRoot", + "IsBranded": 0, + "ManagerMACAddress": "d0:8e:79:bb:3e:ea", + "ServiceTag": "FOOBAR" + } + }, + "Product": "Integrated Dell Remote Access Controller", + "ProtocolFeaturesSupported": { + "ExcerptQuery": false, + "ExpandQuery": { + "ExpandAll": true, + "Levels": true, + "Links": true, + "MaxLevels": 1, + "NoLinks": true + }, + "FilterQuery": true, + "OnlyMemberQuery": true, + "SelectQuery": true + }, + "RedfishVersion": "1.9.0", + "Registries": { + "@odata.id": "/redfish/v1/Registries" + }, + "SessionService": { + "@odata.id": "/redfish/v1/SessionService" + }, + "Systems": { + "@odata.id": "/redfish/v1/Systems" + }, + "Tasks": { + "@odata.id": "/redfish/v1/TaskService" + }, + "TelemetryService": { + "@odata.id": "/redfish/v1/TelemetryService" + }, + "UpdateService": { + "@odata.id": "/redfish/v1/UpdateService" + }, + "Vendor": "Dell" +} \ No newline at end of file diff --git a/providers/redfish/fixtures/v1/dell/system.embedded.1.json b/internal/redfishwrapper/fixtures/dell/system.embedded.1.json similarity index 100% rename from providers/redfish/fixtures/v1/dell/system.embedded.1.json rename to internal/redfishwrapper/fixtures/dell/system.embedded.1.json diff --git a/internal/redfishwrapper/fixtures/dell/systems.json b/internal/redfishwrapper/fixtures/dell/systems.json new file mode 100644 index 00000000..1611fec8 --- /dev/null +++ b/internal/redfishwrapper/fixtures/dell/systems.json @@ -0,0 +1,13 @@ +{ + "@odata.context": "/redfish/v1/$metadata#ComputerSystemCollection.ComputerSystemCollection", + "@odata.id": "/redfish/v1/Systems", + "@odata.type": "#ComputerSystemCollection.ComputerSystemCollection", + "Description": "Collection of Computer Systems", + "Members": [ + { + "@odata.id": "/redfish/v1/Systems/System.Embedded.1" + } + ], + "Members@odata.count": 1, + "Name": "Computer System Collection" +} \ No newline at end of file diff --git a/providers/redfish/bios_test.go b/providers/redfish/bios_test.go deleted file mode 100644 index a4cd7a91..00000000 --- a/providers/redfish/bios_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package redfish - -import ( - "context" - "encoding/json" - "fmt" - "io" - "log" - "os" - "testing" - - "github.com/stretchr/testify/assert" -) - -func Test_GetBiosConfiguration(t *testing.T) { - fixturePath := fixturesDir + "/v1/dell/bios.json" - fh, err := os.Open(fixturePath) - if err != nil { - log.Fatalf("%s, failed to open fixture: %s", err.Error(), fixturePath) - } - - defer fh.Close() - - b, err := io.ReadAll(fh) - if err != nil { - log.Fatalf("%s, failed to read fixture: %s", err.Error(), fixturePath) - } - - var bios map[string]any - err = json.Unmarshal([]byte(b), &bios) - if err != nil { - log.Fatalf("%s, failed to unmarshal fixture: %s", err.Error(), fixturePath) - } - - expectedBiosConfig := make(map[string]string) - for k, v := range bios["Attributes"].(map[string]any) { - expectedBiosConfig[k] = fmt.Sprintf("%v", v) - } - - tests := []struct { - testName string - expectedBiosConfig map[string]string - }{ - { - "GetBiosConfiguration", - expectedBiosConfig, - }, - } - - for _, tc := range tests { - t.Run(tc.testName, func(t *testing.T) { - biosConfig, err := mockClient.GetBiosConfiguration(context.TODO()) - assert.Nil(t, err) - assert.Equal(t, tc.expectedBiosConfig, biosConfig) - }) - } -} From 63f4d53090e793d71797b0332951ec3d89bcec45 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 11:21:00 +0100 Subject: [PATCH 02/11] redfishwrapper/firmware: lets not strip the JID_ prefix, since the method should be generic --- internal/redfishwrapper/firmware.go | 5 ----- internal/redfishwrapper/firmware_test.go | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/internal/redfishwrapper/firmware.go b/internal/redfishwrapper/firmware.go index 00c6031f..9fdf8c7d 100644 --- a/internal/redfishwrapper/firmware.go +++ b/internal/redfishwrapper/firmware.go @@ -192,11 +192,6 @@ func taskIDFromLocationHeader(uri string) (taskID string, err error) { uri = strings.TrimSuffix(uri, "/") switch { - // idracs return /redfish/v1/TaskService/Tasks/JID_467696020275 - case strings.Contains(uri, "JID_"): - taskID = strings.Split(uri, "JID_")[1] - return taskID, nil - // OpenBMC returns /redfish/v1/TaskService/Tasks/12/Monitor case strings.Contains(uri, "/Tasks/") && strings.HasSuffix(uri, "/Monitor"): taskIDPart := strings.Split(uri, "/Tasks/")[1] diff --git a/internal/redfishwrapper/firmware_test.go b/internal/redfishwrapper/firmware_test.go index db371e4e..0929d0a7 100644 --- a/internal/redfishwrapper/firmware_test.go +++ b/internal/redfishwrapper/firmware_test.go @@ -263,7 +263,7 @@ func TestTaskIDFromLocationHeader(t *testing.T) { { name: "task URI with JID", uri: "http://foo/redfish/v1/TaskService/Tasks/JID_12345", - expectedID: "12345", + expectedID: "JID_12345", expectedErr: nil, }, { From 9f0b43941002ee6b33846ba1731abe1ec7bc032e Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 13:44:28 +0100 Subject: [PATCH 03/11] bmc/firmware: initialize metadata object properly --- bmc/firmware.go | 40 ++++++++++++++++++++-------------------- bmc/firmware_test.go | 1 + 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/bmc/firmware.go b/bmc/firmware.go index b068df0a..02786a41 100644 --- a/bmc/firmware.go +++ b/bmc/firmware.go @@ -37,7 +37,7 @@ type firmwareInstallerProvider struct { // firmwareInstall uploads and initiates firmware update for the component func firmwareInstall(ctx context.Context, component, operationApplyTime string, forceInstall bool, reader io.Reader, generic []firmwareInstallerProvider) (taskID string, metadata Metadata, err error) { - var metadataLocal Metadata + metadata = newMetadata() for _, elem := range generic { if elem.FirmwareInstaller == nil { @@ -49,7 +49,7 @@ func firmwareInstall(ctx context.Context, component, operationApplyTime string, return taskID, metadata, err default: - metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name) + metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name) taskID, vErr := elem.FirmwareInstall(ctx, component, operationApplyTime, forceInstall, reader) if vErr != nil { err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name)) @@ -57,12 +57,12 @@ func firmwareInstall(ctx context.Context, component, operationApplyTime string, continue } - metadataLocal.SuccessfulProvider = elem.name - return taskID, metadataLocal, nil + metadata.SuccessfulProvider = elem.name + return taskID, metadata, nil } } - return taskID, metadataLocal, multierror.Append(err, errors.New("failure in FirmwareInstall")) + return taskID, metadata, multierror.Append(err, errors.New("failure in FirmwareInstall")) } // FirmwareInstallFromInterfaces identifies implementations of the FirmwareInstaller interface and passes the found implementations to the firmwareInstall() wrapper @@ -118,7 +118,7 @@ type firmwareInstallVerifierProvider struct { // firmwareInstallStatus returns the status of the firmware install process func firmwareInstallStatus(ctx context.Context, installVersion, component, taskID string, generic []firmwareInstallVerifierProvider) (status string, metadata Metadata, err error) { - var metadataLocal Metadata + metadata = newMetadata() for _, elem := range generic { if elem.FirmwareInstallVerifier == nil { @@ -130,7 +130,7 @@ func firmwareInstallStatus(ctx context.Context, installVersion, component, taskI return status, metadata, err default: - metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name) + metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name) status, vErr := elem.FirmwareInstallStatus(ctx, installVersion, component, taskID) if vErr != nil { err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name)) @@ -138,12 +138,12 @@ func firmwareInstallStatus(ctx context.Context, installVersion, component, taskI continue } - metadataLocal.SuccessfulProvider = elem.name - return status, metadataLocal, nil + metadata.SuccessfulProvider = elem.name + return status, metadata, nil } } - return status, metadataLocal, multierror.Append(err, errors.New("failure in FirmwareInstallStatus")) + return status, metadata, multierror.Append(err, errors.New("failure in FirmwareInstallStatus")) } // FirmwareInstallStatusFromInterfaces identifies implementations of the FirmwareInstallVerifier interface and passes the found implementations to the firmwareInstallStatus() wrapper. @@ -196,7 +196,7 @@ type firmwareInstallerWithOptionsProvider struct { // firmwareInstallUploaded uploads and initiates firmware update for the component func firmwareInstallUploaded(ctx context.Context, component, uploadTaskID string, generic []firmwareInstallerWithOptionsProvider) (installTaskID string, metadata Metadata, err error) { - var metadataLocal Metadata + metadata = newMetadata() for _, elem := range generic { if elem.FirmwareInstallerUploaded == nil { @@ -208,7 +208,7 @@ func firmwareInstallUploaded(ctx context.Context, component, uploadTaskID string return installTaskID, metadata, err default: - metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name) + metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name) var vErr error installTaskID, vErr = elem.FirmwareInstallUploaded(ctx, component, uploadTaskID) if vErr != nil { @@ -217,12 +217,12 @@ func firmwareInstallUploaded(ctx context.Context, component, uploadTaskID string continue } - metadataLocal.SuccessfulProvider = elem.name - return installTaskID, metadataLocal, nil + metadata.SuccessfulProvider = elem.name + return installTaskID, metadata, nil } } - return installTaskID, metadataLocal, multierror.Append(err, errors.New("failure in FirmwareInstallUploaded")) + return installTaskID, metadata, multierror.Append(err, errors.New("failure in FirmwareInstallUploaded")) } // FirmwareInstallerUploadedFromInterfaces identifies implementations of the FirmwareInstallUploaded interface and passes the found implementations to the firmwareInstallUploaded() wrapper @@ -294,7 +294,7 @@ func FirmwareInstallStepsFromInterfaces(ctx context.Context, component string, g } func firmwareInstallSteps(ctx context.Context, component string, generic []firmwareInstallStepsGetterProvider) (steps []constants.FirmwareInstallStep, metadata Metadata, err error) { - var metadataLocal Metadata + metadata = newMetadata() for _, elem := range generic { if elem.FirmwareInstallStepsGetter == nil { @@ -306,7 +306,7 @@ func firmwareInstallSteps(ctx context.Context, component string, generic []firmw return steps, metadata, err default: - metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name) + metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name) steps, vErr := elem.FirmwareInstallSteps(ctx, component) if vErr != nil { err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name)) @@ -314,12 +314,12 @@ func firmwareInstallSteps(ctx context.Context, component string, generic []firmw continue } - metadataLocal.SuccessfulProvider = elem.name - return steps, metadataLocal, nil + metadata.SuccessfulProvider = elem.name + return steps, metadata, nil } } - return steps, metadataLocal, multierror.Append(err, errors.New("failure in FirmwareInstallSteps")) + return steps, metadata, multierror.Append(err, errors.New("failure in FirmwareInstallSteps")) } type FirmwareUploader interface { diff --git a/bmc/firmware_test.go b/bmc/firmware_test.go index 0756bcd2..be19f222 100644 --- a/bmc/firmware_test.go +++ b/bmc/firmware_test.go @@ -10,6 +10,7 @@ import ( "github.com/bmc-toolbox/bmclib/v2/constants" bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/common" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" ) From 37d8981f180cd5d8ebc3ab753a0831e551904560 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 14:32:29 +0100 Subject: [PATCH 04/11] bmc/firmware: defines interface to upload and install firmware in the same method --- bmc/firmware.go | 77 +++++++++++++++++++++++++++++++++- bmc/firmware_test.go | 93 ++++++++++++++++++++++++++++++++++++++++++ client.go | 11 +++++ providers/providers.go | 5 ++- 4 files changed, 184 insertions(+), 2 deletions(-) diff --git a/bmc/firmware.go b/bmc/firmware.go index 02786a41..523ade73 100644 --- a/bmc/firmware.go +++ b/bmc/firmware.go @@ -175,7 +175,82 @@ func FirmwareInstallStatusFromInterfaces(ctx context.Context, installVersion, co return firmwareInstallStatus(ctx, installVersion, component, taskID, implementations) } -// FirmwareInstallerWithOpts defines an interface to install firmware that was previously uploaded with FirmwareUpload +// FirmwareInstallProvider defines an interface to upload and initiate a firmware install in the same implementation method +// +// Its intended to deprecate the FirmwareInstall interface +type FirmwareInstallProvider interface { + // FirmwareInstallUploadAndInitiate uploads _and_ initiates the firmware install process. + // + // return values: + // taskID - A taskID is returned if the update process on the BMC returns an identifier for the update process. + FirmwareInstallUploadAndInitiate(ctx context.Context, component string, file *os.File) (taskID string, err error) +} + +// firmwareInstallProvider is an internal struct to correlate an implementation/provider and its name +type firmwareInstallProvider struct { + name string + FirmwareInstallProvider +} + +// firmwareInstall uploads and initiates firmware update for the component +func firmwareInstallUploadAndInitiate(ctx context.Context, component string, file *os.File, generic []firmwareInstallProvider) (taskID string, metadata Metadata, err error) { + metadata = newMetadata() + + for _, elem := range generic { + if elem.FirmwareInstallProvider == nil { + continue + } + select { + case <-ctx.Done(): + err = multierror.Append(err, ctx.Err()) + + return taskID, metadata, err + default: + metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name) + taskID, vErr := elem.FirmwareInstallUploadAndInitiate(ctx, component, file) + if vErr != nil { + err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name)) + metadata.FailedProviderDetail[elem.name] = err.Error() + continue + } + metadata.SuccessfulProvider = elem.name + return taskID, metadata, nil + } + } + + return taskID, metadata, multierror.Append(err, errors.New("failure in FirmwareInstallUploadAndInitiate")) +} + +// FirmwareInstallUploadAndInitiateFromInterfaces identifies implementations of the FirmwareInstallProvider interface and passes the found implementations to the firmwareInstallUploadAndInitiate() wrapper +func FirmwareInstallUploadAndInitiateFromInterfaces(ctx context.Context, component string, file *os.File, generic []interface{}) (taskID string, metadata Metadata, err error) { + metadata = newMetadata() + + implementations := make([]firmwareInstallProvider, 0) + for _, elem := range generic { + temp := firmwareInstallProvider{name: getProviderName(elem)} + switch p := elem.(type) { + case FirmwareInstallProvider: + temp.FirmwareInstallProvider = p + implementations = append(implementations, temp) + default: + e := fmt.Sprintf("not a FirmwareInstallProvider implementation: %T", p) + err = multierror.Append(err, errors.New(e)) + } + } + if len(implementations) == 0 { + return taskID, metadata, multierror.Append( + err, + errors.Wrap( + bmclibErrs.ErrProviderImplementation, + ("no FirmwareInstallProvider implementations found"), + ), + ) + } + + return firmwareInstallUploadAndInitiate(ctx, component, file, implementations) +} + +// FirmwareInstallerUploaded defines an interface to install firmware that was previously uploaded with FirmwareUpload type FirmwareInstallerUploaded interface { // FirmwareInstallUploaded uploads firmware update payload to the BMC returning the firmware install task ID // diff --git a/bmc/firmware_test.go b/bmc/firmware_test.go index be19f222..dd6f29b0 100644 --- a/bmc/firmware_test.go +++ b/bmc/firmware_test.go @@ -205,6 +205,99 @@ func TestFirmwareInstallStatusFromInterfaces(t *testing.T) { } } +type firmwareInstallUploadAndInitiateTester struct { + returnTaskID string + returnError error +} + +func (f *firmwareInstallUploadAndInitiateTester) FirmwareInstallUploadAndInitiate(ctx context.Context, component string, file *os.File) (taskID string, err error) { + return f.returnTaskID, f.returnError +} + +func (r *firmwareInstallUploadAndInitiateTester) Name() string { + return "foo" +} + +func TestFirmwareInstallUploadAndInitiate(t *testing.T) { + testCases := []struct { + testName string + component string + file *os.File + returnTaskID string + returnError error + ctxTimeout time.Duration + providerName string + providersAttempted int + }{ + {"success with metadata", "componentA", &os.File{}, "1234", nil, 5 * time.Second, "foo", 1}, + {"failure with metadata", "componentB", &os.File{}, "1234", errors.New("failed to upload and initiate"), 5 * time.Second, "foo", 1}, + {"failure with context timeout", "componentC", &os.File{}, "", context.DeadlineExceeded, 1 * time.Nanosecond, "foo", 1}, + } + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + testImplementation := &firmwareInstallUploadAndInitiateTester{returnTaskID: tc.returnTaskID, returnError: tc.returnError} + if tc.ctxTimeout == 0 { + tc.ctxTimeout = time.Second * 3 + } + ctx, cancel := context.WithTimeout(context.Background(), tc.ctxTimeout) + defer cancel() + taskID, metadata, err := firmwareInstallUploadAndInitiate(ctx, tc.component, tc.file, []firmwareInstallProvider{{tc.providerName, testImplementation}}) + if tc.returnError != nil { + assert.ErrorIs(t, err, tc.returnError) + return + } + + if err != nil { + t.Fatal(err) + } + assert.Equal(t, tc.returnTaskID, taskID) + assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) + assert.Equal(t, tc.providersAttempted, len(metadata.ProvidersAttempted)) + }) + } +} + +func TestFirmwareInstallUploadAndInitiateFromInterfaces(t *testing.T) { + testCases := []struct { + testName string + component string + file *os.File + returnTaskID string + returnError error + providerName string + badImplementation bool + }{ + {"success with metadata", "componentA", &os.File{}, "1234", nil, "foo", false}, + {"failure with bad implementation", "componentB", &os.File{}, "1234", bmclibErrs.ErrProviderImplementation, "foo", true}, + } + + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + var generic []interface{} + if tc.badImplementation { + badImplementation := struct{}{} + generic = []interface{}{&badImplementation} + } else { + testImplementation := &firmwareInstallUploadAndInitiateTester{returnTaskID: tc.returnTaskID, returnError: tc.returnError} + generic = []interface{}{testImplementation} + } + taskID, metadata, err := FirmwareInstallUploadAndInitiateFromInterfaces(context.Background(), tc.component, tc.file, generic) + if tc.returnError != nil { + assert.ErrorIs(t, err, tc.returnError) + return + } + + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, tc.returnTaskID, taskID) + assert.Equal(t, tc.providerName, metadata.SuccessfulProvider) + }) + } +} + type firmwareInstallUploadTester struct { TaskID string Err error diff --git a/client.go b/client.go index fe9b3fd7..161f0ea7 100644 --- a/client.go +++ b/client.go @@ -656,3 +656,14 @@ func (c *Client) FirmwareInstallUploaded(ctx context.Context, component, uploadV return installTaskID, err } + +func (c *Client) FirmwareInstallUploadAndInitiate(ctx context.Context, component string, file *os.File) (taskID string, err error) { + ctx, span := c.traceprovider.Tracer(pkgName).Start(ctx, "FirmwareInstallUploadAndInitiate") + defer span.End() + + taskID, metadata, err := bmc.FirmwareInstallUploadAndInitiateFromInterfaces(ctx, component, file, c.registry().GetDriverInterfaces()) + c.setMetadata(metadata) + metadata.RegisterSpanAttributes(c.Auth.Host, span) + + return taskID, err +} diff --git a/providers/providers.go b/providers/providers.go index b7eb0518..41b90f1a 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -44,7 +44,7 @@ const ( FeatureClearSystemEventLog registrar.Feature = "clearsystemeventlog" // FeatureFirmwareInstallSteps means an implementation returns the steps part of the firmware update process. - FeatureFirmwareInstallSteps registrar.Feature = "firmwareinstallactions" + FeatureFirmwareInstallSteps registrar.Feature = "firmwareinstallsteps" // FeatureFirmwareUpload means an implementation that uploads firmware for installing. FeatureFirmwareUpload registrar.Feature = "firmwareupload" @@ -54,4 +54,7 @@ const ( // FeatureFirmwareTaskStatus identifies an implementaton that can return the status of a firmware upload/install task. FeatureFirmwareTaskStatus registrar.Feature = "firmwaretaskstatus" + + // FeatureFirmwareUploadInitiateInstall identifies an implementation that uploads firmware _and_ initiates the install process. + FeatureFirmwareUploadInitiateInstall registrar.Feature = "uploadandinitiateinstall" ) From e14321b647739d9747225139e359c08fad4cf960 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 14:36:51 +0100 Subject: [PATCH 05/11] providers/dell: adds a helper method and implements Inventory(), PowerSet(), PowerStateGet() methods --- providers/dell/idrac.go | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index f1282f5e..c47dee3b 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -40,12 +40,8 @@ var ( ) type Config struct { - HttpClient *http.Client - Port string - // VersionsNotCompatible is the list of incompatible redfish versions. - // - // With this option set, The bmclib.Registry.FilterForCompatible(ctx) method will not proceed on - // devices with the given redfish version(s). + HttpClient *http.Client + Port string VersionsNotCompatible []string RootCAs *x509.CertPool UseBasicAuth bool @@ -126,19 +122,26 @@ func (c *Conn) Open(ctx context.Context) (err error) { // because this uses the redfish interface and the redfish interface // is available across various BMC vendors, we verify the device we're connected to is dell. - manufacturer, err := c.deviceManufacturer(ctx) - if err != nil { + if err := c.deviceSupported(ctx); err != nil { if er := c.redfishwrapper.Close(ctx); er != nil { return fmt.Errorf("%v: %w", err, er) } + return err } - if !strings.Contains(strings.ToLower(manufacturer), common.VendorDell) { - if er := c.redfishwrapper.Close(ctx); er != nil { - return fmt.Errorf("%v: %w", err, er) - } - return bmclibErrs.ErrIncompatibleProvider + return nil +} + +func (c *Conn) deviceSupported(ctx context.Context) error { + manufacturer, err := c.deviceManufacturer(ctx) + if err != nil { + return err + } + + m := strings.ToLower(manufacturer) + if !strings.Contains(m, common.VendorDell) { + return errors.Wrap(bmclibErrs.ErrIncompatibleProvider, m) } return nil @@ -192,6 +195,16 @@ func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { return c.redfishwrapper.SystemPowerStatus(ctx) } +// PowerSet sets the power state of a server +func (c *Conn) PowerSet(ctx context.Context, state string) (ok bool, err error) { + return c.redfishwrapper.PowerSet(ctx, state) +} + +// Inventory collects hardware inventory and install firmware information +func (c *Conn) Inventory(ctx context.Context) (device *common.Device, err error) { + return c.redfishwrapper.Inventory(ctx, false) +} + var errManufacturerUnknown = errors.New("error identifying device manufacturer") // deviceManufacturer returns the device manufacturer and model attributes From ba60fa029f499d68f81e8c56180ff3d308d16ecc Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 14:39:27 +0100 Subject: [PATCH 06/11] providers/dell: Implements FirmwareInstallSteps(), FirmwareInstallUploadedAndInitiate(), FirmwareInstallStatus() methods --- providers/dell/firmware.go | 235 ++++++++++++++++++++++++++++++++ providers/dell/firmware_test.go | 94 +++++++++++++ providers/dell/idrac.go | 6 + 3 files changed, 335 insertions(+) create mode 100644 providers/dell/firmware.go create mode 100644 providers/dell/firmware_test.go diff --git a/providers/dell/firmware.go b/providers/dell/firmware.go new file mode 100644 index 00000000..f311ea73 --- /dev/null +++ b/providers/dell/firmware.go @@ -0,0 +1,235 @@ +package dell + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os" + "strings" + "time" + + "github.com/bmc-toolbox/bmclib/v2/constants" + bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" + rfw "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" + "github.com/bmc-toolbox/common" + "github.com/pkg/errors" + "github.com/stmcginnis/gofish/redfish" +) + +var ( + ErrUnsupportedHardware = errors.New("hardware not supported") +) + +// bmc client interface implementations methods +func (c *Conn) FirmwareInstallSteps(ctx context.Context, component string) ([]constants.FirmwareInstallStep, error) { + if err := c.deviceSupported(ctx); err != nil { + return nil, errors.Wrap(ErrUnsupportedHardware, err.Error()) + } + + return []constants.FirmwareInstallStep{ + constants.FirmwareInstallStepUploadInitiateInstall, + constants.FirmwareInstallStepInstallStatus, + }, nil +} + +func (c *Conn) FirmwareInstallUploadAndInitiate(ctx context.Context, component string, file *os.File) (taskID string, err error) { + if err := c.deviceSupported(ctx); err != nil { + return "", errors.Wrap(ErrUnsupportedHardware, err.Error()) + } + + // // expect atleast 5 minutes left in the deadline to proceed with the upload + d, _ := ctx.Deadline() + if time.Until(d) < 10*time.Minute { + return "", errors.New("remaining context deadline insufficient to perform update: " + time.Until(d).String()) + } + + // list current tasks on BMC + tasks, err := c.redfishwrapper.Tasks(ctx) + if err != nil { + return "", errors.Wrap(err, "error listing bmc redfish tasks") + } + + // validate a new firmware install task can be queued + if err := c.checkQueueability(component, tasks); err != nil { + return "", errors.Wrap(bmcliberrs.ErrFirmwareInstall, err.Error()) + } + + params := &rfw.RedfishUpdateServiceParameters{ + Targets: []string{}, + OperationApplyTime: constants.OnReset, + Oem: []byte(`{}`), + } + + return c.redfishwrapper.FirmwareUpload(ctx, file, params) +} + +// checkQueueability returns an error if an existing firmware task is in progress for the given component +func (c *Conn) checkQueueability(component string, tasks []*redfish.Task) error { + errTaskActive := errors.New("A firmware job was found active for component: " + component) + + // Redfish on the Idrac names firmware install tasks in this manner. + taskNameMap := map[string]string{ + common.SlugBIOS: "Firmware Update: BIOS", + common.SlugBMC: "Firmware Update: iDRAC with Lifecycle Controller", + common.SlugNIC: "Firmware Update: Network", + common.SlugDrive: "Firmware Update: Serial ATA", + common.SlugStorageController: "Firmware Update: SAS RAID", + } + + for _, t := range tasks { + if t.Name == taskNameMap[strings.ToUpper(component)] { + // taskInfo returned in error if any. + taskInfo := fmt.Sprintf("id: %s, state: %s, status: %s", t.ID, t.TaskState, t.TaskStatus) + + // convert redfish task state to bmclib state + convstate := c.redfishwrapper.ConvertTaskState(string(t.TaskState)) + // check if task is active based on converted state + active, err := c.redfishwrapper.TaskStateActive(convstate) + if err != nil { + return errors.Wrap(err, taskInfo) + } + + if active { + return errors.Wrap(errTaskActive, taskInfo) + } + } + } + + return nil +} + +// FirmwareTaskStatus returns the status of a firmware related task queued on the BMC. +func (c *Conn) FirmwareTaskStatus(ctx context.Context, kind constants.FirmwareInstallStep, component, taskID, installVersion string) (state constants.TaskState, status string, err error) { + if err := c.deviceSupported(ctx); err != nil { + return "", "", errors.Wrap(ErrUnsupportedHardware, err.Error()) + } + + // Dell jobs are turned into Redfish tasks on the idrac + // once the Redfish task completes successfully, the Redfish task is purged, + // and the dell Job stays around. + task, err := c.redfishwrapper.Task(ctx, taskID) + if err != nil { + if errors.Is(err, bmcliberrs.ErrTaskNotFound) { + return c.statusFromJob(taskID) + } + + return "", "", err + } + + return c.statusFromTaskOem(taskID, task.Oem) +} + +func (c *Conn) statusFromJob(taskID string) (constants.TaskState, string, error) { + job, err := c.job(taskID) + if err != nil { + return "", "", err + } + + s := strings.ToLower(job.JobState) + state := c.redfishwrapper.ConvertTaskState(s) + + status := fmt.Sprintf( + "id: %s, state: %s, status: %s, progress: %d%%", + taskID, + job.JobState, + job.Message, + job.PercentComplete, + ) + + return state, status, nil +} + +func (c *Conn) statusFromTaskOem(taskID string, oem json.RawMessage) (constants.TaskState, string, error) { + data, err := convFirmwareTaskOem(oem) + if err != nil { + return "", "", err + } + + s := strings.ToLower(data.Dell.JobState) + state := c.redfishwrapper.ConvertTaskState(s) + + status := fmt.Sprintf( + "id: %s, state: %s, status: %s, progress: %d%%", + taskID, + data.Dell.JobState, + data.Dell.Message, + data.Dell.PercentComplete, + ) + + return state, status, nil +} + +func (c *Conn) job(jobID string) (*Dell, error) { + errLookup := errors.New("error querying dell job: " + jobID) + + endpoint := "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs/" + jobID + resp, err := c.redfishwrapper.Get(endpoint) + if err != nil { + return nil, errors.Wrap(errLookup, err.Error()) + } + + if resp.StatusCode != 200 { + return nil, errors.Wrap(errLookup, "unexpected status code: "+resp.Status) + } + + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, errors.Wrap(errLookup, err.Error()) + } + + dell := &Dell{} + err = json.Unmarshal(body, &dell) + if err != nil { + return nil, errors.Wrap(errLookup, err.Error()) + } + + return dell, nil +} + +type oem struct { + Dell `json:"Dell"` +} + +type Dell struct { + OdataType string `json:"@odata.type"` + CompletionTime interface{} `json:"CompletionTime"` + Description string `json:"Description"` + EndTime string `json:"EndTime"` + ID string `json:"Id"` + JobState string `json:"JobState"` + JobType string `json:"JobType"` + Message string `json:"Message"` + MessageArgs []interface{} `json:"MessageArgs"` + MessageID string `json:"MessageId"` + Name string `json:"Name"` + PercentComplete int `json:"PercentComplete"` + StartTime string `json:"StartTime"` + TargetSettingsURI interface{} `json:"TargetSettingsURI"` +} + +func convFirmwareTaskOem(oemdata json.RawMessage) (oem, error) { + oem := oem{} + + errTaskOem := errors.New("error in Task Oem data: " + string(oemdata)) + + if len(oemdata) == 0 || string(oemdata) == `{}` { + return oem, errors.Wrap(errTaskOem, "empty oem data") + } + + if err := json.Unmarshal(oemdata, &oem); err != nil { + return oem, errors.Wrap(errTaskOem, "failed to unmarshal: "+err.Error()) + } + + if oem.Dell.Description == "" || oem.Dell.JobState == "" { + return oem, errors.Wrap(errTaskOem, "invalid oem data") + } + + if oem.Dell.JobType != "FirmwareUpdate" { + return oem, errors.Wrap(errTaskOem, "unexpected job type: "+oem.Dell.JobType) + } + + return oem, nil +} diff --git a/providers/dell/firmware_test.go b/providers/dell/firmware_test.go new file mode 100644 index 00000000..77f099e2 --- /dev/null +++ b/providers/dell/firmware_test.go @@ -0,0 +1,94 @@ +package dell + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestConvFirmwareTaskOem(t *testing.T) { + testCases := []struct { + name string + oemdata []byte + expectedJob oem + expectedErr string + }{ + { + name: "Valid OEM data", + oemdata: []byte(`{ + "Dell": { + "@odata.type": "#DellJob.v1_4_0.DellJob", + "CompletionTime": null, + "Description": "Job Instance", + "EndTime": "TIME_NA", + "Id": "JID_005950769310", + "JobState": "Scheduled", + "JobType": "FirmwareUpdate", + "Message": "Task successfully scheduled.", + "MessageArgs": [], + "MessageId": "IDRAC.2.8.JCP001", + "Name": "Firmware Update: BIOS", + "PercentComplete": 0, + "StartTime": "TIME_NOW", + "TargetSettingsURI": null + } + }`), + expectedJob: oem{ + Dell{ + OdataType: "#DellJob.v1_4_0.DellJob", + CompletionTime: nil, + Description: "Job Instance", + EndTime: "TIME_NA", + ID: "JID_005950769310", + JobState: "Scheduled", + JobType: "FirmwareUpdate", + Message: "Task successfully scheduled.", + MessageArgs: []interface{}{}, + MessageID: "IDRAC.2.8.JCP001", + Name: "Firmware Update: BIOS", + PercentComplete: 0, + StartTime: "TIME_NOW", + TargetSettingsURI: nil, + }, + }, + expectedErr: "", + }, + { + name: "Empty OEM data", + oemdata: []byte(`{}`), + expectedJob: oem{}, + expectedErr: "empty oem data", + }, + { + name: "Invalid OEM data", + oemdata: []byte(`{"InvalidKey": "InvalidValue"}`), + expectedJob: oem{}, + expectedErr: "invalid oem data", + }, + { + name: "Unexpected job type", + oemdata: []byte(`{ + "Dell": { + "JobType": "InvalidJobType", + "Description": "Job Instance", + "JobState": "Scheduled" + } + }`), + expectedJob: oem{}, + expectedErr: "unexpected job type", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + job, err := convFirmwareTaskOem(tc.oemdata) + if tc.expectedErr == "" { + assert.NoError(t, err) + assert.Equal(t, tc.expectedJob, job) + } else { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErr) + } + }) + } +} diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index c47dee3b..22613847 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -36,6 +36,12 @@ var ( // Features implemented by dell redfish Features = registrar.Features{ providers.FeatureScreenshot, + providers.FeaturePowerState, + providers.FeaturePowerSet, + providers.FeatureFirmwareInstallSteps, + providers.FeatureFirmwareUploadInitiateInstall, + providers.FeatureFirmwareTaskStatus, + providers.FeatureInventoryRead, } ) From 6ee715bd68be589b65b6992d653be03831d15e98 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 14:46:15 +0100 Subject: [PATCH 07/11] go: update gofish to include Task Oem data fix https://github.com/stmcginnis/gofish/pull/289 --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 2403a48b..25b46997 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/rs/zerolog v1.31.0 github.com/sirupsen/logrus v1.9.3 - github.com/stmcginnis/gofish v0.14.1-0.20231018151402-dddaff9168fb + github.com/stmcginnis/gofish v0.15.1-0.20231121142100-22a60a77be91 github.com/stretchr/testify v1.8.4 go.opentelemetry.io/otel v1.20.0 go.opentelemetry.io/otel/trace v1.20.0 diff --git a/go.sum b/go.sum index c3e42e8c..75ce7331 100644 --- a/go.sum +++ b/go.sum @@ -61,6 +61,8 @@ github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= github.com/stmcginnis/gofish v0.14.1-0.20231018151402-dddaff9168fb h1:+BpzUuFIEAs71bTshedsUHAAq21VZWvuokbN9ABEQeQ= github.com/stmcginnis/gofish v0.14.1-0.20231018151402-dddaff9168fb/go.mod h1:BLDSFTp8pDlf/xDbLZa+F7f7eW0E/CHCboggsu8CznI= +github.com/stmcginnis/gofish v0.15.1-0.20231121142100-22a60a77be91 h1:WmABtU8y6kTgzoVUn3FWCQGAfyodve3uz3xno28BrRs= +github.com/stmcginnis/gofish v0.15.1-0.20231121142100-22a60a77be91/go.mod h1:BLDSFTp8pDlf/xDbLZa+F7f7eW0E/CHCboggsu8CznI= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= From 41cb5faf168aa63a3bbf11cf97b8f3eac1dd113b Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Tue, 28 Nov 2023 10:23:54 +0100 Subject: [PATCH 08/11] providers/redfish: task methods moved under redfishwrapper package --- providers/redfish/tasks_dell.go | 175 --------------------------- providers/redfish/tasks_dell_test.go | 40 ------ providers/redfish/tasks_test.go | 39 ------ 3 files changed, 254 deletions(-) delete mode 100644 providers/redfish/tasks_dell.go delete mode 100644 providers/redfish/tasks_dell_test.go delete mode 100644 providers/redfish/tasks_test.go diff --git a/providers/redfish/tasks_dell.go b/providers/redfish/tasks_dell.go deleted file mode 100644 index 0ecd92de..00000000 --- a/providers/redfish/tasks_dell.go +++ /dev/null @@ -1,175 +0,0 @@ -package redfish - -import ( - "encoding/json" - "io" - "strconv" - "strings" - - bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" - "github.com/bmc-toolbox/common" - "github.com/pkg/errors" - - gofishcommon "github.com/stmcginnis/gofish/common" - gofishrf "github.com/stmcginnis/gofish/redfish" -) - -// TODO: figure how this can be moved into the dell provider -// -// Dell specific redfish methods - -var ( - componentSlugDellJobName = map[string]string{ - common.SlugBIOS: "Firmware Update: BIOS", - common.SlugBMC: "Firmware Update: iDRAC with Lifecycle Controller", - common.SlugNIC: "Firmware Update: Network", - common.SlugDrive: "Firmware Update: Serial ATA", - common.SlugStorageController: "Firmware Update: SAS RAID", - } -) - -type dellJob struct { - PercentComplete int - OdataID string `json:"@odata.id"` - StartTime string - CompletionTime string - ID string - Message string - Name string - JobState string - JobType string -} - -type dellJobResponseData struct { - Members []*dellJob -} - -// dellJobID formats and returns taskID as a Dell Job ID -func dellJobID(id string) string { - if !strings.HasPrefix(id, "JID") { - return "JID_" + id - } - - return id -} - -func (c *Conn) getDellFirmwareInstallTaskScheduled(slug string) (*gofishrf.Task, error) { - // get tasks by state - tasks, err := c.dellJobs("scheduled") - if err != nil { - return nil, err - } - - // filter to match the task Name based on the component slug - for _, task := range tasks { - if task.Name == componentSlugDellJobName[strings.ToUpper(slug)] { - return task, nil - } - } - - return nil, nil -} - -func (c *Conn) dellPurgeScheduledFirmwareInstallJob(slug string) error { - // get tasks by state - tasks, err := c.dellJobs("scheduled") - if err != nil { - return err - } - - // filter to match the task Name based on the component slug - for _, task := range tasks { - if task.Name == componentSlugDellJobName[strings.ToUpper(slug)] { - err = c.dellPurgeJob(task.ID) - if err != nil { - return err - } - } - } - - return nil -} - -func (c *Conn) dellPurgeJob(id string) error { - id = dellJobID(id) - - endpoint := "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs/" + id - - resp, err := c.redfishwrapper.Delete(endpoint) - if err != nil { - return errors.Wrap(bmcliberrs.ErrTaskPurge, err.Error()) - } - - if resp.StatusCode != 200 { - return errors.Wrap(bmcliberrs.ErrTaskPurge, "response code: "+resp.Status) - } - - return nil -} - -// dellFirmwareUpdateTaskStatus looks up the Dell Job and returns it as a redfish task object -func (c *Conn) dellJobAsRedfishTask(jobID string) (*gofishrf.Task, error) { - jobID = dellJobID(jobID) - - tasks, err := c.dellJobs("") - if err != nil { - return nil, err - } - - for _, task := range tasks { - if task.ID == jobID { - return task, nil - } - } - - return nil, errors.Wrap(bmcliberrs.ErrTaskNotFound, "task with ID not found: "+jobID) -} - -// dellJobs returns all dell jobs as redfish task objects -// state: optional -func (c *Conn) dellJobs(state string) ([]*gofishrf.Task, error) { - endpoint := "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs?$expand=*($levels=1)" - - resp, err := c.redfishwrapper.Get(endpoint) - if err != nil { - return nil, err - } - - if resp.StatusCode != 200 { - return nil, errors.New("dell jobs endpoint returned unexpected status code: " + strconv.Itoa(resp.StatusCode)) - } - - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - - data := dellJobResponseData{} - err = json.Unmarshal(body, &data) - if err != nil { - return nil, err - } - - tasks := []*gofishrf.Task{} - for _, job := range data.Members { - if state != "" && !strings.EqualFold(job.JobState, state) { - continue - } - - tasks = append(tasks, &gofishrf.Task{ - Entity: gofishcommon.Entity{ - ID: job.ID, - ODataID: job.OdataID, - Name: job.Name, - }, - Description: job.Name, - PercentComplete: job.PercentComplete, - StartTime: job.StartTime, - EndTime: job.CompletionTime, - TaskState: gofishrf.TaskState(job.JobState), - TaskStatus: gofishcommon.Health(job.Message), // abuse the TaskStatus to include any status message - }) - } - - return tasks, nil -} diff --git a/providers/redfish/tasks_dell_test.go b/providers/redfish/tasks_dell_test.go deleted file mode 100644 index bf1a376b..00000000 --- a/providers/redfish/tasks_dell_test.go +++ /dev/null @@ -1,40 +0,0 @@ -package redfish - -import ( - "net/http" - "testing" - - "github.com/stretchr/testify/assert" -) - -// handler registered in redfish_test.go -func dellJobs(w http.ResponseWriter, r *http.Request) { - if r.Method != "GET" { - w.WriteHeader(http.StatusNotFound) - } - - _, _ = w.Write(jsonResponse(r.RequestURI)) -} - -func Test_dellFirmwareUpdateTask(t *testing.T) { - // see fixtures/v1/dell/jobs.json for the job IDs - // completed job - status, err := mockClient.dellJobAsRedfishTask("467767920358") - if err != nil { - t.Fatal(err) - } - - assert.NotNil(t, status) - assert.Equal(t, "2022-03-08T16:02:33", status.EndTime) - assert.Equal(t, "2022-03-08T15:59:52", status.StartTime) - assert.Equal(t, 100, status.PercentComplete) - assert.Equal(t, "Completed", string(status.TaskState)) - assert.Equal(t, "Job completed successfully.", string(status.TaskStatus)) -} - -func Test_dellPurgeScheduledFirmwareInstallJob(t *testing.T) { - err := mockClient.dellPurgeScheduledFirmwareInstallJob("bios") - if err != nil { - t.Fatal(err) - } -} diff --git a/providers/redfish/tasks_test.go b/providers/redfish/tasks_test.go deleted file mode 100644 index 795ff670..00000000 --- a/providers/redfish/tasks_test.go +++ /dev/null @@ -1,39 +0,0 @@ -package redfish - -import ( - "context" - "errors" - "testing" - - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" -) - -func Test_activeTask(t *testing.T) { - _, err := mockClient.activeTask(context.TODO()) - // Current mocking should fail - if err == nil { - t.Fatal(err) - } -} - -func Test_GetTask(t *testing.T) { - var err error - - task, err := mockClient.GetTask("15") - if err != nil { - t.Fatal(err) - } - if task.TaskState != "TestState" { - t.Fatal("Wrong test state:", task.TaskState) - } - - // inexistent - task, err = mockClient.GetTask("151515") - if task != nil { - t.Fatal("Task should be nil, but got:", task) - } - if !errors.Is(err, bmclibErrs.ErrTaskNotFound) { - t.Fatal("err should be TaskNotFound:", err) - } - -} From 04eb8ca96994a0945927de367a29aa91e9b9efe7 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Tue, 28 Nov 2023 10:24:25 +0100 Subject: [PATCH 09/11] squash --- providers/redfish/tasks.go | 141 ------------------------------------- 1 file changed, 141 deletions(-) delete mode 100644 providers/redfish/tasks.go diff --git a/providers/redfish/tasks.go b/providers/redfish/tasks.go deleted file mode 100644 index 394f32e8..00000000 --- a/providers/redfish/tasks.go +++ /dev/null @@ -1,141 +0,0 @@ -package redfish - -import ( - "context" - "encoding/json" - "fmt" - "io" - "regexp" - "strings" - - "github.com/bmc-toolbox/bmclib/v2/constants" - bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors" - "github.com/pkg/errors" - gofishcommon "github.com/stmcginnis/gofish/common" - gofishrf "github.com/stmcginnis/gofish/redfish" -) - -func (c *Conn) activeTask(ctx context.Context) (*gofishrf.Task, error) { - resp, err := c.redfishwrapper.Get("/redfish/v1/TaskService/Tasks") - if err != nil { - return nil, err - } - if resp.StatusCode != 200 { - err = errors.Wrap( - bmclibErrs.ErrFirmwareInstallStatus, - "HTTP Error: "+fmt.Sprint(resp.StatusCode), - ) - - return nil, err - } - - data, _ := io.ReadAll(resp.Body) - resp.Body.Close() - - type TaskId struct { - OdataID string `json:"@odata.id"` - TaskState string - TaskStatus string - } - - type Tasks struct { - Members []TaskId - } - - var status Tasks - - err = json.Unmarshal(data, &status) - if err != nil { - return nil, err - } - - // For each task, check if it's running - // It's usually the latest that is running, so it would be faster to - // start by the end, but an easy way to do this is only available in go 1.21 - // for _, t := range slices.Reverse(status.Members) { // when go 1.21 - for _, t := range status.Members { - re := regexp.MustCompile("/redfish/v1/TaskService/Tasks/([0-9]+)") - taskmatch := re.FindSubmatch([]byte(t.OdataID)) - if len(taskmatch) < 1 { - continue - } - - tasknum := string(taskmatch[1]) - - task, err := c.GetTask(tasknum) - if err != nil { - continue - } - - if task.TaskState == "Running" { - return task, nil - } - } - - return nil, nil -} - -// GetFirmwareInstallTaskQueued returns the redfish task object for a queued update task -func (c *Conn) GetFirmwareInstallTaskQueued(ctx context.Context, component string) (*gofishrf.Task, error) { - vendor, _, err := c.redfishwrapper.DeviceVendorModel(ctx) - if err != nil { - return nil, errors.Wrap(err, "unable to determine device vendor, model attributes") - } - - var task *gofishrf.Task - - // check an update task for the component is currently scheduled - switch { - case strings.Contains(vendor, constants.Dell): - task, err = c.getDellFirmwareInstallTaskScheduled(component) - default: - task, err = c.activeTask(ctx) - } - - if err != nil { - return nil, err - } - - return task, nil -} - -// GetTask returns the current Task fir the given TaskID -func (c *Conn) GetTask(taskID string) (task *gofishrf.Task, err error) { - resp, err := c.redfishwrapper.Get("/redfish/v1/TaskService/Tasks/" + taskID) - if err != nil { - if strings.HasPrefix(err.Error(), "404") { - return nil, errors.Wrap(bmclibErrs.ErrTaskNotFound, "task with ID not found: "+taskID) - } - return nil, err - } - if resp.StatusCode != 200 { - err = errors.Wrap( - bmclibErrs.ErrFirmwareInstallStatus, - "HTTP Error: "+fmt.Sprint(resp.StatusCode), - ) - - return nil, err - } - - data, _ := io.ReadAll(resp.Body) - resp.Body.Close() - - type TaskStatus struct { - TaskState string - TaskStatus string - } - - var status TaskStatus - - err = json.Unmarshal(data, &status) - if err != nil { - return nil, err - } - - task = &gofishrf.Task{ - TaskState: gofishrf.TaskState(status.TaskState), - TaskStatus: gofishcommon.Health(status.TaskStatus), - } - - return task, err -} From cdc6f141cfa2ef5ff66eab78572a03cfe2ceb302 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Tue, 28 Nov 2023 10:25:03 +0100 Subject: [PATCH 10/11] providers/redfish: dell tests moved under dell provider --- providers/redfish/main_test.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/providers/redfish/main_test.go b/providers/redfish/main_test.go index 17a525a7..27aa2588 100644 --- a/providers/redfish/main_test.go +++ b/providers/redfish/main_test.go @@ -26,14 +26,10 @@ var ( // jsonResponse returns the fixture json response for a request URI func jsonResponse(endpoint string) []byte { jsonResponsesMap := map[string]string{ - "/redfish/v1/": fixturesDir + "/v1/serviceroot.json", - "/redfish/v1/UpdateService": fixturesDir + "/v1/updateservice.json", - "/redfish/v1/Systems": fixturesDir + "/v1/systems.json", - - "/redfish/v1/Systems/System.Embedded.1": fixturesDir + "/v1/dell/system.embedded.1.json", - "/redfish/v1/Systems/System.Embedded.1/Bios": fixturesDir + "/v1/dell/bios.json", - "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs?$expand=*($levels=1)": fixturesDir + "/v1/dell/jobs.json", - "/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs/JID_467762674724": fixturesDir + "/v1/dell/job_delete_ok.json", + "/redfish/v1/": fixturesDir + "/v1/serviceroot.json", + "/redfish/v1/UpdateService": fixturesDir + "/v1/updateservice.json", + "/redfish/v1/Systems": fixturesDir + "/v1/systems.json", + } fh, err := os.Open(jsonResponsesMap[endpoint]) @@ -57,8 +53,6 @@ func TestMain(m *testing.M) { handler := http.NewServeMux() handler.HandleFunc("/redfish/v1/", serviceRoot) handler.HandleFunc("/redfish/v1/SessionService/Sessions", sessionService) - handler.HandleFunc("/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs?$expand=*($levels=1)", dellJobs) - return httptest.NewTLSServer(handler) }() From 8c3aefb35142fb4b4bd9f34ae78a8b9aaeff0c09 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Tue, 28 Nov 2023 10:25:36 +0100 Subject: [PATCH 11/11] redfishwrapper: minor fix for test --- internal/redfishwrapper/main_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/redfishwrapper/main_test.go b/internal/redfishwrapper/main_test.go index b322015c..b1be7d67 100644 --- a/internal/redfishwrapper/main_test.go +++ b/internal/redfishwrapper/main_test.go @@ -36,6 +36,7 @@ var endpointFunc = func(t *testing.T, file string) http.HandlerFunc { // expect either GET or Delete methods if r.Method != http.MethodGet && r.Method != http.MethodDelete { w.WriteHeader(http.StatusNotFound) + return } _, _ = w.Write(mustReadFile(t, file))