Merge branch 'remove_skip_hashed_storage_upgrade_feature_flag' into 'master'

Remove `skip_hashed_storage_upgrade` feature flag

Closes #33730

See merge request gitlab-org/gitlab!29364

app/services/projects/after_rename_service.rb

@@ -126,8 +126,7 @@ module Projects
 
     def migrate_to_hashed_storage?
       Gitlab::CurrentSettings.hashed_storage_enabled? &&
-        project.storage_upgradable? &&
-        Feature.disabled?(:skip_hashed_storage_upgrade)
+        project.storage_upgradable?
     end
 
     def send_move_instructions?

changelogs/unreleased/remove_skip_hashed_storage_upgrade_feature_flag.yml

+---
+title: Remove skip_hased_storage_upgrade feature flag
+merge_request: 29364
+author: Lee Tickett
+type: other

spec/controllers/projects_controller_spec.rb

@@ -568,25 +568,42 @@ RSpec.describe ProjectsController do
       end
 
       context 'when only renaming a project path' do
-        it "sets the repository to the right path after a rename" do
+        it "doesnt change the disk_path when using hashed storage" do
+          skip unless project.hashed_storage?(:repository)
+
+          hashed_storage_path = ::Storage::Hashed.new(project).disk_path
           original_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
             project.repository.path
           end
 
-          expect { update_project path: 'renamed_path' }
-            .to change { project.reload.path }
+          expect { update_project path: 'renamed_path' }.to change { project.reload.path }
           expect(project.path).to include 'renamed_path'
 
           assign_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
             assigns(:repository).path
           end
 
-          if project.hashed_storage?(:repository)
-            expect(assign_repository_path).to eq(original_repository_path)
-          else
-            expect(assign_repository_path).to include(project.path)
+          expect(original_repository_path).to include(hashed_storage_path)
+          expect(assign_repository_path).to include(hashed_storage_path)
+        end
+
+        it "upgrades and move project to hashed storage when project was originally legacy" do
+          skip if project.hashed_storage?(:repository)
+
+          hashed_storage_path = Storage::Hashed.new(project).disk_path
+          original_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+            project.repository.path
+          end
+
+          expect { update_project path: 'renamed_path' }.to change { project.reload.path }
+          expect(project.path).to include 'renamed_path'
+
+          assign_repository_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+            assigns(:repository).path
           end
 
+          expect(original_repository_path).not_to include(hashed_storage_path)
+          expect(assign_repository_path).to include(hashed_storage_path)
           expect(response).to have_gitlab_http_status(:found)
         end
       end

spec/services/projects/after_rename_service_spec.rb

@@ -22,7 +22,6 @@ RSpec.describe Projects::AfterRenameService do
         # call. This makes testing a bit easier.
         allow(project).to receive(:gitlab_shell).and_return(gitlab_shell)
 
-        stub_feature_flags(skip_hashed_storage_upgrade: false)
         stub_application_setting(hashed_storage_enabled: false)
       end
 
@@ -151,7 +150,6 @@ RSpec.describe Projects::AfterRenameService do
         # call. This makes testing a bit easier.
         allow(project).to receive(:gitlab_shell).and_return(gitlab_shell)
 
-        stub_feature_flags(skip_hashed_storage_upgrade: false)
         stub_application_setting(hashed_storage_enabled: true)
       end
 

spec/services/projects/update_service_spec.rb

@@ -325,20 +325,9 @@ RSpec.describe Projects::UpdateService do
           expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk')
         end
 
-        it 'renames the project without upgrading it' do
-          result = update_project(project, admin, path: 'new-path')
-
-          expect(result).not_to include(status: :error)
-          expect(project).to be_valid
-          expect(project.errors).to be_empty
-          expect(project.disk_path).to include('new-path')
-          expect(project.reload.hashed_storage?(:repository)).to be_falsey
-        end
-
         context 'when hashed storage is enabled' do
           before do
             stub_application_setting(hashed_storage_enabled: true)
-            stub_feature_flags(skip_hashed_storage_upgrade: false)
           end
 
           it 'migrates project to a hashed storage instead of renaming the repo to another legacy name' do
@@ -349,22 +338,6 @@ RSpec.describe Projects::UpdateService do
             expect(project.errors).to be_empty
             expect(project.reload.hashed_storage?(:repository)).to be_truthy
           end
-
-          context 'when skip_hashed_storage_upgrade feature flag is enabled' do
-            before do
-              stub_feature_flags(skip_hashed_storage_upgrade: true)
-            end
-
-            it 'renames the project without upgrading it' do
-              result = update_project(project, admin, path: 'new-path')
-
-              expect(result).not_to include(status: :error)
-              expect(project).to be_valid
-              expect(project.errors).to be_empty
-              expect(project.disk_path).to include('new-path')
-              expect(project.reload.hashed_storage?(:repository)).to be_falsey
-            end
-          end
         end
       end