From 2f507797a165b6d2ac3baa27d41cadec94cb49bf Mon Sep 17 00:00:00 2001 From: nofaralfasi Date: Thu, 12 Sep 2024 18:16:22 +0300 Subject: [PATCH] Add Firmware option to Libvirt VM creation --- app/models/compute_resource.rb | 37 ++++++++++++ .../foreman/model/libvirt.rb | 7 ++- .../form/libvirt/_base.html.erb | 7 +++ bundler.d/libvirt.rb | 4 +- test/models/compute_resource_test.rb | 57 +++++++++++++++++++ 5 files changed, 110 insertions(+), 2 deletions(-) diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb index a555156e9bd..c0cc01dbef6 100644 --- a/app/models/compute_resource.rb +++ b/app/models/compute_resource.rb @@ -399,6 +399,43 @@ def normalize_vm_attrs(vm_attrs) vm_attrs end + # Returns a hash of firmware type identifiers and their corresponding labels for use in the VM creation form. + # + # @return [Hash] a hash mapping firmware type identifiers to labels. + def firmware_types + { + "automatic" => N_("Automatic"), + "bios" => N_("BIOS"), + "uefi" => N_("UEFI"), + }.freeze + end + + # Converts the firmware type from a VM object to the Foreman-compatible format. + # + # @param firmware [String] The firmware type from the VM object. + # @return [String] The converted firmware type. + def firmware_type(firmware) + firmware == 'efi' ? 'uefi' : firmware + end + + # Normalizes the firmware type to 'efi' or defaults to 'bios'. + # + # @param firmware_type [String] The firmware type in Foreman format. + # @return [String] The converted firmware type. + def normalize_firmware_type(firmware_type) + firmware_type == 'uefi' ? 'efi' : 'bios' + end + + # Resolves the firmware setting when it is 'automatic' based on the provided firmware_type, or defaults to 'bios'. + # + # @param firmware [String] The current firmware setting. + # @param firmware_type [String] The type of firmware to be used if firmware is 'automatic'. + # @return [String] the resolved firmware. + def resolve_automatic_firmware(firmware, firmware_type) + return firmware unless firmware == 'automatic' + firmware_type.presence || 'bios' + end + protected def memory_gb_to_bytes(memory_size) diff --git a/app/models/compute_resources/foreman/model/libvirt.rb b/app/models/compute_resources/foreman/model/libvirt.rb index 23d367de42b..787f73480ed 100644 --- a/app/models/compute_resources/foreman/model/libvirt.rb +++ b/app/models/compute_resources/foreman/model/libvirt.rb @@ -148,6 +148,10 @@ def new_vm(attr = { }) opts[:boot_order] = %w[hd] opts[:boot_order].unshift 'network' unless attr[:image_id] + firmware_type = opts.delete(:firmware_type).to_s + firmware = resolve_automatic_firmware(opts[:firmware], firmware_type) + opts[:firmware] = normalize_firmware_type(firmware) + vm = client.servers.new opts vm.memory = opts[:memory] if opts[:memory] vm @@ -289,7 +293,8 @@ def vm_instance_defaults :display => { :type => display_type, :listen => Setting[:libvirt_default_console_address], :password => random_password(console_password_length(display_type)), - :port => '-1' } + :port => '-1' }, + :firmware => 'automatic' ) end diff --git a/app/views/compute_resources_vms/form/libvirt/_base.html.erb b/app/views/compute_resources_vms/form/libvirt/_base.html.erb index ad88274d096..9e1e8a93c09 100644 --- a/app/views/compute_resources_vms/form/libvirt/_base.html.erb +++ b/app/views/compute_resources_vms/form/libvirt/_base.html.erb @@ -8,6 +8,13 @@ <% checked = params[:host] && params[:host][:compute_attributes] && params[:host][:compute_attributes][:start] || '1' %> <%= checkbox_f f, :start, { :checked => (checked == '1'), :help_inline => _("Power ON this machine"), :label => _('Start'), :label_size => "col-md-2"} if new_vm && controller_name != "compute_attributes" %> +<% firmware_type = new_vm ? 'automatic' : compute_resource.firmware_type(f.object.firmware) %> +<%= field(f, :firmware, :disabled => !new_vm, :label => _('Firmware'), :label_help => _("Choose 'Automatic' to set the firmware based on the host's PXE Loader. If no PXE Loader is selected, it defaults to BIOS."), :label_size => "col-md-2") do + compute_resource.firmware_types.collect do |type, name| + radio_button_f f, :firmware, {:checked => (firmware_type == type), :disabled => !new_vm, :value => type, :text => _(name) } + end.join(' ').html_safe +end %> + <% arch ||= nil ; os ||= nil images = possible_images(compute_resource, arch, os) diff --git a/bundler.d/libvirt.rb b/bundler.d/libvirt.rb index 80b3f8c5a12..3404cc0313c 100644 --- a/bundler.d/libvirt.rb +++ b/bundler.d/libvirt.rb @@ -1,4 +1,6 @@ group :libvirt do - gem 'fog-libvirt', '>= 0.12.0' + # gem 'fog-libvirt', '>= 0.12.0' gem 'ruby-libvirt', '~> 0.5', :require => 'libvirt' + # TODO: Remove this line after merging the PR + gem 'fog-libvirt', github: 'nofaralfasi/fog-libvirt', branch: 'sb_libvirt' end diff --git a/test/models/compute_resource_test.rb b/test/models/compute_resource_test.rb index d106e43c955..0f2e51b6bc5 100644 --- a/test/models/compute_resource_test.rb +++ b/test/models/compute_resource_test.rb @@ -391,4 +391,61 @@ def setup assert_equal volume_attributes, volumes end end + + describe '#firmware_type' do + before do + @cr = compute_resources(:mycompute) + end + + test "returns 'uefi' when firmware is 'efi'" do + assert_equal 'uefi', @cr.firmware_type('efi') + end + + test "returns firmware unchanged when firmware is not 'efi'" do + assert_equal 'bios', @cr.firmware_type('bios') + assert_equal '', @cr.firmware_type('') + assert_equal nil, @cr.firmware_type(nil) + end + end + + describe '#normalize_firmware_type' do + before do + @cr = compute_resources(:mycompute) + end + + test "returns 'efi' when firmware is 'uefi'" do + assert_equal 'efi', @cr.normalize_firmware_type('uefi') + end + + test "returns 'bios' when firmware is not 'uefi'" do + assert_equal 'bios', @cr.normalize_firmware_type('bios') + assert_equal 'bios', @cr.normalize_firmware_type('none') + assert_equal 'bios', @cr.normalize_firmware_type('') + assert_equal 'bios', @cr.normalize_firmware_type(nil) + end + end + + describe '#resolve_automatic_firmware' do + before do + @cr = compute_resources(:mycompute) + end + + test "returns firmware_type when firmware is 'automatic' and firmware_type is present" do + assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'automatic', 'uefi') + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', 'bios') + assert_equal 'none', @cr.send(:resolve_automatic_firmware, 'automatic', 'none') + end + + test "returns firmware unchanged when not 'automatic'" do + assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'uefi', 'bios') + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', 'uefi') + assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'uefi', false) + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', '') + end + + test "returns 'bios' when firmware is 'automatic' and firmware_type is not present" do + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', '') + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', nil) + end + end end