[WIP] Using the new tag mapping for graph refresh

[juliancheal]

Fixes BZ https://bugzilla.redhat.com/show_bug.cgi?id=1506404

Tag mapping is missing for Amazon Graph refresh.

[juliancheal]

I'm currently getting this error.

EmsRefresh::Refreshers::EmsRefresherMixin::PartialRefreshError: undefined method `[]' for nil:NilClass

from

manageiq/app/models/ems_refresh/refreshers/ems_refresher_mixin.rb:50:in `refresh'

[juliancheal]

I'm not sure if I'm parsing the tags into the correct format expected? @Ladas @cben

[cben]

manageiq/app/models/ems_refresh/refreshers/ems_refresher_mixin.rb:50:in `refresh'

Line 50 is notorious as a front, hiding all the shady business going on in refresh :-) 🚪 🎰
Try setting EmsRefresh.debug_failures = true to see the actual exception.

I'll also run this now.

[cben]

If you only use this function to feed map_labels, you only need :name and :value.

[juliancheal]

Ok thanks!

[juliancheal]

Line 50 is notorious as a front, hiding all the shady business going on in refresh :-) 🚪 🎰
Try setting EmsRefresh.debug_failures = true to see the actual exception.

I'll also run this now.

Ok so it's showing the error as

NoMethodError: undefined method `[]' for nil:NilClass

/manageiq-providers-amazon/app/models/manageiq/providers/amazon/inventory/parser/cloud_manager.rb:231:in `map_taggings'

[juliancheal]

In case it helps, this is the data I'm passing to map_taggings

parent:

InventoryObject:('i-07cf95973c3875be3', InventoryCollection:<ManageIQ::Providers::Amazon::CloudManager::Vm>, blacklist: [genealogy_parent])

tags_inventory_objects:

[InventoryObject:('141__yipikaye__yipikaye', InventoryCollection:<Tag>)]

[Ladas]

there are no @inv_collections[key]

first, you need to add the inv collection as @cben is doing it https://github.com/ManageIQ/manageiq-providers-kubernetes/pull/162/files#diff-26f0b75803f05ef203ec5099294834dfR380
then the collection will be here as in @collections

then call it e.g. as persister.taggings_for_vm

[Ladas]

the inventory collections should be added here https://github.com/Ladas/manageiq-providers-amazon/blob/f9123d84236a66348c4bd05d499c74f94f6d7594/app/models/manageiq/providers/amazon/inventory/persister/cloud_manager.rb#L26

[Ladas]

btw. NoMethodError: undefined method `[]' for nil:NilClass is because @inv_collections are nil

We have @collections here, but we should not access it directly, that is why we call it as persister.collection_name

But unless you place it on @collections[collection.name] = InventoryCollection.new..., it will not be there

[cben]

I think the taggings collections can be added in app/models/manageiq/providers/amazon/inventory_collection_default/cloud_manager.rb similar to vm_and_template_labels (and called from https://github.com/Ladas/manageiq-providers-amazon/blob/f9123d84236a66348c4bd05d499c74f94f6d7594/app/models/manageiq/providers/amazon/inventory/persister/cloud_manager.rb#L5)
playing with that now...

Also need to add persistor.add_inventory_collection(@tag_mapper.tags_to_resolve_collection) somewhere.

[Ladas]

@cben yes, https://github.com/Ladas/manageiq-providers-amazon/blob/f9123d84236a66348c4bd05d499c74f94f6d7594/app/models/manageiq/providers/amazon/inventory/persister/cloud_manager.rb#L5 is a good place to have shared

Then we need to add it to cloud persitor:
https://github.com/Ladas/manageiq-providers-amazon/blob/f1a67bdf880ad381231e234962284c49717f5ea7/app/models/manageiq/providers/amazon/inventory/persister/cloud_manager.rb#L3

And also to target refresh persistor:
https://github.com/Ladas/manageiq-providers-amazon/blob/f1a67bdf880ad381231e234962284c49717f5ea7/app/models/manageiq/providers/amazon/inventory/persister/target_collection.rb#L2

For targeted refresh, we will need to add targeted_arel and parent_inventory_collections. Just enhanced arel condition we already have, it will look like https://github.com/Ladas/manageiq-providers-amazon/blob/f1a67bdf880ad381231e234962284c49717f5ea7/app/models/manageiq/providers/amazon/inventory_collection_default/cloud_manager.rb#L143 (so the condition will not have all Vms like in :arel, but only the limited set)

(targeted refresh specs share asserts with full refresh, so we should see it fail with missing tag)

[Ladas]

@juliancheal please look closely at ManageIQ/manageiq-providers-kubernetes#162 and also add commits with specs @cben created https://github.com/ManageIQ/manageiq-providers-amazon/pull/382/files

[juliancheal]

calling, persister.taggings_for_vm just results in

NoMethodError: undefined method `taggings_for_vm for
<ManageIQ::Providers::Amazon::Inventory::Persister::CloudManager

[juliancheal]

I've added @cben's specs to my PR and implemented more of the code you both suggested.

[cben]

I think you forgot to git push?
(we're somewhat duplicating efforts, but I don't mind, in the end we'll converge to something that (1) works (2) we both understand ;-)

[juliancheal]

@cben I think I'm following along. Your PR #382 seems much better now than mine :(

[miq-bot]

Checked commits https://github.com/juliancheal/manageiq-providers-amazon/compare/6a212ee93fa1f48b9b62fb043b27c094c5dbfb4f~...58c2ab1f4db689fb066c1b3a817d8389239ef2cb with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 8 offenses detected

app/models/manageiq/providers/amazon/inventory/parser/cloud_manager.rb

• ⚠️ - Line 219, Col 48 - Lint/UnusedMethodArgument - Unused method argument - section. If it's necessary, use _ or _section as an argument name to indicate that it won't be used.
• ⚠️ - Line 219, Col 57 - Lint/UnusedMethodArgument - Unused method argument - source. If it's necessary, use _ or _source as an argument name to indicate that it won't be used.
• ⚠️ - Line 228, Col 5 - Lint/BlockAlignment - end at 228, 4 is not aligned with attributes.each do |tag| at 222, 6.
• ❗ - Line 222, Col 7 - Style/IndentationConsistency - Inconsistent indentation detected.
• ❗ - Line 223, Col 5 - Style/IndentationWidth - Use 2 (not 4) spaces for indentation.
• ❗ - Line 224, Col 11 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 225, Col 11 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.
• ❗ - Line 227, Col 7 - Style/IndentationConsistency - Inconsistent indentation detected.

[juliancheal]

Superseded by #382