kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch '40711-fix-forking-hashed-projects' into 'master'

Browse source 
Fix the fork project functionality for projects with hashed storage

Closes #40711

See merge request gitlab-org/gitlab-ce!15671
This commit is contained in:
Douwe Maan 2017-12-04 11:31:15 +00:00
commit 50eb125282
8 changed files with 60 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
GITLAB_SHELL_VERSION
View file

@ -1 +1 @@
5.9.4
5.10.0

3
app/models/project.rb
View file

@ -562,8 +562,7 @@ class Project < ActiveRecord::Base
if forked?
RepositoryForkWorker.perform_async(id,
forked_from_project.repository_storage_path,
forked_from_project.full_path,
self.namespace.full_path)
forked_from_project.disk_path)
else
RepositoryImportWorker.perform_async(self.id)
end

12
app/workers/repository_fork_worker.rb
View file

@ -8,18 +8,18 @@ class RepositoryForkWorker
sidekiq_options status_expiration: StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION
def perform(project_id, forked_from_repository_storage_path, source_path, target_path)
def perform(project_id, forked_from_repository_storage_path, source_disk_path)
project = Project.find(project_id)
return unless start_fork(project)
Gitlab::Metrics.add_event(:fork_repository,
source_path: source_path,
target_path: target_path)
source_path: source_disk_path,
target_path: project.disk_path)
result = gitlab_shell.fork_repository(forked_from_repository_storage_path, source_path,
project.repository_storage_path, target_path)
raise ForkError, "Unable to fork project #{project_id} for repository #{source_path} -> #{target_path}" unless result
result = gitlab_shell.fork_repository(forked_from_repository_storage_path, source_disk_path,
project.repository_storage_path, project.disk_path)
raise ForkError, "Unable to fork project #{project_id} for repository #{source_disk_path} -> #{project.disk_path}" unless result
project.repository.after_import
raise ForkError, "Project #{project_id} had an invalid repository after fork" unless project.valid_repo?

5
changelogs/unreleased/40711-fix-forking-hashed-projects.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Fix the fork project functionality for projects with hashed storage
merge_request: 15671
author:
type: fixed

23
lib/gitlab/shell.rb
View file

@ -143,20 +143,27 @@ module Gitlab
storage, "#{path}.git", "#{new_path}.git"])
end
# Fork repository to new namespace
# Fork repository to new path
# forked_from_storage - forked-from project's storage path
# path - project path with namespace
# forked_from_disk_path - project disk path
# forked_to_storage - forked-to project's storage path
# fork_namespace - namespace for forked project
# forked_to_disk_path - forked project disk path
#
# Ex.
# fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "randx")
# fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "new-namespace/gitlab-ci")
#
# Gitaly note: JV: not easy to migrate because this involves two Gitaly servers, not one.
def fork_repository(forked_from_storage, path, forked_to_storage, fork_namespace)
gitlab_shell_fast_execute([gitlab_shell_projects_path, 'fork-project',
forked_from_storage, "#{path}.git", forked_to_storage,
fork_namespace])
def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path)
gitlab_shell_fast_execute(
[
gitlab_shell_projects_path,
'fork-repository',
forked_from_storage,
"#{forked_from_disk_path}.git",
forked_to_storage,
"#{forked_to_disk_path}.git"
]
)
end
# Remove repository from file system

8
spec/lib/gitlab/shell_spec.rb
View file

@ -200,18 +200,18 @@ describe Gitlab::Shell do
describe '#fork_repository' do
it 'returns true when the command succeeds' do
expect(Gitlab::Popen).to receive(:popen)
.with([projects_path, 'fork-project', 'current/storage', 'project/path.git', 'new/storage', 'new-namespace'],
.with([projects_path, 'fork-repository', 'current/storage', 'project/path.git', 'new/storage', 'fork/path.git'],
nil, popen_vars).and_return([nil, 0])
expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'new-namespace')).to be true
expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'fork/path')).to be true
end
it 'return false when the command fails' do
expect(Gitlab::Popen).to receive(:popen)
.with([projects_path, 'fork-project', 'current/storage', 'project/path.git', 'new/storage', 'new-namespace'],
.with([projects_path, 'fork-repository', 'current/storage', 'project/path.git', 'new/storage', 'fork/path.git'],
nil, popen_vars).and_return(["error", 1])
expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'new-namespace')).to be false
expect(gitlab_shell.fork_repository('current/storage', 'project/path', 'new/storage', 'fork/path')).to be false
end
end

