Implement revet action for prometheus migration

db/post_migrate/20191220102807_patch_prometheus_services_for_shared_cluster_applications.rb

 # frozen_string_literal: true
 
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class PatchPrometheusServicesForSharedClusterApplications < ActiveRecord::Migration[5.2]
   include Gitlab::Database::MigrationHelpers
 
@@ -19,6 +16,7 @@ class PatchPrometheusServicesForSharedClusterApplications < ActiveRecord::Migrat
         self.table_name = 'clusters_applications_prometheus'
 
         enum status: {
+          errored: -1,
           installed: 3,
           updated: 5
         }
@@ -56,6 +54,25 @@ class PatchPrometheusServicesForSharedClusterApplications < ActiveRecord::Migrat
                    AND clusters_applications_prometheus.status IN (#{Applications::Prometheus.statuses[:installed]}, #{Applications::Prometheus.statuses[:updated]})").exists?
       end
     end
+
+    class PrometheusService < ActiveRecord::Base
+      self.inheritance_column = :_type_disabled
+      self.table_name = 'services'
+
+      default_scope { where("services.type = 'PrometheusService'") }
+
+      scope :managed, -> { where("services.properties = '{}'") }
+      scope :not_active, -> { where.not(active: true) }
+      scope :not_template, -> { where.not('services.template') }
+      scope :join_applications, -> {
+        joins('LEFT JOIN projects ON projects.id = services.project_id')
+          .joins('LEFT JOIN namespaces ON namespaces.id = projects.namespace_id')
+          .joins('LEFT JOIN cluster_groups ON cluster_groups.group_id = namespaces.id')
+          .joins("LEFT JOIN clusters ON clusters.cluster_type = #{Cluster.cluster_types['instance_type']} OR
+                            clusters.id = cluster_groups.cluster_id AND clusters.cluster_type = #{Cluster.cluster_types['group_type']}")
+          .joins('LEFT JOIN clusters_applications_prometheus ON clusters_applications_prometheus.cluster_id = clusters.id')
+      }
+    end
   end
 
   def up
@@ -67,7 +84,9 @@ class PatchPrometheusServicesForSharedClusterApplications < ActiveRecord::Migrat
   end
 
   def down
-    # no-op
+    Migratable::PrometheusService.managed.not_template.not_active.delete_all
+    Migratable::PrometheusService.managed.not_template.where(project_id: services_without_active_application.select('project_id')).delete_all
+    clear_duplicates
   end
 
   private
@@ -88,4 +107,20 @@ class PatchPrometheusServicesForSharedClusterApplications < ActiveRecord::Migrat
       @migrate_instance_cluster = Migratable::Cluster.instance_type.has_prometheus_application?
     end
   end
+
+  def services_without_active_application
+    Migratable::PrometheusService
+      .join_applications
+      .managed
+      .not_template
+      .group('project_id')
+      .having("NOT bool_or(COALESCE(clusters_applications_prometheus.status, #{Migratable::Applications::Prometheus.statuses[:errored]})
+                IN (#{Migratable::Applications::Prometheus.statuses[:installed]}, #{Migratable::Applications::Prometheus.statuses[:updated]}))")
+  end
+
+  def clear_duplicates
+    subquery = Migratable::PrometheusService.managed.not_template.select("id, ROW_NUMBER() OVER(PARTITION BY project_id ORDER BY project_id) AS row_num").to_sql
+    duplicates_filter = "id in (SELECT id FROM (#{subquery}) t WHERE t.row_num > 1)"
+    Migratable::PrometheusService.where(duplicates_filter).delete_all
+  end
 end

spec/migrations/patch_prometheus_services_for_shared_cluster_applications_spec.rb

@@ -13,18 +13,11 @@ describe PatchPrometheusServicesForSharedClusterApplications, :migration, :sidek
   let(:cluster_groups) { table(:cluster_groups) }
   let(:clusters_applications_prometheus) { table(:clusters_applications_prometheus) }
   let(:namespace) { namespaces.create!(name: 'gitlab', path: 'gitlab-org') }
-  let(:project_with_missing_service) { projects.create!(name: 'gitlab', path: 'gitlab-ce', namespace_id: namespace.id) }
-  let(:project_with_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
-  let(:project_with_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
-  let(:project_with_manual_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
-  let(:project_with_manual_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
-  let(:project_with_active_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
-  let(:project_with_inactive_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
 
   let(:application_statuses) do
     {
-      installed: 3,
       errored: -1,
+      installed: 3,
       updated: 5
     }
   end
@@ -36,6 +29,15 @@ describe PatchPrometheusServicesForSharedClusterApplications, :migration, :sidek
     }
   end
 
+  describe '#up' do
+    let!(:project_with_missing_service) { projects.create!(name: 'gitlab', path: 'gitlab-ce', namespace_id: namespace.id) }
+    let(:project_with_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
+    let(:project_with_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
+    let(:project_with_manual_active_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
+    let(:project_with_manual_inactive_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
+    let(:project_with_active_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
+    let(:project_with_inactive_not_prometheus_service) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
+
     before do
       services.create(service_params_for(project_with_inactive_service.id, active: false))
       services.create(service_params_for(project_with_active_service.id, active: true))
@@ -52,7 +54,8 @@ describe PatchPrometheusServicesForSharedClusterApplications, :migration, :sidek
 
           Sidekiq::Testing.fake! do
             Timecop.freeze do
-            background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
+              background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id],
+                                       ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
                                        ["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id],
                                        ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]]
 
@@ -71,7 +74,8 @@ describe PatchPrometheusServicesForSharedClusterApplications, :migration, :sidek
 
           Sidekiq::Testing.fake! do
             Timecop.freeze do
-            background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
+              background_migrations = [["ActivatePrometheusServicesForSharedClusterApplications", project_with_missing_service.id],
+                                       ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_service.id],
                                        ["ActivatePrometheusServicesForSharedClusterApplications", project_with_active_not_prometheus_service.id],
                                        ["ActivatePrometheusServicesForSharedClusterApplications", project_with_inactive_not_prometheus_service.id]]
 
@@ -126,4 +130,43 @@ describe PatchPrometheusServicesForSharedClusterApplications, :migration, :sidek
 
       it_behaves_like 'patch prometheus services post migration'
     end
+  end
+
+  describe '#down' do
+    subject(:migration) { described_class.new }
+
+    let(:group) { namespaces.create!(name: 'group', path: 'gitlab-org') }
+    let(:project) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: namespace.id) }
+    let(:project_with_group_application) { projects.create!(name: 'gitlab', path: 'gitlab-ee', namespace_id: group.id) }
+    let!(:active_service_without_application_on_group_cluster) { services.create(service_params_for(project.id, active: true, title: 'to remove')) }
+    let!(:duplicated_active_service) { services.create(service_params_for(project_with_group_application.id, active: true)) }
+    let!(:inactive_service) { services.create(service_params_for(project_with_group_application.id, active: false)) }
+    let!(:manual_inactive_service) { services.create(service_params_for(project.id, active: false, properties: '{\'some\':\'param\'}')) }
+    let!(:active_service_group_application) { services.create(service_params_for(project_with_group_application.id, active: true)) }
+
+    before do
+      group_cluster = clusters.create(name: 'cluster', cluster_type: cluster_types[:group_type])
+      cluster_groups.create(group_id: group.id, cluster_id: group_cluster.id)
+      clusters_applications_prometheus.create(cluster_id: group_cluster.id, status: application_statuses[:installed], version: '123')
+    end
+
+    context 'application on group cluster' do
+      it 'removes only redundant and inconsistent entries' do
+        migration.down
+        expect(services.all.map(&method(:row_attributes))).to match_array([manual_inactive_service, active_service_group_application].map(&method(:row_attributes)))
+      end
+    end
+
+    context 'application on instance cluster' do
+      it 'removes only redundant and inconsistent entries' do
+        instance_cluster = clusters.create(name: 'cluster', cluster_type: cluster_types[:instance_type])
+        clusters_applications_prometheus.create(cluster_id: instance_cluster.id, status: application_statuses[:updated], version: '123')
+
+        migration.down
+
+        expected_rows = [active_service_without_application_on_group_cluster, manual_inactive_service, active_service_group_application].map(&method(:row_attributes))
+        expect(services.all.map(&method(:row_attributes))).to match_array(expected_rows)
+      end
+    end
+  end
 end

spec/support/migrations_helpers/prometheus_service_helpers.rb

@@ -25,5 +25,11 @@ module MigrationHelpers
         deployment_events: false
       }.merge(params)
     end
+
+    def row_attributes(entity)
+      entity.attributes.with_indifferent_access.tap do |hash|
+        hash.merge!(hash.slice(:created_at, :updated_at).transform_values { |v| v.to_s(:db) })
+      end
+    end
   end
 end