Automatic merge of gitlab-org/gitlab master

changelogs/unreleased/229544-populate-issue-type-for-incident-issues.yml

+---
+title: Migrate all 'incident' labelled issues to have issue type 'incident'
+merge_request: 37668
+author:
+type: added

db/post_migrate/20200723040950_migrate_incident_issues_to_incident_type.rb

+# frozen_string_literal: true
+
+class MigrateIncidentIssuesToIncidentType < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+  BATCH_SIZE = 100
+
+  disable_ddl_transaction!
+
+  LABEL_PROPERTIES = {
+    title: 'incident'
+  }.freeze
+
+  class Issue < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'issues'
+
+    scope :incident_labelled, -> do
+      joins("INNER JOIN label_links ON label_links.target_type = 'Issue' AND label_links.target_id = issues.id")
+        .joins("INNER JOIN labels ON labels.id = label_links.label_id")
+        .where(labels: LABEL_PROPERTIES)
+    end
+
+    enum issue_type: {
+      issue: 0,
+      incident: 1
+    }
+
+    scope :incident_typed, -> { where(issue_type: :incident) }
+  end
+
+  def up
+    incident_issues = Issue.incident_labelled
+
+    incident_issues.each_batch(of: BATCH_SIZE) do |batch|
+      batch.update_all(issue_type: :incident)
+    end
+  end
+
+  def down
+    incident_issues = Issue.incident_typed
+
+    incident_issues.each_batch(of: BATCH_SIZE) do |batch|
+      batch.update_all(issue_type: :issue)
+    end
+  end
+end

db/schema_migrations/20200723040950

+085b3ad0f7da78a1e13f5cd3d2e7df297284a682c3bcd3883e487b18497430ff
\ No newline at end of file

ee/app/finders/geo/container_repository_registry_finder.rb

@@ -7,11 +7,11 @@ module Geo
     end
 
     def count_synced
-      registries.merge(Geo::ContainerRepositoryRegistry.synced).count
+      Geo::ContainerRepositoryRegistry.synced.count
     end
 
     def count_failed
-      registries.merge(Geo::ContainerRepositoryRegistry.failed).count
+      Geo::ContainerRepositoryRegistry.failed.count
     end
 
     def count_registry
@@ -63,43 +63,5 @@ module Geo
         .limit(batch_size)
     end
     # rubocop:enable CodeReuse/ActiveRecord
-
-    # Find limited amount of non replicated container repositories.
-    #
-    # You can pass a list with `except_ids:` so you can exclude items you
-    # already scheduled but haven't finished and aren't persisted to the database yet
-    #
-    # @param [Integer] batch_size used to limit the results returned
-    # @param [Array<Integer>] except_ids ids that will be ignored from the query
-    # rubocop:disable CodeReuse/ActiveRecord
-    def find_unsynced(batch_size:, except_ids: [])
-      current_node_fdw
-        .container_repositories
-        .missing_container_repository_registry
-        .id_not_in(except_ids)
-        .limit(batch_size)
-    end
-    # rubocop:enable CodeReuse/ActiveRecord
-
-    # rubocop:disable CodeReuse/ActiveRecord
-    def find_retryable_failed_ids(batch_size:, except_ids: [])
-      Geo::ContainerRepositoryRegistry
-        .failed
-        .retry_due
-        .model_id_not_in(except_ids)
-        .limit(batch_size)
-        .pluck_container_repository_key
-    end
-    # rubocop:enable CodeReuse/ActiveRecord
-
-    private
-
-    def registries
-      if Geo::ContainerRepositoryRegistry.registry_consistency_worker_enabled?
-        Geo::ContainerRepositoryRegistry.all
-      else
-        current_node_fdw.container_repositories.inner_join_container_repository_registry
-      end
-    end
   end
 end

ee/app/models/geo/container_repository_registry.rb

@@ -55,10 +55,6 @@ class Geo::ContainerRepositoryRegistry < Geo::BaseRegistry
     where(nil).pluck(:container_repository_id)
   end
 
-  def self.registry_consistency_worker_enabled?
-    Feature.enabled?(:geo_container_registry_ssot_sync, default_enabled: true)
-  end
-
   def self.replication_enabled?
     Gitlab.config.geo.registry_replication.enabled
   end

ee/app/models/geo/fdw/container_repository.rb

-# frozen_string_literal: true
-
-module Geo
-  module Fdw
-    class ContainerRepository < ::Geo::BaseFdw
-      self.table_name = Gitlab::Geo::Fdw.foreign_table_name('container_repositories')
-      self.primary_key = :id
-
-      belongs_to :project, class_name: 'Geo::Fdw::Project', inverse_of: :container_repositories
-
-      scope :project_id_in, ->(ids) { joins(:project).merge(Geo::Fdw::Project.id_in(ids)) }
-
-      class << self
-        def inner_join_container_repository_registry
-          join_statement =
-            arel_table
-              .join(container_repository_registry_table, Arel::Nodes::InnerJoin)
-              .on(arel_table[:id].eq(container_repository_registry_table[:container_repository_id]))
-
-          joins(join_statement.join_sources)
-        end
-
-        def missing_container_repository_registry
-          left_outer_join_container_repository_registry
-            .where(container_repository_registry_table[:id].eq(nil))
-        end
-
-        private
-
-        def container_repository_registry_table
-          Geo::ContainerRepositoryRegistry.arel_table
-        end
-
-        def left_outer_join_container_repository_registry
-          join_statement =
-            arel_table
-              .join(container_repository_registry_table, Arel::Nodes::OuterJoin)
-              .on(arel_table[:id].eq(container_repository_registry_table[:container_repository_id]))
-
-          joins(join_statement.join_sources)
-        end
-      end
-    end
-  end
-end

ee/app/models/geo/fdw/geo_node.rb

@@ -26,12 +26,6 @@ module Geo
         end
       end
 
-      def container_repositories
-        return Geo::Fdw::ContainerRepository.all unless selective_sync?
-
-        Geo::Fdw::ContainerRepository.project_id_in(projects)
-      end
-
       private
 
       def projects_for_selected_namespaces

ee/app/models/geo/fdw/project.rb

@@ -8,8 +8,6 @@ module Geo
       self.primary_key = :id
       self.table_name = Gitlab::Geo::Fdw.foreign_table_name('projects')
 
-      has_many :container_repositories, class_name: 'Geo::Fdw::ContainerRepository'
-
       belongs_to :namespace, class_name: 'Geo::Fdw::Namespace'
 
       alias_method :parent, :namespace

ee/app/workers/geo/container_repository_sync_dispatch_worker.rb

@@ -47,26 +47,15 @@ module Geo
     end
 
     def find_container_repository_ids_not_synced(batch_size:)
-      if Geo::ContainerRepositoryRegistry.registry_consistency_worker_enabled?
-        registry_finder
-          .find_never_synced_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
-          .pluck_model_foreign_key
-      else
-        registry_finder
-          .find_unsynced(batch_size: batch_size, except_ids: scheduled_repository_ids)
-          .pluck_primary_key
-      end
+      registry_finder
+        .find_never_synced_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
+        .pluck_model_foreign_key
     end
 
     def find_retryable_container_registry_ids(batch_size:)
-      if Geo::ContainerRepositoryRegistry.registry_consistency_worker_enabled?
-        registry_finder
-          .find_retryable_dirty_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
-          .pluck_model_foreign_key
-      else
-        registry_finder
-            .find_retryable_failed_ids(batch_size: batch_size, except_ids: scheduled_repository_ids)
-      end
+      registry_finder
+        .find_retryable_dirty_registries(batch_size: batch_size, except_ids: scheduled_repository_ids)
+        .pluck_model_foreign_key
     end
 
     def registry_finder

