Fix the fork project functionality for projects with hashed storage
Note the dependency on gitlab-shell v5.10.0
This commit is contained in:
parent
e0f8413056
commit
327a9898a2
8 changed files with 60 additions and 71 deletions
|
@ -1 +1 @@
|
|||
5.9.4
|
||||
5.10.0
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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?
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix the fork project functionality for projects with hashed storage
|
||||
merge_request: 15671
|
||||
author:
|
||||
type: fixed
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in a new issue