consolidate supported storage types

app/models/manageiq/providers/infra_manager/metrics_capture.rb

@@ -78,7 +78,7 @@ def capture_host_targets(ems)
   # @return [Array<Storage>] supported storages
   # hosts preloaded storages and tags
   def capture_storage_targets(hosts)
-    hosts.flat_map(&:storages).uniq.select { |s| Storage.supports?(s.store_type) & s.perf_capture_enabled? }
+    hosts.flat_map(&:storages).uniq.select { |s| s.supports?(:smartstate_analysis) & s.perf_capture_enabled? }
   end
 
   # @param [ExtManagementSystem] ems

app/models/storage.rb

@@ -67,13 +67,15 @@ class Storage < ApplicationRecord
 
   delegate :queue_name_for_ems_operations, :to => :ext_management_system, :allow_nil => true
 
-  SUPPORTED_STORAGE_TYPES = %w( VMFS NFS NFS41 FCP ISCSI GLUSTERFS )
+  SUPPORTED_STORAGE_TYPES = %w[VSAN VMFS NAS NFS NFS41 ISCSI DIR FCP CSVFS NTFS GLUSTERFS].freeze
 
   supports :smartstate_analysis do
     if !ext_management_system&.class&.supports?(:smartstate_analysis)
       _("Smartstate Analysis cannot be performed on selected Datastore")
     elsif !ext_management_system&.authentication_status_ok?
       _("There are no EMSs with valid credentials for this Datastore")
+    elsif !storage_type_supported_for_ssa?
+      _("Smartstate Analysis unsupported for storage type %{store_type}" % {:store_type => store_type})
     end
   end
 
@@ -343,17 +345,12 @@ def self.scan_timer(zone_name = nil)
   end
 
   def scan(userid = "system", _role = "ems_operations")
-    unless SUPPORTED_STORAGE_TYPES.include?(store_type.upcase)
+    unless supports?(:smartstate_analysis)
       raise(MiqException::MiqUnsupportedStorage,
-            _("Action not supported for Datastore type [%{store_type}], [%{name}] with id: [%{id}]") %
-              {:store_type => store_type, :name => name, :id => id})
+            _("Action not supported for Datastore type [%{store_type}], [%{name}] with id: [%{id}] %{error}") %
+              {:store_type => store_type, :name => name, :id => id, :error => unsupported_reason(:smartstate_analysis)})
     end
 
-    unless ext_management_system&.authentication_status_ok?
-      raise(MiqException::MiqStorageError,
-            _("Check that an EMS has valid credentials for Datastore [%{name}] with id: [%{id}]") %
-              {:name => name, :id => id})
-    end
     task_name = "SmartState Analysis for [#{name}]"
     self.class.create_scan_task(task_name, userid, [self])
   end
@@ -815,9 +812,8 @@ def vm_scan_affinity
     with_relationship_type("vm_scan_storage_affinity") { parents }
   end
 
-  # @param [String, Storage] store_type upcased version of the storage type
-  def self.supports?(store_type)
-    Storage::SUPPORTED_STORAGE_TYPES.include?(store_type)
+  def storage_type_supported_for_ssa?
+    SUPPORTED_STORAGE_TYPES.include?(store_type.upcase)
   end
 
   def self.display_name(number = 1)

app/models/vm_or_template.rb

@@ -1143,14 +1143,10 @@ def path
     # Return location for RHEV-M VMs
     return rhevm_config_path if vendor.to_s.downcase == 'redhat'
 
-    case storage.store_type
-    when "VMFS"  then "[#{storage.name}] #{location}"
-    when "VSAN"  then "[#{storage.name}] #{location}"
-    when "NFS"   then "[#{storage.name}] #{location}"
-    when "NFS41" then "[#{storage.name}] #{location}"
-    when "NTFS"  then "[#{storage.name}] #{location}"
-    when "CSVFS" then "[#{storage.name}] #{location}"
-    when "NAS"   then File.join(storage.name, location)
+    if storage.store_type == "NAS"
+      File.join(storage.name, location)
+    elsif storage.storage_type_supported_for_ssa?
+      "[#{storage.name}] #{location}"
     else
       _log.warn("VM [#{name}] storage type [#{storage.store_type}] not supported")
       @path = location

app/models/vm_scan/dispatcher.rb

@@ -252,8 +252,8 @@ def get_eligible_proxies_for_job(job)
           queue_signal(job, {:args => [:abort, msg, "error"]})
           return []
         else