ee/spec/models/geo/fdw/container_repository_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Geo::Fdw::ContainerRepository, :geo, type: :model do
-  context 'relationships' do
-    it { is_expected.to belong_to(:project).class_name('Geo::Fdw::Project') }
-  end
-end

ee/spec/models/geo/fdw/project_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Geo::Fdw::Project, :geo_fdw, type: :model do
-  context 'relationships' do
-    it { is_expected.to have_many(:container_repositories).class_name('Geo::Fdw::ContainerRepository') }
-  end
-end

ee/spec/workers/geo/container_repository_sync_dispatch_worker_spec.rb

@@ -6,8 +6,8 @@ RSpec.describe Geo::ContainerRepositorySyncDispatchWorker, :geo, :use_sql_query_
   include ::EE::GeoHelpers
   include ExclusiveLeaseHelpers
 
-  let(:primary) { create(:geo_node, :primary) }
-  let(:secondary) { create(:geo_node) }
+  let_it_be(:primary) { create(:geo_node, :primary) }
+  let_it_be(:secondary) { create(:geo_node) }
 
   before do
     stub_current_geo_node(secondary)
@@ -49,149 +49,60 @@ RSpec.describe Geo::ContainerRepositorySyncDispatchWorker, :geo, :use_sql_query_
     expect(Geo::ContainerRepositorySyncWorker).not_to receive(:perform_async)
   end
 
-  context 'when geo_container_registry_ssot_sync is disabled', :geo_fdw do
-    before do
-      stub_feature_flags(geo_container_registry_ssot_sync: false)
-    end
-
-    it 'performs Geo::ContainerRepositorySyncWorker' do
-      container_repository = create(:container_repository)
-
-      expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async).with(container_repository.id)
-
-      subject.perform
-    end
-
-    it 'performs Geo::ContainerRepositorySyncWorker for failed syncs' do
-      registry = create(:container_repository_registry, :sync_failed)
-
-      expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async)
-        .with(registry.container_repository_id).once.and_return(spy)
-
-      subject.perform
-    end
-
-    it 'does not perform Geo::ContainerRepositorySyncWorker for synced repositories' do
-      create(:container_repository_registry, :synced)
-
-      expect(Geo::ContainerRepositorySyncWorker).not_to receive(:perform_async)
-
-      subject.perform
-    end
-
-    context 'with a failed sync' do
-      it 'does not stall backfill' do
-        failed_registry = create(:container_repository_registry, :sync_failed)
-        unsynced_container_repository = create(:container_repository)
-
-        stub_const('Geo::Scheduler::SchedulerWorker::DB_RETRIEVE_BATCH_SIZE', 1)
-
-        expect(Geo::ContainerRepositorySyncWorker).not_to receive(:perform_async).with(failed_registry.container_repository_id)
-        expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async).with(unsynced_container_repository.id)
-
-        subject.perform
-      end
-
-      it 'does not retry failed files when retry_at is tomorrow' do
-        failed_registry = create(:container_repository_registry, :sync_failed, retry_at: Date.tomorrow)
-
-        expect(Geo::ContainerRepositorySyncWorker)
-          .not_to receive(:perform_async).with( failed_registry.container_repository_id)
+  it 'performs Geo::ContainerRepositorySyncWorker' do
+    registry = create(:container_repository_registry)
 
-        subject.perform
-      end
+    expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async).with(registry.container_repository_id)
 
-      it 'retries failed files when retry_at is in the past' do
-        failed_registry = create(:container_repository_registry, :sync_failed, retry_at: Date.yesterday)
-
-        expect(Geo::ContainerRepositorySyncWorker)
-          .to receive(:perform_async).with(failed_registry.container_repository_id)
+    subject.perform
+  end
 
-        subject.perform
-      end
-    end
+  it 'performs Geo::ContainerRepositorySyncWorker for failed syncs' do
+    registry = create(:container_repository_registry, :sync_failed)
 
