Commit 8450a8a5 authored by Kamil Trzciński's avatar Kamil Trzciński
Browse files

Merge branch 'feature/sm/artifacts-trace' into 'master'

CE: Trace as artifacts (FileStorage only)

Closes gitlab-ee#4180

See merge request gitlab-org/gitlab-ce!16702
......@@ -21,6 +21,7 @@
has_many :job_artifacts, class_name: 'Ci::JobArtifact', foreign_key: :job_id, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_one :job_artifacts_archive, -> { where(file_type: Ci::JobArtifact.file_types[:archive]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
has_one :job_artifacts_metadata, -> { where(file_type: Ci::JobArtifact.file_types[:metadata]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
has_one :job_artifacts_trace, -> { where(file_type: Ci::JobArtifact.file_types[:trace]) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
# The "environment" field for builds is a String, and is the unexpanded name
def persisted_environment
......
......@@ -9,5 +9,7 @@
mount_uploader :file, JobArtifactUploader
delegate :open, :exists?, to: :file
enum file_type: {
archive: 1,
......@@ -12,6 +14,7 @@
enum file_type: {
archive: 1,
metadata: 2
metadata: 2,
trace: 3
}
def self.artifacts_size_for(project)
......
......@@ -39,7 +39,6 @@
end
def artifacts_size
read_attribute(:artifacts_size).to_i +
job_artifacts_archive&.size.to_i + job_artifacts_metadata&.size.to_i
read_attribute(:artifacts_size).to_i + job_artifacts.sum(:size).to_i
end
end
module Ci
class CreateTraceArtifactService < BaseService
def execute(job)
return if job.job_artifacts_trace
job.trace.read do |stream|
if stream.file?
job.create_job_artifacts_trace!(
project: job.project,
file_type: :trace,
file: stream)
end
end
end
end
end
......@@ -13,6 +13,12 @@
dynamic_segment
end
def open
raise 'Only File System is supported' unless file_storage?
File.open(path, "rb") if path
end
private
def dynamic_segment
......
......@@ -43,6 +43,7 @@
- pipeline_creation:run_pipeline_schedule
- pipeline_default:build_coverage
- pipeline_default:build_trace_sections
- pipeline_default:create_trace_artifact
- pipeline_default:pipeline_metrics
- pipeline_default:pipeline_notification
- pipeline_default:update_head_pipeline_for_merge_request
......
......@@ -6,5 +6,6 @@
def perform(build_id)
Ci::Build.find_by(id: build_id).try do |build|
BuildTraceSectionsWorker.perform_async(build.id)
# We execute that in sync as this access the files in order to access local file, and reduce IO
BuildTraceSectionsWorker.new.perform(build.id)
BuildCoverageWorker.new.perform(build.id)
......@@ -10,5 +11,8 @@
BuildCoverageWorker.new.perform(build.id)
BuildHooksWorker.new.perform(build.id)
# We execute that async as this are two indepentent operations that can be executed after TraceSections and Coverage
BuildHooksWorker.perform_async(build.id)
CreateTraceArtifactWorker.perform_async(build.id)
end
end
end
class CreateTraceArtifactWorker
include ApplicationWorker
include PipelineQueue
def perform(job_id)
Ci::Build.preload(:project, :user).find_by(id: job_id).try do |job|
Ci::CreateTraceArtifactService.new(job.project, job.user).execute(job)
end
end
end
---
title: Save traces as artifacts
merge_request: 16702
author:
type: changed
......@@ -16,7 +16,7 @@
- Project avatars
- Issues/MR/Notes Markdown attachments
- Issues/MR/Notes Legacy Markdown attachments
- CI Build Artifacts
- CI Artifacts (archive, metadata, trace)
- LFS Objects
......@@ -35,7 +35,7 @@
| Project avatars | yes | uploads/-/system/project/avatar/:id/:filename | `AvatarUploader` | Project |
| Issues/MR/Notes Markdown attachments | yes | uploads/:project_path_with_namespace/:random_hex/:filename | `FileUploader` | Project |
| Issues/MR/Notes Legacy Markdown attachments | no | uploads/-/system/note/attachment/:id/:filename | `AttachmentUploader` | Note |
| CI Artifacts (CE) | yes | shared/artifacts/:year_:month/:project_id/:id | `ArtifactUploader` | Ci::Build |
| CI Artifacts (CE) | yes | shared/artifacts/:disk_hash[0..1]/:disk_hash[2..3]/:disk_hash/:year_:month_:date/:job_id/:job_artifact_id (:disk_hash is SHA256 digest of project_id) | `JobArtifactUploader` | Ci::JobArtifact |
| LFS Objects (CE) | yes | shared/lfs-objects/:hex/:hex/:object_hash | `LfsObjectUploader` | LfsObject |
CI Artifacts and LFS Objects behave differently in CE and EE. In CE they inherit the `GitlabUploader`
......
......@@ -52,8 +52,8 @@
end
def exist?
current_path.present? || old_trace.present?
trace_artifact&.exists? || current_path.present? || old_trace.present?
end
def read
stream = Gitlab::Ci::Trace::Stream.new do
......@@ -56,8 +56,10 @@
end
def read
stream = Gitlab::Ci::Trace::Stream.new do
if current_path
if trace_artifact
trace_artifact.open
elsif current_path
File.open(current_path, "rb")
elsif old_trace
StringIO.new(old_trace)
......@@ -82,6 +84,8 @@
end
def erase!
trace_artifact&.destroy
paths.each do |trace_path|
FileUtils.rm(trace_path, force: true)
end
......@@ -137,6 +141,10 @@
"#{job.id}.log"
) if job.project&.ci_id
end
def trace_artifact
job.job_artifacts_trace
end
end
end
end
......@@ -159,4 +159,15 @@
get_trace
end
context 'when job has a trace artifact' do
let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
it 'returns a trace' do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq job.id
expect(json_response['status']).to eq job.status
expect(json_response['html']).to eq(job.trace.html)
end
end
context 'when job has a trace' do
......@@ -162,5 +173,5 @@
context 'when job has a trace' do
let(:job) { create(:ci_build, :trace, pipeline: pipeline) }
let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
it 'returns a trace' do
expect(response).to have_gitlab_http_status(:ok)
......@@ -182,7 +193,7 @@
end
context 'when job has a trace with ANSI sequence and Unicode' do
let(:job) { create(:ci_build, :unicode_trace, pipeline: pipeline) }
let(:job) { create(:ci_build, :unicode_trace_live, pipeline: pipeline) }
it 'returns a trace with Unicode' do
expect(response).to have_gitlab_http_status(:ok)
......@@ -381,7 +392,7 @@
end
context 'when job is erasable' do
let(:job) { create(:ci_build, :erasable, :trace, pipeline: pipeline) }
let(:job) { create(:ci_build, :erasable, :trace_artifact, pipeline: pipeline) }
it 'redirects to the erased job page' do
expect(response).to have_gitlab_http_status(:found)
......@@ -408,7 +419,7 @@
context 'when user is developer' do
let(:role) { :developer }
let(:job) { create(:ci_build, :erasable, :trace, pipeline: pipeline, user: triggered_by) }
let(:job) { create(:ci_build, :erasable, :trace_artifact, pipeline: pipeline, user: triggered_by) }
context 'when triggered by same user' do
let(:triggered_by) { user }
......@@ -439,4 +450,14 @@
get_raw
end
context 'when job has a trace artifact' do
let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
it 'returns a trace' do
expect(response).to have_gitlab_http_status(:ok)
expect(response.content_type).to eq 'text/plain; charset=utf-8'
expect(response.body).to eq job.job_artifacts_trace.open.read
end
end
context 'when job has a trace file' do
......@@ -442,5 +463,5 @@
context 'when job has a trace file' do
let(:job) { create(:ci_build, :trace, pipeline: pipeline) }
let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
it 'send a trace file' do
expect(response).to have_gitlab_http_status(:ok)
......
......@@ -135,9 +135,9 @@
coverage_regex '/(d+)/'
end
trait :trace do
trait :trace_live do
after(:create) do |build, evaluator|
build.trace.set('BUILD TRACE')
end
end
......@@ -139,9 +139,15 @@
after(:create) do |build, evaluator|
build.trace.set('BUILD TRACE')
end
end
trait :unicode_trace do
trait :trace_artifact do
after(:create) do |build, evaluator|
create(:ci_job_artifact, :trace, job: build)
end
end
trait :unicode_trace_live do
after(:create) do |build, evaluator|
trace = File.binread(
File.expand_path(
......
......@@ -26,5 +26,14 @@
Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz'), 'application/x-gzip')
end
end
trait :trace do
file_type :trace
after(:build) do |artifact, evaluator|
artifact.file = fixture_file_upload(
Rails.root.join('spec/fixtures/trace/sample_trace'), 'text/plain')
end
end
end
end
......@@ -7,7 +7,7 @@
let(:project) { create(:project, :repository) }
let(:pipeline) { create(:ci_pipeline, project: project) }
let(:job) { create(:ci_build, :trace, pipeline: pipeline) }
let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
let(:job2) { create(:ci_build) }
let(:artifacts_file) do
......@@ -490,7 +490,19 @@
describe 'GET /:project/jobs/:id/raw', :js do
context 'access source' do
context 'job from project' do
before do
job.run!
context 'when job is running' do
before do
job.run!
end
it 'sends the right headers' do
requests = inspect_requests(inject_headers: { 'X-Sendfile-Type' => 'X-Sendfile' }) do
visit raw_project_job_path(project, job)
end
expect(requests.first.status_code).to eq(200)
expect(requests.first.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
expect(requests.first.response_headers['X-Sendfile']).to eq(job.trace.send(:current_path))
end
end
......@@ -495,6 +507,14 @@
end
it 'sends the right headers' do
requests = inspect_requests(inject_headers: { 'X-Sendfile-Type' => 'X-Sendfile' }) do
visit raw_project_job_path(project, job)
context 'when job is complete' do
let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
it 'sends the right headers' do
requests = inspect_requests(inject_headers: { 'X-Sendfile-Type' => 'X-Sendfile' }) do
visit raw_project_job_path(project, job)
end
expect(requests.first.status_code).to eq(200)
expect(requests.first.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
expect(requests.first.response_headers['X-Sendfile']).to eq(job.job_artifacts_trace.file.path)
end
......@@ -500,8 +520,4 @@
end
expect(requests.first.status_code).to eq(200)
expect(requests.first.response_headers['Content-Type']).to eq('text/plain; charset=utf-8')
expect(requests.first.response_headers['X-Sendfile']).to eq(job.trace.send(:current_path))
end
end
......
This diff is collapsed.
......@@ -7,7 +7,7 @@
let(:namespace) { create(:namespace, name: 'frontend-fixtures' )}
let(:project) { create(:project_empty_repo, namespace: namespace, path: 'builds-project') }
let(:pipeline) { create(:ci_empty_pipeline, project: project) }
let!(:build_with_artifacts) { create(:ci_build, :success, :artifacts, :trace, pipeline: pipeline, stage: 'test', artifacts_expire_at: Time.now + 18.months) }
let!(:build_with_artifacts) { create(:ci_build, :success, :artifacts, :trace_artifact, pipeline: pipeline, stage: 'test', artifacts_expire_at: Time.now + 18.months) }
let!(:failed_build) { create(:ci_build, :failed, pipeline: pipeline, stage: 'build') }
let!(:pending_build) { create(:ci_build, :pending, pipeline: pipeline, stage: 'deploy') }
......
......@@ -238,4 +238,69 @@
end
end
describe '#read' do
shared_examples 'read successfully with IO' do
it 'yields with source' do
trace.read do |stream|
expect(stream).to be_a(Gitlab::Ci::Trace::Stream)
expect(stream.stream).to be_a(IO)
end
end
end
shared_examples 'read successfully with StringIO' do
it 'yields with source' do
trace.read do |stream|
expect(stream).to be_a(Gitlab::Ci::Trace::Stream)
expect(stream.stream).to be_a(StringIO)
end
end
end
shared_examples 'failed to read' do
it 'yields without source' do
trace.read do |stream|
expect(stream).to be_a(Gitlab::Ci::Trace::Stream)
expect(stream.stream).to be_nil
end
end
end
context 'when trace artifact exists' do
before do
create(:ci_job_artifact, :trace, job: build)
end
it_behaves_like 'read successfully with IO'
end
context 'when current_path (with project_id) exists' do
before do
expect(trace).to receive(:default_path) { expand_fixture_path('trace/sample_trace') }
end
it_behaves_like 'read successfully with IO'
end
context 'when current_path (with project_ci_id) exists' do
before do
expect(trace).to receive(:deprecated_path) { expand_fixture_path('trace/sample_trace') }
end
it_behaves_like 'read successfully with IO'
end
context 'when db trace exists' do
before do
build.send(:write_attribute, :trace, "data")
end
it_behaves_like 'read successfully with StringIO'
end
context 'when no sources exist' do
it_behaves_like 'failed to read'
end
end
describe 'trace handling' do
......@@ -241,5 +306,7 @@
describe 'trace handling' do
subject { trace.exist? }
context 'trace does not exist' do
it { expect(trace.exist?).to be(false) }
end
......@@ -242,7 +309,27 @@
context 'trace does not exist' do
it { expect(trace.exist?).to be(false) }
end
context 'when trace artifact exists' do
before do
create(:ci_job_artifact, :trace, job: build)
end
it { is_expected.to be_truthy }
context 'when the trace artifact has been erased' do
before do
trace.erase!
end
it { is_expected.to be_falsy }
it 'removes associations' do
expect(Ci::JobArtifact.exists?(job_id: build.id, file_type: :trace)).to be_falsy
end
end
end
context 'new trace path is used' do
before do
trace.send(:ensure_directory)
......
......@@ -675,7 +675,7 @@
context 'build is erasable' do
context 'new artifacts' do
let!(:build) { create(:ci_build, :trace, :success, :artifacts) }
let!(:build) { create(:ci_build, :trace_artifact, :success, :artifacts) }
describe '#erase' do
before do
......@@ -709,7 +709,7 @@
end
describe '#erased?' do
let!(:build) { create(:ci_build, :trace, :success, :artifacts) }
let!(:build) { create(:ci_build, :trace_artifact, :success, :artifacts) }
subject { build.erased? }
context 'job has not been erased' do
......@@ -744,7 +744,7 @@
context 'old artifacts' do
context 'build is erasable' do
context 'new artifacts' do
let!(:build) { create(:ci_build, :trace, :success, :legacy_artifacts) }
let!(:build) { create(:ci_build, :trace_artifact, :success, :legacy_artifacts) }
describe '#erase' do
before do
......@@ -778,7 +778,7 @@
end
describe '#erased?' do
let!(:build) { create(:ci_build, :trace, :success, :legacy_artifacts) }
let!(:build) { create(:ci_build, :trace_artifact, :success, :legacy_artifacts) }
subject { build.erased? }
context 'job has not been erased' do
......
......@@ -12,6 +12,9 @@
it { is_expected.to respond_to(:created_at) }
it { is_expected.to respond_to(:updated_at) }
it { is_expected.to delegate_method(:open).to(:file) }
it { is_expected.to delegate_method(:exists?).to(:file) }
describe '#set_size' do
it 'sets the size' do
expect(artifact.size).to eq(106365)
......
Supports Markdown
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