Merge pull request ManageIQ#11806 from cben/auto-tagging-noinsert

Rewrite of auto-tagging, fixing multiple bugs

app/models/container_label_tag_mapping.rb

@@ -9,91 +9,93 @@ class ContainerLabelTagMapping < ApplicationRecord
   # - When `label_value` is NULL, we map this name with any value to per-value tags.
   #   In this case, `tag` specifies the category under which to create
   #   the value-specific tag (and classification) on demand.
-  #   We then also add a specific `label_value`->specific `tag` mapping here.
+  #
+  # All involved tags must also have a Classification.
 
   belongs_to :tag
 
-  def self.drop_cache
-    @hash_all_by_name_type_value = nil
-  end
-
-  # Returns {[name, type, value] => [tag, ...]}}} hash.
-  def self.hash_all_by_name_type_value
-    unless @hash_all_by_name_type_value
-      @hash_all_by_name_type_value = {}
-      includes(:tag).find_each { |m| load_mapping_into_hash(m) }
-    end
-    @hash_all_by_name_type_value
-  end
-
-  def self.load_mapping_into_hash(mapping)
-    return unless @hash_all_by_name_type_value
-    key = [mapping.label_name, mapping.labeled_resource_type, mapping.label_value].freeze
-    @hash_all_by_name_type_value[key] ||= []
-    @hash_all_by_name_type_value[key] << mapping.tag
+  # Pass the data this returns to map_* methods.
+  def self.cache
+    # {[name, type, value] => [tag_id, ...]}
+    in_my_region.find_each
+                .group_by { |m| [m.label_name, m.labeled_resource_type, m.label_value].freeze }
+                .transform_values { |mappings| mappings.collect(&:tag_id) }
   end
-  private_class_method :load_mapping_into_hash
 
-  # All specific-value tags that can be assigned by this mapping.
-  def self.mappable_tags
-    hash_all_by_name_type_value.collect_concat do |(_name, _type, value), tags|
-      value ? tags : []
-    end
-  end
+  # We expect labels to be {:name, :value} hashes
+  # and return {:tag_id} or {:category_tag_id, :entry_name, :entry_description} hashes.
 
-  # Main entry point.
-  def self.tags_for_entity(entity)
-    entity.labels.collect_concat { |label| tags_for_label(label) }
+  def self.map_labels(cache, type, labels)
+    labels.collect_concat { |label| map_label(cache, type, label) }.uniq
   end
 
-  def self.tags_for_label(label)
+  def self.map_label(cache, type, label)
     # Apply both specific-type and any-type, independently.
-    (tags_for_name_type_value(label.name, label.resource_type, label.value) +
-     tags_for_name_type_value(label.name, nil,                 label.value))
+    (map_name_type_value(cache, label[:name], type, label[:value]) +
+     map_name_type_value(cache, label[:name], nil,  label[:value]))
   end
 
-  def self.tags_for_name_type_value(name, type, value)
-    specific_value = hash_all_by_name_type_value[[name, type, value]] || []
-    any_value      = hash_all_by_name_type_value[[name, type, nil]]   || []
+  def self.map_name_type_value(cache, name, type, value)
+    specific_value = cache[[name, type, value]] || []
+    any_value      = cache[[name, type, nil]]   || []
     if !specific_value.empty?
-      specific_value
+      specific_value.map { |tag_id| {:tag_id => tag_id} }
     else
-      any_value.map do |category_tag|
-        create_specific_value_mapping(name, type, value, category_tag).tag
+      if value.empty?
+        [] # Don't map empty value to any tag.
+      else
+        # Note: if the way we compute `entry_name` changes,
+        # consider what will happen to previously created tags.
+        any_value.map do |tag_id|
+          {:category_tag_id   => tag_id,
+           :entry_name        => Classification.sanitize_name(value),
+           :entry_description => value}
+        end
       end
     end
   end
-  private_class_method :tags_for_name_type_value
+  private_class_method :map_name_type_value
 
