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

Conversation

jacobweinstock
Copy link
Member

@jacobweinstock jacobweinstock commented May 27, 2023

What does this PR implement/change/remove?

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. This doesn't change the behavior of any methods. After each method call metadata can be interrogated for error messages of any failed providers.

The example metadata below 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
        ]
}

Checklist

  • Tests added
  • Similar commits squashed

The HW vendor this change applies to (if applicable)

The HW model number, product name this change applies to (if applicable)

The BMC firmware and/or BIOS versions that this change applies to (if applicable)

What version of tooling - vendor specific or opensource does this change depend on (if applicable)

Description for changelog/release notes

@codecov
Copy link

codecov bot commented May 27, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.18 🎉

Comparison is base (8e58c9a) 45.13% compared to head (93ad7ae) 45.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #335      +/-   ##
==========================================
+ Coverage   45.13%   45.31%   +0.18%     
==========================================
  Files          40       40              
  Lines        3224     3235      +11     
==========================================
+ Hits         1455     1466      +11     
  Misses       1605     1605              
  Partials      164      164              
Impacted Files Coverage Δ
bmc/boot_device.go 94.87% <100.00%> (+0.27%) ⬆️
bmc/connection.go 94.44% <100.00%> (+0.32%) ⬆️
bmc/power.go 94.66% <100.00%> (+0.38%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@joelrebel joelrebel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this implementation, It would be worth limiting FailedConnDetail to just carry Connection Open/Close errors.

bmc/power.go Outdated Show resolved Hide resolved
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.

The example metadata below 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>
FailedProviderDetail helps indicate that this
metadata field is not for any specific method.

Signed-off-by: Jacob Weinstock <jakobweinstock@gmail.com>
@mergify mergify bot merged commit da28e42 into bmc-toolbox:main May 30, 2023
@jacobweinstock jacobweinstock deleted the add-metadata branch May 30, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants