From c07d1d3f54f8ba4516cd50caadaf272f1f1b78eb Mon Sep 17 00:00:00 2001 From: Moti Asayag Date: Wed, 2 Nov 2016 23:41:41 +0200 Subject: [PATCH] Allow adding disks to vm provision via api and automation Restful AP: ---------- Provisioning of RHV vm via provision restful request is extended to support disks creation. The request format is detailed below: diskscsi##=:: - The controller number and position are ignored for RHV. diskscsi##.datastore= Optional: diskscsi##.filename = diskscsi##.bootable = Default: false diskscsi##.backing.thinProvisioned = Default: false diskscsi##.interface = Default: VIRTIO An example of the disks section as part of the provision request: "vm_fields" : { ... "diskscsi1" : "0:0:200", "diskscsi1.datastore" : "data-40", "diskscsi1.backing.thinProvisioned" : "true", "diskscsi1.bootable" : "true", "diskscsi1.filename" : "prov_disk_1", "diskscsi1.interface": "IDE", } The result is creating a bootable disk named 'prov_disk_1' on datastore 'data-40' with size of 200MB, thinly provisioned and controlled via 'IDE' driver. Automation: ---------- Same support is added for the automation as well to allow adding disks to vm. The signature of adding disk to a vm is exposed via MiqAeServiceManageIQ_Providers_Redhat_InfraManager_Vm#add_disk(disk_name, disk_size_mb, options = {}) Where the 'options' expects: thin_provisioned - Default: false bootable - Default: false datastore - the storage name to create the disk on. If not specified, the VM's storage is used. interface - Default: VIRTIO --- .../providers/redhat/infra_manager.rb | 47 +++-- .../infra_manager/disk_attachment_builder.rb | 50 ++++++ .../infra_manager/event_catcher/runner.rb | 2 +- .../redhat/infra_manager/provision.rb | 1 + .../redhat/infra_manager/provision/disk.rb | 59 +++++++ .../infra_manager/provision/state_machine.rb | 21 +++ .../redhat/infra_manager/vm/reconfigure.rb | 18 +- .../operations/configuration.rb | 9 +- ...ageiq-providers-redhat-infra_manager-vm.rb | 5 +- ...-providers-redhat-infra_manager-vm_spec.rb | 27 +++ .../disk_attachment_builder_spec.rb | 95 ++++++++++ .../infra_manager/provision/disk_spec.rb | 167 ++++++++++++++++++ .../provision/state_machine_spec.rb | 31 +++- .../redhat/infra_manager/provision_spec.rb | 25 ++- .../infra_manager/vm/reconfigure_spec.rb | 35 +--- 15 files changed, 525 insertions(+), 67 deletions(-) create mode 100644 app/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder.rb create mode 100644 app/models/manageiq/providers/redhat/infra_manager/provision/disk.rb create mode 100644 spec/lib/miq_automation_engine/service_methods/miq_ae_service_manageiq-providers-redhat-infra_manager-vm_spec.rb create mode 100644 spec/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder_spec.rb create mode 100644 spec/models/manageiq/providers/redhat/infra_manager/provision/disk_spec.rb diff --git a/app/models/manageiq/providers/redhat/infra_manager.rb b/app/models/manageiq/providers/redhat/infra_manager.rb index a85097b3e43..6748ed47986 100644 --- a/app/models/manageiq/providers/redhat/infra_manager.rb +++ b/app/models/manageiq/providers/redhat/infra_manager.rb @@ -106,24 +106,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) diff --git a/app/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder.rb b/app/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder.rb new file mode 100644 index 00000000000..728963ca272 --- /dev/null +++ b/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 diff --git a/app/models/manageiq/providers/redhat/infra_manager/event_catcher/runner.rb b/app/models/manageiq/providers/redhat/infra_manager/event_catcher/runner.rb index d1faac213bc..8cf9e082f61 100644 --- a/app/models/manageiq/providers/redhat/infra_manager/event_catcher/runner.rb +++ b/app/models/manageiq/providers/redhat/infra_manager/event_catcher/runner.rb @@ -40,7 +40,7 @@ def monitor_events end def queue_event(event) - log.info "#{log_prefix} Caught event [#{event[:name]}]" + _log.info "#{log_prefix} Caught event [#{event[:name]}]" EmsEvent.add_queue('add_rhevm', @cfg[:ems_id], event.to_hash) end diff --git a/app/models/manageiq/providers/redhat/infra_manager/provision.rb b/app/models/manageiq/providers/redhat/infra_manager/provision.rb index 320f9636c21..66e0b92df9f 100644 --- a/app/models/manageiq/providers/redhat/infra_manager/provision.rb +++ b/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" diff --git a/app/models/manageiq/providers/redhat/infra_manager/provision/disk.rb b/app/models/manageiq/providers/redhat/infra_manager/provision/disk.rb new file mode 100644 index 00000000000..b82efb590d4 --- /dev/null +++ b/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 diff --git a/app/models/manageiq/providers/redhat/infra_manager/provision/state_machine.rb b/app/models/manageiq/providers/redhat/infra_manager/provision/state_machine.rb index 248feafae00..211c641609e 100644 --- a/app/models/manageiq/providers/redhat/infra_manager/provision/state_machine.rb +++ b/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 diff --git a/app/models/manageiq/providers/redhat/infra_manager/vm/reconfigure.rb b/app/models/manageiq/providers/redhat/infra_manager/vm/reconfigure.rb index 80ebafe88ae..bfa813e81fa 100644 --- a/app/models/manageiq/providers/redhat/infra_manager/vm/reconfigure.rb +++ b/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 diff --git a/app/models/vm_or_template/operations/configuration.rb b/app/models/vm_or_template/operations/configuration.rb index 3bb49fd67ac..66b2beeface 100644 --- a/app/models/vm_or_template/operations/configuration.rb +++ b/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 = {}) diff --git a/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-redhat-infra_manager-vm.rb b/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-redhat-infra_manager-vm.rb index 2637161c4d6..7c26941d9a2 100644 --- a/lib/miq_automation_engine/service_models/miq_ae_service_manageiq-providers-redhat-infra_manager-vm.rb +++ b/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 diff --git a/spec/lib/miq_automation_engine/service_methods/miq_ae_service_manageiq-providers-redhat-infra_manager-vm_spec.rb b/spec/lib/miq_automation_engine/service_methods/miq_ae_service_manageiq-providers-redhat-infra_manager-vm_spec.rb new file mode 100644 index 00000000000..26c294b1c14 --- /dev/null +++ b/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 diff --git a/spec/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder_spec.rb b/spec/models/manageiq/providers/redhat/infra_manager/disk_attachment_builder_spec.rb new file mode 100644 index 00000000000..a9478f80095 --- /dev/null +++ b/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 diff --git a/spec/models/manageiq/providers/redhat/infra_manager/provision/disk_spec.rb b/spec/models/manageiq/providers/redhat/infra_manager/provision/disk_spec.rb new file mode 100644 index 00000000000..4c907407101 --- /dev/null +++ b/spec/models/manageiq/providers/redhat/infra_manager/provision/disk_spec.rb @@ -0,0 +1,167 @@ +describe ManageIQ::Providers::Redhat::InfraManager::Provision::Disk do + let(:ems) { FactoryGirl.create(:ems_redhat_with_authentication) } + let(:template) { FactoryGirl.create(:template_redhat, :ext_management_system => ems) } + let(:rhevm_vm) { FactoryGirl.build(:vm_redhat) } + let(:vm) { FactoryGirl.build(:vm_redhat, :ext_management_system => ems) } + let(:storage) { FactoryGirl.create(:storage_nfs, :ems_ref => "http://example.com/storages/XYZ") } + + let(:disks_spec) do + [ + { + :disk_size_in_mb => "33", + :persistent => true, + :thin_provisioned => true, + :dependent => true, + :bootable => false, + :datastore => storage.name + } + ] + end + + let(:options) do + { + :src_vm_id => template.id, + :vm_auto_start => true, + :vm_description => "some description", + :vm_target_name => "test_vm_1", + :disk_scsi => disks_spec + } + end + + let(:expected_requested_disks) do + [ + { + :bootable => false, + :interface => "VIRTIO", + :active => nil, + :disk => { + :name => nil, + :provisioned_size => 0, + :sparse => false, + :format => "raw", + :storage_domains => [{ :id=>"XYZ" }] + } + } + ] + end + + before do + @task = FactoryGirl.build(:miq_provision_redhat, + :source => template, + :destination => vm, + :state => 'pending', + :status => 'Ok', + :options => options + ) + end + + context "#configure_dialog_disks" do + it "adds disks spec as specified in the request" do + @task.configure_dialog_disks + + expect(@task.options[:disks_add]).to eq(expected_requested_disks) + end + + context "no disks were specified in the request" do + let(:options) do + { + :src_vm_id => template.id + } + end + + it "inherits the disks from the template" do + expect(@task).not_to receive(:prepare_disks_for_add) + @task.configure_dialog_disks + + expect(@task.options[:disks_add]).to eq(nil) + end + end + + context "storage is missing" do + let(:storage) { double("storage", :name => nil) } + + it "should fail due to a missing datastore" do + expect { @task.configure_dialog_disks }.to raise_error(MiqException::MiqProvisionError) + end + end + + context "storage is unknown" do + let(:storage) { double("storage", :name => "no such datastore") } + + it "should fail due to a non-existing datastore" do + expect { @task.configure_dialog_disks }.to raise_error(MiqException::MiqProvisionError) + end + end + end + + context "#configure_disks" do + it "adds disks as specified in the request" do + expect(@task).to receive(:add_disks).with(expected_requested_disks).once + expect(@task).to receive(:poll_add_disks_complete).once + + @task.configure_disks + end + + context "no disks were specified in the request" do + let(:options) do + { :src_vm_id => template.id } + end + + it "inherits the disks from the template" do + expect(@task).not_to receive(:add_disks) + expect(@task).to receive(:customize_guest).once + + @task.configure_disks + end + end + end + + context "#destination_disks_locked?" do + let(:disk_status) { "locked" } + let(:disk) { double("disk", :status => disk_status, :id => 1) } + let(:disk_attachments_service) { double("disk_attachments_service", :add => nil, :list => [disk]) } + let(:vm_service) { double("vm_service", :disk_attachments_service => disk_attachments_service) } + let(:vms_service) { double("vms_service", :vm_service => vm_service) } + let(:disk_service) { double("disk_service", :get => disk) } + let(:disks_service) { double("disks_service", :disk_service => disk_service) } + let(:system_service) { double("system_service", :vms_service => vms_service, :disks_service => disks_service) } + let(:connection) { double("connection", :system_service => system_service) } + + before do + allow(vm).to receive(:with_provider_object).and_yield(rhevm_vm) + allow(ems).to receive(:with_provider_connection).with(:version => 4).and_yield(connection) + end + + it "returns true if there are locked disks" do + expect(@task.destination_disks_locked?).to eq(true) + end + + context "with no locked disks" do + let(:disk_status) { "ok" } + + it "returns false if there aren't any locked disks" do + expect(@task.destination_disks_locked?).to eq(false) + end + end + end + + context "#poll_add_disks_complete" do + before do + allow(@task).to receive(:update_and_notify_parent) + end + + it "calls customize guest when disks are not locked" do + allow(@task).to receive(:destination_disks_locked?).and_return(false) + expect(@task).to receive(:customize_guest) + + @task.poll_add_disks_complete + end + + it "keeps checking disks status while they are locked" do + allow(@task).to receive(:destination_disks_locked?).and_return(true) + expect(@task).to receive(:requeue_phase) + + @task.poll_add_disks_complete + end + end +end diff --git a/spec/models/manageiq/providers/redhat/infra_manager/provision/state_machine_spec.rb b/spec/models/manageiq/providers/redhat/infra_manager/provision/state_machine_spec.rb index d09391b5ba8..b0338131c75 100644 --- a/spec/models/manageiq/providers/redhat/infra_manager/provision/state_machine_spec.rb +++ b/spec/models/manageiq/providers/redhat/infra_manager/provision/state_machine_spec.rb @@ -3,10 +3,19 @@ let(:cluster) { FactoryGirl.create(:ems_cluster, :ext_management_system => ems) } let(:ems) { FactoryGirl.create(:ems_redhat_with_authentication) } + let(:disk_attachments_service) { double("disk_attachments_service", :add => nil) } let(:rhevm_vm) { double("RHEVM VM") } let(:task) { request.tap(&:create_request_tasks).miq_request_tasks.first } let(:template) { FactoryGirl.create(:template_redhat, :ext_management_system => ems) } - let(:vm) { FactoryGirl.create(:vm_redhat, :ext_management_system => ems, :raw_power_state => "on").tap { |v| allow(v).to receive(:with_provider_object).and_yield(rhevm_vm) } } + let(:vm) do + FactoryGirl.create(:vm_redhat, :ext_management_system => ems, :raw_power_state => "on").tap do |v| + allow(v).to receive(:with_provider_object).and_yield(rhevm_vm) + allow(ems).to receive(:with_disk_attachments_service).with(v).and_return(disk_attachments_service) + allow(ems).to receive(:with_provider_connection).and_return(false) + end + end + + let(:storage) { FactoryGirl.create(:storage_nfs, :ems_ref => "http://example.com/storages/XYZ") } let(:options) do { @@ -17,6 +26,16 @@ :vm_auto_start => true, :vm_description => "some description", :vm_target_name => "test_vm_1", + :disk_scsi => [ + { + :disk_size_in_mb => "33", + :persistent => true, + :thin_provisioned => true, + :dependent => true, + :bootable => false, + :datastore => storage.name + } + ] } end @@ -35,6 +54,8 @@ :poll_clone_complete => {:signals => 1, :calls => 3}, :poll_destination_in_vmdb => {:signals => 1, :calls => 3}, :customize_destination => {:signals => 1, :calls => 3}, + :configure_disks => {:signals => 1, :calls => 1}, + :poll_add_disks_complete => {:signals => 1, :calls => 1}, :customize_guest => {:signals => 1, :calls => 1}, :poll_destination_powered_off_in_provider => {:signals => 1, :calls => 4}, :poll_destination_powered_off_in_vmdb => {:signals => 2, :calls => 2}, @@ -115,4 +136,12 @@ def test_autostart_destination_without_use_cloud_init call_method end + + def test_configure_disks + call_method + end + + def test_poll_add_disks_complete + call_method + end end diff --git a/spec/models/manageiq/providers/redhat/infra_manager/provision_spec.rb b/spec/models/manageiq/providers/redhat/infra_manager/provision_spec.rb index 2c0862fd368..4bb7144206a 100644 --- a/spec/models/manageiq/providers/redhat/infra_manager/provision_spec.rb +++ b/spec/models/manageiq/providers/redhat/infra_manager/provision_spec.rb @@ -11,7 +11,8 @@ :number_of_vms => 1, :cpu_limit => -1, :cpu_reserve => 0, - :provision_type => "rhevm" + :provision_type => "rhevm", + :disks_add => {} } end @@ -144,6 +145,28 @@ @vm_prov.customize_destination end end + + context "configure_disks" do + before do + allow(@vm_prov).to receive(:for_destination).and_return("display_string") + allow(@vm_prov).to receive(:configure_container) + end + + it "when adding disks is required" do + allow(@vm_prov).to receive(:configure_dialog_disks) + allow(@vm_prov).to receive(:add_disks) + expect(@vm_prov).to receive(:poll_add_disks_complete) + + @vm_prov.customize_destination + end + + it "when adding disks is not required" do + expect(@vm_prov).to receive(:configure_disks) + expect(@vm_prov).not_to receive(:poll_add_disks_complete) + + @vm_prov.customize_destination + end + end end end end diff --git a/spec/models/manageiq/providers/redhat/infra_manager/vm/reconfigure_spec.rb b/spec/models/manageiq/providers/redhat/infra_manager/vm/reconfigure_spec.rb index 29c9c44006b..d86405ce489 100644 --- a/spec/models/manageiq/providers/redhat/infra_manager/vm/reconfigure_spec.rb +++ b/spec/models/manageiq/providers/redhat/infra_manager/vm/reconfigure_spec.rb @@ -54,13 +54,13 @@ end it "disksAdd" do - disks = subject["disksAdd"]["disks"] + disks = subject["disksAdd"][:disks] expect(disks.size).to eq(1) disk_to_add = disks[0] expect(disk_to_add["disk_size_in_mb"]).to eq("33") expect(disk_to_add["thin_provisioned"]).to eq(true) expect(disk_to_add["bootable"]).to eq(false) - expect(subject["disksAdd"]["ems_storage_uid"]).to eq("XYZ") + expect(subject["disksAdd"][:storage]).to eq(storage) end it "disksRemove" do @@ -69,35 +69,4 @@ expect(subject["disksRemove"][0]["delete_backing"]).to be_falsey end end - - context "#disk_format_for" do - context "when storage type is file system" do - it "returns 'raw' format for FS storage type" do - expect(vm.disk_format_for(false)).to eq("raw") - end - - it "returns 'raw' format for thin provisioned" do - expect(vm.disk_format_for(true)).to eq("raw") - end - end - - context "when storage type is block" do - let(:storage) { FactoryGirl.create(:storage_block) } - - it "returns 'cow' format for block storage type and thin provisioned" do - expect(vm.disk_format_for(true)).to eq("cow") - end - - it "returns 'raw' format for block storage type and thick provisioned" do - expect(vm.disk_format_for(false)).to eq("raw") - end - end - - context "when storage type is not file system and not blcok" do - let(:storage) { FactoryGirl.create(:storage_unknown) } - it "returns 'raw' format as default" do - expect(vm.disk_format_for(false)).to eq("raw") - end - end - end end