-          unless %w[VSAN VMFS NAS NFS NFS41 ISCSI DIR FCP CSVFS NTFS GLUSTERFS].include?(@vm.storage.store_type)
-            msg = "Vm storage type [#{@vm.storage.store_type}] unsupported [#{job.target_id}], aborting job [#{job.guid}]."
+          unless @vm.storage.supports?(:smartstate_analysis)
+            msg = @vm.storage.unsupported_reason(:smartstate_analysis)
             queue_signal(job, {:args => [:abort, msg, "error"]})
             return []
           end

spec/models/manageiq/providers/infra_manager/metrics_capture_spec.rb

@@ -2,7 +2,7 @@
   include Spec::Support::MetricHelper
 
   let(:miq_server) { EvmSpecHelper.local_miq_server }
-  let(:ems) { FactoryBot.create(:ems_vmware, :zone => miq_server.zone) }
+  let(:ems) { FactoryBot.create(:ems_vmware_with_authentication, :zone => miq_server.zone) }
 
   before do
     storages = FactoryBot.create_list(:storage_vmware, 2)

spec/models/storage_spec.rb

@@ -395,13 +395,26 @@
   end
 
   context "#supports?(:smartstate_analysis)" do
-    it "returns true for VMware Storage when queried whether it supports smartstate analysis" do
-      storage = FactoryBot.create(:storage, :ext_management_system => FactoryBot.create(:ems_vmware_with_authentication))
+    it "supports smartstate if the ems supports smartstate analysis" do
+      ems = FactoryBot.create(:ems_vmware_with_authentication)
+      expect(ems.supports?(:smartstate_analysis)).to eq(true)
+      storage = FactoryBot.create(:storage_nfs, :ext_management_system => ems)
+
       expect(storage.supports?(:smartstate_analysis)).to eq(true)
     end
 
-    it "returns false for non-vmware Storage when queried whether it supports smartstate analysis" do
-      storage = FactoryBot.create(:storage, :ext_management_system => FactoryBot.create(:ems_microsoft_with_authentication))
+    it "doesn't support smartstate for an unknown store_type" do
+      ems = FactoryBot.create(:ems_vmware_with_authentication)
+      storage = FactoryBot.create(:storage_unknown, :ext_management_system => ems)
+
+      expect(storage.supports?(:smartstate_analysis)).to eq(false)
+    end
+
+    it "doesn't support smartstate for an ems that doesn't support smartstate analysis" do
+      ems = FactoryBot.create(:ems_microsoft_with_authentication)
+      expect(ems.supports?(:smartstate_analysis)).not_to eq(true)
+      storage = FactoryBot.create(:storage_nfs, :ext_management_system => ems)
+
       expect(storage.supports?(:smartstate_analysis)).to_not eq(true)
     end
   end
@@ -506,4 +519,16 @@
       expect(storage.tenant_identity.current_tenant).to eq(Tenant.root_tenant)
     end
   end
+
+  describe "#storage_type_supported_for_ssa?" do
+    it "detects bad storage type" do
+      storage = FactoryBot.build(:storage, :store_type => "XYZ")
+      expect(storage.storage_type_supported_for_ssa?).to eq(false)
+    end
+
+    it "detects good storage type" do
+      storage = FactoryBot.build(:storage, :store_type => "NFS")
+      expect(storage.storage_type_supported_for_ssa?).to eq(true)
+    end
+  end
 end

spec/models/vm_scan/dispatcher_embedded_scan_spec.rb

@@ -2,11 +2,11 @@
   describe "dispatch embedded" do
     include Spec::Support::JobProxyDispatcherHelper
 
-    NUM_OF_VMS = 5
-    NUM_OF_REPO_VMS = 0
-    NUM_OF_HOSTS = 3
-    NUM_OF_SERVERS = 3
-    NUM_OF_STORAGES = 3
+    let(:vm_count)      { 5 }
+    let(:repo_vm_count) { 0 }
+    let(:host_count)    { 3 }
+    let(:server_count)  { 3 }
+    let(:storage_count) { 3 }
 
     def assert_at_most_x_scan_jobs_per_y_resource(x_scans, y_resource)
       vms_in_embedded_scanning = Job.where(["dispatch_status = ? AND state != ? AND target_class = ?", "active", "finished", "VmOrTemplate"])
