Commit ab41e9ad authored by Kamil Trzciński's avatar Kamil Trzciński 🔴

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
parents b9d547b1 a2d79e1f
Pipeline #17157745 passed with stages
in 37 minutes and 49 seconds
......@@ -21,6 +21,7 @@ module Ci
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,9 +9,12 @@ module Ci
mount_uploader :file, JobArtifactUploader
delegate :open, :exists?, to: :file
enum file_type: {
archive: 1,
metadata: 2
metadata: 2,
trace: 3
}
def self.artifacts_size_for(project)
......
......@@ -39,7 +39,6 @@ module ArtifactMigratable
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 @@ class JobArtifactUploader < GitlabUploader
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,9 +6,13 @@ class BuildFinishedWorker
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)
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 @@ There are many places where file uploading is used, according to contexts:
- 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 @@ they are still not 100% standardized. You can see them below:
| 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,12 +52,14 @@ module Gitlab
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
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 @@ module Gitlab
end
def erase!
trace_artifact&.destroy
paths.each do |trace_path|
FileUtils.rm(trace_path, force: true)
end
......@@ -137,6 +141,10 @@ module Gitlab
"#{job.id}.log"
) if job.project&.ci_id
end
def trace_artifact
job.job_artifacts_trace
end
end
end
end
......@@ -159,8 +159,19 @@ describe Projects::JobsController do
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
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 @@ describe Projects::JobsController do
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 @@ describe Projects::JobsController do
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 @@ describe Projects::JobsController do
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,8 +450,18 @@ describe Projects::JobsController do
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
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,13 +135,19 @@ FactoryBot.define do
coverage_regex '/(d+)/'
end
trait :trace do
trait :trace_live do
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 @@ FactoryBot.define do
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 @@ feature 'Jobs' do
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,18 +490,34 @@ feature 'Jobs' do
describe 'GET /:project/jobs/:id/raw', :js do
context 'access source' do
context 'job from project' do
before do
job.run!
end
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)
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
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))
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
end
end
......
This diff is collapsed.
......@@ -7,7 +7,7 @@ describe Projects::JobsController, '(JavaScript fixtures)', type: :controller do
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,11 +238,98 @@ describe Gitlab::Ci::Trace do
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
subject { trace.exist? }
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 @@ describe Ci::Build do
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 @@ describe Ci::Build do
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 @@ describe Ci::Build do
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 @@ describe Ci::Build do
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 @@ describe Ci::JobArtifact do
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)
......
......@@ -446,16 +446,27 @@ describe API::Jobs do
end
describe 'GET /projects/:id/jobs/:job_id/trace' do
let(:job) { create(:ci_build, :trace, pipeline: pipeline) }
before do
get api("/projects/#{project.id}/jobs/#{job.id}/trace", api_user)
end
context 'authorized user' do
it 'returns specific job trace' do
expect(response).to have_gitlab_http_status(200)
expect(response.body).to eq(job.trace.raw)
context 'when trace is artifact' do
let(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
it 'returns specific job trace' do
expect(response).to have_gitlab_http_status(200)
expect(response.body).to eq(job.trace.raw)
end
end
context 'when trace is file' do
let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
it 'returns specific job trace' do
expect(response).to have_gitlab_http_status(200)
expect(response.body).to eq(job.trace.raw)
end
end
end
......@@ -543,11 +554,11 @@ describe API::Jobs do
end
context 'job is erasable' do
let(:job) { create(:ci_build, :trace, :artifacts, :success, project: project, pipeline: pipeline) }
let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline) }
it 'erases job content' do
expect(response).to have_gitlab_http_status(201)
expect(job).not_to have_trace
expect(job.trace.exist?).to be_falsy
expect(job.artifacts_file.exists?).to be_falsy
expect(job.artifacts_metadata.exists?).to be_falsy
end
......@@ -561,7 +572,7 @@ describe API::Jobs do
end
context 'job is not erasable' do
let(:job) { create(:ci_build, :trace, project: project, pipeline: pipeline) }
let(:job) { create(:ci_build, :trace_live, project: project, pipeline: pipeline) }
it 'responds with forbidden' do
expect(response).to have_gitlab_http_status(403)
......@@ -570,7 +581,7 @@ describe API::Jobs do
context 'when a developer erases a build' do
let(:role) { :developer }
let(:job) { create(:ci_build, :trace, :artifacts, :success, project: project, pipeline: pipeline, user: owner) }
let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline, user: owner) }
context 'when the build was created by the developer' do
let(:owner) { user }
......@@ -593,7 +604,7 @@ describe API::Jobs do
context 'artifacts did not expire' do
let(:job) do
create(:ci_build, :trace, :artifacts, :success,
create(:ci_build, :trace_artifact, :artifacts, :success,
project: project, pipeline: pipeline, artifacts_expire_at: Time.now + 7.days)
end
......
......@@ -638,7 +638,7 @@ describe API::Runner do
end
describe 'PUT /api/v4/jobs/:id' do
let(:job) { create(:ci_build, :pending, :trace, pipeline: pipeline, runner_id: runner.id) }
let(:job) { create(:ci_build, :pending, :trace_live, pipeline: pipeline, runner_id: runner.id) }
before do
job.run!
......@@ -680,11 +680,17 @@ describe API::Runner do
end
context 'when tace is given' do
it 'updates a running build' do
update_job(trace: 'BUILD TRACE UPDATED')
it 'creates a trace artifact' do
allow_any_instance_of(BuildFinishedWorker).to receive(:perform).with(job.id) do
CreateTraceArtifactWorker.new.perform(job.id)
end
update_job(state: 'success', trace: 'BUILD TRACE UPDATED')
job.reload
expect(response).to have_gitlab_http_status(200)
expect(job.reload.trace.raw).to eq 'BUILD TRACE UPDATED'
expect(job.trace.raw).to eq 'BUILD TRACE UPDATED'
expect(job.job_artifacts_trace.open.read).to eq 'BUILD TRACE UPDATED'
end
end
......@@ -713,7 +719,7 @@ describe API::Runner do
end
describe 'PATCH /api/v4/jobs/:id/trace' do
let(:job) { create(:ci_build, :running, :trace, runner_id: runner.id, pipeline: pipeline) }
let(:job) { create(:ci_build, :running, :trace_live, runner_id: runner.id, pipeline: pipeline) }
let(:headers) { { API::Helpers::Runner::JOB_TOKEN_HEADER => job.token, 'Content-Type' => 'text/plain' } }
let(:headers_with_range) { headers.merge({ 'Content-Range' => '11-20' }) }
let(:update_interval) { 10.seconds.to_i }
......@@ -774,7 +780,7 @@ describe API::Runner do
context 'when project for the build has been deleted' do
let(:job) do
create(:ci_build, :running, :trace, runner_id: runner.id, pipeline: pipeline) do |job|
create(:ci_build, :running, :trace_live, runner_id: runner.id, pipeline: pipeline) do |job|
job.project.update(pending_delete: true)
end
end
......
......@@ -352,7 +352,7 @@ describe API::V3::Builds do
end
describe 'GET /projects/:id/builds/:build_id/trace' do
let(:build) { create(:ci_build, :trace, pipeline: pipeline) }
let(:build) { create(:ci_build, :trace_live, pipeline: pipeline) }
before do
get v3_api("/projects/#{project.id}/builds/#{build.id}/trace", api_user)
......@@ -447,7 +447,7 @@ describe API::V3::Builds do
end
context 'job is erasable' do
let(:build) { create(:ci_build, :trace, :artifacts, :success, project: project, pipeline: pipeline) }
let(:build) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline) }
it 'erases job content' do
expect(response.status).to eq 201
......@@ -463,7 +463,7 @@ describe API::V3::Builds do
end
context 'job is not erasable' do
let(:build) { create(:ci_build, :trace, project: project, pipeline: pipeline) }
let(:build) { create(:ci_build, :trace_live, project: project, pipeline: pipeline) }
it 'responds with forbidden' do
expect(response.status).to eq 403
......@@ -478,7 +478,7 @@ describe API::V3::Builds do
context 'artifacts did not expire' do
let(:build) do
create(:ci_build, :trace, :artifacts, :success,
create(:ci_build, :trace_artifact, :artifacts, :success,
project: project, pipeline: pipeline, artifacts_expire_at: Time.now + 7.days)
end
......
require 'spec_helper'
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') }
allow_any_instance_of(JobArtifactUploader).to receive(:move_to_cache) { false }
allow_any_instance_of(JobArtifactUploader).to receive(:move_to_store) { false }
end
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')
end
context 'when the job has already had trace artifact' do
before do
create(:ci_job_artifact, :trace, job: job)
end
it 'does not create trace artifact' do
expect { subject }.not_to change { Ci::JobArtifact.count }
end
end
end
context 'when the job does not have a trace file' do
it 'does not create trace artifact' do
expect { subject }.not_to change { Ci::JobArtifact.count }
end
end
end
end
end
......@@ -17,7 +17,8 @@ describe Ci::RetryBuildService do
%i[id status user token coverage trace runner artifacts_expire_at
artifacts_file artifacts_metadata artifacts_size created_at
updated_at started_at finished_at queued_at erased_by
erased_at auto_canceled_by job_artifacts job_artifacts_archive job_artifacts_metadata].freeze
erased_at auto_canceled_by job_artifacts job_artifacts_archive
job_artifacts_metadata job_artifacts_trace].freeze
IGNORE_ACCESSORS =
%i[type lock_version target_url base_tags trace_sections
......@@ -36,7 +37,7 @@ describe Ci::RetryBuildService do
let(:build) do
create(:ci_build, :failed, :artifacts, :expired, :erased,
:queued, :coverage, :tags, :allowed_to_fail, :on_tag,
:triggered, :trace, :teardown_environment,
:triggered, :trace_artifact, :teardown_environment,
description: 'my-job', stage: 'test', pipeline: pipeline,
auto_canceled_by: create(:ci_empty_pipeline, project: project)) do |build|
##
......
......@@ -11,6 +11,33 @@ describe JobArtifactUploader do
cache_dir: %r[artifacts/tmp/cache],
work_dir: %r[artifacts/tmp/work]