From 38fd36840d25c3abf9f1e2321217f0de92e1fc56 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miha=20Ple=C5=A1ko?= <miha.plesko@xlab.si>
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.

Signed-off-by: Miha Pleško <miha.plesko@xlab.si>
---
 .../firmware_update_task.rb                   |   3 +
 .../firmware_update_task/state_machine.rb     |  38 +++++++
 .../physical_infra_manager/operations.rb      |   1 +
 .../operations/firmware.rb                    |  58 ++++++++++
 .../firmware_update/state_machine_spec.rb     |  61 +++++++++++
 .../operations/firmware_spec.rb               | 102 ++++++++++++++++++
 6 files changed, 263 insertions(+)
 create mode 100644 app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update_task.rb
 create mode 100644 app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update_task/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_task.rb b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update_task.rb
new file mode 100644
index 0000000..dcd6251
--- /dev/null
+++ b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update_task.rb
@@ -0,0 +1,3 @@
+class ManageIQ::Providers::Redfish::PhysicalInfraManager::FirmwareUpdateTask < ManageIQ::Providers::PhysicalInfraManager::FirmwareUpdateTask
+  include_concern 'StateMachine'
+end
diff --git a/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update_task/state_machine.rb b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update_task/state_machine.rb
new file mode 100644
index 0000000..c37096a
--- /dev/null
+++ b/app/models/manageiq/providers/redfish/physical_infra_manager/firmware_update_task/state_machine.rb
@@ -0,0 +1,38 @@
+module ManageIQ::Providers::Redfish::PhysicalInfraManager::FirmwareUpdateTask::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
+
+    response = miq_request.affected_ems.update_firmware_async(firmware_binary, servers)
+    if !response.done?
+      phase_context[:monitor] = response.to_h
+      signal :poll_firmware_update
+    else
+      signal :done_firmware_update
+    end
+  end
+
+  def poll_firmware_update
+    update_and_notify_parent(:message => msg('poll firmware update'))
+    miq_request.affected_ems.with_provider_connection do |client|
+      old_response = RedfishClient::Response.from_hash(phase_context[:monitor])
+      response = client.get(:path => old_response.monitor)
+      if response.done?
+        signal :done_firmware_update
+      else
+        phase_context[:monitor] = response.to_h
+        requeue_phase
+      end
+    end
+  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..dd26501
--- /dev/null
+++ b/app/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware.rb
@@ -0,0 +1,58 @@
+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.Actions["#UpdateService.SimpleUpdate"].post(
+          :field   => 'target',
+          :payload => {
+            :ImageURI         => url,
+            :TransferProtocol => protocol,
+            :Targets          => servers.map(&:ems_ref),
+          }
+        )
+
+        unless [200, 202].include?(response.status)
+          raise MiqException::MiqFirmwareUpdateError,
+                "Cannot update firmware: (#{response.status}) #{response.data[:body]}"
+        end
+
+        response
+      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
+        params = update_action['TransferProtocol@Redfish.ActionInfo']&.Parameters || []
+        params.find { |p| p.Name == 'TransferProtocol' }&.AllowableValues
+      end
+      if requested.blank?
+        raise MiqException::MiqFirmwareUpdateError, 'Redfish supports zero transfer protocols'
+      end
+
+      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..9b7a331
--- /dev/null
+++ b/spec/models/manageiq/providers/redfish/physical_infra_manager/firmware_update/state_machine_spec.rb
@@ -0,0 +1,61 @@
+describe ManageIQ::Providers::Redfish::PhysicalInfraManager::FirmwareUpdateTask 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(: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 } }
+      let(:response) { double('response', :done? => true) }
+      it do
+        expect(request).to receive(:affected_ems).and_return(ems)
+        expect(ems).to receive(:update_firmware_async).with(firmware_binary, [server]).and_return(response)
+        expect(subject).to receive(:done_firmware_update)
+        subject.start_firmware_update
+      end
+    end
+
+    context 'when all steps succeed after polling' do
+      before { allow(ems).to receive(:with_provider_connection).and_yield(client) }
+      before { allow(client).to receive(:get).and_return(response, response, response_ok) }
+      before { allow(request).to receive(:affected_ems).and_return(ems) }
+      let(:options)     { { :src_ids => [server.id], :firmware_binary_id => firmware_binary.id } }
+      let(:client)      { double('client') }
+      let(:response)    { double('response', :done? => false, :monitor => '/monitor', :to_h => {}) }
+      let(:response_ok) { double('response-ok', :done? => true) }
+      it do
+        expect(ems).to receive(:update_firmware_async).with(firmware_binary, [server]).and_return(response)
+        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..d5f2618
--- /dev/null
+++ b/spec/models/manageiq/providers/redfish/physical_infra_manager/operations/firmware_spec.rb
@@ -0,0 +1,102 @@
+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)  { double('update_service', :Actions => actions) }
+  let(:update_action) do
+    double('update action', :target => 'update-service-url').tap do |d|
+      allow(d).to receive(:post).and_return(response)
+      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