From 267a5f4ddf21e1171d3906ffb70a3bd2272d852e Mon Sep 17 00:00:00 2001 From: Girija Soni Date: Thu, 16 May 2024 16:22:52 +0530 Subject: [PATCH] Fixes #34839 - Add support for NVME Controllers --- .../normalize_controller_attributes.rb | 22 ++++++++++++++ .../controller/normalize_scsi_attributes.rb | 19 ------------ .../parameters/compute_attribute.rb | 6 ++-- .../foreman/controller/parameters/host.rb | 6 ++-- app/helpers/compute_resources_vms_helper.rb | 2 +- .../compute_resources/foreman/model/vmware.rb | 30 +++++++++++++++++-- .../form/vmware/_base.html.erb | 6 ++-- .../__tests__/StorageContainer.fixtures.js | 2 +- .../storage/vmware/__tests__/index.test.js | 3 +- .../vmware/__tests__/integration.test.js | 2 +- .../__snapshots__/controller.test.js.snap | 4 ++- .../vmware/controller/controller.fixtures.js | 1 + .../hosts/storage/vmware/controller/index.js | 2 +- .../components/hosts/storage/vmware/index.js | 5 ++-- .../actions/hosts/storage/vmware.consts.js | 7 +++-- .../redux/reducers/hosts/storage/vmware.js | 13 +++++++- 16 files changed, 85 insertions(+), 45 deletions(-) create mode 100644 app/controllers/concerns/foreman/controller/normalize_controller_attributes.rb delete mode 100644 app/controllers/concerns/foreman/controller/normalize_scsi_attributes.rb diff --git a/app/controllers/concerns/foreman/controller/normalize_controller_attributes.rb b/app/controllers/concerns/foreman/controller/normalize_controller_attributes.rb new file mode 100644 index 00000000000..63cb5068e66 --- /dev/null +++ b/app/controllers/concerns/foreman/controller/normalize_controller_attributes.rb @@ -0,0 +1,22 @@ +module Foreman::Controller::NormalizeControllerAttributes + extend ActiveSupport::Concern + + private + + def normalize_controller_attributes(attrs) + ctrls_and_vol = JSON.parse(attrs["controllers"]). + deep_transform_keys { |key| key.to_s.underscore }. + deep_symbolize_keys + volumes = {} + ctrls_and_vol[:volumes].each_with_index do |vol, index| + volumes[index.to_s] = vol + end + attrs["nvme_controllers"] = ctrls_and_vol[:controllers]&.select { |controller| controller[:type].include?("VirtualNVMEController") } + attrs["scsi_controllers"] = ctrls_and_vol[:controllers]&.select { |controller| !controller[:type].include?("VirtualNVMEController") } + attrs.delete("controllers") + + attrs["volumes_attributes"] = volumes + + attrs + end +end diff --git a/app/controllers/concerns/foreman/controller/normalize_scsi_attributes.rb b/app/controllers/concerns/foreman/controller/normalize_scsi_attributes.rb deleted file mode 100644 index 4a4685c8055..00000000000 --- a/app/controllers/concerns/foreman/controller/normalize_scsi_attributes.rb +++ /dev/null @@ -1,19 +0,0 @@ -module Foreman::Controller::NormalizeScsiAttributes - extend ActiveSupport::Concern - - private - - def normalize_scsi_attributes(attrs) - scsi_and_vol = JSON.parse(attrs["scsi_controllers"]). - deep_transform_keys { |key| key.to_s.underscore }. - deep_symbolize_keys - volumes = {} - scsi_and_vol[:volumes].each_with_index do |vol, index| - volumes[index.to_s] = vol - end - - attrs["scsi_controllers"] = scsi_and_vol[:scsi_controllers] - attrs["volumes_attributes"] = volumes - attrs - end -end diff --git a/app/controllers/concerns/foreman/controller/parameters/compute_attribute.rb b/app/controllers/concerns/foreman/controller/parameters/compute_attribute.rb index 673723c4adb..82c0e1d6678 100644 --- a/app/controllers/concerns/foreman/controller/parameters/compute_attribute.rb +++ b/app/controllers/concerns/foreman/controller/parameters/compute_attribute.rb @@ -1,6 +1,6 @@ module Foreman::Controller::Parameters::ComputeAttribute extend ActiveSupport::Concern - include Foreman::Controller::NormalizeScsiAttributes + include Foreman::Controller::NormalizeControllerAttributes class_methods do def compute_attribute_params_filter @@ -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_controller_attributes(normalized["vm_attrs"]) end normalized.to_h diff --git a/app/controllers/concerns/foreman/controller/parameters/host.rb b/app/controllers/concerns/foreman/controller/parameters/host.rb index dd9386fe46e..8f52ea9049f 100644 --- a/app/controllers/concerns/foreman/controller/parameters/host.rb +++ b/app/controllers/concerns/foreman/controller/parameters/host.rb @@ -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::NormalizeControllerAttributes class_methods do def host_params_filter @@ -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_controller_attributes(normalized["compute_attributes"]) end end end diff --git a/app/helpers/compute_resources_vms_helper.rb b/app/helpers/compute_resources_vms_helper.rb index 6fae3505849..f2d9a437338 100644 --- a/app/helpers/compute_resources_vms_helper.rb +++ b/app/helpers/compute_resources_vms_helper.rb @@ -212,7 +212,7 @@ def vm_delete_action(vm, authorizer = nil) def vsphere_scsi_controllers(compute_resource) scsi_controllers = {} - compute_resource.scsi_controller_types.each { |type| scsi_controllers[type[:key]] = type[:title] } + compute_resource.controller_types.each { |type| scsi_controllers[type[:key]] = type[:title] } scsi_controllers end diff --git a/app/models/compute_resources/foreman/model/vmware.rb b/app/models/compute_resources/foreman/model/vmware.rb index ea56a432e81..0bf99b3aec8 100644 --- a/app/models/compute_resources/foreman/model/vmware.rb +++ b/app/models/compute_resources/foreman/model/vmware.rb @@ -192,12 +192,13 @@ def nictypes } end - def scsi_controller_types + def controller_types { "VirtualBusLogicController" => "Bus Logic Parallel", "VirtualLsiLogicController" => "LSI Logic Parallel", "VirtualLsiLogicSASController" => "LSI Logic SAS", "ParaVirtualSCSIController" => "VMware Paravirtual", + "VirtualNVMEController" => "NVME Controller", } end @@ -484,7 +485,7 @@ def parse_args(args) # 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) + args[:scsi_controllers] ||= [{ type: scsi_type }] if controller_types.key?(scsi_type) add_cdrom = args.delete(:add_cdrom) args[:cdroms] = [new_cdrom] if add_cdrom == '1' @@ -542,8 +543,17 @@ def create_vm(args = { }) raise e end + def unassigned_volumes?(vols) + vols&.map do |vol| + return true unless vol.key?(:controller_key) + end + false + end + def new_vm(args = {}) args = parse_args args + args = args.deep_symbolize_keys + 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 @@ -638,7 +648,12 @@ def new_interface(attr = { }) end def new_volume(attr = { }) - client.volumes.new attr.merge(:size_gb => 10) + { + :thin => true, + :name => 'Hard disk', + :mode => 'persistent', + :size_gb => 10, + } end def new_cdrom(attr = {}) @@ -694,6 +709,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 @@ -732,6 +750,11 @@ def normalize_vm_attrs(vm_attrs) [idx.to_s, ctrl] end.to_h + nvme_controllers = vm_attrs['nvme_controllers'] || {} + normalized['nvme_controllers'] = nvme_controllers.map.with_index do |ctrl, idx| + [idx.to_s, ctrl] + end.to_h + stores = datastores volumes_attributes = vm_attrs['volumes_attributes'] || {} normalized['volumes_attributes'] = volumes_attributes.each_with_object({}) do |(key, vol), volumes| @@ -809,6 +832,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'] diff --git a/app/views/compute_resources_vms/form/vmware/_base.html.erb b/app/views/compute_resources_vms/form/vmware/_base.html.erb index 4722f567972..5d783b2abc2 100644 --- a/app/views/compute_resources_vms/form/vmware/_base.html.erb +++ b/app/views/compute_resources_vms/form/vmware/_base.html.erb @@ -54,13 +54,13 @@ end %> <%= react_component('StorageContainer', { data: { config: { vmExists: !new_vm, - controllerTypes: compute_resource.scsi_controller_types, + controllerTypes: compute_resource.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 }}) %> diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js index 35d89a224cd..5ecfe56003b 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js @@ -56,7 +56,7 @@ export const vmwareData = { }; export const hiddenFieldValue = { - scsiControllers: [{ key: 1000, type: 'VirtualLsiLogicController' }], + controllers: [{ key: 1000, type: 'VirtualLsiLogicController' }], volumes: [ { controllerKey: 1000, diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/index.test.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/index.test.js index 4d1cd046dbc..efad0c37589 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/index.test.js +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/index.test.js @@ -22,8 +22,7 @@ describe('controllersToJsonString', () => { ]; const expectedJson = - '{"scsiControllers":[{"type":"VirtualLsiLogicController","key":1000}],"volumes":[{"thin":true,"name":"Hard disk","mode":"persistent","controllerKey":1000,"size":10485760,"sizeGb":10}]}'; - + "{\"controllers\":[{\"type\":\"VirtualLsiLogicController\",\"key\":1000}],\"volumes\":[{\"thin\":true,\"name\":\"Hard disk\",\"mode\":\"persistent\",\"controllerKey\":1000,\"size\":10485760,\"sizeGb\":10}]}" expect(controllersToJsonString(controllers, volumes)).toEqual(expectedJson); }); }); diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js index 75d96d7338c..3cc657bb9ba 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js @@ -18,7 +18,7 @@ describe('StorageContainer integration test', () => { it('render hidden field correctly', () => { expect( - JSON.parse(component.find('#scsi_controller_hidden').props().value) + JSON.parse(component.find('#controller_hidden').props().value) ).toEqual(hiddenFieldValue); }); diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap index cbac127d01a..4ef829b4358 100644 --- a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap +++ b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap @@ -11,7 +11,7 @@ exports[`StorageContainer should render controller 1`] = ` className="control-label col-md-2 controller-selected-container" >
- +
diff --git a/webpack/assets/javascripts/react_app/redux/actions/hosts/storage/vmware.consts.js b/webpack/assets/javascripts/react_app/redux/actions/hosts/storage/vmware.consts.js index 752df8f03e8..379f8a19007 100644 --- a/webpack/assets/javascripts/react_app/redux/actions/hosts/storage/vmware.consts.js +++ b/webpack/assets/javascripts/react_app/redux/actions/hosts/storage/vmware.consts.js @@ -1,8 +1,9 @@ import { translate as __ } from '../../../../../react_app/common/I18n'; -export const defaultControllerAttributes = { - type: 'ParaVirtualSCSIController', -}; +export const defaultControllerAttributes = + { + type: 'VirtualLsiLogicController', + } const _defaultDiskAttributes = { sizeGb: 10, diff --git a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js index db241fe0432..67caaafb456 100644 --- a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js +++ b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js @@ -26,7 +26,18 @@ const initialState = Immutable({ volumes: [], }); -const availableControllerKeys = [1000, 1001, 1002, 1003, 1004]; +const availableControllerKeys = [ + 1000, + 1001, + 1002, + 1003, + 1004, + 1005, + 1006, + 1007, + 1008, + 1009, +]; const getAvailableKey = controllers => head(