Merge pull request #13318 from masayag/add_disks_via_restapi_provisio…

…n_vm_for_rhevm

Allow adding disks to vm provision via api and automation
(cherry picked from commit b1ded2f)

https://bugzilla.redhat.com/show_bug.cgi?id=1414893

app/models/manageiq/providers/redhat/infra_manager.rb

@@ -102,24 +102,45 @@ def vm_reconfigure(vm, options = {})
   end
 
   def add_disks(add_disks_spec, vm)
-    ems_storage_uid = add_disks_spec["ems_storage_uid"]
+    storage = add_disks_spec[:storage]
     with_disk_attachments_service(vm) do |service|
-      add_disks_spec["disks"].each { |disk_spec| service.add(prepare_disk(disk_spec, ems_storage_uid)) }
+      add_disks_spec[:disks].each { |disk_spec| service.add(prepare_disk(disk_spec, storage)) }
     end
   end
 
-  def prepare_disk(disk_spec, ems_storage_uid)
-    {
-      :bootable  => disk_spec["bootable"],
-      :interface => "VIRTIO",
-      :active    => true,
-      :disk      => {
-        :provisioned_size => disk_spec["disk_size_in_mb"].to_i * 1024 * 1024,
-        :sparse           => disk_spec["thin_provisioned"],
-        :format           => disk_spec["format"],
-        :storage_domains  => [:id => ems_storage_uid]
-      }
+  # prepare disk attachment request payload of adding disk for reconfigure vm
+  def prepare_disk(disk_spec, storage)
+    disk_spec = disk_spec.symbolize_keys
+    da_options = {
+      :size_in_mb       => disk_spec[:disk_size_in_mb],
+      :storage          => storage,
+      :name             => disk_spec[:disk_name],
+      :thin_provisioned => disk_spec[:thin_provisioned],
+      :bootable         => disk_spec[:bootable],
+    }
+
+    disk_attachment_builder = DiskAttachmentBuilder.new(da_options)
+    disk_attachment_builder.disk_attachment
+  end
+
+  # add disk to a virtual machine for a request arrived from an automation call
+  def vm_add_disk(vm, options = {})
+    storage = options[:datastore] || vm.storage
+    raise _("Data Store does not exist, unable to add disk") unless storage
+
+    da_options = {
+      :size_in_mb       => options[:diskSize],
+      :storage          => storage,
+      :name             => options[:diskName],
+      :thin_provisioned => options[:thinProvisioned],
+      :bootable         => options[:bootable],
+      :interface        => options[:interface]
     }
+
+    disk_attachment_builder = DiskAttachmentBuilder.new(da_options)
+    with_disk_attachments_service(vm) do |service|
+      service.add(disk_attachment_builder.disk_attachment)
+    end
   end
 
   def update_vm_memory(vm, virtual)

app/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder.rb

@@ -0,0 +1,50 @@
+class ManageIQ::Providers::Redhat::InfraManager::DiskAttachmentBuilder
+  def initialize(options = {})
+    @size_in_mb = options[:size_in_mb]
+    @storage = options[:storage]
+    @name = options[:name]
+    @thin_provisioned = BooleanParameter.new(options[:thin_provisioned])
+    @bootable = BooleanParameter.new(options[:bootable])
+    @active = options[:active]
+    @interface = options[:interface]
+  end
+
+  def disk_attachment
+    thin_provisioned = @thin_provisioned.true?
+    {
+      :bootable  => @bootable.true?,
+      :interface => @interface || "VIRTIO",
+      :active    => @active,
+      :disk      => {
+        :name             => @name,
+        :provisioned_size => @size_in_mb.to_i.megabytes,
+        :sparse           => thin_provisioned,
+        :format           => self.class.disk_format_for(@storage, thin_provisioned),
+        :storage_domains  => [:id => ManageIQ::Providers::Redhat::InfraManager.extract_ems_ref_id(@storage.ems_ref)]
+      }
+    }
+  end
+
+  FILE_STORAGE_TYPE = %w(NFS GLUSTERFS VMFS).to_set.freeze
+  BLOCK_STORAGE_TYPE = %w(FCP ISCSI).to_set.freeze
+
+  def self.disk_format_for(storage, thin_provisioned)
+    if FILE_STORAGE_TYPE.include?(storage.store_type)
+      "raw"
+    elsif BLOCK_STORAGE_TYPE.include?(storage.store_type)
+      thin_provisioned ? "cow" : "raw"
+    else
+      "raw"
+    end
+  end
+
+  class BooleanParameter
+    def initialize(param)
+      @value = param.to_s == "true"
+    end
+
+    def true?
+      @value
+    end
+  end
+end

app/models/manageiq/providers/redhat/infra_manager/provision.rb

@@ -3,6 +3,7 @@ class ManageIQ::Providers::Redhat::InfraManager::Provision < MiqProvision
   include_concern 'Configuration'
   include_concern 'Placement'
   include_concern 'StateMachine'
+  include_concern 'Disk'
 
   def destination_type
     "Vm"

app/models/manageiq/providers/redhat/infra_manager/provision/disk.rb

@@ -0,0 +1,59 @@
+module ManageIQ::Providers::Redhat::InfraManager::Provision::Disk
+  def configure_dialog_disks
+    added_disks = options[:disk_scsi]
+    return nil if added_disks.blank?
+
+    options[:disks_add] = prepare_disks_for_add(added_disks)
+  end
+
+  def add_disks(disks)
+    destination.ext_management_system.with_disk_attachments_service(destination) do |service|
+      disks.each { |disk| service.add(disk) }
+    end
+  end
+
+  def destination_disks_locked?
+    destination.ext_management_system.with_provider_connection(:version => 4) do |connection|
+      system_service = connection.system_service
+      disks = system_service.vms_service.vm_service(destination.uid_ems).disk_attachments_service.list
+      disks.each do |disk|
+        fetched_disk = system_service.disks_service.disk_service(disk.id).get
+        return true unless fetched_disk.try(:status) == "ok"
+      end
+    end
+
+    false
+  end
+
+  private
+
+  def prepare_disks_for_add(disks_spec)
+    disks_spec.collect do |disk_spec|
+      disk = prepare_disk_for_add(disk_spec)
+      _log.info("disk: #{disk.inspect}")
+      disk
+    end.compact
+  end
+
+  def prepare_disk_for_add(disk_spec)
+    storage_name = disk_spec[:datastore]
+    raise MiqException::MiqProvisionError, "Storage is required for disk: <#{disk_spec.inspect}>" if storage_name.blank?
+
+    storage = Storage.find_by(:name => storage_name)
+    if storage.nil?
+      raise MiqException::MiqProvisionError, "Unable to find storage: <#{storage_name}> for disk: <#{disk_spec.inspect}>"
+    end
+
+    da_options = {
+      :size_in_mb       => disk_spec[:sizeInMB],
+      :storage          => storage,
+      :name             => disk_spec[:filename],
+      :thin_provisioned => disk_spec[:backing] && disk_spec[:backing][:thinprovisioned],
+      :bootable         => disk_spec[:bootable],
+      :interface        => disk_spec[:interface]
+    }
+
+    disk_attachment_builder = ManageIQ::Providers::Redhat::InfraManager::DiskAttachmentBuilder.new(da_options)
+    disk_attachment_builder.disk_attachment
+  end
+end

app/models/manageiq/providers/redhat/infra_manager/provision/state_machine.rb

@@ -41,6 +41,27 @@ def customize_destination
       _log.info("#{message} #{for_destination}")
       update_and_notify_parent(:message => message)
       configure_container
+      signal :configure_disks
+    end
+  end
+
+  def configure_disks
+    configure_dialog_disks
+    requested_disks = options[:disks_add]
+    if requested_disks.nil?
+      _log.info "Disks settings will be inherited from the template."
+      signal :customize_guest
+    else
+      add_disks(requested_disks)
+      signal :poll_add_disks_complete
+    end
+  end
+
+  def poll_add_disks_complete
+    update_and_notify_parent(:message => "Waiting for disks creation to complete for #{dest_name}")
+    if destination_disks_locked?
+      requeue_phase
+    else
       signal :customize_guest
     end
   end

app/models/manageiq/providers/redhat/infra_manager/vm/reconfigure.rb

@@ -34,23 +34,9 @@ def build_config_spec(task_options)
   end
 
   def spec_for_added_disks(disks)
-    disks.each { |disk_spec| disk_spec["format"] = disk_format_for(disk_spec["thin_provisioned"]) }
     {
-      "disks"           => disks,
-      "ems_storage_uid" => ManageIQ::Providers::Redhat::InfraManager.extract_ems_ref_id(storage.ems_ref),
+      :disks   => disks,
+      :storage => storage
     }
   end
-
-  FILE_STORAGE_TYPE = %w(NFS GLUSTERFS VMFS).freeze
-  BLOCK_STORAGE_TYPE = %w(FCP ISCSI).freeze
-
-  def disk_format_for(thin_provisioned)
-    if FILE_STORAGE_TYPE.include?(storage.store_type)
-      "raw"
-    elsif BLOCK_STORAGE_TYPE.include?(storage.store_type)
-      thin_provisioned ? "cow" : "raw"
-    else
-      "raw"
-    end
-  end
 end

app/models/vm_or_template/operations/configuration.rb

@@ -73,8 +73,15 @@ def disconnect_floppies
 
   def raw_add_disk(disk_name, disk_size_mb, options = {})
     raise _("VM has no EMS, unable to add disk") unless ext_management_system
+    if options[:datastore]
+      datastore = Storage.find_by(:name => options[:datastore])
+      raise _("Data Store does not exist, unable to add disk") unless datastore
+    end
+
     run_command_via_parent(:vm_add_disk, :diskName => disk_name, :diskSize => disk_size_mb,
-        :thinProvisioned => options[:thin_provisioned], :dependent => options[:dependent], :persistent => options[:persistent])
+        :thinProvisioned => options[:thin_provisioned], :dependent => options[:dependent],
+        :persistent => options[:persistent], :bootable => options[:bootable], :datastore => datastore,
+        :interface => options[:interface])
   end
 
   def add_disk(disk_name, disk_size_mb, options = {})

lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-redhat-infra_manager-vm.rb

@@ -1,4 +1,7 @@
 module MiqAeMethodService
-  class MiqAeServiceManageIQ_Providers_Redhat_InfraManager_Vm < MiqAeServiceVmInfra
+  class MiqAeServiceManageIQ_Providers_Redhat_InfraManager_Vm < MiqAeServiceManageIQ_Providers_InfraManager_Vm
+    def add_disk(disk_name, disk_size_mb, options = {})
+      sync_or_async_ems_operation(options[:sync], "add_disk", [disk_name, disk_size_mb, options])
+    end
   end
 end

spec/lib/miq_automation_engine/service_methods/miq_ae_service_manageiq-providers-redhat-infra_manager-vm_spec.rb

@@ -0,0 +1,27 @@
+module MiqAeServiceManageIQ_Providers_Redhat_InfraManager_VmSpec
+  describe MiqAeMethodService::MiqAeServiceManageIQ_Providers_Redhat_InfraManager_Vm do
+    let(:vm)         { FactoryGirl.create(:vm_redhat) }
+    let(:service_vm) { MiqAeMethodService::MiqAeServiceManageIQ_Providers_Redhat_InfraManager_Vm.find(vm.id) }
+
+    before do
+      allow(MiqServer).to receive(:my_zone).and_return('default')
+      @base_queue_options = {
+        :class_name  => vm.class.name,
+        :instance_id => vm.id,
+        :zone        => 'default',
+        :role        => 'ems_operations',
+        :task_id     => nil
+      }
+    end
+
+    it "#add_disk" do
+      service_vm.add_disk('disk_1', 100, :interface => "IDE", :bootable => true)
+
+      expect(MiqQueue.first).to have_attributes(
+        @base_queue_options.merge(
+          :method_name => 'add_disk',
+          :args        => ['disk_1', 100, {:interface => "IDE", :bootable => true}])
+      )
+    end
+  end
+end

spec/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder_spec.rb

@@ -0,0 +1,95 @@
+describe ManageIQ::Providers::Redhat::InfraManager::DiskAttachmentBuilder do
+  context "#disk_format_for" do
+    context "when storage type is file system" do
+      let(:storage) { FactoryGirl.build(:storage_nfs) }
+      it "returns 'raw' format for FS storage type" do
+        expect(described_class.disk_format_for(storage, false)).to eq("raw")
+      end
+
+      it "returns 'raw' format for thin provisioned" do
+        expect(described_class.disk_format_for(storage, true)).to eq("raw")
+      end
+    end
+
+    context "when storage type is block" do
+      let(:storage) { FactoryGirl.build(:storage_block) }
+
+      it "returns 'cow' format for block storage type and thin provisioned" do
+        expect(described_class.disk_format_for(storage, true)).to eq("cow")
+      end
+
+      it "returns 'raw' format for block storage type and thick provisioned" do
+        expect(described_class.disk_format_for(storage, false)).to eq("raw")
+      end
+    end
+
+    context "when storage type is not file system and not blcok" do
+      let(:storage) { FactoryGirl.build(:storage_unknown) }
+      it "returns 'raw' format as default" do
+        expect(described_class.disk_format_for(storage, false)).to eq("raw")
+      end
+    end
+  end
+
+  context "#disk_attachment" do
+    let(:storage) { FactoryGirl.build(:storage_nfs, :ems_ref => "http://example.com/storages/XYZ") }
+
+    it "creates disk attachment" do
+      builder = described_class.new(:size_in_mb => 10, :storage => storage, :name => "disk-1",
+                                    :thin_provisioned => true, :bootable => true, :active => false, :interface => "IDE")
+      expected_disk_attachment = {
+        :bootable  => true,
+        :interface => "IDE",
+        :active    => false,
+        :disk      => {
+          :name             => "disk-1",
+          :provisioned_size => 10 * 1024 * 1024,
+          :sparse           => true,
+          :format           => "raw",
+          :storage_domains  => [:id => "XYZ"]
+        }
+      }
+
+      expect(builder.disk_attachment).to eq(expected_disk_attachment)
+    end
+  end
+
+  describe ManageIQ::Providers::Redhat::InfraManager::DiskAttachmentBuilder::BooleanParameter do
+    let(:param) { nil }
+    subject { described_class.new(param).true? }
+
+    context "param is true" do
+      let(:param) { true }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context "param is false" do
+      let(:param) { false }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context "param is 'true'" do
+      let(:param) { "true" }
+
+      it { is_expected.to be_truthy }
+    end
+
+    context "param is 'false'" do
+      let(:param) { "false" }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context "param is nil" do
+      it { is_expected.to be_falsey }
+    end
+
+    context "param is 'invalid'" do
+      let(:param) { 'invalid' }
+
+      it { is_expected.to be_falsey }
+    end
+  end
+end