From b584eaa8332084913a35f7f68f7c04835aef20f7 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Mon, 13 Nov 2023 11:59:08 +0100 Subject: [PATCH] outofband/bmc: bmclibv2 is now main, purge trace spans added into bmclib The trace spans are being moved into the bmclib library where its easier to encapsulate the methods within a span. --- internal/outofband/bmc.go | 112 ++++++++++-------------------- internal/outofband/bmc_helpers.go | 22 +++--- 2 files changed, 48 insertions(+), 86 deletions(-) diff --git a/internal/outofband/bmc.go b/internal/outofband/bmc.go index a88400b3..ce499477 100644 --- a/internal/outofband/bmc.go +++ b/internal/outofband/bmc.go @@ -7,22 +7,15 @@ import ( "time" "github.com/pkg/errors" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" - bmclibv2 "github.com/bmc-toolbox/bmclib/v2" - bmclibv2consts "github.com/bmc-toolbox/bmclib/v2/constants" - bmclibv2errs "github.com/bmc-toolbox/bmclib/v2/errors" + bmclib "github.com/bmc-toolbox/bmclib/v2" + bconsts "github.com/bmc-toolbox/bmclib/v2/constants" "github.com/bmc-toolbox/common" "github.com/metal-toolbox/flasher/internal/model" "github.com/sirupsen/logrus" ) -const ( - pkgName = "internal/outofband" -) - var ( // logoutTimeout is the timeout value when logging out of a bmc logoutTimeout = 1 * time.Minute @@ -42,16 +35,16 @@ var ( errBMCLogout = errors.New("bmc logout error") - ErrBMCQuery = errors.New("error occurred in bmc query") - ErrMaxBMCQueryAttempts = errors.New("reached maximum BMC query attempts") - ErrTaskNotFound = errors.New("task not found") - ErrFirmwareInstallFailed = errors.New("firmware install failed") - ErrFirmwareInstallStatusUnexpected = errors.New("firmware install status unexpected") + ErrBMCQuery = errors.New("error occurred in bmc query") + ErrMaxBMCQueryAttempts = errors.New("reached maximum BMC query attempts") + ErrTaskNotFound = errors.New("task not found") + ErrFirmwareInstallFailed = errors.New("firmware install failed") + ErrFirmwareTaskStateUnexpected = errors.New("BMC returned unexpected firmware task state") ) // bmc wraps the bmclib client and implements the bmcQueryor interface type bmc struct { - client *bmclibv2.Client + client *bmclib.Client logger *logrus.Entry } @@ -73,13 +66,8 @@ func (e *ErrBmcQuery) Error() string { // Open creates a BMC session func (b *bmc) Open(ctx context.Context) error { - ctx, span := otel.Tracer(pkgName).Start(ctx, "bmclib.Open") - defer span.End() - - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) - if b.client == nil { - return errors.Wrap(errBMCLogin, "bmclibv2 client not initialized") + return errors.Wrap(errBMCLogin, "bmclib client not initialized") } // login to the bmc with retries @@ -88,19 +76,11 @@ func (b *bmc) Open(ctx context.Context) error { // Close logs out of the BMC func (b *bmc) Close(traceCtx context.Context) error { - // this context is not used for the close method further below - // since we want to make sure the BMC session is always closed and is not left open - // because of a context cancellation. - _, span := otel.Tracer(pkgName).Start(traceCtx, "bmclib.Close") - defer span.End() - - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) - if b.client == nil { return nil } - ctxClose, cancel := context.WithTimeout(context.Background(), logoutTimeout) + ctxClose, cancel := context.WithTimeout(traceCtx, logoutTimeout) defer cancel() if err := b.client.Close(ctxClose); err != nil { @@ -116,30 +96,15 @@ func (b *bmc) Close(traceCtx context.Context) error { // PowerStatus returns the device power status func (b *bmc) PowerStatus(ctx context.Context) (string, error) { - ctx, span := otel.Tracer(pkgName).Start(ctx, "bmclib.PowerStatus") - defer span.End() - - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) - if err := b.Open(ctx); err != nil { return "", err } - status, err := b.client.GetPowerState(ctx) - if err != nil { - return "", err - } - - return status, nil + return b.client.GetPowerState(ctx) } // SetPowerState sets the given power state on the device func (b *bmc) SetPowerState(ctx context.Context, state string) error { - ctx, span := otel.Tracer(pkgName).Start(ctx, "bmclib.SetPowerState") - defer span.End() - - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) - if err := b.Open(ctx); err != nil { return err } @@ -151,11 +116,6 @@ func (b *bmc) SetPowerState(ctx context.Context, state string) error { // ResetBMC cold resets the BMC func (b *bmc) ResetBMC(ctx context.Context) error { - ctx, span := otel.Tracer(pkgName).Start(ctx, "bmclib.ResetBMC") - defer span.End() - - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) - if err := b.Open(ctx); err != nil { return err } @@ -167,11 +127,6 @@ func (b *bmc) ResetBMC(ctx context.Context) error { // Inventory queries the BMC for the device inventory and returns an object with the device inventory. func (b *bmc) Inventory(ctx context.Context) (*common.Device, error) { - ctx, span := otel.Tracer(pkgName).Start(ctx, "bmclib.Inventory") - defer span.End() - - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) - if err := b.Open(ctx); err != nil { return nil, err } @@ -192,12 +147,15 @@ func (b *bmc) Inventory(ctx context.Context) (*common.Device, error) { return inventory, nil } -func (b *bmc) FirmwareInstall(ctx context.Context, componentSlug string, force bool, file *os.File) (bmcTaskID string, err error) { - ctx, span := otel.Tracer(pkgName).Start(ctx, "bmclib.FirmwareInstall") - defer span.End() +func (b *bmc) FirmwareInstallSteps(ctx context.Context, component string) ([]bconsts.FirmwareInstallStep, error) { + if err := b.Open(ctx); err != nil { + return nil, err + } - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) + return b.client.FirmwareInstallSteps(ctx, component) +} +func (b *bmc) FirmwareInstall(ctx context.Context, componentSlug string, force bool, file *os.File) (bmcTaskID string, err error) { if err := b.Open(ctx); err != nil { return "", err } @@ -205,32 +163,36 @@ func (b *bmc) FirmwareInstall(ctx context.Context, componentSlug string, force b installCtx, cancel := context.WithTimeout(ctx, firmwareInstallTimeout) defer cancel() - return b.client.FirmwareInstall(installCtx, componentSlug, bmclibv2consts.FirmwareApplyOnReset, force, file) + return b.client.FirmwareInstall(installCtx, componentSlug, string(bconsts.OnReset), force, file) } -// FirmwareInstallStatus looks up the firmware install status based on the given installVersion, componentSlug, bmcTaskID parameters -func (b *bmc) FirmwareInstallStatus(ctx context.Context, installVersion, componentSlug, bmcTaskID string) (model.ComponentFirmwareInstallStatus, error) { - ctx, span := otel.Tracer(pkgName).Start(ctx, "bmclib.FirmwareInstallStatus") - defer span.End() +// FirmwareTaskStatus looks up the firmware upload/install state and status values +func (b *bmc) FirmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string) (state, status string, err error) { + if err := b.Open(ctx); err != nil { + return "", "", errors.Wrap(ErrBMCQuery, err.Error()) + } - span.SetAttributes(attribute.String("bmc-ip", b.client.Auth.Host)) + return b.client.FirmwareTaskStatus(ctx, kind, component, taskID, installVersion) +} +func (b *bmc) FirmwareUpload(ctx context.Context, component string, file *os.File) (uploadTaskID string, err error) { if err := b.Open(ctx); err != nil { - return model.StatusInstallUnknown, errors.Wrap(ErrBMCQuery, err.Error()) + return "", err } - status, err := b.client.FirmwareInstallStatus(ctx, installVersion, componentSlug, bmcTaskID) - if err != nil { - // If BMC has rebooted, task won't be found, we assume success (failure don't reboot) - if strings.EqualFold(componentSlug, "bmc") && errors.Is(err, bmclibv2errs.ErrTaskNotFound) { - return model.StatusInstallComplete, nil - } + installCtx, cancel := context.WithTimeout(ctx, firmwareInstallTimeout) + defer cancel() - return model.StatusInstallUnknown, errors.Wrap(ErrBMCQuery, err.Error()) - } + return b.client.FirmwareUpload(installCtx, component, file) +} func (b *bmc) FirmwareInstallUploaded(ctx context.Context, component, uploadVerifyTaskID string) (installTaskID string, err error) { if err := b.Open(ctx); err != nil { return "", err } + + installCtx, cancel := context.WithTimeout(ctx, firmwareInstallTimeout) + defer cancel() + + return b.client.FirmwareInstallUploaded(installCtx, component, uploadVerifyTaskID) } diff --git a/internal/outofband/bmc_helpers.go b/internal/outofband/bmc_helpers.go index 3a383957..f6e5d685 100644 --- a/internal/outofband/bmc_helpers.go +++ b/internal/outofband/bmc_helpers.go @@ -10,7 +10,7 @@ import ( "strings" "time" - bmclibv2 "github.com/bmc-toolbox/bmclib/v2" + bmclib "github.com/bmc-toolbox/bmclib/v2" logrusrv2 "github.com/bombsimon/logrusr/v2" "github.com/hashicorp/go-multierror" "github.com/jpillora/backoff" @@ -48,8 +48,8 @@ func newHTTPClient() *http.Client { } } -// newBmclibv2Client initializes a bmclibv2 client with the given credentials -func newBmclibv2Client(_ context.Context, asset *model.Asset, l *logrus.Entry) *bmclibv2.Client { +// newBmclibv2Client initializes a bmclib client with the given credentials +func newBmclibv2Client(_ context.Context, asset *model.Asset, l *logrus.Entry) *bmclib.Client { logger := logrus.New() logger.Formatter = l.Logger.Formatter @@ -67,17 +67,17 @@ func newBmclibv2Client(_ context.Context, asset *model.Asset, l *logrus.Entry) * logruslogr := logrusrv2.New(logger) - bmcClient := bmclibv2.NewClient( + bmcClient := bmclib.NewClient( asset.BmcAddress.String(), asset.BmcUsername, asset.BmcPassword, - bmclibv2.WithLogger(logruslogr), - bmclibv2.WithHTTPClient(newHTTPClient()), - bmclibv2.WithPerProviderTimeout(loginTimeout), - bmclibv2.WithRedfishEtagMatchDisabled(true), + bmclib.WithLogger(logruslogr), + bmclib.WithHTTPClient(newHTTPClient()), + bmclib.WithPerProviderTimeout(loginTimeout), + bmclib.WithRedfishEtagMatchDisabled(true), ) - // set bmclibv2 driver + // set bmclib driver // // The bmclib drivers here are limited to the HTTPS means of connection, // that is, drivers like ipmi are excluded. @@ -107,7 +107,7 @@ func newBmclibv2Client(_ context.Context, asset *model.Asset, l *logrus.Entry) * func (b *bmc) sessionActive(ctx context.Context) error { if b.client == nil { - return errors.Wrap(errBMCSession, "bmclibv2 client not initialized") + return errors.Wrap(errBMCSession, "bmclib client not initialized") } // check if we're able to query the power state @@ -186,7 +186,7 @@ func (b *bmc) loginWithRetries(ctx context.Context, tries int) error { attempts++ - time.Sleep(delay.Duration()) + sleepWithContext(ctx, delay.ForAttempt(float64(attempts))) continue }