From 37d8981f180cd5d8ebc3ab753a0831e551904560 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 27 Nov 2023 14:32:29 +0100 Subject: [PATCH] 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" )