Fixes #34839 - Add support for NVME Controllers

theforeman · May 16, 2024 · 267a5f4 · 267a5f4
1 parent 5b00e9e
commit 267a5f4
Show file tree

Hide file tree

Showing 16 changed files with 85 additions and 45 deletions.
diff --git a/app/controllers/concerns/foreman/controller/normalize_controller_attributes.rb 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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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/...ascripts/react_app/components/hosts/storage/vmware/__tests__/StorageContainer.fixtures.js b/...ascripts/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
@@ -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/...ssets/javascripts/react_app/components/hosts/storage/vmware/__tests__/integration.test.js b/...ssets/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/...eact_app/components/hosts/storage/vmware/controller/__snapshots__/controller.test.js.snap b/...eact_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"
     >
       <label>
-        Create SCSI controller
+        Create controller
       </label>
     </div>
     <div
@@ -31,6 +31,7 @@ exports[`StorageContainer should render controller 1`] = `
             "VirtualBusLogicController": "Bus Logic Parallel",
             "VirtualLsiLogicController": "LSI Logic Parallel",
             "VirtualLsiLogicSASController": "LSI Logic SAS",
+            "VirtualNVMEController": "NVME Controller",
           }
         }
         status="RESOLVED"
@@ -76,6 +77,7 @@ exports[`StorageContainer should render controller 1`] = `
             "VirtualBusLogicController": "Bus Logic Parallel",
             "VirtualLsiLogicController": "LSI Logic Parallel",
             "VirtualLsiLogicSASController": "LSI Logic SAS",
+            "VirtualNVMEController": "NVME Controller",
           },
           "diskModeTypes": Object {
             "independent_nonpersistent": "Independent - Nonpersistent",

diff --git a/...s/javascripts/react_app/components/hosts/storage/vmware/controller/controller.fixtures.js b/...s/javascripts/react_app/components/hosts/storage/vmware/controller/controller.fixtures.js
@@ -19,6 +19,7 @@ export const props = {
       VirtualLsiLogicController: 'LSI Logic Parallel',
       VirtualLsiLogicSASController: 'LSI Logic SAS',
       ParaVirtualSCSIController: 'VMware Paravirtual',
+      VirtualNVMEController: "NVME Controller"
     },
     diskModeTypes: {
       persistent: 'Persistent',

diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/index.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/controller/index.js
@@ -106,7 +106,7 @@ const Controller = ({
     <div className="controller-container">
       <div className="controller-header">
         <div className="control-label col-md-2 controller-selected-container">
-          <label>{__('Create SCSI controller')}</label>
+          <label>{__('Create controller')}</label>
         </div>
         <div className="controller-type-container col-md-4">
           <Select

diff --git a/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/index.js b/webpack/assets/javascripts/react_app/components/hosts/storage/vmware/index.js
@@ -21,7 +21,7 @@ const filterKeyFromVolume = volume => {
 
 export const controllersToJsonString = (controllers, volumes) =>
   JSON.stringify({
-    scsiControllers: controllers,
+    controllers,
     volumes: volumes.map(v => filterKeyFromVolume(v)),
   });
 
@@ -31,7 +31,6 @@ class StorageContainer extends React.Component {
       data: { config, controllers, volumes, cluster },
       initController,
     } = this.props;
-
     initController(config, cluster, controllers, volumes);
   }
 
@@ -133,7 +132,7 @@ class StorageContainer extends React.Component {
           {this.renderControllers(controllers)}
           <input
             value={controllersToJsonString(controllers, volumes)}
-            id="scsi_controller_hidden"
+            id="controller_hidden"
             name={paramsScope}
             type="hidden"
           />

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
@@ -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
@@ -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(