3
spec/models/project_spec.rb
View file

@ -1717,8 +1717,7 @@ describe Project do
expect(RepositoryForkWorker).to receive(:perform_async).with(
project.id,
forked_from_project.repository_storage_path,
forked_from_project.disk_path,
project.namespace.full_path).and_return(import_jid)
forked_from_project.disk_path).and_return(import_jid)
expect(project.add_import_job).to eq(import_jid)
end

75
spec/workers/repository_fork_worker_spec.rb
View file

@ -1,8 +1,8 @@
require 'spec_helper'
describe RepositoryForkWorker do
let(:project) { create(:project, :repository, :import_scheduled) }
let(:fork_project) { create(:project, :repository, forked_from_project: project) }
let(:project) { create(:project, :repository) }
let(:fork_project) { create(:project, :repository, :import_scheduled, forked_from_project: project) }
let(:shell) { Gitlab::Shell.new }
subject { described_class.new }
@ -12,50 +12,39 @@ describe RepositoryForkWorker do
end
describe "#perform" do
def perform!
subject.perform(fork_project.id, '/test/path', project.disk_path)
end
def expect_fork_repository
expect(shell).to receive(:fork_repository).with(
'/test/path',
project.disk_path,
fork_project.repository_storage_path,
fork_project.disk_path
)
end
describe 'when a worker was reset without cleanup' do
let(:jid) { '12345678' }
let(:started_project) { create(:project, :repository, :import_started) }
it 'creates a new repository from a fork' do
allow(subject).to receive(:jid).and_return(jid)
expect(shell).to receive(:fork_repository).with(
'/test/path',
project.full_path,
project.repository_storage_path,
fork_project.namespace.full_path
).and_return(true)
expect_fork_repository.and_return(true)
subject.perform(
project.id,
'/test/path',
project.full_path,
fork_project.namespace.full_path)
perform!
end
end
it "creates a new repository from a fork" do
expect(shell).to receive(:fork_repository).with(
'/test/path',
project.full_path,
project.repository_storage_path,
fork_project.namespace.full_path
).and_return(true)
expect_fork_repository.and_return(true)
subject.perform(
project.id,
'/test/path',
project.full_path,
fork_project.namespace.full_path)
perform!
end
it 'flushes various caches' do
expect(shell).to receive(:fork_repository).with(
'/test/path',
project.full_path,
project.repository_storage_path,
fork_project.namespace.full_path
).and_return(true)
expect_fork_repository.and_return(true)
expect_any_instance_of(Repository).to receive(:expire_emptiness_caches)
.and_call_original
@ -63,32 +52,22 @@ describe RepositoryForkWorker do
expect_any_instance_of(Repository).to receive(:expire_exists_cache)
.and_call_original
subject.perform(project.id, '/test/path', project.full_path,
fork_project.namespace.full_path)
perform!
end
it "handles bad fork" do
source_path = project.full_path
target_path = fork_project.namespace.full_path
error_message = "Unable to fork project #{project.id} for repository #{source_path} -> #{target_path}"
error_message = "Unable to fork project #{fork_project.id} for repository #{project.full_path} -> #{fork_project.full_path}"
expect(shell).to receive(:fork_repository).and_return(false)
expect_fork_repository.and_return(false)
expect do
subject.perform(project.id, '/test/path', source_path, target_path)
end.to raise_error(RepositoryForkWorker::ForkError, error_message)
expect { perform! }.to raise_error(RepositoryForkWorker::ForkError, error_message)
end
it 'handles unexpected error' do
source_path = project.full_path
target_path = fork_project.namespace.full_path
expect_fork_repository.and_raise(RuntimeError)
allow_any_instance_of(Gitlab::Shell).to receive(:fork_repository).and_raise(RuntimeError)
expect do
subject.perform(project.id, '/test/path', source_path, target_path)
end.to raise_error(RepositoryForkWorker::ForkError)
expect(project.reload.import_status).to eq('failed')
expect { perform! }.to raise_error(RepositoryForkWorker::ForkError)
expect(fork_project.reload.import_status).to eq('failed')
end
end
end