-    context 'when node has namespace restrictions', :request_store do
-      let(:synced_group) { create(:group) }
-      let(:project_in_synced_group) { create(:project, group: synced_group) }
-      let(:unsynced_project) { create(:project) }
+    expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async)
+      .with(registry.container_repository_id).once.and_return(spy)
 
-      before do
-        secondary.update!(selective_sync_type: 'namespaces', namespaces: [synced_group])
-      end
+    subject.perform
+  end
 
-      it 'does not perform Geo::ContainerRepositorySyncWorker for repositories that does not belong to selected namespaces' do
-        container_repository = create(:container_repository, project: project_in_synced_group)
-        create(:container_repository, project: unsynced_project)
+  it 'does not perform Geo::ContainerRepositorySyncWorker for synced repositories' do
+    create(:container_repository_registry, :synced)
 
-        expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async)
-          .with(container_repository.id).once.and_return(spy)
+    expect(Geo::ContainerRepositorySyncWorker).not_to receive(:perform_async)
 
-        subject.perform
-      end
-    end
+    subject.perform
   end
 
-  context 'when geo_container_registry_ssot_sync is enabled' do
-    before do
-      stub_feature_flags(geo_container_registry_ssot_sync: true)
-    end
+  context 'with a failed sync' do
+    it 'does not stall backfill' do
+      failed_registry = create(:container_repository_registry, :sync_failed)
+      unsynced_registry = create(:container_repository_registry)
 
-    it 'performs Geo::ContainerRepositorySyncWorker' do
-      registry = create(:container_repository_registry)
+      stub_const('Geo::Scheduler::SchedulerWorker::DB_RETRIEVE_BATCH_SIZE', 1)
 
-      expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async).with(registry.container_repository_id)
+      expect(Geo::ContainerRepositorySyncWorker).not_to receive(:perform_async).with(failed_registry.container_repository_id)
+      expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async).with(unsynced_registry.container_repository_id)
 
       subject.perform
     end
 
-    it 'performs Geo::ContainerRepositorySyncWorker for failed syncs' do
-      registry = create(:container_repository_registry, :sync_failed)
+    it 'does not retry failed files when retry_at is tomorrow' do
+      failed_registry = create(:container_repository_registry, :sync_failed, retry_at: Date.tomorrow)
 
-      expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async)
-        .with(registry.container_repository_id).once.and_return(spy)
+      expect(Geo::ContainerRepositorySyncWorker)
+        .not_to receive(:perform_async).with( failed_registry.container_repository_id)
 
       subject.perform
     end
 
-    it 'does not perform Geo::ContainerRepositorySyncWorker for synced repositories' do
-      create(:container_repository_registry, :synced)
+    it 'retries failed files when retry_at is in the past' do
+      failed_registry = create(:container_repository_registry, :sync_failed, retry_at: Date.yesterday)
 
-      expect(Geo::ContainerRepositorySyncWorker).not_to receive(:perform_async)
+      expect(Geo::ContainerRepositorySyncWorker)
+        .to receive(:perform_async).with(failed_registry.container_repository_id)
 
       subject.perform
     end
-
-    context 'with a failed sync' do
-      it 'does not stall backfill' do
-        failed_registry = create(:container_repository_registry, :sync_failed)
-        unsynced_registry = create(:container_repository_registry)
-
-        stub_const('Geo::Scheduler::SchedulerWorker::DB_RETRIEVE_BATCH_SIZE', 1)
-
-        expect(Geo::ContainerRepositorySyncWorker).not_to receive(:perform_async).with(failed_registry.container_repository_id)
-        expect(Geo::ContainerRepositorySyncWorker).to receive(:perform_async).with(unsynced_registry.container_repository_id)
-
-        subject.perform
-      end
-
-      it 'does not retry failed files when retry_at is tomorrow' do
-        failed_registry = create(:container_repository_registry, :sync_failed, retry_at: Date.tomorrow)
-
-        expect(Geo::ContainerRepositorySyncWorker)
-          .not_to receive(:perform_async).with( failed_registry.container_repository_id)
-
-        subject.perform
-      end
-
-      it 'retries failed files when retry_at is in the past' do
-        failed_registry = create(:container_repository_registry, :sync_failed, retry_at: Date.yesterday)
-
-        expect(Geo::ContainerRepositorySyncWorker)
-          .to receive(:perform_async).with(failed_registry.container_repository_id)
-
-        subject.perform
-      end
-    end
   end
 end

