Commit 0fca3bce authored by Shinya Maeda's avatar Shinya Maeda

Copy original file to temp file always to prevent data loss

parent 801e4271
......@@ -8,7 +8,7 @@ module Ci
temp_file!(JobArtifactUploader.workhorse_upload_path) do |temp_path|
FileUtils.cp(stream.path, temp_path)
create_job_trace!(temp_path)
create_job_trace!(job, temp_path)
FileUtils.rm(stream.path)
end
end
......@@ -16,17 +16,17 @@ module Ci
private
def create_job_trace!(path)
def create_job_trace!(job, path)
job.create_job_artifacts_trace!(
project: job.project,
file_type: :trace,
file: UploadedFile.new(path, 'build.log', 'application/octet-stream')
file: UploadedFile.new(path, 'job.log', 'application/octet-stream')
)
end
def temp_file!(temp_dir)
FileUtils.mkdir_p(temp_dir)
temp_file = Tempfile.new('file', temp_dir)
temp_file = Tempfile.new('legacy-trace-tmp-', temp_dir)
temp_file&.close
yield(temp_file.path)
ensure
......
......@@ -4,40 +4,56 @@ describe Ci::CreateTraceArtifactService do
describe '#execute' do
subject { described_class.new(nil, nil).execute(job) }
let(:job) { create(:ci_build) }
context 'when the job does not have trace artifact' do
context 'when the job has a trace file' do
before do
allow_any_instance_of(Gitlab::Ci::Trace)
.to receive(:default_path) { expand_fixture_path('trace/sample_trace') }
let!(:job) { create(:ci_build, :trace_live) }
let!(:legacy_path) { job.trace.read { |stream| return stream.path } }
allow_any_instance_of(JobArtifactUploader).to receive(:move_to_cache) { false }
allow_any_instance_of(JobArtifactUploader).to receive(:move_to_store) { false }
end
it { expect(File.exists?(legacy_path)).to be_truthy }
it 'creates trace artifact' do
expect { subject }.to change { Ci::JobArtifact.count }.by(1)
expect(job.job_artifacts_trace.read_attribute(:file)).to eq('sample_trace')
expect(File.exists?(legacy_path)).to be_falsy
expect(File.exists?(job.job_artifacts_trace.file.path)).to be_truthy
expect(job.job_artifacts_trace.exists?).to be_truthy
expect(job.job_artifacts_trace.file.filename).to eq('job.log')
end
context 'when the job has already had trace artifact' do
context 'when failed to create trace artifact record' do
before do
create(:ci_job_artifact, :trace, job: job)
# When ActiveRecord error happens
allow_any_instance_of(Ci::JobArtifact).to receive(:save).and_return(false)
allow_any_instance_of(Ci::JobArtifact).to receive_message_chain(:errors, :full_messages)
.and_return("Error")
subject rescue nil
job.reload
end
it 'does not create trace artifact' do
expect { subject }.not_to change { Ci::JobArtifact.count }
it 'keeps legacy trace and removes trace artifact' do
expect(File.exists?(legacy_path)).to be_truthy
expect(job.job_artifacts_trace).to be_nil
end
end
end
context 'when the job does not have a trace file' do
let!(:job) { create(:ci_build) }
it 'does not create trace artifact' do
expect { subject }.not_to change { Ci::JobArtifact.count }
end
end
end
context 'when the job has already had trace artifact' do
let!(:job) { create(:ci_build, :trace_artifact) }
it 'does not create trace artifact' do
expect { subject }.not_to change { Ci::JobArtifact.count }
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment