Skip to content

Commit

Permalink
Add metadata field for error messages:
Browse files Browse the repository at this point in the history
This adds the error message for failed method
calls per providers to metadata. These erorrs
were previously not accessible. These errors
are captured in metadata regardless of whether
the method returns successful or not. This is
helpful in debugging providers that error but
the method call succeeds.

Example metadata shows that while the open method
call succeeded for ipmitool, dell, and gofish it did
not succeed for IntelAMT and asrockrack. We now have
access to the error messages for those failed one.

```
{
    SuccessfulProvider:""
    ProvidersAttempted:[ipmitool asrockrack gofish IntelAMT dell]
    SuccessfulOpenConns:[ipmitool dell gofish]
    SuccessfulCloseConns:[]
    FailedConnDetail:
        map[IntelAMT:provider: IntelAMT: unable to perform digest auth with http://192.168.2.205:16992/wsman: Post "http://192.168.2.205:16992/wsman": dial tcp 192.168.2.205:16992: connect: connection refused
            asrockrack:provider: asrockrack: error unmarshalling response payload: invalid character '<' looking for beginning of value
        ]
}
```

Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
  • Loading branch information
jacobweinstock committed May 27, 2023
1 parent 2714c74 commit 12fa10c
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
// FailedConnDetail holds the failed providers error messages for Open and Close methods
FailedConnDetail 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{
FailedConnDetail: 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.FailedConnDetail[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{
FailedConnDetail: 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.FailedConnDetail[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{
FailedConnDetail: 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.FailedConnDetail[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{
FailedConnDetail: 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.FailedConnDetail[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{
FailedConnDetail: 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.FailedConnDetail[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 12fa10c

Please sign in to comment.