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

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

EE: Trace as artifacts (FileStorage and ObjectStorage)

Closes #4171

See merge request gitlab-org/gitlab-ee!4258
parent 72d43764af14
class Projects::JobsController < Projects::ApplicationController
prepend EE::Projects::JobsController
before_action :build, except: [:index, :cancel_all]
before_action :authorize_read_build!,
......
......@@ -20,6 +20,7 @@ class Build < CommitStatus
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
......
......@@ -16,9 +16,12 @@ class JobArtifact < ActiveRecord::Base
end
end
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 @@ def remove_artifacts_metadata!
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
class JobArtifactUploader < GitlabUploader
prepend EE::JobArtifactUploader
extend Workhorse::UploadPath
include ObjectStorage::Concern
......@@ -14,6 +15,12 @@ def store_dir
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`
......
module EE
module Projects
module JobsController
extend ActiveSupport::Concern
include SendFileUpload
def raw
if trace_artifact_file
send_upload(trace_artifact_file,
send_params: raw_send_params,
redirect_params: raw_redirect_params)
else
super
end
end
private
def raw_send_params
{ type: 'text/plain; charset=utf-8', disposition: 'inline' }
end
def raw_redirect_params
{ query: { 'response-content-type' => 'text/plain; charset=utf-8', 'response-content-disposition' => 'inline' } }
end
def trace_artifact_file
@trace_artifact_file ||= build.job_artifacts_trace&.file
end
end
end
end
......@@ -9,11 +9,11 @@ module Ci::JobArtifact
prepended do
after_destroy :log_geo_event
scope :with_files_stored_locally, -> { where(file_store: [nil, JobArtifactUploader::Store::LOCAL]) }
scope :with_files_stored_locally, -> { where(file_store: [nil, ::JobArtifactUploader::Store::LOCAL]) }
end
def local_store?
[nil, JobArtifactUploader::Store::LOCAL].include?(self.file_store)
[nil, ::JobArtifactUploader::Store::LOCAL].include?(self.file_store)
end
private
......
module EE
module JobArtifactUploader
extend ActiveSupport::Concern
def open
if file_storage?
super
else
::Gitlab::Ci::Trace::HttpIO.new(url, size) if url
end
end
end
end
##
# This class is compatible with IO class (https://ruby-doc.org/core-2.3.1/IO.html)
# source: https://gitlab.com/snippets/1685610
module Gitlab
module Ci
class Trace
class HttpIO
BUFFER_SIZE = 128.kilobytes
InvalidURLError = Class.new(StandardError)
FailedToGetChunkError = Class.new(StandardError)
attr_reader :uri, :size
attr_reader :tell
attr_reader :chunk, :chunk_range
alias_method :pos, :tell
def initialize(url, size)
raise InvalidURLError unless ::Gitlab::UrlSanitizer.valid?(url)
@uri = URI(url)
@size = size
@tell = 0
end
def close
# no-op
end
def binmode
# no-op
end
def binmode?
true
end
def path
@uri.to_s
end
def seek(pos, where = IO::SEEK_SET)
new_pos =
case where
when IO::SEEK_END
size + pos
when IO::SEEK_SET
pos
when IO::SEEK_CUR
tell + pos
else
-1
end
raise 'new position is outside of file' if new_pos < 0 || new_pos > size
@tell = new_pos
end
def eof?
tell == size
end
def each_line
until eof?
line = readline
break if line.nil?
yield(line)
end
end
def read(length = nil)
out = ""
until eof? || (length && out.length >= length)
data = get_chunk
break if data.empty?
out << data
@tell += data.bytesize
end
out = out[0, length] if length && out.length > length
out
end
def readline
out = ""
until eof?
data = get_chunk
new_line = data.index("\n")
if !new_line.nil?
out << data[0..new_line]
@tell += new_line + 1
break
else
out << data
@tell += data.bytesize
end
end
out
end
def write(data)
raise NotImplementedError
end
def truncate(offset)
raise NotImplementedError
end
def flush
raise NotImplementedError
end
def present?
true
end
private
##
# The below methods are not implemented in IO class
#
def in_range?
@chunk_range&.include?(tell)
end
def get_chunk
unless in_range?
response = Net::HTTP.start(uri.hostname, uri.port, use_ssl: uri.scheme == 'https') do |http|
http.request(request)
end
raise FailedToGetChunkError unless response.code == '200' || response.code == '206'
@chunk = response.body.force_encoding(Encoding::BINARY)
@chunk_range = response.content_range
##
# Note: If provider does not return content_range, then we set it as we requested
# Provider: minio
# - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206
# - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206
# Provider: AWS
# - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206
# - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206
# Provider: GCS
# - When the file size is larger than requested Content-range, the Content-range is included in responces with Net::HTTPPartialContent 206
# - When the file size is smaller than requested Content-range, the Content-range is included in responces with Net::HTTPOK 200
@chunk_range ||= (chunk_start...(chunk_start + @chunk.length))
end
@chunk[chunk_offset..BUFFER_SIZE]
end
def request
Net::HTTP::Get.new(uri).tap do |request|
request.set_range(chunk_start, BUFFER_SIZE)
end
end
def chunk_offset
tell % BUFFER_SIZE
end
def chunk_start
(tell / BUFFER_SIZE) * BUFFER_SIZE
end
def chunk_end
[chunk_start + BUFFER_SIZE, size].min
end
end
end
end
end
......@@ -52,12 +52,14 @@ def append(data, offset)
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 @@ def write
end
def erase!
trace_artifact&.destroy
paths.each do |trace_path|
FileUtils.rm(trace_path, force: true)
end
......@@ -137,6 +141,10 @@ def deprecated_path
"#{job.id}.log"
) if job.project&.ci_id
end
def trace_artifact
job.job_artifacts_trace
end
end
end
end
......@@ -159,8 +159,19 @@ def get_show(**extra_params)
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 @@ def get_show(**extra_params)
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 @@ def post_cancel_all
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 @@ def post_cancel_all
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 @@ def post_erase
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)
......
require 'spec_helper'
describe Projects::JobsController do
include ApiHelpers
include HttpIOHelpers
let(:project) { create(:project, :public) }
let(:pipeline) { create(:ci_pipeline, project: project) }
describe 'GET trace.json' do
context 'when trace artifact is in ObjectStorage' do
let!(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) }
before do
allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
allow_any_instance_of(JobArtifactUploader).to receive(:url) { remote_trace_url }
allow_any_instance_of(JobArtifactUploader).to receive(:size) { remote_trace_size }
end
context 'when there are no network issues' do
before do
stub_remote_trace_206
get_trace
end
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 there is a network issue' do
before do
stub_remote_trace_500
end
it 'returns a trace' do
expect { get_trace }.to raise_error(Gitlab::Ci::Trace::HttpIO::FailedToGetChunkError)
end
end
end
def get_trace
get :trace, namespace_id: project.namespace,
project_id: project,
id: job.id,
format: :json
end
end
describe 'GET raw' do
subject do
post :raw, namespace_id: project.namespace,
project_id: project,
id: job.id
end
context 'when the trace artifact is in ObjectStorage' do
let!(:job) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
before do
allow_any_instance_of(JobArtifactUploader).to receive(:file_storage?) { false }
end
it 'redirect to the trace file url' do
expect(subject).to redirect_to(job.job_artifacts_trace.file.url)
end
end
end
end
require 'spec_helper'
describe Gitlab::Ci::Trace::HttpIO do
include HttpIOHelpers
let(:http_io) { described_class.new(url, size) }
let(:url) { remote_trace_url }
let(:size) { remote_trace_size }
describe 'Interchangeability between IO and HttpIO' do
EXCEPT_METHODS = %i[read_nonblock raw raw! cooked cooked! getch echo= echo?
winsize winsize= iflush oflush ioflush beep goto cursor cursor= pressed?
getpass write_nonblock stat pathconf wait_readable wait_writable getbyte <<
wait lines bytes chars codepoints getc readpartial set_encoding printf print
putc puts readlines gets each each_byte each_char each_codepoint to_io reopen
syswrite to_i fileno sysread fdatasync fsync sync= sync lineno= lineno readchar
ungetbyte readbyte ungetc nonblock= nread rewind pos= eof close_on_exec?
close_on_exec= closed? close_read close_write isatty tty? binmode? sysseek
advise ioctl fcntl pid external_encoding internal_encoding autoclose? autoclose=
posix_fileno nonblock? ready? noecho nonblock].freeze
it 'HttpIO covers core interfaces in IO' do
expected_interfaces = ::IO.instance_methods(false)
expected_interfaces -= EXCEPT_METHODS
expect(expected_interfaces - described_class.instance_methods).to be_empty
end
end
describe '#close' do
subject { http_io.close }
it { is_expected.to be_nil }
end
describe '#binmode' do
subject { http_io.binmode }
it { is_expected.to be_nil }
end
describe '#binmode?' do
subject { http_io.binmode? }