From 52c3b8f31264230814d2ffa79d0987c1491676b3 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Jun 2017 05:29:35 +0000 Subject: Merge branch 'zj-object-store-artifacts' into 'master' Object store for artifacts Closes gitlab-ce#29203 See merge request !1762 --- spec/uploaders/artifact_uploader_spec.rb | 27 +++- spec/uploaders/object_store_uploader_spec.rb | 231 +++++++++++++++++++++++++++ 2 files changed, 252 insertions(+), 6 deletions(-) create mode 100644 spec/uploaders/object_store_uploader_spec.rb (limited to 'spec/uploaders') diff --git a/spec/uploaders/artifact_uploader_spec.rb b/spec/uploaders/artifact_uploader_spec.rb index 2a3bd0e3bb2..f4ba4a8207f 100644 --- a/spec/uploaders/artifact_uploader_spec.rb +++ b/spec/uploaders/artifact_uploader_spec.rb @@ -1,9 +1,10 @@ require 'rails_helper' describe ArtifactUploader do - let(:job) { create(:ci_build) } + let(:store) { described_class::LOCAL_STORE } + let(:job) { create(:ci_build, artifacts_file_store: store) } let(:uploader) { described_class.new(job, :artifacts_file) } - let(:path) { Gitlab.config.artifacts.path } + let(:local_path) { Gitlab.config.artifacts.path } describe '.local_artifacts_store' do subject { described_class.local_artifacts_store } @@ -17,16 +18,30 @@ describe ArtifactUploader do describe '.artifacts_upload_path' do subject { described_class.artifacts_upload_path } - - it { is_expected.to start_with(path) } + + it { is_expected.to start_with(local_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}") } + let(:path) { "#{job.created_at.utc.strftime('%Y_%m')}/#{job.project_id}/#{job.id}" } + + context 'when using local storage' do + it { is_expected.to start_with(local_path) } + it { is_expected.to end_with(path) } + end + + context 'when using remote storage' do + let(:store) { described_class::REMOTE_STORE } + + before do + stub_artifacts_object_storage + end + + it { is_expected.to eq(path) } + end end describe '#cache_dir' do diff --git a/spec/uploaders/object_store_uploader_spec.rb b/spec/uploaders/object_store_uploader_spec.rb new file mode 100644 index 00000000000..c6c7d47e703 --- /dev/null +++ b/spec/uploaders/object_store_uploader_spec.rb @@ -0,0 +1,231 @@ +require 'rails_helper' +require 'carrierwave/storage/fog' + +describe ObjectStoreUploader do + let(:uploader_class) { Class.new(described_class) } + let(:object) { double } + let(:uploader) { uploader_class.new(object, :artifacts_file) } + + describe '#object_store' do + it "calls artifacts_file_store on object" do + expect(object).to receive(:artifacts_file_store) + + uploader.object_store + end + end + + describe '#object_store=' do + it "calls artifacts_file_store= on object" do + expect(object).to receive(:artifacts_file_store=).with(described_class::REMOTE_STORE) + + uploader.object_store = described_class::REMOTE_STORE + end + end + + context 'when using ArtifactsUploader' do + let(:job) { create(:ci_build, :artifacts, artifacts_file_store: store) } + let(:uploader) { job.artifacts_file } + + context 'checking described_class' do + let(:store) { described_class::LOCAL_STORE } + + it "uploader is of a described_class" do + expect(uploader).to be_a(described_class) + end + end + + describe '#use_file' do + context 'when file is stored locally' do + let(:store) { described_class::LOCAL_STORE } + + it "calls a regular path" do + expect { |b| uploader.use_file(&b) }.not_to yield_with_args(/tmp\/cache/) + end + end + + context 'when file is stored remotely' do + let(:store) { described_class::REMOTE_STORE } + + before do + stub_artifacts_object_storage + end + + it "calls a cache path" do + expect { |b| uploader.use_file(&b) }.to yield_with_args(/tmp\/cache/) + end + end + end + + describe '#migrate!' do + let(:job) { create(:ci_build, :artifacts, artifacts_file_store: store) } + let(:uploader) { job.artifacts_file } + let(:store) { described_class::LOCAL_STORE } + + subject { uploader.migrate!(new_store) } + + context 'when using the same storage' do + let(:new_store) { store } + + it "to not migrate the storage" do + subject + + expect(uploader.object_store).to eq(store) + end + end + + context 'when migrating to local storage' do + let(:store) { described_class::REMOTE_STORE } + let(:new_store) { described_class::LOCAL_STORE } + + before do + stub_artifacts_object_storage + end + + it "local file does not exist" do + expect(File.exist?(uploader.path)).to eq(false) + end + + it "does migrate the file" do + subject + + expect(uploader.object_store).to eq(new_store) + expect(File.exist?(uploader.path)).to eq(true) + end + end + + context 'when migrating to remote storage' do + let(:new_store) { described_class::REMOTE_STORE } + let!(:current_path) { uploader.path } + + it "file does exist" do + expect(File.exist?(current_path)).to eq(true) + end + + context 'when storage is disabled' do + before do + stub_artifacts_object_storage(enabled: false) + end + + it "to raise an error" do + expect { subject }.to raise_error(/Object Storage is not enabled/) + end + end + + context 'when credentials are set' do + before do + stub_artifacts_object_storage + end + + it "does migrate the file" do + subject + + expect(uploader.object_store).to eq(new_store) + expect(File.exist?(current_path)).to eq(false) + end + + it "does delete original file" do + subject + + expect(File.exist?(current_path)).to eq(false) + end + + context 'when subject save fails' do + before do + expect(job).to receive(:save!).and_raise(RuntimeError, "exception") + end + + it "does catch an error" do + expect { subject }.to raise_error(/exception/) + end + + it "original file is not removed" do + begin + subject + rescue + end + + expect(File.exist?(current_path)).to eq(true) + end + end + end + end + end + end + + describe '#fog_directory' do + let(:remote_directory) { 'directory' } + + before do + uploader_class.storage_options double( + object_store: double(remote_directory: remote_directory)) + end + + subject { uploader.fog_directory } + + it { is_expected.to eq(remote_directory) } + end + + describe '#fog_credentials' do + let(:connection) { 'connection' } + + before do + uploader_class.storage_options double( + object_store: double(connection: connection)) + end + + subject { uploader.fog_credentials } + + it { is_expected.to eq(connection) } + end + + describe '#fog_public' do + subject { uploader.fog_public } + + it { is_expected.to eq(false) } + end + + describe '#verify_license!' do + subject { uploader.verify_license!(nil) } + + context 'when using local storage' do + before do + expect(object).to receive(:artifacts_file_store) { described_class::LOCAL_STORE } + end + + it "does not raise an error" do + expect { subject }.not_to raise_error + end + end + + context 'when using remote storage' do + let(:project) { double } + + before do + uploader_class.storage_options double( + object_store: double(enabled: true)) + expect(object).to receive(:artifacts_file_store) { described_class::REMOTE_STORE } + expect(object).to receive(:project) { project } + end + + context 'feature is not available' do + before do + expect(project).to receive(:feature_available?).with(:object_storage) { false } + end + + it "does raise an error" do + expect { subject }.to raise_error(/Object Storage feature is missing/) + end + end + + context 'feature is available' do + before do + expect(project).to receive(:feature_available?).with(:object_storage) { true } + end + + it "does not raise an error" do + expect { subject }.not_to raise_error + end + end + end + end +end -- cgit v1.2.3