diff --git a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb index 73a4f0fad94..1f734309fad 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb @@ -667,7 +667,8 @@ def parse_container_state(state_hash) res end - def parse_container_image(image, imageID) + # may return nil if store_new_images = false + def parse_container_image(image, imageID, store_new_images: true) container_image, container_image_registry = parse_image_name(image, imageID) host_port = nil @@ -691,6 +692,7 @@ def parse_container_image(image, imageID) :container_image, :by_digest, container_image_identity) if stored_container_image.nil? + return nil unless store_new_images @data_index.store_path( :container_image, :by_digest, container_image_identity, container_image diff --git a/app/models/manageiq/providers/openshift/container_manager/refresh_parser.rb b/app/models/manageiq/providers/openshift/container_manager/refresh_parser.rb index be9ae3e97b9..1c593b28548 100644 --- a/app/models/manageiq/providers/openshift/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/openshift/container_manager/refresh_parser.rb @@ -8,13 +8,13 @@ def ems_inv_to_hashes(inventory, options = Config::Options.new) get_builds(inventory) get_build_pods(inventory) get_templates(inventory) - get_openshift_images(inventory) if options.get_container_images + get_openshift_images(inventory, options) if options.get_container_images EmsRefresh.log_inv_debug_trace(@data, "data:") @data end - def get_openshift_images(inventory) - inventory["image"].each { |img| parse_openshift_image(img) } + def get_openshift_images(inventory, options) + inventory["image"].each { |img| parse_openshift_image(img, options) } end def get_builds(inventory) @@ -169,10 +169,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? # store_unused_images = false and wasn't mentioned by any pod if openshift_image[:dockerImageManifest].present? begin diff --git a/config/settings.yml b/config/settings.yml index cb964169c95..91981f7c047 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -144,6 +144,7 @@ :openshift: :refresh_interval: 15.minutes :get_container_images: true + :store_unused_images: true :raise_vm_snapshot_complete_if_created_within: 15.minutes :refresh_interval: 24.hours :scvmm: diff --git a/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb index 27d3144f609..af2bbc73ff2 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager/refresh_parser_spec.rb @@ -676,6 +676,15 @@ expect(first_obj).to be(second_obj) end end + + it "returns existing image or nil with store_new_images=false" do + obj1 = parser.parse_container_image(shared_image_without_host, shared_ref) + obj2 = parser.parse_container_image(shared_image_without_host, shared_ref, :store_new_images => false) + obj3 = parser.parse_container_image(shared_image_without_host, unique_ref, :store_new_images => false) + expect(obj1).not_to be nil + expect(obj2).to be obj2 + expect(obj3).to be nil + end end describe "cross_link_node" do diff --git a/spec/models/manageiq/providers/openshift/container_manager/refresh_parser_spec.rb b/spec/models/manageiq/providers/openshift/container_manager/refresh_parser_spec.rb index 4c6d87427af..1a345cbc4d8 100644 --- a/spec/models/manageiq/providers/openshift/container_manager/refresh_parser_spec.rb +++ b/spec/models/manageiq/providers/openshift/container_manager/refresh_parser_spec.rb @@ -1,7 +1,34 @@ require 'recursive-open-struct' describe ManageIQ::Providers::Openshift::ContainerManager::RefreshParser do + let(:store_unused_images) { true } + let(:options) do + # using Struct not OpenStruct ensures we specify all options the code actually accesses + Struct.new(:store_unused_images).new(store_unused_images) + end let(:parser) { described_class.new } + let(:parser_data) { parser.instance_variable_get('@data') } + let(:parser_data_index) { parser.instance_variable_get('@data_index') } + + def given_image(image) + parser_data[:container_images] ||= [] + parser_data[:container_images] << image + parser_data_index.store_path(:container_image, :by_digest, image[:digest], image) + end + + def check_data_index_images + expect(parser_data[:container_images].size).to eq(parser_data_index[:container_image][:by_digest].size) + parser_data[:container_images].each do |image| + expect(parser_data_index[:container_image][:by_digest][image[:digest]]).to be(image) + end + end + + def given_image_registry(registry) + parser_data[:container_image_registries] ||= [] + parser_data[:container_image_registries] << registry + parser_data_index.store_path(:container_image_registry, :by_host_and_port, + "#{image_registry}:#{image_registry_port}", registry) + end describe "get_openshift_images" do let(:image_name) { "image_name" } @@ -69,7 +96,7 @@ it "collects data from openshift images correctly" do expect(parser.send(:parse_openshift_image, - image_from_openshift)).to eq( + image_from_openshift, options)).to eq( :name => image_name, :registered_on => Time.parse('2015-08-17T09:16:46Z').utc, :digest => image_digest, @@ -100,7 +127,7 @@ 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", @@ -111,7 +138,7 @@ it "handles openshift image without dockerConfig" do expect(parser.send(:parse_openshift_image, - image_without_dockerConfig)).to eq( + image_without_dockerConfig, options)).to eq( :container_image_registry => nil, :digest => nil, :image_ref => "docker-pullable://sha256:abcdefg", @@ -129,7 +156,7 @@ # 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)).to eq( + image_without_environment_variables, options)).to eq( :container_image_registry => nil, :digest => nil, :image_ref => "docker-pullable://sha256:abcdefg", @@ -150,62 +177,77 @@ end it "doesn't add duplicated images" do - parser.instance_variable_get('@data')[:container_images] = [{ + given_image( :name => image_name, :tag => image_tag, :digest => image_digest, :image_ref => image_ref - },] - parser.instance_variable_get('@data_index').store_path( - :container_image, - :by_digest, - image_digest, - parser.instance_variable_get('@data')[:container_images][0]) + ) inventory = {"image" => [image_from_openshift,]} - parser.get_openshift_images(inventory) - expect(parser.instance_variable_get('@data')[:container_images].size).to eq(1) - expect(parser.instance_variable_get('@data')[:container_images][0]).to eq( - parser.instance_variable_get('@data_index')[:container_image][:by_digest].values[0]) - expect(parser.instance_variable_get('@data')[:container_images][0][:architecture]).to eq('amd64') + 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 FIRST_REF = "first_ref".freeze - parser.instance_variable_get('@data')[:container_images] = [{ - :name => FIRST_NAME, - :tag => FIRST_TAG, - :digest => image_digest, - :image_ref => FIRST_REF - },] - parser.instance_variable_get('@data_index').store_path( - :container_image, - :by_digest, - image_digest, - parser.instance_variable_get('@data')[:container_images][0] + given_image( + :name => FIRST_NAME, + :tag => FIRST_TAG, + :digest => image_digest, + :image_ref => FIRST_REF, + :registered_on => Time.now.utc - 2.minutes ) inventory = {"image" => [image_from_openshift,]} - parser.get_openshift_images(inventory) - expect(parser.instance_variable_get('@data')[:container_images].size).to eq(1) - expect(parser.instance_variable_get('@data')[:container_images][0]).to eq( - parser.instance_variable_get('@data_index')[:container_image][:by_digest].values[0] - ) - expect(parser.instance_variable_get('@data')[:container_images][0][:architecture]).to eq('amd64') - expect(parser.instance_variable_get('@data')[:container_images][0][:name]).to eq(FIRST_NAME) + 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) + check_data_index_images end context "image registries from openshift images" do def parse_single_openshift_image_with_registry inventory = {"image" => [image_from_openshift]} - parser.get_openshift_images(inventory) - expect(parser.instance_variable_get('@data_index')[:container_image_registry][:by_host_and_port].size).to eq(1) - expect(parser.instance_variable_get('@data')[:container_image_registries].size).to eq(1) + 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 it "collects image registries from openshift images that are not also running pods images" do @@ -213,19 +255,21 @@ def parse_single_openshift_image_with_registry end it "avoids duplicate image registries from both running pods and openshift images" do - parser.instance_variable_get('@data')[:container_image_registries] = [{ + given_image_registry( :name => image_registry, :host => image_registry, :port => image_registry_port, - },] - parser.instance_variable_get('@data_index').store_path( - :container_image_registry, - :by_host_and_port, - "#{image_registry}:#{image_registry_port}", - parser.instance_variable_get('@data')[:container_image_registries][0] ) 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 @@ -336,20 +380,20 @@ def parse_single_openshift_image_with_registry expect(parser.send(:parse_build_pod, RecursiveOpenStruct.new(build_pod) )).to eq(:name => 'ruby-sample-build-1', - :ems_ref => 'af3d1a10-44c0-11e5-b186-0aaeec44370e', - :namespace => 'test-namespace', - :ems_created_on => '2015-08-17T09:16:46Z', - :resource_version => '165339', - :message => 'we come in peace', - :phase => 'set to stun', - :reason => 'this is a reason', - :duration => '33', - :completion_timestamp => '50', - :start_timestamp => '17', - :labels => [], + :ems_ref => 'af3d1a10-44c0-11e5-b186-0aaeec44370e', + :namespace => 'test-namespace', + :ems_created_on => '2015-08-17T09:16:46Z', + :resource_version => '165339', + :message => 'we come in peace', + :phase => 'set to stun', + :reason => 'this is a reason', + :duration => '33', + :completion_timestamp => '50', + :start_timestamp => '17', + :labels => [], :build_config => nil, - :output_docker_image_reference => 'host:port/path/to/image' - ) + :output_docker_image_reference => 'host:port/path/to/image' + ) end context "build config and pods linking" do @@ -365,14 +409,14 @@ def parse_entities(namespace_pod, namespace_config) it "links correct build pods to build configurations in same namespace" do parse_entities('namespace_1', 'namespace_1') - expect(parser.instance_variable_get('@data')[:container_build_pods].first[:build_config]).to eq( - parser.instance_variable_get('@data')[:container_builds].first + expect(parser_data[:container_build_pods].first[:build_config]).to eq( + parser_data[:container_builds].first ) end it "doesn't link build pods to build configurations in other namespace" do parse_entities('namespace_1', 'namespace_2') - expect(parser.instance_variable_get('@data')[:container_build_pods].first[:build_config]).to eq(nil) + expect(parser_data[:container_build_pods].first[:build_config]).to eq(nil) end end end @@ -446,10 +490,10 @@ def parse_entities(namespace_pod, namespace_config) expect(parser.send(:parse_project, RecursiveOpenStruct.new( :metadata => { - :annotations => { - 'openshift.io/display-name' => 'example' - }, - }, + :annotations => { + 'openshift.io/display-name' => 'example' + }, + }, ))).to eq(nil) end end diff --git a/spec/models/manageiq/providers/openshift/container_manager/refresher_spec.rb b/spec/models/manageiq/providers/openshift/container_manager/refresher_spec.rb index 95981e9ef7e..b44bfd8a7aa 100644 --- a/spec/models/manageiq/providers/openshift/container_manager/refresher_spec.rb +++ b/spec/models/manageiq/providers/openshift/container_manager/refresher_spec.rb @@ -91,6 +91,42 @@ def normal_refresh assert_container_node_with_no_hawk_attributes end + it 'will skip container_images if get_container_images = false' do + 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 + EmsRefresh.refresh(@ems) + end + + @ems.reload + + expect(ContainerImage.count).to eq(pod_images_count) + assert_specific_used_container_image(:metadata => false) + end + + it 'will not delete previously collected metadata if get_container_images = false' do + 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 + EmsRefresh.refresh(@ems) + end + + @ems.reload + + # Unused images are archived, 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 + context "when refreshing an empty DB" do # CREATING FIRST VCR # To recreate the tested objects in OpenShift use the template file: @@ -236,6 +272,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)