Merge pull request #894 from Dhamo1107/add-volume-to-root-disk

Add the volume to the root disk (vda) if the selected flavor has a ro…

Author: Fryguy

app/models/manageiq/providers/openstack/cloud_manager/provision/volume_attachment.rb

@@ -1,10 +1,18 @@
 module ManageIQ::Providers::Openstack::CloudManager::Provision::VolumeAttachment
   def create_requested_volumes(requested_volumes)
-    volumes_attrs_list = [default_volume_attributes]
+    volumes_attrs_list = instance_type.root_disk_size > 0 ? [default_volume_attributes] : []
 
     connection_options = {:service => "volume", :tenant_name => cloud_tenant.try(:name)}
     source.ext_management_system.with_provider_connection(connection_options) do |service|
-      requested_volumes.each do |volume_attrs|
+      requested_volumes.each_with_index do |volume_attrs, index|
+        if instance_type.root_disk_size == 0 # Handle root disk logic only when root_disk_size is 0
+          if index == 0  # The first volume should be the root disk, typically assigned to vda
+            volume_attrs[:imageRef] = source.ems_ref # Set the image reference for booting
+            volume_attrs[:bootable] = true
+            volume_attrs[:boot_index] = 0
+          end
+        end
+
         new_volume_id = service.volumes.create(volume_attrs).id
         new_volume_attrs = volume_attrs.clone
         new_volume_attrs[:uuid]             = new_volume_id

spec/models/manageiq/providers/openstack/cloud_manager/provision/volume_attachment_spec.rb

@@ -1,82 +1,102 @@
 describe ManageIQ::Providers::Openstack::CloudManager::Provision::VolumeAttachment do
-  before do
-    @ems = FactoryBot.create(:ems_openstack_with_authentication)
-    @template = FactoryBot.create(:template_openstack, :ext_management_system => @ems)
-    @flavor = FactoryBot.create(:flavor_openstack)
-    @volume = FactoryBot.create(:cloud_volume_openstack)
+  let(:ems)          { FactoryBot.create(:ems_openstack_with_authentication) }
+  let(:flavor)       { FactoryBot.create(:flavor_openstack) }
+  let(:template)     { FactoryBot.create(:template_openstack, :ext_management_system => ems) }
+  let(:volume)       { FactoryBot.create(:cloud_volume_openstack) }
+  let(:service)      { double("volume service") }
+  let(:task_options) { {:instance_type => flavor, :src_vm_id => template.id, :volumes => task_volumes} }
+  let(:task_volumes) { [{:name => "custom_volume_1", :size => 2}] }
+  let(:task)         { FactoryBot.create(:miq_provision_openstack, :source => template, :state => 'pending', :status => 'Ok', :options => task_options) }
 
+  before do
     # We're storing objects in the instance_type, so we must permit loading this class
-    ActiveRecord.yaml_column_permitted_classes = YamlPermittedClasses.app_yaml_permitted_classes | [@flavor.class]
-
-    @task = FactoryBot.create(:miq_provision_openstack,
-                               :source  => @template,
-                               :state   => 'pending',
-                               :status  => 'Ok',
-                               :options => {
-                                 :instance_type => @flavor,
-                                 :src_vm_id     => @template.id,
-                                 :volumes       => [{:name => "custom_volume_1", :size => 2}]
-                               })
+    ActiveRecord.yaml_column_permitted_classes = YamlPermittedClasses.app_yaml_permitted_classes | [flavor.class]
   end
 
   context "#configure_volumes" do
-    it "create volumes" do
-      service = double
-      allow(service).to receive_message_chain('volumes.create').and_return @volume
-      allow(@task.source.ext_management_system).to receive(:with_provider_connection)\
+    before do
+      allow(service).to receive_message_chain(:volumes, :create).and_return(volume)
+      allow(task.source.ext_management_system).to receive(:with_provider_connection)
         .with({:service => 'volume', :tenant_name => nil}).and_yield(service)
-      allow(@task).to receive(:instance_type).and_return @flavor
+      allow(task).to receive(:instance_type).and_return(flavor)
+    end
 
+    it "create volumes" do
       default_volume = {:name => "root", :size => 1, :source_type => "image", :destination_type => "local",
                         :boot_index => 0, :delete_on_termination => true, :uuid => nil}
