Skip to content

Commit

Permalink
Add metadata field for error messages: (#335)
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored May 30, 2023
2 parents 8e58c9a + 93ad7ae commit da28e42
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 20 deletions.
2 changes: 2 additions & 0 deletions bmc/bmc.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ type Metadata struct {
SuccessfulOpenConns []string
// SuccessfulCloseConns is a slice of provider names that were closed successfully
SuccessfulCloseConns []string
// FailedProviderDetail holds the failed providers error messages for called methods
FailedProviderDetail map[string]string
}
5 changes: 4 additions & 1 deletion bmc/boot_device.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ type bootDeviceProviders 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) {
var metadataLocal Metadata
metadataLocal := Metadata{
FailedProviderDetail: make(map[string]string),
}

for _, elem := range b {
if elem.bootDeviceSetter == nil {
Expand All @@ -43,6 +45,7 @@ func setBootDevice(ctx context.Context, timeout time.Duration, bootDevice string
ok, setErr := elem.bootDeviceSetter.BootDeviceSet(ctx, bootDevice, setPersistent, efiBoot)
if setErr != nil {
err = multierror.Append(err, errors.WithMessagef(setErr, "provider: %v", elem.name))
metadataLocal.FailedProviderDetail[elem.name] = setErr.Error()
continue
}
if !ok {
Expand Down
12 changes: 9 additions & 3 deletions bmc/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ type connectionProviders struct {
// OpenConnectionFromInterfaces will try all opener interfaces and remove failed ones.
// The reason failed ones need to be removed is so that when other methods are called (like powerstate)
// implementations that have connections wont nil pointer error when their connection fails.
func OpenConnectionFromInterfaces(ctx context.Context, timeout time.Duration, providers []interface{}) (opened []interface{}, _ Metadata, err error) {
var metadata Metadata
func OpenConnectionFromInterfaces(ctx context.Context, timeout time.Duration, providers []interface{}) (opened []interface{}, metadata Metadata, err error) {
metadata = Metadata{
FailedProviderDetail: make(map[string]string),
}

// Return immediately if the context is done.
select {
Expand Down Expand Up @@ -92,6 +94,7 @@ func OpenConnectionFromInterfaces(ctx context.Context, timeout time.Duration, pr
for res := range results {
if res.Err != nil {
err = multierror.Append(err, res.Err)
metadata.FailedProviderDetail[res.ProviderName] = res.Err.Error()
continue
}

Expand All @@ -108,7 +111,9 @@ func OpenConnectionFromInterfaces(ctx context.Context, timeout time.Duration, pr

// closeConnection closes a connection to a BMC, trying all interface implementations passed in
func closeConnection(ctx context.Context, c []connectionProviders) (_ Metadata, err error) {
var metadata Metadata
var metadata = Metadata{
FailedProviderDetail: make(map[string]string),
}
var connClosed bool

for _, elem := range c {
Expand All @@ -119,6 +124,7 @@ func closeConnection(ctx context.Context, c []connectionProviders) (_ Metadata,
closeErr := elem.closer.Close(ctx)
if closeErr != nil {
err = multierror.Append(err, errors.WithMessagef(closeErr, "provider: %v", elem.name))
metadata.FailedProviderDetail[elem.name] = closeErr.Error()
continue
}
connClosed = true
Expand Down
25 changes: 16 additions & 9 deletions bmc/power.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ type powerProviders struct {
}

// setPowerState sets the power state for a BMC, trying all interface implementations passed in
func setPowerState(ctx context.Context, timeout time.Duration, state string, p []powerProviders) (ok bool, metadata Metadata, err error) {
var metadataLocal Metadata
func setPowerState(ctx context.Context, timeout time.Duration, state string, p []powerProviders) (ok bool, m Metadata, err error) {
metadataLocal := Metadata{
FailedProviderDetail: make(map[string]string),
}

for _, elem := range p {
if elem.powerSetter == nil {
Expand All @@ -49,14 +51,15 @@ func setPowerState(ctx context.Context, timeout time.Duration, state string, p [
case <-ctx.Done():
err = multierror.Append(err, ctx.Err())

return false, metadata, err
return false, metadataLocal, err
default:
metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name)
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
ok, setErr := elem.powerSetter.PowerSet(ctx, state)
if setErr != nil {
err = multierror.Append(err, errors.WithMessagef(setErr, "provider: %v", elem.name))
metadataLocal.FailedProviderDetail[elem.name] = setErr.Error()
continue
}
if !ok {
Expand Down Expand Up @@ -91,7 +94,10 @@ func SetPowerStateFromInterfaces(ctx context.Context, timeout time.Duration, sta
}

// getPowerState gets the power state for a BMC, trying all interface implementations passed in
func getPowerState(ctx context.Context, timeout time.Duration, p []powerProviders) (state string, metadata Metadata, err error) {
func getPowerState(ctx context.Context, timeout time.Duration, p []powerProviders) (state string, m Metadata, err error) {
metadataLocal := Metadata{
FailedProviderDetail: make(map[string]string),
}
for _, elem := range p {
if elem.powerStateGetter == nil {
continue
Expand All @@ -100,21 +106,22 @@ func getPowerState(ctx context.Context, timeout time.Duration, p []powerProvider
case <-ctx.Done():
err = multierror.Append(err, ctx.Err())

return state, metadata, err
return state, metadataLocal, err
default:
metadata.ProvidersAttempted = append(metadata.ProvidersAttempted, elem.name)
metadataLocal.ProvidersAttempted = append(metadataLocal.ProvidersAttempted, elem.name)
ctx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
state, stateErr := elem.powerStateGetter.PowerStateGet(ctx)
if stateErr != nil {
err = multierror.Append(err, errors.WithMessagef(stateErr, "provider: %v", elem.name))
metadataLocal.FailedProviderDetail[elem.name] = stateErr.Error()
continue
}
metadata.SuccessfulProvider = elem.name
return state, metadata, nil
metadataLocal.SuccessfulProvider = elem.name
return state, metadataLocal, nil
}
}
return state, metadata, multierror.Append(err, errors.New("failed to get power state"))
return state, metadataLocal, multierror.Append(err, errors.New("failed to get power state"))
}

// GetPowerStateFromInterfaces identifies implementations of the PostStateGetter interface and passes the found implementations to the getPowerState() wrapper.
Expand Down
12 changes: 5 additions & 7 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,38 +21,36 @@ func TestBMC(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
defer cancel()
cl := NewClient(host, user, pass, WithLogger(log), WithPerProviderTimeout(5*time.Second))
cl.FilterForCompatible(ctx)
var err error
err = cl.Open(ctx)
err := cl.Open(ctx)
if err != nil {
t.Logf("%+v", cl.GetMetadata())
t.Fatal(err)
}
defer cl.Close(ctx)
t.Logf("metadata: %+v", cl.GetMetadata())
t.Logf("metadata for Open: %+v", cl.GetMetadata())

cl.Registry.Drivers = cl.Registry.PreferDriver("non-existent")
state, err := cl.GetPowerState(ctx)
if err != nil {
t.Fatal(err)
}
t.Log(state)
t.Logf("metadata %+v", cl.GetMetadata())
t.Logf("metadata for GetPowerState: %+v", cl.GetMetadata())

cl.Registry.Drivers = cl.Registry.PreferDriver("ipmitool")
state, err = cl.PreferProvider("gofish").GetPowerState(ctx)
if err != nil {
t.Fatal(err)
}
t.Log(state)
t.Logf("metadata: %+v", cl.GetMetadata())
t.Logf("metadata for GetPowerState: %+v", cl.GetMetadata())

users, err := cl.ReadUsers(ctx)
if err != nil {
t.Fatal(err)
}
t.Log(users)
t.Logf("metadata: %+v", cl.GetMetadata())
t.Logf("metadata for ReadUsers: %+v", cl.GetMetadata())

t.Fatal()
}
Expand Down

0 comments on commit da28e42

Please sign in to comment.