@@ -42,22 +42,18 @@ def assert_at_most_x_scan_jobs_per_y_resource(x_scans, y_resource)
       let(:zone) { FactoryBot.create(:zone) }
       before do
         server = EvmSpecHelper.local_miq_server(:is_master => true, :name => "test_server_main_server", :zone => zone)
-        (NUM_OF_SERVERS - 1).times do |i|
-          FactoryBot.create(:miq_server, :zone => server.zone, :name => "test_server_#{i}")
-        end
+        FactoryBot.create_list(:miq_server, server_count - 1, :zone => server.zone)
 
         # TODO: We should be able to set values so we don't need to stub behavior
         allow_any_instance_of(MiqServer).to receive_messages(:is_vix_disk? => true)
         allow_any_instance_of(MiqServer).to receive_messages(:is_a_proxy? => true)
         allow_any_instance_of(MiqServer).to receive_messages(:has_active_role? => true)
-        allow_any_instance_of(ManageIQ::Providers::Vmware::InfraManager).to receive_messages(:authentication_status_ok? => true)
-        allow_any_instance_of(Host).to receive_messages(:authentication_status_ok? => true)
 
         @hosts, @proxies, @storages, @vms, @repo_vms = build_entities(
-          :hosts    => NUM_OF_HOSTS,
-          :storages => NUM_OF_STORAGES,
-          :vms      => NUM_OF_VMS,
-          :repo_vms => NUM_OF_REPO_VMS,
+          :hosts    => host_count,
+          :storages => storage_count,
+          :vms      => vm_count,
+          :repo_vms => repo_vm_count,
           :zone     => zone,
         )
       end
@@ -78,6 +74,7 @@ def assert_at_most_x_scan_jobs_per_y_resource(x_scans, y_resource)
             it "should signal 2 jobs to start" do
               stub_settings(:coresident_miqproxy => {:concurrent_per_ems => 2},
                             :ems                 => {:ems_amazon => {}})
+              MiqQueue.truncate
               VmScan::Dispatcher.dispatch
               expect(MiqQueue.count).to eq(2)
             end

spec/models/vm_scan/dispatcher_spec.rb

@@ -71,11 +71,9 @@
 
       context "with a vm without a storage" do
         before do
-          # Test a vm without a storage (ie, removed from VC but retained in the VMDB)
-          @vm = @vms.first
-          @vm.storage = nil
-          @vm.save
-          @vm.raw_scan
+          vm = @vms.first
+          vm.update(:storage => nil)
+          vm.raw_scan
         end
 
         it "should expect queue_signal and dispatch without errors" do
@@ -86,47 +84,42 @@
 
       context "with a Microsoft vm without a storage" do
         before do
-          # Test a Microsoft vm without a storage
-          @vm = @vms.first
-          @vm.storage = nil
-          @vm.vendor = "microsoft"
-          @vm.save
-          @vm.raw_scan
+          vm = @vms.first
+          vm.update(:storage => nil, :vendor => "microsoft")
+          vm.raw_scan
         end
 
-        it "should run dispatch without calling queue_signal" do
-          expect(dispatcher).not_to receive(:queue_signal)
+        it "triggers an error" do
+          expect(dispatcher).to receive(:queue_signal).with(anything, hash_including(:args => [:abort, /not located on a storage/, "error"]))
+          dispatcher.dispatch
         end
       end
 
       context "with a Microsoft vm with a Microsoft storage" do
         before do
-          # Test a Microsoft vm without a storage
-          @vm = @vms.first
-          @vm.storage.store_type = "CSVFS"
-          @vm.vendor = "microsoft"
-          @vm.save
-          @vm.raw_scan
+          vm = @vms.first
+          vm.storage.update(:store_type => "CSVFS")
+          vm.update(:vendor => "microsoft")
+          vm.raw_scan
         end
 
-        it "should run dispatch without calling queue_signal" do
-          expect(dispatcher).not_to receive(:queue_signal)
+        it "start collecting" do
+          expect(dispatcher).to receive(:queue_signal).with(anything, hash_including(:args => ["start"]))
+          dispatcher.dispatch
         end
       end
 
       context "with a Microsoft vm with an invalid storage" do
         before do
-          # Test a Microsoft vm without a storage
           @vm = @vms.first
-          @vm.storage.store_type = "XFS"
-          @vm.vendor = "microsoft"
-          @vm.save
+          @vm.storage.update(:store_type => "ABC")
+          @vm.update(:vendor => "microsoft")
           @vm.raw_scan
         end
 
