[WIP] Add unique indexes for containers tables

app/models/container.rb

@@ -47,8 +47,6 @@ def disconnect_inv
     return if ems_id.nil?
     _log.info "Disconnecting Container [#{name}] id [#{id}] from EMS "
     self.deleted_on = Time.now.utc
-    self.old_ems_id = self.ems_id
-    self.ems_id = nil
     save
   end
 end

app/models/container_definition.rb

@@ -15,8 +15,6 @@ def disconnect_inv
     _log.info "Disconnecting Container definition [#{name}] id [#{id}]"
     self.container.try(:disconnect_inv)
     self.deleted_on = Time.now.utc
-    self.old_ems_id = self.ems_id
-    self.ems_id = nil
     save
   end
 end

app/models/container_group.rb

@@ -85,8 +85,6 @@ def disconnect_inv
     _log.info "Disconnecting Pod [#{name}] id [#{id}] from EMS [#{ext_management_system.name}]" \
     "id [#{ext_management_system.id}] "
     self.container_definitions.each(&:disconnect_inv)
-    self.old_ems_id = ems_id
-    self.ext_management_system = nil
     self.container_node_id = nil
     self.container_services = []
     self.container_replicator_id = nil

app/models/container_image.rb

@@ -106,8 +106,6 @@ def disconnect_inv
     _log.info "Disconnecting Image [#{name}] id [#{id}] from EMS [#{ext_management_system.name}]" \
     "id [#{ext_management_system.id}] "
     self.container_image_registry = nil
-    self.old_ems_id = ems_id
-    self.ext_management_system = nil
     self.deleted_on = Time.now.utc
     save
   end

app/models/container_project.rb

@@ -64,8 +64,6 @@ def disconnect_inv
     return if ems_id.nil?
     _log.info "Disconnecting Container Project [#{name}] id [#{id}] from EMS [#{ext_management_system.name}]" \
     "id [#{ext_management_system.id}] "
-    self.old_ems_id = ems_id
-    self.ext_management_system = nil
     self.deleted_on = Time.now.utc
     save
   end

app/models/manageiq/providers/container_manager.rb

@@ -4,15 +4,16 @@ class ContainerManager < BaseManager
     include SupportsFeatureMixin
 
     has_many :container_nodes, :foreign_key => :ems_id, :dependent => :destroy
-    has_many :container_groups, :foreign_key => :ems_id, :dependent => :destroy
+    has_many :container_groups, -> { active }, :foreign_key => :ems_id
     has_many :container_services, :foreign_key => :ems_id, :dependent => :destroy
     has_many :container_replicators, :foreign_key => :ems_id, :dependent => :destroy
-    has_many :containers, :foreign_key => :ems_id
-    has_many :container_projects, :foreign_key => :ems_id, :dependent => :destroy
+    has_many :containers, -> { active }, :foreign_key => :ems_id
+    has_many :container_definitions, -> { active }, :foreign_key => :ems_id
+    has_many :container_projects, -> { active }, :foreign_key => :ems_id
     has_many :container_quotas, :foreign_key => :ems_id, :dependent => :destroy
     has_many :container_limits, :foreign_key => :ems_id, :dependent => :destroy
     has_many :container_image_registries, :foreign_key => :ems_id, :dependent => :destroy
-    has_many :container_images, :foreign_key => :ems_id, :dependent => :destroy
+    has_many :container_images, -> { active }, :foreign_key => :ems_id
     has_many :persistent_volumes, :as => :parent, :dependent => :destroy
     has_many :persistent_volume_claims, :foreign_key => :ems_id, :dependent => :destroy
     has_many :container_component_statuses, :foreign_key => :ems_id, :dependent => :destroy
@@ -26,7 +27,6 @@ class ContainerManager < BaseManager
     has_many :computer_system_hardwares, :through => :computer_systems, :source => :hardware
     has_many :computer_system_operating_systems, :through => :computer_systems, :source => :operating_system
     has_many :container_volumes, :through => :container_groups
-    has_many :container_definitions, :through => :container_groups
     has_many :container_port_configs, :through => :container_definitions
     has_many :container_env_vars, :through => :container_definitions
     has_many :security_contexts, :through => :container_definitions
@@ -36,10 +36,13 @@ class ContainerManager < BaseManager
     has_many :container_limit_items, :through => :container_limits
     has_many :container_template_parameters, :through => :container_templates
 
