Internal state machine implementation for firmware update #69
Conversation
miha-plesko
force-pushed
the
firmware-update
branch
from
37d0e62
to
6891e2a
Compare
miha-plesko
force-pushed
the
firmware-update
branch
from
6891e2a
to
eaa5e49
Compare
| protocol, url = compatible_firmware_url(update_service, firmware_binary) | ||
|
|
||
| response = update_service.post( | ||
| :path => update_service.Actions['#UpdateService.SimpleUpdate'].target, |
There was a problem hiding this comment.
You can write this as response = update_service.Actions["#UpdateService.SimpleUpdate"].post(:field => "target", ...).
| } | ||
| ) | ||
|
|
||
| if response.status.to_i != 200 |
There was a problem hiding this comment.
If the Redfish service started the firmware update operation, it will return status 202, not 200.
And a nitpick: all of the RedfishClient::Responses that are returned by the Redfish client are guaranteed to return integer status, so no need to call to_i here.
| update_action = update_service.Actions['#UpdateService.SimpleUpdate'] | ||
| requested = update_action['TransferProtocol@Redfish.AllowableValues'] | ||
| requested ||= begin | ||
| info = update_action['TransferProtocol@Redfish.ActionInfo'].Parameters.find { |p| p.Name == 'TransferProtocol' } |
There was a problem hiding this comment.
update_action["@Redfish.ActionInfo"] is the action info field name. And some semi-broken implementations provide none of the inspected fields, in which case we will get some nil related exception. We should probably do something like this here:
params = update_action['@Redfish.ActionInfo']&.Parameters || []
params.find { |p| p.Name == 'TransferProtocol' }&.AllowableValues
There was a problem hiding this comment.
Oh boy, was hoping that server either supports update_action or not. This fine=no :)
| info = update_action['TransferProtocol@Redfish.ActionInfo'].Parameters.find { |p| p.Name == 'TransferProtocol' } | ||
| info&.AllowableValues | ||
| end | ||
| raise MiqException::MiqFirmwareUpdateError, 'Redfish supports zero transfer protocols' if requested.empty? |
There was a problem hiding this comment.
requested can be nil if both methods for obtaining supported protocols fail.
miha-plesko
force-pushed
the
firmware-update
branch
from
eaa5e49
to
de12b9c
Compare
There was a problem hiding this comment.
Thanks for a review @tadeboro , you comments actually make sense 👍 :)
| protocol, url = compatible_firmware_url(update_service, firmware_binary) | ||
|
|
||
| response = update_service.post( | ||
| :path => update_service.Actions['#UpdateService.SimpleUpdate'].target, |
| update_action = update_service.Actions['#UpdateService.SimpleUpdate'] | ||
| requested = update_action['TransferProtocol@Redfish.AllowableValues'] | ||
| requested ||= begin | ||
| info = update_action['TransferProtocol@Redfish.ActionInfo'].Parameters.find { |p| p.Name == 'TransferProtocol' } |
There was a problem hiding this comment.
Oh boy, was hoping that server either supports update_action or not. This fine=no :)
With this commit we implement internal state machine that carries out the firmware update process. We only request a single task no matter the number of servers to update firmware for because Redfish supports such bulk operation. Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
miha-plesko
force-pushed
the
firmware-update
branch
from
de12b9c
to
38fd368
Compare
|
Checked commit xlab-si@38fd368 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
|
Travis should turn green when @gmcculloug pushes the button on ManageIQ/manageiq#18801 |
Physical Server: Firmware Update
Below please find a complete list of the PRs to support this functionality.
a) Firmware Registry
b) Firmware Update Process
With this commit we implement internal state machine that carries out the firmware update process. We only request a single task no matter the number of servers to update firmware for because Redfish supports such bulk operation.
At this very moment we are not able to poll for update process status yet so we assume it succeeds immediately. Later, when redfish_client gem supports async operation polling, we'll be able to enhance this state machine as well.