-      requested_volume = {:name => "custom_volume_1", :size => 2, :uuid => @volume.id, :source_type => "volume",
+      requested_volume = {:name => "custom_volume_1", :size => 2, :uuid => volume.id, :source_type => "volume",
                           :destination_type => "volume"}
 
-      expect(@task.create_requested_volumes(@task.options[:volumes])).to eq [default_volume, requested_volume]
+      expect(task.create_requested_volumes(task.options[:volumes])).to eq([default_volume, requested_volume])
+    end
+
+    context "with a flavor that has no root disk" do
+      let(:flavor) { FactoryBot.create(:flavor_openstack, :root_disk_size => 0) }
+
+      it "sets the requested volume as a boot disk" do
+        expected_volume = {:name => "custom_volume_1", :size => 2, :uuid => volume.id, :source_type => "volume",
+                           :destination_type => "volume", :boot_index => 0, :bootable => true, :imageRef => template.ems_ref}
+
+        expect(task.create_requested_volumes(task.options[:volumes])).to eq([expected_volume])
+      end
+
+      context "with multiple requested volumes" do
+        let(:task_volumes) { [{:name => "custom_volume_1", :size => 2}, {:name => "custom_volume_2", :size => 4}] }
+
+        it "only sets boot_index for first volumes" do
+          expected_volume_1 = {:name => "custom_volume_1", :size => 2, :uuid => volume.id, :source_type => "volume",
+                               :destination_type => "volume", :boot_index => 0, :bootable => true, :imageRef => template.ems_ref}
+          expected_volume_2 = {:name => "custom_volume_2", :size => 4, :uuid => volume.id, :source_type => "volume",
+                               :destination_type => "volume"}
+
+          expect(task.create_requested_volumes(task.options[:volumes])).to eq([expected_volume_1, expected_volume_2])
+        end
+      end
     end
   end
 
   context "#check_volumes" do
     it "status pending" do
       pending_volume_attrs = {:source_type => "volume"}
-      service = double
-      allow(service).to receive_message_chain('volumes.get').and_return FactoryBot.build(:cloud_volume_openstack,
-                                                                                          :status => "pending")
-      allow(@task.source.ext_management_system).to receive(:with_provider_connection)\
+
+      allow(service).to receive_message_chain('volumes.get')
+        .and_return(FactoryBot.build(:cloud_volume_openstack, :status => "pending"))
+      allow(task.source.ext_management_system).to receive(:with_provider_connection)
         .with({:service => 'volume', :tenant_name => nil}).and_yield(service)
 
-      expect(@task.do_volume_creation_check([pending_volume_attrs])).to eq [false, "pending"]
+      expect(task.do_volume_creation_check([pending_volume_attrs])).to eq([false, "pending"])
     end
 
     it "check creation status available" do
       pending_volume_attrs = {:source_type => "volume"}
-      service = double
-      allow(service).to receive_message_chain('volumes.get').and_return FactoryBot.build(:cloud_volume_openstack,
-                                                                                          :status => "available")
-      allow(@task.source.ext_management_system).to receive(:with_provider_connection)\
+
+      allow(service).to receive_message_chain('volumes.get')
+        .and_return(FactoryBot.build(:cloud_volume_openstack, :status => "available"))
+      allow(task.source.ext_management_system).to receive(:with_provider_connection)
         .with({:service => 'volume', :tenant_name => nil}).and_yield(service)
 
-      expect(@task.do_volume_creation_check([pending_volume_attrs])).to eq true
+      expect(task.do_volume_creation_check([pending_volume_attrs])).to be_truthy
     end
 
     it "check creation status - not found" do
       pending_volume_attrs = {:source_type => "volume"}
-      service = double
-      allow(service).to receive_message_chain('volumes.get').and_return nil
-      allow(@task.source.ext_management_system).to receive(:with_provider_connection)\
+
+      allow(service).to receive_message_chain('volumes.get').and_return(nil)
+      allow(task.source.ext_management_system).to receive(:with_provider_connection)
         .with({:service => 'volume', :tenant_name => nil}).and_yield(service)
 
-      expect(@task.do_volume_creation_check([pending_volume_attrs])).to eq [false, nil]
+      expect(task.do_volume_creation_check([pending_volume_attrs])).to eq([false, nil])
     end
 
     it "status error" do
       pending_volume_attrs = {:source_type => "volume"}
-      service = double
-      allow(service).to receive_message_chain('volumes.get').and_return FactoryBot.build(:cloud_volume_openstack,
-                                                                                          :status => "error")
-      allow(@task.source.ext_management_system).to receive(:with_provider_connection)\
+
+      allow(service).to receive_message_chain('volumes.get')
+        .and_return(FactoryBot.build(:cloud_volume_openstack, :status => "error"))
+      allow(task.source.ext_management_system).to receive(:with_provider_connection)
         .with({:service => 'volume', :tenant_name => nil}).and_yield(service)
-      expect { @task.do_volume_creation_check([pending_volume_attrs]) }.to raise_error(MiqException::MiqProvisionError)
+
+      expect { task.do_volume_creation_check([pending_volume_attrs]) }.to raise_error(MiqException::MiqProvisionError)
     end
   end
 end