-  # If this is an open ended any-value mapping, finds or creates a
-  # specific-value mapping to a specific tag.
-  def self.create_specific_value_mapping(name, type, value, category_tag)
-    new_tag = create_tag(name, value, category_tag)
-    new_mapping = create!(:labeled_resource_type => type, :label_name => name, :label_value => value,
-                          :tag => new_tag)
-    load_mapping_into_hash(new_mapping)
-    new_mapping
+  # Given a hash built by `map_*` methods, returns a Tag (creating if needed).
+  def self.find_or_create_tag(tag_hash)
+    if tag_hash[:tag_id]
+      Tag.find(tag_hash[:tag_id])
+    else
+      category = Tag.find(tag_hash[:category_tag_id]).classification
+      entry = category.find_entry_by_name(tag_hash[:entry_name])
+      unless entry
+        category.lock :exclusive do
+          begin
+            entry = category.add_entry(:name        => tag_hash[:entry_name],
+                                       :description => tag_hash[:entry_description])
+            entry.save!
+          rescue ActiveRecord::RecordInvalid
+            entry = category.find_entry_by_name(tag_hash[:entry_name])
+          end
+        end
+      end
+      entry.tag
+    end
   end
-  private_class_method :create_specific_value_mapping
 
-  def self.create_tag(name, value, category_tag)
-    category = category_tag.classification
-    unless category
-      category = Classification.create_category!(:description => "Kubernetes label '#{name}'",
-                                                 :read_only   => true,
-                                                 :tag         => category_tag)
-    end
+  def self.controls_tag?(tag)
+    return false unless tag.classification.try(:read_only) # never touch user-assignable tags.
+    tag_ids = [tag.id, tag.category.tag_id].uniq
+    where(:tag_id => tag_ids).any?
+  end
 
-    if value.empty?
-      entry_name = ':empty:' # ':' character won't occur in kubernetes values.
-      description = '<empty value>'
-    else
-      entry_name = Classification.sanitize_name(value)
-      description = value
+  # Assign/unassign mapping-controlled tags, preserving user-assigned tags.
+  def self.retag_entity(entity, tag_hashes)
+    mapped_tags = tag_hashes.map { |tag_hash| find_or_create_tag(tag_hash) }
+    existing_tags = entity.tags
+    Tagging.transaction do
+      (mapped_tags - existing_tags).each do |tag|
+        Tagging.create!(:taggable => entity, :tag => tag)
+      end
+      (existing_tags - mapped_tags).select { |tag| controls_tag?(tag) }.tap do |tags|
+        Tagging.where(:taggable => entity, :tag => tags.collect(&:id)).destroy_all
+      end
     end
-    entry = category.add_entry(:name => entry_name, :description => description)
-    entry.save!
-    entry.tag
+    entity.tags.reset
   end
-  private_class_method :create_tag
 end

app/models/ems_refresh/save_inventory_container.rb

@@ -28,7 +28,7 @@ def save_container_projects_inventory(ems, hashes, target = nil)
               end
 
     save_inventory_multi(ems.container_projects, hashes, deletes, [:ems_ref],
-                         [:labels_and_tags], [], true)
+                         [:labels, :tags], [], true)
     store_ids_for_new_records(ems.container_projects, hashes, :ems_ref)
   end
 
@@ -146,7 +146,7 @@ def save_container_routes_inventory(ems, hashes, target = nil)
     end
 
     save_inventory_multi(ems.container_routes, hashes, deletes, [:ems_ref],
-                         [:labels_and_tags], [:container_service, :project, :namespace])
+                         [:labels, :tags], [:container_service, :project, :namespace])
     store_ids_for_new_records(ems.container_routes, hashes, :ems_ref)
   end
 
@@ -162,7 +162,7 @@ def save_container_nodes_inventory(ems, hashes, target = nil)
               end
 
     save_inventory_multi(ems.container_nodes, hashes, deletes, [:ems_ref],
-                         [:labels_and_tags, :computer_system, :container_conditions], [:namespace])
+                         [:labels, :tags, :computer_system, :container_conditions], [:namespace])
     store_ids_for_new_records(ems.container_nodes, hashes, :ems_ref)
   end
 