-        it "should expect queue_signal and dispatch without errors" do
-          expect(dispatcher).to receive(:queue_signal)
-          expect { dispatcher.dispatch }.not_to raise_error
+        it "triggers an error" do
+          expect(dispatcher).to receive(:queue_signal).with(anything, hash_including(:args => [:abort, /unsupported/, "error"]))
+          dispatcher.dispatch
         end
       end
 

spec/models/vm_scan/dispatcher_vm_storage2proxies_spec.rb

@@ -83,20 +83,21 @@
 
             context "a vm template and invalid VC authentication" do
               before do
-                allow_any_instance_of(ManageIQ::Providers::Vmware::InfraManager).to receive_messages(:missing_credentials? => true)
-                allow(@vm).to receive_messages(:template? => true)
+                @vm.template = true
+                @hosts.each { |host| host.authentications.destroy_all }
                 @ems1 = FactoryBot.create(:ems_vmware, :name => "Ems1")
                 @vm.ext_management_system = @ems1
                 @vm.save
               end
+
               it "Vm#storage2active_proxies will return an empty list" do
                 expect(@vm.storage2active_proxies).to be_empty
               end
             end
 
             context "a vm and invalid host authentication" do
               before do
-                allow_any_instance_of(Host).to receive_messages(:missing_credentials? => true)
+                @hosts.each { |host| host.authentications.destroy_all }
                 allow(@vm).to receive_messages(:template? => false)
               end
               it "Vm#storage2active_proxies will return an empty list" do

spec/support/job_proxy_dispatcher_helper.rb

@@ -5,16 +5,13 @@ def build_entities(options = {})
         options = {:hosts => 2, :storages => 2, :vms => 3, :repo_vms => 3, :container_providers => [1, 2], :zone => FactoryBot.create(:zone)}.merge(options)
 
         proxies = []
-        storages = []
-        options[:storages].times do |i|
-          storage = FactoryBot.create(:storage, :name => "test_storage_#{i}", :store_type => "VMFS")
-          storages << storage
-        end
 
-        ems = FactoryBot.create(:ems_vmware, :name => "ems1", :zone => options[:zone])
+        ems = FactoryBot.create(:ems_vmware, :name => "ems1", :zone => options[:zone], :authtype => :default)
+        storages = FactoryBot.create_list(:storage, options[:storages], :store_type => "VMFS", :ext_management_system => ems)
+
         hosts = []
         options[:hosts].times do |i|
-          host = FactoryBot.create(:host, :name => "test_host_#{i}", :hostname => "test_host_#{i}")
+          host = FactoryBot.create(:host_with_authentication, :name => "test_host_#{i}", :hostname => "test_host_#{i}")
           max = i > storages.length ? storages.length : i
           host.storages = storages[0..max]
           host.ext_management_system = ems
@@ -32,30 +29,19 @@ def build_entities(options = {})
           vms << vm
         end
 
-        repo_vms = []
-
-        repo_storage = FactoryBot.create(:storage, :name => "test_repo_storage", :store_type => "VMFS")
-        repo_storage.hosts = []
-        repo_storage.save
-
-        options[:repo_vms].times do |i|
-          vm = FactoryBot.create(:vm_vmware, :name => "test_repo_vm_#{i}", :location => "abc/abc.vmx")
-          vm.storage = repo_storage
-          vm.host = nil
-          vm.save
-          repo_vms << vm
-        end
+        repo_storage = FactoryBot.create(:storage, :name => "test_repo_storage", :store_type => "VMFS", :hosts => [], :ext_management_system => ems)
+        repo_vms = FactoryBot.create_list(:vm_vmware, options[:repo_vms], :location => "abc/abc.vmx", :ext_management_system => ems, :host => nil, :storage => repo_storage)
 
         container_providers = []
         options[:container_providers].each_with_index do |images_count, i|
-          ems_openshift = FactoryBot.create(:ems_openshift, :name => "test_container_provider_#{i}", :zone => options[:zone])
+          ems_openshift = FactoryBot.create(:ems_openshift, :name => "test_container_provider_#{i}", :zone => options[:zone], :authtype => :default)
           container_providers << ems_openshift
           container_image_classes = ContainerImage.descendants.append(ContainerImage)
           images_count.times do |idx|
             container_image_classes.each do |cic|
               FactoryBot.create(:container_image,
                                  :name   => "test_container_images_#{idx}",
-                                 :ems_id => ems_openshift.id,
+                                 :ext_management_system => ems_openshift,
                                  :type   => cic.name)
             end
           end