Merge branch '357670-statuses-coherence-checking-when-reconciling-them' into 'master'

Enqueuer: update #reconcile_import_status to fix status coherence

See merge request gitlab-org/gitlab!84289

app/models/container_repository.rb

@@ -16,8 +16,6 @@ class ContainerRepository < ApplicationRecord
   ABORTABLE_MIGRATION_STATES = (ACTIVE_MIGRATION_STATES + %w[pre_import_done default]).freeze
   SKIPPABLE_MIGRATION_STATES = (ABORTABLE_MIGRATION_STATES + %w[import_aborted]).freeze
 
-  IRRECONCILABLE_MIGRATIONS_STATUSES = %w[import_in_progress pre_import_in_progress pre_import_canceled import_canceled].freeze
-
   MIGRATION_PHASE_1_STARTED_AT = Date.new(2021, 11, 4).freeze
 
   TooManyImportsError = Class.new(StandardError)
@@ -113,7 +111,7 @@ class ContainerRepository < ApplicationRecord
     end
 
     event :start_pre_import do
-      transition default: :pre_importing
+      transition %i[default pre_importing importing import_aborted] => :pre_importing
     end
 
     event :finish_pre_import do
@@ -153,7 +151,10 @@ class ContainerRepository < ApplicationRecord
       container_repository.migration_pre_import_done_at = nil
     end
 
-    after_transition any => :pre_importing do |container_repository|
+    after_transition any => :pre_importing do |container_repository, transition|
+      forced = transition.args.first.try(:[], :forced)
+      next if forced
+
       container_repository.try_import do
         container_repository.migration_pre_import
       end
@@ -168,7 +169,10 @@ class ContainerRepository < ApplicationRecord
       container_repository.migration_import_done_at = nil
     end
 
-    after_transition any => :importing do |container_repository|
+    after_transition any => :importing do |container_repository, transition|
+      forced = transition.args.first.try(:[], :forced)
+      next if forced
+
       container_repository.try_import do
         container_repository.migration_import
       end
@@ -259,10 +263,10 @@ class ContainerRepository < ApplicationRecord
     super
   end
 
-  def start_pre_import
+  def start_pre_import(*args)
     return false unless ContainerRegistry::Migration.enabled?
 
-    super
+    super(*args)
   end
 
   def retry_pre_import
@@ -295,8 +299,18 @@ class ContainerRepository < ApplicationRecord
     case status
     when 'native'
       finish_import_as(:native_import)
-    when *IRRECONCILABLE_MIGRATIONS_STATUSES
-      nil
+    when 'pre_import_in_progress'
+      return if pre_importing?
+
+      start_pre_import(forced: true)
+    when 'import_in_progress'
+      return if importing?
+
+      start_import(forced: true)
+    when 'import_canceled', 'pre_import_canceled'
+      return if import_skipped?
+
+      skip_import(reason: :migration_canceled)
     when 'import_complete'
       finish_import
     when 'import_failed'

spec/models/container_repository_spec.rb

@@ -224,7 +224,7 @@ RSpec.describe ContainerRepository, :aggregate_failures do
         end
       end
 
-      it_behaves_like 'transitioning from allowed states', %w[default]
+      it_behaves_like 'transitioning from allowed states', %w[default pre_importing importing import_aborted]
       it_behaves_like 'transitioning to pre_importing'
     end
 

spec/support/shared_examples/requests/api/container_repositories_shared_examples.rb

@@ -118,11 +118,14 @@ RSpec.shared_examples 'not hitting graphql network errors with the container reg
 end
 
 RSpec.shared_examples 'reconciling migration_state' do
-  shared_examples 'no action' do
-    it 'does nothing' do
-      expect { subject }.not_to change { repository.reload.migration_state }
+  shared_examples 'enforcing states coherence to' do |expected_migration_state|
+    it 'leaves the repository in the expected migration_state' do
+      expect(repository.gitlab_api_client).not_to receive(:pre_import_repository)
+      expect(repository.gitlab_api_client).not_to receive(:import_repository)
 
-      expect(subject).to eq(nil)
+      subject
+
+      expect(repository.reload.migration_state).to eq(expected_migration_state)
     end
   end
 
@@ -155,7 +158,7 @@ RSpec.shared_examples 'reconciling migration_state' do
   context 'import_in_progress response' do
     let(:status) { 'import_in_progress' }
 
-    it_behaves_like 'no action'
+    it_behaves_like 'enforcing states coherence to', 'importing'
   end
 
   context 'import_complete response' do
@@ -175,7 +178,7 @@ RSpec.shared_examples 'reconciling migration_state' do
   context 'pre_import_in_progress response' do
     let(:status) { 'pre_import_in_progress' }
 
-    it_behaves_like 'no action'
+    it_behaves_like 'enforcing states coherence to', 'pre_importing'
   end
 
   context 'pre_import_complete response' do
@@ -194,4 +197,12 @@ RSpec.shared_examples 'reconciling migration_state' do
 
     it_behaves_like 'retrying the pre_import'
   end
+
+  %w[pre_import_canceled import_canceled].each do |canceled_status|
+    context "#{canceled_status} response" do
+      let(:status) { canceled_status }
+
+      it_behaves_like 'enforcing states coherence to', 'import_skipped'
+    end
+  end
 end