-    # Archived entities to destroy when the container manager is deleted
-    has_many :old_container_groups, :foreign_key => :old_ems_id, :dependent => :destroy, :class_name => "ContainerGroup"
-    has_many :old_container_projects, :foreign_key => :old_ems_id, :dependent => :destroy, :class_name => "ContainerProject"
-    has_many :old_container_images, :foreign_key => :old_ems_id, :dependent => :destroy, :class_name => "ContainerImage"
+    # Archived and active entities to destroy when the container manager is deleted
+    has_many :all_containers, :foreign_key => :ems_id, :dependent => :destroy, :class_name => "Container"
+    has_many :all_container_groups, :foreign_key => :ems_id, :dependent => :destroy, :class_name => "ContainerGroup"
+    has_many :all_container_projects, :foreign_key => :ems_id, :dependent => :destroy, :class_name => "ContainerProject"
+    has_many :all_container_images, :foreign_key => :ems_id, :dependent => :destroy, :class_name => "ContainerImage"
+    has_many :all_container_definitions, :foreign_key => :ems_id, :dependent => :destroy, :class_name => "ContainerDefinition"
+
 
     virtual_column :port_show, :type => :string
 

app/models/mixins/archived_mixin.rb

@@ -2,11 +2,18 @@ module ArchivedMixin
   extend ActiveSupport::Concern
 
   included do
+    scope :archived, -> { where.not(:deleted_on => nil) }
+    scope :active, -> { where(:deleted_on => nil) }
+
     belongs_to :old_ext_management_system, :foreign_key => :old_ems_id, :class_name => 'ExtManagementSystem'
   end
 
   def archived?
-    ems_id.nil?
+    !active?
+  end
+
+  def active?
+    deleted_on.nil?
   end
 
   # Needed for metrics

db/migrate/20170530102506_add_deleted_on_indexes_to_containers_tables.rb

@@ -0,0 +1,14 @@
+class AddDeletedOnIndexesToContainersTables < ActiveRecord::Migration[5.0]
+  def change
+    add_index :container_definitions, :deleted_on,
+              :name  => "index_container_definitions_on_deleted_on"
+    add_index :container_groups, :deleted_on,
+              :name  => "container_groups_on_deleted_on"
+    add_index :container_images, :deleted_on,
+              :name  => "index_container_images_on_deleted_on"
+    add_index :container_projects, :deleted_on,
+              :name  => "index_container_projects_on_deleted_on"
+    add_index :containers, :deleted_on,
+              :name  => "index_containers_on_deleted_on"
+  end
+end

db/migrate/20170530102536_use_deleted_on_in_containers_tables.rb

@@ -0,0 +1,68 @@
+class UseDeletedOnInContainersTables < ActiveRecord::Migration[5.0]
+  class ContainerDefinition < ActiveRecord::Base; end
+
+  class ContainerGroup < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+  end
+
+  class ContainerImage < ActiveRecord::Base; end
+
+  class ContainerProject < ActiveRecord::Base; end
+
+  class Container < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+  end
+
+  def disconnect_to_soft_delete(model)
+    model.where(:ems_id => nil).where(:deleted_on => nil).update_all(:deleted_on => Time.now.utc)
+    model.where(:ems_id => nil).update_all("ems_id = old_ems_id")
+  end
+
+  def soft_delete_to_disconnect(model)
+    model.where.not(:deleted_on => nil).update_all(:ems_id => nil)
+  end
+
+  def up
+    say_with_time("Changes :ems_id => nil to :deleted_on => Time.now and set :ems_id using :old_ems_id for ContainerDefinition") do
+      disconnect_to_soft_delete(ContainerDefinition)
+    end
+
+    say_with_time("Changes :ems_id => nil to :deleted_on => Time.now and set :ems_id using :old_ems_id for ContainerGroup") do
+      disconnect_to_soft_delete(ContainerGroup)
+    end
+
+    say_with_time("Changes :ems_id => nil to :deleted_on => Time.now and set :ems_id using :old_ems_id for ContainerImages") do
+      disconnect_to_soft_delete(ContainerImage)
+    end
+
+    say_with_time("Changes :ems_id => nil to :deleted_on => Time.now and set :ems_id using :old_ems_id for ContainerProject") do
+      disconnect_to_soft_delete(ContainerProject)
+    end
+
+    say_with_time("Changes :ems_id => nil to :deleted_on => Time.now and set :ems_id using :old_ems_id for Container") do
+      disconnect_to_soft_delete(Container)
+    end
+  end
+
+  def down
+    say_with_time("Changes :deleted_on => Time.now to :ems_id => nil for ContainerDefinition") do
+      soft_delete_to_disconnect(ContainerDefinition)
+    end
+
+    say_with_time("Changes :deleted_on => Time.now to :ems_id => nil for ContainerGroup") do
+      soft_delete_to_disconnect(ContainerGroup)
+    end
+
+    say_with_time("Changes :deleted_on => Time.now to :ems_id => nil for ContainerImages") do
+      soft_delete_to_disconnect(ContainerImage)
+    end
+
+    say_with_time("Changes :deleted_on => Time.now to :ems_id => nil for ContainerProject") do
+      soft_delete_to_disconnect(ContainerProject)
+    end
+
+    say_with_time("Changes :deleted_on => Time.now to :ems_id => nil for Container") do
+      soft_delete_to_disconnect(Container)
+    end
+  end
+end