@@ -186,7 +186,7 @@ def save_container_replicators_inventory(ems, hashes, target = nil)
     end
 
     save_inventory_multi(ems.container_replicators, hashes, deletes, [:ems_ref],
-                         [:labels_and_tags, :selector_parts], [:project, :namespace])
+                         [:labels, :tags, :selector_parts], [:project, :namespace])
     store_ids_for_new_records(ems.container_replicators, hashes, :ems_ref)
   end
 
@@ -208,7 +208,7 @@ def save_container_services_inventory(ems, hashes, target = nil)
     end
 
     save_inventory_multi(ems.container_services, hashes, deletes, [:ems_ref],
-                         [:labels_and_tags, :selector_parts, :container_service_port_configs],
+                         [:labels, :tags, :selector_parts, :container_service_port_configs],
                          [:container_groups, :project, :container_image_registry, :namespace])
 
     store_ids_for_new_records(ems.container_services, hashes, :ems_ref)
@@ -234,7 +234,7 @@ def save_container_groups_inventory(ems, hashes, target = nil)
     end
 
     save_inventory_multi(ems.container_groups, hashes, deletes, [:ems_ref],
-                         [:container_definitions, :containers, :labels_and_tags,
+                         [:container_definitions, :containers, :labels, :tags,
                           :node_selector_parts, :container_conditions, :container_volumes],
                          [:container_node, :container_replicator, :project, :namespace, :build_pod_name],
                          true)
@@ -409,21 +409,14 @@ def save_labels_inventory(entity, hashes, target = nil)
     store_ids_for_new_records(entity.labels, hashes, [:section, :name])
   end
 
-  def save_labels_and_tags_inventory(entity, hashes, target = nil)
-    save_labels_inventory(entity, hashes, target)
-    save_tags_generated_from_labels(entity, hashes, target)
-  end
-
-  def save_tags_generated_from_labels(entity, hashes, _target = nil)
-    labels = hashes.collect do |label_hash|
-      OpenStruct.new(:resource_type => entity.class.base_class.name,
-                     :name          => label_hash[:name],
-                     :value         => label_hash[:value])
-    end
-    current_tags = labels.collect_concat { |label| ContainerLabelTagMapping.tags_for_label(label) }
-    mappable_tags = ContainerLabelTagMapping.mappable_tags
+  def save_tags_inventory(entity, hashes, _target = nil)
+    return if hashes.nil?
 
-    entity.tags = entity.tags - mappable_tags + current_tags
+    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)
@@ -464,7 +457,7 @@ def save_container_builds_inventory(ems, hashes, target = nil)
     hashes.each do |h|
       h[:container_project_id] = h.fetch_path(:project, :id)
     end
