Add ems_refresh.openshift.store_unused_images setting

Allows to still get_container_images=true, but instead of saving
metadata (labels etc) on all images openshift gave us,
save it only for images that have been mentioned by pods.

app/models/manageiq/providers/openshift/container_manager/refresh_parser.rb

@@ -8,14 +8,15 @@ def ems_inv_to_hashes(inventory, options = Config::Options.new)
         get_builds(inventory)
         get_build_pods(inventory)
         get_templates(inventory)
-        get_openshift_images(inventory)
+        # /oapi/v1/images parsing assumes we've already parsed images from pods.
+        get_openshift_images(inventory, options)
         EmsRefresh.log_inv_debug_trace(@data, "data:")
         @data
       end
 
-      def get_openshift_images(inventory)
+      def get_openshift_images(inventory, options)
         return unless inventory["image"]
-        inventory["image"].each { |img| parse_openshift_image(img) }
+        inventory["image"].each { |img| parse_openshift_image(img, options) }
       end
 
       def get_builds(inventory)
@@ -167,10 +168,11 @@ def parse_env_variables(env_variables)
         end
       end
 
-      def parse_openshift_image(openshift_image)
+      def parse_openshift_image(openshift_image, options)
         id = openshift_image[:dockerImageReference] || openshift_image[:metadata][:name]
         ref = "#{ContainerImage::DOCKER_PULLABLE_PREFIX}#{id}"
-        new_result = parse_container_image(id, ref)
+        new_result = parse_container_image(id, ref, :store_new_images => options.store_unused_images)
+        return if new_result.nil? # not storing because store_unused_images = false and wasn't mentioned by any pod
 
         if openshift_image[:dockerImageManifest].present?
           begin

config/settings.yml

@@ -36,6 +36,9 @@
         :network:
           :critical:
             - POD_HOSTPORTCONFLICT
+:ems_refresh:
+  :openshift:
+    :store_unused_images: true
 :http_proxy:
   :openshift:
     :host:

spec/models/manageiq/providers/openshift/container_manager/refresh_parser_spec.rb

@@ -5,6 +5,12 @@
   let(:parser_data) { parser.instance_variable_get('@data') }
   let(:parser_data_index) { parser.instance_variable_get('@data_index') }
 
+  let(:store_unused_images) { true }
+  let(:options) do
+    # using Struct not OpenStruct to make sure we specify all options the code actually accesses
+    Struct.new(:store_unused_images).new(store_unused_images)
+  end
+
   def given_image(image)
     parser_data[:container_images] ||= []
     parser_data[:container_images] << image