db/migrate/20170530102630_clean_up_duplicates_in_containers_tables.rb

@@ -0,0 +1,102 @@
+class CleanUpDuplicatesInContainersTables < ActiveRecord::Migration[5.0]
+  class ContainerBuild < ActiveRecord::Base; end
+  class ContainerBuildPod < ActiveRecord::Base; end
+  class ContainerGroup < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+  end
+  class ContainerLimit < ActiveRecord::Base; end
+  class ContainerNode < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+  end
+  class ContainerProject < ActiveRecord::Base; end
+  class ContainerQuota < ActiveRecord::Base; end
+  class ContainerReplicator < ActiveRecord::Base; end
+  class ContainerRoute < ActiveRecord::Base; end
+  class ContainerService < ActiveRecord::Base; end
+  class ContainerTemplate < ActiveRecord::Base; end
+  class PersistentVolumeClaim < ActiveRecord::Base; end
+
+  class ContainerComponentStatus < ActiveRecord::Base; end
+  class ContainerImage < ActiveRecord::Base; end
+  class ContainerImageRegistry < ActiveRecord::Base; end
+
+  class ContainerCondition < ActiveRecord::Base; end
+  class SecurityContext < ActiveRecord::Base; end
+  class ContainerEnvVar < ActiveRecord::Base; end
+  class ContainerLimitItem < ActiveRecord::Base; end
+  class ContainerPortConfig < ActiveRecord::Base; end
+  class ContainerQuotaItem < ActiveRecord::Base; end
+  class ContainerServicePortConfig < ActiveRecord::Base; end
+  class ContainerTemplateParameter < ActiveRecord::Base; end
+  class ContainerVolume < ActiveRecord::Base; end
+  class CustomAttribute < ActiveRecord::Base; end
+
+  class ContainerDefinition < ActiveRecord::Base; end
+  class Container < ActiveRecord::Base
+    self.inheritance_column = :_type_disabled
+  end
+
+  def duplicate_data_query_returning_batches(model, unique_index_columns)
+    model.group(unique_index_columns)
+         .select("#{unique_index_columns.join(", ")}, min(id) AS first_duplicate, array_agg(id) AS duplicate_ids")
+         .having("COUNT(id) > 1")
+  end
+
+  def cleanup_duplicate_data_batch(model, unique_index_columns)
+    duplicate_data_query_returning_batches(model, unique_index_columns).each do |duplicate|
+      # TODO(lsmola) do I need to do some merging of occurrences, e.g. reconnecting metrics, events, etc.?
+      # TODO(lsmola) calling .destroy so it cascade deletes will be expensive
+      model.where(:id => duplicate.duplicate_ids[1..--1]).delete_all
+    end
+  end
+
+  def duplicate_data_query_returning_min_id(model, unique_index_columns)
+    model.group(unique_index_columns).select("min(id)")
+  end
+
+  def cleanup_duplicate_data_delete_all(model, unique_index_columns)
+    model.where.not(:id => duplicate_data_query_returning_min_id(model, unique_index_columns)).delete_all
+  end
+
+  UNIQUE_INDEXES_FOR_MODELS = {
+    # Just having :ems_id & :ems_ref
+    ContainerBuild             => [:ems_id, :ems_ref],
+    ContainerBuildPod          => [:ems_id, :ems_ref],
+    ContainerGroup             => [:ems_id, :ems_ref],
+    ContainerLimit             => [:ems_id, :ems_ref],
+    ContainerNode              => [:ems_id, :ems_ref],
+    ContainerProject           => [:ems_id, :ems_ref],
+    ContainerQuota             => [:ems_id, :ems_ref],
+    ContainerReplicator        => [:ems_id, :ems_ref],
+    ContainerRoute             => [:ems_id, :ems_ref],
+    ContainerService           => [:ems_id, :ems_ref],
+    ContainerTemplate          => [:ems_id, :ems_ref],
+    PersistentVolumeClaim      => [:ems_id, :ems_ref],
+    # Having :ems_id but not ems_ref
+    ContainerComponentStatus   => [:ems_id, :name],
+    ContainerImage             => [:ems_id, :image_ref, :container_image_registry_id],
+    ContainerImageRegistry     => [:ems_id, :host, :port],
+    # Nested tables, not having :ems_id and the foreign_key is a part of the unique index
+    ContainerCondition         => [:container_entity_id, :container_entity_type, :name],
+    SecurityContext            => [:resource_id, :resource_type],
+    ContainerEnvVar            => [:container_definition_id, :name, :value, :field_path],
+    ContainerLimitItem         => [:container_limit_id, :resource, :item_type],
+    ContainerPortConfig        => [:container_definition_id, :ems_ref],
+    ContainerQuotaItem         => [:container_quota_id, :resource],
+    ContainerServicePortConfig => [:container_service_id, :ems_ref, :protocol],
+    ContainerTemplateParameter => [:container_template_id, :name],
+    ContainerVolume            => [:parent_id, :parent_type, :name],
+    CustomAttribute            => [:resource_id, :resource_type, :name, :unique_name, :section, :source],
+    # Questionable
+    ContainerDefinition        => [:ems_id, :ems_ref],
+    Container                  => [:ems_id, :ems_ref]
+  }.freeze
+
+  def up
+    UNIQUE_INDEXES_FOR_MODELS.each do |model, unique_indexes_columns|
+      say_with_time("Cleanup duplicate data for model #{model}") do
+        cleanup_duplicate_data_delete_all(model, unique_indexes_columns)
+      end
+    end
+  end
+end

