summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/services/projects/destroy_service.rb9
-rw-r--r--app/workers/project_destroy_worker.rb3
-rw-r--r--db/migrate/20170428064307_add_column_delete_error_to_projects.rb31
-rw-r--r--db/schema.rb1
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/services/projects/destroy_service_spec.rb63
6 files changed, 104 insertions, 4 deletions
diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb
index e2b2660ea71..682407ac896 100644
--- a/app/services/projects/destroy_service.rb
+++ b/app/services/projects/destroy_service.rb
@@ -26,9 +26,6 @@ module Projects
Projects::UnlinkForkService.new(project, current_user).execute
Project.transaction do
- project.team.truncate
- project.destroy!
-
unless remove_legacy_registry_tags
raise_error('Failed to remove some tags in project container registry. Please try again or contact administrator.')
end
@@ -40,10 +37,14 @@ module Projects
unless remove_repository(wiki_path)
raise_error('Failed to remove wiki repository. Please try again or contact administrator.')
end
+
+ project.team.truncate
+ project.destroy!
end
- log_info("Project \"#{project.path_with_namespace}\" was removed")
system_hook_service.execute_hooks_for(project, :destroy)
+
+ log_info("Project \"#{project.path_with_namespace}\" was removed")
true
end
diff --git a/app/workers/project_destroy_worker.rb b/app/workers/project_destroy_worker.rb
index b462327490e..482e1e38cd1 100644
--- a/app/workers/project_destroy_worker.rb
+++ b/app/workers/project_destroy_worker.rb
@@ -12,5 +12,8 @@ class ProjectDestroyWorker
user = User.find(user_id)
::Projects::DestroyService.new(project, user, params.symbolize_keys).execute
+ rescue StandardError => error
+ project.assign_attributes(delete_error: error.message, pending_delete: false)
+ project.save!(validate: false)
end
end
diff --git a/db/migrate/20170428064307_add_column_delete_error_to_projects.rb b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
new file mode 100644
index 00000000000..ef5fc2cdea5
--- /dev/null
+++ b/db/migrate/20170428064307_add_column_delete_error_to_projects.rb
@@ -0,0 +1,31 @@
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddColumnDeleteErrorToProjects < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ # Set this constant to true if this migration requires downtime.
+ DOWNTIME = false
+
+ # When a migration requires downtime you **must** uncomment the following
+ # constant and define a short and easy to understand explanation as to why the
+ # migration requires downtime.
+ # DOWNTIME_REASON = ''
+
+ # When using the methods "add_concurrent_index", "remove_concurrent_index" or
+ # "add_column_with_default" you must disable the use of transactions
+ # as these methods can not run in an existing transaction.
+ # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
+ # that either of them is the _only_ method called in the migration,
+ # any other changes should go in a separate migration.
+ # This ensures that upon failure _only_ the index creation or removing fails
+ # and can be retried or reverted easily.
+ #
+ # To disable transactions uncomment the following line and remove these
+ # comments:
+ # disable_ddl_transaction!
+
+ def change
+ add_column :projects, :delete_error, :text
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 284b2068166..0ba2bd31517 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1134,6 +1134,7 @@ ActiveRecord::Schema.define(version: 20170717150329) do
t.integer "cached_markdown_version"
t.datetime "last_repository_updated_at"
t.string "ci_config_path"
+ t.text "delete_error"
end
add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 4ef3db3721f..0f2db3380a7 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -396,6 +396,7 @@ Project:
- build_allow_git_fetch
- last_repository_updated_at
- ci_config_path
+- delete_error
Author:
- name
ProjectFeature:
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index b399d3402fd..a629afe723d 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -36,6 +36,27 @@ describe Projects::DestroyService, services: true do
end
end
+ shared_examples 'handles errors thrown during async destroy' do |error_message|
+ it 'does not allow the error to bubble up' do
+ expect do
+ Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+ end.not_to raise_error
+ end
+
+ it 'unmarks the project as "pending deletion"' do
+ Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+
+ expect(project.reload.pending_delete).to be(false)
+ end
+
+ it 'stores an error message in `projects.delete_error`' do
+ Sidekiq::Testing.inline! { destroy_project(project, user, {}) }
+
+ expect(project.reload.delete_error).to be_present
+ expect(project.delete_error).to include(error_message)
+ end
+ end
+
context 'Sidekiq inline' do
before do
# Run sidekiq immediatly to check that renamed repository will be removed
@@ -89,10 +110,52 @@ describe Projects::DestroyService, services: true do
end
it_behaves_like 'deleting the project with pipeline and build'
+
+ context 'errors' do
+ context 'when `remove_legacy_registry_tags` fails' do
+ before do
+ expect_any_instance_of(Projects::DestroyService)
+ .to receive(:remove_legacy_registry_tags).and_return(false)
+ end
+
+ it_behaves_like 'handles errors thrown during async destroy', "Failed to remove some tags"
+ end
+
+ context 'when `remove_repository` fails' do
+ before do
+ expect_any_instance_of(Projects::DestroyService)
+ .to receive(:remove_repository).and_return(false)
+ end
+
+ it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository"
+ end
+
+ context 'when `execute` raises any other error' do
+ before do
+ expect_any_instance_of(Projects::DestroyService)
+ .to receive(:execute).and_raise(ArgumentError.new("Other error message"))
+ end
+
+ it_behaves_like 'handles errors thrown during async destroy', "Other error message"
+ end
+ end
end
context 'with execute' do
it_behaves_like 'deleting the project with pipeline and build'
+
+ context 'when `execute` raises an error' do
+ before do
+ expect_any_instance_of(Projects::DestroyService)
+ .to receive(:execute).and_raise(ArgumentError)
+ end
+
+ it 'allows the error to bubble up' do
+ expect do
+ Sidekiq::Testing.inline! { Projects::DestroyService.new(project, user, {}).execute }
+ end.to raise_error(ArgumentError)
+ end
+ end
end
describe 'container registry' do