diff options
29 files changed, 401 insertions, 148 deletions
diff --git a/app/services/projects/hashed_storage/base_attachment_service.rb b/app/services/projects/hashed_storage/base_attachment_service.rb index 828ab616bab..f8852c206e3 100644 --- a/app/services/projects/hashed_storage/base_attachment_service.rb +++ b/app/services/projects/hashed_storage/base_attachment_service.rb @@ -16,6 +16,12 @@ module Projects # Returns the logger currently in use attr_reader :logger + def initialize(project:, old_disk_path:, logger: nil) + @project = project + @old_disk_path = old_disk_path + @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger + end + # Return whether this operation was skipped or not # # @return [Boolean] true if skipped of false otherwise @@ -23,6 +29,14 @@ module Projects @skipped end + # Check if target path has discardable content + # + # @param [String] new_path + # @return [Boolean] whether we can discard the target path or not + def target_path_discardable?(new_path) + false + end + protected def move_folder!(old_path, new_path) @@ -34,8 +48,13 @@ module Projects end if File.exist?(new_path) - logger.error("Cannot move attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})") - raise AttachmentCannotMoveError, "Target path '#{new_path}' already exists" + if target_path_discardable?(new_path) + discard_path!(new_path) + else + logger.error("Cannot move attachments from '#{old_path}' to '#{new_path}', target path already exist (PROJECT_ID=#{project.id})") + + raise AttachmentCannotMoveError, "Target path '#{new_path}' already exists" + end end # Create base path folder on the new storage layout @@ -46,6 +65,16 @@ module Projects true end + + # Rename a path adding a suffix in order to prevent data-loss. + # + # @param [String] new_path + def discard_path!(new_path) + discarded_path = "#{new_path}-#{Time.now.utc.to_i}" + + logger.info("Moving existing empty attachments folder from '#{new_path}' to '#{discarded_path}', (PROJECT_ID=#{project.id})") + FileUtils.mv(new_path, discarded_path) + end end end end diff --git a/app/services/projects/hashed_storage/base_repository_service.rb b/app/services/projects/hashed_storage/base_repository_service.rb index b7e9d3e8791..8b1bcaf17b7 100644 --- a/app/services/projects/hashed_storage/base_repository_service.rb +++ b/app/services/projects/hashed_storage/base_repository_service.rb @@ -10,7 +10,7 @@ module Projects attr_reader :old_disk_path, :new_disk_path, :old_wiki_disk_path, :old_storage_version, :logger, :move_wiki - def initialize(project, old_disk_path, logger: nil) + def initialize(project:, old_disk_path:, logger: nil) @project = project @logger = logger || Gitlab::AppLogger @old_disk_path = old_disk_path diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb index 0cbff283102..3d9d03c4a95 100644 --- a/app/services/projects/hashed_storage/migrate_attachments_service.rb +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -3,18 +3,19 @@ module Projects module HashedStorage class MigrateAttachmentsService < BaseAttachmentService - def initialize(project, old_disk_path, logger: nil) - @project = project - @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger - @old_disk_path = old_disk_path + extend ::Gitlab::Utils::Override + + # List of paths that can be excluded while evaluation if a target can be discarded + DISCARDABLE_PATHS = %w(tmp tmp/cache tmp/work).freeze + + def initialize(project:, old_disk_path:, logger: nil) + super + @skipped = false end def execute - 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) + origin = find_old_attachments_path(project) project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments] target = FileUploader.absolute_base_dir(project) @@ -27,13 +28,38 @@ module Projects project.save!(validate: false) yield if block_given? - else - # Rollback changes - project.rollback! end result end + + override :target_path_discardable? + # Check if target path has discardable content + # + # @param [String] new_path + # @return [Boolean] whether we can discard the target path or not + def target_path_discardable?(new_path) + return false unless File.directory?(new_path) + + found = Dir.glob(File.join(new_path, '**', '**')) + + (found - discardable_paths(new_path)).empty? + end + + private + + def discardable_paths(new_path) + DISCARDABLE_PATHS.collect { |path| File.join(new_path, path) } + end + + def find_old_attachments_path(project) + 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) + end end end end diff --git a/app/services/projects/hashed_storage/migration_service.rb b/app/services/projects/hashed_storage/migration_service.rb index f132dca61c9..57a775a8f9e 100644 --- a/app/services/projects/hashed_storage/migration_service.rb +++ b/app/services/projects/hashed_storage/migration_service.rb @@ -14,12 +14,12 @@ module Projects def execute # Migrate repository from Legacy to Hashed Storage unless project.hashed_storage?(:repository) - return false unless migrate_repository + return false unless migrate_repository_service.execute end # Migrate attachments from Legacy to Hashed Storage unless project.hashed_storage?(:attachments) - return false unless migrate_attachments + return false unless migrate_attachments_service.execute end true @@ -27,12 +27,12 @@ module Projects private - def migrate_repository - HashedStorage::MigrateRepositoryService.new(project, old_disk_path, logger: logger).execute + def migrate_repository_service + HashedStorage::MigrateRepositoryService.new(project: project, old_disk_path: old_disk_path, logger: logger) end - def migrate_attachments - HashedStorage::MigrateAttachmentsService.new(project, old_disk_path, logger: logger).execute + def migrate_attachments_service + HashedStorage::MigrateAttachmentsService.new(project: project, old_disk_path: old_disk_path, logger: logger) end end end diff --git a/app/services/projects/hashed_storage/rollback_attachments_service.rb b/app/services/projects/hashed_storage/rollback_attachments_service.rb index fb09eaa4586..4bb8cb605a3 100644 --- a/app/services/projects/hashed_storage/rollback_attachments_service.rb +++ b/app/services/projects/hashed_storage/rollback_attachments_service.rb @@ -3,14 +3,9 @@ module Projects module HashedStorage class RollbackAttachmentsService < BaseAttachmentService - def initialize(project, logger: nil) - @project = project - @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger - @old_disk_path = project.disk_path - end - def execute origin = FileUploader.absolute_base_dir(project) + project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository] target = FileUploader.absolute_base_dir(project) diff --git a/app/services/projects/hashed_storage/rollback_service.rb b/app/services/projects/hashed_storage/rollback_service.rb index ee41aae64a5..c437001c440 100644 --- a/app/services/projects/hashed_storage/rollback_service.rb +++ b/app/services/projects/hashed_storage/rollback_service.rb @@ -5,32 +5,26 @@ module Projects class RollbackService < BaseService attr_reader :logger, :old_disk_path - def initialize(project, old_disk_path, logger: nil) - @project = project - @old_disk_path = old_disk_path - @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger - end - def execute # Rollback attachments from Hashed Storage to Legacy if project.hashed_storage?(:attachments) - return false unless rollback_attachments + return false unless rollback_attachments_service.execute end # Rollback repository from Hashed Storage to Legacy if project.hashed_storage?(:repository) - rollback_repository + rollback_repository_service.execute end end private - def rollback_attachments - HashedStorage::RollbackAttachmentsService.new(project, logger: logger).execute + def rollback_attachments_service + HashedStorage::RollbackAttachmentsService.new(project: project, old_disk_path: old_disk_path, logger: logger) end - def rollback_repository - HashedStorage::RollbackRepositoryService.new(project, old_disk_path, logger: logger).execute + def rollback_repository_service + HashedStorage::RollbackRepositoryService.new(project: project, old_disk_path: old_disk_path, logger: logger) end end end diff --git a/app/workers/hashed_storage/project_migrate_worker.rb b/app/workers/hashed_storage/project_migrate_worker.rb index f00a459a097..8c0ec97638f 100644 --- a/app/workers/hashed_storage/project_migrate_worker.rb +++ b/app/workers/hashed_storage/project_migrate_worker.rb @@ -16,7 +16,7 @@ module HashedStorage project = Project.without_deleted.find_by(id: project_id) break unless project - old_disk_path ||= project.disk_path + old_disk_path ||= Storage::LegacyProject.new(project).disk_path ::Projects::HashedStorage::MigrationService.new(project, old_disk_path, logger: logger).execute end diff --git a/changelogs/unreleased/32367-hashed-storage-migration-handle-failed-attachment-migrations-wth-ex.yml b/changelogs/unreleased/32367-hashed-storage-migration-handle-failed-attachment-migrations-wth-ex.yml new file mode 100644 index 00000000000..6690318d8ac --- /dev/null +++ b/changelogs/unreleased/32367-hashed-storage-migration-handle-failed-attachment-migrations-wth-ex.yml @@ -0,0 +1,6 @@ +--- +title: 'Hashed Storage Migration: Handle failed attachment migrations with existing + target path' +merge_request: 19061 +author: +type: fixed diff --git a/doc/administration/high_availability/README.md b/doc/administration/high_availability/README.md index c9c9b469a89..a1ac2486528 100644 --- a/doc/administration/high_availability/README.md +++ b/doc/administration/high_availability/README.md @@ -229,7 +229,7 @@ users are, how much automation you use, mirroring, and repo/change size. | 3 GitLab Rails <br> - Puma workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 28.8GB Memory | n1-highcpu-32 | | 3 PostgreSQL | 4 vCPU, 15GB Memory | n1-standard-4 | | 1 PgBouncer | 2 vCPU, 1.8GB Memory | n1-highcpu-2 | -| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 90% of available CPUs with 16 threads | 16 vCPU, 60GB Memory | n1-standard-16 | +| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 20% of available CPUs | 16 vCPU, 60GB Memory | n1-standard-16 | | 3 Redis Cache + Sentinel <br> - Cache maxmemory set to 90% of available memory | 4 vCPU, 15GB Memory | n1-standard-4 | | 3 Redis Persistent + Sentinel | 4 vCPU, 15GB Memory | n1-standard-4 | | 4 Sidekiq | 4 vCPU, 15GB Memory | n1-standard-4 | @@ -256,7 +256,7 @@ vendors a best effort like for like can be used. | 7 GitLab Rails <br> - Puma workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 28.8GB Memory | n1-highcpu-32 | | 3 PostgreSQL | 8 vCPU, 30GB Memory | n1-standard-8 | | 1 PgBouncer | 2 vCPU, 1.8GB Memory | n1-highcpu-2 | -| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 120GB Memory | n1-standard-32 | +| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 20% of available CPUs | 32 vCPU, 120GB Memory | n1-standard-32 | | 3 Redis Cache + Sentinel <br> - Cache maxmemory set to 90% of available memory | 4 vCPU, 15GB Memory | n1-standard-4 | | 3 Redis Persistent + Sentinel | 4 vCPU, 15GB Memory | n1-standard-4 | | 4 Sidekiq | 4 vCPU, 15GB Memory | n1-standard-4 | @@ -285,7 +285,7 @@ may be adjusted prior to certification based on performance testing. | 15 GitLab Rails <br> - Puma workers on each node set to 90% of available CPUs with 16 threads | 32 vCPU, 28.8GB Memory | n1-highcpu-32 | | 3 PostgreSQL | 8 vCPU, 30GB Memory | n1-standard-8 | | 1 PgBouncer | 2 vCPU, 1.8GB Memory | n1-highcpu-2 | -| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 90% of available CPUs with 16 threads | 64 vCPU, 240GB Memory | n1-standard-64 | +| X Gitaly[^1] <br> - Gitaly Ruby workers on each node set to 20% of available CPUs | 64 vCPU, 240GB Memory | n1-standard-64 | | 3 Redis Cache + Sentinel <br> - Cache maxmemory set to 90% of available memory | 4 vCPU, 15GB Memory | n1-standard-4 | | 3 Redis Persistent + Sentinel | 4 vCPU, 15GB Memory | n1-standard-4 | | 4 Sidekiq | 4 vCPU, 15GB Memory | n1-standard-4 | diff --git a/doc/administration/operations/index.md b/doc/administration/operations/index.md index df208b3f427..1d80e23eecf 100644 --- a/doc/administration/operations/index.md +++ b/doc/administration/operations/index.md @@ -21,3 +21,6 @@ Keep your GitLab instance up and running smoothly. performance can have a big impact on GitLab performance, especially for actions that read or write Git repositories. This information will help benchmark filesystem performance against known good and bad real-world systems. +- [ChatOps Scripts](https://gitlab.com/gitlab-com/chatops): The GitLab.com Infrastructure team uses this repository to house + common ChatOps scripts they use to troubleshoot and maintain the production instance of GitLab.com. + These scripts are likely useful to administrators of GitLab instances of all sizes. diff --git a/doc/ci/chatops/README.md b/doc/ci/chatops/README.md index 234e7f4ed80..d9236b47a9a 100644 --- a/doc/ci/chatops/README.md +++ b/doc/ci/chatops/README.md @@ -58,6 +58,11 @@ ls: - echo -e "section_start:$( date +%s ):chat_reply\r\033[0K\n$( ls -la )\nsection_end:$( date +%s ):chat_reply\r\033[0K" ``` +## GitLab ChatOps Examples + +The GitLab.com team created a repository of [common ChatOps scripts they use to interact with our Production instance of GitLab](https://gitlab.com/gitlab-com/chatops). They are likely useful +to other adminstrators of GitLab instances and can serve as inspiration for ChatOps scripts you can write to interact with your own applications. + ## GitLab ChatOps icon Say Hi to our ChatOps bot. diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md index 4dc8dd927dc..21a4e3d8ead 100644 --- a/doc/user/project/milestones/index.md +++ b/doc/user/project/milestones/index.md @@ -12,23 +12,21 @@ Milestones allow you to organize issues and merge requests into a cohesive group ## Milestones as Agile sprints -Milestones can be used as Agile sprints. -Set the milestone start date and due date to represent -the start and end of your Agile sprint. -Set the milestone title to the name of your Agile sprint, -such as `November 2018 sprint`. -Add an issue to your Agile sprint by associating -the milestone to the issue. +Milestones can be used as Agile sprints so that you can track all issues and merge requests related to a particular sprint. To do so: + +1. Set the milestone start date and due date to represent the start and end of your Agile sprint. +1. Set the milestone title to the name of your Agile sprint, such as `November 2018 sprint`. +1. Add an issue to your Agile sprint by associating the desired milestone from the issue's right-hand sidebar. ## Milestones as releases -Milestones can be used as releases. -Set the milestone due date to represent the release date of your release. -(And leave the milestone start date blank.) -Set the milestone title to the version of your release, -such as `Version 9.4`. -Add an issue to your release by associating -the milestone to the issue. +Similarily, milestones can be used as releases. To do so: + +1. Set the milestone due date to represent the release date of your release and leave the milestone start date blank. +1. Set the milestone title to the version of your release, such as `Version 9.4`. +1. Add an issue to your release by associating the desired milestone from the issue's right-hand sidebar. + +Additionally, you can integrate milestones with GitLab's [Releases feature](../releases/index.md#releases-associated-with-milestones). ## Project milestones and group milestones diff --git a/doc/user/project/releases/img/milestone_list_with_releases_v12_5.png b/doc/user/project/releases/img/milestone_list_with_releases_v12_5.png Binary files differnew file mode 100644 index 00000000000..2e3ec08ba87 --- /dev/null +++ b/doc/user/project/releases/img/milestone_list_with_releases_v12_5.png diff --git a/doc/user/project/releases/img/milestone_with_releases_v12_5.png b/doc/user/project/releases/img/milestone_with_releases_v12_5.png Binary files differnew file mode 100644 index 00000000000..8719a58ce4e --- /dev/null +++ b/doc/user/project/releases/img/milestone_with_releases_v12_5.png diff --git a/doc/user/project/releases/img/release_with_milestone_v12_5.png b/doc/user/project/releases/img/release_with_milestone_v12_5.png Binary files differnew file mode 100644 index 00000000000..2a7a2ee9754 --- /dev/null +++ b/doc/user/project/releases/img/release_with_milestone_v12_5.png diff --git a/doc/user/project/releases/index.md b/doc/user/project/releases/index.md index d31df77f8a0..8372aefc94c 100644 --- a/doc/user/project/releases/index.md +++ b/doc/user/project/releases/index.md @@ -58,6 +58,31 @@ links from your GitLab instance. NOTE: **NOTE** You can manipulate links of each release entry with [Release Links API](../../../api/releases/links.md) +#### Releases associated with milestones + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/29020) in GitLab 12.5. + +Releases can optionally be associated with one or more +[project milestones](../milestones/index.md#project-milestones-and-group-milestones) +by including a `milestones` array in your requests to the +[Releases API](../../../api/releases/index.md#create-a-release). + +Releases display this association with the **Milestone** indicator near +the top of the Release block on the **Project overview > Releases** page. + + + +Below is an example of milestones with no Releases, one Release, and two +Releases, respectively. + + + +This relationship is also visible in the **Releases** section of the sidebar +when viewing a specific milestone. Below is an example of a milestone +associated with a large number of Releases. + + + ## Releases list Navigate to **Project > Releases** in order to see the list of releases for a given diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index ace42ebd872..c75ae87a985 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -114,7 +114,10 @@ module Gitlab entry :rules, Entry::Rules, description: 'List of evaluable Rules to determine job inclusion.', - inherit: false + inherit: false, + metadata: { + allowed_when: %w[on_success on_failure always never manual delayed].freeze + } entry :needs, Entry::Needs, description: 'Needs configuration for this job.', diff --git a/lib/gitlab/ci/config/entry/rules/rule.rb b/lib/gitlab/ci/config/entry/rules/rule.rb index 5d6d1c026e3..59e0ef583ae 100644 --- a/lib/gitlab/ci/config/entry/rules/rule.rb +++ b/lib/gitlab/ci/config/entry/rules/rule.rb @@ -8,9 +8,9 @@ module Gitlab include ::Gitlab::Config::Entry::Validatable include ::Gitlab::Config::Entry::Attributable - CLAUSES = %i[if changes exists].freeze - ALLOWED_KEYS = %i[if changes exists when start_in].freeze - ALLOWED_WHEN = %w[on_success on_failure always never manual delayed].freeze + CLAUSES = %i[if changes exists].freeze + ALLOWED_KEYS = %i[if changes exists when start_in].freeze + ALLOWABLE_WHEN = %w[on_success on_failure always never manual delayed].freeze attributes :if, :changes, :exists, :when, :start_in @@ -25,7 +25,14 @@ module Gitlab with_options allow_nil: true do validates :if, expression: true validates :changes, :exists, array_of_strings: true, length: { maximum: 50 } - validates :when, allowed_values: { in: ALLOWED_WHEN } + validates :when, allowed_values: { in: ALLOWABLE_WHEN } + end + + validate do + validates_with Gitlab::Config::Entry::Validators::AllowedValuesValidator, + attributes: %i[when], + allow_nil: true, + in: opt(:allowed_when) end end diff --git a/package.json b/package.json index c1ddb7be2bf..f247b972bd2 100644 --- a/package.json +++ b/package.json @@ -37,9 +37,9 @@ "@babel/plugin-syntax-dynamic-import": "^7.2.0", "@babel/plugin-syntax-import-meta": "^7.2.0", "@babel/preset-env": "^7.6.2", - "@gitlab/svgs": "^1.80.0", - "@gitlab/ui": "7.5.0", - "@gitlab/visual-review-tools": "1.0.3", + "@gitlab/svgs": "^1.82.0", + "@gitlab/ui": "7.11.0", + "@gitlab/visual-review-tools": "1.2.0", "@sentry/browser": "^5.7.1", "apollo-cache-inmemory": "^1.6.3", "apollo-client": "^2.6.4", diff --git a/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb index 1f54f6ec537..216f5d0c77d 100644 --- a/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/rules/rule_spec.rb @@ -6,7 +6,17 @@ require 'support/helpers/stub_feature_flags' require_dependency 'active_model' describe Gitlab::Ci::Config::Entry::Rules::Rule do - let(:entry) { described_class.new(config) } + let(:factory) do + Gitlab::Config::Entry::Factory.new(described_class) + .metadata(metadata) + .value(config) + end + + let(:metadata) do + { allowed_when: %w[on_success on_failure always never manual delayed] } + end + + let(:entry) { factory.create! } describe '.new' do subject { entry } @@ -212,6 +222,112 @@ describe Gitlab::Ci::Config::Entry::Rules::Rule do .to include(/should be a hash/) end end + + context 'when: validation' do + context 'with an invalid boolean when:' do + let(:config) do + { if: '$THIS == "that"', when: false } + end + + it { is_expected.to be_a(described_class) } + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: false/) + end + + context 'when composed' do + before do + subject.compose! + end + + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: false/) + end + end + end + + context 'with an invalid string when:' do + let(:config) do + { if: '$THIS == "that"', when: 'explode' } + end + + it { is_expected.to be_a(described_class) } + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: explode/) + end + + context 'when composed' do + before do + subject.compose! + end + + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: explode/) + end + end + end + + context 'with a string passed in metadata but not allowed in the class' do + let(:metadata) { { allowed_when: %w[explode] } } + + let(:config) do + { if: '$THIS == "that"', when: 'explode' } + end + + it { is_expected.to be_a(described_class) } + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: explode/) + end + + context 'when composed' do + before do + subject.compose! + end + + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: explode/) + end + end + end + + context 'with a string allowed in the class but not passed in metadata' do + let(:metadata) { { allowed_when: %w[always never] } } + + let(:config) do + { if: '$THIS == "that"', when: 'on_success' } + end + + it { is_expected.to be_a(described_class) } + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: on_success/) + end + + context 'when composed' do + before do + subject.compose! + end + + it { is_expected.not_to be_valid } + + it 'returns an error about invalid when:' do + expect(subject.errors).to include(/when unknown value: on_success/) + end + end + end + end end describe '#value' do diff --git a/spec/lib/gitlab/ci/config/entry/rules_spec.rb b/spec/lib/gitlab/ci/config/entry/rules_spec.rb index 926d3fd1678..3c050801023 100644 --- a/spec/lib/gitlab/ci/config/entry/rules_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/rules_spec.rb @@ -5,7 +5,14 @@ require 'support/helpers/stub_feature_flags' require_dependency 'active_model' describe Gitlab::Ci::Config::Entry::Rules do - let(:entry) { described_class.new(config) } + let(:factory) do + Gitlab::Config::Entry::Factory.new(described_class) + .metadata(metadata) + .value(config) + end + + let(:metadata) { { allowed_when: %w[always never] } } + let(:entry) { factory.create! } describe '.new' do subject { entry } @@ -18,7 +25,7 @@ describe Gitlab::Ci::Config::Entry::Rules do it { is_expected.to be_a(described_class) } it { is_expected.to be_valid } - context 'after #compose!' do + context 'when composed' do before do subject.compose! end @@ -38,7 +45,7 @@ describe Gitlab::Ci::Config::Entry::Rules do it { is_expected.to be_a(described_class) } it { is_expected.to be_valid } - context 'after #compose!' do + context 'when composed' do before do subject.compose! end @@ -54,48 +61,6 @@ describe Gitlab::Ci::Config::Entry::Rules do it { is_expected.not_to be_valid } end - - context 'with an invalid boolean when:' do - let(:config) do - [{ if: '$THIS == "that"', when: false }] - end - - it { is_expected.to be_a(described_class) } - it { is_expected.to be_valid } - - context 'after #compose!' do - before do - subject.compose! - end - - it { is_expected.not_to be_valid } - - it 'returns an error about invalid when:' do - expect(subject.errors).to include(/when unknown value: false/) - end - end - end - - context 'with an invalid string when:' do - let(:config) do - [{ if: '$THIS == "that"', when: 'explode' }] - end - - it { is_expected.to be_a(described_class) } - it { is_expected.to be_valid } - - context 'after #compose!' do - before do - subject.compose! - end - - it { is_expected.not_to be_valid } - - it 'returns an error about invalid when:' do - expect(subject.errors).to include(/when unknown value: explode/) - end - end - end end describe '#value' do diff --git a/spec/services/projects/hashed_storage/base_attachment_service_spec.rb b/spec/services/projects/hashed_storage/base_attachment_service_spec.rb new file mode 100644 index 00000000000..34c37be6703 --- /dev/null +++ b/spec/services/projects/hashed_storage/base_attachment_service_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::HashedStorage::BaseAttachmentService do + let(:project) { create(:project, :repository, storage_version: 0, skip_disk_validation: true) } + + subject(:service) { described_class.new(project: project, old_disk_path: project.full_path, logger: nil) } + + describe '#old_disk_path' do + it { is_expected.to respond_to :old_disk_path } + end + + describe '#new_disk_path' do + it { is_expected.to respond_to :new_disk_path } + end + + describe '#skipped?' do + it { is_expected.to respond_to :skipped? } + end + + describe '#target_path_discardable?' do + it 'returns false' do + expect(subject.target_path_discardable?('something/something')).to be_falsey + end + end + + describe '#discard_path!' do + it 'renames target path adding a timestamp at the end' do + target_path = Dir.mktmpdir + expect(Dir.exist?(target_path)).to be_truthy + + Timecop.freeze do + suffix = Time.now.utc.to_i + subject.send(:discard_path!, target_path) + + expected_renamed_path = "#{target_path}-#{suffix}" + + expect(Dir.exist?(target_path)).to be_falsey + expect(Dir.exist?(expected_renamed_path)).to be_truthy + end + end + end + + describe '#move_folder!' do + context 'when old_path is not a directory' do + it 'adds information to the logger and returns true' do + Tempfile.create do |old_path| + new_path = "#{old_path}-new" + + expect(subject.send(:move_folder!, old_path, new_path)).to be_truthy + end + end + end + end +end diff --git a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb index 32ebec318f2..ab9d2bdba8f 100644 --- a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::HashedStorage::MigrateAttachmentsService do - subject(:service) { described_class.new(project, project.full_path, logger: nil) } + subject(:service) { described_class.new(project: project, old_disk_path: project.full_path, logger: nil) } let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } let(:legacy_storage) { Storage::LegacyProject.new(project) } @@ -72,7 +72,23 @@ describe Projects::HashedStorage::MigrateAttachmentsService do FileUtils.mkdir_p(base_path(hashed_storage)) end - it 'raises AttachmentCannotMoveError' do + it 'succeed when target is empty' do + expect { service.execute }.not_to raise_error + end + + it 'succeed when target include only discardable items' do + Projects::HashedStorage::MigrateAttachmentsService::DISCARDABLE_PATHS.each do |path_fragment| + discardable_path = File.join(base_path(hashed_storage), path_fragment) + FileUtils.mkdir_p(discardable_path) + end + + expect { service.execute }.not_to raise_error + end + + it 'raises AttachmentCannotMoveError when there are non discardable items on target path' do + not_discardable_path = File.join(base_path(hashed_storage), 'something') + FileUtils.mkdir_p(not_discardable_path) + expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) @@ -100,6 +116,18 @@ describe Projects::HashedStorage::MigrateAttachmentsService do end end + context '#target_path_discardable?' do + it 'returns true when it include only items on the discardable list' do + hashed_attachments_path = File.join(base_path(hashed_storage)) + Projects::HashedStorage::MigrateAttachmentsService::DISCARDABLE_PATHS.each do |path_fragment| + discardable_path = File.join(hashed_attachments_path, path_fragment) + FileUtils.mkdir_p(discardable_path) + end + + expect(service.target_path_discardable?(hashed_attachments_path)).to be_truthy + end + end + def base_path(storage) File.join(FileUploader.root, storage.disk_path) end diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 70785c606a5..132b895fc35 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -10,7 +10,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do let(:legacy_storage) { Storage::LegacyProject.new(project) } let(:hashed_storage) { Storage::HashedProject.new(project) } - subject(:service) { described_class.new(project, project.disk_path) } + subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path) } describe '#execute' do let(:old_disk_path) { legacy_storage.disk_path } diff --git a/spec/services/projects/hashed_storage/migration_service_spec.rb b/spec/services/projects/hashed_storage/migration_service_spec.rb index e3191cd7ebc..f3ac26e7761 100644 --- a/spec/services/projects/hashed_storage/migration_service_spec.rb +++ b/spec/services/projects/hashed_storage/migration_service_spec.rb @@ -10,13 +10,14 @@ describe Projects::HashedStorage::MigrationService do describe '#execute' do context 'repository migration' do - let(:repository_service) { Projects::HashedStorage::MigrateRepositoryService.new(project, project.full_path, logger: logger) } + let(:repository_service) do + Projects::HashedStorage::MigrateRepositoryService.new(project: project, + old_disk_path: project.full_path, + logger: logger) + end it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do - expect(Projects::HashedStorage::MigrateRepositoryService) - .to receive(:new) - .with(project, project.full_path, logger: logger) - .and_return(repository_service) + expect(service).to receive(:migrate_repository_service).and_return(repository_service) expect(repository_service).to receive(:execute) service.execute @@ -31,13 +32,14 @@ describe Projects::HashedStorage::MigrationService do end context 'attachments migration' do - let(:attachments_service) { Projects::HashedStorage::MigrateAttachmentsService.new(project, project.full_path, logger: logger) } + let(:attachments_service) do + Projects::HashedStorage::MigrateAttachmentsService.new(project: project, + old_disk_path: project.full_path, + logger: logger) + end it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do - expect(Projects::HashedStorage::MigrateAttachmentsService) - .to receive(:new) - .with(project, project.full_path, logger: logger) - .and_return(attachments_service) + expect(service).to receive(:migrate_attachments_service).and_return(attachments_service) expect(attachments_service).to receive(:execute) service.execute diff --git a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb index 815c85e0866..c2ba9626f41 100644 --- a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::HashedStorage::RollbackAttachmentsService do - subject(:service) { described_class.new(project, logger: nil) } + subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path, logger: nil) } let(:project) { create(:project, :repository, skip_disk_validation: true) } let(:legacy_storage) { Storage::LegacyProject.new(project) } diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb index 3ca9ee5bee5..97c7c0af946 100644 --- a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb @@ -10,7 +10,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis let(:legacy_storage) { Storage::LegacyProject.new(project) } let(:hashed_storage) { Storage::HashedProject.new(project) } - subject(:service) { described_class.new(project, project.disk_path) } + subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path) } describe '#execute' do let(:old_disk_path) { hashed_storage.disk_path } diff --git a/spec/services/projects/hashed_storage/rollback_service_spec.rb b/spec/services/projects/hashed_storage/rollback_service_spec.rb index 427d1535559..48d4eac9eb7 100644 --- a/spec/services/projects/hashed_storage/rollback_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_service_spec.rb @@ -6,17 +6,15 @@ describe Projects::HashedStorage::RollbackService do let(:project) { create(:project, :empty_repo, :wiki_repo) } let(:logger) { double } - subject(:service) { described_class.new(project, project.full_path, logger: logger) } + subject(:service) { described_class.new(project, project.disk_path, logger: logger) } describe '#execute' do context 'attachments rollback' do let(:attachments_service_class) { Projects::HashedStorage::RollbackAttachmentsService } - let(:attachments_service) { attachments_service_class.new(project, logger: logger) } + let(:attachments_service) { attachments_service_class.new(project: project, old_disk_path: project.disk_path, logger: logger) } it 'delegates rollback to Projects::HashedStorage::RollbackAttachmentsService' do - expect(attachments_service_class).to receive(:new) - .with(project, logger: logger) - .and_return(attachments_service) + expect(service).to receive(:rollback_attachments_service).and_return(attachments_service) expect(attachments_service).to receive(:execute) service.execute @@ -31,15 +29,12 @@ describe Projects::HashedStorage::RollbackService do end context 'repository rollback' do + let(:project) { create(:project, :empty_repo, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } let(:repository_service_class) { Projects::HashedStorage::RollbackRepositoryService } - let(:repository_service) { repository_service_class.new(project, project.full_path, logger: logger) } + let(:repository_service) { repository_service_class.new(project: project, old_disk_path: project.disk_path, logger: logger) } it 'delegates rollback to RollbackRepositoryService' do - project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:repository] - - expect(repository_service_class).to receive(:new) - .with(project, project.full_path, logger: logger) - .and_return(repository_service) + expect(service).to receive(:rollback_repository_service).and_return(repository_service) expect(repository_service).to receive(:execute) service.execute diff --git a/yarn.lock b/yarn.lock index df69f48f972..ba6a14cbba0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -717,15 +717,15 @@ dependencies: vue-eslint-parser "^6.0.4" -"@gitlab/svgs@^1.80.0": - version "1.80.0" - resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.80.0.tgz#52b2d25f002cdfe9bd7c366a043c1849687ad64b" - integrity sha512-hsyX3EZV/hk9bMTvvoxVcNC0EO6sy771BC2vXjqGtzjye4hTs0BTAzu3V0UPWuDompHtKXi/plVcJU+NxNLQ6Q== +"@gitlab/svgs@^1.82.0": + version "1.82.0" + resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.82.0.tgz#c059c460afc13ebfe9df370521ca8963fa5afb80" + integrity sha512-9L4Brys2LCk44lHvFsCFDKN768lYjoMVYDb4PD7FSjqUEruQQ1SRj0rvb1RWKLhiTCDKrtDOXkH6I1TTEms24w== -"@gitlab/ui@7.5.0": - version "7.5.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-7.5.0.tgz#d25567157d20bb64741ab51b6b9f770ea49e634d" - integrity sha512-h7RxNMtQ1+KHK2uV+nb5d7UlqBVOtj9VGXqRXqVinc1b1x0onnvFFnYjgxf7XbXdsZq85ZyTlZa1SkduRig+Eg== +"@gitlab/ui@7.11.0": + version "7.11.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-7.11.0.tgz#b5c981f3b1edbf0ad75bcca8fa1cd81017676b3b" + integrity sha512-PxZkgdY2j/XdriTdp3jsnsif9cgcxd1wUF8PVOho2HIyJqU244E8ELewIXkDozQq3p3ZXzWnjR/GvYcNMZtGmA== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.2.1" @@ -740,10 +740,10 @@ vue "^2.6.10" vue-loader "^15.4.2" -"@gitlab/visual-review-tools@1.0.3": - version "1.0.3" - resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.0.3.tgz#b49c4a6fd8af3a1517d7e7d04096562f8bcb5d14" - integrity sha512-96j+0+Ivon5nYvT2doDCLQoBzU/GZYfQGLBmZZE3FZVMsIPAEsqDcSV/6+XCikUzU3B8VnH6er6l9OxE5x1RVw== +"@gitlab/visual-review-tools@1.2.0": + version "1.2.0" + resolved "https://registry.yarnpkg.com/@gitlab/visual-review-tools/-/visual-review-tools-1.2.0.tgz#8d6757917193c1023012bb4a316dc1a97309a27a" + integrity sha512-GaV/lYLmOF0hWtv8K8MLWGaCZ7PL1LF4D0/gargXYf9HO0Cw4wtz4oWyaLS15wFposJIYdPIHSNfrLVk4Dk9sQ== "@gitlab/vue-toasted@^1.2.1": version "1.2.1" |
