Skip to content

Commit

Permalink
Fixes #34839 - Add support for VMware NVME Controllers
Browse files Browse the repository at this point in the history
  • Loading branch information
girijaasoni committed Jul 22, 2024
1 parent b5638c7 commit e72fcb8
Show file tree
Hide file tree
Showing 21 changed files with 130 additions and 106 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module Foreman::Controller::NormalizeVmwareStorageControllerAttributes
extend ActiveSupport::Concern

private

def normalize_vmware_storage_controller_attributes(attrs)
ctrls_and_vol = JSON.parse(attrs["controllers"]).
deep_transform_keys { |key| key.to_s.underscore }.
deep_symbolize_keys
attrs["volumes_attributes"] = ctrls_and_vol[:volumes].each_with_index.to_h { |vol, index| [index.to_s, vol] }
attrs["nvme_controllers"], attrs["scsi_controllers"] = ctrls_and_vol[:controllers]&.partition { |controller| controller[:type].include?("VirtualNVMEController") }
attrs.delete("controllers")
attrs
end
end
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module Foreman::Controller::Parameters::ComputeAttribute
extend ActiveSupport::Concern
include Foreman::Controller::NormalizeScsiAttributes
include Foreman::Controller::NormalizeVmwareStorageControllerAttributes

class_methods do
def compute_attribute_params_filter
Expand All @@ -19,8 +19,8 @@ def compute_attribute_params
def normalized_compute_attribute_params
normalized = compute_attribute_params

if normalized["vm_attrs"] && normalized["vm_attrs"]["scsi_controllers"]
normalize_scsi_attributes(normalized["vm_attrs"])
if normalized["vm_attrs"] && normalized["vm_attrs"]["controllers"]
normalize_vmware_storage_controller_attributes(normalized["vm_attrs"])
end

normalized.to_h
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module Foreman::Controller::Parameters::Host
extend ActiveSupport::Concern
include Foreman::Controller::Parameters::HostBase
include Foreman::Controller::Parameters::HostCommon
include Foreman::Controller::NormalizeScsiAttributes
include Foreman::Controller::NormalizeVmwareStorageControllerAttributes

class_methods do
def host_params_filter
Expand Down Expand Up @@ -33,8 +33,8 @@ def host_params_filter

def host_params(top_level_hash = controller_name.singularize)
self.class.host_params_filter.filter_params(params, parameter_filter_context, top_level_hash).tap do |normalized|
if parameter_filter_context.ui? && normalized["compute_attributes"] && normalized["compute_attributes"]["scsi_controllers"]
normalize_scsi_attributes(normalized["compute_attributes"])
if parameter_filter_context.ui? && normalized["compute_attributes"] && normalized["compute_attributes"]["controllers"]
normalize_vmware_storage_controller_attributes(normalized["compute_attributes"])
end
end
end
Expand Down
6 changes: 0 additions & 6 deletions app/helpers/compute_resources_vms_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,12 +210,6 @@ def vm_delete_action(vm, authorizer = nil)
display_delete_if_authorized(hash_for_compute_resource_vm_path(:compute_resource_id => @compute_resource, :id => vm.identity).merge(:auth_object => @compute_resource, :authorizer => authorizer), :class => 'btn btn-danger')
end

def vsphere_scsi_controllers(compute_resource)
scsi_controllers = {}
compute_resource.scsi_controller_types.each { |type| scsi_controllers[type[:key]] = type[:title] }
scsi_controllers
end

def new_vm?(host)
return true unless host.present?
compute_object = host.compute_object
Expand Down
42 changes: 27 additions & 15 deletions app/models/compute_resources/foreman/model/vmware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,13 @@ def nictypes
}
end

def scsi_controller_types
def storage_controller_types
{
"VirtualBusLogicController" => "Bus Logic Parallel",
"VirtualLsiLogicController" => "LSI Logic Parallel",
"VirtualLsiLogicSASController" => "LSI Logic SAS",
"ParaVirtualSCSIController" => "VMware Paravirtual",
"VirtualNVMEController" => "NVME Controller",
}
end

Expand Down Expand Up @@ -482,10 +483,6 @@ def parse_args(args)
args[collection] = nested_attributes_for(collection, nested_attrs) if nested_attrs
end

# see #26402 - consume scsi_controller_type from hammer as a default scsi type
scsi_type = args.delete(:scsi_controller_type)
args[:scsi_controllers] ||= [{ type: scsi_type }] if scsi_controller_types.key?(scsi_type)

add_cdrom = args.delete(:add_cdrom)
args[:cdroms] = [new_cdrom] if add_cdrom == '1'

Expand Down Expand Up @@ -542,8 +539,15 @@ def create_vm(args = { })
raise e
end

def unassigned_volumes?(vols)
vols&.any? { |vol| !vol.key?(:controller_key) } || false
end

