Commit 1a36493d authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'save-artifacts_sizes' into 'master'

Save artifacts sizes

## What does this MR do?

Introduce ci_builds.artifacts_size as an integer, so that it's easier to access than reading from the file again.

## What are the relevant issue numbers?

Closes #18869

See merge request !4964
parents f60b48bd 36a73929
...@@ -10,6 +10,7 @@ v 8.10.0 (unreleased) ...@@ -10,6 +10,7 @@ v 8.10.0 (unreleased)
- Display last commit of deleted branch in push events !4699 (winniehell) - Display last commit of deleted branch in push events !4699 (winniehell)
- Apply the trusted_proxies config to the rack request object for use with rack_attack - Apply the trusted_proxies config to the rack request object for use with rack_attack
- Add Sidekiq queue duration to transaction metrics. - Add Sidekiq queue duration to transaction metrics.
- Add a new column `artifacts_size` to table `ci_builds` !4964
- Let Workhorse serve format-patch diffs - Let Workhorse serve format-patch diffs
- Make images fit to the size of the viewport !4810 - Make images fit to the size of the viewport !4810
- Fix check for New Branch button on Issue page !4630 (winniehell) - Fix check for New Branch button on Issue page !4630 (winniehell)
......
...@@ -19,6 +19,7 @@ module Ci ...@@ -19,6 +19,7 @@ module Ci
acts_as_taggable acts_as_taggable
before_save :update_artifacts_size, if: :artifacts_file_changed?
before_destroy { project } before_destroy { project }
after_create :execute_hooks after_create :execute_hooks
...@@ -329,7 +330,12 @@ module Ci ...@@ -329,7 +330,12 @@ module Ci
end end
def artifacts_metadata_entry(path, **options) def artifacts_metadata_entry(path, **options)
Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path, **options).to_entry metadata = Gitlab::Ci::Build::Artifacts::Metadata.new(
artifacts_metadata.path,
path,
**options)
metadata.to_entry
end end
def erase_artifacts! def erase_artifacts!
...@@ -375,6 +381,14 @@ module Ci ...@@ -375,6 +381,14 @@ module Ci
private private
def update_artifacts_size
self.artifacts_size = if artifacts_file.exists?
artifacts_file.size
else
nil
end
end
def erase_trace! def erase_trace!
self.trace = nil self.trace = nil
end end
......
class AddArtifactsSizeToCiBuilds < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
def change
add_column(:ci_builds, :artifacts_size, :integer)
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20160620115026) do ActiveRecord::Schema.define(version: 20160628085157) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -164,6 +164,7 @@ ActiveRecord::Schema.define(version: 20160620115026) do ...@@ -164,6 +164,7 @@ ActiveRecord::Schema.define(version: 20160620115026) do
t.datetime "erased_at" t.datetime "erased_at"
t.string "environment" t.string "environment"
t.datetime "artifacts_expire_at" t.datetime "artifacts_expire_at"
t.integer "artifacts_size"
end end
add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree
......
...@@ -195,8 +195,7 @@ module Ci ...@@ -195,8 +195,7 @@ module Ci
not_found! unless build not_found! unless build
authenticate_build_token!(build) authenticate_build_token!(build)
build.remove_artifacts_file! build.erase_artifacts!
build.remove_artifacts_metadata!
end end
end end
end end
......
...@@ -293,33 +293,52 @@ describe Ci::API::API do ...@@ -293,33 +293,52 @@ describe Ci::API::API do
allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/') allow(ArtifactUploader).to receive(:artifacts_upload_path).and_return('/')
end end
context 'build has been erased' do describe 'build has been erased' do
let(:build) { create(:ci_build, erased_at: Time.now) } let(:build) { create(:ci_build, erased_at: Time.now) }
before { upload_artifacts(file_upload, headers_with_token) }
before do
upload_artifacts(file_upload, headers_with_token)
end
it 'should respond with forbidden' do it 'should respond with forbidden' do
expect(response.status).to eq 403 expect(response.status).to eq 403
end end
end end
context "should post artifact to running build" do describe 'uploading artifacts for a running build' do
it "uses regual file post" do shared_examples 'successful artifacts upload' do
upload_artifacts(file_upload, headers_with_token, false) it 'updates successfully' do
response_filename =
json_response['artifacts_file']['filename']
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(json_response["artifacts_file"]["filename"]).to eq(file_upload.original_filename) expect(response_filename).to eq(file_upload.original_filename)
end
end
context 'uses regular file post' do
before do
upload_artifacts(file_upload, headers_with_token, false)
end
it_behaves_like 'successful artifacts upload'
end end
it "uses accelerated file post" do context 'uses accelerated file post' do
before do
upload_artifacts(file_upload, headers_with_token, true) upload_artifacts(file_upload, headers_with_token, true)
expect(response).to have_http_status(201)
expect(json_response["artifacts_file"]["filename"]).to eq(file_upload.original_filename)
end end
it "updates artifact" do it_behaves_like 'successful artifacts upload'
upload_artifacts(file_upload, headers_with_token) end
context 'updates artifact' do
before do
upload_artifacts(file_upload2, headers_with_token) upload_artifacts(file_upload2, headers_with_token)
expect(response).to have_http_status(201) upload_artifacts(file_upload, headers_with_token)
expect(json_response["artifacts_file"]["filename"]).to eq(file_upload2.original_filename) end
it_behaves_like 'successful artifacts upload'
end end
end end
...@@ -329,6 +348,7 @@ describe Ci::API::API do ...@@ -329,6 +348,7 @@ describe Ci::API::API do
let(:stored_artifacts_file) { build.reload.artifacts_file.file } let(:stored_artifacts_file) { build.reload.artifacts_file.file }
let(:stored_metadata_file) { build.reload.artifacts_metadata.file } let(:stored_metadata_file) { build.reload.artifacts_metadata.file }
let(:stored_artifacts_size) { build.reload.artifacts_size }
before do before do
post(post_url, post_data, headers_with_token) post(post_url, post_data, headers_with_token)
...@@ -346,6 +366,7 @@ describe Ci::API::API do ...@@ -346,6 +366,7 @@ describe Ci::API::API do
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(stored_artifacts_file.original_filename).to eq(artifacts.original_filename) expect(stored_artifacts_file.original_filename).to eq(artifacts.original_filename)
expect(stored_metadata_file.original_filename).to eq(metadata.original_filename) expect(stored_metadata_file.original_filename).to eq(metadata.original_filename)
expect(stored_artifacts_size).to eq(71759)
end end
end end
...@@ -455,12 +476,17 @@ describe Ci::API::API do ...@@ -455,12 +476,17 @@ describe Ci::API::API do
describe 'DELETE /builds/:id/artifacts' do describe 'DELETE /builds/:id/artifacts' do
let(:build) { create(:ci_build, :artifacts) } let(:build) { create(:ci_build, :artifacts) }
before { delete delete_url, token: build.token }
before do
delete delete_url, token: build.token
build.reload
end
it 'should remove build artifacts' do it 'should remove build artifacts' do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(build.artifacts_file.exists?).to be_falsy expect(build.artifacts_file.exists?).to be_falsy
expect(build.artifacts_metadata.exists?).to be_falsy expect(build.artifacts_metadata.exists?).to be_falsy
expect(build.artifacts_size).to be_nil
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