-
Notifications
You must be signed in to change notification settings - Fork 36
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
Redfish override timeout for FirmwareInstall #337
Conversation
This fixes the issue where the FirmwareInstall step on BMCs with Redfish have too little time to complete because of the way the Gofish Redfish driver used in bmclib sets timeouts. related bmc-toolbox/bmclib#337
// this gives the BMC enough time to have the firmware uploaded and return a response to the client. | ||
ctxDeadline, _ := ctx.Deadline() | ||
if time.Until(ctxDeadline) < 10*time.Minute { | ||
return "", errors.Wrap(errInsufficientCtxTimeout, " "+time.Until(ctxDeadline).String()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the operational impact of this error? Is this something we expect to have retried? Would we expect a retry to work? Does an exit here leave the BMC in a sane state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in with all bmclib functions, its upto the caller to retry with a higher timeout value and/or to close the bmc connection. We provide examples for invoking these methods - https://github.com/bmc-toolbox/bmclib/blob/main/examples/install-firmware/main.go
This fixes the issue where the FirmwareInstall step on BMCs with Redfish have too little time to complete because of the way the Gofish Redfish driver used in bmclib sets timeouts. related bmc-toolbox/bmclib#337
cbbab46
to
725e651
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #337 +/- ##
==========================================
+ Coverage 42.59% 42.67% +0.07%
==========================================
Files 44 44
Lines 3679 3691 +12
==========================================
+ Hits 1567 1575 +8
- Misses 1933 1937 +4
Partials 179 179
☔ View full report in Codecov by Sentry. |
What does this PR implement/change/remove?
FirmwareInstall
method.FirmwareInstall
method.The Redfish wrapper client - Gofish has a context timeout on the HTTP client set when
Open()
is invoked, this timeout value is often insufficient for theFirmwareInstall
method to complete successfully, and so theFirmwareInstall
method overrides this timeout value and restores it when done.Checklist
Description for changelog/release notes