def new_vm(args = {})
args = parse_args args
args = args.deep_symbolize_keys
# we will pass empty scsi controllers if the volumes are assigned to nvme controllers to avoid creation of a default scsi controller.
args[:scsi_controllers] = [] if !args.key?(:scsi_controllers) && !args[:volumes].empty? && !unassigned_volumes?(args[:volumes])
opts = vm_instance_defaults.symbolize_keys.merge(args.symbolize_keys).deep_symbolize_keys
client.servers.new opts
end
Expand Down Expand Up @@ -637,7 +641,7 @@ def new_interface(attr = { })
client.interfaces.new attr
end

def new_volume(attr = { })
def new_volume(attr = {})
client.volumes.new attr.merge(:size_gb => 10)
end

Expand Down Expand Up @@ -694,6 +698,9 @@ def vm_compute_attributes(vm)
vm_attrs[:scsi_controllers] = vm.scsi_controllers.map do |controller|
controller.attributes
end
vm_attrs[:nvme_controllers] = vm.nvme_controllers.map do |controller|
controller.attributes
end
vm_attrs
end

Expand Down Expand Up @@ -726,11 +733,15 @@ def normalize_vm_attrs(vm_attrs)
normalized['add_cdrom'] = to_bool(vm_attrs['add_cdrom'])

normalized['image_name'] = images.find_by(:uuid => vm_attrs['image_id']).try(:name)

scsi_controllers = vm_attrs['scsi_controllers'] || {}
normalized['scsi_controllers'] = scsi_controllers.map.with_index do |ctrl, idx|
normalized['scsi_controllers'] = scsi_controllers.each_with_index.to_h do |ctrl, idx|
[idx.to_s, ctrl]
end.to_h
end

nvme_controllers = vm_attrs['nvme_controllers'] || {}
normalized['nvme_controllers'] = nvme_controllers.each_with_index.to_h do |ctrl, idx|
[idx.to_s, ctrl]
end

stores = datastores
volumes_attributes = vm_attrs['volumes_attributes'] || {}
Expand Down Expand Up @@ -809,6 +820,7 @@ def vm_instance_defaults
:interfaces => [new_interface],
:volumes => [new_volume],
:scsi_controllers => [{ :type => scsi_controller_default_type }],
:nvme_controllers => [],
:datacenter => datacenter,
:firmware => 'automatic',
:boot_order => ['network', 'disk']
Expand All @@ -827,12 +839,12 @@ def set_vm_volumes_attributes(vm, vm_attrs)
end

def build_vmrc_uri(host, vmid, ticket)
uri = URI::Generic.build(:scheme => 'vmrc',
:userinfo => "clone:#{ticket}",
:host => host,
:port => 443,
:path => '/',
:query => "moid=#{vmid}").to_s
uri = URI::Generic.build(:scheme => 'vmrc',
:userinfo => "clone:#{ticket}",
:host => host,
:port => 443,
:path => '/',
:query => "moid=#{vmid}").to_s
# VMRC doesn't like brackets around IPv6 addresses
uri.sub(/(.*)\[/, '\1').sub(/(.*)\]/, '\1')
end
Expand Down
6 changes: 3 additions & 3 deletions app/views/compute_resources_vms/form/vmware/_base.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ end %>
<%= react_component('StorageContainer', { data: {
config: {
vmExists: !new_vm,
controllerTypes: compute_resource.scsi_controller_types,
controllerTypes: compute_resource.storage_controller_types,
diskModeTypes: compute_resource.disk_mode_types,
paramsScope: "#{f.object_name}[scsi_controllers]",
paramsScope: "#{f.object_name}[controllers]",
datastoresUrl: available_storage_domains_api_compute_resource_path(compute_resource),
storagePodsUrl: available_storage_pods_api_compute_resource_path(compute_resource)
},
volumes: f.object.volumes.map { |volume| volume.attributes.merge(:size_gb => volume.size_gb).deep_transform_keys { |key| key.to_s.camelize(:lower).to_sym }.reject { |k,v| k == :size } },
controllers: f.object.scsi_controllers,
controllers: f.object.scsi_controllers + f.object.nvme_controllers,
cluster: f.object.cluster
}}) %>
7 changes: 4 additions & 3 deletions test/controllers/compute_attributes_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,21 @@ class ComputeAttributesControllerTest < ActionController::TestCase
assert_equal "t2.medium", compute_attributes(:one).reload.vm_attrs['flavor_id']
end

