Merge pull request #16743 from agrare/bz_1529725_storage_smartstate_v…

…ia_ems

Scan Storage via EMS not Host
(cherry picked from commit 74768c1)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1534753

app/models/storage.rb

@@ -69,6 +69,8 @@ class Storage < ApplicationRecord
   supports :smartstate_analysis do
     if ext_management_systems.blank? || !ext_management_system.class.supports_smartstate_analysis?
       unsupported_reason_add(:smartstate_analysis, _("Smartstate Analysis cannot be performed on selected Datastore"))
+    elsif ext_management_systems_with_authentication_status_ok.blank?
+      unsupported_reason_add(:smartstate_analysis, _("There are no EMSs with valid credentials for this Datastore"))
     end
   end
 
@@ -99,6 +101,14 @@ def ext_management_systems_in_zone(zone_name)
     ext_management_systems.select { |ems| ems.my_zone == zone_name }
   end
 
+  def ext_management_systems_with_authentication_status_ok
+    ext_management_systems.select(&:authentication_status_ok?)
+  end
+
+  def ext_management_systems_with_authentication_status_ok_in_zone(zone_name)
+    ext_management_systems_with_authentication_status_ok.select { |ems| ems.my_zone == zone_name }
+  end
+
   def storage_clusters
     parents.select { |parent| parent.kind_of?(StorageCluster) }
   end
@@ -125,14 +135,14 @@ def my_zone
     ext_management_system.my_zone
   end
 
-  def scan_starting(miq_task_id, host)
+  def scan_starting(miq_task_id, ems)
     miq_task = MiqTask.find_by(:id => miq_task_id)
     if miq_task.nil?
       _log.warn("MiqTask with ID: [#{miq_task_id}] cannot be found")
       return
     end
 
-    message = "Starting File refresh for Storage [#{name}] via Host [#{host.name}]"
+    message = "Starting File refresh for Storage [#{name}] via EMS [#{ems.name}]"
     miq_task.update_message(message)
   end
 
@@ -298,8 +308,8 @@ def self.max_qitems_per_scan_request
     ::Settings.storage.max_qitems_per_scan_request
   end
 
-  def self.max_parallel_storage_scans_per_host
-    ::Settings.storage.max_parallel_scans_per_host
+  def self.max_parallel_storage_scans_per_ems
+    ::Settings.storage.max_parallel_scans_per_ems
   end
 
   def self.scan_eligible_storages(zone_name = nil)
@@ -364,10 +374,10 @@ def scan(userid = "system", _role = "ems_operations")
               {:store_type => store_type, :name => name, :id => id})
     end
 
-    hosts = active_hosts_with_authentication_status_ok
-    if hosts.empty?
+    emss = ext_management_systems_with_authentication_status_ok
+    if emss.empty?
       raise(MiqException::MiqStorageError,
-            _("Check that a Host is running and has valid credentials for Datastore [%{name}] with id: [%{id}]") %
+            _("Check that an EMS has valid credentials for Datastore [%{name}] with id: [%{id}]") %
               {:name => name, :id => id})
     end
     task_name = "SmartState Analysis for [#{name}]"
@@ -507,12 +517,12 @@ def qmessage?(method_name)
     ($_miq_worker_current_msg.class_name == self.class.name) && ($_miq_worker_current_msg.instance_id = id) && ($_miq_worker_current_msg.method_name == method_name)
   end
 
-  def smartstate_analysis_count_for_host_id(host_id)
+  def smartstate_analysis_count_for_ems_id(ems_id)
     MiqQueue.where(
       :class_name  => self.class.name,
       :instance_id => id,
       :method_name => "smartstate_analysis",
-      :target_id   => host_id,
+      :target_id   => ems_id,
       :state       => "dequeue"
     ).count
   end
@@ -525,38 +535,38 @@ def smartstate_analysis(miq_task_id = nil)
       miq_task.state_active unless miq_task.nil?
     end
 
-    hosts = active_hosts_with_authentication_status_ok_in_zone(MiqServer.my_zone)
-    if hosts.empty?
-      message = "There are no active Hosts with valid credentials connected to Storage: [#{name}] in Zone: [#{MiqServer.my_zone}]."
+    emss = ext_management_systems_with_authentication_status_ok_in_zone(MiqServer.my_zone)
+    if emss.empty?
+      message = "There are no EMSs with valid credentials connected to Storage: [#{name}] in Zone: [#{MiqServer.my_zone}]."
       _log.warn(message)
       raise MiqException::MiqUnreachableStorage,
-            _("There are no active Hosts with valid credentials connected to Storage: [%{name}] in Zone: [%{zone}].") %
+            _("There are no EMSs with valid credentials connected to Storage: [%{name}] in Zone: [%{zone}].") %
               {:name => name, :zone => MiqServer.my_zone}
     end
 
-    max_parallel_storage_scans_per_host = self.class.max_parallel_storage_scans_per_host
-    host = nil
-    hosts.each do |h|
-      next if smartstate_analysis_count_for_host_id(h.id) >= max_parallel_storage_scans_per_host
-      host = h
+    max_parallel_storage_scans_per_ems = self.class.max_parallel_storage_scans_per_ems
+    ems = nil
+    emss.each do |e|
+      next if smartstate_analysis_count_for_ems_id(e.id) >= max_parallel_storage_scans_per_ems
+      ems = e
       break
     end
 
