-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Map Amazon labels to tags #14436
Map Amazon labels to tags #14436
Changes from 6 commits
eaf2277
668adfd
93937df
90d0f39
ccd3f0f
27eff65
e733440
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 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 +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, collection, _target = nil) | ||
return if collection.blank? | ||
tags = collection.is_a?(Hash) ? collection[:tags] : collection | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we actually use the Hash somewhere? The saving code should also pass it as a child, so collection[:tags] should be always passed, right? |
||
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}].") | ||
_log.log_backtrace(err) | ||
end | ||
|
||
def save_operating_system_inventory(parent, hash) | ||
return if hash.nil? | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
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') | ||
@tag3 = FactoryGirl.create(:tag, :name => '/managed/kubernetes:foo') # All | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Completely doesn't matter in this test: An actual "All" mapping would use |
||
|
||
@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) | ||
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 => '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(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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little surprised you need to deal with {:tags => ...} form, IIUC ManageIQ/manageiq-providers-amazon#183 creates :labels and :tags on same level, you have :labels and :tags in child keys on same level, and
save_child_inventory
callssave_#{k}_inventory
onhashes[k]
so I'd expectsave_tags_inventory
to not see the :tags, just the value.But I don't mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cben This is something we plan to refactor in the future, i.e. generate the tags on the core side rather than the refresher side.