diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index fa3999e2a82208282bf6ea5d03d12385b494903c..77f935d82a7ef3553e620ab1d92d75f6ecd09561 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -818f3d85a2c8e6596376f1d2276aa22660203a6c +d513d220b183d83ae7219ec52f49aa3b4f7fc551 diff --git a/Gemfile b/Gemfile index 2e2227d98753c48df07aad0ca42cf770daadf09f..e4e68837111fbca78e07e9d887e18c4b0c7d82d7 100644 --- a/Gemfile +++ b/Gemfile @@ -472,7 +472,7 @@ end gem 'spamcheck', '~> 0.1.0' # Gitaly GRPC protocol definitions -gem 'gitaly', '~> 14.1.0.pre.rc4' +gem 'gitaly', '~> 14.2.0.pre.rc2' # KAS GRPC protocol definitions gem 'kas-grpc', '~> 0.0.2' diff --git a/Gemfile.lock b/Gemfile.lock index 7cd04312b948e9873b0886d76467230cf605003e..ea5cc5c26508e1331cc9fa00c56a2999f235e226 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -452,7 +452,7 @@ GEM rails (>= 3.2.0) git (1.7.0) rchardet (~> 1.8) - gitaly (14.1.0.pre.rc4) + gitaly (14.2.0.pre.rc2) grpc (~> 1.0) github-markup (1.7.0) gitlab (4.16.1) @@ -1464,7 +1464,7 @@ DEPENDENCIES gettext (~> 3.3) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly (~> 14.1.0.pre.rc4) + gitaly (~> 14.2.0.pre.rc2) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) gitlab-dangerfiles (~> 2.3.0) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 9914b443eba9eef9c8431f9cc93745a779b92588..d9844ecd4f179e023a4dd0b2c21ea16a92ea4a9b 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -528,7 +528,7 @@ def force_share_with_group_lock_on_descendants def write_projects_repository_config all_projects.find_each do |project| - project.write_repository_config + project.set_full_path project.track_project_repository end end diff --git a/app/models/project.rb b/app/models/project.rb index 7f64b25ee5098f7543d356bfe2465bb90af0b107..6d7e9ddbe9a23f35eecd58bfcdb84b745a1e9fc8 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1889,11 +1889,11 @@ def set_first_pages_deployment!(deployment) .update_all(deployed: deployment.present?, pages_deployment_id: deployment&.id) end - def write_repository_config(gl_full_path: full_path) + def set_full_path(gl_full_path: full_path) # We'd need to keep track of project full path otherwise directory tree # created with hashed storage enabled cannot be usefully imported using # the import rake task. - repository.raw_repository.write_config(full_path: gl_full_path) + repository.raw_repository.set_full_path(full_path: gl_full_path) rescue Gitlab::Git::Repository::NoRepository => e Gitlab::AppLogger.error("Error writing to .git/config for project #{full_path} (#{id}): #{e.message}.") nil @@ -1917,7 +1917,7 @@ def after_import after_create_default_branch join_pool_repository refresh_markdown_cache! - write_repository_config + set_full_path end def update_project_counter_caches diff --git a/app/services/projects/after_rename_service.rb b/app/services/projects/after_rename_service.rb index 6d389035922d43a2c7dcc57b2f88a9b78c22c2c8..953b386b754d4f46dd01ca4995f3db3919540f77 100644 --- a/app/services/projects/after_rename_service.rb +++ b/app/services/projects/after_rename_service.rb @@ -83,7 +83,7 @@ def execute_system_hooks def update_repository_configuration project.reload_repository! - project.write_repository_config + project.set_full_path project.track_project_repository end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 9a3d7b86a6321585fd5659658ddf7f7bbecf76a6..261c82df02db283121c114ef667e8ae267c0e4a4 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -92,7 +92,7 @@ def after_create_actions # Skip writing the config for project imports/forks because it # will always fail since the Git directory doesn't exist until # a background job creates it (see Project#add_import_job). - @project.write_repository_config unless @project.import? + @project.set_full_path unless @project.import? unless @project.gitlab_project_import? @project.create_wiki unless skip_wiki? diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb index c7989e04607b3fda532f444bd5a3e668f33a0fdf..b65d0e63fd354dced2a7279f4cf7ffb43999f298 100644 --- a/app/services/projects/hashed_storage/migrate_repository_service.rb +++ b/app/services/projects/hashed_storage/migrate_repository_service.rb @@ -14,7 +14,7 @@ def execute result = move_repositories if result - project.write_repository_config + project.set_full_path project.track_project_repository else rollback_folder_move diff --git a/app/services/projects/hashed_storage/rollback_repository_service.rb b/app/services/projects/hashed_storage/rollback_repository_service.rb index 6ab4963060300d85d4839c6ada57b2bcfcd5141f..f4146ff9158c6412bf7f9b04cdd9bb1e57436a8e 100644 --- a/app/services/projects/hashed_storage/rollback_repository_service.rb +++ b/app/services/projects/hashed_storage/rollback_repository_service.rb @@ -14,7 +14,7 @@ def execute result = move_repositories if result - project.write_repository_config + project.set_full_path project.track_project_repository else rollback_folder_move diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 43eafc1825561e8a49f8c7dc4fd8ff53c24b0cd1..c69c58e99e21836008b5e9baece6dcc57502cbd2 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -135,7 +135,7 @@ def update_namespace_and_visibility(to_namespace) end def update_repository_configuration(full_path) - project.write_repository_config(gl_full_path: full_path) + project.set_full_path(gl_full_path: full_path) project.track_project_repository end diff --git a/config/feature_flags/development/set_full_path.yml b/config/feature_flags/development/set_full_path.yml new file mode 100644 index 0000000000000000000000000000000000000000..a2f249b60fd4abaad7f8b314dfef00e89fe7d3f3 --- /dev/null +++ b/config/feature_flags/development/set_full_path.yml @@ -0,0 +1,8 @@ +--- +name: set_full_path +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66929 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/337002 +milestone: '14.2' +type: development +group: group::gitaly +default_enabled: false diff --git a/doc/raketasks/import.md b/doc/raketasks/import.md index b18413987a384431862c1c1c0df85adc4815fc64..7f817f9c4225e2845b6347bd9ee65a0b5bef374e 100644 --- a/doc/raketasks/import.md +++ b/doc/raketasks/import.md @@ -142,7 +142,7 @@ to do so. In a Rails console session, run the following to migrate a project: ```ruby project = Project.find_by_full_path('gitlab-org/gitlab') -project.write_repository_config +project.set_full_path ``` In a Rails console session, run the following to migrate all of a namespace's diff --git a/lib/gitlab/bare_repository_import/importer.rb b/lib/gitlab/bare_repository_import/importer.rb index 44106897df889a97ab75e7fd1236bc7b0c68a2c4..28f1a10f9a7b88ce5d05f58435ec9fa82a168043 100644 --- a/lib/gitlab/bare_repository_import/importer.rb +++ b/lib/gitlab/bare_repository_import/importer.rb @@ -73,7 +73,7 @@ def create_project if project.persisted? && mv_repositories(project) log " * Created #{project.name} (#{project_full_path})".color(:green) - project.write_repository_config + project.set_full_path ProjectCacheWorker.perform_async(project.id) else diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index ef615029730816e07ef6418599ea6d5b5da976d2..f88c32d9e27ac050bb2b8a36e4e17ee59d7ef9b8 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -905,13 +905,17 @@ def multi_action( end # rubocop:enable Metrics/ParameterLists - def write_config(full_path:) + def set_full_path(full_path:) return unless full_path.present? # This guard avoids Gitaly log/error spam raise NoRepository, 'repository does not exist' unless exists? - set_config('gitlab.fullpath' => full_path) + if Feature.enabled?(:set_full_path) + gitaly_repository_client.set_full_path(full_path) + else + set_config('gitlab.fullpath' => full_path) + end end def set_config(entries) diff --git a/lib/gitlab/gitaly_client/repository_service.rb b/lib/gitlab/gitaly_client/repository_service.rb index 009aeaf868a73d1518a988393888256195be0e69..1efce802d22182668524de4a6f71d25467e61d66 100644 --- a/lib/gitlab/gitaly_client/repository_service.rb +++ b/lib/gitlab/gitaly_client/repository_service.rb @@ -263,6 +263,21 @@ def write_ref(ref_path, ref, old_ref) GitalyClient.call(@storage, :repository_service, :write_ref, request, timeout: GitalyClient.fast_timeout) end + def set_full_path(path) + GitalyClient.call( + @storage, + :repository_service, + :set_full_path, + Gitaly::SetFullPathRequest.new( + repository: @gitaly_repo, + path: path + ), + timeout: GitalyClient.fast_timeout + ) + + nil + end + def set_config(entries) return if entries.empty? diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 98ba2c8e243f53cbb4343d612aa986939a6e6d8a..e1198349b5c1bc8d6da7902f8e420be7e7cc83d6 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1729,43 +1729,61 @@ def commit_files(commit) end end - describe '#write_config' do - before do - repository_rugged.config["gitlab.fullpath"] = repository_path - end + describe '#set_full_path' do + shared_examples '#set_full_path' do + before do + repository_rugged.config["gitlab.fullpath"] = repository_path + end - context 'is given a path' do - it 'writes it to disk' do - repository.write_config(full_path: "not-the/real-path.git") + context 'is given a path' do + it 'writes it to disk' do + repository.set_full_path(full_path: "not-the/real-path.git") - config = File.read(File.join(repository_path, "config")) + config = File.read(File.join(repository_path, "config")) - expect(config).to include("[gitlab]") - expect(config).to include("fullpath = not-the/real-path.git") + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = not-the/real-path.git") + end end - end - context 'it is given an empty path' do - it 'does not write it to disk' do - repository.write_config(full_path: "") + context 'it is given an empty path' do + it 'does not write it to disk' do + repository.set_full_path(full_path: "") - config = File.read(File.join(repository_path, "config")) + config = File.read(File.join(repository_path, "config")) - expect(config).to include("[gitlab]") - expect(config).to include("fullpath = #{repository_path}") + expect(config).to include("[gitlab]") + expect(config).to include("fullpath = #{repository_path}") + end + end + + context 'repository does not exist' do + it 'raises NoRepository and does not call Gitaly WriteConfig' do + repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project') + + expect(repository.gitaly_repository_client).not_to receive(:set_full_path) + + expect do + repository.set_full_path(full_path: 'foo/bar.git') + end.to raise_error(Gitlab::Git::Repository::NoRepository) + end end end - context 'repository does not exist' do - it 'raises NoRepository and does not call Gitaly WriteConfig' do - repository = Gitlab::Git::Repository.new('default', 'does/not/exist.git', '', 'group/project') + context 'with :set_full_path enabled' do + before do + stub_feature_flags(set_full_path: true) + end - expect(repository.gitaly_repository_client).not_to receive(:write_config) + it_behaves_like '#set_full_path' + end - expect do - repository.write_config(full_path: 'foo/bar.git') - end.to raise_error(Gitlab::Git::Repository::NoRepository) + context 'with :set_full_path disabled' do + before do + stub_feature_flags(set_full_path: false) end + + it_behaves_like '#set_full_path' end end diff --git a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb index 53805d67f9f21a3204ef27213505773ed2233b81..95063861b77c720a37ec7e80ec70d5b7e6d40eb1 100644 --- a/spec/lib/gitlab/gitaly_client/repository_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/repository_service_spec.rb @@ -333,4 +333,17 @@ client.replicate(source_repository) end end + + describe '#set_full_path' do + let(:path) { 'repo/path' } + + it 'sends a set_full_path message' do + expect_any_instance_of(Gitaly::RepositoryService::Stub) + .to receive(:set_full_path) + .with(gitaly_request_with_params(path: path), kind_of(Hash)) + .and_return(double) + + client.set_full_path(path) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2006ec85978f50a873867be3c848ac3c8b373d6e..3d0afaee645acbb49637de4dc59cc7036673657b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -5228,7 +5228,7 @@ def subject expect(InternalId).to receive(:flush_records!).with(project: project) expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:repository_size]) expect(DetectRepositoryLanguagesWorker).to receive(:perform_async).with(project.id) - expect(project).to receive(:write_repository_config) + expect(project).to receive(:set_full_path) project.after_import end @@ -5297,25 +5297,25 @@ def subject end end - describe '#write_repository_config' do + describe '#set_full_path' do let_it_be(:project) { create(:project, :repository) } it 'writes full path in .git/config when key is missing' do - project.write_repository_config + project.set_full_path expect(rugged_config['gitlab.fullpath']).to eq project.full_path end it 'updates full path in .git/config when key is present' do - project.write_repository_config(gl_full_path: 'old/path') + project.set_full_path(gl_full_path: 'old/path') - expect { project.write_repository_config }.to change { rugged_config['gitlab.fullpath'] }.from('old/path').to(project.full_path) + expect { project.set_full_path }.to change { rugged_config['gitlab.fullpath'] }.from('old/path').to(project.full_path) end it 'does not raise an error with an empty repository' do project = create(:project_empty_repo) - expect { project.write_repository_config }.not_to raise_error + expect { project.set_full_path }.not_to raise_error end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 5e835e89b08a602b0d6c8a3fc36b0c437dc899f8..92f9c2356cd8ba1f712dbcc828d26810936316eb 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -335,7 +335,7 @@ def wiki_repo(project) it 'does not write repository config' do expect_next_instance_of(Project) do |project| - expect(project).not_to receive(:write_repository_config) + expect(project).not_to receive(:set_full_path) end imported_project diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 47252bcf7a7516a004ccd67009736e454374bae3..d0064873972d2e9b45b163ab358a212b64ef6e84 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -96,13 +96,13 @@ end it 'handles Gitlab::Git::CommandError' do - expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError) + expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) expect { service.execute }.not_to raise_exception end it 'ensures rollback when Gitlab::Git::CommandError' do - expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError) + expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) expect(service).to receive(:rollback_folder_move).and_call_original service.execute diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb index af128a532b9fa0cffb4b601090b9f089a65ab3f7..23e776b72bc34bb1680cbb09615b6c882dee0b1d 100644 --- a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb @@ -96,13 +96,13 @@ end it 'handles Gitlab::Git::CommandError' do - expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError) + expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) expect { service.execute }.not_to raise_exception end it 'ensures rollback when Gitlab::Git::CommandError' do - expect(project).to receive(:write_repository_config).and_raise(Gitlab::Git::CommandError) + expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) expect(service).to receive(:rollback_folder_move).and_call_original service.execute