-    if host.nil?
+    if ems.nil?
       raise MiqException::MiqQueueRetryLater.new(:deliver_on => Time.now.utc + 1.minute) if qmessage?(method_name)
-      host = hosts.random_element
+      ems = emss.random_element
     end
 
-    $_miq_worker_current_msg.update_attributes!(:target_id => host.id) if qmessage?(method_name)
+    $_miq_worker_current_msg.update_attributes!(:target_id => ems.id) if qmessage?(method_name)
 
     st = Time.now
-    message = "Storage [#{name}] via Host [#{host.name}]"
+    message = "Storage [#{name}] via EMS [#{ems.name}]"
     _log.info("#{message}...Starting")
-    scan_starting(miq_task_id, host)
-    if host.respond_to?(:refresh_files_on_datastore)
-      host.refresh_files_on_datastore(self)
+    scan_starting(miq_task_id, ems)
+    if ems.respond_to?(:refresh_files_on_datastore)
+      ems.refresh_files_on_datastore(self)
     else
-      _log.warn("#{message}...Not Supported for #{host.class.name}")
+      _log.warn("#{message}...Not Supported for #{ems.class.name}")
     end
     update_attribute(:last_scan_on, Time.now.utc)
     _log.info("#{message}...Completed in [#{Time.now - st}] seconds")

config/settings.yml

@@ -1034,7 +1034,7 @@
       - vmtx
       - vswp
       - "%redo%"
-  :max_parallel_scans_per_host: 1
+  :max_parallel_scans_per_ems: 5
   :max_qitems_per_scan_request: 0
   :watchdog_interval: 1.minute
 :task:

spec/factories/ext_management_system.rb

@@ -139,6 +139,13 @@
     end
   end
 
+  factory :ems_vmware_with_valid_authentication,
+          :parent => :ems_vmware do
+    after(:create) do |x|
+      x.authentications << FactoryGirl.create(:authentication, :status => "Valid")
+    end
+  end
+
   factory :ems_microsoft,
           :aliases => ["manageiq/providers/microsoft/infra_manager"],
           :class   => "ManageIQ::Providers::Microsoft::InfraManager",

spec/models/storage_spec.rb

@@ -18,9 +18,9 @@
     expect(Storage.scan_watchdog_interval).to eq(5.minutes)
   end
 
-  it "#max_parallel_storage_scans_per_host" do
-    stub_settings(:storage => {'max_parallel_scans_per_host' => 3})
-    expect(Storage.max_parallel_storage_scans_per_host).to eq(3)
+  it "#max_parallel_storage_scans_per_ems" do
+    stub_settings(:storage => {'max_parallel_scans_per_ems' => 3})
+    expect(Storage.max_parallel_storage_scans_per_ems).to eq(3)
   end
 
   it "#max_qitems_per_scan_request" do
@@ -75,8 +75,8 @@
       @zone   = @server.zone
 
       @zone2     = FactoryGirl.create(:zone, :name => 'Bedrock')
-      @ems1      = FactoryGirl.create(:ems_vmware, :name => "test_vcenter1",     :zone => @zone)
-      @ems2      = FactoryGirl.create(:ems_vmware, :name => "test_vcenter2",     :zone => @zone2)
+      @ems1      = FactoryGirl.create(:ems_vmware_with_valid_authentication, :name => "test_vcenter1",     :zone => @zone)
+      @ems2      = FactoryGirl.create(:ems_vmware_with_authentication,       :name => "test_vcenter2",     :zone => @zone2)
       @storage1  = FactoryGirl.create(:storage,               :name => "test_storage_vmfs", :store_type => "VMFS")
       @storage2  = FactoryGirl.create(:storage,               :name => "test_storage_nfs",  :store_type => "NFS")
       @storage3  = FactoryGirl.create(:storage,               :name => "test_storage_foo",  :store_type => "FOO")
@@ -155,18 +155,35 @@
       expect(watchdog.deliver_on).to eq(deliver_on)
     end
 
-    context "on a host without credentials" do
+    context "on an ems without credentials" do
       it "#scan will raise error" do
-        expect { @storage1.scan }.to raise_error(MiqException::MiqStorageError)
+        expect { @storage2.scan }.to raise_error(MiqException::MiqStorageError)
       end
     end
 
-    context "on a host authentication status ok" do
+    context "on a host with authentication status ok" do
       before(:each) do
         allow_any_instance_of(Authentication).to receive(:after_authentication_changed)
         FactoryGirl.create(:authentication, :resource => @host1, :status => "Valid")
       end
 
