Always run CleanUp before writing the git configtc-backfill-full-path-config

3 files changed, 20 insertions, 17 deletions

diff --git a/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb b/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb
index 36eff3078aa..bbe75607351 100644
--- a/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb
+++ b/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config.rb
@@ -144,7 +144,6 @@ module Gitlab
 
             return unless project
 
-            project.cleanup_repository
             migration_class.new.safe_perform_one(project, retry_count)
           end
         end
@@ -178,6 +177,7 @@ module Gitlab
         end
 
         def perform_one(project)
+          project.cleanup_repository
           project.add_fullpath_config
         end
       end
@@ -192,6 +192,7 @@ module Gitlab
         end
 
         def perform_one(project)
+          project.cleanup_repository
           project.remove_fullpath_config
         end
       end
diff --git a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb
index b7c79027186..10cefe2d264 100644
--- a/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb
+++ b/spec/lib/gitlab/background_migration/backfill_project_fullpath_in_repo_config_spec.rb
@@ -46,10 +46,12 @@ describe Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig, :migr
         projects.create!(namespace_id: subgroup.id, name: 'buzz', path: 'buzz', storage_version: 1)
 
         expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service|
+          allow(repository_service).to receive(:cleanup)
           expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/baz')
         end
 
         expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service|
+          allow(repository_service).to receive(:cleanup)
           expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => 'foo/bar/buzz')
         end
 
@@ -65,8 +67,10 @@ describe Gitlab::BackgroundMigration::BackfillProjectFullpathInRepoConfig, :migr
       it 'asks the gitaly client to set config' do
         projects.create!(namespace_id: subgroup.id, name: 'baz', path: 'baz')
 
-        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService)
-          .to receive(:delete_config).with(['gitlab.fullpath'])
+        expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service|
+          allow(repository_service).to receive(:cleanup)
+          expect(repository_service).to receive(:delete_config).with(['gitlab.fullpath'])
+        end
 
         migrate
       end
diff --git a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb
index aa5b82e0e43..34f4a36d63d 100644
--- a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb
+++ b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb
@@ -20,21 +20,19 @@ describe BackfillStoreProjectFullPathInRepo, :migration do
 
   describe '#up' do
     shared_examples_for 'writes the full path to git config' do
-      let(:repository_service) { spy(:repository_service) }
-
-      def stub_repository_service
-        allow(Gitlab::GitalyClient::RepositoryService).to receive(:new).and_return(repository_service)
-      end
-
       it 'writes the git config' do
-        expect_any_instance_of(Gitlab::GitalyClient::RepositoryService)
-          .to receive(:set_config).with('gitlab.fullpath' => expected_path)
+        expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service|
+          allow(repository_service).to receive(:cleanup)
+          expect(repository_service).to receive(:set_config).with('gitlab.fullpath' => expected_path)
+        end
 
         migration.up
       end
 
       it 'retries in case of failure' do
-        stub_repository_service
+        repository_service = spy(:repository_service)
+
+        allow(Gitlab::GitalyClient::RepositoryService).to receive(:new).and_return(repository_service)
 
         allow(repository_service).to receive(:set_config).and_raise(GRPC::BadStatus, 'Retry me')
         expect(repository_service).to receive(:set_config).exactly(3).times
@@ -42,11 +40,11 @@ describe BackfillStoreProjectFullPathInRepo, :migration do
         migration.up
       end
 
-      it 'cleans up repository in case of failure' do
-        stub_repository_service
-
-        allow(repository_service).to receive(:set_config).and_raise(GRPC::BadStatus, 'Retry me')
-        expect(repository_service).to receive(:cleanup)
+      it 'cleans up repository before writing the config' do
+        expect_next_instance_of(Gitlab::GitalyClient::RepositoryService) do |repository_service|
+          expect(repository_service).to receive(:cleanup).ordered
+          expect(repository_service).to receive(:set_config).ordered
+        end
 
         migration.up
       end