Merge pull request #15519 from enoodle/pending_container_scan_jobs_fo…

…r_all_images

JobProxyDispatcher should use all container image classes

app/models/job_proxy_dispatcher.rb

@@ -1,5 +1,6 @@
 class JobProxyDispatcher
   include Vmdb::Logging
+
   def self.dispatch
     new.dispatch
   end
@@ -93,6 +94,10 @@ def dispatch
     _log.info "Complete - Timings: #{t.inspect}"
   end
 
+  def container_image_scan_class
+    ManageIQ::Providers::Kubernetes::ContainerManager::Scanning::Job
+  end
+
   def dispatch_container_scan_jobs
     jobs_by_ems, = Benchmark.realtime_block(:pending_container_jobs) { pending_container_jobs }
     Benchmark.current_realtime[:container_jobs_to_dispatch_count] = jobs_by_ems.values.reduce(0) { |m, o| m + o.length }
@@ -192,25 +197,23 @@ def self.waiting?
     Job.where(:state => 'waiting_to_start').exists?
   end
 
-  def pending_jobs(target_class = VmOrTemplate)
-    class_name = target_class.base_class.name
+  def pending_jobs(job_class = VmScan)
     @zone = MiqServer.my_zone
-    Job.order(:id)
-       .where(:state           => "waiting_to_start")
-       .where(:dispatch_status => "pending")
-       .where(:target_class    => class_name)
-       .where("zone is null or zone = ?", @zone)
-       .where("sync_key is NULL or
-         sync_key not in (
-           select sync_key from jobs where
-             dispatch_status = 'active' and
-             state != 'finished' and
-             (zone is null or zone = ?) and
-             sync_key is not NULL)", @zone)
+    job_class.order(:id)
+             .where(:state           => "waiting_to_start")
+             .where(:dispatch_status => "pending")
+             .where("zone is null or zone = ?", @zone)
+             .where("sync_key is NULL or
+                sync_key not in (
+                  select sync_key from jobs where
+                    dispatch_status = 'active' and
+                    state != 'finished' and
+                    (zone is null or zone = ?) and
+                    sync_key is not NULL)", @zone)
   end
 
   def pending_container_jobs
-    pending_jobs(ContainerImage).each_with_object(Hash.new { |h, k| h[k] = [] }) do |job, h|
+    pending_jobs(container_image_scan_class).each_with_object(Hash.new { |h, k| h[k] = [] }) do |job, h|
       h[job.options[:ems_id]] << job
     end
   end
@@ -253,25 +256,24 @@ def busy_proxies
     end
   end
 
-  def active_scans_by_zone(target_class, count = true)
-    class_name = target_class.base_class.name
+  def active_scans_by_zone(job_class, count = true)
     actives = Hash.new(0)
-    jobs = Job.where(:zone => @zone, :dispatch_status => "active", :target_class => class_name)
+    jobs = job_class.where(:zone => @zone, :dispatch_status => "active")
               .where.not(:state => "finished")
     actives[@zone] = count ? jobs.count : jobs
     actives
   end
 
   def active_vm_scans_by_zone
-    @active_vm_scans_by_zone ||= active_scans_by_zone(VmOrTemplate)
+    @active_vm_scans_by_zone ||= active_scans_by_zone(VmScan)
   end
 
   def active_container_scans_by_zone_and_ems
     @active_container_scans_by_zone_and_ems ||= begin
       memo = Hash.new do |h, k|
         h[k] = Hash.new(0)
       end
-      active_scans_by_zone(ContainerImage, false)[@zone].each do |job|
+      active_scans_by_zone(container_image_scan_class, false)[@zone].each do |job|
         memo[@zone][job.options[:ems_id]] += 1
       end
       memo

spec/models/job_proxy_dispatcher_spec.rb

@@ -174,6 +174,7 @@
     end
 
     context "with container and vms jobs" do
+      let (:container_image_classes) { ContainerImage.descendants.collect(&:name).append('ContainerImage') }
       before(:each) do
         @jobs = (@vms + @repo_vms).collect(&:raw_scan)
         User.current_user = FactoryGirl.create(:user)
@@ -191,10 +192,10 @@
         end
 
         it "returns only container images jobs when requested" do
-          jobs = dispatcher.pending_jobs(ContainerImage)
+          jobs = dispatcher.pending_jobs(dispatcher.container_image_scan_class)
           expect(jobs.count).to eq(@container_images.count)
           jobs.each do |x|
-            expect(x.target_class).to eq 'ContainerImage'
+            expect(container_image_classes).to include x.target_class
           end
           expect(jobs.count).to be > 0 # in case something unexpected goes wrong
         end
@@ -205,14 +206,14 @@
           jobs_by_ems, = dispatcher.pending_container_jobs
           expect(jobs_by_ems.keys).to match_array(@container_providers.map(&:id))
 
-          expect(jobs_by_ems[@container_providers.first.id].count).to eq(1)
-          expect(jobs_by_ems[@container_providers.second.id].count).to eq(2)
+          expect(jobs_by_ems[@container_providers.first.id].count).to eq(1 * container_image_classes.count)
+          expect(jobs_by_ems[@container_providers.second.id].count).to eq(2 * container_image_classes.count)
         end
       end
 
       describe "#active_container_scans_by_zone_and_ems" do
         it "returns active container acans for zone" do
-          job = @jobs.find { |j| j.target_class == ContainerImage.name }
+          job = @jobs.find { |j| container_image_classes.include?(j.target_class) }
           job.update(:dispatch_status => "active")
           provider = ExtManagementSystem.find(job.options[:ems_id])
           dispatcher.instance_variable_set(:@zone, MiqServer.my_zone) # memoized during pending_jobs call
@@ -226,30 +227,36 @@
         it "dispatches jobs until reaching limit" do
           stub_settings(:container_scanning => {:concurrent_per_ems => 1})
           dispatcher.dispatch_container_scan_jobs
-          expect(Job.where(:target_class => ContainerImage, :dispatch_status => "pending").count).to eq(1)
-          # 1 per ems, one ems has 1 job and the other 2
+          expect(Job.where(:target_class => container_image_classes, :dispatch_status => "pending").count).to eq(
+            (3 * container_image_classes.count) - 2)
+          # 1 per ems, one ems has 1* job and the other 2*
+          # initial number of images per ems is multiplied by container_image_classes.count
         end
 
         it "does not dispach if limit is already reached" do
           stub_settings(:container_scanning => {:concurrent_per_ems => 1})
           dispatcher.dispatch_container_scan_jobs
-          expect(Job.where(:target_class => ContainerImage, :dispatch_status => "pending").count).to eq(1)
+          expect(Job.where(:target_class => container_image_classes, :dispatch_status => "pending").count).to eq(
+            (3 * container_image_classes.count) - 2)
           dispatcher.dispatch_container_scan_jobs
-          expect(Job.where(:target_class => ContainerImage, :dispatch_status => "pending").count).to eq(1)
+          expect(Job.where(:target_class => container_image_classes, :dispatch_status => "pending").count).to eq(
+            (3 * container_image_classes.count) - 2)
         end
 
         it "does not apply limit when concurrent_per_ems is 0" do
           stub_settings(:container_scanning => {:concurrent_per_ems => 0})
           dispatcher.dispatch_container_scan_jobs
-          expect(Job.where(:target_class => ContainerImage, :dispatch_status => "pending").count).to eq(0)
-          # 1 per ems, one ems has 1 job and the other 2
+          expect(Job.where(:target_class => container_image_classes, :dispatch_status => "pending").count).to eq(0)
+          # 1 per ems, one ems has 1* job and the other 2*
+          # initial number of images per ems is multiplied by container_image_classes.count
         end
 
         it "does not apply limit when concurrent_per_ems is -1" do
           stub_settings(:container_scanning => {:concurrent_per_ems => -1})
           dispatcher.dispatch_container_scan_jobs
-          expect(Job.where(:target_class => ContainerImage, :dispatch_status => "pending").count).to eq(0)
-          # 1 per ems, one ems has 1 job and the other 2
+          expect(Job.where(:target_class => container_image_classes, :dispatch_status => "pending").count).to eq(0)
+          # 1 per ems, one ems has 1* job and the other 2*
+          # initial number of images per ems is multiplied by container_image_classes.count
         end
       end
     end

spec/support/job_proxy_dispatcher_helper.rb

@@ -48,10 +48,16 @@ def build_entities(options = {})
 
         container_providers = []
         options[:container_providers].each_with_index do |images_count, i|
-          ems_kubernetes = FactoryGirl.create(:ems_kubernetes, :name => "test_container_provider_#{i}")
-          container_providers << ems_kubernetes
+          ems_openshift = FactoryGirl.create(:ems_openshift, :name => "test_container_provider_#{i}")
+          container_providers << ems_openshift
+          container_image_classes = ContainerImage.descendants.append(ContainerImage)
           images_count.times do |idx|
-            FactoryGirl.create(:container_image, :name => "test_container_images_#{idx}", :ems_id => ems_kubernetes.id)
+            container_image_classes.each do |cic|
+              FactoryGirl.create(:container_image,
+                                 :name   => "test_container_images_#{idx}",
+                                 :ems_id => ems_openshift.id,
+                                 :type   => cic.name)
+            end
           end
         end
         return hosts, proxies, storages, vms, repo_vms, container_providers