Merge branch 'migrate-old-artifacts' into 'master'

Fix data inconsistency issue for old artifacts by moving them to a currently used path

Closes #32036

See merge request !11824

3 files changed, 211 insertions, 0 deletions

diff --git a/spec/migrations/migrate_old_artifacts_spec.rb b/spec/migrations/migrate_old_artifacts_spec.rb
new file mode 100644
index 00000000000..50f4bbda001
--- /dev/null
+++ b/spec/migrations/migrate_old_artifacts_spec.rb
@@ -0,0 +1,117 @@
+# encoding: utf-8
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20170523083112_migrate_old_artifacts.rb')
+
+describe MigrateOldArtifacts do
+  let(:migration) { described_class.new }
+  let!(:directory) { Dir.mktmpdir }
+
+  before do
+    allow(Gitlab.config.artifacts).to receive(:path).and_return(directory)
+  end
+  
+  after do
+    FileUtils.remove_entry_secure(directory)
+  end
+
+  context 'with migratable data' do
+    let(:project1) { create(:empty_project, ci_id: 2) }
+    let(:project2) { create(:empty_project, ci_id: 3) }
+    let(:project3) { create(:empty_project) }
+
+    let(:pipeline1) { create(:ci_empty_pipeline, project: project1) }
+    let(:pipeline2) { create(:ci_empty_pipeline, project: project2) }
+    let(:pipeline3) { create(:ci_empty_pipeline, project: project3) }
+
+    let!(:build_with_legacy_artifacts) { create(:ci_build, pipeline: pipeline1) }
+    let!(:build_without_artifacts) { create(:ci_build, pipeline: pipeline1) }
+    let!(:build2) { create(:ci_build, :artifacts, pipeline: pipeline2) }
+    let!(:build3) { create(:ci_build, :artifacts, pipeline: pipeline3) }
+
+    before do
+      store_artifacts_in_legacy_path(build_with_legacy_artifacts)
+    end
+
+    it "legacy artifacts are not accessible" do
+      expect(build_with_legacy_artifacts.artifacts?).to be_falsey
+    end
+
+    it "legacy artifacts are set" do
+      expect(build_with_legacy_artifacts.artifacts_file_identifier).not_to be_nil
+    end
+
+    describe '#min_id' do
+      subject { migration.send(:min_id) }
+
+      it 'returns the newest build for which ci_id is not defined' do
+        is_expected.to eq(build3.id)
+      end
+    end
+
+    describe '#builds_with_artifacts' do
+      subject { migration.send(:builds_with_artifacts).map(&:id) }
+
+      it 'returns a list of builds that has artifacts and could be migrated' do
+        is_expected.to contain_exactly(build_with_legacy_artifacts.id, build2.id)
+      end
+    end
+
+    describe '#up' do
+      context 'when migrating artifacts' do
+        before do
+          migration.up
+        end
+
+        it 'all files do have artifacts' do
+          Ci::Build.with_artifacts do |build|
+            expect(build).to have_artifacts
+          end
+        end
+
+        it 'artifacts are no longer present on legacy path' do
+          expect(File.exist?(legacy_path(build_with_legacy_artifacts))).to eq(false)
+        end
+      end
+
+      context 'when there are aritfacts in old and new directory' do
+        before do
+          store_artifacts_in_legacy_path(build2)
+
+          migration.up
+        end
+
+        it 'does not move old files' do
+          expect(File.exist?(legacy_path(build2))).to eq(true)
+        end
+      end
+    end
+
+    private
+
+    def store_artifacts_in_legacy_path(build)
+      FileUtils.mkdir_p(legacy_path(build))
+
+      FileUtils.copy(
+        Rails.root.join('spec/fixtures/ci_build_artifacts.zip'),
+        File.join(legacy_path(build), "ci_build_artifacts.zip"))
+  
+      FileUtils.copy(
+        Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'),
+        File.join(legacy_path(build), "ci_build_artifacts_metadata.gz"))
+
+      build.update_columns(
+        artifacts_file: 'ci_build_artifacts.zip',
+        artifacts_metadata: 'ci_build_artifacts_metadata.gz')
+
+      build.reload
+    end
+
+    def legacy_path(build)
+      File.join(directory,
+        build.created_at.utc.strftime('%Y_%m'),
+        build.project.ci_id.to_s,
+        build.id.to_s)
+    end
+  end
+end
diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/artifact_uploader_spec.rb
new file mode 100644
index 00000000000..24e2e3a9f0e
--- /dev/null
+++ b/spec/uploaders/artifact_uploader_spec.rb
@@ -0,0 +1,38 @@
+require 'rails_helper'
+
+describe ArtifactUploader do
+  let(:job) { create(:ci_build) }
+  let(:uploader) { described_class.new(job, :artifacts_file) }
+  let(:path) { Gitlab.config.artifacts.path }
+
+  describe '.local_artifacts_store' do
+    subject { described_class.local_artifacts_store }
+
+    it "delegate to artifacts path" do
+      expect(Gitlab.config.artifacts).to receive(:path)
+
+      subject
+    end
+  end
+
+  describe '.artifacts_upload_path' do
+    subject { described_class.artifacts_upload_path }
+    
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with('tmp/uploads/') }
+  end
+
+  describe '#store_dir' do
+    subject { uploader.store_dir }
+    
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with("#{job.project_id}/#{job.id}") }
+  end
+
+  describe '#cache_dir' do
+    subject { uploader.cache_dir }
+    
+    it { is_expected.to start_with(path) }
+    it { is_expected.to end_with('tmp/cache') }
+  end
+end
diff --git a/spec/uploaders/gitlab_uploader_spec.rb b/spec/uploaders/gitlab_uploader_spec.rb
new file mode 100644
index 00000000000..78e9d9cf46c
--- /dev/null
+++ b/spec/uploaders/gitlab_uploader_spec.rb
@@ -0,0 +1,56 @@
+require 'rails_helper'
+require 'carrierwave/storage/fog'
+
+describe GitlabUploader do
+  let(:uploader_class) { Class.new(described_class) }
+
+  subject { uploader_class.new }
+
+  describe '#file_storage?' do
+    context 'when file storage is used' do
+      before do
+        uploader_class.storage(:file)
+      end
+
+      it { is_expected.to be_file_storage }
+    end
+
+    context 'when is remote storage' do
+      before do
+        uploader_class.storage(:fog)
+      end
+
+      it { is_expected.not_to be_file_storage }
+    end
+  end
+
+  describe '#file_cache_storage?' do
+    context 'when file storage is used' do
+      before do
+        uploader_class.cache_storage(:file)
+      end
+
+      it { is_expected.to be_file_cache_storage }
+    end
+
+    context 'when is remote storage' do
+      before do
+        uploader_class.cache_storage(:fog)
+      end
+
+      it { is_expected.not_to be_file_cache_storage }
+    end
+  end
+
+  describe '#move_to_cache' do
+    it 'is true' do
+      expect(subject.move_to_cache).to eq(true)
+    end
+  end
+
+  describe '#move_to_store' do
+    it 'is true' do
+      expect(subject.move_to_store).to eq(true)
+    end
+  end
+end