@@ -89,7 +95,7 @@ def given_image_registry(registry)
 
     it "collects data from openshift images correctly" do
       expect(parser.send(:parse_openshift_image,
-                         image_from_openshift).except(:registered_on)).to eq(
+                         image_from_openshift, options).except(:registered_on)).to eq(
                            :name                     => image_name,
                            :digest                   => image_digest,
                            :image_ref                => image_ref,
@@ -119,7 +125,7 @@ def given_image_registry(registry)
 
     it "handles openshift images without dockerImageManifest and dockerImageMetadata" do
       expect(parser.send(:parse_openshift_image,
-                         image_without_dockerImage_fields).except(:registered_on)).to eq(
+                         image_without_dockerImage_fields, options).except(:registered_on)).to eq(
                            :container_image_registry => nil,
                            :digest                   => nil,
                            :image_ref                => "docker-pullable://sha256:abcdefg",
@@ -130,7 +136,7 @@ def given_image_registry(registry)
 
     it "handles openshift image without dockerConfig" do
       expect(parser.send(:parse_openshift_image,
-                         image_without_dockerConfig).except(:registered_on)).to eq(
+                         image_without_dockerConfig, options).except(:registered_on)).to eq(
                            :container_image_registry => nil,
                            :digest                   => nil,
                            :image_ref                => "docker-pullable://sha256:abcdefg",
@@ -147,7 +153,7 @@ def given_image_registry(registry)
     # check https://bugzilla.redhat.com/show_bug.cgi?id=1414508
     it "handles openshift image without environment variables" do
       expect(parser.send(:parse_openshift_image,
-                         image_without_environment_variables).except(:registered_on)).to eq(
+                         image_without_environment_variables, options).except(:registered_on)).to eq(
                            :container_image_registry => nil,
                            :digest                   => nil,
                            :image_ref                => "docker-pullable://sha256:abcdefg",
@@ -177,12 +183,40 @@ def given_image_registry(registry)
 
       inventory = {"image" => [image_from_openshift,]}
 
-      parser.get_openshift_images(inventory)
+      parser.get_openshift_images(inventory, options)
       expect(parser_data[:container_images].size).to eq(1)
       expect(parser_data[:container_images][0][:architecture]).to eq('amd64')
       check_data_index_images
     end
 
+    context "store_unused_images=false" do
+      let(:store_unused_images) { false }
+
+      it "adds metadata to existing image" do
+        given_image(
+          :name          => image_name,
+          :tag           => image_tag,
+          :digest        => image_digest,
+          :image_ref     => image_ref,
+          :registered_on => Time.now.utc - 2.minutes
+        )
+
+        inventory = {"image" => [image_from_openshift,]}
+
+        parser.get_openshift_images(inventory, options)
+        expect(parser_data[:container_images].size).to eq(1)
+        expect(parser_data[:container_images][0][:architecture]).to eq('amd64')
+        check_data_index_images
+      end
+
+      it "doesn't add new images" do
+        inventory = {"image" => [image_from_openshift,]}
+
+        parser.get_openshift_images(inventory, options)
+        expect(parser_data[:container_images].blank?).to be true
+      end
+    end
+
     it "matches images by digest" do
       FIRST_NAME = "first_name".freeze
       FIRST_TAG = "first_tag".freeze
@@ -203,7 +237,7 @@ def given_image_registry(registry)
 
       inventory = {"image" => [image_from_openshift,]}
 
-      parser.get_openshift_images(inventory)
+      parser.get_openshift_images(inventory, options)
       expect(parser_data[:container_images].size).to eq(1)
       expect(parser_data[:container_images][0][:architecture]).to eq('amd64')
       expect(parser_data[:container_images][0][:name]).to eq(FIRST_NAME)
@@ -214,7 +248,7 @@ def given_image_registry(registry)
       def parse_single_openshift_image_with_registry
         inventory = {"image" => [image_from_openshift]}
 
-        parser.get_openshift_images(inventory)
+        parser.get_openshift_images(inventory, options)
         expect(parser_data_index[:container_image_registry][:by_host_and_port].size).to eq(1)
         expect(parser_data[:container_image_registries].size).to eq(1)
       end
@@ -231,6 +265,14 @@ def parse_single_openshift_image_with_registry
         )
         parse_single_openshift_image_with_registry
       end
+
+      context "store_unused_images=false" do
+        let(:store_unused_images) { false }
+
+        it "still adds the registries" do
+          parse_single_openshift_image_with_registry
+        end
+      end
     end
   end
 

spec/models/manageiq/providers/openshift/container_manager/refresher_spec.rb

@@ -112,7 +112,6 @@ def normal_refresh
     stub_settings_merge(
       :ems_refresh => {:openshift => {:get_container_images => false}},
     )
-
     VCR.use_cassette(described_class.name.underscore,
                      :match_requests_on              => [:path,],
                      :allow_unused_http_interactions => true) do # , :record => :new_episodes) do
@@ -127,6 +126,33 @@ def normal_refresh
     assert_specific_unused_container_image(:metadata => true, :connected => false)
   end
 
+  it 'will store only images used by pods if store_unused_images = false' do
+    stub_settings_merge(
+      :ems_refresh => {:openshift => {:store_unused_images => false}},
+    )
+    normal_refresh
+
+    @ems.reload
+
+    expect(ContainerImage.count).to eq(pod_images_count)
+    assert_specific_used_container_image(:metadata => true)
+  end
+
+  it 'will not delete previously collected metadata if store_unused_images = false' do
+    normal_refresh
+    stub_settings_merge(
+      :ems_refresh => {:openshift => {:store_unused_images => false}},
+    )
+    normal_refresh
+
+    @ems.reload
+
+    # Unused images are disconnected, metadata is retained either way.
+    expect(@ems.container_images.count).to eq(pod_images_count)
+    assert_specific_used_container_image(:metadata => true)
+    assert_specific_unused_container_image(:metadata => true, :connected => false)
+  end
+
   def assert_table_counts
     expect(ContainerGroup.count).to eq(20)
     expect(ContainerNode.count).to eq(2)