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

[1/3] Redfishwrapper changes #359

Merged
merged 21 commits into from
Nov 13, 2023
Merged

[1/3] Redfishwrapper changes #359

merged 21 commits into from
Nov 13, 2023

Conversation

joelrebel
Copy link
Member

@joelrebel joelrebel commented Nov 7, 2023

What does this PR implement/change/remove?

Note: to be merged after #362

  • Adds various helper methods in the redfishwrapper for querying Redfish Task, Odata IDs
  • Adds generic Redfish methods to Upload firmware and query its status
  • Better test coverage

The firmware methods from the redfish provider will be moved into vendor specific providers,
which will call into the redfishwrapper where required.

Since each vendor requires its own set of OEM parameters and theres differences in the way the Tasks are handled, the current redfish firmware install methods are getting cumbersome to maintain and extend.

Checklist

  • Tests added
  • Similar commits squashed

@joelrebel joelrebel changed the title Redfishwrapper changes [1/3] Redfishwrapper changes Nov 7, 2023
internal/redfishwrapper/client.go Outdated Show resolved Hide resolved
internal/redfishwrapper/firmware.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 10, 2023

Codecov Report

Attention: 489 lines in your changes are missing coverage. Please review.

Comparison is base (b6c7d29) 44.46% compared to head (e6f756a) 44.99%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #359      +/-   ##
==========================================
+ Coverage   44.46%   44.99%   +0.52%     
==========================================
  Files          56       60       +4     
  Lines        4421     5025     +604     
==========================================
+ Hits         1966     2261     +295     
- Misses       2238     2508     +270     
- Partials      217      256      +39     
Files Coverage Δ
providers/asrockrack/inventory.go 75.93% <100.00%> (ø)
providers/redfish/redfish.go 23.52% <ø> (-2.01%) ⬇️
providers/redfish/tasks.go 38.82% <100.00%> (ø)
providers/supermicro/errors.go 0.00% <ø> (ø)
internal/redfishwrapper/task.go 93.33% <93.33%> (ø)
providers/redfish/firmware.go 52.10% <62.50%> (-1.47%) ⬇️
providers/supermicro/x11_firmware_bmc.go 68.54% <78.57%> (ø)
providers/supermicro/floppy.go 0.00% <0.00%> (ø)
client.go 53.07% <0.00%> (-3.49%) ⬇️
internal/redfishwrapper/client.go 52.38% <43.75%> (+30.04%) ⬆️
... and 7 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Since we need to accept more than just those two OperationApplyTime parameters
…ed, task status

The FirmwareTaskStatus method is to replace FirmwareInstallStatus
There has been no real use for having an io.Reader passed in
and this interface is to expect a file instead.
… x11, x12

based on feedback recieved this approach made more sense
Older X11 BMC firmwares don't include CSRF tokens in requests to the BMC API.
@joelrebel joelrebel force-pushed the redfishwrapper-changes branch from e37fd77 to e6f756a Compare November 13, 2023 15:12
@joelrebel
Copy link
Member Author

rebase on main

@joelrebel joelrebel merged commit 4666b85 into main Nov 13, 2023
6 of 7 checks passed
@joelrebel joelrebel deleted the redfishwrapper-changes branch November 13, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants