Commit 79b62b32 authored by James Fargher's avatar James Fargher

Add state machine to storage moves

Fixes various specs
parent 5c9a9c53
...@@ -2079,13 +2079,7 @@ class Project < ApplicationRecord ...@@ -2079,13 +2079,7 @@ class Project < ApplicationRecord
source_storage_name: repository_storage, source_storage_name: repository_storage,
destination_storage_name: new_repository_storage_key destination_storage_name: new_repository_storage_key
) )
storage_move.schedule!
run_after_commit do
ProjectUpdateRepositoryStorageWorker.perform_async(
id, new_repository_storage_key,
repository_storage_move_id: storage_move.id
)
end
self.repository_read_only = true self.repository_read_only = true
end end
......
...@@ -4,16 +4,51 @@ ...@@ -4,16 +4,51 @@
# project. For example, moving a project to another gitaly node to help # project. For example, moving a project to another gitaly node to help
# balance storage capacity. # balance storage capacity.
class ProjectRepositoryStorageMove < ApplicationRecord class ProjectRepositoryStorageMove < ApplicationRecord
belongs_to :project, inverse_of: :repository_storage_moves include AfterCommitQueue
enum state: { scheduled: 1, started: 2, finished: 3, failed: 4 } belongs_to :project, inverse_of: :repository_storage_moves
validates :project, presence: true validates :project, presence: true
validates :state, presence: true validates :state, presence: true
validates :source_storage_name, validates :source_storage_name,
on: :create,
presence: true, presence: true,
inclusion: { in: ->(_) { Gitlab.config.repositories.storages.keys } } inclusion: { in: ->(_) { Gitlab.config.repositories.storages.keys } }
validates :destination_storage_name, validates :destination_storage_name,
on: :create,
presence: true, presence: true,
inclusion: { in: ->(_) { Gitlab.config.repositories.storages.keys } } 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 end
...@@ -14,7 +14,7 @@ module Projects ...@@ -14,7 +14,7 @@ module Projects
end end
def execute def execute
repository_storage_move.update!(state: :started) repository_storage_move.start!
raise SameFilesystemError if same_filesystem?(repository.storage, destination_storage_name) raise SameFilesystemError if same_filesystem?(repository.storage, destination_storage_name)
...@@ -23,7 +23,7 @@ module Projects ...@@ -23,7 +23,7 @@ module Projects
project.transaction do project.transaction do
mark_old_paths_for_archive mark_old_paths_for_archive
repository_storage_move.update!(state: :finished) repository_storage_move.finish!
project.update!(repository_storage: destination_storage_name, repository_read_only: false) project.update!(repository_storage: destination_storage_name, repository_read_only: false)
project.leave_pool_repository project.leave_pool_repository
project.track_project_repository project.track_project_repository
...@@ -35,7 +35,7 @@ module Projects ...@@ -35,7 +35,7 @@ module Projects
rescue StandardError => e rescue StandardError => e
project.transaction do project.transaction do
repository_storage_move.update!(state: :failed) repository_storage_move.do_fail!
project.update!(repository_read_only: false) project.update!(repository_read_only: false)
end end
......
...@@ -17,7 +17,7 @@ describe Projects::UpdateRepositoryStorageService do ...@@ -17,7 +17,7 @@ describe Projects::UpdateRepositoryStorageService do
let(:project) { create(:project, :repository, repository_read_only: true) } let(:project) { create(:project, :repository, repository_read_only: true) }
let(:repository) { project.design_repository } let(:repository) { project.design_repository }
let(:destination) { 'test_second_storage' } let(:destination) { 'test_second_storage' }
let(:repository_storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: destination) } let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) }
before do before do
project.design_repository.create_if_not_exists project.design_repository.create_if_not_exists
......
...@@ -6,5 +6,9 @@ FactoryBot.define do ...@@ -6,5 +6,9 @@ FactoryBot.define do
source_storage_name { 'default' } source_storage_name { 'default' }
destination_storage_name { 'default' } destination_storage_name { 'default' }
trait :scheduled do
state { ProjectRepositoryStorageMove.state_machines[:state].states[:scheduled].value }
end
end end
end end
...@@ -31,4 +31,33 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do ...@@ -31,4 +31,33 @@ RSpec.describe ProjectRepositoryStorageMove, type: :model do
end end
end 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 end
...@@ -320,8 +320,10 @@ describe Projects::ForkService do ...@@ -320,8 +320,10 @@ describe Projects::ForkService do
allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum) allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum)
.and_return(::Gitlab::Git::BLANK_SHA) .and_return(::Gitlab::Git::BLANK_SHA)
storage_move = project.repository_storage_moves.create!( storage_move = create(
source_storage_name: project.repository_storage, :project_repository_storage_move,
:scheduled,
project: project,
destination_storage_name: 'test_second_storage' destination_storage_name: 'test_second_storage'
) )
Projects::UpdateRepositoryStorageService.new(storage_move).execute Projects::UpdateRepositoryStorageService.new(storage_move).execute
......
...@@ -18,7 +18,7 @@ describe Projects::UpdateRepositoryStorageService do ...@@ -18,7 +18,7 @@ describe Projects::UpdateRepositoryStorageService do
context 'without wiki and design repository' do context 'without wiki and design repository' do
let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: false) } let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: false) }
let(:destination) { 'test_second_storage' } let(:destination) { 'test_second_storage' }
let(:repository_storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: destination) } let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) }
let!(:checksum) { project.repository.checksum } let!(:checksum) { project.repository.checksum }
let(:project_repository_double) { double(:repository) } let(:project_repository_double) { double(:repository) }
...@@ -134,7 +134,7 @@ describe Projects::UpdateRepositoryStorageService do ...@@ -134,7 +134,7 @@ describe Projects::UpdateRepositoryStorageService do
let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: true) } let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: true) }
let(:repository) { project.wiki.repository } let(:repository) { project.wiki.repository }
let(:destination) { 'test_second_storage' } let(:destination) { 'test_second_storage' }
let(:repository_storage_move) { create(:project_repository_storage_move, project: project, destination_storage_name: destination) } let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) }
before do before do
project.create_wiki project.create_wiki
......
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