+      it "#active_hosts_with_authentication_status_ok" do
+        expect(@storage1.active_hosts_with_authentication_status_ok).to eq([@host1])
+        expect(@storage2.active_hosts_with_authentication_status_ok).to eq([])
+        expect(@storage3.active_hosts_with_authentication_status_ok).to eq([@host1])
+      end
+
+      it "#active_hosts_with_authentication_status_ok_in_zone" do
+        expect(@storage1.active_hosts_with_authentication_status_ok_in_zone(@zone.name)).to eq([@host1])
+        expect(@storage1.active_hosts_with_authentication_status_ok_in_zone(@zone2.name)).to eq([])
+        expect(@storage2.active_hosts_with_authentication_status_ok_in_zone(@zone.name)).to eq([])
+        expect(@storage2.active_hosts_with_authentication_status_ok_in_zone(@zone2.name)).to eq([])
+        expect(@storage3.active_hosts_with_authentication_status_ok_in_zone(@zone.name)).to eq([@host1])
+        expect(@storage3.active_hosts_with_authentication_status_ok_in_zone(@zone2.name)).to eq([])
+      end
+    end
+
+    context "on an ems with authentication status ok" do
       it "#ext_management_systems" do
         expect(@storage1.ext_management_systems).to eq([@ems1])
         expect(@storage2.ext_management_systems).to eq([@ems2])
@@ -182,19 +199,13 @@
         expect(@storage3.ext_management_systems_in_zone(@zone2.name)).to eq([@ems2])
       end
 
-      it "#active_hosts_with_authentication_status_ok" do
-        expect(@storage1.active_hosts_with_authentication_status_ok).to eq([@host1])
-        expect(@storage2.active_hosts_with_authentication_status_ok).to eq([])
-        expect(@storage3.active_hosts_with_authentication_status_ok).to eq([@host1])
+      it "#ext_management_systems_with_authentication_status_ok" do
+        expect(@storage1.ext_management_systems_with_authentication_status_ok).to eq([@ems1])
       end
 
-      it "#active_hosts_with_authentication_status_ok_in_zone" do
-        expect(@storage1.active_hosts_with_authentication_status_ok_in_zone(@zone.name)).to eq([@host1])
-        expect(@storage1.active_hosts_with_authentication_status_ok_in_zone(@zone2.name)).to eq([])
-        expect(@storage2.active_hosts_with_authentication_status_ok_in_zone(@zone.name)).to eq([])
-        expect(@storage2.active_hosts_with_authentication_status_ok_in_zone(@zone2.name)).to eq([])
-        expect(@storage3.active_hosts_with_authentication_status_ok_in_zone(@zone.name)).to eq([@host1])
-        expect(@storage3.active_hosts_with_authentication_status_ok_in_zone(@zone2.name)).to eq([])
+      it "#ext_management_systems_with_authentication_status_ok_in_zone" do
+        expect(@storage1.ext_management_systems_with_authentication_status_ok_in_zone(@zone.name)).to eq([@ems1])
+        expect(@storage1.ext_management_systems_with_authentication_status_ok_in_zone(@zone2.name)).to eq([])
       end
 
       it "#scan" do
@@ -414,8 +425,9 @@
 
     it "returns true for VMware Storage when queried whether it supports smartstate analysis" do
       FactoryGirl.create(:host_vmware,
-                         :ext_management_system => FactoryGirl.create(:ems_vmware),
+                         :ext_management_system => FactoryGirl.create(:ems_vmware_with_valid_authentication),
                          :storages              => [@storage])
+
       expect(@storage.supports_smartstate_analysis?).to eq(true)
     end
 
@@ -435,7 +447,7 @@
 
     it "returns true when queried whether the storage supports smarstate analysis" do
       FactoryGirl.create(:host_vmware,
-                         :ext_management_system => FactoryGirl.create(:ems_vmware),
+                         :ext_management_system => FactoryGirl.create(:ems_vmware_with_valid_authentication),
                          :storages              => [@storage])
       expect(Storage.batch_operation_supported?(:smartstate_analysis, [@storage.id])).to eq(true)
     end
@@ -480,7 +492,7 @@
 
     it "returns true for VMware Storage when queried whether it supports smartstate analysis" do
       FactoryGirl.create(:host_vmware,
-                         :ext_management_system => FactoryGirl.create(:ems_vmware),
+                         :ext_management_system => FactoryGirl.create(:ems_vmware_with_valid_authentication),
                          :storages              => [@storage])
       expect(@storage.supports_smartstate_analysis?).to eq(true)
     end
@@ -489,17 +501,17 @@
       expect(@storage.supports_smartstate_analysis?).to_not eq(true)
     end
   end
-  describe "#smartstate_analysis_count_for_host_id" do
+  describe "#smartstate_analysis_count_for_ems_id" do
     it "returns counts" do
       EvmSpecHelper.local_miq_server
-      host = FactoryGirl.create(:host)
+      ems = FactoryGirl.create(:ems_vmware)
       storage = FactoryGirl.create(:storage)
       storage.scan_queue_item(1)
       storage.scan_queue_item(2)
-      MiqQueue.update_all(:target_id => host.id, :state => "dequeue")
+      MiqQueue.update_all(:target_id => ems.id, :state => "dequeue")
       storage.scan_queue_item(3)
 
-      expect(storage.smartstate_analysis_count_for_host_id(host.id)).to eq(2)
+      expect(storage.smartstate_analysis_count_for_ems_id(ems.id)).to eq(2)
     end
   end