-    save_inventory_multi(ems.container_builds, hashes, deletes, [:ems_ref], [:labels_and_tags],
+    save_inventory_multi(ems.container_builds, hashes, deletes, [:ems_ref], [:labels, :tags],
                          [:project, :resources])
     store_ids_for_new_records(ems.container_builds, hashes, :ems_ref)
   end

app/models/manageiq/providers/kubernetes/container_manager/refresh_parser.rb

@@ -10,6 +10,7 @@ def self.ems_inv_to_hashes(inventory)
     def initialize
       @data = {}
       @data_index = {}
+      @label_tag_mapping = ContainerLabelTagMapping.cache
     end
 
     def ems_inv_to_hashes(inventory)
@@ -138,6 +139,10 @@ def process_collection_item(item, key)
       new_result
     end
 
+    def map_labels(model_name, labels)
+      ContainerLabelTagMapping.map_labels(@label_tag_mapping, model_name, labels)
+    end
+
     def find_host_by_provider_id(provider_id)
       scheme, instance_uri = provider_id.split("://", 2)
       prov, name_field = scheme_to_provider_mapping[scheme]
@@ -187,12 +192,14 @@ def cross_link_node(new_result)
     def parse_node(node)
       new_result = parse_base_item(node)
 
+      labels = parse_labels(node)
       new_result.merge!(
-        :type            => 'ManageIQ::Providers::Kubernetes::ContainerManager::ContainerNode',
-        :identity_infra  => node.spec.providerID,
-        :labels_and_tags => parse_labels(node),
-        :lives_on_id     => nil,
-        :lives_on_type   => nil
+        :type           => 'ManageIQ::Providers::Kubernetes::ContainerManager::ContainerNode',
+        :identity_infra => node.spec.providerID,
+        :labels         => labels,
+        :tags           => map_labels('ContainerNode', labels),
+        :lives_on_id    => nil,
+        :lives_on_type  => nil
       )
 
       node_info = node.status.try(:nodeInfo)
@@ -250,13 +257,14 @@ def parse_service(service)
         container_groups << cg unless cg.nil?
       end
 
+      labels = parse_labels(service)
       new_result.merge!(
         # TODO: We might want to change portal_ip to clusterIP
         :portal_ip        => service.spec.clusterIP,
         :session_affinity => service.spec.sessionAffinity,
         :service_type     => service.spec.type,
-
-        :labels_and_tags  => parse_labels(service),
+        :labels           => labels,
+        :tags             => map_labels('ContainerService', labels),
         :selector_parts   => parse_selector_parts(service),
         :container_groups => container_groups
       )
@@ -333,7 +341,8 @@ def parse_pod(pod)
 
       new_result[:container_conditions] = parse_conditions(pod)
 
-      new_result[:labels_and_tags] = parse_labels(pod)
+      new_result[:labels] = parse_labels(pod)
+      new_result[:tags] = map_labels('ContainerGroup', new_result[:labels])
       new_result[:node_selector_parts] = parse_node_selector_parts(pod)
       new_result[:container_volumes] = parse_volumes(pod)
       new_result
@@ -360,7 +369,8 @@ def parse_endpoint(entity)
 
     def parse_namespace(namespace)
       new_result = parse_base_item(namespace).except(:namespace)
-      new_result[:labels_and_tags] = parse_labels(namespace)
+      new_result[:labels] = parse_labels(namespace)
+      new_result[:tags] = map_labels('ContainerProject', new_result[:labels])
       new_result
     end
 
@@ -511,11 +521,13 @@ def create_limits_matrix
     def parse_replication_controllers(container_replicator)
       new_result = parse_base_item(container_replicator)
 
+      labels = parse_labels(container_replicator)
       # TODO: parse template
       new_result.merge!(
         :replicas         => container_replicator.spec.replicas,
         :current_replicas => container_replicator.status.replicas,
-        :labels_and_tags  => parse_labels(container_replicator),
+        :labels           => labels,
+        :tags             => map_labels('ContainerReplicator', labels),
         :selector_parts   => parse_selector_parts(container_replicator)
       )
 

app/models/manageiq/providers/openshift/container_manager/refresh_parser.rb

@@ -61,11 +61,13 @@ def get_service_name(route)
       def parse_route(route)
         new_result = parse_base_item(route)
 
+        labels = parse_labels(route)
         new_result.merge!(
           # TODO: persist tls
-          :host_name       => route.spec.try(:host),
-          :labels_and_tags => parse_labels(route),
-          :path            => route.path
+          :host_name => route.spec.try(:host),
+          :labels    => labels,
+          :tags      => map_labels('ContainerRoute', labels),
+          :path      => route.path
         )
 
         new_result[:project] = @data_index.fetch_path(:container_projects, :by_name,
@@ -89,8 +91,10 @@ def parse_build_source(source_item)
       def parse_build(build)
         new_result = parse_base_item(build)
         new_result.merge! parse_build_source(build.spec.source)
+        labels = parse_labels(build)
         new_result.merge!(
-          :labels_and_tags             => parse_labels(build),
+          :labels                      => labels,
+          :tags                        => map_labels('ContainerBuild', labels),
           :service_account             => build.spec.serviceAccount,
           :completion_deadline_seconds => build.spec.try(:completionDeadlineSeconds),
           :output_name                 => build.spec.try(:output).try(:to).try(:name)