From eaf2277d27b0b25188c9d4b839e345547e599929 Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Tue, 21 Mar 2017 10:49:17 -0600 Subject: [PATCH 1/7] Map labels to tags. --- app/models/ems_refresh/save_inventory.rb | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/app/models/ems_refresh/save_inventory.rb b/app/models/ems_refresh/save_inventory.rb index d0c7655a31b..b62374e2042 100644 --- a/app/models/ems_refresh/save_inventory.rb +++ b/app/models/ems_refresh/save_inventory.rb @@ -40,7 +40,7 @@ def save_vms_inventory(ems, hashes, target = nil) [] end - child_keys = [:operating_system, :hardware, :custom_attributes, :snapshots, :advanced_settings, :labels] + child_keys = [:operating_system, :hardware, :custom_attributes, :snapshots, :advanced_settings, :labels, :tags] extra_infra_keys = [:host, :ems_cluster, :storage, :storages, :storage_profile, :raw_power_state, :parent_vm] extra_cloud_keys = [ :flavor, @@ -181,6 +181,26 @@ def save_vms_inventory(ems, hashes, target = nil) end end + # Convert all mapped hashes into actual tags and associate them with the object. + # The hash[:tags] object should be an array of hashes, probably created by + # the ContainerLabelTagMapping.map_labels method. Each hash in the array should + # have the following basic structure: + # + # {:category_tag_id=>139, :entry_name=>"foo", :entry_description=>"bar"} + # + # For now, the object must be a VmOrTemplate object. + # + def save_tags_inventory(object, hash) + return if hash.blank? + return unless object.kind_of?(VmOrTemplate) + + ContainerLabelTagMapping.retag_entity(object, hash[:tags]) + rescue => err + raise if EmsRefresh.debug_failures + _log.error("Auto-tagging failed on #{object.class} [#{object.name}] with error [#{err}].") + _log.log_backtrace(err) + end + def save_operating_system_inventory(parent, hash) return if hash.nil? From 668adfd5b086eacdbd060ccf2379945d788080d7 Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Fri, 24 Mar 2017 14:23:28 -0600 Subject: [PATCH 2/7] Initial commit. --- .../save_tags_inventory_spec.rb | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb diff --git a/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb new file mode 100644 index 00000000000..59358cc4381 --- /dev/null +++ b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb @@ -0,0 +1,56 @@ +context "save_vms_inventory mapping tags" do + before(:each) do + @zone = FactoryGirl.create(:zone) + @ems = FactoryGirl.create(:ems_amazon, :zone => @zone) + @vm = FactoryGirl.create(:vm, :ext_management_system => @ems) + + @tag1 = FactoryGirl.create(:tag, :name => '/managed/amazon:vm:owner') + @tag2 = FactoryGirl.create(:tag, :name => '/managed/kubernetes:container_node:stuff') + @tag3 = FactoryGirl.create(:tag, :name => '/managed/kubernetes:foo') # All + + @cat1 = FactoryGirl.create(:category, :description => 'amazon_vm_owner', :tag => @tag1) + @cat2 = FactoryGirl.create(:category, :description => 'department', :tag => @tag2) + @cat3 = FactoryGirl.create(:category, :description => 'location', :tag => @tag3) + + @map1 = FactoryGirl.create( + :container_label_tag_mapping, + :labeled_resource_type => 'Vm', + :label_name => 'owner', + :tag => @tag1 + ) + +=begin + @map2 = FactoryGirl.create( + :container_label_tag_mapping, + :labeled_resource_type => 'ContainerNode', + :label_name => 'owner', + :tag => @tag2 + ) + + @map3 = FactoryGirl.create( + :container_label_tag_mapping, + :labeled_resource_type => nil, + :label_name => 'owner', + :tag => @tag3 + ) +=end + end + + let(:data) do + {:tags => + [ + { + :category_tag_id => @cat1.tag_id, + :entry_name => 'amazon_vm_owner', + :entry_description => 'Daniel' + } + ] + } + end + + it "creates the expected number of taggings" do + EmsRefresh.save_tags_inventory(@vm, data) + taggings = Tagging.all + expect(taggings.size).to eql(1) + end +end From 93937dfcd908ec1814bd118907edba9cd59139eb Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Fri, 24 Mar 2017 16:46:25 -0600 Subject: [PATCH 3/7] Did some refactoring, added some tests. Remove the save_tags_inventory method, use the one in save_inventory.rb. Remove type check, add 3rd argument for backwards compatibility. Update taggings tests, add node tests. --- app/models/ems_refresh/save_inventory.rb | 6 +- .../ems_refresh/save_inventory_container.rb | 12 +--- .../save_tags_inventory_spec.rb | 66 +++++++++++-------- 3 files changed, 42 insertions(+), 42 deletions(-) diff --git a/app/models/ems_refresh/save_inventory.rb b/app/models/ems_refresh/save_inventory.rb index b62374e2042..7e134869b70 100644 --- a/app/models/ems_refresh/save_inventory.rb +++ b/app/models/ems_refresh/save_inventory.rb @@ -188,12 +188,8 @@ def save_vms_inventory(ems, hashes, target = nil) # # {:category_tag_id=>139, :entry_name=>"foo", :entry_description=>"bar"} # - # For now, the object must be a VmOrTemplate object. - # - def save_tags_inventory(object, hash) + def save_tags_inventory(object, hash, _target = nil) return if hash.blank? - return unless object.kind_of?(VmOrTemplate) - ContainerLabelTagMapping.retag_entity(object, hash[:tags]) rescue => err raise if EmsRefresh.debug_failures diff --git a/app/models/ems_refresh/save_inventory_container.rb b/app/models/ems_refresh/save_inventory_container.rb index e5750ce45da..fc3a5faacb1 100644 --- a/app/models/ems_refresh/save_inventory_container.rb +++ b/app/models/ems_refresh/save_inventory_container.rb @@ -230,7 +230,7 @@ def save_container_groups_inventory(ems, hashes, target = nil) h[:container_node_id] = h.fetch_path(:container_node, :id) h[:container_replicator_id] = h.fetch_path(:container_replicator, :id) h[:container_project_id] = h.fetch_path(:project, :id) - h[:container_build_pod_id] = ems.container_build_pods.find_by(:name => + h[:container_build_pod_id] = ems.container_build_pods.find_by(:name => h[:build_pod_name]).try(:id) end @@ -424,16 +424,6 @@ def save_docker_labels_inventory(entity, hashes, target = nil) save_custom_attribute_attribute_inventory(entity, :docker_labels, hashes, target) end - def save_tags_inventory(entity, hashes, _target = nil) - return if hashes.nil? - - ContainerLabelTagMapping.retag_entity(entity, hashes) # Keeps user-assigned tags. - rescue => err - raise if EmsRefresh.debug_failures - _log.error("Auto-tagging failed on #{entity.class} [#{entity.name}] with error [#{err}].") - _log.log_backtrace(err) - end - def save_selector_parts_inventory(entity, hashes, target = nil) return if hashes.nil? diff --git a/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb index 59358cc4381..01a781879eb 100644 --- a/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb +++ b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb @@ -1,8 +1,10 @@ -context "save_vms_inventory mapping tags" do +context "save_tags_inventory" do before(:each) do @zone = FactoryGirl.create(:zone) @ems = FactoryGirl.create(:ems_amazon, :zone => @zone) + @vm = FactoryGirl.create(:vm, :ext_management_system => @ems) + @node = FactoryGirl.create(:container_node, :ext_management_system => @ems) @tag1 = FactoryGirl.create(:tag, :name => '/managed/amazon:vm:owner') @tag2 = FactoryGirl.create(:tag, :name => '/managed/kubernetes:container_node:stuff') @@ -11,46 +13,58 @@ @cat1 = FactoryGirl.create(:category, :description => 'amazon_vm_owner', :tag => @tag1) @cat2 = FactoryGirl.create(:category, :description => 'department', :tag => @tag2) @cat3 = FactoryGirl.create(:category, :description => 'location', :tag => @tag3) - - @map1 = FactoryGirl.create( - :container_label_tag_mapping, - :labeled_resource_type => 'Vm', - :label_name => 'owner', - :tag => @tag1 - ) - -=begin - @map2 = FactoryGirl.create( - :container_label_tag_mapping, - :labeled_resource_type => 'ContainerNode', - :label_name => 'owner', - :tag => @tag2 - ) - - @map3 = FactoryGirl.create( - :container_label_tag_mapping, - :labeled_resource_type => nil, - :label_name => 'owner', - :tag => @tag3 - ) -=end end + # This is what ContainerLabelTagMapping.map_labels(cache, 'Type', labels) would + # return in the refresh parser. Note that we don't explicitly test the mapping + # creation here, the assumption is that these were the generated mappings. + # let(:data) do {:tags => [ { :category_tag_id => @cat1.tag_id, - :entry_name => 'amazon_vm_owner', + :entry_name => 'owner', :entry_description => 'Daniel' + }, + { + :category_tag_id => @cat2.tag_id, + :entry_name => 'stuff', + :entry_description => 'Ladas' + }, + { + :category_tag_id => @cat3.tag_id, + :entry_name => 'foo', + :entry_description => 'Bronagh' } ] } end + # Note that in these tests we're explicitly passing the object, so that's + # why the object type may not match what you would expect from the tag + # name type above. + it "creates the expected number of taggings" do EmsRefresh.save_tags_inventory(@vm, data) taggings = Tagging.all - expect(taggings.size).to eql(1) + expect(taggings.size).to eql(3) + expect(@vm.tags.size).to eql(3) + end + + it "creates the expected taggings for a VM" do + EmsRefresh.save_tags_inventory(@vm, data) + taggings = Tagging.all + expect(taggings.all?{ |e| e.taggable_id == @vm.id }).to be true + expect(taggings.map(&:taggable_type).all?{ |e| e == 'VmOrTemplate' }).to be true + expect(@vm.tags.map(&:id).sort).to eql(taggings.map(&:tag_id).sort) + end + + it "creates the expected taggings for a container node" do + EmsRefresh.save_tags_inventory(@node, data) + taggings = Tagging.all + expect(taggings.all?{ |e| e.taggable_id == @node.id }).to be true + expect(taggings.map(&:taggable_type).all?{ |e| e == 'ContainerNode' }).to be true + expect(@node.tags.map(&:id).sort).to eql(taggings.map(&:tag_id).sort) end end From 90d0f390f480d8fae7e6ef72970713e1d9a1cca3 Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Sun, 26 Mar 2017 14:36:54 -0600 Subject: [PATCH 4/7] Restore backwards compatibility with original Container handling. --- app/models/ems_refresh/save_inventory.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/ems_refresh/save_inventory.rb b/app/models/ems_refresh/save_inventory.rb index 7e134869b70..8c61fc8884e 100644 --- a/app/models/ems_refresh/save_inventory.rb +++ b/app/models/ems_refresh/save_inventory.rb @@ -188,9 +188,13 @@ def save_vms_inventory(ems, hashes, target = nil) # # {:category_tag_id=>139, :entry_name=>"foo", :entry_description=>"bar"} # + # The +hash+ argument can either be a Hash, in which case the argument should + # have a single :tags key, or a simple Array of hashes. + # def save_tags_inventory(object, hash, _target = nil) return if hash.blank? - ContainerLabelTagMapping.retag_entity(object, hash[:tags]) + tags = hash.is_a?(Hash) ? hash[:tags] : hash + ContainerLabelTagMapping.retag_entity(object, tags) rescue => err raise if EmsRefresh.debug_failures _log.error("Auto-tagging failed on #{object.class} [#{object.name}] with error [#{err}].") From ccd3f0f67978ecf1f653a562fd43df8cd57238bd Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Sun, 26 Mar 2017 14:37:11 -0600 Subject: [PATCH 5/7] Rubocop fixes. --- .../save_inventory/save_tags_inventory_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb index 01a781879eb..8d1e8fb0fb7 100644 --- a/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb +++ b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb @@ -55,16 +55,16 @@ it "creates the expected taggings for a VM" do EmsRefresh.save_tags_inventory(@vm, data) taggings = Tagging.all - expect(taggings.all?{ |e| e.taggable_id == @vm.id }).to be true - expect(taggings.map(&:taggable_type).all?{ |e| e == 'VmOrTemplate' }).to be true + expect(taggings.all? { |e| e.taggable_id == @vm.id }).to be true + expect(taggings.map(&:taggable_type).all? { |e| e == 'VmOrTemplate' }).to be true expect(@vm.tags.map(&:id).sort).to eql(taggings.map(&:tag_id).sort) end it "creates the expected taggings for a container node" do EmsRefresh.save_tags_inventory(@node, data) taggings = Tagging.all - expect(taggings.all?{ |e| e.taggable_id == @node.id }).to be true - expect(taggings.map(&:taggable_type).all?{ |e| e == 'ContainerNode' }).to be true + expect(taggings.all? { |e| e.taggable_id == @node.id }).to be true + expect(taggings.map(&:taggable_type).all? { |e| e == 'ContainerNode' }).to be true expect(@node.tags.map(&:id).sort).to eql(taggings.map(&:tag_id).sort) end end From 27eff65f73847c4f9812949dbf8b63e071250072 Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Sun, 26 Mar 2017 14:43:53 -0600 Subject: [PATCH 6/7] Updated variable names and comments. --- app/models/ems_refresh/save_inventory.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/ems_refresh/save_inventory.rb b/app/models/ems_refresh/save_inventory.rb index 8c61fc8884e..1950b7b4a89 100644 --- a/app/models/ems_refresh/save_inventory.rb +++ b/app/models/ems_refresh/save_inventory.rb @@ -182,18 +182,18 @@ def save_vms_inventory(ems, hashes, target = nil) end # Convert all mapped hashes into actual tags and associate them with the object. - # The hash[:tags] object should be an array of hashes, probably created by - # the ContainerLabelTagMapping.map_labels method. Each hash in the array should - # have the following basic structure: + # The collection or collection[:tags] object should be an array of hashes, probably + # created by the ContainerLabelTagMapping.map_labels method. Each hash in the array + # should have the following basic structure: # # {:category_tag_id=>139, :entry_name=>"foo", :entry_description=>"bar"} # - # The +hash+ argument can either be a Hash, in which case the argument should - # have a single :tags key, or a simple Array of hashes. + # The +collection+ argument can either be a Hash, in which case the argument + # should have a single :tags key, or a simple Array of hashes. # - def save_tags_inventory(object, hash, _target = nil) - return if hash.blank? - tags = hash.is_a?(Hash) ? hash[:tags] : hash + def save_tags_inventory(object, collection, _target = nil) + return if collection.blank? + tags = collection.is_a?(Hash) ? collection[:tags] : collection ContainerLabelTagMapping.retag_entity(object, tags) rescue => err raise if EmsRefresh.debug_failures From e7334400a6367083e65ff9bc9e4663809f9d19c6 Mon Sep 17 00:00:00 2001 From: Daniel Berger Date: Mon, 27 Mar 2017 10:06:43 -0600 Subject: [PATCH 7/7] More rubocop fixes. --- app/models/ems_refresh/save_inventory.rb | 2 +- app/models/ems_refresh/save_inventory_container.rb | 3 +-- .../ems_refresh/save_inventory/save_tags_inventory_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/app/models/ems_refresh/save_inventory.rb b/app/models/ems_refresh/save_inventory.rb index 1950b7b4a89..d7be2d178bc 100644 --- a/app/models/ems_refresh/save_inventory.rb +++ b/app/models/ems_refresh/save_inventory.rb @@ -193,7 +193,7 @@ def save_vms_inventory(ems, hashes, target = nil) # def save_tags_inventory(object, collection, _target = nil) return if collection.blank? - tags = collection.is_a?(Hash) ? collection[:tags] : collection + tags = collection.kind_of?(Hash) ? collection[:tags] : collection ContainerLabelTagMapping.retag_entity(object, tags) rescue => err raise if EmsRefresh.debug_failures diff --git a/app/models/ems_refresh/save_inventory_container.rb b/app/models/ems_refresh/save_inventory_container.rb index fc3a5faacb1..0fbcdb10c9a 100644 --- a/app/models/ems_refresh/save_inventory_container.rb +++ b/app/models/ems_refresh/save_inventory_container.rb @@ -230,8 +230,7 @@ def save_container_groups_inventory(ems, hashes, target = nil) h[:container_node_id] = h.fetch_path(:container_node, :id) h[:container_replicator_id] = h.fetch_path(:container_replicator, :id) h[:container_project_id] = h.fetch_path(:project, :id) - h[:container_build_pod_id] = ems.container_build_pods.find_by(:name => - h[:build_pod_name]).try(:id) + h[:container_build_pod_id] = ems.container_build_pods.find_by(:name => h[:build_pod_name]).try(:id) end save_inventory_multi(ems.container_groups, hashes, deletes, [:ems_ref], diff --git a/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb index 8d1e8fb0fb7..516a6a117e3 100644 --- a/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb +++ b/spec/models/ems_refresh/save_inventory/save_tags_inventory_spec.rb @@ -20,8 +20,8 @@ # creation here, the assumption is that these were the generated mappings. # let(:data) do - {:tags => - [ + { + :tags => [ { :category_tag_id => @cat1.tag_id, :entry_name => 'owner',