From f3ffbc7271d9e7f94ff84c23b76669f2ed38a6b0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 5 Jul 2021 15:10:13 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-12-stable-ee --- app/models/namespace.rb | 8 +++- .../recursive_approach_for_all_projects.yml | 8 ++++ .../base_migrater.rb | 6 +-- .../artifact_migrater_spec.rb | 14 ++++++ .../pages_deployment_migrater_spec.rb | 14 ++++++ spec/models/namespace_spec.rb | 22 +++++---- ...and_remote_storage_migration_shared_examples.rb | 53 ++++++++++++++++++++++ 7 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 config/feature_flags/development/recursive_approach_for_all_projects.yml create mode 100644 spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb create mode 100644 spec/lib/gitlab/local_and_remote_storage_migration/pages_deployment_migrater_spec.rb create mode 100644 spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 8f03c6145cb..73a69c9f215 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -271,8 +271,12 @@ class Namespace < ApplicationRecord # Includes projects from this namespace and projects from all subgroups # that belongs to this namespace def all_projects - namespace = user? ? self : self_and_descendants - Project.where(namespace: namespace) + if Feature.enabled?(:recursive_approach_for_all_projects, default_enabled: :yaml) + namespace = user? ? self : self_and_descendants + Project.where(namespace: namespace) + else + Project.inside_path(full_path) + end end # Includes pipelines from this namespace and pipelines from all subgroups diff --git a/config/feature_flags/development/recursive_approach_for_all_projects.yml b/config/feature_flags/development/recursive_approach_for_all_projects.yml new file mode 100644 index 00000000000..e2d656b7de2 --- /dev/null +++ b/config/feature_flags/development/recursive_approach_for_all_projects.yml @@ -0,0 +1,8 @@ +--- +name: recursive_approach_for_all_projects +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/64632 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334817 +milestone: '14.1' +type: development +group: group::fulfillment +default_enabled: true diff --git a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb index f859d293e76..0484957a6fe 100644 --- a/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb +++ b/lib/gitlab/local_and_remote_storage_migration/base_migrater.rb @@ -8,7 +8,7 @@ module Gitlab end def migrate_to_remote_storage - logger.info('Starting transfer to remote storage') + logger.info('Starting transfer to object storage') migrate(items_with_files_stored_locally, ObjectStorage::Store::REMOTE) end @@ -38,11 +38,11 @@ module Gitlab end def log_success(item, store) - logger.info("Transferred #{item.class.name} ID #{item.id} of type #{item.file_type} with size #{item.size} to #{storage_label(store)} storage") + logger.info("Transferred #{item.class.name} ID #{item.id} with size #{item.size} to #{storage_label(store)} storage") end def log_error(err, item) - logger.warn("Failed to transfer #{item.class.name} of type #{item.file_type} and ID #{item.id} with error: #{err.message}") + logger.warn("Failed to transfer #{item.class.name} ID #{item.id} with error: #{err.message}") end def storage_label(store) diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb new file mode 100644 index 00000000000..b3f2003c207 --- /dev/null +++ b/spec/lib/gitlab/local_and_remote_storage_migration/artifact_migrater_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples' + +RSpec.describe Gitlab::LocalAndRemoteStorageMigration::ArtifactMigrater do + before do + stub_artifacts_object_storage(enabled: true) + end + + let!(:item) { create(:ci_job_artifact, :archive, file_store: start_store) } + + it_behaves_like 'local and remote storage migration' +end diff --git a/spec/lib/gitlab/local_and_remote_storage_migration/pages_deployment_migrater_spec.rb b/spec/lib/gitlab/local_and_remote_storage_migration/pages_deployment_migrater_spec.rb new file mode 100644 index 00000000000..2cc48b445f1 --- /dev/null +++ b/spec/lib/gitlab/local_and_remote_storage_migration/pages_deployment_migrater_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples' + +RSpec.describe Gitlab::LocalAndRemoteStorageMigration::PagesDeploymentMigrater do + before do + stub_pages_object_storage(::Pages::DeploymentUploader, enabled: true) + end + + let!(:item) { create(:pages_deployment, file_store: start_store) } + + it_behaves_like 'local and remote storage migration' +end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 56afe49e15f..b9cfd95f89c 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1010,7 +1010,7 @@ RSpec.describe Namespace do end end - describe '#all_projects' do + shared_examples '#all_projects' do context 'when namespace is a group' do let(:namespace) { create(:group) } let(:child) { create(:group, parent: namespace) } @@ -1019,12 +1019,6 @@ RSpec.describe Namespace do it { expect(namespace.all_projects.to_a).to match_array([project2, project1]) } it { expect(child.all_projects.to_a).to match_array([project2]) } - - it 'queries for the namespace and its descendants' do - expect(Project).to receive(:where).with(namespace: [namespace, child]) - - namespace.all_projects - end end context 'when namespace is a user namespace' do @@ -1033,12 +1027,20 @@ RSpec.describe Namespace do let_it_be(:project) { create(:project, namespace: user_namespace) } it { expect(user_namespace.all_projects.to_a).to match_array([project]) } + end + end - it 'only queries for the namespace itself' do - expect(Project).to receive(:where).with(namespace: user_namespace) + describe '#all_projects' do + context 'when recursive approach is enabled' do + include_examples '#all_projects' + end - user_namespace.all_projects + context 'when recursive approach is disabled' do + before do + stub_feature_flags(recursive_approach_for_all_projects: false) end + + include_examples '#all_projects' end end diff --git a/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb new file mode 100644 index 00000000000..27ca27a9035 --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/local_and_remote_storage_migration_shared_examples.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'local and remote storage migration' do + let(:logger) { Logger.new("/dev/null") } + let(:migrater) { described_class.new(logger) } + + using RSpec::Parameterized::TableSyntax + + where(:start_store, :end_store, :method) do + ObjectStorage::Store::LOCAL | ObjectStorage::Store::REMOTE | :migrate_to_remote_storage + ObjectStorage::Store::REMOTE | ObjectStorage::Store::REMOTE | :migrate_to_remote_storage # rubocop:disable Lint/BinaryOperatorWithIdenticalOperands + ObjectStorage::Store::REMOTE | ObjectStorage::Store::LOCAL | :migrate_to_local_storage + ObjectStorage::Store::LOCAL | ObjectStorage::Store::LOCAL | :migrate_to_local_storage # rubocop:disable Lint/BinaryOperatorWithIdenticalOperands + end + + with_them do + let(:storage_name) { end_store == ObjectStorage::Store::REMOTE ? 'object' : 'local' } + + it 'successfully migrates' do + expect(logger).to receive(:info).with("Starting transfer to #{storage_name} storage") + + if start_store != end_store + expect(logger).to receive(:info).with("Transferred #{item.class.name} ID #{item.id} with size #{item.size} to #{storage_name} storage") + end + + expect(item.file_store).to eq(start_store) + + migrater.send(method) + + expect(item.reload.file_store).to eq(end_store) + end + end + + context 'when migration fails' do + let(:start_store) { ObjectStorage::Store::LOCAL } + + it 'prints error' do + expect_next_instance_of(item.file.class) do |file| + expect(file).to receive(:migrate!).and_raise("error message") + end + + expect(logger).to receive(:info).with("Starting transfer to object storage") + + expect(logger).to receive(:warn).with("Failed to transfer #{item.class.name} ID #{item.id} with error: error message") + + expect(item.file_store).to eq(start_store) + + migrater.migrate_to_remote_storage + + expect(item.reload.file_store).to eq(start_store) + end + end +end -- cgit v1.2.3