Allow to receive datastore by name or by id

Datastores on the same setup may have the same name. In order to add a
disk on a specific datastore, it is better to use the datastore id.

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

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

@@ -36,14 +36,7 @@ def prepare_disks_for_add(disks_spec)
   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
-
+    storage = find_storage!(disk_spec)
     da_options = {
       :size_in_mb       => disk_spec[:sizeInMB],
       :storage          => storage,
@@ -56,4 +49,27 @@ def prepare_disk_for_add(disk_spec)
     disk_attachment_builder = ManageIQ::Providers::Redhat::InfraManager::DiskAttachmentBuilder.new(da_options)
     disk_attachment_builder.disk_attachment
   end
+
+  def find_storage!(disk_spec)
+    storage_id = disk_spec[:datastore_id]
+    storage_name = disk_spec[:datastore]
+    if storage_name.blank? && storage_id.blank?
+      raise MiqException::MiqProvisionError, "Storage is required for disk: <#{disk_spec.inspect}>"
+    end
+
+    if storage_id
+      storage = Storage.find(storage_id)
+      raise_storage_not_found!(disk_spec, storage_id) if storage.nil?
+    else
+      storage = Storage.find_by(:name => storage_name)
+      raise_storage_not_found!(disk_spec, storage_name) if storage.nil?
+    end
+
+    storage
+  end
+
+  def raise_storage_not_found!(disk_spec, storage_desc)
+    error = "Unable to find storage: <#{storage_desc}> for disk: <#{disk_spec.inspect}>"
+    raise MiqException::MiqProvisionError
+  end
 end

spec/models/manageiq/providers/redhat/infra_manager/provision/disk_spec.rb

@@ -164,4 +164,63 @@
       @task.poll_add_disks_complete
     end
   end
+
+  context "#find_storage!" do
+    let(:datastore_name) { "test_storage" }
+    let(:datastore_id) { 123 }
+
+    context "when storage exists" do
+      let(:storage) { FactoryGirl.create(:storage, :name => "test_storage", :id => 123 ) }
+
+      it "searches for a storage by its ID" do
+        disk_spec = { :datastore_id => 123 }
+        allow(Storage).to receive(:find).with(datastore_id).and_return(storage)
+
+        expect(@task.send(:find_storage!, disk_spec)).to eq(storage)
+      end
+
+      it "searches for a storage by its name" do
+        disk_spec = { :datastore => datastore_name }
+        allow(Storage).to receive(:find_by).with(:name => datastore_name).and_return(storage)
+
+        expect(@task.send(:find_storage!, disk_spec)).to eq(storage)
+      end
+
+      it "searches for a storage both by its id and name" do
+        disk_spec = { :datastore_id => 123, :datastore => datastore_name }
+        allow(Storage).to receive(:find).with(datastore_id).and_return(storage)
+
+        expect(Storage).not_to receive(:find_by)
+        expect(@task.send(:find_storage!, disk_spec)).to eq(storage)
+      end
+    end
+
+    context "when storage does not exist" do
+      it "searches for a storage by its ID" do
+        disk_spec = { :datastore_id => 123 }
+        allow(Storage).to receive(:find).with(datastore_id).and_return(nil)
+
+        expect{ @task.send(:find_storage!, disk_spec) }.to raise_error(MiqException::MiqProvisionError)
+      end
+
+      it "searches for a storage by its name" do
+        disk_spec = { :datastore => datastore_name }
+        allow(Storage).to receive(:find_by).with(:name => datastore_name).and_return(nil)
+
+        expect{ @task.send(:find_storage!, disk_spec) }.to raise_error(MiqException::MiqProvisionError)
+      end
+
+      it "searches for a storage both by its id and name" do
+        disk_spec = { :datastore => datastore_name, :datastore_id => 123 }
+        allow(Storage).to receive(:find).with(datastore_id).and_return(nil)
+        expect(Storage).not_to receive(:find_by).with(:name => datastore_name)
+
+        expect{ @task.send(:find_storage!, disk_spec) }.to raise_error(MiqException::MiqProvisionError)
+      end
+
+      it "searches for a storage without specifying id or name" do
+        expect{ @task.send(:find_storage!, {}) }.to raise_error(MiqException::MiqProvisionError)
+      end
+    end
+  end
 end