From bc60233d8a5240c338934afa2e379876f4b4b0ca Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 07:39:41 +0100
Subject: [PATCH 01/24] redfish/inventory: move Inventory method under
 internal/redfishwrapper

This enables other providers to reuse the Inventory method and customise
its use based on the vendor/model
---
 .../redfishwrapper}/inventory.go              | 126 +++++++++---------
 .../redfishwrapper}/inventory_collect.go      |  70 +++++-----
 .../redfishwrapper}/inventory_collect_test.go |  38 +++---
 3 files changed, 118 insertions(+), 116 deletions(-)
 rename {providers/redfish => internal/redfishwrapper}/inventory.go (64%)
 rename {providers/redfish => internal/redfishwrapper}/inventory_collect.go (78%)
 rename {providers/redfish => internal/redfishwrapper}/inventory_collect_test.go (86%)

diff --git a/providers/redfish/inventory.go b/internal/redfishwrapper/inventory.go
similarity index 64%
rename from providers/redfish/inventory.go
rename to internal/redfishwrapper/inventory.go
index fec1b5fb..05046eb2 100644
--- a/providers/redfish/inventory.go
+++ b/internal/redfishwrapper/inventory.go
@@ -1,20 +1,19 @@
-package redfish
+package redfishwrapper
 
 import (
 	"context"
 	"strings"
 
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
-	"github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper"
 	"github.com/pkg/errors"
 
 	"github.com/bmc-toolbox/common"
-	gofishrf "github.com/stmcginnis/gofish/redfish"
+	redfish "github.com/stmcginnis/gofish/redfish"
 )
 
 var (
 	// Supported Chassis Odata IDs
-	chassisOdataIDs = []string{
+	KnownChassisOdataIDs = []string{
 		// Dells
 		"/redfish/v1/Chassis/Enclosure.Internal.0-1",
 		"/redfish/v1/Chassis/System.Embedded.1",
@@ -28,7 +27,7 @@ var (
 	}
 
 	// Supported System Odata IDs
-	systemsOdataIDs = []string{
+	knownSystemsOdataIDs = []string{
 		// Dells
 		"/redfish/v1/Systems/System.Embedded.1",
 		"/redfish/v1/Systems/System.Embedded.1/Bios",
@@ -53,26 +52,29 @@ var (
 	}
 )
 
-// inventory struct wraps redfish connection
-type inventory struct {
-	client            *redfishwrapper.Client
-	failOnError       bool
-	softwareInventory []*gofishrf.SoftwareInventory
-}
+// TODO: consider removing this
+func (c *Client) compatibleOdataID(OdataID string, knownOdataIDs []string) bool {
+	for _, url := range knownOdataIDs {
+		if url == OdataID {
+			return true
+		}
+	}
 
-func (c *Conn) Inventory(ctx context.Context) (device *common.Device, err error) {
-	// initialize inventory object
-	// the redfish client is assigned here to perform redfish Get/Delete requests
-	inv := &inventory{client: c.redfishwrapper, failOnError: c.failInventoryOnError}
+	return false
+}
 
-	updateService, err := c.redfishwrapper.UpdateService()
-	if err != nil && inv.failOnError {
+func (c *Client) Inventory(ctx context.Context, failOnError bool) (device *common.Device, err error) {
+	updateService, err := c.UpdateService()
+	if err != nil && failOnError {
 		return nil, errors.Wrap(bmclibErrs.ErrRedfishSoftwareInventory, err.Error())
 	}
 
+	softwareInventory := []*redfish.SoftwareInventory{}
+
 	if updateService != nil {
-		inv.softwareInventory, err = updateService.FirmwareInventories()
-		if err != nil && inv.failOnError {
+		// nolint
+		softwareInventory, err = updateService.FirmwareInventories()
+		if err != nil && failOnError {
 			return nil, errors.Wrap(bmclibErrs.ErrRedfishSoftwareInventory, err.Error())
 		}
 	}
@@ -82,20 +84,20 @@ func (c *Conn) Inventory(ctx context.Context) (device *common.Device, err error)
 	device = &newDevice
 
 	// populate device Chassis components attributes
-	err = inv.chassisAttributes(ctx, device)
-	if err != nil && inv.failOnError {
+	err = c.chassisAttributes(ctx, device, failOnError, softwareInventory)
+	if err != nil && failOnError {
 		return nil, err
 	}
 
 	// populate device System components attributes
-	err = inv.systemAttributes(ctx, device)
-	if err != nil && inv.failOnError {
+	err = c.systemAttributes(ctx, device, failOnError, softwareInventory)
+	if err != nil && failOnError {
 		return nil, err
 	}
 
 	// populate device BMC component attributes
-	err = inv.bmcAttributes(ctx, device)
-	if err != nil && inv.failOnError {
+	err = c.bmcAttributes(ctx, device, failOnError, softwareInventory)
+	if err != nil && failOnError {
 		return nil, err
 	}
 
@@ -105,15 +107,15 @@ func (c *Conn) Inventory(ctx context.Context) (device *common.Device, err error)
 // DeviceVendorModel returns the device vendor and model attributes
 
 // bmcAttributes collects BMC component attributes
-func (i *inventory) bmcAttributes(ctx context.Context, device *common.Device) (err error) {
-	managers, err := i.client.Managers(ctx)
+func (c *Client) bmcAttributes(ctx context.Context, device *common.Device, failOnError bool, softwareInventory []*redfish.SoftwareInventory) (err error) {
+	managers, err := c.Managers(ctx)
 	if err != nil {
 		return err
 	}
 
 	var compatible int
 	for _, manager := range managers {
-		if !compatibleOdataID(manager.ODataID, managerOdataIDs) {
+		if !c.compatibleOdataID(manager.ODataID, managerOdataIDs) {
 			continue
 		}
 
@@ -141,7 +143,7 @@ func (i *inventory) bmcAttributes(ctx context.Context, device *common.Device) (e
 		}
 
 		// include additional firmware attributes from redfish firmware inventory
-		i.firmwareAttributes("", device.BMC.ID, device.BMC.Firmware)
+		c.firmwareAttributes("", device.BMC.ID, device.BMC.Firmware, softwareInventory)
 	}
 
 	if compatible == 0 {
@@ -152,34 +154,34 @@ func (i *inventory) bmcAttributes(ctx context.Context, device *common.Device) (e
 }
 
 // chassisAttributes populates the device chassis attributes
-func (i *inventory) chassisAttributes(ctx context.Context, device *common.Device) (err error) {
-	chassis, err := i.client.Chassis(ctx)
+func (c *Client) chassisAttributes(ctx context.Context, device *common.Device, failOnError bool, softwareInventory []*redfish.SoftwareInventory) (err error) {
+	chassis, err := c.Chassis(ctx)
 	if err != nil {
 		return err
 	}
 
 	compatible := 0
 	for _, ch := range chassis {
-		if !compatibleOdataID(ch.ODataID, chassisOdataIDs) {
+		if !c.compatibleOdataID(ch.ODataID, KnownChassisOdataIDs) {
 			continue
 		}
 
 		compatible++
 
-		err = i.collectEnclosure(ch, device)
-		if err != nil && i.failOnError {
+		err = c.collectEnclosure(ch, device, softwareInventory)
+		if err != nil && failOnError {
 			return err
 		}
 
-		err = i.collectPSUs(ch, device)
-		if err != nil && i.failOnError {
+		err = c.collectPSUs(ch, device, softwareInventory)
+		if err != nil && failOnError {
 			return err
 		}
 
 	}
 
-	err = i.collectCPLDs(device)
-	if err != nil && i.failOnError {
+	err = c.collectCPLDs(device, softwareInventory)
+	if err != nil && failOnError {
 		return err
 	}
 
@@ -191,15 +193,15 @@ func (i *inventory) chassisAttributes(ctx context.Context, device *common.Device
 
 }
 
-func (i *inventory) systemAttributes(ctx context.Context, device *common.Device) (err error) {
-	systems, err := i.client.Systems()
+func (c *Client) systemAttributes(ctx context.Context, device *common.Device, failOnError bool, softwareInventory []*redfish.SoftwareInventory) (err error) {
+	systems, err := c.Systems()
 	if err != nil {
 		return err
 	}
 
 	compatible := 0
 	for _, sys := range systems {
-		if !compatibleOdataID(sys.ODataID, systemsOdataIDs) {
+		if !c.compatibleOdataID(sys.ODataID, knownSystemsOdataIDs) {
 			continue
 		}
 
@@ -211,21 +213,27 @@ func (i *inventory) systemAttributes(ctx context.Context, device *common.Device)
 			device.Serial = sys.SerialNumber
 		}
 
+		type collectorFuncs []func(
+			sys *redfish.ComputerSystem,
+			device *common.Device,
+			softwareInventory []*redfish.SoftwareInventory,
+		) error
+
 		// slice of collector methods
-		funcs := []func(sys *gofishrf.ComputerSystem, device *common.Device) error{
-			i.collectCPUs,
-			i.collectDIMMs,
-			i.collectDrives,
-			i.collectBIOS,
-			i.collectNICs,
-			i.collectTPMs,
-			i.collectStorageControllers,
+		funcs := collectorFuncs{
+			c.collectCPUs,
+			c.collectDIMMs,
+			c.collectDrives,
+			c.collectBIOS,
+			c.collectNICs,
+			c.collectTPMs,
+			c.collectStorageControllers,
 		}
 
 		// execute collector methods
 		for _, f := range funcs {
-			err := f(sys, device)
-			if err != nil && i.failOnError {
+			err := f(sys, device, softwareInventory)
+			if err != nil && failOnError {
 				return err
 			}
 		}
@@ -245,8 +253,8 @@ func (i *inventory) systemAttributes(ctx context.Context, device *common.Device)
 // slug - the component slug constant
 // id - the component ID
 // previous - when true returns previously installed firmware, else returns the current
-func (i *inventory) firmwareAttributes(slug, id string, firmwareObj *common.Firmware) {
-	if len(i.softwareInventory) == 0 {
+func (c *Client) firmwareAttributes(slug, id string, firmwareObj *common.Firmware, softwareInventory []*redfish.SoftwareInventory) {
+	if len(softwareInventory) == 0 {
 		return
 	}
 
@@ -254,7 +262,7 @@ func (i *inventory) firmwareAttributes(slug, id string, firmwareObj *common.Firm
 		id = slug
 	}
 
-	for _, inv := range i.softwareInventory {
+	for _, inv := range softwareInventory {
 		// include previously installed firmware attributes
 		if strings.HasPrefix(inv.ID, "Previous") {
 			if strings.Contains(inv.ID, id) || strings.EqualFold(slug, inv.Name) {
@@ -292,13 +300,3 @@ func (i *inventory) firmwareAttributes(slug, id string, firmwareObj *common.Firm
 		}
 	}
 }
-
-func compatibleOdataID(OdataID string, knownOdataIDs []string) bool {
-	for _, url := range knownOdataIDs {
-		if url == OdataID {
-			return true
-		}
-	}
-
-	return false
-}
diff --git a/providers/redfish/inventory_collect.go b/internal/redfishwrapper/inventory_collect.go
similarity index 78%
rename from providers/redfish/inventory_collect.go
rename to internal/redfishwrapper/inventory_collect.go
index aad65d3b..e441638d 100644
--- a/providers/redfish/inventory_collect.go
+++ b/internal/redfishwrapper/inventory_collect.go
@@ -1,17 +1,17 @@
-package redfish
+package redfishwrapper
 
 import (
 	"math"
 	"strings"
 
 	"github.com/bmc-toolbox/common"
-	gofishrf "github.com/stmcginnis/gofish/redfish"
+	"github.com/stmcginnis/gofish/redfish"
 )
 
 // defines various inventory collection helper methods
 
 // collectEnclosure collects Enclosure information
-func (i *inventory) collectEnclosure(ch *gofishrf.Chassis, device *common.Device) (err error) {
+func (c *Client) collectEnclosure(ch *redfish.Chassis, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	e := &common.Enclosure{
 		Common: common.Common{
 			Description: ch.Description,
@@ -34,7 +34,7 @@ func (i *inventory) collectEnclosure(ch *gofishrf.Chassis, device *common.Device
 	}
 
 	// include additional firmware attributes from redfish firmware inventory
-	i.firmwareAttributes(common.SlugEnclosure, e.ID, e.Firmware)
+	c.firmwareAttributes(common.SlugEnclosure, e.ID, e.Firmware, softwareInventory)
 
 	device.Enclosures = append(device.Enclosures, e)
 
@@ -42,7 +42,7 @@ func (i *inventory) collectEnclosure(ch *gofishrf.Chassis, device *common.Device
 }
 
 // collectPSUs collects Power Supply Unit component information
-func (i *inventory) collectPSUs(ch *gofishrf.Chassis, device *common.Device) (err error) {
+func (c *Client) collectPSUs(ch *redfish.Chassis, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	power, err := ch.Power()
 	if err != nil {
 		return err
@@ -74,7 +74,7 @@ func (i *inventory) collectPSUs(ch *gofishrf.Chassis, device *common.Device) (er
 		}
 
 		// include additional firmware attributes from redfish firmware inventory
-		i.firmwareAttributes(common.SlugPSU, psu.ID, p.Firmware)
+		c.firmwareAttributes(common.SlugPSU, psu.ID, p.Firmware, softwareInventory)
 
 		device.PSUs = append(device.PSUs, p)
 
@@ -83,7 +83,7 @@ func (i *inventory) collectPSUs(ch *gofishrf.Chassis, device *common.Device) (er
 }
 
 // collectTPMs collects Trusted Platform Module component information
-func (i *inventory) collectTPMs(sys *gofishrf.ComputerSystem, device *common.Device) (err error) {
+func (c *Client) collectTPMs(sys *redfish.ComputerSystem, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	for _, module := range sys.TrustedModules {
 		tpm := &common.TPM{
 			Common: common.Common{
@@ -100,7 +100,7 @@ func (i *inventory) collectTPMs(sys *gofishrf.ComputerSystem, device *common.Dev
 		}
 
 		// include additional firmware attributes from redfish firmware inventory
-		i.firmwareAttributes(common.SlugTPM, "TPM", tpm.Firmware)
+		c.firmwareAttributes(common.SlugTPM, "TPM", tpm.Firmware, softwareInventory)
 
 		device.TPMs = append(device.TPMs, tpm)
 	}
@@ -109,7 +109,7 @@ func (i *inventory) collectTPMs(sys *gofishrf.ComputerSystem, device *common.Dev
 }
 
 // collectNICs collects network interface component information
-func (i *inventory) collectNICs(sys *gofishrf.ComputerSystem, device *common.Device) (err error) {
+func (c *Client) collectNICs(sys *redfish.ComputerSystem, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	if sys == nil || device == nil {
 		return nil
 	}
@@ -163,17 +163,17 @@ func (i *inventory) collectNICs(sys *gofishrf.ComputerSystem, device *common.Dev
 
 			// populate network ports general data
 			nicPort := &common.NICPort{}
-			i.collectNetworkPortInfo(nicPort, adapter, networkPort, portFirmwareVersion)
+			c.collectNetworkPortInfo(nicPort, adapter, networkPort, portFirmwareVersion, softwareInventory)
 
-			if networkPort.ActiveLinkTechnology == gofishrf.EthernetLinkNetworkTechnology {
+			if networkPort.ActiveLinkTechnology == redfish.EthernetLinkNetworkTechnology {
 				// ethernet specific data
-				i.collectEthernetInfo(nicPort, ethernetInterfaces)
+				c.collectEthernetInfo(nicPort, ethernetInterfaces)
 			}
 			n.NICPorts = append(n.NICPorts, nicPort)
 		}
 
 		// include additional firmware attributes from redfish firmware inventory
-		i.firmwareAttributes(common.SlugNIC, n.ID, n.Firmware)
+		c.firmwareAttributes(common.SlugNIC, n.ID, n.Firmware, softwareInventory)
 		if len(portFirmwareVersion) > 0 {
 			if n.Firmware == nil {
 				n.Firmware = &common.Firmware{}
@@ -187,8 +187,13 @@ func (i *inventory) collectNICs(sys *gofishrf.ComputerSystem, device *common.Dev
 	return nil
 }
 
-func (i *inventory) collectNetworkPortInfo(
-	nicPort *common.NICPort, adapter *gofishrf.NetworkAdapter, networkPort *gofishrf.NetworkPort, firmware string) {
+func (c *Client) collectNetworkPortInfo(
+	nicPort *common.NICPort,
+	adapter *redfish.NetworkAdapter,
+	networkPort *redfish.NetworkPort,
+	firmware string,
+	softwareInventory []*redfish.SoftwareInventory,
+) {
 
 	if adapter != nil {
 		nicPort.Vendor = adapter.Manufacturer
@@ -221,7 +226,7 @@ func (i *inventory) collectNetworkPortInfo(
 			}
 		}
 
-		i.firmwareAttributes(common.SlugNIC, networkPort.ID, nicPort.Firmware)
+		c.firmwareAttributes(common.SlugNIC, networkPort.ID, nicPort.Firmware, softwareInventory)
 	}
 	if len(firmware) > 0 {
 		if nicPort.Firmware == nil {
@@ -231,7 +236,7 @@ func (i *inventory) collectNetworkPortInfo(
 	}
 }
 
-func (i *inventory) collectEthernetInfo(nicPort *common.NICPort, ethernetInterfaces []*gofishrf.EthernetInterface) {
+func (c *Client) collectEthernetInfo(nicPort *common.NICPort, ethernetInterfaces []*redfish.EthernetInterface) {
 	if nicPort == nil {
 		return
 	}
@@ -273,7 +278,7 @@ func (i *inventory) collectEthernetInfo(nicPort *common.NICPort, ethernetInterfa
 	}
 }
 
-func getFirmwareVersionFromController(controllers []gofishrf.Controllers, portCount int) string {
+func getFirmwareVersionFromController(controllers []redfish.Controllers, portCount int) string {
 	for _, controller := range controllers {
 		if controller.ControllerCapabilities.NetworkPortCount == portCount {
 			return controller.FirmwarePackageVersion
@@ -282,7 +287,7 @@ func getFirmwareVersionFromController(controllers []gofishrf.Controllers, portCo
 	return ""
 }
 
-func (i *inventory) collectBIOS(sys *gofishrf.ComputerSystem, device *common.Device) (err error) {
+func (c *Client) collectBIOS(sys *redfish.ComputerSystem, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	device.BIOS = &common.BIOS{
 		Common: common.Common{
 			Firmware: &common.Firmware{
@@ -301,13 +306,13 @@ func (i *inventory) collectBIOS(sys *gofishrf.ComputerSystem, device *common.Dev
 	}
 
 	// include additional firmware attributes from redfish firmware inventory
-	i.firmwareAttributes(common.SlugBIOS, "BIOS", device.BIOS.Firmware)
+	c.firmwareAttributes(common.SlugBIOS, "BIOS", device.BIOS.Firmware, softwareInventory)
 
 	return nil
 }
 
 // collectDrives collects drive component information
-func (i *inventory) collectDrives(sys *gofishrf.ComputerSystem, device *common.Device) (err error) {
+func (c *Client) collectDrives(sys *redfish.ComputerSystem, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	storage, err := sys.Storage()
 	if err != nil {
 		return err
@@ -351,7 +356,7 @@ func (i *inventory) collectDrives(sys *gofishrf.ComputerSystem, device *common.D
 			}
 
 			// include additional firmware attributes from redfish firmware inventory
-			i.firmwareAttributes("Disk", drive.ID, d.Firmware)
+			c.firmwareAttributes("Disk", drive.ID, d.Firmware, softwareInventory)
 
 			device.Drives = append(device.Drives, d)
 
@@ -363,7 +368,7 @@ func (i *inventory) collectDrives(sys *gofishrf.ComputerSystem, device *common.D
 }
 
 // collectStorageControllers populates the device with Storage controller component attributes
-func (i *inventory) collectStorageControllers(sys *gofishrf.ComputerSystem, device *common.Device) (err error) {
+func (c *Client) collectStorageControllers(sys *redfish.ComputerSystem, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	storage, err := sys.Storage()
 	if err != nil {
 		return err
@@ -372,7 +377,7 @@ func (i *inventory) collectStorageControllers(sys *gofishrf.ComputerSystem, devi
 	for _, member := range storage {
 		for _, controller := range member.StorageControllers {
 
-			c := &common.StorageController{
+			cs := &common.StorageController{
 				Common: common.Common{
 					Description: controller.Name,
 					Vendor:      common.FormatVendorName(controller.Manufacturer),
@@ -392,23 +397,22 @@ func (i *inventory) collectStorageControllers(sys *gofishrf.ComputerSystem, devi
 			}
 
 			// In some cases the storage controller model number is present in the Name field
-			if strings.TrimSpace(c.Model) == "" && strings.TrimSpace(controller.Name) != "" {
-				c.Model = controller.Name
+			if strings.TrimSpace(cs.Model) == "" && strings.TrimSpace(controller.Name) != "" {
+				cs.Model = controller.Name
 			}
 
 			// include additional firmware attributes from redfish firmware inventory
-			i.firmwareAttributes(c.Description, c.ID, c.Firmware)
+			c.firmwareAttributes(cs.Description, cs.ID, cs.Firmware, softwareInventory)
 
-			device.StorageControllers = append(device.StorageControllers, c)
+			device.StorageControllers = append(device.StorageControllers, cs)
 		}
-
 	}
 
 	return nil
 }
 
 // collectCPUs populates the device with CPU component attributes
-func (i *inventory) collectCPUs(sys *gofishrf.ComputerSystem, device *common.Device) (err error) {
+func (c *Client) collectCPUs(sys *redfish.ComputerSystem, device *common.Device, _ []*redfish.SoftwareInventory) (err error) {
 	procs, err := sys.Processors()
 	if err != nil {
 		return err
@@ -447,7 +451,7 @@ func (i *inventory) collectCPUs(sys *gofishrf.ComputerSystem, device *common.Dev
 }
 
 // collectDIMMs populates the device with memory component attributes
-func (i *inventory) collectDIMMs(sys *gofishrf.ComputerSystem, device *common.Device) (err error) {
+func (c *Client) collectDIMMs(sys *redfish.ComputerSystem, device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 	dimms, err := sys.Memory()
 	if err != nil {
 		return err
@@ -479,7 +483,7 @@ func (i *inventory) collectDIMMs(sys *gofishrf.ComputerSystem, device *common.De
 }
 
 // collecCPLDs populates the device with CPLD component attributes
-func (i *inventory) collectCPLDs(device *common.Device) (err error) {
+func (c *Client) collectCPLDs(device *common.Device, softwareInventory []*redfish.SoftwareInventory) (err error) {
 
 	cpld := &common.CPLD{
 		Common: common.Common{
@@ -489,7 +493,7 @@ func (i *inventory) collectCPLDs(device *common.Device) (err error) {
 		},
 	}
 
-	i.firmwareAttributes(common.SlugCPLD, "", cpld.Firmware)
+	c.firmwareAttributes(common.SlugCPLD, "", cpld.Firmware, softwareInventory)
 	name, exists := cpld.Firmware.Metadata["name"]
 	if exists {
 		cpld.Description = name
diff --git a/providers/redfish/inventory_collect_test.go b/internal/redfishwrapper/inventory_collect_test.go
similarity index 86%
rename from providers/redfish/inventory_collect_test.go
rename to internal/redfishwrapper/inventory_collect_test.go
index 766cf312..35f95d5b 100644
--- a/providers/redfish/inventory_collect_test.go
+++ b/internal/redfishwrapper/inventory_collect_test.go
@@ -1,20 +1,20 @@
-package redfish
+package redfishwrapper
 
 import (
-	"github.com/bmc-toolbox/common"
-	common2 "github.com/stmcginnis/gofish/common"
-	gofishrf "github.com/stmcginnis/gofish/redfish"
 	"reflect"
 	"testing"
-)
 
-func Test_inventory_collectNetworkPortInfo(t *testing.T) {
+	"github.com/bmc-toolbox/common"
+	common2 "github.com/stmcginnis/gofish/common"
+	redfish "github.com/stmcginnis/gofish/redfish"
+)
 
-	testAdapter := &gofishrf.NetworkAdapter{
+func TestInventoryCollectNetworkPortInfo(t *testing.T) {
+	testAdapter := &redfish.NetworkAdapter{
 		Manufacturer: "Acme",
 		Model:        "Anvil 3000",
 	}
-	testNetworkPort := &gofishrf.NetworkPort{
+	testNetworkPort := &redfish.NetworkPort{
 		Entity:                     common2.Entity{ID: "NetworkPort-1"},
 		Description:                "NetworkPort One",
 		VendorID:                   "Vendor-ID",
@@ -67,8 +67,8 @@ func Test_inventory_collectNetworkPortInfo(t *testing.T) {
 	tests := []struct {
 		name          string
 		nicPort       *common.NICPort
-		adapter       *gofishrf.NetworkAdapter
-		networkPort   *gofishrf.NetworkPort
+		adapter       *redfish.NetworkAdapter
+		networkPort   *redfish.NetworkPort
 		firmware      string
 		wantedNicPort *common.NICPort
 	}{
@@ -103,8 +103,8 @@ func Test_inventory_collectNetworkPortInfo(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			i := &inventory{}
-			i.collectNetworkPortInfo(tt.nicPort, tt.adapter, tt.networkPort, tt.firmware)
+			c := Client{}
+			c.collectNetworkPortInfo(tt.nicPort, tt.adapter, tt.networkPort, tt.firmware, []*redfish.SoftwareInventory{})
 			if !reflect.DeepEqual(tt.nicPort, tt.wantedNicPort) {
 				t.Errorf("collectNetworkPortInfo() gotNicPort = %v, want %v", tt.nicPort, tt.wantedNicPort)
 			}
@@ -113,17 +113,17 @@ func Test_inventory_collectNetworkPortInfo(t *testing.T) {
 
 }
 
-func Test_inventory_collectEthernetInfo(t *testing.T) {
+func TestInventoryCollectEthernetInfo(t *testing.T) {
 	testNicPortID := "test NIC port ID"
 	testEthernetID := "test NIC port ID ethernet"
 	testNicPort := &common.NICPort{
 		ID: testNicPortID,
 	}
-	testUnmatchingEthList := []*gofishrf.EthernetInterface{
+	testUnmatchingEthList := []*redfish.EthernetInterface{
 		{Entity: common2.Entity{ID: "other ID"}},
 		{Entity: common2.Entity{ID: "another one"}},
 	}
-	testMatchingEth := &gofishrf.EthernetInterface{
+	testMatchingEth := &redfish.EthernetInterface{
 		Entity:      common2.Entity{ID: testEthernetID},
 		Description: "Ethernet Interface",
 		Status: common2.Status{
@@ -155,12 +155,12 @@ func Test_inventory_collectEthernetInfo(t *testing.T) {
 	tests := []struct {
 		name               string
 		nicPort            *common.NICPort
-		ethernetInterfaces []*gofishrf.EthernetInterface
+		ethernetInterfaces []*redfish.EthernetInterface
 		wantedNicPort      *common.NICPort
 	}{
 		{name: "nil"},
 		{name: "empty", nicPort: testNicPort, wantedNicPort: testNicPort},
-		{name: "empty ethernet list", nicPort: testNicPort, ethernetInterfaces: []*gofishrf.EthernetInterface{}, wantedNicPort: testNicPort},
+		{name: "empty ethernet list", nicPort: testNicPort, ethernetInterfaces: []*redfish.EthernetInterface{}, wantedNicPort: testNicPort},
 		{name: "unmatching ethernet list", nicPort: testNicPort, ethernetInterfaces: testUnmatchingEthList, wantedNicPort: testNicPort},
 		{
 			name:               "full",
@@ -170,8 +170,8 @@ func Test_inventory_collectEthernetInfo(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			i := &inventory{}
-			i.collectEthernetInfo(tt.nicPort, tt.ethernetInterfaces)
+			c := Client{}
+			c.collectEthernetInfo(tt.nicPort, tt.ethernetInterfaces)
 		})
 	}
 }

From 551b70930e83d1c7c8a02e4c2eb54deaa863d192 Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 07:47:26 +0100
Subject: [PATCH 02/24] providers/redfish: firmware methods moved into
 redfishwrapper

The FirmwareUpload and related methods in redfishwrapper are more
generic and can be re-used by providers with OEM specific update
parameters. Having these methods in the redfish provider makes it
cumbersome to maintain and extend.
---
 providers/redfish/firmware.go      | 430 -----------------------------
 providers/redfish/firmware_test.go | 280 -------------------
 2 files changed, 710 deletions(-)
 delete mode 100644 providers/redfish/firmware.go
 delete mode 100644 providers/redfish/firmware_test.go

diff --git a/providers/redfish/firmware.go b/providers/redfish/firmware.go
deleted file mode 100644
index 8606bfdd..00000000
--- a/providers/redfish/firmware.go
+++ /dev/null
@@ -1,430 +0,0 @@
-package redfish
-
-import (
-	"bytes"
-	"context"
-	"encoding/json"
-	"fmt"
-	"io"
-	"mime/multipart"
-	"net/http"
-	"net/textproto"
-	"os"
-	"path/filepath"
-	"strconv"
-	"strings"
-	"time"
-
-	"github.com/pkg/errors"
-	gofishrf "github.com/stmcginnis/gofish/redfish"
-
-	"github.com/bmc-toolbox/bmclib/v2/constants"
-	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
-)
-
-var (
-	errInsufficientCtxTimeout = errors.New("remaining context timeout insufficient to install firmware")
-	errMultiPartPayload       = errors.New("error preparing multipart payload")
-)
-
-type installMethod string
-
-const (
-	unstructuredHttpPush installMethod = "unstructuredHttpPush"
-	multipartHttpUpload  installMethod = "multipartUpload"
-)
-
-// FirmwareInstall uploads and initiates the firmware install process
-func (c *Conn) FirmwareInstall(ctx context.Context, component string, operationApplyTime string, forceInstall bool, reader io.Reader) (taskID string, err error) {
-	// limit to *os.File until theres a need for other types of readers
-	updateFile, ok := reader.(*os.File)
-	if !ok {
-		return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "method expects an *os.File object")
-	}
-
-	installMethod, installURI, err := c.firmwareInstallMethodURI(ctx)
-	if err != nil {
-		return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error())
-	}
-
-	// expect atleast 10 minutes left in the deadline to proceed with the update
-	//
-	// this gives the BMC enough time to have the firmware uploaded and return a response to the client.
-	ctxDeadline, _ := ctx.Deadline()
-	if time.Until(ctxDeadline) < 10*time.Minute {
-		return "", errors.Wrap(errInsufficientCtxTimeout, " "+time.Until(ctxDeadline).String())
-	}
-
-	// list redfish firmware install task if theres one present
-	task, err := c.GetFirmwareInstallTaskQueued(ctx, component)
-	if err != nil {
-		return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error())
-	}
-
-	if task != nil {
-		msg := fmt.Sprintf("task for %s firmware install present: %s", component, task.ID)
-		c.Log.V(2).Info("warn", msg)
-
-		if forceInstall {
-			err = c.purgeQueuedFirmwareInstallTask(ctx, component)
-			if err != nil {
-				return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, err.Error())
-			}
-		} else {
-			return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, msg)
-		}
-	}
-
-	// override the gofish HTTP client timeout,
-	// since the context timeout is set at Open() and is at a lower value than required for this operation.
-	//
-	// record the http client timeout to be restored when this method returns
-	httpClientTimeout := c.redfishwrapper.HttpClientTimeout()
-	defer func() {
-		c.redfishwrapper.SetHttpClientTimeout(httpClientTimeout)
-	}()
-
-	c.redfishwrapper.SetHttpClientTimeout(time.Until(ctxDeadline))
-
-	var resp *http.Response
-
-	switch installMethod {
-	case multipartHttpUpload:
-		var uploadErr error
-		resp, uploadErr = c.multipartHTTPUpload(ctx, installURI, operationApplyTime, updateFile)
-		if uploadErr != nil {
-			return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, uploadErr.Error())
-		}
-
-	case unstructuredHttpPush:
-		var uploadErr error
-		resp, uploadErr = c.unstructuredHttpUpload(ctx, installURI, operationApplyTime, reader)
-		if uploadErr != nil {
-			return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, uploadErr.Error())
-		}
-
-	default:
-		return "", errors.Wrap(bmclibErrs.ErrFirmwareInstall, "unsupported install method: "+string(installMethod))
-	}
-
-	if resp.StatusCode != http.StatusAccepted {
-		return "", errors.Wrap(
-			bmclibErrs.ErrFirmwareUpload,
-			"non 202 status code returned: "+strconv.Itoa(resp.StatusCode),
-		)
-	}
-
-	// The response contains a location header pointing to the task URI
-	// Location: /redfish/v1/TaskService/Tasks/JID_467696020275
-	var location = resp.Header.Get("Location")
-
-	taskID, err = TaskIDFromLocationURI(location)
-
-	return taskID, err
-}
-
-func TaskIDFromLocationURI(uri string) (taskID string, err error) {
-
-	if strings.Contains(uri, "JID_") {
-		taskID = strings.Split(uri, "JID_")[1]
-	} else if strings.Contains(uri, "/Monitor") {
-		// OpenBMC returns a monitor URL in Location
-		// Location: /redfish/v1/TaskService/Tasks/12/Monitor
-		splits := strings.Split(uri, "/")
-		if len(splits) >= 6 {
-			taskID = splits[5]
-		} else {
-			taskID = ""
-		}
-	}
-
-	if taskID == "" {
-		return "", bmclibErrs.ErrTaskNotFound
-	}
-
-	return taskID, nil
-}
-
-type multipartPayload struct {
-	updateParameters []byte
-	updateFile       *os.File
-}
-
-func (c *Conn) multipartHTTPUpload(ctx context.Context, url string, operationApplyTime string, update *os.File) (*http.Response, error) {
-	if url == "" {
-		return nil, fmt.Errorf("unable to execute request, no target provided")
-	}
-
-	parameters, err := json.Marshal(struct {
-		Targets            []string `json:"Targets"`
-		RedfishOpApplyTime string   `json:"@Redfish.OperationApplyTime"`
-		Oem                struct{} `json:"Oem"`
-	}{
-		[]string{},
-		operationApplyTime,
-		struct{}{},
-	})
-
-	if err != nil {
-		return nil, errors.Wrap(err, "error preparing multipart UpdateParameters payload")
-	}
-
-	// payload ordered in the format it ends up in the multipart form
-	payload := &multipartPayload{
-		updateParameters: []byte(parameters),
-		updateFile:       update,
-	}
-
-	return c.runRequestWithMultipartPayload(url, payload)
-}
-
-func (c *Conn) unstructuredHttpUpload(ctx context.Context, url string, operationApplyTime string, update io.Reader) (*http.Response, error) {
-	if url == "" {
-		return nil, fmt.Errorf("unable to execute request, no target provided")
-	}
-
-	// TODO: transform this to read the update so that we don't hold the data in memory
-	b, _ := io.ReadAll(update)
-	payloadReadSeeker := bytes.NewReader(b)
-
-	return c.redfishwrapper.RunRawRequestWithHeaders(http.MethodPost, url, payloadReadSeeker, "application/octet-stream", nil)
-
-}
-
-// firmwareUpdateMethodURI returns the updateMethod and URI
-func (c *Conn) firmwareInstallMethodURI(ctx context.Context) (method installMethod, updateURI string, err error) {
-	updateService, err := c.redfishwrapper.UpdateService()
-	if err != nil {
-		return "", "", errors.Wrap(bmclibErrs.ErrRedfishUpdateService, err.Error())
-	}
-
-	// update service disabled
-	if !updateService.ServiceEnabled {
-		return "", "", errors.Wrap(bmclibErrs.ErrRedfishUpdateService, "service disabled")
-	}
-
-	switch {
-	case updateService.MultipartHTTPPushURI != "":
-		return multipartHttpUpload, updateService.MultipartHTTPPushURI, nil
-	case updateService.HTTPPushURI != "":
-		return unstructuredHttpPush, updateService.HTTPPushURI, nil
-	}
-
-	return "", "", errors.Wrap(bmclibErrs.ErrRedfishUpdateService, "unsupported update method")
-}
-
-// pipeReaderFakeSeeker wraps the io.PipeReader and implements the io.Seeker interface
-// to meet the API requirements for the Gofish client https://github.com/stmcginnis/gofish/blob/46b1b33645ed1802727dc4df28f5d3c3da722b15/client.go#L434
-//
-// The Gofish method linked does not currently perform seeks and so a PR will be suggested
-// to change the method signature to accept an io.Reader instead.
-type pipeReaderFakeSeeker struct {
-	*io.PipeReader
-}
-
-// Seek impelements the io.Seeker interface only to panic if called
-func (p pipeReaderFakeSeeker) Seek(offset int64, whence int) (int64, error) {
-	return 0, errors.New("Seek() not implemented for fake pipe reader seeker.")
-}
-
-// multipartPayloadSize prepares a temporary multipart form to determine the form size
-//
-// It creates a temporary form without reading in the update file payload and returns
-// sizeOf(form) + sizeOf(update file)
-func multipartPayloadSize(payload *multipartPayload) (int64, *bytes.Buffer, error) {
-	body := &bytes.Buffer{}
-	form := multipart.NewWriter(body)
-
-	// Add UpdateParameters field part
-	part, err := updateParametersFormField("UpdateParameters", form)
-	if err != nil {
-		return 0, body, err
-	}
-
-	if _, err = io.Copy(part, bytes.NewReader(payload.updateParameters)); err != nil {
-		return 0, body, err
-	}
-
-	// Add updateFile form
-	_, err = form.CreateFormFile("UpdateFile", filepath.Base(payload.updateFile.Name()))
-	if err != nil {
-		return 0, body, err
-	}
-
-	// determine update file size
-	finfo, err := payload.updateFile.Stat()
-	if err != nil {
-		return 0, body, err
-	}
-
-	// add terminating boundary to multipart form
-	err = form.Close()
-	if err != nil {
-		return 0, body, err
-	}
-
-	return int64(body.Len()) + finfo.Size(), body, nil
-}
-
-// runRequestWithMultipartPayload is a copy of https://github.com/stmcginnis/gofish/blob/main/client.go#L349
-// with a change to add the UpdateParameters multipart form field with a json content type header
-// the resulting form ends up in this format
-//
-// Content-Length: 416
-// Content-Type: multipart/form-data; boundary=--------------------
-// ----1771f60800cb2801
-
-// --------------------------1771f60800cb2801
-// Content-Disposition: form-data; name="UpdateParameters"
-// Content-Type: application/json
-
-// {"Targets": [], "@Redfish.OperationApplyTime": "OnReset", "Oem":
-//  {}}
-// --------------------------1771f60800cb2801
-// Content-Disposition: form-data; name="UpdateFile"; filename="dum
-// myfile"
-// Content-Type: application/octet-stream
-
-// hey.
-// --------------------------1771f60800cb2801--
-func (c *Conn) runRequestWithMultipartPayload(url string, payload *multipartPayload) (*http.Response, error) {
-	if url == "" {
-		return nil, fmt.Errorf("unable to execute request, no target provided")
-	}
-
-	// A content-length header is passed in to indicate the payload size
-	//
-	// The Content-length is set explicitly since the payload is an io.Reader,
-	// https://github.com/golang/go/blob/ddad9b618cce0ed91d66f0470ddb3e12cfd7eeac/src/net/http/request.go#L861
-	//
-	// Without the content-length header the http client will set the Transfer-Encoding to 'chunked'
-	// and that does not work for some BMCs (iDracs).
-	contentLength, _, err := multipartPayloadSize(payload)
-	if err != nil {
-		return nil, errors.Wrap(err, "error determining multipart payload size")
-	}
-
-	headers := map[string]string{
-		"Content-Length": strconv.FormatInt(contentLength, 10),
-	}
-
-	// setup pipe
-	pipeReader, pipeWriter := io.Pipe()
-	defer pipeReader.Close()
-
-	// initiate a mulitpart writer
-	form := multipart.NewWriter(pipeWriter)
-
-	// go routine blocks on the io.Copy until the http request is made
-	go func() {
-		var err error
-		defer func() {
-			if err != nil {
-				c.Log.Error(err, "multipart upload error occurred")
-			}
-		}()
-
-		defer pipeWriter.Close()
-
-		// Add UpdateParameters part
-		parametersPart, err := updateParametersFormField("UpdateParameters", form)
-		if err != nil {
-			c.Log.Error(errMultiPartPayload, err.Error()+": UpdateParameters part copy error")
-
-			return
-		}
-
-		if _, err = io.Copy(parametersPart, bytes.NewReader(payload.updateParameters)); err != nil {
-			c.Log.Error(errMultiPartPayload, err.Error()+": UpdateParameters part copy error")
-
-			return
-		}
-
-		// Add UpdateFile part
-		updateFilePart, err := form.CreateFormFile("UpdateFile", filepath.Base(payload.updateFile.Name()))
-		if err != nil {
-			c.Log.Error(errMultiPartPayload, err.Error()+": UpdateFile part create error")
-
-			return
-		}
-
-		if _, err = io.Copy(updateFilePart, payload.updateFile); err != nil {
-			c.Log.Error(errMultiPartPayload, err.Error()+": UpdateFile part copy error")
-
-			return
-		}
-
-		// add terminating boundary to multipart form
-		form.Close()
-	}()
-
-	// pipeReader wrapped as a io.ReadSeeker to satisfy the gofish method signature
-	reader := pipeReaderFakeSeeker{pipeReader}
-
-	return c.redfishwrapper.RunRawRequestWithHeaders(http.MethodPost, url, reader, form.FormDataContentType(), headers)
-}
-
-// sets up the UpdateParameters MIMEHeader for the multipart form
-// the Go multipart writer CreateFormField does not currently let us set Content-Type on a MIME Header
-// https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/mime/multipart/writer.go;l=151
-func updateParametersFormField(fieldName string, writer *multipart.Writer) (io.Writer, error) {
-	if fieldName != "UpdateParameters" {
-		return nil, errors.New("")
-	}
-
-	h := make(textproto.MIMEHeader)
-	h.Set("Content-Disposition", `form-data; name="UpdateParameters"`)
-	h.Set("Content-Type", "application/json")
-
-	return writer.CreatePart(h)
-}
-
-// FirmwareInstallStatus returns the status of the firmware install task queued
-func (c *Conn) FirmwareInstallStatus(ctx context.Context, installVersion, component, taskID string) (state string, err error) {
-	vendor, _, err := c.redfishwrapper.DeviceVendorModel(ctx)
-	if err != nil {
-		return state, errors.Wrap(err, "unable to determine device vendor, model attributes")
-	}
-
-	// component is not used, we hack it for tests, easier than mocking
-	if component == "testOpenbmc" {
-		vendor = "defaultVendor"
-	}
-
-	var task *gofishrf.Task
-	switch {
-	case strings.Contains(vendor, constants.Dell):
-		task, err = c.dellJobAsRedfishTask(taskID)
-	default:
-		task, err = c.GetTask(taskID)
-	}
-
-	if err != nil {
-		return state, err
-	}
-
-	if task == nil {
-		return state, errors.New("failed to lookup task status for task ID: " + taskID)
-	}
-
-	state = strings.ToLower(string(task.TaskState))
-
-	// so much for standards...
-	switch state {
-	case "starting", "downloading", "downloaded":
-		return constants.FirmwareInstallInitializing, nil
-	case "running", "stopping", "cancelling", "scheduling":
-		return constants.FirmwareInstallRunning, nil
-	case "pending", "new":
-		return constants.FirmwareInstallQueued, nil
-	case "scheduled":
-		return constants.FirmwareInstallPowerCycleHost, nil
-	case "interrupted", "killed", "exception", "cancelled", "suspended", "failed":
-		return constants.FirmwareInstallFailed, nil
-	case "completed":
-		return constants.FirmwareInstallComplete, nil
-	default:
-		return constants.FirmwareInstallUnknown + ": " + state, nil
-	}
-
-}
diff --git a/providers/redfish/firmware_test.go b/providers/redfish/firmware_test.go
deleted file mode 100644
index 9af54a34..00000000
--- a/providers/redfish/firmware_test.go
+++ /dev/null
@@ -1,280 +0,0 @@
-package redfish
-
-import (
-	"context"
-	"encoding/json"
-	"fmt"
-	"io"
-	"log"
-	"net/http"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-
-	"github.com/bmc-toolbox/bmclib/v2/constants"
-	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
-	"github.com/bmc-toolbox/common"
-)
-
-// handler registered in mock_test.go
-func multipartUpload(w http.ResponseWriter, r *http.Request) {
-	if r.Method != "POST" {
-		w.WriteHeader(http.StatusNotFound)
-	}
-
-	body, err := io.ReadAll(r.Body)
-	if err != nil {
-		log.Fatal(err)
-	}
-
-	// payload size
-	expectedContentLength := "476"
-
-	expected := []string{
-		`Content-Disposition: form-data; name="UpdateParameters"`,
-		`Content-Type: application/json`,
-		`{"Targets":[],"@Redfish.OperationApplyTime":"OnReset","Oem":{}}`,
-		`Content-Disposition: form-data; name="UpdateFile"; filename="test.bin"`,
-		`Content-Type: application/octet-stream`,
-		`HELLOWORLD`,
-	}
-
-	for _, want := range expected {
-		if !strings.Contains(string(body), want) {
-			fmt.Println(string(body))
-			log.Fatal("expected value not in multipartUpload payload: " + string(want))
-		}
-	}
-
-	if r.Header.Get("Content-Length") != expectedContentLength {
-		log.Fatal("Header Content-Length does not match expected")
-	}
-
-	w.Header().Add("Location", "/redfish/v1/TaskService/Tasks/JID_467696020275")
-	w.WriteHeader(http.StatusAccepted)
-}
-
-func TestFirmwareInstall(t *testing.T) {
-	// curl -Lv -s -k -u root:calvin \
-	// -F 'UpdateParameters={"Targets": [], "@Redfish.OperationApplyTime": "OnReset", "Oem": {}};type=application/json' \
-	// -F'foo.bin=@/tmp/dummyfile;application/octet-stream'
-	// https://192.168.1.1/redfish/v1/UpdateService/MultipartUpload --trace-ascii /dev/stdout
-
-	tmpdir := t.TempDir()
-	binPath := filepath.Join(tmpdir, "test.bin")
-	err := os.WriteFile(binPath, []byte(`HELLOWORLD`), 0600)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	fh, err := os.Open(binPath)
-	if err != nil {
-		t.Fatalf("%s -> %s", err.Error(), binPath)
-	}
-
-	defer os.Remove(binPath)
-
-	tests := []struct {
-		component          string
-		applyAt            constants.OperationApplyTime
-		forceInstall       bool
-		setRequiredTimeout bool
-		reader             io.Reader
-		expectTaskID       string
-		expectErr          error
-		expectErrSubStr    string
-		testName           string
-	}{
-		{
-			common.SlugBIOS,
-			constants.OnReset,
-			false,
-			false,
-			nil,
-			"",
-			bmclibErrs.ErrFirmwareInstall,
-			"method expects an *os.File object",
-			"expect *os.File object",
-		},
-		{
-			common.SlugBIOS,
-			constants.OnReset,
-			false,
-			false,
-			&os.File{},
-			"",
-			errInsufficientCtxTimeout,
-			"",
-			"remaining context deadline",
-		},
-		{
-			common.SlugBIOS,
-			constants.OnReset,
-			false,
-			true,
-			fh,
-			"467696020275",
-			bmclibErrs.ErrFirmwareInstall,
-			"task for BIOS firmware install present",
-			"task ID exists",
-		},
-		{
-			common.SlugBIOS,
-			constants.OnReset,
-			true,
-			true,
-			fh,
-			"467696020275",
-			nil,
-			"task for BIOS firmware install present",
-			"task created (previous task purged with force)",
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.testName, func(t *testing.T) {
-			ctx, cancel := context.WithTimeout(context.TODO(), 1*time.Second)
-			if tc.setRequiredTimeout {
-				ctx, cancel = context.WithTimeout(context.TODO(), 20*time.Minute)
-			}
-
-			taskID, err := mockClient.FirmwareInstall(ctx, tc.component, string(tc.applyAt), tc.forceInstall, tc.reader)
-			if tc.expectErr != nil {
-				assert.ErrorIs(t, err, tc.expectErr)
-				if tc.expectErrSubStr != "" {
-					assert.ErrorContains(t, err, tc.expectErrSubStr)
-				}
-			} else {
-				assert.Nil(t, err)
-				assert.Equal(t, tc.expectTaskID, taskID)
-			}
-
-			defer cancel()
-		})
-	}
-
-}
-
-func TestMultipartPayloadSize(t *testing.T) {
-	updateParameters, err := json.Marshal(struct {
-		Targets            []string `json:"Targets"`
-		RedfishOpApplyTime string   `json:"@Redfish.OperationApplyTime"`
-		Oem                struct{} `json:"Oem"`
-	}{
-		[]string{},
-		"foobar",
-		struct{}{},
-	})
-
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	tmpdir := t.TempDir()
-	binPath := filepath.Join(tmpdir, "test.bin")
-	err = os.WriteFile(binPath, []byte(`HELLOWORLD`), 0600)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	testfileFH, err := os.Open(binPath)
-	if err != nil {
-		t.Fatalf("%s -> %s", err.Error(), binPath)
-	}
-
-	testCases := []struct {
-		testName     string
-		payload      *multipartPayload
-		expectedSize int64
-		errorMsg     string
-	}{
-		{
-			"content length as expected",
-			&multipartPayload{
-				updateParameters: updateParameters,
-				updateFile:       testfileFH,
-			},
-			475,
-			"",
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.testName, func(t *testing.T) {
-			gotSize, _, err := multipartPayloadSize(tc.payload)
-			if tc.errorMsg != "" {
-				assert.Contains(t, err.Error(), tc.errorMsg)
-			}
-
-			assert.Nil(t, err)
-			assert.Equal(t, tc.expectedSize, gotSize)
-		})
-	}
-}
-
-// referenced in main_test.go
-func openbmcStatus(w http.ResponseWriter, r *http.Request) {
-
-	if r.URL.Path != "/redfish/v1/TaskService/Tasks/15" {
-		// return an HTTP error, don't care to return correct data after
-		http.Error(w, "404 page not found:"+r.URL.Path, http.StatusNotFound)
-	}
-
-	mytask := `{
-  "@odata.id": "/redfish/v1/TaskService/Tasks/15",
-  "@odata.type": "#Task.v1_4_3.Task",
-  "Id": "15",
-  "Messages": [
-    {
-      "@odata.type": "#Message.v1_1_1.Message",
-      "Message": "The task with Id '15' has started.",
-      "MessageArgs": [
-        "15"
-      ],
-      "MessageId": "TaskEvent.1.0.3.TaskStarted",
-      "MessageSeverity": "OK",
-      "Resolution": "None."
-    }
-  ],
-  "Name": "Task 15",
-  "TaskState": "TestState",
-  "TaskStatus": "TestStatus"
-}
-`
-	_, _ = w.Write([]byte(mytask))
-
-}
-
-func Test_FirmwareInstall2(t *testing.T) {
-	state, err := mockClient.FirmwareInstallStatus(context.TODO(), "", "testOpenbmc", "15")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if state != "unknown: teststate" {
-		t.Fatal("Wrong test state:", state)
-	}
-}
-
-func Test_TaskIDFromLocationURI(t *testing.T) {
-	var task string
-	var err error
-
-	task, err = TaskIDFromLocationURI("/redfish/v1/TaskService/Tasks/JID_467696020275")
-	if err != nil || task != "467696020275" {
-		t.Fatal("Wrong task ID 467696020275. task,err=", task, err)
-	}
-
-	task, err = TaskIDFromLocationURI("/redfish/v1/TaskService/Tasks/12/Monitor")
-	if err != nil || task != "12" {
-		t.Fatal("Wrong task ID 12. task,err=", task, err)
-	}
-
-	task, err = TaskIDFromLocationURI("/redfish/v1/TaskService/Tasks/NO-TASK-ID")
-	if err == nil {
-		t.Fatal("Should return an error. task,err=", task, err)
-	}
-}

From 1cb2383b09db4bd039a767f2653fb811c1311640 Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 07:52:36 +0100
Subject: [PATCH 03/24] providers/redfish: moved GetBiosConfiguration method
 under redfishwrapper

The wrapper provides implementations other providers can call into for
code reuse
---
 {providers/redfish => internal/redfishwrapper}/bios.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
 rename {providers/redfish => internal/redfishwrapper}/bios.go (68%)

diff --git a/providers/redfish/bios.go b/internal/redfishwrapper/bios.go
similarity index 68%
rename from providers/redfish/bios.go
rename to internal/redfishwrapper/bios.go
index deb95ce5..a08be5a9 100644
--- a/providers/redfish/bios.go
+++ b/internal/redfishwrapper/bios.go
@@ -1,4 +1,4 @@
-package redfish
+package redfishwrapper
 
 import (
 	"context"
@@ -6,15 +6,15 @@ import (
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
 )
 
-func (c *Conn) GetBiosConfiguration(ctx context.Context) (biosConfig map[string]string, err error) {
-	systems, err := c.redfishwrapper.Systems()
+func (c *Client) GetBiosConfiguration(ctx context.Context) (biosConfig map[string]string, err error) {
+	systems, err := c.Systems()
 	if err != nil {
 		return nil, err
 	}
 
 	biosConfig = make(map[string]string)
 	for _, sys := range systems {
-		if !compatibleOdataID(sys.ODataID, systemsOdataIDs) {
+		if !c.compatibleOdataID(sys.ODataID, knownSystemsOdataIDs) {
 			continue
 		}
 

From f59502670cb30b0b1287ce362e9f5bae8cd5ad37 Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 07:54:27 +0100
Subject: [PATCH 04/24] redfishwrapper/power: move implementation here for
 re-use

---
 internal/redfishwrapper/power.go | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/internal/redfishwrapper/power.go b/internal/redfishwrapper/power.go
index f2df6b5a..53d412c8 100644
--- a/internal/redfishwrapper/power.go
+++ b/internal/redfishwrapper/power.go
@@ -11,6 +11,25 @@ import (
 	rf "github.com/stmcginnis/gofish/redfish"
 )
 
+// PowerSet sets the power state of a server
+func (c *Client) PowerSet(ctx context.Context, state string) (ok bool, err error) {
+	// TODO: create consts for the state values
+	switch strings.ToLower(state) {
+	case "on":
+		return c.SystemPowerOn(ctx)
+	case "off":
+		return c.SystemForceOff(ctx)
+	case "soft":
+		return c.SystemPowerOff(ctx)
+	case "reset":
+		return c.SystemReset(ctx)
+	case "cycle":
+		return c.SystemPowerCycle(ctx)
+	default:
+		return false, errors.New("unknown power action")
+	}
+}
+
 // BMCReset powercycles the BMC.
 func (c *Client) BMCReset(ctx context.Context, resetType string) (ok bool, err error) {
 	if err := c.SessionActive(); err != nil {

From 79d80d04bd9cd51df9edb9e0aedf4dd69ccbcbdf Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 07:56:18 +0100
Subject: [PATCH 05/24] providers/redfish: Inventory, FirmwareInstall,
 PowerSet, PowerState moved method internals

into the redfishwrapper so they can be reused by other providers
---
 providers/redfish/redfish.go | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/providers/redfish/redfish.go b/providers/redfish/redfish.go
index 458f1522..1dae9e80 100644
--- a/providers/redfish/redfish.go
+++ b/providers/redfish/redfish.go
@@ -4,14 +4,13 @@ import (
 	"context"
 	"crypto/x509"
 	"net/http"
-	"strings"
 
 	"github.com/bmc-toolbox/bmclib/v2/internal/httpclient"
 	"github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper"
 	"github.com/bmc-toolbox/bmclib/v2/providers"
+	"github.com/bmc-toolbox/common"
 	"github.com/go-logr/logr"
 	"github.com/jacobweinstock/registrar"
-	"github.com/pkg/errors"
 
 	"github.com/bmc-toolbox/bmclib/v2/bmc"
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
@@ -35,8 +34,6 @@ var (
 		providers.FeatureBootDeviceSet,
 		providers.FeatureVirtualMedia,
 		providers.FeatureInventoryRead,
-		providers.FeatureFirmwareInstall,
-		providers.FeatureFirmwareInstallStatus,
 		providers.FeatureBmcReset,
 		providers.FeatureClearSystemEventLog,
 	}
@@ -193,20 +190,7 @@ func (c *Conn) PowerStateGet(ctx context.Context) (state string, err error) {
 
 // PowerSet sets the power state of a server
 func (c *Conn) PowerSet(ctx context.Context, state string) (ok bool, err error) {
-	switch strings.ToLower(state) {
-	case "on":
-		return c.redfishwrapper.SystemPowerOn(ctx)
-	case "off":
-		return c.redfishwrapper.SystemForceOff(ctx)
-	case "soft":
-		return c.redfishwrapper.SystemPowerOff(ctx)
-	case "reset":
-		return c.redfishwrapper.SystemReset(ctx)
-	case "cycle":
-		return c.redfishwrapper.SystemPowerCycle(ctx)
-	default:
-		return false, errors.New("unknown power action")
-	}
+	return c.redfishwrapper.PowerSet(ctx, state)
 }
 
 // BootDeviceSet sets the boot device
@@ -223,3 +207,13 @@ func (c *Conn) BootDeviceOverrideGet(ctx context.Context) (bmc.BootDeviceOverrid
 func (c *Conn) SetVirtualMedia(ctx context.Context, kind string, mediaURL string) (ok bool, err error) {
 	return c.redfishwrapper.SetVirtualMedia(ctx, kind, mediaURL)
 }
+
+// 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, c.failInventoryOnError)
+}
+
+// GetBiosConfiguration return bios configuration
+func (c *Conn) GetBiosConfiguration(ctx context.Context) (biosConfig map[string]string, err error) {
+	return c.redfishwrapper.GetBiosConfiguration(ctx)
+}

From b9c4846135bc068f53e3982913e0ad85292e83df Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 09:29:29 +0100
Subject: [PATCH 06/24] redfishwrapper: minor lint fixes

---
 internal/redfishwrapper/power.go     |  1 +
 internal/redfishwrapper/system.go    | 12 ++++++------
 internal/redfishwrapper/task.go      |  5 +++--
 internal/redfishwrapper/task_test.go |  2 --
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/internal/redfishwrapper/power.go b/internal/redfishwrapper/power.go
index 53d412c8..7e91b8e7 100644
--- a/internal/redfishwrapper/power.go
+++ b/internal/redfishwrapper/power.go
@@ -212,6 +212,7 @@ func (c *Client) SystemForceOff(ctx context.Context) (ok bool, err error) {
 
 		system.DisableEtagMatch(c.disableEtagMatch)
 
+
 		err = system.Reset(rf.ForceOffResetType)
 		if err != nil {
 			return false, err
diff --git a/internal/redfishwrapper/system.go b/internal/redfishwrapper/system.go
index 5d2683a7..784e5681 100644
--- a/internal/redfishwrapper/system.go
+++ b/internal/redfishwrapper/system.go
@@ -6,13 +6,13 @@ import (
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
 
 	"github.com/pkg/errors"
-	gofishrf "github.com/stmcginnis/gofish/redfish"
+	redfish "github.com/stmcginnis/gofish/redfish"
 )
 
 // The methods here should be a thin wrapper so as to only guard the client from authentication failures.
 
 // AccountService gets the Redfish AccountService.d
-func (c *Client) AccountService() (*gofishrf.AccountService, error) {
+func (c *Client) AccountService() (*redfish.AccountService, error) {
 	if err := c.SessionActive(); err != nil {
 		return nil, errors.Wrap(bmclibErrs.ErrNotAuthenticated, err.Error())
 	}
@@ -21,7 +21,7 @@ func (c *Client) AccountService() (*gofishrf.AccountService, error) {
 }
 
 // UpdateService gets the update service instance.
-func (c *Client) UpdateService() (*gofishrf.UpdateService, error) {
+func (c *Client) UpdateService() (*redfish.UpdateService, error) {
 	if err := c.SessionActive(); err != nil {
 		return nil, errors.Wrap(bmclibErrs.ErrNotAuthenticated, err.Error())
 	}
@@ -30,7 +30,7 @@ func (c *Client) UpdateService() (*gofishrf.UpdateService, error) {
 }
 
 // Systems get the system instances from the service.
-func (c *Client) Systems() ([]*gofishrf.ComputerSystem, error) {
+func (c *Client) Systems() ([]*redfish.ComputerSystem, error) {
 	if err := c.SessionActive(); err != nil {
 		return nil, err
 	}
@@ -39,7 +39,7 @@ func (c *Client) Systems() ([]*gofishrf.ComputerSystem, error) {
 }
 
 // Managers gets the manager instances of this service.
-func (c *Client) Managers(ctx context.Context) ([]*gofishrf.Manager, error) {
+func (c *Client) Managers(ctx context.Context) ([]*redfish.Manager, error) {
 	if err := c.SessionActive(); err != nil {
 		return nil, errors.Wrap(bmclibErrs.ErrNotAuthenticated, err.Error())
 	}
@@ -48,7 +48,7 @@ func (c *Client) Managers(ctx context.Context) ([]*gofishrf.Manager, error) {
 }
 
 // Chassis gets the chassis instances managed by this service.
-func (c *Client) Chassis(ctx context.Context) ([]*gofishrf.Chassis, error) {
+func (c *Client) Chassis(ctx context.Context) ([]*redfish.Chassis, error) {
 	if err := c.SessionActive(); err != nil {
 		return nil, errors.Wrap(bmclibErrs.ErrNotAuthenticated, err.Error())
 	}
diff --git a/internal/redfishwrapper/task.go b/internal/redfishwrapper/task.go
index 592712f2..2a27596e 100644
--- a/internal/redfishwrapper/task.go
+++ b/internal/redfishwrapper/task.go
@@ -8,14 +8,15 @@ import (
 	"github.com/bmc-toolbox/bmclib/v2/constants"
 	bmclibErrs "github.com/bmc-toolbox/bmclib/v2/errors"
 	"github.com/pkg/errors"
-	gofishrf "github.com/stmcginnis/gofish/redfish"
+	"github.com/stmcginnis/gofish/common"
+	redfish "github.com/stmcginnis/gofish/redfish"
 )
 
 var (
 	errUnexpectedTaskState = errors.New("unexpected task state")
 )
 
-func (c *Client) Task(ctx context.Context, taskID string) (*gofishrf.Task, error) {
+func (c *Client) Task(ctx context.Context, taskID string) (*redfish.Task, error) {
 	tasks, err := c.Tasks(ctx)
 	if err != nil {
 		return nil, errors.Wrap(err, "error querying redfish tasks")
diff --git a/internal/redfishwrapper/task_test.go b/internal/redfishwrapper/task_test.go
index cd7fbabd..43aa5d26 100644
--- a/internal/redfishwrapper/task_test.go
+++ b/internal/redfishwrapper/task_test.go
@@ -2,7 +2,6 @@ package redfishwrapper
 
 import (
 	"context"
-	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -288,7 +287,6 @@ func TestTask(t *testing.T) {
 
 			got, err := client.Task(ctx, tc.taskID)
 			if tc.err != nil {
-				fmt.Println(err)
 				assert.ErrorContains(t, err, tc.err.Error())
 				return
 			}

From 05d943e1e6fd6544e2627b390a15d590f259aeea Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 09:30:19 +0100
Subject: [PATCH 07/24] redfishwrapper/task: include task message in info

This helps with debugging failed tasks
---
 internal/redfishwrapper/task.go | 34 ++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/internal/redfishwrapper/task.go b/internal/redfishwrapper/task.go
index 2a27596e..a60b974f 100644
--- a/internal/redfishwrapper/task.go
+++ b/internal/redfishwrapper/task.go
@@ -38,10 +38,38 @@ func (c *Client) TaskStatus(ctx context.Context, taskID string) (constants.TaskS
 	if err != nil {
 		return "", "", errors.Wrap(err, "error querying redfish for taskID: "+taskID)
 	}
-	taskInfo := fmt.Sprintf("id: %s, state: %s, status: %s", task.ID, task.TaskState, task.TaskStatus)
 
-	state := strings.ToLower(string(task.TaskState))
-	return c.ConvertTaskState(state), taskInfo, nil
+	taskInfo := fmt.Sprintf(
+		"id: %s, state: %s, status: %s",
+		task.ID,
+		task.TaskState,
+		task.TaskStatus,
+	)
+
+	// task message include information that help debug a cause of failure
+	if msgs := c.taskMessagesAsString(task.Messages); msgs != "" {
+		taskInfo += ", messages: " + msgs
+	}
+
+	s := c.ConvertTaskState(string(task.TaskState))
+	return s, taskInfo, nil
+}
+
+func (c *Client) taskMessagesAsString(messages []common.Message) string {
+	if len(messages) == 0 {
+		return ""
+	}
+
+	var found []string
+	for _, m := range messages {
+		if m.Message == "" {
+			continue
+		}
+
+		found = append(found, m.Message)
+	}
+
+	return strings.Join(found, ",")
 }
 
 func (c *Client) ConvertTaskState(state string) constants.TaskState {

From 619188e11f119fabb61e0d7f438c5fb43412b4d8 Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 09:31:00 +0100
Subject: [PATCH 08/24] redfishwrapper/task: lowercase task status before match

---
 internal/redfishwrapper/task.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/internal/redfishwrapper/task.go b/internal/redfishwrapper/task.go
index a60b974f..5932b0d1 100644
--- a/internal/redfishwrapper/task.go
+++ b/internal/redfishwrapper/task.go
@@ -73,7 +73,7 @@ func (c *Client) taskMessagesAsString(messages []common.Message) string {
 }
 
 func (c *Client) ConvertTaskState(state string) constants.TaskState {
-	switch state {
+	switch strings.ToLower(state) {
 	case "starting", "downloading", "downloaded":
 		return constants.Initializing
 	case "running", "stopping", "cancelling", "scheduling":

From d58aaece9c66d481b035339204b9183d1b7d7e2b Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 09:32:26 +0100
Subject: [PATCH 09/24] bmc/firmware: fix up inconsistent metadata obj init and
 error collection

---
 bmc/firmware.go | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/bmc/firmware.go b/bmc/firmware.go
index c6fb44ec..b068df0a 100644
--- a/bmc/firmware.go
+++ b/bmc/firmware.go
@@ -53,7 +53,7 @@ func firmwareInstall(ctx context.Context, component, operationApplyTime string,
 			taskID, vErr := elem.FirmwareInstall(ctx, component, operationApplyTime, forceInstall, reader)
 			if vErr != nil {
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
-				err = multierror.Append(err, vErr)
+				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
 
 			}
@@ -134,7 +134,7 @@ func firmwareInstallStatus(ctx context.Context, installVersion, component, taskI
 			status, vErr := elem.FirmwareInstallStatus(ctx, installVersion, component, taskID)
 			if vErr != nil {
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
-				err = multierror.Append(err, vErr)
+				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
 
 			}
@@ -213,7 +213,7 @@ func firmwareInstallUploaded(ctx context.Context, component, uploadTaskID string
 			installTaskID, vErr = elem.FirmwareInstallUploaded(ctx, component, uploadTaskID)
 			if vErr != nil {
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
-				err = multierror.Append(err, vErr)
+				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
 
 			}
@@ -310,7 +310,7 @@ func firmwareInstallSteps(ctx context.Context, component string, generic []firmw
 			steps, vErr := elem.FirmwareInstallSteps(ctx, component)
 			if vErr != nil {
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
-				err = multierror.Append(err, vErr)
+				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
 
 			}
@@ -362,7 +362,7 @@ func FirmwareUploadFromInterfaces(ctx context.Context, component string, file *o
 }
 
 func firmwareUpload(ctx context.Context, component string, file *os.File, generic []firmwareUploaderProvider) (taskID string, metadata Metadata, err error) {
-	var metadataLocal Metadata
+	metadata = newMetadata()
 
 	for _, elem := range generic {
 		if elem.FirmwareUploader == nil {
@@ -374,20 +374,20 @@ func firmwareUpload(ctx context.Context, component string, file *os.File, generi
 
 			return taskID, metadata, err
 		default:
-			metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name)
+			metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name)
 			taskID, vErr := elem.FirmwareUpload(ctx, component, file)
 			if vErr != nil {
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
-				err = multierror.Append(err, vErr)
+				metadata.FailedProviderDetail[elem.name] = err.Error()
 				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 FirmwareUpload"))
+	return taskID, metadata, multierror.Append(err, errors.New("failure in FirmwareUpload"))
 }
 
 // FirmwareTaskVerifier defines an interface to check the status for firmware related tasks queued on the BMC.
@@ -416,8 +416,9 @@ type firmwareTaskVerifierProvider struct {
 }
 
 // firmwareTaskStatus returns the status of the firmware upload process.
+
 func firmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, component, taskID, installVersion string, generic []firmwareTaskVerifierProvider) (state constants.TaskState, status string, metadata Metadata, err error) {
-	var metadataLocal Metadata
+	metadata = newMetadata()
 
 	for _, elem := range generic {
 		if elem.FirmwareTaskVerifier == nil {
@@ -429,20 +430,20 @@ func firmwareTaskStatus(ctx context.Context, kind bconsts.FirmwareInstallStep, c
 
 			return state, status, metadata, err
 		default:
-			metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name)
+			metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name)
 			state, status, vErr := elem.FirmwareTaskStatus(ctx, kind, component, taskID, installVersion)
 			if vErr != nil {
 				err = multierror.Append(err, errors.WithMessagef(vErr, "provider: %v", elem.name))
-				err = multierror.Append(err, vErr)
+				metadata.FailedProviderDetail[elem.name] = err.Error()
 				continue
-
 			}
-			metadataLocal.SuccessfulProvider = elem.name
-			return state, status, metadataLocal, nil
+
+			metadata.SuccessfulProvider = elem.name
+			return state, status, metadata, nil
 		}
 	}
 
-	return state, status, metadataLocal, multierror.Append(err, errors.New("failure in FirmwareTaskStatus"))
+	return state, status, metadata, multierror.Append(err, errors.New("failure in FirmwareTaskStatus"))
 }
 
 // FirmwareTaskStatusFromInterfaces identifies implementations of the FirmwareTaskVerifier interface and passes the found implementations to the firmwareTaskStatus() wrapper.

From 3e6a6f1c44320090914d749dfec321dbbcba614a Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 09:48:52 +0100
Subject: [PATCH 10/24] providers/redfish: purge un-used methods

---
 providers/redfish/main_test.go |  2 --
 providers/redfish/tasks.go     | 21 ---------------------
 2 files changed, 23 deletions(-)

diff --git a/providers/redfish/main_test.go b/providers/redfish/main_test.go
index 379292ba..17a525a7 100644
--- a/providers/redfish/main_test.go
+++ b/providers/redfish/main_test.go
@@ -57,9 +57,7 @@ func TestMain(m *testing.M) {
 		handler := http.NewServeMux()
 		handler.HandleFunc("/redfish/v1/", serviceRoot)
 		handler.HandleFunc("/redfish/v1/SessionService/Sessions", sessionService)
-		handler.HandleFunc("/redfish/v1/UpdateService/MultipartUpload", multipartUpload)
 		handler.HandleFunc("/redfish/v1/Managers/iDRAC.Embedded.1/Oem/Dell/Jobs?$expand=*($levels=1)", dellJobs)
-		handler.HandleFunc("/redfish/v1/TaskService/Tasks/", openbmcStatus)
 
 		return httptest.NewTLSServer(handler)
 	}()
diff --git a/providers/redfish/tasks.go b/providers/redfish/tasks.go
index c5450980..394f32e8 100644
--- a/providers/redfish/tasks.go
+++ b/providers/redfish/tasks.go
@@ -99,27 +99,6 @@ func (c *Conn) GetFirmwareInstallTaskQueued(ctx context.Context, component strin
 	return task, nil
 }
 
-// purgeQueuedFirmwareInstallTask removes any existing queued firmware install task for the given component slug
-func (c *Conn) purgeQueuedFirmwareInstallTask(ctx context.Context, component string) error {
-	vendor, _, err := c.redfishwrapper.DeviceVendorModel(ctx)
-	if err != nil {
-		return errors.Wrap(err, "unable to determine device vendor, model attributes")
-	}
-
-	// check an update task for the component is currently scheduled
-	switch {
-	case strings.Contains(vendor, constants.Dell):
-		err = c.dellPurgeScheduledFirmwareInstallJob(component)
-	default:
-		err = errors.Wrap(
-			bmclibErrs.ErrFirmwareInstall,
-			"Update is already running",
-		)
-	}
-
-	return err
-}
-
 // 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)

From b7f3de9920aa7d1c4f193f5e6cad96da8f6a7e8d Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 09:56:44 +0100
Subject: [PATCH 11/24] supermicro: implement Inventory, PowerSet,
 PowerStateGet methods

---
 providers/supermicro/docs/x11.md   | 31 ++++++++++++++++++++++++++++++
 providers/supermicro/supermicro.go | 31 ++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)
 create mode 100644 providers/supermicro/docs/x11.md

diff --git a/providers/supermicro/docs/x11.md b/providers/supermicro/docs/x11.md
new file mode 100644
index 00000000..bd8f6d67
--- /dev/null
+++ b/providers/supermicro/docs/x11.md
@@ -0,0 +1,31 @@
+#### x11 XML API power commands
+
+power-off - immediate - `op=POWER_INFO.XML&r=(1,0)&_=`
+power-on - `op=POWER_INFO.XML&r=(1,1)&_=`
+power-off - `acpi/orderly - op=POWER_INFO.XML&r=(1,5)&_=`
+reset server - cold powercycle - `op=POWER_INFO.XML&r=(1,3)&_=`
+power cycle - `op=POWER_INFO.XML&r=(1,2)&_=`
+
+
+ref invocation
+```go
+// powerCycle using SMC XML API
+func (c *x11) powerCycle(ctx context.Context) (bool, error) {
+	payload := []byte(`op=POWER_INFO.XML&r=(1,3)&_=`)
+
+	headers := map[string]string{
+		"Content-type": "application/x-www-form-urlencoded; charset=UTF-8",
+	}
+
+	body, status, err := c.serviceClient.query(ctx, "cgi/ipmi.cgi", http.MethodPost, bytes.NewBuffer(payload), headers, 0)
+	if err != nil {
+		return false, err
+	}
+
+	if status != http.StatusOK {
+		return false, unexpectedResponseErr(payload, body, status)
+	}
+
+	return true, nil
+}
+```
diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go
index 47f167b9..575d7e2f 100644
--- a/providers/supermicro/supermicro.go
+++ b/providers/supermicro/supermicro.go
@@ -20,6 +20,7 @@ import (
 	"github.com/bmc-toolbox/bmclib/v2/internal/httpclient"
 	"github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper"
 	"github.com/bmc-toolbox/bmclib/v2/providers"
+	"github.com/bmc-toolbox/common"
 
 	"github.com/go-logr/logr"
 	"github.com/jacobweinstock/registrar"
@@ -46,6 +47,9 @@ var (
 		providers.FeatureFirmwareInstallUploaded,
 		providers.FeatureFirmwareTaskStatus,
 		providers.FeatureFirmwareInstallSteps,
+		providers.FeatureInventoryRead,
+		providers.FeaturePowerSet,
+		providers.FeaturePowerState,
 	}
 )
 
@@ -184,6 +188,33 @@ func (c *Client) Open(ctx context.Context) (err error) {
 	return nil
 }
 
+// PowerStateGet gets the power state of a BMC machine
+func (c *Client) PowerStateGet(ctx context.Context) (state string, err error) {
+	if c.serviceClient == nil || c.serviceClient.redfish == nil {
+		return "", errors.Wrap(bmclibErrs.ErrLoginFailed, "client not initialized")
+	}
+
+	return c.serviceClient.redfish.SystemPowerStatus(ctx)
+}
+
+// PowerSet sets the power state of a server
+func (c *Client) PowerSet(ctx context.Context, state string) (ok bool, err error) {
+	if c.serviceClient == nil || c.serviceClient.redfish == nil {
+		return false, errors.Wrap(bmclibErrs.ErrLoginFailed, "client not initialized")
+	}
+
+	return c.serviceClient.redfish.PowerSet(ctx, state)
+}
+
+// Inventory collects hardware inventory and install firmware information
+func (c *Client) Inventory(ctx context.Context) (device *common.Device, err error) {
+	if c.serviceClient == nil || c.serviceClient.redfish == nil {
+		return nil, errors.Wrap(bmclibErrs.ErrLoginFailed, "client not initialized")
+	}
+
+	return c.serviceClient.redfish.Inventory(ctx, false)
+}
+
 func (c *Client) bmcQueryor(ctx context.Context) (bmcQueryor, error) {
 	x11 := newX11Client(c.serviceClient, c.log)
 	x12 := newX12Client(c.serviceClient, c.log)

From bbc734ded491690a4e254d77137448e27a489c83 Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 09:57:17 +0100
Subject: [PATCH 12/24] providers/supermicro: fix up redfish session init and
 purge unused methods

---
 providers/supermicro/floppy.go     |  2 +-
 providers/supermicro/supermicro.go | 51 +++---------------------------
 2 files changed, 5 insertions(+), 48 deletions(-)

diff --git a/providers/supermicro/floppy.go b/providers/supermicro/floppy.go
index 3c652890..835b70f4 100644
--- a/providers/supermicro/floppy.go
+++ b/providers/supermicro/floppy.go
@@ -20,7 +20,7 @@ var (
 )
 
 func (c *Client) floppyImageMounted(ctx context.Context) (bool, error) {
-	if err := c.openRedfish(ctx); err != nil {
+	if err := c.serviceClient.redfishSession(ctx); err != nil {
 		return false, err
 	}
 
diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go
index 575d7e2f..7add3b80 100644
--- a/providers/supermicro/supermicro.go
+++ b/providers/supermicro/supermicro.go
@@ -249,21 +249,6 @@ func (c *Client) bmcQueryor(ctx context.Context) (bmcQueryor, error) {
 	return queryor, nil
 }
 
-func (c *Client) openRedfish(ctx context.Context) error {
-	if c.serviceClient.redfish != nil && c.serviceClient.redfish.SessionActive() == nil {
-		return nil
-	}
-
-	rfclient := redfishwrapper.NewClient(c.serviceClient.host, "", c.serviceClient.user, c.serviceClient.pass)
-	if err := rfclient.Open(ctx); err != nil {
-		return err
-	}
-
-	c.serviceClient.redfish = rfclient
-
-	return nil
-}
-
 func parseToken(body []byte) string {
 	var key string
 	if bytes.Contains(body, []byte(`CSRF-TOKEN`)) {
@@ -380,38 +365,6 @@ func (c *Client) initScreenPreview(ctx context.Context) error {
 	return nil
 }
 
-// PowerSet sets the power state of a server
-func (c *Client) PowerSet(ctx context.Context, state string) (ok bool, err error) {
-	switch strings.ToLower(state) {
-	case "cycle":
-		return c.powerCycle(ctx)
-	default:
-		return false, errors.New("action not implemented for provider")
-	}
-}
-
-// powerCycle using SMC XML API
-//
-// This method is only here for the case when firmware updates are being applied using this provider.
-func (c *Client) powerCycle(ctx context.Context) (bool, error) {
-	payload := []byte(`op=SET_POWER_INFO.XML&r=(1,3)&_=`)
-
-	headers := map[string]string{
-		"Content-type": "application/x-www-form-urlencoded; charset=UTF-8",
-	}
-
-	body, status, err := c.serviceClient.query(ctx, "cgi/ipmi.cgi", http.MethodPost, bytes.NewBuffer(payload), headers, 0)
-	if err != nil {
-		return false, err
-	}
-
-	if status != http.StatusOK {
-		return false, unexpectedResponseErr(payload, body, status)
-	}
-
-	return true, nil
-}
-
 type serviceClient struct {
 	host      string
 	port      string
@@ -435,6 +388,10 @@ func (c *serviceClient) setCsrfToken(t string) {
 }
 
 func (c *serviceClient) redfishSession(ctx context.Context) (err error) {
+	if c.redfish != nil && c.redfish.SessionActive() == nil {
+		return nil
+	}
+
 	c.redfish = redfishwrapper.NewClient(c.host, "", c.user, c.pass, redfishwrapper.WithHTTPClient(c.client))
 	if err := c.redfish.Open(ctx); err != nil {
 		return err

From d22930ad1be3ee3309224dbe05a07a40f61c565d Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Tue, 28 Nov 2023 10:14:11 +0100
Subject: [PATCH 13/24] providers/supermicro: fix TestOpen()

---
 .../supermicro/fixtures/serviceroot.json      | 62 +++++++++++++++++++
 providers/supermicro/supermicro.go            | 12 +++-
 providers/supermicro/supermicro_test.go       | 48 +++++++++++++-
 3 files changed, 120 insertions(+), 2 deletions(-)
 create mode 100644 providers/supermicro/fixtures/serviceroot.json

diff --git a/providers/supermicro/fixtures/serviceroot.json b/providers/supermicro/fixtures/serviceroot.json
new file mode 100644
index 00000000..11078082
--- /dev/null
+++ b/providers/supermicro/fixtures/serviceroot.json
@@ -0,0 +1,62 @@
+{
+    "@odata.type": "#ServiceRoot.v1_5_2.ServiceRoot",
+    "@odata.id": "/redfish/v1",
+    "Id": "ServiceRoot",
+    "Name": "Root Service",
+    "RedfishVersion": "1.9.0",
+    "UUID": "00000000-0000-0000-0000-3CECEFCEFEDA",
+    "Systems": {
+        "@odata.id": "/redfish/v1/Systems"
+    },
+    "Chassis": {
+        "@odata.id": "/redfish/v1/Chassis"
+    },
+    "Managers": {
+        "@odata.id": "/redfish/v1/Managers"
+    },
+    "Tasks": {
+        "@odata.id": "/redfish/v1/TaskService"
+    },
+    "SessionService": {
+        "@odata.id": "/redfish/v1/SessionService"
+    },
+    "AccountService": {
+        "@odata.id": "/redfish/v1/AccountService"
+    },
+    "EventService": {
+        "@odata.id": "/redfish/v1/EventService"
+    },
+    "UpdateService": {
+        "@odata.id": "/redfish/v1/UpdateService"
+    },
+    "CertificateService": {
+        "@odata.id": "/redfish/v1/CertificateService"
+    },
+    "Registries": {
+        "@odata.id": "/redfish/v1/Registries"
+    },
+    "JsonSchemas": {
+        "@odata.id": "/redfish/v1/JsonSchemas"
+    },
+    "TelemetryService": {
+        "@odata.id": "/redfish/v1/TelemetryService"
+    },
+    "Links": {
+        "Sessions": {
+            "@odata.id": "/redfish/v1/SessionService/Sessions"
+        }
+    },
+    "ProtocolFeaturesSupported": {
+        "FilterQuery": true,
+        "SelectQuery": true,
+        "ExcerptQuery": false,
+        "OnlyMemberQuery": false,
+        "ExpandQuery": {
+            "Links": true,
+            "NoLinks": true,
+            "ExpandAll": true,
+            "Levels": true,
+            "MaxLevels": 2
+        }
+    }
+}
\ No newline at end of file
diff --git a/providers/supermicro/supermicro.go b/providers/supermicro/supermicro.go
index 7add3b80..b201e374 100644
--- a/providers/supermicro/supermicro.go
+++ b/providers/supermicro/supermicro.go
@@ -185,6 +185,10 @@ func (c *Client) Open(ctx context.Context) (err error) {
 		return errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error())
 	}
 
+	if err := c.serviceClient.redfishSession(ctx); err != nil {
+		return errors.Wrap(bmclibErrs.ErrLoginFailed, err.Error())
+	}
+
 	return nil
 }
 
@@ -392,7 +396,13 @@ func (c *serviceClient) redfishSession(ctx context.Context) (err error) {
 		return nil
 	}
 
-	c.redfish = redfishwrapper.NewClient(c.host, "", c.user, c.pass, redfishwrapper.WithHTTPClient(c.client))
+	c.redfish = redfishwrapper.NewClient(
+		c.host,
+		c.port,
+		c.user,
+		c.pass,
+		redfishwrapper.WithHTTPClient(c.client),
+	)
 	if err := c.redfish.Open(ctx); err != nil {
 		return err
 	}
diff --git a/providers/supermicro/supermicro_test.go b/providers/supermicro/supermicro_test.go
index ca0313c8..f503813a 100644
--- a/providers/supermicro/supermicro_test.go
+++ b/providers/supermicro/supermicro_test.go
@@ -7,12 +7,18 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"os"
 	"testing"
 
+	"github.com/bmc-toolbox/bmclib/v2/internal/redfishwrapper"
 	"github.com/go-logr/logr"
 	"github.com/stretchr/testify/assert"
 )
 
+const (
+	fixturesDir = "./fixtures"
+)
+
 func TestParseToken(t *testing.T) {
 	testcases := []struct {
 		name        string
@@ -66,6 +72,37 @@ func TestParseToken(t *testing.T) {
 	}
 }
 
+func mustReadFile(t *testing.T, filename string) []byte {
+	t.Helper()
+
+	fixture := fixturesDir + "/" + filename
+	fh, err := os.Open(fixture)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	defer fh.Close()
+
+	b, err := io.ReadAll(fh)
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	return b
+}
+
+var endpointFunc = func(t *testing.T, file string) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		// expect either GET or Delete methods
+		if r.Method != http.MethodGet && r.Method != http.MethodPost && r.Method != http.MethodDelete {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+
+		_, _ = w.Write(mustReadFile(t, file))
+	}
+}
+
 func TestOpen(t *testing.T) {
 	type handlerFuncMap map[string]func(http.ResponseWriter, *http.Request)
 	testcases := []struct {
@@ -84,6 +121,7 @@ func TestOpen(t *testing.T) {
 				"/": func(w http.ResponseWriter, r *http.Request) {
 					w.WriteHeader(http.StatusOK)
 				},
+				"/redfish/v1/": endpointFunc(t, "serviceroot.json"),
 				// first request to login
 				"/cgi/login.cgi": func(w http.ResponseWriter, r *http.Request) {
 					assert.Equal(t, r.Method, http.MethodPost)
@@ -182,13 +220,21 @@ func TestOpen(t *testing.T) {
 			server := httptest.NewTLSServer(mux)
 			defer server.Close()
 
-			server.Config.ErrorLog = log.Default()
+			server.Config.ErrorLog = log.New(os.Stdout, "foo", 3)
 			parsedURL, err := url.Parse(server.URL)
 			if err != nil {
 				t.Fatal(err)
 			}
 
 			client := NewClient(parsedURL.Hostname(), tc.user, tc.pass, logr.Discard(), WithPort(parsedURL.Port()))
+			client.serviceClient.redfish = redfishwrapper.NewClient(
+				parsedURL.Hostname(),
+				parsedURL.Port(),
+				tc.user,
+				tc.pass,
+				redfishwrapper.WithHTTPClient(client.serviceClient.client),
+			)
+
 			err = client.Open(context.Background())
 			if tc.errorContains != "" {
 				assert.ErrorContains(t, err, tc.errorContains)

From 5622adc5fa87ceda7cfc2c18cf6585fe5424d4c9 Mon Sep 17 00:00:00 2001
From: Joel Rebello <jrebello@packet.com>
Date: Mon, 27 Nov 2023 11:17:04 +0100
Subject: [PATCH 14/24] 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 <jrebello@packet.com>
Date: Mon, 27 Nov 2023 11:21:00 +0100
Subject: [PATCH 15/24] 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 <jrebello@packet.com>
Date: Mon, 27 Nov 2023 13:44:28 +0100
Subject: [PATCH 16/24] 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 <jrebello@packet.com>
Date: Mon, 27 Nov 2023 14:32:29 +0100
Subject: [PATCH 17/24] 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 <jrebello@packet.com>
Date: Mon, 27 Nov 2023 14:36:51 +0100
Subject: [PATCH 18/24] 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 <jrebello@packet.com>
Date: Mon, 27 Nov 2023 14:39:27 +0100
Subject: [PATCH 19/24] 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 <jrebello@packet.com>
Date: Mon, 27 Nov 2023 14:46:15 +0100
Subject: [PATCH 20/24] 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 <jrebello@packet.com>
Date: Tue, 28 Nov 2023 10:23:54 +0100
Subject: [PATCH 21/24] 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 <jrebello@packet.com>
Date: Tue, 28 Nov 2023 10:24:25 +0100
Subject: [PATCH 22/24] 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 <jrebello@packet.com>
Date: Tue, 28 Nov 2023 10:25:03 +0100
Subject: [PATCH 23/24] 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 <jrebello@packet.com>
Date: Tue, 28 Nov 2023 10:25:36 +0100
Subject: [PATCH 24/24] 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))