From 6891e2a9223f6ab0e9a5b3256a3a5918fb4020a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miha=20Ple=C5=A1ko?= Date: Thu, 30 May 2019 09:35:41 +0200 Subject: [PATCH] Internal state machine implementation for firmware update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Miha Pleško --- .../physical_infra_manager/firmware_update.rb | 7 ++ .../firmware_update/state_machine.rb | 29 +++++ .../physical_infra_manager/operations.rb | 1 + .../operations/firmware.rb | 53 +++++++++ .../firmware_update/state_machine_spec.rb | 45 ++++++++ .../operations/firmware_spec.rb | 105 ++++++++++++++++++ 6 files changed, 240 insertions(+) create mode 100644 app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update.rb create mode 100644 app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine.rb create mode 100644 app/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware.rb create mode 100644 spec/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine_spec.rb create mode 100644 spec/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware_spec.rb diff --git a/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update.rb b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update.rb new file mode 100644 index 0000000..545be92 --- /dev/null +++ b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update.rb @@ -0,0 +1,7 @@ +class ManageIQ::Providers::Redfish::PhysicalInfraManager::FirmwareUpdate < ::PhysicalServerFirmwareUpdateTask + # Firmware update is triggered in a single Redfish API call for a list of servers + # hence we don't need more than one task per firmware update request. + SINGLE_TASK = true + + include_concern 'StateMachine' +end diff --git a/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine.rb b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine.rb new file mode 100644 index 0000000..eeb15eb --- /dev/null +++ b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine.rb @@ -0,0 +1,29 @@ +module ManageIQ::Providers::Redfish::PhysicalInfraManager::FirmwareUpdate::StateMachine + def start_firmware_update + update_and_notify_parent(:message => msg('start firmware update')) + signal :trigger_firmware_update + end + + def trigger_firmware_update + update_and_notify_parent(:message => msg('trigger firmware update via Redfish')) + unless (firmware_binary = FirmwareBinary.find_by(:id => options[:firmware_binary_id])) + raise MiqException::MiqFirmwareUpdateError, "FirmwareBinary with id #{options[:firmware_binary_id]} not found" + end + unless (servers = PhysicalServer.where(:id => options[:src_ids])) && servers.size == options[:src_ids].size + raise MiqException::MiqFirmwareUpdateError, "At least one PhysicalServer of #{options[:src_ids]} not found" + end + + source.ext_management_system.update_firmware_async(firmware_binary, servers) + signal :poll_firmware_update + end + + def poll_firmware_update + update_and_notify_parent(:message => msg('poll firmware update')) + + # TODO(miha-plesko): redfish_client gem does not support monitor polling yet. + # We have to update the state machine as soon as it does, but for now we just + # assume the firmware update succeeded as long as we were able to successfully + # trigger it. + signal :done_firmware_update + end +end diff --git a/app/models/manageiq/providers/redfish/physical_infra_manager/operations.rb b/app/models/manageiq/providers/redfish/physical_infra_manager/operations.rb index 652247f..ba6255e 100644 --- a/app/models/manageiq/providers/redfish/physical_infra_manager/operations.rb +++ b/app/models/manageiq/providers/redfish/physical_infra_manager/operations.rb @@ -4,5 +4,6 @@ module PhysicalInfraManager::Operations include_concern "Power" include_concern "Led" + include_concern "Firmware" end end diff --git a/app/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware.rb b/app/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware.rb new file mode 100644 index 0000000..78b816f --- /dev/null +++ b/app/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware.rb @@ -0,0 +1,53 @@ +module ManageIQ::Providers::Redfish + module PhysicalInfraManager::Operations::Firmware + def update_firmware_async(firmware_binary, servers) + validate_update_firmware(firmware_binary, servers) + + with_provider_connection do |client| + update_service = client.UpdateService + raise MiqException::MiqFirmwareUpdateError, 'UpdateService is not enabled' unless update_service + + protocol, url = compatible_firmware_url(update_service, firmware_binary) + + response = update_service.post( + :path => update_service.Actions['#UpdateService.SimpleUpdate'].target, + :payload => { + :ImageURI => url, + :TransferProtocol => protocol, + :Targets => servers.map(&:ems_ref), + } + ) + + if response.status.to_i != 200 + raise MiqException::MiqFirmwareUpdateError, "Cannot update firmware: (#{response.status}) #{response.data[:body]}" + end + end + end + + def compatible_firmware_url(update_service, firmware_binary) + 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' } + info&.AllowableValues + end + raise MiqException::MiqFirmwareUpdateError, 'Redfish supports zero transfer protocols' if requested.empty? + + url = firmware_binary.endpoints.map(&:url).find { |u| requested.include?(u.split('://').first.upcase) } + raise MiqException::MiqFirmwareUpdateError, 'No compatible transfer protocol' unless url + + [url.split('://').first.upcase, url] + end + + private + + def validate_update_firmware(firmware_binary, servers) + raise MiqException::MiqFirmwareUpdateError, 'At least one server must be selected' if servers.empty? + + incompatible = servers.reject { |s| s.firmware_compatible?(firmware_binary) } + unless incompatible.empty? + raise MiqException::MiqFirmwareUpdateError, "Servers not compatible with firmware: #{incompatible.map(&:id)}" + end + end + end +end diff --git a/spec/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine_spec.rb b/spec/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine_spec.rb new file mode 100644 index 0000000..eb5b1a0 --- /dev/null +++ b/spec/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine_spec.rb @@ -0,0 +1,45 @@ +describe ManageIQ::Providers::Redfish::PhysicalInfraManager::FirmwareUpdate do + before { EvmSpecHelper.create_guid_miq_server_zone } + + let(:ems) { FactoryBot.create(:ems_redfish_physical_infra) } + let(:server) { FactoryBot.create(:physical_server, :ext_management_system => ems) } + let(:request) { FactoryBot.create(:physical_server_firmware_update_request, :options => options) } + let(:firmware_binary) { FactoryBot.create(:firmware_binary) } + + subject { described_class.new(:source => server, :miq_request => request) } + + describe 'run state machine' do + before { subject.update_attribute(:options, options) } + before { allow(subject).to receive(:requeue_phase) { subject.send(subject.phase) } } + before do + allow(subject).to receive(:signal) do |method| + subject.phase = method + subject.send(method) + end + end + + context 'abort when missing firmware binary' do + let(:options) { { :firmware_binary_id => 'missing' } } + it do + expect { subject.start_firmware_update }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + + context 'abort when missing one of servers' do + let(:options) { { :src_ids => [server.id, 'missing'], :firmware_binary_id => firmware_binary.id } } + it do + expect { subject.start_firmware_update }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + + context 'when all steps succeed' do + let(:options) { { :src_ids => [server.id], :firmware_binary_id => firmware_binary.id } } + it do + expect(request).to receive(:affected_ems).and_return(ems) + expect(ems).to receive(:update_firmware_async).with(firmware_binary, [server]).and_return(nil) + expect(subject).to receive(:done_firmware_update) + subject.start_firmware_update + end + end + end +end diff --git a/spec/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware_spec.rb b/spec/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware_spec.rb new file mode 100644 index 0000000..3458a02 --- /dev/null +++ b/spec/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware_spec.rb @@ -0,0 +1,105 @@ +describe ManageIQ::Providers::Redfish::PhysicalInfraManager do + before { EvmSpecHelper.create_guid_miq_server_zone } + before { allow(server1).to receive(:firmware_compatible?).with(firmware_binary).and_return(true) } + before { allow(server2).to receive(:firmware_compatible?).with(firmware_binary).and_return(true) } + before { allow(subject).to receive(:with_provider_connection).and_yield(client) } + + let(:ems) { FactoryBot.create(:ems_physical_infra) } + let(:server1) { FactoryBot.create(:physical_server, :ext_management_system => ems) } + let(:server2) { FactoryBot.create(:physical_server, :ext_management_system => ems) } + let(:servers) { [server1, server2] } + let(:request) { FactoryBot.create(:physical_server_firmware_update_request) } + let(:firmware_binary) { FactoryBot.create(:firmware_binary) } + let(:client) { double('client', :UpdateService => update_service) } + let(:response) { double('response', :status => 200) } + let(:actions) { { '#UpdateService.SimpleUpdate' => update_action } } + let(:update_service) do + double('update_service', :Actions => actions).tap do |update_service| + allow(update_service).to receive(:post).and_return(response) + end + end + let(:update_action) do + double('update action', :target => 'update-service-url').tap do |d| + allow(d).to receive(:[]).with('TransferProtocol@Redfish.AllowableValues').and_return(['HTTP']) + end + end + + describe '#update_firmware_async' do + before { allow(subject).to receive(:compatible_firmware_url).and_return(%w[HTTP http://url]) } + + context 'when firmware update succeeds' do + it 'no error is raised' do + expect { subject.update_firmware_async(firmware_binary, servers) }.not_to raise_error + end + end + + context 'when missing server' do + it 'handled error is raised' do + expect { subject.update_firmware_async(firmware_binary, []) }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + + context 'when firmware and server are incompatible' do + before { allow(server1).to receive(:firmware_compatible?).with(firmware_binary).and_return(false) } + it 'handled error is raised' do + expect { subject.update_firmware_async(firmware_binary, servers) }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + + context 'when update service not available' do + let(:update_service) { nil } + it 'handled error is raised' do + expect { subject.update_firmware_async(firmware_binary, servers) }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + + context 'when update service returns bad response' do + let(:response) { double('response', :status => 500, :data => { :body => 'MSG' }) } + it 'handled error is raised' do + expect { subject.update_firmware_async(firmware_binary, servers) }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + end + + describe '#compatible_firmware_url' do + before { firmware_binary.endpoints = [binary_http, binary_https] } + + let(:binary_http) { FactoryBot.create(:endpoint, :url => 'http://test') } + let(:binary_https) { FactoryBot.create(:endpoint, :url => 'https://test') } + + context 'when protocols are listed in @Redfish.AllowableValues' do + it 'HTTP endpoint is returned' do + expect(update_action).to receive(:[]).with('TransferProtocol@Redfish.AllowableValues').and_return(['HTTP']) + protocol, url = subject.compatible_firmware_url(update_service, firmware_binary) + expect(protocol).to eq('HTTP') + expect(url).to eq(binary_http.url) + end + end + + context 'when protocols are listed in @Redfish.ActionInfo' do + let(:params) { [double('param1', :Name => 'TransferProtocol', :AllowableValues => ['HTTP'])] } + + it 'HTTP endpoint is returned' do + expect(update_action).to receive(:[]).with('TransferProtocol@Redfish.AllowableValues').and_return(nil) + expect(update_action).to receive(:[]).with('TransferProtocol@Redfish.ActionInfo').and_return(double(:Parameters => params)) + protocol, url = subject.compatible_firmware_url(update_service, firmware_binary) + expect(protocol).to eq('HTTP') + expect(url).to eq(binary_http.url) + end + end + + context 'when no protocol is listed as supported' do + it 'managed error is raised' do + expect(update_action).to receive(:[]).with('TransferProtocol@Redfish.AllowableValues').and_return([]) + expect { subject.compatible_firmware_url(update_service, firmware_binary) }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + + context 'when no supported protocol has corresponding endpoint' do + it 'managed error is raised' do + expect(update_action).to receive(:[]).with('TransferProtocol@Redfish.AllowableValues').and_return(['MISSING']) + expect { subject.compatible_firmware_url(update_service, firmware_binary) }.to raise_error(MiqException::MiqFirmwareUpdateError) + end + end + end +end