test "should update compute_attribute with scsi normalization" do
json_scsi_data = "{\"scsiControllers\":[{\"type\":\"VirtualLsiLogicController\",\"key\":1000}],\"volumes\":[{\"thin\":true,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000,\"size\":10485760,\"sizeGb\":10,\"storagePod\":\"POD-ZERO\"},{\"sizeGb\":10,\"datastore\":\"\",\"storagePod\":\"POD-ZERO\",\"thin\":false,\"eagerZero\":false,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000}]}"
test "should update compute_attribute with controllers normalization" do
json_controller_data = "{\"controllers\":[{\"type\":\"VirtualLsiLogicController\",\"key\":1000},{\"type\":\"VirtualNVMEController\",\"key\":2000}],\"volumes\":[{\"thin\":true,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000,\"size\":10485760,\"sizeGb\":10,\"storagePod\":\"POD-ZERO\"},{\"sizeGb\":10,\"datastore\":\"\",\"storagePod\":\"POD-ZERO\",\"thin\":false,\"eagerZero\":false,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000}]}"
@request.session[:redirect_path] = compute_profile_path(@compute_profile.to_param)
put :update, params: {
:id => @set,
:compute_profile_id => @compute_profile.to_param,
:compute_attribute => {
:compute_resource_id => @set.compute_resource_id,
:compute_profile_id => @set.compute_profile_id,
:vm_attrs => {"scsi_controllers" => json_scsi_data},
:vm_attrs => {"controllers" => json_controller_data},
},
}, session: set_session_user
saved_attrs = compute_attributes(:one).reload.vm_attrs
assert_equal [{"type" => "VirtualLsiLogicController", "key" => 1000}], saved_attrs['scsi_controllers']
assert_equal [{"type" => "VirtualNVMEController", "key" => 2000}], saved_attrs['nvme_controllers']
volumes_attrs = {
'0' => {
'thin' => true,
Expand Down
6 changes: 4 additions & 2 deletions test/controllers/concerns/parameters/host_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,16 @@ class HostParametersTest < ActiveSupport::TestCase
assert_equal({'type' => 'awesome', 'network' => 'superawesome'}, filtered['interfaces_attributes'][0]['compute_attributes'].to_h)
end

test 'normalizes json scsi attributes' do
inner_params = {:name => 'test.example.com', :compute_attributes => {"scsi_controllers" => "{\"scsiControllers\":[{\"type\":\"VirtualLsiLogicController\",\"key\":1000}],\"volumes\":[{\"thin\":true,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000,\"size\":10485760,\"sizeGb\":10,\"storagePod\":\"Example-Pod\"}]}"}}
test 'normalizes json controller attributes' do
inner_params = {:name => 'test.example.com', :compute_attributes => {"controllers" => "{\"controllers\":[{\"type\":\"VirtualLsiLogicController\",\"key\":1000},{\"type\":\"VirtualNVMEController\",\"key\":2000}],\"volumes\":[{\"thin\":true,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000,\"size\":10485760,\"sizeGb\":10,\"storagePod\":\"Example-Pod\"}]}"}}
expects(:params).at_least_once.returns(ActionController::Parameters.new(:host => inner_params))
expects(:parameter_filter_context).at_least_once.returns(ui_context)
filtered = host_params

assert_equal 'test.example.com', filtered['name']
assert_equal [{"type" => "VirtualLsiLogicController", "key" => 1000}], filtered['compute_attributes']['scsi_controllers']
assert_equal [{"type" => "VirtualNVMEController", "key" => 2000}], filtered['compute_attributes']['nvme_controllers']

assert_equal({"0" => {"thin" => true, "name" => "Hard disk", "mode" => "persistent", "controller_key" => 1000, "size" => 10485760, "size_gb" => 10, "storage_pod" => "Example-Pod"}}, filtered['compute_attributes']['volumes_attributes'])
end
end
6 changes: 3 additions & 3 deletions test/controllers/hosts_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1408,7 +1408,7 @@ class Host::Test < Host::Base; end
teardown { Fog.unmock! }

it 'allows edit disk size' do
scsi_controllers = [{ 'type' => 'VirtualLsiLogicController', 'key' => 1000 }]
controllers = [{ 'type' => 'VirtualLsiLogicController', 'key' => 1000 }]
volume_params = {
'thin' => true,
'name' => 'Hard disk',
Expand All @@ -1424,15 +1424,15 @@ class Host::Test < Host::Base; end

Host::Managed.any_instance.expects('compute_attributes=').with(
{
'scsi_controllers' => scsi_controllers,
'scsi_controllers' => controllers,
'volumes_attributes' => { '0' => volume_attributes },
}
)

put :update, params: {
commit: "Update",
id: @vmware_host.name,
host: { compute_attributes: { scsi_controllers: { 'scsiControllers' => scsi_controllers, 'volumes' => [volume_params] }.to_json } },
host: { compute_attributes: { controllers: { 'controllers' => controllers, 'volumes' => [volume_params] }.to_json } },
}, session: set_session_user

assert_redirected_to host_details_page_path(@vmware_host.to_param)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def allowed_vm_attr_names
resource_pool_name
scheduler_hint_filter
scsi_controllers
nvme_controllers
security_groups
security_group_id
security_group_name
Expand Down
Loading

0 comments on commit e72fcb8

Please sign in to comment.