Commit 0d4ec8cf authored by Steve Abrams's avatar Steve Abrams Committed by David Fernandez

Add a force param to the gitlab registry client

Adds a force param to the gitlab cancel import
client endpoint.

This param is meant as a safety device so admin
users can cancel an import if needed during the Phase
2 migration.
parent 795ae397
...@@ -33,7 +33,17 @@ class ContainerRepository < ApplicationRecord ...@@ -33,7 +33,17 @@ class ContainerRepository < ApplicationRecord
enum status: { delete_scheduled: 0, delete_failed: 1 } enum status: { delete_scheduled: 0, delete_failed: 1 }
enum expiration_policy_cleanup_status: { cleanup_unscheduled: 0, cleanup_scheduled: 1, cleanup_unfinished: 2, cleanup_ongoing: 3 } enum expiration_policy_cleanup_status: { cleanup_unscheduled: 0, cleanup_scheduled: 1, cleanup_unfinished: 2, cleanup_ongoing: 3 }
enum migration_skipped_reason: { not_in_plan: 0, too_many_retries: 1, too_many_tags: 2, root_namespace_in_deny_list: 3, migration_canceled: 4, not_found: 5, native_import: 6 }
enum migration_skipped_reason: {
not_in_plan: 0,
too_many_retries: 1,
too_many_tags: 2,
root_namespace_in_deny_list: 3,
migration_canceled: 4,
not_found: 5,
native_import: 6,
migration_forced_canceled: 7
}
delegate :client, :gitlab_api_client, to: :registry delegate :client, :gitlab_api_client, to: :registry
...@@ -496,6 +506,19 @@ class ContainerRepository < ApplicationRecord ...@@ -496,6 +506,19 @@ class ContainerRepository < ApplicationRecord
gitlab_api_client.cancel_repository_import(self.path) gitlab_api_client.cancel_repository_import(self.path)
end end
# This method is not meant for consumption by the code
# It is meant for manual use in the case that a migration needs to be
# cancelled by an admin or SRE
def force_migration_cancel
return :error unless gitlab_api_client.supports_gitlab_api?
response = gitlab_api_client.cancel_repository_import(self.path, force: true)
skip_import(reason: :migration_forced_canceled) if response[:status] == :ok
response
end
def self.build_from_path(path) def self.build_from_path(path)
self.new(project: path.repository_project, self.new(project: path.repository_project,
name: path.repository_name) name: path.repository_name)
......
...@@ -52,10 +52,12 @@ module ContainerRegistry ...@@ -52,10 +52,12 @@ module ContainerRegistry
IMPORT_RESPONSES.fetch(response.status, :error) IMPORT_RESPONSES.fetch(response.status, :error)
end end
# https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#import-repository # https://gitlab.com/gitlab-org/container-registry/-/blob/master/docs-gitlab/api.md#cancel-repository-import
def cancel_repository_import(path) def cancel_repository_import(path, force: false)
response = with_import_token_faraday do |faraday_client| response = with_import_token_faraday do |faraday_client|
faraday_client.delete(import_url_for(path)) faraday_client.delete(import_url_for(path)) do |req|
req.params['force'] = true if force
end
end end
status = IMPORT_RESPONSES.fetch(response.status, :error) status = IMPORT_RESPONSES.fetch(response.status, :error)
......
...@@ -107,7 +107,9 @@ RSpec.describe ContainerRegistry::GitlabApiClient do ...@@ -107,7 +107,9 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
end end
describe '#cancel_repository_import' do describe '#cancel_repository_import' do
subject { client.cancel_repository_import(path) } let(:force) { false }
subject { client.cancel_repository_import(path, force: force) }
where(:status_code, :expected_result) do where(:status_code, :expected_result) do
200 | :already_imported 200 | :already_imported
...@@ -124,7 +126,7 @@ RSpec.describe ContainerRegistry::GitlabApiClient do ...@@ -124,7 +126,7 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
with_them do with_them do
before do before do
stub_import_cancel(path, status_code) stub_import_cancel(path, status_code, force: force)
end end
it { is_expected.to eq({ status: expected_result, migration_state: nil }) } it { is_expected.to eq({ status: expected_result, migration_state: nil }) }
...@@ -134,11 +136,21 @@ RSpec.describe ContainerRegistry::GitlabApiClient do ...@@ -134,11 +136,21 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
let(:status) { 'this_is_a_test' } let(:status) { 'this_is_a_test' }
before do before do
stub_import_cancel(path, 400, status: status) stub_import_cancel(path, 400, status: status, force: force)
end end
it { is_expected.to eq({ status: :bad_request, migration_state: status }) } it { is_expected.to eq({ status: :bad_request, migration_state: status }) }
end end
context 'force cancel' do
let(:force) { true }
before do
stub_import_cancel(path, 202, force: force)
end
it { is_expected.to eq({ status: :ok, migration_state: nil }) }
end
end end
describe '#import_status' do describe '#import_status' do
...@@ -287,15 +299,24 @@ RSpec.describe ContainerRegistry::GitlabApiClient do ...@@ -287,15 +299,24 @@ RSpec.describe ContainerRegistry::GitlabApiClient do
) )
end end
def stub_import_cancel(path, http_status, status: nil) def stub_import_cancel(path, http_status, status: nil, force: false)
body = {} body = {}
if http_status == 400 if http_status == 400
body = { status: status } body = { status: status }
end end
stub_request(:delete, "#{registry_api_url}/gitlab/v1/import/#{path}/") headers = {
.with(headers: { 'Accept' => described_class::JSON_TYPE, 'Authorization' => "bearer #{import_token}" }) 'Accept' => described_class::JSON_TYPE,
'Authorization' => "bearer #{import_token}",
'User-Agent' => "GitLab/#{Gitlab::VERSION}",
'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3'
}
params = force ? '?force=true' : ''
stub_request(:delete, "#{registry_api_url}/gitlab/v1/import/#{path}/#{params}")
.with(headers: headers)
.to_return( .to_return(
status: http_status, status: http_status,
body: body.to_json, body: body.to_json,
......
...@@ -762,6 +762,61 @@ RSpec.describe ContainerRepository, :aggregate_failures do ...@@ -762,6 +762,61 @@ RSpec.describe ContainerRepository, :aggregate_failures do
it_behaves_like 'gitlab migration client request', :cancel_repository_import it_behaves_like 'gitlab migration client request', :cancel_repository_import
end end
describe '#force_migration_cancel' do
subject { repository.force_migration_cancel }
shared_examples 'returning the same response as the client' do
it 'returns the same response' do
expect(repository.gitlab_api_client)
.to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
expect(subject).to eq(client_response)
end
end
context 'successful cancellation' do
let(:client_response) { { status: :ok } }
it_behaves_like 'returning the same response as the client'
it 'skips the migration' do
expect(repository.gitlab_api_client)
.to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
expect { subject }.to change { repository.reload.migration_state }.to('import_skipped')
.and change { repository.migration_skipped_reason }.to('migration_forced_canceled')
.and change { repository.migration_skipped_at }
end
end
context 'failed cancellation' do
let(:client_response) { { status: :error } }
it_behaves_like 'returning the same response as the client'
it 'does not skip the migration' do
expect(repository.gitlab_api_client)
.to receive(:cancel_repository_import).with(repository.path, force: true).and_return(client_response)
expect { subject }.to not_change { repository.reload.migration_state }
.and not_change { repository.migration_skipped_reason }
.and not_change { repository.migration_skipped_at }
end
end
context 'when the gitlab_api feature is not supported' do
before do
allow(repository.gitlab_api_client).to receive(:supports_gitlab_api?).and_return(false)
end
it 'returns :error' do
expect(repository.gitlab_api_client).not_to receive(:cancel_repository_import)
expect(subject).to eq(:error)
end
end
end
end end
describe '.build_from_path' do describe '.build_from_path' do
......
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