ee/spec/workers/geo/secondary/registry_consistency_worker_spec.rb

@@ -108,30 +108,6 @@ RSpec.describe Geo::Secondary::RegistryConsistencyWorker, :geo do
       expect(Geo::ContainerRepositoryRegistry.where(container_repository_id: container_repository.id).count).to eq(1)
     end
 
-    context 'when geo_container_registry_ssot_sync is disabled' do
-      before do
-        stub_feature_flags(geo_container_registry_ssot_sync: false)
-      end
-
-      it 'returns false' do
-        expect(subject.perform).to be_falsey
-      end
-
-      it 'does not execute RegistryConsistencyService for container repositories' do
-        allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::JobArtifactRegistry, batch_size: batch_size).and_call_original
-        allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::LfsObjectRegistry, batch_size: batch_size).and_call_original
-        allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::PackageFileRegistry, batch_size: batch_size).and_call_original
-        allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::UploadRegistry, batch_size: batch_size).and_call_original
-        allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::ProjectRegistry, batch_size: batch_size).and_call_original
-        allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::DesignRegistry, batch_size: batch_size).and_call_original
-        allow(Geo::RegistryConsistencyService).to receive(:new).with(Geo::VulnerabilityExportRegistry, batch_size: batch_size).and_call_original
-
-        expect(Geo::RegistryConsistencyService).not_to receive(:new).with(Geo::ContainerRepositoryRegistry, batch_size: batch_size)
-
-        subject.perform
-      end
-    end
-
     context 'when the current Geo node is disabled or primary' do
       before do
         stub_primary_node

spec/migrations/migrate_incident_issues_to_incident_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200723040950_migrate_incident_issues_to_incident_type.rb')
+
+RSpec.describe MigrateIncidentIssuesToIncidentType do
+  let(:migration) { described_class.new }
+
+  let(:projects) { table(:projects) }
+  let(:namespaces) { table(:namespaces) }
+  let(:labels) { table(:labels) }
+  let(:issues) { table(:issues) }
+  let(:label_links) { table(:label_links) }
+  let(:label_props) { IncidentManagement::CreateIncidentLabelService::LABEL_PROPERTIES }
+
+  let(:namespace) { namespaces.create!(name: 'foo', path: 'foo') }
+  let!(:project) { projects.create!(namespace_id: namespace.id) }
+  let(:label) { labels.create!(project_id: project.id, **label_props) }
+  let!(:incident_issue) { issues.create!(project_id: project.id) }
+  let!(:other_issue) { issues.create!(project_id: project.id) }
+
+  # Issue issue_type enum
+  let(:issue_type) { 0 }
+  let(:incident_type) { 1 }
+
+  before do
+    label_links.create!(target_id: incident_issue.id, label_id: label.id, target_type: 'Issue')
+  end
+
+  describe '#up' do
+    it 'updates the incident issue type' do
+      expect { migrate! }
+        .to change { incident_issue.reload.issue_type }
+        .from(issue_type)
+        .to(incident_type)
+
+      expect(other_issue.reload.issue_type).to eql(issue_type)
+    end
+  end
+
+  describe '#down' do
+    let!(:incident_issue) { issues.create!(project_id: project.id, issue_type: issue_type) }
+
+    it 'updates the incident issue type' do
+      migration.up
+
+      expect { migration.down }
+        .to change { incident_issue.reload.issue_type }
+        .from(incident_type)
+        .to(issue_type)
+
+      expect(other_issue.reload.issue_type).to eql(issue_type)
+    end
+  end
+end