diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-08-22 08:26:24 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-08-22 08:26:24 +0000 |
commit | c0563b8bf6f22fb72bd3dffe660a21c1fba4684e (patch) | |
tree | 4e8c7790296335f28f6bbaa63f9078360fbea37c | |
parent | 539ed0a6375d5bb6d734e688b801373e4b8006f9 (diff) | |
parent | fb9e059a412d95ee3b7442a472ecfd9c67eafd7b (diff) | |
download | gitlab-ce-c0563b8bf6f22fb72bd3dffe660a21c1fba4684e.tar.gz |
Merge branch '28283-uuid-storage' into 'master'
Hashed Storage support for Repositories
Closes #28283
See merge request !13246
21 files changed, 498 insertions, 162 deletions
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 150188f0b65..3b76da238e0 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -116,6 +116,7 @@ module ApplicationSettingsHelper :email_author_in_body, :enabled_git_access_protocol, :gravatar_enabled, + :hashed_storage_enabled, :help_page_hide_commercial_content, :help_page_support_url, :help_page_text, diff --git a/app/models/concerns/storage/legacy_project.rb b/app/models/concerns/storage/legacy_project.rb deleted file mode 100644 index 815db712285..00000000000 --- a/app/models/concerns/storage/legacy_project.rb +++ /dev/null @@ -1,76 +0,0 @@ -module Storage - module LegacyProject - extend ActiveSupport::Concern - - def disk_path - full_path - end - - def ensure_storage_path_exist - gitlab_shell.add_namespace(repository_storage_path, namespace.full_path) - end - - def rename_repo - path_was = previous_changes['path'].first - old_path_with_namespace = File.join(namespace.full_path, path_was) - new_path_with_namespace = File.join(namespace.full_path, path) - - Rails.logger.error "Attempting to rename #{old_path_with_namespace} -> #{new_path_with_namespace}" - - if has_container_registry_tags? - Rails.logger.error "Project #{old_path_with_namespace} cannot be renamed because container registry tags are present!" - - # we currently doesn't support renaming repository if it contains images in container registry - raise StandardError.new('Project cannot be renamed, because images are present in its container registry') - end - - expire_caches_before_rename(old_path_with_namespace) - - if gitlab_shell.mv_repository(repository_storage_path, old_path_with_namespace, new_path_with_namespace) - # If repository moved successfully we need to send update instructions to users. - # However we cannot allow rollback since we moved repository - # So we basically we mute exceptions in next actions - begin - gitlab_shell.mv_repository(repository_storage_path, "#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki") - send_move_instructions(old_path_with_namespace) - expires_full_path_cache - - @old_path_with_namespace = old_path_with_namespace - - SystemHooksService.new.execute_hooks_for(self, :rename) - - @repository = nil - rescue => e - Rails.logger.error "Exception renaming #{old_path_with_namespace} -> #{new_path_with_namespace}: #{e}" - # Returning false does not rollback after_* transaction but gives - # us information about failing some of tasks - false - end - else - Rails.logger.error "Repository could not be renamed: #{old_path_with_namespace} -> #{new_path_with_namespace}" - - # if we cannot move namespace directory we should rollback - # db changes in order to prevent out of sync between db and fs - raise StandardError.new('repository cannot be renamed') - end - - Gitlab::AppLogger.info "Project was renamed: #{old_path_with_namespace} -> #{new_path_with_namespace}" - - Gitlab::UploadsTransfer.new.rename_project(path_was, path, namespace.full_path) - Gitlab::PagesTransfer.new.rename_project(path_was, path, namespace.full_path) - end - - def create_repository(force: false) - # Forked import is handled asynchronously - return if forked? && !force - - if gitlab_shell.add_repository(repository_storage_path, path_with_namespace) - repository.after_create - true - else - errors.add(:base, 'Failed to create repository via gitlab-shell') - false - end - end - end -end diff --git a/app/models/project.rb b/app/models/project.rb index be248bc99e1..37f4dd08355 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -17,7 +17,6 @@ class Project < ActiveRecord::Base include ProjectFeaturesCompatibility include SelectForProjectAuthorization include Routable - include Storage::LegacyProject extend Gitlab::ConfigHelper @@ -25,6 +24,7 @@ class Project < ActiveRecord::Base NUMBER_OF_PERMITTED_BOARDS = 1 UNKNOWN_IMPORT_URL = 'http://unknown.git'.freeze + LATEST_STORAGE_VERSION = 1 cache_markdown_field :description, pipeline: :description @@ -32,6 +32,8 @@ class Project < ActiveRecord::Base :merge_requests_enabled?, :issues_enabled?, to: :project_feature, allow_nil: true + delegate :base_dir, :disk_path, :ensure_storage_path_exists, to: :storage + default_value_for :archived, false default_value_for :visibility_level, gitlab_config_features.visibility_level default_value_for :container_registry_enabled, gitlab_config_features.container_registry @@ -44,32 +46,24 @@ class Project < ActiveRecord::Base default_value_for :snippets_enabled, gitlab_config_features.snippets default_value_for :only_allow_merge_if_all_discussions_are_resolved, false - after_create :ensure_storage_path_exist - after_create :create_project_feature, unless: :project_feature - after_save :update_project_statistics, if: :namespace_id_changed? + add_authentication_token_field :runners_token + before_save :ensure_runners_token - # set last_activity_at to the same as created_at + after_save :update_project_statistics, if: :namespace_id_changed? + after_create :create_project_feature, unless: :project_feature after_create :set_last_activity_at - def set_last_activity_at - update_column(:last_activity_at, self.created_at) - end - after_create :set_last_repository_updated_at - def set_last_repository_updated_at - update_column(:last_repository_updated_at, self.created_at) - end + after_update :update_forks_visibility_level before_destroy :remove_private_deploy_keys after_destroy -> { run_after_commit { remove_pages } } - # update visibility_level of forks - after_update :update_forks_visibility_level - after_validation :check_pending_delete - # Legacy Storage specific hooks - - after_save :ensure_storage_path_exist, if: :namespace_id_changed? + # Storage specific hooks + after_initialize :use_hashed_storage + after_create :ensure_storage_path_exists + after_save :ensure_storage_path_exists, if: :namespace_id_changed? acts_as_taggable @@ -238,9 +232,6 @@ class Project < ActiveRecord::Base presence: true, inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } - add_authentication_token_field :runners_token - before_save :ensure_runners_token - mount_uploader :avatar, AvatarUploader has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent @@ -487,6 +478,10 @@ class Project < ActiveRecord::Base @repository ||= Repository.new(full_path, self, disk_path: disk_path) end + def reload_repository! + @repository = nil + end + def container_registry_url if Gitlab.config.registry.enabled "#{Gitlab.config.registry.host_port}/#{full_path.downcase}" @@ -1004,6 +999,19 @@ class Project < ActiveRecord::Base end end + def create_repository(force: false) + # Forked import is handled asynchronously + return if forked? && !force + + if gitlab_shell.add_repository(repository_storage_path, disk_path) + repository.after_create + true + else + errors.add(:base, 'Failed to create repository via gitlab-shell') + false + end + end + def hook_attrs(backward: true) attrs = { name: name, @@ -1086,6 +1094,7 @@ class Project < ActiveRecord::Base !!repository.exists? end + # update visibility_level of forks def update_forks_visibility_level return unless visibility_level < visibility_level_was @@ -1213,7 +1222,8 @@ class Project < ActiveRecord::Base end def pages_path - File.join(Settings.pages.path, disk_path) + # TODO: when we migrate Pages to work with new storage types, change here to use disk_path + File.join(Settings.pages.path, full_path) end def public_pages_path @@ -1252,6 +1262,50 @@ class Project < ActiveRecord::Base end end + def rename_repo + new_full_path = build_full_path + + Rails.logger.error "Attempting to rename #{full_path_was} -> #{new_full_path}" + + if has_container_registry_tags? + Rails.logger.error "Project #{full_path_was} cannot be renamed because container registry tags are present!" + + # we currently doesn't support renaming repository if it contains images in container registry + raise StandardError.new('Project cannot be renamed, because images are present in its container registry') + end + + expire_caches_before_rename(full_path_was) + + if storage.rename_repo + Gitlab::AppLogger.info "Project was renamed: #{full_path_was} -> #{new_full_path}" + rename_repo_notify! + after_rename_repo + else + Rails.logger.error "Repository could not be renamed: #{full_path_was} -> #{new_full_path}" + + # if we cannot move namespace directory we should rollback + # db changes in order to prevent out of sync between db and fs + raise StandardError.new('repository cannot be renamed') + end + end + + def rename_repo_notify! + send_move_instructions(full_path_was) + expires_full_path_cache + + self.old_path_with_namespace = full_path_was + SystemHooksService.new.execute_hooks_for(self, :rename) + + reload_repository! + end + + def after_rename_repo + path_before_change = previous_changes['path'].first + + Gitlab::UploadsTransfer.new.rename_project(path_before_change, self.path, namespace.full_path) + Gitlab::PagesTransfer.new.rename_project(path_before_change, self.path, namespace.full_path) + end + def running_or_pending_build_count(force: false) Rails.cache.fetch(['projects', id, 'running_or_pending_build_count'], force: force) do builds.running_or_pending.count(:all) @@ -1410,6 +1464,10 @@ class Project < ActiveRecord::Base end end + def full_path_was + File.join(namespace.full_path, previous_changes['path'].first) + end + alias_method :name_with_namespace, :full_name alias_method :human_name, :full_name # @deprecated cannot remove yet because it has an index with its name in elasticsearch @@ -1419,8 +1477,36 @@ class Project < ActiveRecord::Base Projects::ForksCountService.new(self).count end + def legacy_storage? + self.storage_version.nil? + end + private + def storage + @storage ||= + if self.storage_version && self.storage_version >= 1 + Storage::HashedProject.new(self) + else + Storage::LegacyProject.new(self) + end + end + + def use_hashed_storage + if self.new_record? && current_application_settings.hashed_storage_enabled + self.storage_version = LATEST_STORAGE_VERSION + end + end + + # set last_activity_at to the same as created_at + def set_last_activity_at + update_column(:last_activity_at, self.created_at) + end + + def set_last_repository_updated_at + update_column(:last_repository_updated_at, self.created_at) + end + def cross_namespace_reference?(from) case from when Project diff --git a/app/models/storage/hashed_project.rb b/app/models/storage/hashed_project.rb new file mode 100644 index 00000000000..fae1b64961a --- /dev/null +++ b/app/models/storage/hashed_project.rb @@ -0,0 +1,42 @@ +module Storage + class HashedProject + attr_accessor :project + delegate :gitlab_shell, :repository_storage_path, to: :project + + ROOT_PATH_PREFIX = '@hashed'.freeze + + def initialize(project) + @project = project + end + + # Base directory + # + # @return [String] directory where repository is stored + def base_dir + "#{ROOT_PATH_PREFIX}/#{disk_hash[0..1]}/#{disk_hash[2..3]}" if disk_hash + end + + # Disk path is used to build repository and project's wiki path on disk + # + # @return [String] combination of base_dir and the repository own name without `.git` or `.wiki.git` extensions + def disk_path + "#{base_dir}/#{disk_hash}" if disk_hash + end + + def ensure_storage_path_exists + gitlab_shell.add_namespace(repository_storage_path, base_dir) + end + + def rename_repo + true + end + + private + + # Generates the hash for the project path and name on disk + # If you need to refer to the repository on disk, use the `#disk_path` + def disk_hash + @disk_hash ||= Digest::SHA2.hexdigest(project.id.to_s) if project.id + end + end +end diff --git a/app/models/storage/legacy_project.rb b/app/models/storage/legacy_project.rb new file mode 100644 index 00000000000..9d9e5e1d352 --- /dev/null +++ b/app/models/storage/legacy_project.rb @@ -0,0 +1,51 @@ +module Storage + class LegacyProject + attr_accessor :project + delegate :namespace, :gitlab_shell, :repository_storage_path, to: :project + + def initialize(project) + @project = project + end + + # Base directory + # + # @return [String] directory where repository is stored + def base_dir + namespace.full_path + end + + # Disk path is used to build repository and project's wiki path on disk + # + # @return [String] combination of base_dir and the repository own name without `.git` or `.wiki.git` extensions + def disk_path + project.full_path + end + + def ensure_storage_path_exists + return unless namespace + + gitlab_shell.add_namespace(repository_storage_path, base_dir) + end + + def rename_repo + new_full_path = project.build_full_path + + if gitlab_shell.mv_repository(repository_storage_path, project.full_path_was, new_full_path) + # If repository moved successfully we need to send update instructions to users. + # However we cannot allow rollback since we moved repository + # So we basically we mute exceptions in next actions + begin + gitlab_shell.mv_repository(repository_storage_path, "#{project.full_path_was}.wiki", "#{new_full_path}.wiki") + return true + rescue => e + Rails.logger.error "Exception renaming #{project.full_path_was} -> #{new_full_path}: #{e}" + # Returning false does not rollback after_* transaction but gives + # us information about failing some of tasks + return false + end + end + + false + end + end +end diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index f565612a89d..e3f9d9ee95d 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -13,7 +13,7 @@ module Groups # Execute the destruction of the models immediately to ensure atomic cleanup. # Skip repository removal because we remove directory with namespace # that contain all these repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: project.legacy_storage?).execute end group.children.each do |group| diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 9d7237c2fbb..8e20de8dfa5 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -35,16 +35,18 @@ module Users Groups::DestroyService.new(group, current_user).execute end + namespace = user.namespace + namespace.prepare_for_destroy + user.personal_projects.each do |project| # Skip repository removal because we remove directory with namespace # that contain all this repositories - ::Projects::DestroyService.new(project, current_user, skip_repo: true).execute + ::Projects::DestroyService.new(project, current_user, skip_repo: project.legacy_storage?).execute end MigrateToGhostUserService.new(user).execute unless options[:hard_delete] # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing - namespace = user.namespace user_data = user.destroy namespace.really_destroy! diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index dc585054316..959af5c0d13 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -493,6 +493,16 @@ %fieldset %legend Repository Storage .form-group + .col-sm-offset-2.col-sm-10 + .checkbox + = f.label :hashed_storage_enabled do + = f.check_box :hashed_storage_enabled + Create new projects using hashed storage paths + .help-block + Enable immutable, hash-based paths and repository names to store repositories on disk. This prevents + repositories from having to be moved or renamed when the Project URL changes and may improve disk I/O performance. + %em (EXPERIMENTAL) + .form-group = f.label :repository_storages, 'Storage paths for new projects', class: 'control-label col-sm-2' .col-sm-10 = f.select :repository_storages, repository_storages_options_for_select, {include_hidden: false}, multiple: true, class: 'form-control' @@ -501,6 +511,7 @@ = succeed "." do = link_to "repository storages documentation", help_page_path("administration/repository_storages") + %fieldset %legend Repository Checks .form-group diff --git a/changelogs/unreleased/28283-uuid-storage.yml b/changelogs/unreleased/28283-uuid-storage.yml new file mode 100644 index 00000000000..283e06d4b7f --- /dev/null +++ b/changelogs/unreleased/28283-uuid-storage.yml @@ -0,0 +1,4 @@ +--- +title: Hashed Storage support for Repositories (EXPERIMENTAL) +merge_request: 13246 +author: diff --git a/db/migrate/20170802013652_add_storage_fields_to_project.rb b/db/migrate/20170802013652_add_storage_fields_to_project.rb new file mode 100644 index 00000000000..c2381a9d0b2 --- /dev/null +++ b/db/migrate/20170802013652_add_storage_fields_to_project.rb @@ -0,0 +1,16 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddStorageFieldsToProject < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + add_column :projects, :storage_version, :integer, limit: 2 + end + + def down + remove_column :projects, :storage_version + end +end diff --git a/db/migrate/20170807071105_add_hashed_storage_to_settings.rb b/db/migrate/20170807071105_add_hashed_storage_to_settings.rb new file mode 100644 index 00000000000..0846557add8 --- /dev/null +++ b/db/migrate/20170807071105_add_hashed_storage_to_settings.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddHashedStorageToSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :application_settings, :hashed_storage_enabled, :boolean, default: false + end + + def down + remove_columns :application_settings, :hashed_storage_enabled + end +end diff --git a/db/schema.rb b/db/schema.rb index c31bff3a8f2..cd488630237 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -128,6 +128,7 @@ ActiveRecord::Schema.define(version: 20170820100558) do t.integer "performance_bar_allowed_group_id" t.boolean "password_authentication_enabled" t.boolean "project_export_enabled", default: true, null: false + t.boolean "hashed_storage_enabled", default: false, null: false end create_table "audit_events", force: :cascade do |t| @@ -1208,6 +1209,7 @@ ActiveRecord::Schema.define(version: 20170820100558) do t.datetime "last_repository_updated_at" t.string "ci_config_path" t.text "delete_error" + t.integer "storage_version", limit: 2 end add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree diff --git a/lib/backup/repository.rb b/lib/backup/repository.rb index 88821ae56e0..4e92be85110 100644 --- a/lib/backup/repository.rb +++ b/lib/backup/repository.rb @@ -75,7 +75,7 @@ module Backup path_to_project_repo = path_to_repo(project) path_to_project_bundle = path_to_bundle(project) - project.ensure_storage_path_exist + project.ensure_storage_path_exists cmd = if File.exist?(path_to_project_bundle) %W(#{Gitlab.config.git.bin_path} clone --bare #{path_to_project_bundle} #{path_to_project_repo}) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 9d9ebcb389a..894950e341f 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -98,6 +98,7 @@ excluded_attributes: - :last_activity_at - :last_repository_updated_at - :last_repository_check_at + - :storage_version snippets: - :expired_at merge_request_diff: diff --git a/lib/tasks/gitlab/import.rake b/lib/tasks/gitlab/import.rake index 48bd9139ce8..6e10ba374bf 100644 --- a/lib/tasks/gitlab/import.rake +++ b/lib/tasks/gitlab/import.rake @@ -11,6 +11,12 @@ namespace :gitlab do # desc "GitLab | Import bare repositories from repositories -> storages into GitLab project instance" task repos: :environment do + if Project.current_application_settings.hashed_storage_enabled + puts 'Cannot import repositories when Hashed Storage is enabled'.color(:red) + + exit 1 + end + Gitlab.config.repositories.storages.each_value do |repository_storage| git_base_path = repository_storage['path'] repos_to_import = Dir.glob(git_base_path + '/**/*.git') diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 4a2034b31b3..9ebda0ba03b 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -81,6 +81,10 @@ FactoryGirl.define do archived true end + trait :hashed do + storage_version Project::LATEST_STORAGE_VERSION + end + trait :access_requestable do request_access_enabled true end diff --git a/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb b/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb index 12cac1d033d..b47f3314926 100644 --- a/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb +++ b/spec/migrations/cleanup_namespaceless_pending_delete_projects_spec.rb @@ -4,7 +4,7 @@ require Rails.root.join('db', 'post_migrate', '20170502101023_cleanup_namespacel describe CleanupNamespacelessPendingDeleteProjects do before do # Stub after_save callbacks that will fail when Project has no namespace - allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil) + allow_any_instance_of(Project).to receive(:ensure_storage_path_exists).and_return(nil) allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil) end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5e60511f3a8..2e613c44357 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1251,60 +1251,6 @@ describe Project do end end - describe '#rename_repo' do - let(:project) { create(:project, :repository) } - let(:gitlab_shell) { Gitlab::Shell.new } - - before do - # Project#gitlab_shell returns a new instance of Gitlab::Shell on every - # call. This makes testing a bit easier. - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - allow(project).to receive(:previous_changes).and_return('path' => ['foo']) - end - - it 'renames a repository' do - stub_container_registry_config(enabled: false) - - expect(gitlab_shell).to receive(:mv_repository) - .ordered - .with(project.repository_storage_path, "#{project.namespace.full_path}/foo", "#{project.full_path}") - .and_return(true) - - expect(gitlab_shell).to receive(:mv_repository) - .ordered - .with(project.repository_storage_path, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") - .and_return(true) - - expect_any_instance_of(SystemHooksService) - .to receive(:execute_hooks_for) - .with(project, :rename) - - expect_any_instance_of(Gitlab::UploadsTransfer) - .to receive(:rename_project) - .with('foo', project.path, project.namespace.full_path) - - expect(project).to receive(:expire_caches_before_rename) - - expect(project).to receive(:expires_full_path_cache) - - project.rename_repo - end - - context 'container registry with images' do - let(:container_repository) { create(:container_repository) } - - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags(repository: :any, tags: ['tag']) - project.container_repositories << container_repository - end - - subject { project.rename_repo } - - it { expect {subject}.to raise_error(StandardError) } - end - end - describe '#expire_caches_before_rename' do let(:project) { create(:project, :repository) } let(:repo) { double(:repo, exists?: true) } @@ -2367,4 +2313,181 @@ describe Project do expect(project.forks_count).to eq(1) end end + + context 'legacy storage' do + let(:project) { create(:project, :repository) } + let(:gitlab_shell) { Gitlab::Shell.new } + + before do + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + end + + describe '#base_dir' do + it 'returns base_dir based on namespace only' do + expect(project.base_dir).to eq(project.namespace.full_path) + end + end + + describe '#disk_path' do + it 'returns disk_path based on namespace and project path' do + expect(project.disk_path).to eq("#{project.namespace.full_path}/#{project.path}") + end + end + + describe '#ensure_storage_path_exists' do + it 'delegates to gitlab_shell to ensure namespace is created' do + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, project.base_dir) + + project.ensure_storage_path_exists + end + end + + describe '#legacy_storage?' do + it 'returns true when storage_version is nil' do + project = build(:project) + + expect(project.legacy_storage?).to be_truthy + end + end + + describe '#rename_repo' do + before do + # Project#gitlab_shell returns a new instance of Gitlab::Shell on every + # call. This makes testing a bit easier. + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + end + + it 'renames a repository' do + stub_container_registry_config(enabled: false) + + expect(gitlab_shell).to receive(:mv_repository) + .ordered + .with(project.repository_storage_path, "#{project.namespace.full_path}/foo", "#{project.full_path}") + .and_return(true) + + expect(gitlab_shell).to receive(:mv_repository) + .ordered + .with(project.repository_storage_path, "#{project.namespace.full_path}/foo.wiki", "#{project.full_path}.wiki") + .and_return(true) + + expect_any_instance_of(SystemHooksService) + .to receive(:execute_hooks_for) + .with(project, :rename) + + expect_any_instance_of(Gitlab::UploadsTransfer) + .to receive(:rename_project) + .with('foo', project.path, project.namespace.full_path) + + expect(project).to receive(:expire_caches_before_rename) + + expect(project).to receive(:expires_full_path_cache) + + project.rename_repo + end + + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository + end + + subject { project.rename_repo } + + it { expect { subject }.to raise_error(StandardError) } + end + end + + describe '#pages_path' do + it 'returns a path where pages are stored' do + expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) + end + end + end + + context 'hashed storage' do + let(:project) { create(:project, :repository) } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + + before do + stub_application_setting(hashed_storage_enabled: true) + allow(Digest::SHA2).to receive(:hexdigest) { hash } + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + end + + describe '#base_dir' do + it 'returns base_dir based on hash of project id' do + expect(project.base_dir).to eq('@hashed/6b/86') + end + end + + describe '#disk_path' do + it 'returns disk_path based on hash of project id' do + hashed_path = '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' + + expect(project.disk_path).to eq(hashed_path) + end + end + + describe '#ensure_storage_path_exists' do + it 'delegates to gitlab_shell to ensure namespace is created' do + expect(gitlab_shell).to receive(:add_namespace).with(project.repository_storage_path, '@hashed/6b/86') + + project.ensure_storage_path_exists + end + end + + describe '#rename_repo' do + before do + # Project#gitlab_shell returns a new instance of Gitlab::Shell on every + # call. This makes testing a bit easier. + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + allow(project).to receive(:previous_changes).and_return('path' => ['foo']) + end + + it 'renames a repository' do + stub_container_registry_config(enabled: false) + + expect(gitlab_shell).not_to receive(:mv_repository) + + expect_any_instance_of(SystemHooksService) + .to receive(:execute_hooks_for) + .with(project, :rename) + + expect_any_instance_of(Gitlab::UploadsTransfer) + .to receive(:rename_project) + .with('foo', project.path, project.namespace.full_path) + + expect(project).to receive(:expire_caches_before_rename) + + expect(project).to receive(:expires_full_path_cache) + + project.rename_repo + end + + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } + + before do + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository + end + + subject { project.rename_repo } + + it { expect { subject }.to raise_error(StandardError) } + end + end + + describe '#pages_path' do + it 'returns a path where pages are stored' do + expect(project.pages_path).to eq(File.join(Settings.pages.path, project.namespace.full_path, project.path)) + end + end + end end diff --git a/spec/services/groups/destroy_service_spec.rb b/spec/services/groups/destroy_service_spec.rb index 1b2ce3cd03e..ac4b9c02ba7 100644 --- a/spec/services/groups/destroy_service_spec.rb +++ b/spec/services/groups/destroy_service_spec.rb @@ -8,8 +8,8 @@ describe Groups::DestroyService do let!(:nested_group) { create(:group, parent: group) } let!(:project) { create(:project, namespace: group) } let!(:notification_setting) { create(:notification_setting, source: group)} - let!(:gitlab_shell) { Gitlab::Shell.new } - let!(:remove_path) { group.path + "+#{group.id}+deleted" } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:remove_path) { group.path + "+#{group.id}+deleted" } before do group.add_user(user, Gitlab::Access::OWNER) @@ -134,4 +134,26 @@ describe Groups::DestroyService do it_behaves_like 'group destruction', false end + + describe 'repository removal' do + before do + destroy_group(group, user, false) + end + + context 'legacy storage' do + let!(:project) { create(:project, :empty_repo, namespace: group) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + + context 'hashed storage' do + let!(:project) { create(:project, :hashed, :empty_repo, namespace: group) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + end end diff --git a/spec/services/users/destroy_service_spec.rb b/spec/services/users/destroy_service_spec.rb index a82567f6f43..58a5bede3de 100644 --- a/spec/services/users/destroy_service_spec.rb +++ b/spec/services/users/destroy_service_spec.rb @@ -4,9 +4,10 @@ describe Users::DestroyService do describe "Deletes a user and all their personal projects" do let!(:user) { create(:user) } let!(:admin) { create(:admin) } - let!(:namespace) { create(:namespace, owner: user) } + let!(:namespace) { user.namespace } let!(:project) { create(:project, namespace: namespace) } let(:service) { described_class.new(admin) } + let(:gitlab_shell) { Gitlab::Shell.new } context 'no options are given' do it 'deletes the user' do @@ -14,7 +15,7 @@ describe Users::DestroyService do expect { user_data['email'].to eq(user.email) } expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) - expect { Namespace.with_deleted.find(user.namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.with_deleted.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) end it 'will delete the project' do @@ -165,5 +166,27 @@ describe Users::DestroyService do expect(Issue.exists?(issue.id)).to be_falsy end end + + describe "user personal's repository removal" do + before do + Sidekiq::Testing.inline! { service.execute(user) } + end + + context 'legacy storage' do + let!(:project) { create(:project, :empty_repo, namespace: user.namespace) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + + context 'hashed storage' do + let!(:project) { create(:project, :empty_repo, :hashed, namespace: user.namespace) } + + it 'removes repository' do + expect(gitlab_shell.exists?(project.repository_storage_path, "#{project.disk_path}.git")).to be_falsey + end + end + end end end diff --git a/spec/workers/namespaceless_project_destroy_worker_spec.rb b/spec/workers/namespaceless_project_destroy_worker_spec.rb index f2706254284..817e103fd9a 100644 --- a/spec/workers/namespaceless_project_destroy_worker_spec.rb +++ b/spec/workers/namespaceless_project_destroy_worker_spec.rb @@ -5,7 +5,7 @@ describe NamespacelessProjectDestroyWorker do before do # Stub after_save callbacks that will fail when Project has no namespace - allow_any_instance_of(Project).to receive(:ensure_storage_path_exist).and_return(nil) + allow_any_instance_of(Project).to receive(:ensure_storage_path_exists).and_return(nil) allow_any_instance_of(Project).to receive(:update_project_statistics).and_return(nil) end |