Refactor switch statements, error message, and pointer returns

bmc/boot_device.go

@@ -16,7 +16,7 @@ type BootDeviceSetter interface {
 
 // BootDeviceOverrideGetter gets boot override settings for a machine
 type BootDeviceOverrideGetter interface {
-	BootDeviceOverrideGet(ctx context.Context) (override *BootDeviceOverride, err error)
+	BootDeviceOverrideGet(ctx context.Context) (override BootDeviceOverride, err error)
 }
 
 // bootDeviceProviders is an internal struct to correlate an implementation/provider and its name
@@ -27,8 +27,8 @@ type bootDeviceProviders struct {
 
 // bootOverrideProvider is an internal struct to correlate an implementation/provider and its name
 type bootOverrideProvider struct {
-	name          string
-	bootOverrider BootDeviceOverrideGetter
+	name               string
+	bootOverrideGetter BootDeviceOverrideGetter
 }
 
 type BootDeviceOverride struct {
@@ -42,9 +42,7 @@ type BootDeviceOverride struct {
 // setPersistent persists the next boot device.
 // efiBoot sets up the device to boot off UEFI instead of legacy.
 func setBootDevice(ctx context.Context, timeout time.Duration, bootDevice string, setPersistent, efiBoot bool, b []bootDeviceProviders) (ok bool, metadata Metadata, err error) {
-	metadataLocal := Metadata{
-		FailedProviderDetail: make(map[string]string),
-	}
+	metadataLocal := newMetadata()
 
 	for _, elem := range b {
 		if elem.bootDeviceSetter == nil {
@@ -103,24 +101,24 @@ func getBootDeviceOverride(
 	timeout time.Duration,
 	provider *bootOverrideProvider,
 	metadata *Metadata,
-) (override *BootDeviceOverride, err error) {
+) (override BootDeviceOverride, ok bool, err error) {
 	select {
 	case <-ctx.Done():
 		err = multierror.Append(err, ctx.Err())
-		return nil, err
+		return override, ok, err
 	default:
 		metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, provider.name)
 		ctx, cancel := context.WithTimeout(ctx, timeout)
 		defer cancel()
 
-		override, err = provider.bootOverrider.BootDeviceOverrideGet(ctx)
+		override, err = provider.bootOverrideGetter.BootDeviceOverrideGet(ctx)
 		if err != nil {
 			metadata.FailedProviderDetail[provider.name] = err.Error()
-			return nil, nil
+			return override, ok, nil
 		}
 
 		metadata.SuccessfulProvider = provider.name
-		return override, nil
+		return override, true, nil
 	}
 }
 
@@ -130,19 +128,15 @@ func GetBootDeviceOverrideFromInterface(
 	ctx context.Context,
 	timeout time.Duration,
 	providers []interface{},
-) (*BootDeviceOverride, Metadata, error) {
-	var err error
-	metadata := Metadata{
-		FailedProviderDetail: make(map[string]string),
-	}
+) (override BootDeviceOverride, metadata Metadata, err error) {
+	metadata = newMetadata()
 
 	for _, elem := range providers {
-		provider := &bootOverrideProvider{name: getProviderName(elem)}
 		switch p := elem.(type) {
 		case BootDeviceOverrideGetter:
-			provider.bootOverrider = p
-			override, getErr := getBootDeviceOverride(ctx, timeout, provider, &metadata)
-			if getErr != nil || override != nil {
+			provider := &bootOverrideProvider{name: getProviderName(elem), bootOverrideGetter: p}
+			override, ok, getErr := getBootDeviceOverride(ctx, timeout, provider, &metadata)
+			if getErr != nil || ok {
 				return override, metadata, getErr
 			}
 		default:
@@ -157,5 +151,5 @@ func GetBootDeviceOverrideFromInterface(
 		err = multierror.Append(err, errors.New("failed to get boot device override settings"))
 	}
 
-	return nil, metadata, err
+	return override, metadata, err
 }

bmc/boot_device_test.go

@@ -121,20 +121,20 @@ func TestSetBootDeviceFromInterfaces(t *testing.T) {
 }
 
 type mockBootDeviceOverrideGetter struct {
-	overrideReturn *BootDeviceOverride
+	overrideReturn BootDeviceOverride
 	errReturn      error
 }
 
 func (m *mockBootDeviceOverrideGetter) Name() string {
 	return "Mock"
 }
 
-func (m *mockBootDeviceOverrideGetter) BootDeviceOverrideGet(_ context.Context) (override *BootDeviceOverride, err error) {
+func (m *mockBootDeviceOverrideGetter) BootDeviceOverrideGet(_ context.Context) (BootDeviceOverride, error) {
 	return m.overrideReturn, m.errReturn
 }
 
 func TestBootDeviceOverrideGet(t *testing.T) {
-	successOverride := &BootDeviceOverride{
+	successOverride := BootDeviceOverride{
 		IsPersistent: false,
 		IsEFIBoot:    true,
 		Device:       "disk",
@@ -172,7 +172,7 @@ func TestBootDeviceOverrideGet(t *testing.T) {
 		name             string
 		expectedErrorMsg string
 		expectedMetadata *Metadata
-		expectedOverride *BootDeviceOverride
+		expectedOverride BootDeviceOverride
 		getters          []interface{}
 	}{
 		{

client.go

@@ -455,7 +455,8 @@ func (c *Client) ReadUsers(ctx context.Context) (users []map[string]string, err
 	return users, err
 }
 
-func (c *Client) GetBootDeviceOverride(ctx context.Context) (override *bmc.BootDeviceOverride, err error) {
+// GetBootDeviceOverride pass through to library function
+func (c *Client) GetBootDeviceOverride(ctx context.Context) (override bmc.BootDeviceOverride, err error) {
 	override, metadata, err := bmc.GetBootDeviceOverrideFromInterface(ctx, c.perProviderTimeout(ctx), c.registry().GetDriverInterfaces())
 	c.setMetadata(metadata)
 	return override, err

errors/errors.go

@@ -118,8 +118,8 @@ var (
 	// ErrSystemVendorModel is returned when the system vendor, model attributes could not be identified.
 	ErrSystemVendorModel = errors.New("error identifying system vendor, model attributes")
 
-	// ErrNoSystemsAvailable is returned when the API of the device provides and empty array of systems.
-	ErrNoSystemsAvailable = errors.New("no systems were found on the device")
+	// ErrRedfishNoSystems is returned when the API of the device provides and empty array of systems.
+	ErrRedfishNoSystems = errors.New("redfish: no Systems were found on the device")
 )
 
 type ErrUnsupportedHardware struct {

internal/redfishwrapper/boot_device.go

@@ -9,6 +9,80 @@ import (
 	rf "github.com/stmcginnis/gofish/redfish"
 )
 
+type bootDeviceType struct {
+	Name          string
+	RedFishTarget rf.BootSourceOverrideTarget
+}
+
+var bootDeviceTypes = []bootDeviceType{
+	{
+		Name:          "bios",
+		RedFishTarget: rf.BiosSetupBootSourceOverrideTarget,
+	},
+	{
+		Name:          "cdrom",
+		RedFishTarget: rf.CdBootSourceOverrideTarget,
+	},
+	{
+		Name:          "diag",
+		RedFishTarget: rf.DiagsBootSourceOverrideTarget,
+	},
+	{
+		Name:          "floppy",
+		RedFishTarget: rf.FloppyBootSourceOverrideTarget,
+	},
+	{
+		Name:          "disk",
+		RedFishTarget: rf.HddBootSourceOverrideTarget,
+	},
+	{
+		Name:          "none",
+		RedFishTarget: rf.NoneBootSourceOverrideTarget,
+	},
+	{
+		Name:          "pxe",
+		RedFishTarget: rf.PxeBootSourceOverrideTarget,
+	},
+	{
+		Name:          "remote_drive",
+		RedFishTarget: rf.RemoteDriveBootSourceOverrideTarget,
+	},
+	{
+		Name:          "sd_card",
+		RedFishTarget: rf.SDCardBootSourceOverrideTarget,
+	},
+	{
+		Name:          "usb",
+		RedFishTarget: rf.UsbBootSourceOverrideTarget,
+	},
+	{
+		Name:          "utilities",
+		RedFishTarget: rf.UtilitiesBootSourceOverrideTarget,
+	},
+}
+
+// bootDeviceToTarget gets the RedFish BootSourceOverrideTarget that corresponds to the given device,
+// or an error if the device is not a RedFish BootSourceOverrideTarget.
+func bootDeviceToTarget(device string) (rf.BootSourceOverrideTarget, error) {
+	for _, bootDevice := range bootDeviceTypes {
+		if bootDevice.Name == device {
+			return bootDevice.RedFishTarget, nil
+		}
+	}
+	return "", errors.New("invalid boot device")
+}
+
+// bootTargetToDevice converts the redfish boot target to a bmclib supported device string.
+// if the target is unknown or unsupported, then an error is returned.
+func bootTargetToDevice(target rf.BootSourceOverrideTarget) (string, error) {
+	for _, bootDevice := range bootDeviceTypes {
+		if bootDevice.RedFishTarget == target {
+			return bootDevice.Name, nil
+		}
+	}
+	return "", errors.New("invalid boot device")
+}
+
 // SystemBootDeviceSet set the boot device for the system.
 func (c *Client) SystemBootDeviceSet(_ context.Context, bootDevice string, setPersistent, efiBoot bool) (ok bool, err error) {
 	if err := c.SessionActive(); err != nil {
@@ -23,31 +97,9 @@ func (c *Client) SystemBootDeviceSet(_ context.Context, bootDevice string, setPe
 	for _, system := range systems {
 		boot := system.Boot
 
-		switch bootDevice {
-		case "bios":
-			boot.BootSourceOverrideTarget = rf.BiosSetupBootSourceOverrideTarget
-		case "cdrom":
-			boot.BootSourceOverrideTarget = rf.CdBootSourceOverrideTarget
-		case "diag":
-			boot.BootSourceOverrideTarget = rf.DiagsBootSourceOverrideTarget
-		case "floppy":
-			boot.BootSourceOverrideTarget = rf.FloppyBootSourceOverrideTarget
-		case "disk":
-			boot.BootSourceOverrideTarget = rf.HddBootSourceOverrideTarget
-		case "none":
-			boot.BootSourceOverrideTarget = rf.NoneBootSourceOverrideTarget
-		case "pxe":
-			boot.BootSourceOverrideTarget = rf.PxeBootSourceOverrideTarget
-		case "remote_drive":
-			boot.BootSourceOverrideTarget = rf.RemoteDriveBootSourceOverrideTarget
-		case "sd_card":
-			boot.BootSourceOverrideTarget = rf.SDCardBootSourceOverrideTarget
-		case "usb":
-			boot.BootSourceOverrideTarget = rf.UsbBootSourceOverrideTarget
-		case "utilities":
-			boot.BootSourceOverrideTarget = rf.UtilitiesBootSourceOverrideTarget
-		default:
-			return false, errors.New("invalid boot device")
+		boot.BootSourceOverrideTarget, err = bootDeviceToTarget(bootDevice)
+		if err != nil {
+			return false, err
 		}
 
 		if setPersistent {
@@ -78,48 +130,15 @@ func (c *Client) SystemBootDeviceSet(_ context.Context, bootDevice string, setPe
 	return true, nil
 }
 
-// bootTargetToDevice tries to convert the redfish boot target to a bmclib supported device string.
-// if the target is unknown or unsupported, then a string of the target is returned.
-func bootTargetToDevice(target rf.BootSourceOverrideTarget) (device string) {
-	switch target {
-	case rf.BiosSetupBootSourceOverrideTarget:
-		device = "bios"
-	case rf.CdBootSourceOverrideTarget:
-		device = "cdrom"
-	case rf.DiagsBootSourceOverrideTarget:
-		device = "diag"
-	case rf.FloppyBootSourceOverrideTarget:
-		device = "floppy"
-	case rf.HddBootSourceOverrideTarget:
-		device = "disk"
-	case rf.NoneBootSourceOverrideTarget:
-		device = "none"
-	case rf.PxeBootSourceOverrideTarget:
-		device = "pxe"
-	case rf.RemoteDriveBootSourceOverrideTarget:
-		device = "remote_drive"
-	case rf.SDCardBootSourceOverrideTarget:
-		device = "sd_card"
-	case rf.UsbBootSourceOverrideTarget:
-		device = "usb"
-	case rf.UtilitiesBootSourceOverrideTarget:
-		device = "utilities"
-	default:
-		device = string(target)
-	}
-
-	return device
-}
-
 // GetBootDeviceOverride returns the current boot override settings
-func (c *Client) GetBootDeviceOverride(_ context.Context) (*bmc.BootDeviceOverride, error) {
+func (c *Client) GetBootDeviceOverride(_ context.Context) (override bmc.BootDeviceOverride, err error) {
 	if err := c.SessionActive(); err != nil {
-		return nil, errors.Wrap(bmclibErrs.ErrNotAuthenticated, err.Error())
+		return override, errors.Wrap(bmclibErrs.ErrNotAuthenticated, err.Error())
 	}
 
 	systems, err := c.client.Service.Systems()
 	if err != nil {
-		return nil, err
+		return override, err
 	}
 
 	for _, system := range systems {
@@ -128,14 +147,19 @@ func (c *Client) GetBootDeviceOverride(_ context.Context) (*bmc.BootDeviceOverri
 		}
 
 		boot := system.Boot
-		override := &bmc.BootDeviceOverride{
+		bootDevice, err := bootTargetToDevice(boot.BootSourceOverrideTarget)
+		if err != nil {
+			bootDevice = string(boot.BootSourceOverrideTarget)
+		}
+
+		override = bmc.BootDeviceOverride{
 			IsPersistent: boot.BootSourceOverrideEnabled == rf.ContinuousBootSourceOverrideEnabled,
 			IsEFIBoot:    boot.BootSourceOverrideMode == rf.UEFIBootSourceOverrideMode,
-			Device:       bootTargetToDevice(boot.BootSourceOverrideTarget),
+			Device:       bootDevice,
 		}
 
 		return override, nil
 	}
 
-	return nil, bmclibErrs.ErrNoSystemsAvailable
+	return override, bmclibErrs.ErrRedfishNoSystems
 }

providers/redfish/redfish.go

@@ -214,7 +214,8 @@ func (c *Conn) BootDeviceSet(ctx context.Context, bootDevice string, setPersiste
 	return c.redfishwrapper.SystemBootDeviceSet(ctx, bootDevice, setPersistent, efiBoot)
 }
 
-func (c *Conn) BootDeviceOverrideGet(ctx context.Context) (*bmc.BootDeviceOverride, error) {
+// BootDeviceOverrideGet gets the boot override device information
+func (c *Conn) BootDeviceOverrideGet(ctx context.Context) (bmc.BootDeviceOverride, error) {
 	return c.redfishwrapper.GetBootDeviceOverride(ctx)
 }