From b63a50aa66d1157a8712ea519283e7d1bc7aa8d9 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 17 Apr 2024 16:27:36 +0200 Subject: [PATCH 1/9] providers/supermicro: add x11spo-ntf in supported list This has been tested and works --- providers/supermicro/firmware.go | 1 + 1 file changed, 1 insertion(+) diff --git a/providers/supermicro/firmware.go b/providers/supermicro/firmware.go index e4a7c8ca..2deebd32 100644 --- a/providers/supermicro/firmware.go +++ b/providers/supermicro/firmware.go @@ -26,6 +26,7 @@ var ( "X11DPT-B", "X11SSE-F", "X12STH-SYS", + "X12SPO-NTF", } errUploadTaskIDExpected = errors.New("expected an firmware upload taskID") From aa42442237c4accac7a675057f03523909c09fb7 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 17 Apr 2024 16:28:38 +0200 Subject: [PATCH 2/9] providers/supermicro: Close session if any login dependencies fail This prevents the client from assumptions that the client has an active session available --- providers/supermicro/supermicro.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go index ec22e08a..2ba88d33 100644 --- a/providers/supermicro/supermicro.go +++ b/providers/supermicro/supermicro.go @@ -160,18 +160,24 @@ func (c *Client) Open(ctx context.Context) (err error) { return errors.Wrap(bmclibErrs.ErrLoginFailed, strconv.Itoa(status)) } + // called after a session was opened but further login dependencies failed + closeWithError := func(ctx context.Context, err error) error { + _ = c.Close(ctx) + return err + } + if !bytes.Contains(body, []byte(`url_redirect.cgi?url_name=mainmenu`)) && !bytes.Contains(body, []byte(`url_redirect.cgi?url_name=topmenu`)) { - return errors.Wrap(bmclibErrs.ErrLoginFailed, "unexpected response contents") + return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, "unexpected response contents")) } contentsTopMenu, status, err := c.serviceClient.query(ctx, "cgi/url_redirect.cgi?url_name=topmenu", http.MethodGet, nil, nil, 0) if err != nil { - return errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error()) + return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error())) } if status != 200 { - return errors.Wrap(bmclibErrs.ErrLoginFailed, strconv.Itoa(status)) + return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, strconv.Itoa(status))) } // Note: older firmware version on the X11s don't use a CSRF token @@ -183,11 +189,11 @@ func (c *Client) Open(ctx context.Context) (err error) { c.bmc, err = c.bmcQueryor(ctx) if err != nil { - return errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error()) + return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error())) } if err := c.serviceClient.redfishSession(ctx); err != nil { - return errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error()) + return closeWithError(ctx, errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error())) } return nil @@ -257,7 +263,7 @@ func (c *Client) bmcQueryor(ctx context.Context) (bmcQueryor, error) { model := strings.ToLower(queryor.deviceModel()) if !strings.HasPrefix(model, "x12") && !strings.HasPrefix(model, "x11") { - return nil, errors.Wrap(ErrModelUnsupported, model) + return nil, errors.Wrap(ErrModelUnsupported, "expected one of X11* or X12*, got:"+model) } return queryor, nil From afecbf2f4d980a77f44dd5cb68a2abfe48434b87 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 17 Apr 2024 18:27:25 +0200 Subject: [PATCH 3/9] providers/dell: reuse ErrUnsupportedHardware from defined in bmclib/errors instead of defining another one --- providers/dell/firmware.go | 10 +++------- providers/dell/idrac.go | 1 + 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/providers/dell/firmware.go b/providers/dell/firmware.go index f311ea73..47ca86de 100644 --- a/providers/dell/firmware.go +++ b/providers/dell/firmware.go @@ -17,14 +17,10 @@ import ( "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 nil, bmcliberrs.NewErrUnsupportedHardware(err.Error()) } return []constants.FirmwareInstallStep{ @@ -35,7 +31,7 @@ func (c *Conn) FirmwareInstallSteps(ctx context.Context, component string) ([]co 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()) + return "", bmcliberrs.NewErrUnsupportedHardware(err.Error()) } // // expect atleast 5 minutes left in the deadline to proceed with the upload @@ -102,7 +98,7 @@ func (c *Conn) checkQueueability(component string, tasks []*redfish.Task) error // 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()) + return "", "", bmcliberrs.NewErrUnsupportedHardware(err.Error()) } // Dell jobs are turned into Redfish tasks on the idrac diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index 7ea6e724..6a23e7ea 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -10,6 +10,7 @@ import ( "net/http" "strings" + bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" "github.com/bmc-toolbox/bmclib/v2/providers" From 43d8535dec56d6ac9bdf5c4b25e5d851330aecba Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 17 Apr 2024 18:28:29 +0200 Subject: [PATCH 4/9] providers/dell: verify vendor model before proceeding This is to ensure the code within this provider only executes on dells --- providers/dell/idrac.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index 6a23e7ea..67441b91 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -205,21 +205,37 @@ func (c *Conn) Compatible(ctx context.Context) bool { // PowerStateGet gets the power state of a BMC machine func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { + if err := c.deviceSupported(ctx); err != nil { + return "", bmcliberrs.NewErrUnsupportedHardware(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) { + if err := c.deviceSupported(ctx); err != nil { + return false, bmcliberrs.NewErrUnsupportedHardware(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) { + if err := c.deviceSupported(ctx); err != nil { + return nil, bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + return c.redfishwrapper.Inventory(ctx, false) } // BmcReset power cycles the BMC func (c *Conn) BmcReset(ctx context.Context, resetType string) (ok bool, err error) { + if err := c.deviceSupported(ctx); err != nil { + return false, bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + return c.redfishwrapper.BMCReset(ctx, resetType) } @@ -240,6 +256,10 @@ func (c *Conn) ResetBiosConfiguration(ctx context.Context) (err error) { // SendNMI tells the BMC to issue an NMI to the device func (c *Conn) SendNMI(ctx context.Context) error { + if err := c.deviceSupported(ctx); err != nil { + return bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + return c.redfishwrapper.SendNMI(ctx) } @@ -260,6 +280,10 @@ func (c *Conn) deviceManufacturer(ctx context.Context) (vendor string, err error } func (c *Conn) Screenshot(ctx context.Context) (image []byte, fileType string, err error) { + if err := c.deviceSupported(ctx); err != nil { + return nil, "", bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + fileType = "png" resp, err := c.redfishwrapper.PostWithHeaders( From 82d4c113a4457e0de739fbe9d27e6b070041675e Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 17 Apr 2024 18:29:16 +0200 Subject: [PATCH 5/9] providers/asrr: verify hardware supported before firmware action --- providers/asrockrack/firmware.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/providers/asrockrack/firmware.go b/providers/asrockrack/firmware.go index 817f6273..24bd2973 100644 --- a/providers/asrockrack/firmware.go +++ b/providers/asrockrack/firmware.go @@ -24,6 +24,10 @@ const ( // bmc client interface implementations methods func (a *ASRockRack) FirmwareInstallSteps(ctx context.Context, component string) ([]constants.FirmwareInstallStep, error) { + if err := a.supported(ctx); err != nil { + return nil, bmclibErrs.NewErrUnsupportedHardware(err.Error()) + } + switch strings.ToUpper(component) { case common.SlugBMC: return []constants.FirmwareInstallStep{ From 902ddd166b09206235070846d50a5624732fea8d Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Wed, 17 Apr 2024 18:29:56 +0200 Subject: [PATCH 6/9] providers/openbmc: verify hardware support in all methods This is to ensure the Openbmc provider executes code only on hardware identified to be Openbmc --- providers/openbmc/firmware.go | 4 ++++ providers/openbmc/openbmc.go | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/providers/openbmc/firmware.go b/providers/openbmc/firmware.go index d5fd492a..851ce040 100644 --- a/providers/openbmc/firmware.go +++ b/providers/openbmc/firmware.go @@ -96,5 +96,9 @@ func (c *Conn) checkQueueability(component string, tasks []*redfish.Task) error // 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 "", "", bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + return c.redfishwrapper.TaskStatus(ctx, taskID) } diff --git a/providers/openbmc/openbmc.go b/providers/openbmc/openbmc.go index 8e9cde11..556f698a 100644 --- a/providers/openbmc/openbmc.go +++ b/providers/openbmc/openbmc.go @@ -15,6 +15,8 @@ import ( "github.com/go-logr/logr" "github.com/jacobweinstock/registrar" "github.com/pkg/errors" + + bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" ) const ( @@ -167,25 +169,45 @@ func (c *Conn) Name() string { // PowerStateGet gets the power state of a BMC machine func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { + if err := c.deviceSupported(ctx); err != nil { + return "", bmcliberrs.NewErrUnsupportedHardware(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) { + if err := c.deviceSupported(ctx); err != nil { + return false, bmcliberrs.NewErrUnsupportedHardware(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) { + if err := c.deviceSupported(ctx); err != nil { + return nil, bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + return c.redfishwrapper.Inventory(ctx, false) } // BmcReset power cycles the BMC func (c *Conn) BmcReset(ctx context.Context, resetType string) (ok bool, err error) { + if err := c.deviceSupported(ctx); err != nil { + return false, bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + return c.redfishwrapper.BMCReset(ctx, resetType) } // SendNMI tells the BMC to issue an NMI to the device func (c *Conn) SendNMI(ctx context.Context) error { + if err := c.deviceSupported(ctx); err != nil { + return bmcliberrs.NewErrUnsupportedHardware(err.Error()) + } + return c.redfishwrapper.SendNMI(ctx) } From 47b297877bb930ed80f9ad67df2474726fe35569 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Thu, 18 Apr 2024 09:53:13 +0200 Subject: [PATCH 7/9] Revert "providers/dell: verify vendor model before proceeding" This reverts commit 43d8535dec56d6ac9bdf5c4b25e5d851330aecba. --- providers/dell/idrac.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index 67441b91..6a23e7ea 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -205,37 +205,21 @@ func (c *Conn) Compatible(ctx context.Context) bool { // PowerStateGet gets the power state of a BMC machine func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { - if err := c.deviceSupported(ctx); err != nil { - return "", bmcliberrs.NewErrUnsupportedHardware(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) { - if err := c.deviceSupported(ctx); err != nil { - return false, bmcliberrs.NewErrUnsupportedHardware(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) { - if err := c.deviceSupported(ctx); err != nil { - return nil, bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - return c.redfishwrapper.Inventory(ctx, false) } // BmcReset power cycles the BMC func (c *Conn) BmcReset(ctx context.Context, resetType string) (ok bool, err error) { - if err := c.deviceSupported(ctx); err != nil { - return false, bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - return c.redfishwrapper.BMCReset(ctx, resetType) } @@ -256,10 +240,6 @@ func (c *Conn) ResetBiosConfiguration(ctx context.Context) (err error) { // SendNMI tells the BMC to issue an NMI to the device func (c *Conn) SendNMI(ctx context.Context) error { - if err := c.deviceSupported(ctx); err != nil { - return bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - return c.redfishwrapper.SendNMI(ctx) } @@ -280,10 +260,6 @@ func (c *Conn) deviceManufacturer(ctx context.Context) (vendor string, err error } func (c *Conn) Screenshot(ctx context.Context) (image []byte, fileType string, err error) { - if err := c.deviceSupported(ctx); err != nil { - return nil, "", bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - fileType = "png" resp, err := c.redfishwrapper.PostWithHeaders( From 89f2f49e23f4c845e2305236289c27981032dd0d Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Thu, 18 Apr 2024 11:10:58 +0200 Subject: [PATCH 8/9] Revert "providers/openbmc: verify hardware support in all methods" This reverts commit 902ddd166b09206235070846d50a5624732fea8d. --- providers/openbmc/firmware.go | 4 ---- providers/openbmc/openbmc.go | 22 ---------------------- 2 files changed, 26 deletions(-) diff --git a/providers/openbmc/firmware.go b/providers/openbmc/firmware.go index 851ce040..d5fd492a 100644 --- a/providers/openbmc/firmware.go +++ b/providers/openbmc/firmware.go @@ -96,9 +96,5 @@ func (c *Conn) checkQueueability(component string, tasks []*redfish.Task) error // 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 "", "", bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - return c.redfishwrapper.TaskStatus(ctx, taskID) } diff --git a/providers/openbmc/openbmc.go b/providers/openbmc/openbmc.go index 556f698a..8e9cde11 100644 --- a/providers/openbmc/openbmc.go +++ b/providers/openbmc/openbmc.go @@ -15,8 +15,6 @@ import ( "github.com/go-logr/logr" "github.com/jacobweinstock/registrar" "github.com/pkg/errors" - - bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" ) const ( @@ -169,45 +167,25 @@ func (c *Conn) Name() string { // PowerStateGet gets the power state of a BMC machine func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) { - if err := c.deviceSupported(ctx); err != nil { - return "", bmcliberrs.NewErrUnsupportedHardware(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) { - if err := c.deviceSupported(ctx); err != nil { - return false, bmcliberrs.NewErrUnsupportedHardware(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) { - if err := c.deviceSupported(ctx); err != nil { - return nil, bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - return c.redfishwrapper.Inventory(ctx, false) } // BmcReset power cycles the BMC func (c *Conn) BmcReset(ctx context.Context, resetType string) (ok bool, err error) { - if err := c.deviceSupported(ctx); err != nil { - return false, bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - return c.redfishwrapper.BMCReset(ctx, resetType) } // SendNMI tells the BMC to issue an NMI to the device func (c *Conn) SendNMI(ctx context.Context) error { - if err := c.deviceSupported(ctx); err != nil { - return bmcliberrs.NewErrUnsupportedHardware(err.Error()) - } - return c.redfishwrapper.SendNMI(ctx) } From 1753bef979cb001e61b64979c3d63b975e464fb7 Mon Sep 17 00:00:00 2001 From: Joel Rebello Date: Thu, 18 Apr 2024 11:12:08 +0200 Subject: [PATCH 9/9] provider/dell: remove unused import --- providers/dell/idrac.go | 1 - 1 file changed, 1 deletion(-) diff --git a/providers/dell/idrac.go b/providers/dell/idrac.go index 6a23e7ea..7ea6e724 100644 --- a/providers/dell/idrac.go +++ b/providers/dell/idrac.go @@ -10,7 +10,6 @@ import ( "net/http" "strings" - bmcliberrs "github.com/bmc-toolbox/bmclib/v2/errors" "github.com/bmc-toolbox/bmclib/v2/internal/httpclient" "github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper" "github.com/bmc-toolbox/bmclib/v2/providers"