Commit 5ba71874 authored by Dylan Griffith's avatar Dylan Griffith

Merge branch 'initial_repository_storage_move' into 'master'

Store status of repository storage moves

See merge request gitlab-org/gitlab!29095
parents 077e4624 dab95ba5
......@@ -324,6 +324,8 @@ class Project < ApplicationRecord
has_many :daily_report_results, class_name: 'Ci::DailyReportResult'
has_many :repository_storage_moves, class_name: 'ProjectRepositoryStorageMove'
accepts_nested_attributes_for :variables, allow_destroy: true
accepts_nested_attributes_for :project_feature, update_only: true
accepts_nested_attributes_for :import_data
......@@ -2073,7 +2075,12 @@ class Project < ApplicationRecord
raise ArgumentError unless ::Gitlab.config.repositories.storages.key?(new_repository_storage_key)
run_after_commit { ProjectUpdateRepositoryStorageWorker.perform_async(id, new_repository_storage_key) }
storage_move = repository_storage_moves.create!(
source_storage_name: repository_storage,
destination_storage_name: new_repository_storage_key
)
storage_move.schedule!
self.repository_read_only = true
end
......
# frozen_string_literal: true
# ProjectRepositoryStorageMove are details of repository storage moves for a
# project. For example, moving a project to another gitaly node to help
# balance storage capacity.
class ProjectRepositoryStorageMove < ApplicationRecord
include AfterCommitQueue
belongs_to :project, inverse_of: :repository_storage_moves
validates :project, presence: true
validates :state, presence: true
validates :source_storage_name,
on: :create,
presence: true,
inclusion: { in: ->(_) { Gitlab.config.repositories.storages.keys } }
validates :destination_storage_name,
on: :create,
presence: true,
inclusion: { in: ->(_) { Gitlab.config.repositories.storages.keys } }
state_machine initial: :initial do
event :schedule do
transition initial: :scheduled
end
event :start do
transition scheduled: :started
end
event :finish do
transition started: :finished
end
event :do_fail do
transition [:initial, :scheduled, :started] => :failed
end
after_transition initial: :scheduled do |storage_move, _|
storage_move.run_after_commit do
ProjectUpdateRepositoryStorageWorker.perform_async(
storage_move.project_id, storage_move.destination_storage_name,
repository_storage_move_id: storage_move.id
)
end
end
state :initial, value: 1
state :scheduled, value: 2
state :started, value: 3
state :finished, value: 4
state :failed, value: 5
end
end
# frozen_string_literal: true
module Projects
class UpdateRepositoryStorageService < BaseService
include Gitlab::ShellAdapter
class UpdateRepositoryStorageService
Error = Class.new(StandardError)
SameFilesystemError = Class.new(Error)
def initialize(project)
@project = project
attr_reader :repository_storage_move
delegate :project, :destination_storage_name, to: :repository_storage_move
delegate :repository, to: :project
def initialize(repository_storage_move)
@repository_storage_move = repository_storage_move
end
def execute(new_repository_storage_key)
raise SameFilesystemError if same_filesystem?(project.repository.storage, new_repository_storage_key)
def execute
repository_storage_move.start!
raise SameFilesystemError if same_filesystem?(repository.storage, destination_storage_name)
mirror_repositories(new_repository_storage_key)
mirror_repositories
project.transaction do
mark_old_paths_for_archive
project.update!(repository_storage: new_repository_storage_key, repository_read_only: false)
repository_storage_move.finish!
project.update!(repository_storage: destination_storage_name, repository_read_only: false)
project.leave_pool_repository
project.track_project_repository
end
enqueue_housekeeping
success
ServiceResponse.success
rescue StandardError => e
project.transaction do
repository_storage_move.do_fail!
project.update!(repository_read_only: false)
end
Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path)
error(s_("UpdateRepositoryStorage|Error moving repository storage for %{project_full_path} - %{message}") % { project_full_path: project.full_path, message: e.message })
ServiceResponse.error(
message: s_("UpdateRepositoryStorage|Error moving repository storage for %{project_full_path} - %{message}") % { project_full_path: project.full_path, message: e.message }
)
end
private
......@@ -40,15 +52,15 @@ module Projects
Gitlab::GitalyClient.filesystem_id(old_storage) == Gitlab::GitalyClient.filesystem_id(new_storage)
end
def mirror_repositories(new_repository_storage_key)
mirror_repository(new_repository_storage_key)
def mirror_repositories
mirror_repository
if project.wiki.repository_exists?
mirror_repository(new_repository_storage_key, type: Gitlab::GlRepository::WIKI)
mirror_repository(type: Gitlab::GlRepository::WIKI)
end
end
def mirror_repository(new_storage_key, type: Gitlab::GlRepository::PROJECT)
def mirror_repository(type: Gitlab::GlRepository::PROJECT)
unless wait_for_pushes(type)
raise Error, s_('UpdateRepositoryStorage|Timeout waiting for %{type} repository pushes') % { type: type.name }
end
......@@ -60,7 +72,7 @@ module Projects
# Initialize a git repository on the target path
new_repository = Gitlab::Git::Repository.new(
new_storage_key,
destination_storage_name,
raw_repository.relative_path,
raw_repository.gl_repository,
full_path
......
......@@ -5,9 +5,19 @@ class ProjectUpdateRepositoryStorageWorker # rubocop:disable Scalability/Idempot
feature_category :gitaly
def perform(project_id, new_repository_storage_key)
def perform(project_id, new_repository_storage_key, repository_storage_move_id: nil)
repository_storage_move =
if repository_storage_move_id
ProjectRepositoryStorageMove.find(repository_storage_move_id)
else
# maintain compatibility with workers queued before release
project = Project.find(project_id)
project.repository_storage_moves.create!(
source_storage_name: project.repository_storage,
destination_storage_name: new_repository_storage_key
)
end
::Projects::UpdateRepositoryStorageService.new(project).execute(new_repository_storage_key)
::Projects::UpdateRepositoryStorageService.new(repository_storage_move).execute
end
end
---
title: Store status of repository storage moves
merge_request: 29095
author:
type: changed
# frozen_string_literal: true
class CreateProjectRepositoryStorageMoves < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
create_table :project_repository_storage_moves do |t|
t.timestamps_with_timezone
t.integer :project_id, limit: 8, null: false
t.integer :state, limit: 2, default: 1, null: false
t.text :source_storage_name, null: false
t.text :destination_storage_name, null: false
end
add_index :project_repository_storage_moves, :project_id
add_text_limit(:project_repository_storage_moves, :source_storage_name, 255, constraint_name: 'project_repository_storage_moves_source_storage_name')
add_text_limit(:project_repository_storage_moves, :destination_storage_name, 255, constraint_name: 'project_repository_storage_moves_destination_storage_name')
end
def down
remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_source_storage_name')
remove_check_constraint(:project_repository_storage_moves, 'project_repository_storage_moves_destination_storage_name')
drop_table :project_repository_storage_moves
end
end
# frozen_string_literal: true
class AddFkToProjectRepositoryStorageMoves < ActiveRecord::Migration[6.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
with_lock_retries do
add_foreign_key :project_repository_storage_moves, :projects, on_delete: :cascade # rubocop:disable Migration/AddConcurrentForeignKey
end
end
def down
with_lock_retries do
remove_foreign_key :project_repository_storage_moves, :projects
end
end
end
......@@ -5171,6 +5171,27 @@ CREATE SEQUENCE public.project_repository_states_id_seq
ALTER SEQUENCE public.project_repository_states_id_seq OWNED BY public.project_repository_states.id;
CREATE TABLE public.project_repository_storage_moves (
id bigint NOT NULL,
created_at timestamp with time zone NOT NULL,
updated_at timestamp with time zone NOT NULL,
project_id bigint NOT NULL,
state smallint DEFAULT 1 NOT NULL,
source_storage_name text NOT NULL,
destination_storage_name text NOT NULL,
CONSTRAINT project_repository_storage_moves_destination_storage_name CHECK ((char_length(destination_storage_name) <= 255)),
CONSTRAINT project_repository_storage_moves_source_storage_name CHECK ((char_length(source_storage_name) <= 255))
);
CREATE SEQUENCE public.project_repository_storage_moves_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE public.project_repository_storage_moves_id_seq OWNED BY public.project_repository_storage_moves.id;
CREATE TABLE public.project_settings (
project_id integer NOT NULL,
created_at timestamp with time zone NOT NULL,
......@@ -7640,6 +7661,8 @@ ALTER TABLE ONLY public.project_repositories ALTER COLUMN id SET DEFAULT nextval
ALTER TABLE ONLY public.project_repository_states ALTER COLUMN id SET DEFAULT nextval('public.project_repository_states_id_seq'::regclass);
ALTER TABLE ONLY public.project_repository_storage_moves ALTER COLUMN id SET DEFAULT nextval('public.project_repository_storage_moves_id_seq'::regclass);
ALTER TABLE ONLY public.project_statistics ALTER COLUMN id SET DEFAULT nextval('public.project_statistics_id_seq'::regclass);
ALTER TABLE ONLY public.project_tracing_settings ALTER COLUMN id SET DEFAULT nextval('public.project_tracing_settings_id_seq'::regclass);
......@@ -8525,6 +8548,9 @@ ALTER TABLE ONLY public.project_repositories
ALTER TABLE ONLY public.project_repository_states
ADD CONSTRAINT project_repository_states_pkey PRIMARY KEY (id);
ALTER TABLE ONLY public.project_repository_storage_moves
ADD CONSTRAINT project_repository_storage_moves_pkey PRIMARY KEY (id);
ALTER TABLE ONLY public.project_settings
ADD CONSTRAINT project_settings_pkey PRIMARY KEY (project_id);
......@@ -10188,6 +10214,8 @@ CREATE INDEX index_project_repositories_on_shard_id ON public.project_repositori
CREATE UNIQUE INDEX index_project_repository_states_on_project_id ON public.project_repository_states USING btree (project_id);
CREATE INDEX index_project_repository_storage_moves_on_project_id ON public.project_repository_storage_moves USING btree (project_id);
CREATE UNIQUE INDEX index_project_settings_on_push_rule_id ON public.project_settings USING btree (push_rule_id);
CREATE INDEX index_project_statistics_on_namespace_id ON public.project_statistics USING btree (namespace_id);
......@@ -11774,6 +11802,9 @@ ALTER TABLE ONLY public.merge_request_diff_files
ALTER TABLE ONLY public.status_page_settings
ADD CONSTRAINT fk_rails_506e5ba391 FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
ALTER TABLE ONLY public.project_repository_storage_moves
ADD CONSTRAINT fk_rails_5106dbd44a FOREIGN KEY (project_id) REFERENCES public.projects(id) ON DELETE CASCADE;
ALTER TABLE ONLY public.x509_commit_signatures
ADD CONSTRAINT fk_rails_53fe41188f FOREIGN KEY (x509_certificate_id) REFERENCES public.x509_certificates(id) ON DELETE CASCADE;
......@@ -13575,6 +13606,7 @@ COPY "schema_migrations" (version) FROM STDIN;
20200407171133
20200407171417
20200407182205
20200407222647
20200408110856
20200408133211
20200408153842
......@@ -13638,5 +13670,6 @@ COPY "schema_migrations" (version) FROM STDIN;
20200424050250
20200424101920
20200427064130
20200429015603
\.
......@@ -6,11 +6,11 @@ module EE
extend ::Gitlab::Utils::Override
override :mirror_repositories
def mirror_repositories(new_repository_storage_key)
def mirror_repositories
super
if project.design_repository.exists?
mirror_repository(new_repository_storage_key, type: ::Gitlab::GlRepository::DESIGN)
mirror_repository(type: ::Gitlab::GlRepository::DESIGN)
end
end
......
......@@ -5,7 +5,7 @@ require 'spec_helper'
describe Projects::UpdateRepositoryStorageService do
include Gitlab::ShellAdapter
subject { described_class.new(project) }
subject { described_class.new(repository_storage_move) }
before do
allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage])
......@@ -16,6 +16,8 @@ describe Projects::UpdateRepositoryStorageService do
include_examples 'moves repository to another storage', 'design' do
let(:project) { create(:project, :repository, repository_read_only: true) }
let(:repository) { project.design_repository }
let(:destination) { 'test_second_storage' }
let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) }
before do
project.design_repository.create_if_not_exists
......
# frozen_string_literal: true
FactoryBot.define do
factory :project_repository_storage_move, class: 'ProjectRepositoryStorageMove' do
project
source_storage_name { 'default' }
destination_storage_name { 'default' }
trait :scheduled do
state { ProjectRepositoryStorageMove.state_machines[:state].states[:scheduled].value }
end
end
end
......@@ -489,6 +489,7 @@ project:
- compliance_framework_setting
- metrics_users_starred_dashboards
- alert_management_alerts
- repository_storage_moves
award_emoji:
- awardable
- user
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ProjectRepositoryStorageMove, type: :model do
describe 'associations' do
it { is_expected.to belong_to(:project) }
end
describe 'validations' do
it { is_expected.to validate_presence_of(:project) }
it { is_expected.to validate_presence_of(:state) }
it { is_expected.to validate_presence_of(:source_storage_name) }
it { is_expected.to validate_presence_of(:destination_storage_name) }
context 'source_storage_name inclusion' do
subject { build(:project_repository_storage_move, source_storage_name: 'missing') }
it "does not allow repository storages that don't match a label in the configuration" do
expect(subject).not_to be_valid
expect(subject.errors[:source_storage_name].first).to match(/is not included in the list/)
end
end
context 'destination_storage_name inclusion' do
subject { build(:project_repository_storage_move, destination_storage_name: 'missing') }
it "does not allow repository storages that don't match a label in the configuration" do
expect(subject).not_to be_valid
expect(subject.errors[:destination_storage_name].first).to match(/is not included in the list/)
end
end
end
describe 'state transitions' do
using RSpec::Parameterized::TableSyntax
context 'when in the default state' do
subject(:storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: 'test_second_storage') }
let(:project) { create(:project) }
before do
stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/extra_storage' })
end
context 'and transits to scheduled' do
it 'triggers ProjectUpdateRepositoryStorageWorker' do
expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'test_second_storage', repository_storage_move_id: storage_move.id)
storage_move.schedule!
end
end
context 'and transits to started' do
it 'does not allow the transition' do
expect { storage_move.start! }
.to raise_error(StateMachines::InvalidTransition)
end
end
end
end
end
......@@ -115,6 +115,7 @@ describe Project do
it { is_expected.to have_many(:alert_management_alerts) }
it { is_expected.to have_many(:jira_imports) }
it { is_expected.to have_many(:metrics_users_starred_dashboards).inverse_of(:project) }
it { is_expected.to have_many(:repository_storage_moves) }
it_behaves_like 'model with repository' do
let_it_be(:container) { create(:project, :repository, path: 'somewhere') }
......@@ -2837,12 +2838,16 @@ describe Project do
end
it 'schedules the transfer of the repository to the new storage and locks the project' do
expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'test_second_storage')
expect(ProjectUpdateRepositoryStorageWorker).to receive(:perform_async).with(project.id, 'test_second_storage', repository_storage_move_id: anything)
project.change_repository_storage('test_second_storage')
project.save!
expect(project).to be_repository_read_only
expect(project.repository_storage_moves.last).to have_attributes(
source_storage_name: "default",
destination_storage_name: "test_second_storage"
)
end
it "doesn't schedule the transfer if the repository is already read-only" do
......
......@@ -320,7 +320,13 @@ describe Projects::ForkService do
allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum)
.and_return(::Gitlab::Git::BLANK_SHA)
Projects::UpdateRepositoryStorageService.new(project).execute('test_second_storage')
storage_move = create(
:project_repository_storage_move,
:scheduled,
project: project,
destination_storage_name: 'test_second_storage'
)
Projects::UpdateRepositoryStorageService.new(storage_move).execute
fork_after_move = fork_project(project)
pool_repository_before_move = PoolRepository.joins(:shard)
.find_by(source_project: project, shards: { name: 'default' })
......
......@@ -5,7 +5,7 @@ require 'spec_helper'
describe Projects::UpdateRepositoryStorageService do
include Gitlab::ShellAdapter
subject { described_class.new(project) }
subject { described_class.new(repository_storage_move) }
describe "#execute" do
let(:time) { Time.now }
......@@ -17,6 +17,8 @@ describe Projects::UpdateRepositoryStorageService do
context 'without wiki and design repository' do
let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: false) }
let(:destination) { 'test_second_storage' }
let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) }
let!(:checksum) { project.repository.checksum }
let(:project_repository_double) { double(:repository) }
......@@ -42,9 +44,9 @@ describe Projects::UpdateRepositoryStorageService do
expect(project_repository_double).to receive(:checksum)
.and_return(checksum)
result = subject.execute('test_second_storage')
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result).to be_success
expect(project).not_to be_repository_read_only
expect(project.repository_storage).to eq('test_second_storage')
expect(gitlab_shell.repository_exists?('default', old_path)).to be(false)
......@@ -53,11 +55,13 @@ describe Projects::UpdateRepositoryStorageService do
end
context 'when the filesystems are the same' do
let(:destination) { project.repository_storage }
it 'bails out and does nothing' do
result = subject.execute(project.repository_storage)
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:message]).to match(/SameFilesystemError/)
expect(result).to be_error
expect(result.message).to match(/SameFilesystemError/)
end
end
......@@ -73,9 +77,9 @@ describe Projects::UpdateRepositoryStorageService do
.and_raise(Gitlab::Git::CommandError)
expect(GitlabShellWorker).not_to receive(:perform_async)
result = subject.execute('test_second_storage')
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result).to be_error
expect(project).not_to be_repository_read_only
expect(project.repository_storage).to eq('default')
end
......@@ -94,9 +98,9 @@ describe Projects::UpdateRepositoryStorageService do
.and_return('not matching checksum')
expect(GitlabShellWorker).not_to receive(:perform_async)
result = subject.execute('test_second_storage')
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result).to be_error
expect(project).not_to be_repository_read_only
expect(project.repository_storage).to eq('default')
end
......@@ -116,9 +120,9 @@ describe Projects::UpdateRepositoryStorageService do
expect(project_repository_double).to receive(:checksum)
.and_return(checksum)
result = subject.execute('test_second_storage')
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result).to be_success
expect(project.repository_storage).to eq('test_second_storage')
expect(project.reload_pool_repository).to be_nil
end
......@@ -129,6 +133,8 @@ describe Projects::UpdateRepositoryStorageService do
include_examples 'moves repository to another storage', 'wiki' do
let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: true) }
let(:repository) { project.wiki.repository }
let(:destination) { 'test_second_storage' }
let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) }
before do
project.create_wiki
......
......@@ -47,9 +47,9 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
old_repository_path = repository.full_path
result = subject.execute('test_second_storage')
result = subject.execute
expect(result[:status]).to eq(:success)
expect(result).to be_success
expect(project).not_to be_repository_read_only
expect(project.repository_storage).to eq('test_second_storage')
expect(gitlab_shell.repository_exists?('default', old_project_repository_path)).to be(false)
......@@ -62,7 +62,7 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
end
it 'does not enqueue a GC run' do
expect { subject.execute('test_second_storage') }
expect { subject.execute }
.not_to change(GitGarbageCollectWorker.jobs, :count)
end
end
......@@ -75,23 +75,25 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
it 'does not enqueue a GC run if housekeeping is disabled' do
stub_application_setting(housekeeping_enabled: false)
expect { subject.execute('test_second_storage') }
expect { subject.execute }
.not_to change(GitGarbageCollectWorker.jobs, :count)
end
it 'enqueues a GC run' do
expect { subject.execute('test_second_storage') }
expect { subject.execute }
.to change(GitGarbageCollectWorker.jobs, :count).by(1)
end
end
end
context 'when the filesystems are the same' do
let(:destination) { project.repository_storage }
it 'bails out and does nothing' do
result = subject.execute(project.repository_storage)
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result[:message]).to match(/SameFilesystemError/)
expect(result).to be_error
expect(result.message).to match(/SameFilesystemError/)
end
end
......@@ -114,9 +116,9 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
expect(GitlabShellWorker).not_to receive(:perform_async)
result = subject.execute('test_second_storage')
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result).to be_error
expect(project).not_to be_repository_read_only
expect(project.repository_storage).to eq('default')
end
......@@ -142,9 +144,9 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
expect(GitlabShellWorker).not_to receive(:perform_async)
result = subject.execute('test_second_storage')
result = subject.execute
expect(result[:status]).to eq(:error)
expect(result).to be_error
expect(project).not_to be_repository_read_only
expect(project.repository_storage).to eq('default')
end
......
......@@ -9,12 +9,40 @@ describe ProjectUpdateRepositoryStorageWorker do
subject { described_class.new }
describe "#perform" do
let(:service) { double(:update_repository_storage_service) }
before do
allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage])
end
context 'without repository storage move' do
it "calls the update repository storage service" do
expect_next_instance_of(Projects::UpdateRepositoryStorageService) do |instance|
expect(instance).to receive(:execute).with('new_storage')
expect(Projects::UpdateRepositoryStorageService).to receive(:new).and_return(service)
expect(service).to receive(:execute)
expect do
subject.perform(project.id, 'test_second_storage')
end.to change(ProjectRepositoryStorageMove, :count).by(1)
storage_move = project.repository_storage_moves.last
expect(storage_move).to have_attributes(
source_storage_name: "default",
destination_storage_name: "test_second_storage"
)
end
end
context 'with repository storage move' do
let!(:repository_storage_move) { create(:project_repository_storage_move) }
subject.perform(project.id, 'new_storage')
it "calls the update repository storage service" do
expect(Projects::UpdateRepositoryStorageService).to receive(:new).and_return(service)
expect(service).to receive(:execute)
expect do
subject.perform(nil, nil, repository_storage_move_id: repository_storage_move.id)
end.not_to change(ProjectRepositoryStorageMove, :count)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment