Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metadata field for error messages: #335

Merged
merged 2 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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