db/migrate/20170530102659_add_unique_indexes_to_containers_tables.rb

@@ -0,0 +1,75 @@
+class AddUniqueIndexesToContainersTables < ActiveRecord::Migration[5.0]
+  def change
+    # Just having :ems_id & :ems_ref
+    add_index :container_builds,         [:ems_id, :ems_ref], :unique => true
+    add_index :container_build_pods,     [:ems_id, :ems_ref], :unique => true
+    add_index :container_groups,         [:ems_id, :ems_ref], :unique => true
+    add_index :container_limits,         [:ems_id, :ems_ref], :unique => true
+    add_index :container_nodes,          [:ems_id, :ems_ref], :unique => true
+    add_index :container_projects,       [:ems_id, :ems_ref], :unique => true
+    add_index :container_quotas,         [:ems_id, :ems_ref], :unique => true
+    add_index :container_replicators,    [:ems_id, :ems_ref], :unique => true
+    add_index :container_routes,         [:ems_id, :ems_ref], :unique => true
+    add_index :container_services,       [:ems_id, :ems_ref], :unique => true
+    add_index :container_templates,      [:ems_id, :ems_ref], :unique => true
+    add_index :persistent_volume_claims, [:ems_id, :ems_ref], :unique => true
+
+    # Having :ems_id but not ems_ref
+    add_index :container_component_statuses,
+              [:ems_id, :name],
+              :unique => true
+    add_index :container_images,
+              [:ems_id, :image_ref, :container_image_registry_id],
+              :unique => true,
+              :name   => "index_container_images_unique_multi_column"
+    add_index :container_image_registries,
+              [:ems_id, :host, :port],
+              :unique => true
+
+    # Nested tables, not having :ems_id and the foreign_key is a part of the unique index
+    add_index :container_conditions,
+              [:container_entity_id, :container_entity_type, :name],
+              :unique => true,
+              :name   => "index_container_conditions_unique_multi_column"
+    add_index :security_contexts,
+              [:resource_id, :resource_type],
+              :unique => true,
+              :name   => "index_security_contexts_unique_multi_column"
+    add_index :container_env_vars,
+              [:container_definition_id, :name, :value, :field_path],
+              :unique => true,
+              :name   => "index_container_env_vars_unique_multi_column"
+    add_index :container_limit_items,
+              [:container_limit_id, :resource, :item_type],
+              :unique => true,
+              :name   => "index_container_limit_items_unique_multi_column"
+    add_index :container_port_configs,
+              [:container_definition_id, :ems_ref],
+              :unique => true,
+              :name   => "index_container_port_configs_unique_multi_column"
+    add_index :container_quota_items,
+              [:container_quota_id, :resource],
+              :unique => true
+    add_index :container_service_port_configs,
+              [:container_service_id, :ems_ref, :protocol], # FIXME(lsmola) I see duplicate ems_refs, because protocol is not part of it
+              :unique => true,
+              :name   => "index_container_service_port_configs_unique_multi_column"
+    add_index :container_template_parameters,
+              [:container_template_id, :name],
+              :unique => true,
+              :name   => "index_container_template_parameters_unique_multi_column"
+    add_index :container_volumes,
+              [:parent_id, :parent_type, :name],
+              :unique => true # FIXME(lsmola) has unused :ems_ref
+    add_index :custom_attributes,
+              [:resource_id, :resource_type, :name, :unique_name, :section, :source],
+              :unique => true,
+              :name   => "index_custom_attributes_parameters_unique_multi_column"
+
+    # FIXME(lsmola) questionable, these were modeled as nested, but they have :ems_id & :ems_ref
+    # Is ems_ref unique? we were saving these under container_group
+    add_index :container_definitions, [:ems_id, :ems_ref], :unique => true
+    # Is ems_ref unique? we were saving these under container_definition
+    add_index :containers,            [:ems_id, :ems_ref], :unique => true
+  end
+end