From cb0100247f93e6be4195b9f233f1409a09dbd3c4 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 12 Jul 2017 15:47:27 +0300 Subject: [PATCH 1/9] hook for openshift to parse more stuff before get_container_image*s_graph --- .../container_manager/refresh_parser.rb | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) 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 25069078fc..ceca91f0f4 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb @@ -41,8 +41,21 @@ def ems_inv_to_hashes(inventory, _options = Config::Options.new) @data end - def ems_inv_to_inv_collections(ems, inventory, _options = Config::Options.new) + def ems_inv_to_inv_collections(ems, inventory, options = Config::Options.new) initialize_inventory_collections(ems) + + ems_inv_populate_collections(inventory, options) + + # The following take parsed hashes from @data_index, populated during + # parsing pods and possibly openshift images, so must be called at the end. + get_container_images_graph + get_container_image_registries_graph + + # Returning an array triggers ManagerRefresh::SaveInventory code path. + @inv_collections.values + end + + def ems_inv_populate_collections(inventory, _options) get_additional_attributes_graph(inventory) # TODO: untested? get_nodes_graph(inventory) get_namespaces_graph(inventory) @@ -54,13 +67,6 @@ def ems_inv_to_inv_collections(ems, inventory, _options = Config::Options.new) get_pods_graph(inventory) get_endpoints_and_services_graph(inventory) get_component_statuses_graph(inventory) - # The following use images resulting from parsing pods, so must be called after. - # TODO: openshift images parsing will have to plug before this. - get_container_images_graph - get_container_image_registries_graph - - # Returning an array triggers ManagerRefresh::SaveInventory code path. - @inv_collections.values end def get_nodes(inventory) From 558932de7982b6fb410f1da132831fc72a077538 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Thu, 13 Jul 2017 15:29:20 +0300 Subject: [PATCH 2/9] link pods - build pods (an openshift concept but the way the relation goes it belongs with pod handling.) --- .../kubernetes/container_manager/refresh_parser.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 ceca91f0f4..8fed8ab6d8 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb @@ -124,6 +124,8 @@ def get_pods(inventory) path_for_entity("replication_controller"), :by_namespace_and_name, replicator_ref[:namespace], replicator_ref[:name] ) + # Note: save_container_groups_inventory also links build_pod by :build_pod_name. + @data_index.store_path(key, :by_namespace_and_name, cg[:namespace], cg[:name], cg) end @@ -392,7 +394,9 @@ def get_pods_graph(inv) h[:container_project] = lazy_find_project(:name => h[:namespace]) h[:container_node] = lazy_find_node(:name => h.delete(:container_node_name)) h[:container_replicator] = lazy_find_replicator(h.delete(:container_replicator_ref)) - _build_pod_name = h.delete(:build_pod_name) + h[:container_build_pod] = lazy_find_build_pod(:namespace => h[:namespace], + :name => h.delete(:build_pod_name)) + custom_attrs = h.extract!(:labels, :node_selector_parts) children = h.extract!(:container_definitions, :containers, :container_conditions, :container_volumes) @@ -1293,5 +1297,10 @@ def lazy_find_image_registry(hash) return nil if hash.nil? @inv_collections[:container_image_registries].lazy_find_by(hash) end + + def lazy_find_build_pod(hash) + return nil if hash.nil? + @inv_collections[:container_build_pods].lazy_find_by(hash, :ref => :by_namespace_and_name) + end end end From 9ed2d5de1045c0d258ae1e8c587532c6da33d619 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 12 Jul 2017 15:48:56 +0300 Subject: [PATCH 3/9] collection fixes for openshift --- .../inventory_collections.rb | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb index 82e3362a8f..3d736a55db 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb @@ -87,6 +87,7 @@ def initialize_inventory_collections(ems) # TODO: should match on digest when available :manager_ref => [:image_ref], ) + initialize_custom_attributes_collections(ems.container_images, %w(labels docker_labels)) @inv_collections[:container_groups] = ::ManagerRefresh::InventoryCollection.new( @@ -172,13 +173,17 @@ def initialize_inventory_collections(ems) :parent => ems, :association => :container_service_port_configs, ) + @inv_collections[:container_routes] = ::ManagerRefresh::InventoryCollection.new( - :model_class => ContainerRoute, - :parent => ems, - :builder_params => {:ems_id => ems.id}, - :association => :container_routes, + :model_class => ContainerRoute, + :parent => ems, + :builder_params => {:ems_id => ems.id}, + :association => :container_routes, + :attributes_blacklist => [:namespace], ) + initialize_custom_attributes_collections(ems.container_routes, %w(labels)) + @inv_collections[:container_component_statuses] = ::ManagerRefresh::InventoryCollection.new( :model_class => ContainerComponentStatus, @@ -187,6 +192,7 @@ def initialize_inventory_collections(ems) :association => :container_component_statuses, :manager_ref => [:name], ) + @inv_collections[:container_templates] = ::ManagerRefresh::InventoryCollection.new( :model_class => ContainerTemplate, @@ -195,6 +201,7 @@ def initialize_inventory_collections(ems) :association => :container_templates, :attributes_blacklist => [:namespace], ) + initialize_custom_attributes_collections(ems.container_templates, %w(labels)) @inv_collections[:container_template_parameters] = ::ManagerRefresh::InventoryCollection.new( :model_class => ContainerTemplateParameter, @@ -202,24 +209,28 @@ def initialize_inventory_collections(ems) :association => :container_template_parameters, :manager_ref => [:container_template, :name], ) + @inv_collections[:container_builds] = ::ManagerRefresh::InventoryCollection.new( :model_class => ContainerBuild, :parent => ems, :builder_params => {:ems_id => ems.id}, :association => :container_builds, + :secondary_refs => {:by_namespace_and_name => [:namespace, :name]}, ) + initialize_custom_attributes_collections(ems.container_builds, %w(labels)) @inv_collections[:container_build_pods] = ::ManagerRefresh::InventoryCollection.new( :model_class => ContainerBuildPod, :parent => ems, :builder_params => {:ems_id => ems.id}, :association => :container_build_pods, - # TODO: is this unique? build pods do have uid that becomes ems_ref, - # but we need lazy_find by name for lookup from container_group - # TODO: rename namespace -> container_project column? + # TODO: convert namespace column -> container_project_id? :manager_ref => [:namespace, :name], + :secondary_refs => {:by_namespace_and_name => [:namespace, :name]}, ) + initialize_custom_attributes_collections(ems.container_build_pods, %w(labels)) + @inv_collections[:persistent_volumes] = ::ManagerRefresh::InventoryCollection.new( :model_class => PersistentVolume, From 77f6a9fc50397df8dad2df82a911e29f84a093fc Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Thu, 13 Jul 2017 15:25:08 +0300 Subject: [PATCH 4/9] service/registry matching (BUG: last port only) --- .../kubernetes/container_manager/refresh_parser.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 8fed8ab6d8..a24fa09686 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb @@ -491,11 +491,18 @@ def get_endpoints_and_services_graph(inv) h[:container_project] = lazy_find_project(:name => h[:namespace]) # TODO: untested? + # TODO: with multiple ports, how can I match any of them to known registries, + # like https://github.com/ManageIQ/manageiq-providers-kubernetes/pull/57 ? + if h[:container_service_port_configs].any? + registry_port = h[:container_service_port_configs].last[:port] + h[:container_image_registry] = lazy_find_image_registry( + :host => h[:portal_ip], :port => registry_port + ) + end + custom_attrs = h.extract!(:labels, :selector_parts) children = h.extract!(:container_service_port_configs) - _container_image_registry = h.delete(:container_image_registry) # TODO: derive from container_service_port_configs - h[:container_groups] = cgs_by_namespace_and_name.fetch_path(h[:namespace], h[:name]) h.except!(:tags) From d9b7e1c4098e500e5d10f4f52a2d2d1fd10cb35e Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sun, 16 Jul 2017 16:46:31 +0300 Subject: [PATCH 5/9] better error for missing custom attribute collections --- .../container_manager/inventory_collections.rb | 11 ++++++----- .../kubernetes/container_manager/refresh_parser.rb | 4 +++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb index 3d736a55db..6d3f179d99 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb @@ -267,11 +267,12 @@ def initialize_custom_attributes_collections(relation, sections) query = CustomAttribute.where(:resource_type => relation.model.name, :resource_id => relation, :section => section.to_s) - @inv_collections[[:custom_attributes_for, relation.model.name, section]] = ::ManagerRefresh::InventoryCollection.new( - :model_class => CustomAttribute, - :arel => query, - :manager_ref => [:resource, :section, :name], - ) + @inv_collections[[:custom_attributes_for, relation.model.name, section.to_s]] = + ::ManagerRefresh::InventoryCollection.new( + :model_class => CustomAttribute, + :arel => query, + :manager_ref => [:resource, :section, :name], + ) end end end 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 a24fa09686..6f3b75265d 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb @@ -559,7 +559,9 @@ def get_container_images_graph def get_custom_attributes_graph(parent, hashes_by_section) model_name = parent.inventory_collection.model_class.name hashes_by_section.each do |section, hashes| - collection = @inv_collections[[:custom_attributes_for, model_name, section.to_s]] + key = [:custom_attributes_for, model_name, section.to_s] + collection = @inv_collections[key] + raise("can't save: missing @inv_collections[#{key}]") if collection.nil? hashes.to_a.each do |h| h = h.merge(:resource => parent) if h[:section].to_s != section.to_s From 2ebd8414f455e005341cad7ed37c54f72b6ac54d Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Sun, 16 Jul 2017 17:27:06 +0300 Subject: [PATCH 6/9] inventory collections fix: project labels (TODO: needs tests) --- .../kubernetes/container_manager/inventory_collections.rb | 2 ++ .../providers/kubernetes/container_manager/refresh_parser.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb index 6d3f179d99..da6be923f9 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb @@ -9,6 +9,8 @@ def initialize_inventory_collections(ems) :association => :container_projects, :secondary_refs => {:by_name => [:name]}, ) + initialize_custom_attributes_collections(ems.container_projects, %w(labels additional_attributes)) + @inv_collections[:container_quotas] = ::ManagerRefresh::InventoryCollection.new( :model_class => ContainerQuota, :parent => ems, 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 6f3b75265d..66b79dc1cc 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb @@ -285,7 +285,7 @@ def get_namespaces_graph(inv) container_project = collection.build(h) - get_custom_attributes_graph(container_project, custom_attrs) + get_custom_attributes_graph(container_project, custom_attrs) # TODO: untested end end From d519de20e0a9ac0e8cab0bf76aa6a642e8af95ed Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 19 Jul 2017 13:04:07 +0300 Subject: [PATCH 7/9] Unhide ems_refresh.kubernetes.inventory_object_refresh setting --- .../providers/kubernetes/container_manager/refresher.rb | 2 +- config/settings.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/manageiq/providers/kubernetes/container_manager/refresher.rb b/app/models/manageiq/providers/kubernetes/container_manager/refresher.rb index a7794c0e35..9a741d7246 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresher.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresher.rb @@ -7,7 +7,7 @@ def parse_legacy_inventory(ems) entities = ems.with_provider_connection { |client| fetch_entities(client, KUBERNETES_ENTITIES) } EmsRefresh.log_inv_debug_trace(entities, "inv_hash:") - if refresher_options.try(:[], :inventory_object_refresh) + if refresher_options.inventory_object_refresh ManageIQ::Providers::Kubernetes::ContainerManager::RefreshParser.ems_inv_to_inv_collections(ems, entities, refresher_options) else ManageIQ::Providers::Kubernetes::ContainerManager::RefreshParser.ems_inv_to_hashes(entities, refresher_options) diff --git a/config/settings.yml b/config/settings.yml index d4eab9dff2..d383aebf5b 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -50,6 +50,7 @@ :ems_refresh: :kubernetes: :refresh_interval: 15.minutes + :inventory_object_refresh: false :workers: :worker_base: :event_catcher: From 6c24d7dab856f7382bcbbc97cd1466da4d873497 Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Wed, 19 Jul 2017 17:06:54 +0300 Subject: [PATCH 8/9] Implement archiving for graph refresh, run deletion tests too --- .../inventory_collections.rb | 5 ++ .../container_manager/refresher_spec.rb | 50 ++++++++++++------- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb index da6be923f9..80e51ba4ac 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb @@ -8,6 +8,7 @@ def initialize_inventory_collections(ems) :builder_params => {:ems_id => ems.id}, :association => :container_projects, :secondary_refs => {:by_name => [:name]}, + :delete_method => :disconnect_inv, ) initialize_custom_attributes_collections(ems.container_projects, %w(labels additional_attributes)) @@ -88,6 +89,7 @@ def initialize_inventory_collections(ems) # TODO: old save matches on [:image_ref, :container_image_registry_id] # TODO: should match on digest when available :manager_ref => [:image_ref], + :delete_method => :disconnect_inv, ) initialize_custom_attributes_collections(ems.container_images, %w(labels docker_labels)) @@ -99,6 +101,7 @@ def initialize_inventory_collections(ems) :association => :container_groups, :secondary_refs => {:by_namespace_and_name => [:namespace, :name]}, :attributes_blacklist => [:namespace], + :delete_method => :disconnect_inv, ) initialize_container_conditions_collection(ems.container_groups) initialize_custom_attributes_collections(ems.container_groups, %w(labels node_selectors)) @@ -109,6 +112,7 @@ def initialize_inventory_collections(ems) :builder_params => {:ems_id => ems.id}, :association => :container_definitions, # parser sets :ems_ref => "#{pod_id}_#{container_def.name}_#{container_def.image}" + :delete_method => :disconnect_inv, ) @inv_collections[:container_volumes] = ::ManagerRefresh::InventoryCollection.new( @@ -124,6 +128,7 @@ def initialize_inventory_collections(ems) :builder_params => {:ems_id => ems.id}, :association => :containers, # parser sets :ems_ref => "#{pod_id}_#{container.name}_#{container.image}" + :delete_method => :disconnect_inv, ) @inv_collections[:container_port_configs] = ::ManagerRefresh::InventoryCollection.new( diff --git a/spec/models/manageiq/providers/kubernetes/container_manager/refresher_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager/refresher_spec.rb index dcf7c947f8..2b3dbc6b95 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager/refresher_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager/refresher_spec.rb @@ -1,4 +1,7 @@ -describe ManageIQ::Providers::Kubernetes::ContainerManager::Refresher do +# instantiated at the end, for both classical and graph refresh +shared_examples "kubernetes refresher VCR tests" do |check_tag_mapping: true| + let(:check_tag_mapping) { check_tag_mapping } + before(:each) do allow(MiqServer).to receive(:my_zone).and_return("default") auth = AuthToken.new(:name => "test", :auth_key => "valid-token") @@ -28,8 +31,6 @@ ) end - let(:check_tag_mapping) { true } - def full_refresh_test 3.times do # Run three times to verify that second & third runs with existing data do not change anything VCR.use_cassette(described_class.name.underscore) do # , :record => :new_episodes) do @@ -61,22 +62,6 @@ def full_refresh_test full_refresh_test end - describe "graph refresh" do - let(:check_tag_mapping) { false } # TODO: pending implementation - - it "will perform a full refresh with inventory_objects on k8s" do - stub_settings_merge( - :ems_refresh => { - :kubernetes => { - :inventory_object_refresh => true - } - } - ) - - full_refresh_test - end - end - def assert_table_counts expect(ContainerGroup.count).to eq(2) expect(ContainerNode.count).to eq(2) @@ -528,3 +513,30 @@ def assert_disconnected(object) expect(object.archived?).to be true end end + +describe ManageIQ::Providers::Kubernetes::ContainerManager::Refresher do + context "classical refresh" do + before(:each) do + stub_settings_merge( + :ems_refresh => {:kubernetes => {:inventory_object_refresh => false}} + ) + + expect(ManageIQ::Providers::Kubernetes::ContainerManager::RefreshParser).not_to receive(:ems_inv_to_inv_collections) + end + + include_examples "kubernetes refresher VCR tests" + end + + context "graph refresh" do + before(:each) do + stub_settings_merge( + :ems_refresh => {:kubernetes => {:inventory_object_refresh => true}} + ) + + expect(ManageIQ::Providers::Kubernetes::ContainerManager::RefreshParser).not_to receive(:ems_inv_to_hashes) + end + + # TODO: pending graph tag mapping implementation + include_examples "kubernetes refresher VCR tests", :check_tag_mapping => false + end +end From c15d51f9272d278e8f4eb6bba2f09f9f92af4f0a Mon Sep 17 00:00:00 2001 From: Beni Cherniavsky-Paskin Date: Thu, 20 Jul 2017 16:40:19 +0300 Subject: [PATCH 9/9] Leave images labels & docker_labels collections up to openshift Openshift refresh_parser has a setting where it won't fetch image metadata but should not remove previously set metadata, which requires not including these InventoryCollections. --- .../kubernetes/container_manager/inventory_collections.rb | 4 ++-- .../providers/kubernetes/container_manager/refresh_parser.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb index 80e51ba4ac..070ea40333 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/inventory_collections.rb @@ -1,5 +1,5 @@ module ManageIQ::Providers::Kubernetes::ContainerManager::InventoryCollections - def initialize_inventory_collections(ems) + def initialize_inventory_collections(ems, _options) # TODO: Targeted refreshes will require adjusting the associations / arels. (duh) @inv_collections = {} @inv_collections[:container_projects] = ::ManagerRefresh::InventoryCollection.new( @@ -91,7 +91,7 @@ def initialize_inventory_collections(ems) :manager_ref => [:image_ref], :delete_method => :disconnect_inv, ) - initialize_custom_attributes_collections(ems.container_images, %w(labels docker_labels)) + # images have custom_attributes but that's done conditionally in openshift parser @inv_collections[:container_groups] = ::ManagerRefresh::InventoryCollection.new( 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 66b79dc1cc..a002c09f9a 100644 --- a/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb +++ b/app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb @@ -42,7 +42,7 @@ def ems_inv_to_hashes(inventory, _options = Config::Options.new) end def ems_inv_to_inv_collections(ems, inventory, options = Config::Options.new) - initialize_inventory_collections(ems) + initialize_inventory_collections(ems, options) ems_inv_populate_collections(inventory, options)