diff options
author | Valery Sizov <valery@gitlab.com> | 2018-08-03 17:34:28 +0300 |
---|---|---|
committer | Douglas Barbosa Alexandre <dbalexandre@gmail.com> | 2018-08-03 17:34:28 +0300 |
commit | 10df0eb7cb57a6aefcb869e64e33a1a9a204e349 (patch) | |
tree | 05be6e9fa7cfc709ac063e0bcce067030190473b /app | |
parent | 7ae5879bdda17a5dd9d0e02fb47371bfc8270c71 (diff) |
Resolve "Hashed storage: extend "Enable hashed storage for all new projects" to "for all new and renamed projects""
Diffstat (limited to 'app')
7 files changed, 108 insertions, 84 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index 16d63639141..02e956911a9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1568,47 +1568,33 @@ class Project < ActiveRecord::Base end def rename_repo - new_full_path = build_full_path + path_before = previous_changes['path'].first + full_path_before = full_path_was + full_path_after = build_full_path - Rails.logger.error "Attempting to rename #{full_path_was} -> #{new_full_path}" + Gitlab::AppLogger.info("Attempting to rename #{full_path_was} -> #{full_path_after}") if has_container_registry_tags? - Rails.logger.error "Project #{full_path_was} cannot be renamed because container registry tags are present!" + Gitlab::AppLogger.info("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 + # we currently don'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) + expire_caches_before_rename(full_path_before) - if storage.rename_repo - Gitlab::AppLogger.info "Project was renamed: #{full_path_was} -> #{new_full_path}" - rename_repo_notify! - after_rename_repo + if rename_or_migrate_repository! + Gitlab::AppLogger.info("Project was renamed: #{full_path_before} -> #{full_path_after}") + after_rename_repository(full_path_before, path_before) else - Rails.logger.error "Repository could not be renamed: #{full_path_was} -> #{new_full_path}" + Gitlab::AppLogger.info("Repository could not be renamed: #{full_path_before} -> #{full_path_after}") # 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') + raise StandardError.new('Repository cannot be renamed') end end - def after_rename_repo - write_repository_config - - path_before_change = previous_changes['path'].first - - # We need to check if project had been rolled out to move resource to hashed storage or not and decide - # if we need execute any take action or no-op. - - unless hashed_storage?(:attachments) - Gitlab::UploadsTransfer.new.rename_project(path_before_change, self.path, namespace.full_path) - end - - Gitlab::PagesTransfer.new.rename_project(path_before_change, self.path, namespace.full_path) - end - def write_repository_config(gl_full_path: full_path) # We'd need to keep track of project full path otherwise directory tree # created with hashed storage enabled cannot be usefully imported using @@ -1619,17 +1605,6 @@ class Project < ActiveRecord::Base nil end - def rename_repo_notify! - # When we import a project overwriting the original project, there - # is a move operation. In that case we don't want to send the instructions. - send_move_instructions(full_path_was) unless import_started? - - self.old_path_with_namespace = full_path_was - SystemHooksService.new.execute_hooks_for(self, :rename) - - reload_repository! - end - def after_import repository.after_import wiki.repository.after_import @@ -2054,6 +2029,39 @@ class Project < ActiveRecord::Base private + def rename_or_migrate_repository! + if Gitlab::CurrentSettings.hashed_storage_enabled? && storage_version != LATEST_STORAGE_VERSION + ::Projects::HashedStorageMigrationService.new(self, full_path_was).execute + else + storage.rename_repo + end + end + + def after_rename_repository(full_path_before, path_before) + execute_rename_repository_hooks!(full_path_before) + + write_repository_config + + # We need to check if project had been rolled out to move resource to hashed storage or not and decide + # if we need execute any take action or no-op. + unless hashed_storage?(:attachments) + Gitlab::UploadsTransfer.new.rename_project(path_before, self.path, namespace.full_path) + end + + Gitlab::PagesTransfer.new.rename_project(path_before, self.path, namespace.full_path) + end + + def execute_rename_repository_hooks!(full_path_before) + # When we import a project overwriting the original project, there + # is a move operation. In that case we don't want to send the instructions. + send_move_instructions(full_path_before) unless import_started? + + self.old_path_with_namespace = full_path_before + SystemHooksService.new.execute_hooks_for(self, :rename) + + reload_repository! + end + def storage @storage ||= if hashed_storage?(:repository) diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb index 649c916a593..a1f0302aeb7 100644 --- a/app/services/projects/hashed_storage/migrate_attachments_service.rb +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -5,18 +5,20 @@ module Projects AttachmentMigrationError = Class.new(StandardError) class MigrateAttachmentsService < BaseService - attr_reader :logger, :old_path, :new_path + attr_reader :logger, :old_disk_path, :new_disk_path - def initialize(project, logger = nil) + def initialize(project, old_disk_path, logger: nil) @project = project @logger = logger || Rails.logger + @old_disk_path = old_disk_path + @new_disk_path = project.disk_path end def execute - @old_path = project.full_path - @new_path = project.disk_path - origin = FileUploader.absolute_base_dir(project) + # It's possible that old_disk_path does not match project.disk_path. For example, that happens when we rename a project + origin.sub!(/#{Regexp.escape(project.full_path)}\z/, old_disk_path) + project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments] target = FileUploader.absolute_base_dir(project) @@ -32,22 +34,22 @@ module Projects private - def move_folder!(old_path, new_path) - unless File.directory?(old_path) - logger.info("Skipped attachments migration from '#{old_path}' to '#{new_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})") + def move_folder!(old_disk_path, new_disk_path) + unless File.directory?(old_disk_path) + logger.info("Skipped attachments migration from '#{old_disk_path}' to '#{new_disk_path}', source path doesn't exist or is not a directory (PROJECT_ID=#{project.id})") return end - if File.exist?(new_path) - logger.error("Cannot migrate attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})") - raise AttachmentMigrationError, "Target path '#{new_path}' already exist" + if File.exist?(new_disk_path) + logger.error("Cannot migrate attachments from '#{old_disk_path}' to '#{new_disk_path}', target path already exist (PROJECT_ID=#{project.id})") + raise AttachmentMigrationError, "Target path '#{new_disk_path}' already exist" end # Create hashed storage base path folder - FileUtils.mkdir_p(File.dirname(new_path)) + FileUtils.mkdir_p(File.dirname(new_disk_path)) - FileUtils.mv(old_path, new_path) - logger.info("Migrated project attachments from '#{old_path}' to '#{new_path}' (PROJECT_ID=#{project.id})") + FileUtils.mv(old_disk_path, new_disk_path) + logger.info("Migrated project attachments from '#{old_disk_path}' to '#{new_disk_path}' (PROJECT_ID=#{project.id})") true end diff --git a/app/services/projects/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb index 70f00b7fdeb..641d46e6591 100644 --- a/app/services/projects/hashed_storage/migrate_repository_service.rb +++ b/app/services/projects/hashed_storage/migrate_repository_service.rb @@ -5,28 +5,27 @@ module Projects class MigrateRepositoryService < BaseService include Gitlab::ShellAdapter - attr_reader :old_disk_path, :new_disk_path, :old_wiki_disk_path, :old_storage_version, :logger + attr_reader :old_disk_path, :new_disk_path, :old_wiki_disk_path, :old_storage_version, :logger, :move_wiki - def initialize(project, logger = nil) + def initialize(project, old_disk_path, logger: nil) @project = project @logger = logger || Rails.logger + @old_disk_path = old_disk_path + @old_wiki_disk_path = "#{old_disk_path}.wiki" + @move_wiki = has_wiki? end def execute - @old_disk_path = project.disk_path - has_wiki = project.wiki.repository_exists? - @old_storage_version = project.storage_version project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository] project.ensure_storage_path_exists @new_disk_path = project.disk_path - result = move_repository(@old_disk_path, @new_disk_path) + result = move_repository(old_disk_path, new_disk_path) - if has_wiki - @old_wiki_disk_path = "#{@old_disk_path}.wiki" - result &&= move_repository("#{@old_wiki_disk_path}", "#{@new_disk_path}.wiki") + if move_wiki + result &&= move_repository("#{old_wiki_disk_path}", "#{new_disk_path}.wiki") end if result @@ -48,6 +47,10 @@ module Projects private + def has_wiki? + gitlab_shell.exists?(project.repository_storage, "#{old_wiki_disk_path}.git") + end + def move_repository(from_name, to_name) from_exists = gitlab_shell.exists?(project.repository_storage, "#{from_name}.git") to_exists = gitlab_shell.exists?(project.repository_storage, "#{to_name}.git") @@ -66,8 +69,8 @@ module Projects end def rollback_folder_move - move_repository(@new_disk_path, @old_disk_path) - move_repository("#{@new_disk_path}.wiki", "#{@old_disk_path}.wiki") + move_repository(new_disk_path, old_disk_path) + move_repository("#{new_disk_path}.wiki", old_wiki_disk_path) end end end diff --git a/app/services/projects/hashed_storage_migration_service.rb b/app/services/projects/hashed_storage_migration_service.rb index 1828c99a65e..a0e734005f8 100644 --- a/app/services/projects/hashed_storage_migration_service.rb +++ b/app/services/projects/hashed_storage_migration_service.rb @@ -2,23 +2,26 @@ module Projects class HashedStorageMigrationService < BaseService - attr_reader :logger + attr_reader :logger, :old_disk_path - def initialize(project, logger = nil) + def initialize(project, old_disk_path, logger: nil) @project = project + @old_disk_path = old_disk_path @logger = logger || Rails.logger end def execute # Migrate repository from Legacy to Hashed Storage unless project.hashed_storage?(:repository) - return unless HashedStorage::MigrateRepositoryService.new(project, logger).execute + return unless HashedStorage::MigrateRepositoryService.new(project, old_disk_path, logger: logger).execute end # Migrate attachments from Legacy to Hashed Storage unless project.hashed_storage?(:attachments) - HashedStorage::MigrateAttachmentsService.new(project, logger).execute + HashedStorage::MigrateAttachmentsService.new(project, old_disk_path, logger: logger).execute end + + true end end end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 31ab4fbe49e..97f181ccea8 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -4,33 +4,27 @@ module Projects class UpdateService < BaseService include UpdateVisibilityLevel - def execute - unless valid_visibility_level_change?(project, params[:visibility_level]) - return error('New visibility level not allowed!') - end - - if renaming_project_with_container_registry_tags? - return error('Cannot rename project because it contains container registry tags!') - end + ValidationError = Class.new(StandardError) - if changing_default_branch? - return error("Could not set the default branch") unless project.change_head(params[:default_branch]) - end + def execute + validate! ensure_wiki_exists if enabling_wiki? yield if block_given? # If the block added errors, don't try to save the project - return validation_failed! if project.errors.any? + return update_failed! if project.errors.any? if project.update(params.except(:default_branch)) after_update success else - validation_failed! + update_failed! end + rescue ValidationError => e + error(e.message) end def run_auto_devops_pipeline? @@ -41,6 +35,20 @@ module Projects private + def validate! + unless valid_visibility_level_change?(project, params[:visibility_level]) + raise ValidationError.new('New visibility level not allowed!') + end + + if renaming_project_with_container_registry_tags? + raise ValidationError.new('Cannot rename project because it contains container registry tags!') + end + + if changing_default_branch? + raise ValidationError.new("Could not set the default branch") unless project.change_head(params[:default_branch]) + end + end + def after_update todos_features_changes = %w( issues_access_level @@ -65,7 +73,7 @@ module Projects update_pages_config if changing_pages_https_only? end - def validation_failed! + def update_failed! model_errors = project.errors.full_messages.to_sentence error_message = model_errors.presence || 'Project could not be updated!' @@ -87,7 +95,7 @@ module Projects end def enabling_wiki? - return false if @project.wiki_enabled? + return false if project.wiki_enabled? params.dig(:project_feature_attributes, :wiki_access_level).to_i > ProjectFeature::DISABLED end diff --git a/app/views/admin/application_settings/_repository_storage.html.haml b/app/views/admin/application_settings/_repository_storage.html.haml index 685de99917e..596385abcb1 100644 --- a/app/views/admin/application_settings/_repository_storage.html.haml +++ b/app/views/admin/application_settings/_repository_storage.html.haml @@ -7,7 +7,7 @@ .form-check = f.check_box :hashed_storage_enabled, class: 'form-check-input' = f.label :hashed_storage_enabled, class: 'form-check-label' do - Create new projects using hashed storage paths + Use hashed storage paths for newly created and renamed projects .form-text.text-muted 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. diff --git a/app/workers/project_migrate_hashed_storage_worker.rb b/app/workers/project_migrate_hashed_storage_worker.rb index 9e4d66250a4..ad0003e7bff 100644 --- a/app/workers/project_migrate_hashed_storage_worker.rb +++ b/app/workers/project_migrate_hashed_storage_worker.rb @@ -5,13 +5,13 @@ class ProjectMigrateHashedStorageWorker LEASE_TIMEOUT = 30.seconds.to_i - def perform(project_id) + def perform(project_id, old_disk_path = nil) project = Project.find_by(id: project_id) return if project.nil? || project.pending_delete? uuid = lease_for(project_id).try_obtain if uuid - ::Projects::HashedStorageMigrationService.new(project, logger).execute + ::Projects::HashedStorageMigrationService.new(project, old_disk_path || project.full_path, logger: logger).execute else false end |