From 7a1b2e4f94e3e651d3264aa566a9056fe0f554e9 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 18 May 2016 15:28:46 -0500 Subject: [PATCH 01/28] Added when to artifacts --- CHANGELOG | 1 + lib/ci/gitlab_ci_yaml_processor.rb | 24 ++++++++++++++++++++ spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 23 +++++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 364690286e..1f6c1d40e6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -36,6 +36,7 @@ v 8.9.0 (unreleased) - Links from a wiki page to other wiki pages should be rewritten as expected - Add option to project to only allow merge requests to be merged if the build succeeds (Rui Santos) - Fix issues filter when ordering by milestone + - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3 - Todos will display target state if issuable target is 'Closed' or 'Merged' - Fix bug when sorting issues by milestone due date and filtering by two or more labels - Add support for using Yubikeys (U2F) for two-factor authentication diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 130f5b0892..15d57a46eb 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -8,6 +8,8 @@ module Ci ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, :allow_failure, :type, :stage, :when, :artifacts, :cache, :dependencies, :before_script, :after_script, :variables] + ALLOWED_CACHE_KEYS = [:key, :untracked, :paths] + ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when] attr_reader :before_script, :after_script, :image, :services, :path, :cache @@ -135,6 +137,12 @@ module Ci end def validate_global_cache! + @cache.keys.each do |key| + unless ALLOWED_CACHE_KEYS.include? key + raise ValidationError, "#{name} cache unknown parameter #{key}" + end + end + if @cache[:key] && !validate_string(@cache[:key]) raise ValidationError, "cache:key parameter should be a string" end @@ -233,6 +241,12 @@ module Ci end def validate_job_cache!(name, job) + job[:cache].keys.each do |key| + unless ALLOWED_CACHE_KEYS.include? key + raise ValidationError, "#{name} job: cache unknown parameter #{key}" + end + end + if job[:cache][:key] && !validate_string(job[:cache][:key]) raise ValidationError, "#{name} job: cache:key parameter should be a string" end @@ -247,6 +261,12 @@ module Ci end def validate_job_artifacts!(name, job) + job[:artifacts].keys.each do |key| + unless ALLOWED_ARTIFACTS_KEYS.include? key + raise ValidationError, "#{name} job: artifacts unknown parameter #{key}" + end + end + if job[:artifacts][:name] && !validate_string(job[:artifacts][:name]) raise ValidationError, "#{name} job: artifacts:name parameter should be a string" end @@ -258,6 +278,10 @@ module Ci if job[:artifacts][:paths] && !validate_array_of_strings(job[:artifacts][:paths]) raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings" end + + if job[:artifacts][:when] && !job[:artifacts][:when].in?(%w(on_success on_failure always)) + raise ValidationError, "#{name} job: artifacts:when parameter should be on_success, on_failure or always" + end end def validate_job_dependencies!(name, job) diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 7375539cf1..3d3715f0ef 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -601,6 +601,22 @@ module Ci allow_failure: false }) end + + %w(on_success on_failure always).each do |when_state| + it "returns artifacts for when #{when_state} defined" do + config = YAML.dump({ + rspec: { + script: "rspec", + artifacts: { paths: ["logs/", "binaries/"], when: when_state } + } + }) + + config_processor = GitlabCiYamlProcessor.new(config, path) + builds = config_processor.builds_for_stage_and_ref("test", "master") + expect(builds.size).to eq(1) + expect(builds.first[:options][:artifacts][:when]).to eq(when_state) + end + end end describe "Dependencies" do @@ -967,6 +983,13 @@ EOT end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:name parameter should be a string") end + it "returns errors if job artifacts:when is not an a predefined value" do + config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { when: 1 } } }) + expect do + GitlabCiYamlProcessor.new(config) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:when parameter should be on_success, on_failure or always") + end + it "returns errors if job artifacts:untracked is not an array of strings" do config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { untracked: "string" } } }) expect do From aea4041ce96f18afea70da15af3cbe1be4fa1f94 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 18 May 2016 15:21:51 -0500 Subject: [PATCH 02/28] Allow to expire build artifacts --- Gemfile | 3 +++ Gemfile.lock | 4 ++++ app/controllers/projects/builds_controller.rb | 6 ++++++ app/models/ci/build.rb | 18 ++++++++++++++++-- app/views/projects/builds/_sidebar.html.haml | 9 +++++++++ app/workers/expire_build_artifacts.rb | 12 ++++++++++++ config/gitlab.yml.example | 3 +++ config/initializers/1_settings.rb | 3 +++ config/routes.rb | 1 + ...1_add_artifacts_expire_date_to_ci_builds.rb | 5 +++++ lib/ci/api/builds.rb | 2 ++ lib/ci/gitlab_ci_yaml_processor.rb | 2 +- 12 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 app/workers/expire_build_artifacts.rb create mode 100644 db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb diff --git a/Gemfile b/Gemfile index b2660144f2..f56daa099a 100644 --- a/Gemfile +++ b/Gemfile @@ -210,6 +210,9 @@ gem 'mousetrap-rails', '~> 1.4.6' # Detect and convert string character encoding gem 'charlock_holmes', '~> 0.7.3' +# Parse duration +gem 'chronic_duration', '~> 0.10.6' + gem "sass-rails", '~> 5.0.0' gem "coffee-rails", '~> 4.1.0' gem "uglifier", '~> 2.7.2' diff --git a/Gemfile.lock b/Gemfile.lock index dfc1570049..2b2e2d2bb0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -133,6 +133,8 @@ GEM mime-types (>= 1.16) cause (0.1) charlock_holmes (0.7.3) + chronic_duration (0.10.6) + numerizer (~> 0.1.1) chunky_png (1.3.5) cliver (0.3.2) coderay (1.1.0) @@ -424,6 +426,7 @@ GEM nokogiri (1.6.8) mini_portile2 (~> 2.1.0) pkg-config (~> 1.1.7) + numerizer (0.1.1) oauth (0.4.7) oauth2 (1.0.0) faraday (>= 0.8, < 0.10) @@ -857,6 +860,7 @@ DEPENDENCIES capybara-screenshot (~> 1.0.0) carrierwave (~> 0.10.0) charlock_holmes (~> 0.7.3) + chronic_duration (~> 0.10.6) coffee-rails (~> 4.1.0) connection_pool (~> 2.0) coveralls (~> 0.8.2) diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index 14c8282634..514f1b507f 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -78,6 +78,12 @@ class Projects::BuildsController < Projects::ApplicationController end end + def keep_artifacts + @build.keep_artifacts + redirect_to namespace_project_build_path(project.namespace, project, @build), + notice: "Artifacts will not be removed!" + end + private def build diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 6a64ca451f..74084b650c 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -11,6 +11,8 @@ module Ci scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } + scope :with_artifacts, ->() { where.not(artifacts_file: nil) } + scope :with_artifacts_expired, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } mount_uploader :artifacts_file, ArtifactUploader mount_uploader :artifacts_metadata, ArtifactUploader @@ -328,11 +330,15 @@ module Ci Gitlab::Ci::Build::Artifacts::Metadata.new(artifacts_metadata.path, path, **options).to_entry end + def erase_artifacts! + remove_artifacts_file! + remove_artifacts_metadata! + end + def erase(opts = {}) return false unless erasable? - remove_artifacts_file! - remove_artifacts_metadata! + erase_artifacts! erase_trace! update_erased!(opts[:erased_by]) end @@ -345,6 +351,14 @@ module Ci !self.erased_at.nil? end + def artifacts_expired? + self.artifacts_expire_at < Time.now && !artifacts? + end + + def keep_artifacts + self.update(artifacts_expire_at: nil) + end + private def erase_trace! diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index 5d931389df..d1a0da29ef 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -44,6 +44,15 @@ %p.build-detail-row %span.build-light-text Erased: #{time_ago_with_tooltip(@build.erased_at)} + - elsif @build.artifacts_expired? + %p.build-detail-row.artifacts-expired.alert.alert-warning + The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} + - elsif @build.artifacts_expire_at + %p.build-detail-row.artifacts-expired.alert.alert-info + The artifacts will be removed at #{time_ago_with_tooltip(@build.artifacts_expire_at)} + .pull-right + = link_to keep_artifacts_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-primary' do + Keep %p.build-detail-row %span.build-light-text Runner: - if @build.runner && current_user && current_user.admin diff --git a/app/workers/expire_build_artifacts.rb b/app/workers/expire_build_artifacts.rb new file mode 100644 index 0000000000..3d809d8ab6 --- /dev/null +++ b/app/workers/expire_build_artifacts.rb @@ -0,0 +1,12 @@ +class ExpireBuildArtifacts + include Sidekiq::Worker + + def perform + Rails.logger.info 'Cleaning old build artifacts' + + builds = Ci::Build.with_artifacts_expired + builds.find_each(batch_size: 50).each do |build| + build.erase_artifacts! + end + end +end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 1048ef6e24..7b37e92ed4 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -164,6 +164,9 @@ production: &base # Flag stuck CI builds as failed stuck_ci_builds_worker: cron: "0 0 * * *" + # Remove old artifacts + expire_build_artifacts: + cron: "50 * * * *" # Periodically run 'git fsck' on all repositories. If started more than # once per hour you will have concurrent 'git fsck' jobs. repository_check_worker: diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 436751b9d1..b412d1e098 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -279,6 +279,9 @@ Settings['cron_jobs'] ||= Settingslogic.new({}) Settings.cron_jobs['stuck_ci_builds_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['stuck_ci_builds_worker']['cron'] ||= '0 0 * * *' Settings.cron_jobs['stuck_ci_builds_worker']['job_class'] = 'StuckCiBuildsWorker' +Settings.cron_jobs['expire_build_artifacts'] ||= Settingslogic.new({}) +Settings.cron_jobs['expire_build_artifacts']['cron'] ||= '0 0 * * *' +Settings.cron_jobs['expire_build_artifacts']['job_class'] = 'ExpireBuildArtifacts' Settings.cron_jobs['repository_check_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_check_worker']['cron'] ||= '20 * * * *' Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::BatchWorker' diff --git a/config/routes.rb b/config/routes.rb index 95fbe7dd9d..3d092d98c8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -714,6 +714,7 @@ Rails.application.routes.draw do post :cancel post :retry post :erase + post :keep_artifacts get :trace get :raw end diff --git a/db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb b/db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb new file mode 100644 index 0000000000..915167b038 --- /dev/null +++ b/db/migrate/20160518200441_add_artifacts_expire_date_to_ci_builds.rb @@ -0,0 +1,5 @@ +class AddArtifactsExpireDateToCiBuilds < ActiveRecord::Migration + def change + add_column :ci_builds, :artifacts_expire_at, :timestamp + end +end diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 607359769d..54f5626c7d 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -114,6 +114,7 @@ module Ci # id (required) - The ID of a build # token (required) - The build authorization token # file (required) - Artifacts file + # expire_in (optional) - Specify when artifacts should expire (ex. 7d) # Parameters (accelerated by GitLab Workhorse): # file.path - path to locally stored body (generated by Workhorse) # file.name - real filename as send in Content-Disposition @@ -145,6 +146,7 @@ module Ci build.artifacts_file = artifacts build.artifacts_metadata = metadata + build.artifacts_expire_at = Time.now + ChronicDuration.parse(params['expire_in']) if build.save present(build, with: Entities::BuildDetails) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 15d57a46eb..b1297565eb 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -9,7 +9,7 @@ module Ci :allow_failure, :type, :stage, :when, :artifacts, :cache, :dependencies, :before_script, :after_script, :variables] ALLOWED_CACHE_KEYS = [:key, :untracked, :paths] - ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when] + ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in] attr_reader :before_script, :after_script, :image, :services, :path, :cache From ffe8dbde9b2aec2425e7859aeed5ad1642c53938 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 8 Jun 2016 17:18:54 +0200 Subject: [PATCH 03/28] Move keep to ArtifactsController --- .../projects/artifacts_controller.rb | 5 +++++ app/models/ci/build.rb | 8 ++++---- app/views/projects/builds/_sidebar.html.haml | 19 ++++++++++--------- ...ts.rb => expire_build_artifacts_worker.rb} | 3 ++- config/gitlab.yml.example | 4 ++-- config/initializers/1_settings.rb | 6 +++--- config/routes.rb | 2 +- 7 files changed, 27 insertions(+), 20 deletions(-) rename app/workers/{expire_build_artifacts.rb => expire_build_artifacts_worker.rb} (66%) diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 832d7deb57..028e1f7711 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -34,6 +34,11 @@ class Projects::ArtifactsController < Projects::ApplicationController end end + def keep + build.keep_artifacts! + redirect_to namespace_project_build_path(project.namespace, project, build) + end + private def build diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 74084b650c..5eb9fe5f1f 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -12,7 +12,7 @@ module Ci scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } scope :with_artifacts, ->() { where.not(artifacts_file: nil) } - scope :with_artifacts_expired, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } + scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } mount_uploader :artifacts_file, ArtifactUploader mount_uploader :artifacts_metadata, ArtifactUploader @@ -352,10 +352,10 @@ module Ci end def artifacts_expired? - self.artifacts_expire_at < Time.now && !artifacts? + !artifacts? && artifacts_expire_at && artifacts_expire_at < Time.now end - def keep_artifacts + def keep_artifacts! self.update(artifacts_expire_at: nil) end @@ -366,7 +366,7 @@ module Ci end def update_erased!(user = nil) - self.update(erased_by: user, erased_at: Time.now) + self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil) end def yaml_variables diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index d1a0da29ef..e1fdd7019f 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -44,15 +44,16 @@ %p.build-detail-row %span.build-light-text Erased: #{time_ago_with_tooltip(@build.erased_at)} - - elsif @build.artifacts_expired? - %p.build-detail-row.artifacts-expired.alert.alert-warning - The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} - - elsif @build.artifacts_expire_at - %p.build-detail-row.artifacts-expired.alert.alert-info - The artifacts will be removed at #{time_ago_with_tooltip(@build.artifacts_expire_at)} - .pull-right - = link_to keep_artifacts_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-primary' do - Keep + - else + - if @build.artifacts_expired? + .artifacts-expired.alert.alert-warning + The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} + - elsif @build.artifacts_expire_at + .artifacts-expired.alert.alert-warning + The artifacts will be removed in #{duration_in_words(@build.artifacts_expire_at, Time.now)} + .pull-right + = link_to keep_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-xs btn-primary', method: :post do + Keep %p.build-detail-row %span.build-light-text Runner: - if @build.runner && current_user && current_user.admin diff --git a/app/workers/expire_build_artifacts.rb b/app/workers/expire_build_artifacts_worker.rb similarity index 66% rename from app/workers/expire_build_artifacts.rb rename to app/workers/expire_build_artifacts_worker.rb index 3d809d8ab6..17b3b5f227 100644 --- a/app/workers/expire_build_artifacts.rb +++ b/app/workers/expire_build_artifacts_worker.rb @@ -4,8 +4,9 @@ class ExpireBuildArtifacts def perform Rails.logger.info 'Cleaning old build artifacts' - builds = Ci::Build.with_artifacts_expired + builds = Ci::Build.with_expired_artifacts builds.find_each(batch_size: 50).each do |build| + Rails.logger.debug "Removing artifacts build #{build.id}..." build.erase_artifacts! end end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 7b37e92ed4..75e1a3c109 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -164,8 +164,8 @@ production: &base # Flag stuck CI builds as failed stuck_ci_builds_worker: cron: "0 0 * * *" - # Remove old artifacts - expire_build_artifacts: + # Remove expired build artifacts + expire_build_artifacts_worker: cron: "50 * * * *" # Periodically run 'git fsck' on all repositories. If started more than # once per hour you will have concurrent 'git fsck' jobs. diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index b412d1e098..a7320c3a0a 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -279,9 +279,9 @@ Settings['cron_jobs'] ||= Settingslogic.new({}) Settings.cron_jobs['stuck_ci_builds_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['stuck_ci_builds_worker']['cron'] ||= '0 0 * * *' Settings.cron_jobs['stuck_ci_builds_worker']['job_class'] = 'StuckCiBuildsWorker' -Settings.cron_jobs['expire_build_artifacts'] ||= Settingslogic.new({}) -Settings.cron_jobs['expire_build_artifacts']['cron'] ||= '0 0 * * *' -Settings.cron_jobs['expire_build_artifacts']['job_class'] = 'ExpireBuildArtifacts' +Settings.cron_jobs['expire_build_artifacts_worker'] ||= Settingslogic.new({}) +Settings.cron_jobs['expire_build_artifacts_worker']['cron'] ||= '0 0 * * *' +Settings.cron_jobs['expire_build_artifacts_worker']['job_class'] = 'ExpireBuildArtifactsWorker' Settings.cron_jobs['repository_check_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_check_worker']['cron'] ||= '20 * * * *' Settings.cron_jobs['repository_check_worker']['job_class'] = 'RepositoryCheck::BatchWorker' diff --git a/config/routes.rb b/config/routes.rb index 3d092d98c8..59724b737f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -714,7 +714,6 @@ Rails.application.routes.draw do post :cancel post :retry post :erase - post :keep_artifacts get :trace get :raw end @@ -723,6 +722,7 @@ Rails.application.routes.draw do get :download get :browse, path: 'browse(/*path)', format: false get :file, path: 'file/*path', format: false + post :keep end end From 897bc59761ad410728136308a20a184cbd9340c9 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 13:26:36 +0200 Subject: [PATCH 04/28] Added description of artifacts:when --- doc/ci/yaml/README.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index a3481f58c6..39fad549a0 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -30,6 +30,7 @@ If you want a quick introduction to GitLab CI, follow our - [when](#when) - [artifacts](#artifacts) - [artifacts:name](#artifacts-name) + - [artifacts:when](#artifacts-when) - [dependencies](#dependencies) - [before_script and after_script](#before_script-and-after_script) - [Hidden jobs](#hidden-jobs) @@ -651,6 +652,32 @@ job: untracked: true ``` +#### artifacts:when + +>**Note:** +Introduced in GitLab 8.9 and GitLab Runner v1.3.0. + +`artifacts:when` is used to upload artifacts on build failure or despite the +failure. + +`artifacts:when` can be set to one of the following values: + +1. `on_success` - upload artifacts only when build succeeds. This is the default +1. `on_failure` - upload artifacts only when build fails +1. `always` - upload artifacts despite the build status + +--- + +**Example configurations** + +To upload artifacts only when build fails + +```yaml +job: + artifacts: + when: on_failure +``` + ### dependencies >**Note:** From 4e9e4e22af38750d8948c0b3ccb532866975a023 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 14:25:38 +0200 Subject: [PATCH 05/28] Enable exceptions on ChronicDuration --- config/initializers/chronic_duration.rb | 1 + 1 file changed, 1 insertion(+) create mode 100644 config/initializers/chronic_duration.rb diff --git a/config/initializers/chronic_duration.rb b/config/initializers/chronic_duration.rb new file mode 100644 index 0000000000..b65b06c813 --- /dev/null +++ b/config/initializers/chronic_duration.rb @@ -0,0 +1 @@ +ChronicDuration.raise_exceptions = true From ee7c5539f38c5e66d06610d457efe983196372e2 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 14:25:54 +0200 Subject: [PATCH 06/28] Add artifacts_expire_in method for Ci::Build --- app/models/ci/build.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 5eb9fe5f1f..9f66ae63a5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -355,6 +355,18 @@ module Ci !artifacts? && artifacts_expire_at && artifacts_expire_at < Time.now end + def artifacts_expire_in + artifacts_expire_at - Time.now if artifacts_expire_at + end + + def artifacts_expire_in=(value) + if value + self.artifacts_expire_at = Time.now + ChronicDuration.parse(value) + else + self.artifacts_expire_at = nil + end + end + def keep_artifacts! self.update(artifacts_expire_at: nil) end From 1501940ee0452f01acc5a228df17928e2f91cf39 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 14:26:12 +0200 Subject: [PATCH 07/28] Validate artifacts:expire_in in yaml processor --- lib/ci/gitlab_ci_yaml_processor.rb | 10 ++++++++ spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 24 ++++++++++++++++++-- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index b1297565eb..88fa079f30 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -282,6 +282,10 @@ module Ci if job[:artifacts][:when] && !job[:artifacts][:when].in?(%w(on_success on_failure always)) raise ValidationError, "#{name} job: artifacts:when parameter should be on_success, on_failure or always" end + + if job[:artifacts][:expire_in] && !validate_duration(job[:artifacts][:expire_in]) + raise ValidationError, "#{name} job: artifacts:expire_in parameter should be a duration" + end end def validate_job_dependencies!(name, job) @@ -300,6 +304,12 @@ module Ci end end + def validate_duration(value) + value.is_a?(String) && ChronicDuration.parse(value) + rescue ChronicDuration::DurationParseError + false + end + def validate_array_of_strings(values) values.is_a?(Array) && values.all? { |value| validate_string(value) } end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 3d3715f0ef..00a04683e5 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -572,7 +572,12 @@ module Ci services: ["mysql"], before_script: ["pwd"], rspec: { - artifacts: { paths: ["logs/", "binaries/"], untracked: true, name: "custom_name" }, + artifacts: { + paths: ["logs/", "binaries/"], + untracked: true, + name: "custom_name", + expire_in: "7d" + }, script: "rspec" } }) @@ -594,7 +599,8 @@ module Ci artifacts: { name: "custom_name", paths: ["logs/", "binaries/"], - untracked: true + untracked: true, + expire_in: "7d" } }, when: "on_success", @@ -990,6 +996,20 @@ EOT end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:when parameter should be on_success, on_failure or always") end + it "returns errors if job artifacts:expire_in is not an a string" do + config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { expire_in: 1 } } }) + expect do + GitlabCiYamlProcessor.new(config) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:expire_in parameter should be a duration") + end + + it "returns errors if job artifacts:expire_in is not an a valid duration" do + config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { expire_in: "7 elephants" } } }) + expect do + GitlabCiYamlProcessor.new(config) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:expire_in parameter should be a duration") + end + it "returns errors if job artifacts:untracked is not an array of strings" do config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { untracked: "string" } } }) expect do From 86800bf51aec25eef970eac82838bcba087703f8 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 14:26:31 +0200 Subject: [PATCH 08/28] Support expiration date in CI API when uploading artifacts --- lib/ci/api/builds.rb | 2 +- lib/ci/api/entities.rb | 1 + spec/requests/ci/api/builds_spec.rb | 36 +++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 54f5626c7d..9f270f7b38 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -146,7 +146,7 @@ module Ci build.artifacts_file = artifacts build.artifacts_metadata = metadata - build.artifacts_expire_at = Time.now + ChronicDuration.parse(params['expire_in']) + build.artifacts_expire_in = params['expire_in'] if build.save present(build, with: Entities::BuildDetails) diff --git a/lib/ci/api/entities.rb b/lib/ci/api/entities.rb index a902ced35d..352d92e7cc 100644 --- a/lib/ci/api/entities.rb +++ b/lib/ci/api/entities.rb @@ -29,6 +29,7 @@ module Ci expose :before_sha expose :allow_git_fetch expose :token + expose :artifacts_expire_at, if: lambda { |build, opts| build.artifacts? } expose :options do |model| model.options diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index e8508f8f95..dd2ade368f 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -364,6 +364,42 @@ describe Ci::API::API do end end + context 'expire date' do + let!(:artifacts) { file_upload } + + let(:post_data) do + { 'file.path' => artifacts.path, + 'file.name' => artifacts.original_filename, + 'expire_in' => expire_in } + end + + before do + post(post_url, post_data, headers_with_token) + end + + context 'updates when specified' do + let(:expire_in) { '7 days' } + + it do + build.reload + expect(response.status).to eq(201) + expect(json_response['artifacts_expire_at']).not_to be_empty + expect(build.artifacts_expire_at).to be_within(5.minutes).of(Time.now + 7.days) + end + end + + context 'ignores if not specified' do + let(:expire_in) { nil } + + it do + build.reload + expect(response.status).to eq(201) + expect(json_response['artifacts_expire_at']).to be_nil + expect(build.artifacts_expire_at).to be_nil + end + end + end + context "artifacts file is too large" do it "should fail to post too large artifact" do stub_application_setting(max_artifacts_size: 0) From c59947112f352a12e74563453a1bec3082baab41 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 14:39:36 +0200 Subject: [PATCH 09/28] Validate existence of artifacts in ArtifactsController, render 404 if not found --- app/controllers/projects/artifacts_controller.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 028e1f7711..0ab95cd951 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -1,22 +1,17 @@ class Projects::ArtifactsController < Projects::ApplicationController layout 'project' before_action :authorize_read_build! + before_action :validate_artifacts! def download unless artifacts_file.file_storage? return redirect_to artifacts_file.url end - unless artifacts_file.exists? - return render_404 - end - send_file artifacts_file.path, disposition: 'attachment' end def browse - return render_404 unless build.artifacts? - directory = params[:path] ? "#{params[:path]}/" : '' @entry = build.artifacts_metadata_entry(directory) @@ -41,6 +36,10 @@ class Projects::ArtifactsController < Projects::ApplicationController private + def validate_artifacts! + render_404 unless build.artifacts? + end + def build @build ||= project.builds.find_by!(id: params[:build_id]) end From 950d78f6d9ae43bf5c807d95326cf18afcfceedb Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 14:39:58 +0200 Subject: [PATCH 10/28] Remove keep_artifacts from BuildsController --- app/controllers/projects/builds_controller.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb index 514f1b507f..14c8282634 100644 --- a/app/controllers/projects/builds_controller.rb +++ b/app/controllers/projects/builds_controller.rb @@ -78,12 +78,6 @@ class Projects::BuildsController < Projects::ApplicationController end end - def keep_artifacts - @build.keep_artifacts - redirect_to namespace_project_build_path(project.namespace, project, @build), - notice: "Artifacts will not be removed!" - end - private def build From fab1c4a81b7eef247abe6bdd3775cf0ce42badc1 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 14:40:25 +0200 Subject: [PATCH 11/28] Show the artifacts expiration prompt in Build Artifacts widget --- app/models/ci/build.rb | 4 +-- app/views/projects/builds/_sidebar.html.haml | 35 ++++++++++---------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 9f66ae63a5..80702b274d 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -319,7 +319,7 @@ module Ci end def artifacts? - artifacts_file.exists? + !artifacts_expired? && artifacts_file.exists? end def artifacts_metadata? @@ -352,7 +352,7 @@ module Ci end def artifacts_expired? - !artifacts? && artifacts_expire_at && artifacts_expire_at < Time.now + artifacts_expire_at && artifacts_expire_at < Time.now end def artifacts_expire_in diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index e1fdd7019f..0741426b5a 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -11,17 +11,28 @@ %p.build-detail-row #{@build.coverage}% - - if can?(current_user, :read_build, @project) && @build.artifacts? + - if can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?) .block{ class: ("block-first" if !@build.coverage) } .title Build artifacts - .btn-group.btn-group-justified{ role: :group } - = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do - Download + - if @build.artifacts_expired? + .artifacts-expired.alert.alert-warning + The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} + - elsif @build.artifacts_expire_at + .artifacts-expired.alert.alert-warning + The artifacts will be removed in #{time_interval_in_words(@build.artifacts_expire_in)} + - if @build.artifacts? + .btn-group.btn-group-justified{ role: :group } + - if @build.artifacts_expire_at + = link_to keep_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do + Keep - - if @build.artifacts_metadata? - = link_to browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do - Browse + = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do + Download + + - if @build.artifacts_metadata? + = link_to browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do + Browse .block{ class: ("block-first" if !@build.coverage && !(can?(current_user, :read_build, @project) && @build.artifacts?)) } .title @@ -44,16 +55,6 @@ %p.build-detail-row %span.build-light-text Erased: #{time_ago_with_tooltip(@build.erased_at)} - - else - - if @build.artifacts_expired? - .artifacts-expired.alert.alert-warning - The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} - - elsif @build.artifacts_expire_at - .artifacts-expired.alert.alert-warning - The artifacts will be removed in #{duration_in_words(@build.artifacts_expire_at, Time.now)} - .pull-right - = link_to keep_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-xs btn-primary', method: :post do - Keep %p.build-detail-row %span.build-light-text Runner: - if @build.runner && current_user && current_user.admin From 304979f89777f4aca52b382fdbe3a593dc7e50f3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 15:11:53 +0200 Subject: [PATCH 12/28] Allow to show the time in the future --- app/assets/javascripts/application.js.coffee | 2 ++ app/helpers/time_helper.rb | 1 - app/views/projects/builds/_sidebar.html.haml | 11 +++++++---- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index 69d4c4f5dd..33e593f437 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/app/assets/javascripts/application.js.coffee @@ -254,6 +254,8 @@ $ -> .on "resize.app", (e) -> fitSidebarForSize() + jQuery.timeago.settings.allowFuture = true; + gl.awardsHandler = new AwardsHandler() checkInitialSidebarSize() new Aside() diff --git a/app/helpers/time_helper.rb b/app/helpers/time_helper.rb index 8142f733e7..b04b0a5114 100644 --- a/app/helpers/time_helper.rb +++ b/app/helpers/time_helper.rb @@ -20,7 +20,6 @@ module TimeHelper end end - def date_from_to(from, to) "#{from.to_s(:short)} - #{to.to_s(:short)}" end diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index 0741426b5a..1457114531 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -16,11 +16,14 @@ .title Build artifacts - if @build.artifacts_expired? - .artifacts-expired.alert.alert-warning - The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} + %p + The artifacts were removed + #{time_ago_with_tooltip(@build.artifacts_expire_at)} - elsif @build.artifacts_expire_at - .artifacts-expired.alert.alert-warning - The artifacts will be removed in #{time_interval_in_words(@build.artifacts_expire_in)} + %p + The artifacts will be removed in + #{time_ago_with_tooltip(@build.artifacts_expire_at)} + - if @build.artifacts? .btn-group.btn-group-justified{ role: :group } - if @build.artifacts_expire_at From 7e9273dd946f46b2b2bcc0a751316dc704089a16 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 16:20:11 +0200 Subject: [PATCH 13/28] Test controllers if they allow to keep artifacts --- .../projects/artifacts_controller.rb | 1 + spec/features/builds_spec.rb | 44 +++++++++++++ spec/models/build_spec.rb | 65 ++++++++++++++++++- 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 0ab95cd951..f11c832146 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -1,6 +1,7 @@ class Projects::ArtifactsController < Projects::ApplicationController layout 'project' before_action :authorize_read_build! + before_action :authorize_update_build!, only: [:keep] before_action :validate_artifacts! def download diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index b8ecc356b4..a5c3f7cc0b 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -97,6 +97,48 @@ describe "Builds" do end end + context 'Artifacts expire date' do + before do + @build.update_attributes(artifacts_file: artifacts_file, artifacts_expire_at: expire_at) + visit namespace_project_build_path(@project.namespace, @project, @build) + end + + context 'no expire date defined' do + let(:expire_at) { nil } + + it 'should not have the Keep button' do + page.within('.artifacts') do + expect(page).not_to have_content 'Keep' + end + end + end + + context 'when expire date is defined' do + let(:expire_at) { Time.now + 7.days } + + it 'should keep artifacts when Keep button is clicked' do + page.within('.artifacts') do + expect(page).to have_content 'The artifacts will be removed' + click_link 'Keep' + end + + expect(page).not_to have_link 'Keep' + expect(page).not_to have_content 'The artifacts will be removed' + end + end + + context 'when artifacts expired' do + let(:expire_at) { Time.now - 7.days } + + it 'should not have the Keep button' do + page.within('.artifacts') do + expect(page).to have_content 'The artifacts were removed' + expect(page).not_to have_link 'Keep' + end + end + end + end + context 'Build raw trace' do before do @build.run! @@ -108,6 +150,8 @@ describe "Builds" do expect(page).to have_link 'Raw' end end + + context '' end describe "POST /:project/builds/:id/cancel" do diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 2beb6cc598..a2e4639dbf 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -397,9 +397,32 @@ describe Ci::Build, models: true do context 'artifacts archive exists' do let(:build) { create(:ci_build, :artifacts) } it { is_expected.to be_truthy } + + context 'is expired' do + before { build.update(artifacts_expire_at: Time.now - 7.days) } + it { is_expected.to be_falsy } + end + + context 'is not expired' do + before { build.update(artifacts_expire_at: Time.now + 7.days) } + it { is_expected.to be_truthy } + end end end + describe '#artifacts_expired?' do + subject { build.artifacts_expired? } + + context 'is expired' do + before { build.update(artifacts_expire_at: Time.now - 7.days) } + it { is_expected.to be_falsy } + end + + context 'is not expired' do + before { build.update(artifacts_expire_at: Time.now + 7.days) } + it { is_expected.to be_truthy } + end + end describe '#artifacts_metadata?' do subject { build.artifacts_metadata? } @@ -412,7 +435,6 @@ describe Ci::Build, models: true do it { is_expected.to be_truthy } end end - describe '#repo_url' do let(:build) { create(:ci_build) } let(:project) { build.project } @@ -427,6 +449,47 @@ describe Ci::Build, models: true do it { is_expected.to include(project.web_url[7..-1]) } end + describe '#artifacts_expire_in' do + subject { build.artifacts_expire_in } + it { is_expected.to be_nil } + + context 'when artifacts_expire_at is specified' do + let(:expire_at) { Time.now + 7.days } + + before { build.artifacts_expire_at = expire_at } + + it { is_expected.to be_within(5).of(expire_at - Time.now) } + end + end + + describe '#artifacts_expire_in=' do + subject { build.artifacts_expire_in } + + it 'when assigning valid duration' do + build.artifacts_expire_in = '7 days' + is_expected.to be_within(10).of(7.days.to_i) + end + + it 'when assigning invalid duration' do + expect{ build.artifacts_expire_in = '7 elephants' }.not_to raise_error + is_expected.to be_nil + end + + it 'when resseting value' do + build.artifacts_expire_in = nil + is_expected.to be_nil + end + end + + describe '#keep_artifacts!' do + let(:build) { create(:ci_build, artifacts_expire_at: Time.now + 7.days) } + + it 'to reset expire_at' do + build.keep_artifacts! + expect(build.artifacts_expire_at).to be_nil + end + end + describe '#depends_on_builds' do let!(:build) { create(:ci_build, pipeline: pipeline, name: 'build', stage_idx: 0, stage: 'build') } let!(:rspec_test) { create(:ci_build, pipeline: pipeline, name: 'rspec', stage_idx: 1, stage: 'test') } From 6013768fec33e3bf084019d97dbfb7cca78f8e82 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 17:11:27 +0200 Subject: [PATCH 14/28] Added keep artifacts API endpoint --- doc/api/builds.md | 50 ++++++++++++++++++++++++++++++++ lib/api/builds.rb | 19 ++++++++++++ spec/requests/api/builds_spec.rb | 27 +++++++++++++++++ 3 files changed, 96 insertions(+) diff --git a/doc/api/builds.md b/doc/api/builds.md index 5669bd0cdd..0f9f4e99ea 100644 --- a/doc/api/builds.md +++ b/doc/api/builds.md @@ -443,3 +443,53 @@ Example of response "user": null } ``` + +## Keep artifacts + +Prevents artifacts from being deleted when expiration is set + +``` +POST /projects/:id/builds/:build_id/artifacts/keep +``` + +Parameters + +| Attribute | Type | required | Description | +|-------------|---------|----------|---------------------| +| `id` | integer | yes | The ID of a project | +| `build_id` | integer | yes | The ID of a build | + +Example of request + +``` +curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/artifacts/keep" +``` + +Example of response + +```json +{ + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "download_url": null, + "id": 69, + "name": "rubocop", + "ref": "master", + "runner": null, + "stage": "test", + "created_at": "2016-01-11T10:13:33.506Z", + "started_at": "2016-01-11T10:13:33.506Z", + "finished_at": "2016-01-11T10:15:10.506Z", + "status": "failed", + "tag": false, + "user": null +} +``` diff --git a/lib/api/builds.rb b/lib/api/builds.rb index 0ff8fa74a8..704654e9e8 100644 --- a/lib/api/builds.rb +++ b/lib/api/builds.rb @@ -166,6 +166,25 @@ module API present build, with: Entities::Build, user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) end + + # Keep the artifacts to prevent them to be deleted + # + # Parameters: + # id (required) - The ID of a build + # Example Request: + # POST /projects/:id/builds/:build_id/artifacts/keep + post ':id/builds/:build_id/artifacts/keep' do + authorize_update_builds! + + build = get_build(params[:build_id]) + return not_found!(build) unless build && build.artifacts? + + build.keep_artifacts! + + status 200 + present build, with: Entities::Build, + user_can_download_artifacts: can?(current_user, :read_build, user_project) + end end helpers do diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index 6cb7be188e..b92d991b99 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -241,4 +241,31 @@ describe API::API, api: true do end end end + + describe 'POST /projects/:id/builds/:build_id/artifacts/keep' do + before do + post api("/projects/#{project.id}/builds/#{build.id}/artifacts/keep", user) + end + + context 'artifacts did not expire' do + let(:build) do + create(:ci_build, :trace, :artifacts, :success, + project: project, pipeline: pipeline, artifacts_expire_at: Time.now + 7.days) + end + + it 'should keep artifacts' do + expect(response.status).to eq 200 + build.reload + expect(build.artifacts_expire_at).to be_nil + end + end + + context 'no artifacts' do + let(:build) { create(:ci_build, project: project, pipeline: pipeline) } + + it 'should respond with not found' do + expect(response.status).to eq 404 + end + end + end end From 1c60ff0b7ae190a5c6c1cc8c72358af6ef66c05e Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 17:27:49 +0200 Subject: [PATCH 15/28] Test ExpireBuildArtifactsWorker --- app/workers/expire_build_artifacts_worker.rb | 2 +- .../expire_build_artifacts_worker_spec.rb | 55 +++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 spec/workers/expire_build_artifacts_worker_spec.rb diff --git a/app/workers/expire_build_artifacts_worker.rb b/app/workers/expire_build_artifacts_worker.rb index 17b3b5f227..c64ea108d5 100644 --- a/app/workers/expire_build_artifacts_worker.rb +++ b/app/workers/expire_build_artifacts_worker.rb @@ -1,4 +1,4 @@ -class ExpireBuildArtifacts +class ExpireBuildArtifactsWorker include Sidekiq::Worker def perform diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb new file mode 100644 index 0000000000..c9ccddc2a0 --- /dev/null +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe ExpireBuildArtifactsWorker do + include RepoHelpers + + let(:worker) { ExpireBuildArtifactsWorker.new } + + describe '#perform' do + context 'with expired artifacts' do + let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } + + it do + expect_any_instance_of(Ci::Build).to receive(:erase_artifacts!) + worker.perform + build.reload + expect(build.artifacts_expired?).to be_truthy + end + end + + context 'with not yet expired artifacts' do + let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days) } + + it do + expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) + worker.perform + build.reload + expect(build.artifacts_expired?).to be_falsey + end + end + + context 'without expire date' do + let!(:build) { create(:ci_build, :artifacts) } + + it do + expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) + worker.perform + end + end + + context 'for expired artifacts' do + let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } + + before do + build.erase_artifacts! + end + + it do + expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) + worker.perform + build.reload + expect(build.artifacts_expired?).to be_truthy + end + end + end +end From e0673f82c9ee222cf807438520a5abbd75a70456 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 17:31:22 +0200 Subject: [PATCH 16/28] Save database after erasing artifacts --- spec/workers/expire_build_artifacts_worker_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb index c9ccddc2a0..64a55e8c58 100644 --- a/spec/workers/expire_build_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -42,6 +42,7 @@ describe ExpireBuildArtifactsWorker do before do build.erase_artifacts! + build.save end it do From 93080b65cd42ca90ccab8c9ecb2b68c79aafa193 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 10 Jun 2016 17:19:17 +0100 Subject: [PATCH 17/28] Displays time remaining relative to now --- app/assets/javascripts/application.js.coffee | 2 -- app/assets/javascripts/ci/build.coffee | 9 +++++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/application.js.coffee b/app/assets/javascripts/application.js.coffee index 33e593f437..69d4c4f5dd 100644 --- a/app/assets/javascripts/application.js.coffee +++ b/app/assets/javascripts/application.js.coffee @@ -254,8 +254,6 @@ $ -> .on "resize.app", (e) -> fitSidebarForSize() - jQuery.timeago.settings.allowFuture = true; - gl.awardsHandler = new AwardsHandler() checkInitialSidebarSize() new Aside() diff --git a/app/assets/javascripts/ci/build.coffee b/app/assets/javascripts/ci/build.coffee index f763ba96e3..2d515d7efa 100644 --- a/app/assets/javascripts/ci/build.coffee +++ b/app/assets/javascripts/ci/build.coffee @@ -17,6 +17,8 @@ class @CiBuild .off 'resize.build' .on 'resize.build', @hideSidebar + @updateArtifactRemoveDate() + if $('#build-trace').length @getInitialBuildTrace() @initScrollButtonAffix() @@ -103,3 +105,10 @@ class @CiBuild $('.js-build-sidebar') .removeClass 'right-sidebar-collapsed' .addClass 'right-sidebar-expanded' + + updateArtifactRemoveDate: -> + $date = $('.js-artifacts-remove') + + if $date.length + date = $date.text() + $date.text $.timefor(new Date(date), ' ') From d23b91b0d9b8db16801872c49a1fb1d3be3a7144 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 21:25:48 +0200 Subject: [PATCH 18/28] Improve after review --- doc/ci/yaml/README.md | 2 +- lib/ci/gitlab_ci_yaml_processor.rb | 4 ++-- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 39fad549a0..0707555e39 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -670,7 +670,7 @@ failure. **Example configurations** -To upload artifacts only when build fails +To upload artifacts only when build fails. ```yaml job: diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 88fa079f30..76d84433cb 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -208,7 +208,7 @@ module Ci raise ValidationError, "#{name} job: allow_failure parameter should be an boolean" end - if job[:when] && !job[:when].in?(%w(on_success on_failure always)) + if job[:when] && !job[:when].in?(%w[on_success on_failure always]) raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always" end end @@ -279,7 +279,7 @@ module Ci raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings" end - if job[:artifacts][:when] && !job[:artifacts][:when].in?(%w(on_success on_failure always)) + if job[:artifacts][:when] && !job[:artifacts][:when].in?(%w[on_success on_failure always]) raise ValidationError, "#{name} job: artifacts:when parameter should be on_success, on_failure or always" end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 00a04683e5..ad693dd05f 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -608,7 +608,7 @@ module Ci }) end - %w(on_success on_failure always).each do |when_state| + %w[on_success on_failure always].each do |when_state| it "returns artifacts for when #{when_state} defined" do config = YAML.dump({ rspec: { @@ -618,6 +618,7 @@ module Ci }) config_processor = GitlabCiYamlProcessor.new(config, path) + builds = config_processor.builds_for_stage_and_ref("test", "master") expect(builds.size).to eq(1) expect(builds.first[:options][:artifacts][:when]).to eq(when_state) From 9281709b41ce5be5637194cda191a6dd76ddd495 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 10 Jun 2016 20:40:25 +0100 Subject: [PATCH 19/28] Added missing span element around time --- app/views/projects/builds/_sidebar.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index 1457114531..af69490019 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -16,13 +16,13 @@ .title Build artifacts - if @build.artifacts_expired? - %p + %p.build-detail-row The artifacts were removed #{time_ago_with_tooltip(@build.artifacts_expire_at)} - elsif @build.artifacts_expire_at - %p + %p.build-detail-row The artifacts will be removed in - #{time_ago_with_tooltip(@build.artifacts_expire_at)} + %span.js-artifacts-remove= @build.artifacts_expire_at - if @build.artifacts? .btn-group.btn-group-justified{ role: :group } From 421be01dabb13cd1f45d0118b4e1be9d33baef61 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 21:45:06 +0200 Subject: [PATCH 20/28] Improve design based on review --- app/models/ci/build.rb | 9 +- doc/api/builds.md | 528 +++++++++--------- lib/api/builds.rb | 5 +- lib/ci/api/entities.rb | 4 +- spec/features/builds_spec.rb | 8 +- spec/models/build_spec.rb | 3 + spec/requests/api/builds_spec.rb | 7 +- spec/requests/ci/api/builds_spec.rb | 6 +- .../expire_build_artifacts_worker_spec.rb | 18 +- 9 files changed, 296 insertions(+), 292 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 80702b274d..89a1f8b3f5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -360,11 +360,10 @@ module Ci end def artifacts_expire_in=(value) - if value - self.artifacts_expire_at = Time.now + ChronicDuration.parse(value) - else - self.artifacts_expire_at = nil - end + self.artifacts_expire_at = + if value + Time.now + ChronicDuration.parse(value) + end end def keep_artifacts! diff --git a/doc/api/builds.md b/doc/api/builds.md index 0f9f4e99ea..de99894435 100644 --- a/doc/api/builds.md +++ b/doc/api/builds.md @@ -21,85 +21,85 @@ Example of response ```json [ - { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "created_at": "2015-12-24T15:51:21.802Z", - "artifacts_file": { - "filename": "artifacts.zip", - "size": 1000 - }, - "finished_at": "2015-12-24T17:54:27.895Z", - "id": 7, - "name": "teaspoon", - "ref": "master", - "runner": null, - "stage": "test", - "started_at": "2015-12-24T17:54:27.722Z", - "status": "failed", - "tag": false, - "user": { - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "bio": null, - "created_at": "2015-12-21T13:14:24.077Z", - "id": 1, - "is_admin": true, - "linkedin": "", - "name": "Administrator", - "skype": "", - "state": "active", - "twitter": "", - "username": "root", - "web_url": "http://gitlab.dev/u/root", - "website_url": "" - } + { + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." }, - { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "created_at": "2015-12-24T15:51:21.727Z", - "artifacts_file": null, - "finished_at": "2015-12-24T17:54:24.921Z", - "id": 6, - "name": "spinach:other", - "ref": "master", - "runner": null, - "stage": "test", - "started_at": "2015-12-24T17:54:24.729Z", - "status": "failed", - "tag": false, - "user": { - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "bio": null, - "created_at": "2015-12-21T13:14:24.077Z", - "id": 1, - "is_admin": true, - "linkedin": "", - "name": "Administrator", - "skype": "", - "state": "active", - "twitter": "", - "username": "root", - "web_url": "http://gitlab.dev/u/root", - "website_url": "" - } + "coverage": null, + "created_at": "2015-12-24T15:51:21.802Z", + "artifacts_file": { + "filename": "artifacts.zip", + "size": 1000 + }, + "finished_at": "2015-12-24T17:54:27.895Z", + "id": 7, + "name": "teaspoon", + "ref": "master", + "runner": null, + "stage": "test", + "started_at": "2015-12-24T17:54:27.722Z", + "status": "failed", + "tag": false, + "user": { + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "bio": null, + "created_at": "2015-12-21T13:14:24.077Z", + "id": 1, + "is_admin": true, + "linkedin": "", + "name": "Administrator", + "skype": "", + "state": "active", + "twitter": "", + "username": "root", + "web_url": "http://gitlab.dev/u/root", + "website_url": "" } + }, + { + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "created_at": "2015-12-24T15:51:21.727Z", + "artifacts_file": null, + "finished_at": "2015-12-24T17:54:24.921Z", + "id": 6, + "name": "spinach:other", + "ref": "master", + "runner": null, + "stage": "test", + "started_at": "2015-12-24T17:54:24.729Z", + "status": "failed", + "tag": false, + "user": { + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "bio": null, + "created_at": "2015-12-21T13:14:24.077Z", + "id": 1, + "is_admin": true, + "linkedin": "", + "name": "Administrator", + "skype": "", + "state": "active", + "twitter": "", + "username": "root", + "web_url": "http://gitlab.dev/u/root", + "website_url": "" + } + } ] ``` @@ -125,68 +125,68 @@ Example of response ```json [ - { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "created_at": "2016-01-11T10:13:33.506Z", - "artifacts_file": null, - "finished_at": "2016-01-11T10:14:09.526Z", - "id": 69, - "name": "rubocop", - "ref": "master", - "runner": null, - "stage": "test", - "started_at": null, - "status": "canceled", - "tag": false, - "user": null + { + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." }, - { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "created_at": "2015-12-24T15:51:21.957Z", - "artifacts_file": null, - "finished_at": "2015-12-24T17:54:33.913Z", - "id": 9, - "name": "brakeman", - "ref": "master", - "runner": null, - "stage": "test", - "started_at": "2015-12-24T17:54:33.727Z", - "status": "failed", - "tag": false, - "user": { - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "bio": null, - "created_at": "2015-12-21T13:14:24.077Z", - "id": 1, - "is_admin": true, - "linkedin": "", - "name": "Administrator", - "skype": "", - "state": "active", - "twitter": "", - "username": "root", - "web_url": "http://gitlab.dev/u/root", - "website_url": "" - } + "coverage": null, + "created_at": "2016-01-11T10:13:33.506Z", + "artifacts_file": null, + "finished_at": "2016-01-11T10:14:09.526Z", + "id": 69, + "name": "rubocop", + "ref": "master", + "runner": null, + "stage": "test", + "started_at": null, + "status": "canceled", + "tag": false, + "user": null + }, + { + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "created_at": "2015-12-24T15:51:21.957Z", + "artifacts_file": null, + "finished_at": "2015-12-24T17:54:33.913Z", + "id": 9, + "name": "brakeman", + "ref": "master", + "runner": null, + "stage": "test", + "started_at": "2015-12-24T17:54:33.727Z", + "status": "failed", + "tag": false, + "user": { + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "bio": null, + "created_at": "2015-12-21T13:14:24.077Z", + "id": 1, + "is_admin": true, + "linkedin": "", + "name": "Administrator", + "skype": "", + "state": "active", + "twitter": "", + "username": "root", + "web_url": "http://gitlab.dev/u/root", + "website_url": "" } + } ] ``` @@ -211,42 +211,42 @@ Example of response ```json { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "created_at": "2015-12-24T15:51:21.880Z", - "artifacts_file": null, - "finished_at": "2015-12-24T17:54:31.198Z", - "id": 8, - "name": "rubocop", - "ref": "master", - "runner": null, - "stage": "test", - "started_at": "2015-12-24T17:54:30.733Z", - "status": "failed", - "tag": false, - "user": { - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", - "bio": null, - "created_at": "2015-12-21T13:14:24.077Z", - "id": 1, - "is_admin": true, - "linkedin": "", - "name": "Administrator", - "skype": "", - "state": "active", - "twitter": "", - "username": "root", - "web_url": "http://gitlab.dev/u/root", - "website_url": "" - } + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "created_at": "2015-12-24T15:51:21.880Z", + "artifacts_file": null, + "finished_at": "2015-12-24T17:54:31.198Z", + "id": 8, + "name": "rubocop", + "ref": "master", + "runner": null, + "stage": "test", + "started_at": "2015-12-24T17:54:30.733Z", + "status": "failed", + "tag": false, + "user": { + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "bio": null, + "created_at": "2015-12-21T13:14:24.077Z", + "id": 1, + "is_admin": true, + "linkedin": "", + "name": "Administrator", + "skype": "", + "state": "active", + "twitter": "", + "username": "root", + "web_url": "http://gitlab.dev/u/root", + "website_url": "" + } } ``` @@ -323,28 +323,28 @@ Example of response ```json { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "created_at": "2016-01-11T10:13:33.506Z", - "artifacts_file": null, - "finished_at": "2016-01-11T10:14:09.526Z", - "id": 69, - "name": "rubocop", - "ref": "master", - "runner": null, - "stage": "test", - "started_at": null, - "status": "canceled", - "tag": false, - "user": null + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "created_at": "2016-01-11T10:13:33.506Z", + "artifacts_file": null, + "finished_at": "2016-01-11T10:14:09.526Z", + "id": 69, + "name": "rubocop", + "ref": "master", + "runner": null, + "stage": "test", + "started_at": null, + "status": "canceled", + "tag": false, + "user": null } ``` @@ -369,28 +369,28 @@ Example of response ```json { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "created_at": "2016-01-11T10:13:33.506Z", - "artifacts_file": null, - "finished_at": null, - "id": 69, - "name": "rubocop", - "ref": "master", - "runner": null, - "stage": "test", - "started_at": null, - "status": "pending", - "tag": false, - "user": null + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "created_at": "2016-01-11T10:13:33.506Z", + "artifacts_file": null, + "finished_at": null, + "id": 69, + "name": "rubocop", + "ref": "master", + "runner": null, + "stage": "test", + "started_at": null, + "status": "pending", + "tag": false, + "user": null } ``` @@ -419,34 +419,34 @@ Example of response ```json { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "download_url": null, - "id": 69, - "name": "rubocop", - "ref": "master", - "runner": null, - "stage": "test", - "created_at": "2016-01-11T10:13:33.506Z", - "started_at": "2016-01-11T10:13:33.506Z", - "finished_at": "2016-01-11T10:15:10.506Z", - "status": "failed", - "tag": false, - "user": null + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "download_url": null, + "id": 69, + "name": "rubocop", + "ref": "master", + "runner": null, + "stage": "test", + "created_at": "2016-01-11T10:13:33.506Z", + "started_at": "2016-01-11T10:13:33.506Z", + "finished_at": "2016-01-11T10:15:10.506Z", + "status": "failed", + "tag": false, + "user": null } ``` ## Keep artifacts -Prevents artifacts from being deleted when expiration is set +Prevents artifacts from being deleted when expiration is set. ``` POST /projects/:id/builds/:build_id/artifacts/keep @@ -459,37 +459,37 @@ Parameters | `id` | integer | yes | The ID of a project | | `build_id` | integer | yes | The ID of a build | -Example of request +Example request: ``` curl -X POST -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v3/projects/1/builds/1/artifacts/keep" ``` -Example of response +Example response: ```json { - "commit": { - "author_email": "admin@example.com", - "author_name": "Administrator", - "created_at": "2015-12-24T16:51:14.000+01:00", - "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", - "message": "Test the CI integration.", - "short_id": "0ff3ae19", - "title": "Test the CI integration." - }, - "coverage": null, - "download_url": null, - "id": 69, - "name": "rubocop", - "ref": "master", - "runner": null, - "stage": "test", - "created_at": "2016-01-11T10:13:33.506Z", - "started_at": "2016-01-11T10:13:33.506Z", - "finished_at": "2016-01-11T10:15:10.506Z", - "status": "failed", - "tag": false, - "user": null + "commit": { + "author_email": "admin@example.com", + "author_name": "Administrator", + "created_at": "2015-12-24T16:51:14.000+01:00", + "id": "0ff3ae198f8601a285adcf5c0fff204ee6fba5fd", + "message": "Test the CI integration.", + "short_id": "0ff3ae19", + "title": "Test the CI integration." + }, + "coverage": null, + "download_url": null, + "id": 69, + "name": "rubocop", + "ref": "master", + "runner": null, + "stage": "test", + "created_at": "2016-01-11T10:13:33.506Z", + "started_at": "2016-01-11T10:13:33.506Z", + "finished_at": "2016-01-11T10:15:10.506Z", + "status": "failed", + "tag": false, + "user": null } ``` diff --git a/lib/api/builds.rb b/lib/api/builds.rb index 704654e9e8..644e5a2a99 100644 --- a/lib/api/builds.rb +++ b/lib/api/builds.rb @@ -167,10 +167,11 @@ module API user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project) end - # Keep the artifacts to prevent them to be deleted + # Keep the artifacts to prevent them from being deleted # # Parameters: - # id (required) - The ID of a build + # id (required) - the id of a project + # build_id (required) - The ID of a build # Example Request: # POST /projects/:id/builds/:build_id/artifacts/keep post ':id/builds/:build_id/artifacts/keep' do diff --git a/lib/ci/api/entities.rb b/lib/ci/api/entities.rb index 352d92e7cc..3f5bdaba3f 100644 --- a/lib/ci/api/entities.rb +++ b/lib/ci/api/entities.rb @@ -20,7 +20,7 @@ module Ci expose :name, :token, :stage expose :project_id expose :project_name - expose :artifacts_file, using: ArtifactFile, if: lambda { |build, opts| build.artifacts? } + expose :artifacts_file, using: ArtifactFile, if: ->(build, _) { build.artifacts? } end class BuildDetails < Build @@ -29,7 +29,7 @@ module Ci expose :before_sha expose :allow_git_fetch expose :token - expose :artifacts_expire_at, if: lambda { |build, opts| build.artifacts? } + expose :artifacts_expire_at, if: ->(build, _) { build.artifacts? } expose :options do |model| model.options diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index a5c3f7cc0b..0fd9529538 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -106,7 +106,7 @@ describe "Builds" do context 'no expire date defined' do let(:expire_at) { nil } - it 'should not have the Keep button' do + it 'does not have the Keep button' do page.within('.artifacts') do expect(page).not_to have_content 'Keep' end @@ -116,7 +116,7 @@ describe "Builds" do context 'when expire date is defined' do let(:expire_at) { Time.now + 7.days } - it 'should keep artifacts when Keep button is clicked' do + it 'keeps artifacts when Keep button is clicked' do page.within('.artifacts') do expect(page).to have_content 'The artifacts will be removed' click_link 'Keep' @@ -130,7 +130,7 @@ describe "Builds" do context 'when artifacts expired' do let(:expire_at) { Time.now - 7.days } - it 'should not have the Keep button' do + it 'does not have the Keep button' do page.within('.artifacts') do expect(page).to have_content 'The artifacts were removed' expect(page).not_to have_link 'Keep' @@ -150,8 +150,6 @@ describe "Builds" do expect(page).to have_link 'Raw' end end - - context '' end describe "POST /:project/builds/:id/cancel" do diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index a2e4639dbf..f25b676651 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -467,6 +467,7 @@ describe Ci::Build, models: true do it 'when assigning valid duration' do build.artifacts_expire_in = '7 days' + is_expected.to be_within(10).of(7.days.to_i) end @@ -477,6 +478,7 @@ describe Ci::Build, models: true do it 'when resseting value' do build.artifacts_expire_in = nil + is_expected.to be_nil end end @@ -486,6 +488,7 @@ describe Ci::Build, models: true do it 'to reset expire_at' do build.keep_artifacts! + expect(build.artifacts_expire_at).to be_nil end end diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb index b92d991b99..ac85f34092 100644 --- a/spec/requests/api/builds_spec.rb +++ b/spec/requests/api/builds_spec.rb @@ -253,17 +253,16 @@ describe API::API, api: true do project: project, pipeline: pipeline, artifacts_expire_at: Time.now + 7.days) end - it 'should keep artifacts' do + it 'keeps artifacts' do expect(response.status).to eq 200 - build.reload - expect(build.artifacts_expire_at).to be_nil + expect(build.reload.artifacts_expire_at).to be_nil end end context 'no artifacts' do let(:build) { create(:ci_build, project: project, pipeline: pipeline) } - it 'should respond with not found' do + it 'responds with not found' do expect(response.status).to eq 404 end end diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index dd2ade368f..616b41eabe 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -364,7 +364,7 @@ describe Ci::API::API do end end - context 'expire date' do + context 'with an expire date' do let!(:artifacts) { file_upload } let(:post_data) do @@ -377,7 +377,7 @@ describe Ci::API::API do post(post_url, post_data, headers_with_token) end - context 'updates when specified' do + context 'with an expire_in given' do let(:expire_in) { '7 days' } it do @@ -388,7 +388,7 @@ describe Ci::API::API do end end - context 'ignores if not specified' do + context 'with no expire_in given' do let(:expire_in) { nil } it do diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb index 64a55e8c58..501ca630e5 100644 --- a/spec/workers/expire_build_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe ExpireBuildArtifactsWorker do include RepoHelpers - let(:worker) { ExpireBuildArtifactsWorker.new } + let(:worker) { described_class.new } describe '#perform' do context 'with expired artifacts' do @@ -11,9 +11,10 @@ describe ExpireBuildArtifactsWorker do it do expect_any_instance_of(Ci::Build).to receive(:erase_artifacts!) + worker.perform - build.reload - expect(build.artifacts_expired?).to be_truthy + + expect(build.reload.artifacts_expired?).to be_truthy end end @@ -22,9 +23,10 @@ describe ExpireBuildArtifactsWorker do it do expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) + worker.perform - build.reload - expect(build.artifacts_expired?).to be_falsey + + expect(build.reload.artifacts_expired?).to be_falsey end end @@ -33,6 +35,7 @@ describe ExpireBuildArtifactsWorker do it do expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) + worker.perform end end @@ -47,9 +50,10 @@ describe ExpireBuildArtifactsWorker do it do expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) + worker.perform - build.reload - expect(build.artifacts_expired?).to be_truthy + + expect(build.reload.artifacts_expired?).to be_truthy end end end From cf9c5b54c68218281ac066cac5d3c002fb72153a Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 10 Jun 2016 21:53:57 +0200 Subject: [PATCH 21/28] Added documentation to artifacts expire --- config/initializers/1_settings.rb | 2 +- doc/ci/yaml/README.md | 35 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index a7320c3a0a..916fd33e76 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -280,7 +280,7 @@ Settings.cron_jobs['stuck_ci_builds_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['stuck_ci_builds_worker']['cron'] ||= '0 0 * * *' Settings.cron_jobs['stuck_ci_builds_worker']['job_class'] = 'StuckCiBuildsWorker' Settings.cron_jobs['expire_build_artifacts_worker'] ||= Settingslogic.new({}) -Settings.cron_jobs['expire_build_artifacts_worker']['cron'] ||= '0 0 * * *' +Settings.cron_jobs['expire_build_artifacts_worker']['cron'] ||= '50 * * * *' Settings.cron_jobs['expire_build_artifacts_worker']['job_class'] = 'ExpireBuildArtifactsWorker' Settings.cron_jobs['repository_check_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['repository_check_worker']['cron'] ||= '20 * * * *' diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 0707555e39..d71ce6d6b1 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -31,6 +31,7 @@ If you want a quick introduction to GitLab CI, follow our - [artifacts](#artifacts) - [artifacts:name](#artifacts-name) - [artifacts:when](#artifacts-when) + - [artifacts:expire_in](#artifacts-expire_in) - [dependencies](#dependencies) - [before_script and after_script](#before_script-and-after_script) - [Hidden jobs](#hidden-jobs) @@ -678,6 +679,40 @@ job: when: on_failure ``` +#### artifacts:expire_in + +>**Note:** +Introduced in GitLab 8.9 and GitLab Runner v1.3.0. + +`artifacts:expire_in` is used to remove uploaded artifacts after specified time. +By default artifacts are stored on GitLab forver. +`expire_in` allows to specify after what time the artifacts should be removed. +The artifacts will expire counting from the moment when they are uploaded and stored on GitLab. + +After artifacts uploading you can use the **Keep** button on build page to keep the artifacts forever. + +Artifacts are removed every hour, but they are not accessible after expire date. + +The value of `expire_in` is a elapsed time. The example of parsable values: +- '3 mins 4 sec' +- '2 hrs 20 min' +- '2h20min' +- '6 mos 1 day' +- '47 yrs 6 mos and 4d' +- '3 weeks and 2 days' + +--- + +**Example configurations** + +To expire artifacts after 1 week from the moment that they are uploaded: + +```yaml +job: + artifacts: + expire_in: 1 week +``` + ### dependencies >**Note:** From b0b1b85d7197b211c472779c07410de70b39e548 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 13 Jun 2016 11:12:38 +0100 Subject: [PATCH 22/28] Fixed spacing with row below in build sidebar --- app/views/projects/builds/_sidebar.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index af69490019..7127acf388 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -37,7 +37,7 @@ = link_to browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do Browse - .block{ class: ("block-first" if !@build.coverage && !(can?(current_user, :read_build, @project) && @build.artifacts?)) } + .block{ class: ("block-first" if !@build.coverage && !(can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?))) } .title Build details - if @build.retryable? From c534d2e89ed00ff98c83a197674b5ac66a8aca93 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 13 Jun 2016 16:05:23 +0200 Subject: [PATCH 23/28] Improve tests --- spec/models/build_spec.rb | 2 +- spec/requests/ci/api/builds_spec.rb | 4 ++-- .../expire_build_artifacts_worker_spec.rb | 24 +++++++------------ 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index f25b676651..c07832a4b5 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -472,7 +472,7 @@ describe Ci::Build, models: true do end it 'when assigning invalid duration' do - expect{ build.artifacts_expire_in = '7 elephants' }.not_to raise_error + expect { build.artifacts_expire_in = '7 elephants' }.not_to raise_error is_expected.to be_nil end diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index 616b41eabe..7e50bea90d 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -380,7 +380,7 @@ describe Ci::API::API do context 'with an expire_in given' do let(:expire_in) { '7 days' } - it do + it 'updates when specified' do build.reload expect(response.status).to eq(201) expect(json_response['artifacts_expire_at']).not_to be_empty @@ -391,7 +391,7 @@ describe Ci::API::API do context 'with no expire_in given' do let(:expire_in) { nil } - it do + it 'ignores if not specified' do build.reload expect(response.status).to eq(201) expect(json_response['artifacts_expire_at']).to be_nil diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb index 501ca630e5..8168ad9806 100644 --- a/spec/workers/expire_build_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -6,14 +6,14 @@ describe ExpireBuildArtifactsWorker do let(:worker) { described_class.new } describe '#perform' do + before { build } + + subject! { worker.perform } + context 'with expired artifacts' do let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } - it do - expect_any_instance_of(Ci::Build).to receive(:erase_artifacts!) - - worker.perform - + it 'does expire' do expect(build.reload.artifacts_expired?).to be_truthy end end @@ -21,22 +21,16 @@ describe ExpireBuildArtifactsWorker do context 'with not yet expired artifacts' do let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days) } - it do - expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) - - worker.perform - - expect(build.reload.artifacts_expired?).to be_falsey + it 'does not expire' do + expect(build.reload.artifacts_expired?).to be_truthy end end context 'without expire date' do let!(:build) { create(:ci_build, :artifacts) } - it do - expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) - - worker.perform + it 'does not expire' do + expect(build.reload.artifacts_expired?).to be_falsey end end From 33db51f9154f8421dfdc2e07d04684b1c1f404d9 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 13 Jun 2016 18:18:24 +0200 Subject: [PATCH 24/28] Improve ExpireBuildArtifactsWorker spec --- spec/workers/expire_build_artifacts_worker_spec.rb | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb index 8168ad9806..eb8afb2027 100644 --- a/spec/workers/expire_build_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -35,18 +35,13 @@ describe ExpireBuildArtifactsWorker do end context 'for expired artifacts' do - let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } + let!(:build) { create(:ci_build, artifacts_expire_at: Time.now - 7.days) } - before do - build.erase_artifacts! - build.save + it 'does not erase artifacts' do + expect_any_instance_of(Ci::Build).not_to have_received(:erase_artifacts!) end - it do - expect_any_instance_of(Ci::Build).not_to receive(:erase_artifacts!) - - worker.perform - + it 'does expire' do expect(build.reload.artifacts_expired?).to be_truthy end end From 58b4e5531bfaaa385e31aa71dfb2236372733f48 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 13 Jun 2016 19:00:09 +0200 Subject: [PATCH 25/28] Update db/schema.rb --- db/schema.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index aac327797e..91b9cb0a98 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160608155312) do +ActiveRecord::Schema.define(version: 20160610301627) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -144,9 +144,9 @@ ActiveRecord::Schema.define(version: 20160608155312) do t.text "commands" t.integer "job_id" t.string "name" - t.boolean "deploy", default: false + t.boolean "deploy", default: false t.text "options" - t.boolean "allow_failure", default: false, null: false + t.boolean "allow_failure", default: false, null: false t.string "stage" t.integer "trigger_request_id" t.integer "stage_idx" @@ -161,6 +161,7 @@ ActiveRecord::Schema.define(version: 20160608155312) do t.text "artifacts_metadata" t.integer "erased_by_id" t.datetime "erased_at" + t.datetime "artifacts_expire_at" 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 @@ -670,8 +671,8 @@ ActiveRecord::Schema.define(version: 20160608155312) do create_table "notification_settings", force: :cascade do |t| t.integer "user_id", null: false - t.integer "source_id", null: false - t.string "source_type", null: false + t.integer "source_id" + t.string "source_type" t.integer "level", default: 0, null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -988,7 +989,6 @@ ActiveRecord::Schema.define(version: 20160608155312) do t.boolean "can_create_team", default: true, null: false t.string "state" t.integer "color_scheme_id", default: 1, null: false - t.integer "notification_level", default: 1, null: false t.datetime "password_expires_at" t.integer "created_by_id" t.datetime "last_credential_check_at" From 278a0e1a0fb094dc2eceec0d82d9a56be81c8046 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 13 Jun 2016 19:19:11 +0200 Subject: [PATCH 26/28] Fix keep action of build artifacts widget --- app/views/projects/builds/_sidebar.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index 7127acf388..cab21f0cf1 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -27,7 +27,7 @@ - if @build.artifacts? .btn-group.btn-group-justified{ role: :group } - if @build.artifacts_expire_at - = link_to keep_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do + = link_to keep_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default', method: :post do Keep = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), class: 'btn btn-sm btn-default' do From 60e0137c864e26fee0120dc4447bb95acc46ce51 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 14 Jun 2016 11:38:34 +0200 Subject: [PATCH 27/28] Fix specs --- lib/api/builds.rb | 2 +- spec/features/builds_spec.rb | 16 +++----- spec/models/build_spec.rb | 6 ++- .../expire_build_artifacts_worker_spec.rb | 40 +++++++++++-------- 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/lib/api/builds.rb b/lib/api/builds.rb index 644e5a2a99..645e2dda0b 100644 --- a/lib/api/builds.rb +++ b/lib/api/builds.rb @@ -184,7 +184,7 @@ module API status 200 present build, with: Entities::Build, - user_can_download_artifacts: can?(current_user, :read_build, user_project) + user_can_download_artifacts: can?(current_user, :read_build, user_project) end end diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb index 0fd9529538..16832c297a 100644 --- a/spec/features/builds_spec.rb +++ b/spec/features/builds_spec.rb @@ -107,9 +107,7 @@ describe "Builds" do let(:expire_at) { nil } it 'does not have the Keep button' do - page.within('.artifacts') do - expect(page).not_to have_content 'Keep' - end + expect(page).not_to have_content 'Keep' end end @@ -117,10 +115,8 @@ describe "Builds" do let(:expire_at) { Time.now + 7.days } it 'keeps artifacts when Keep button is clicked' do - page.within('.artifacts') do - expect(page).to have_content 'The artifacts will be removed' - click_link 'Keep' - end + expect(page).to have_content 'The artifacts will be removed' + click_link 'Keep' expect(page).not_to have_link 'Keep' expect(page).not_to have_content 'The artifacts will be removed' @@ -131,10 +127,8 @@ describe "Builds" do let(:expire_at) { Time.now - 7.days } it 'does not have the Keep button' do - page.within('.artifacts') do - expect(page).to have_content 'The artifacts were removed' - expect(page).not_to have_link 'Keep' - end + expect(page).to have_content 'The artifacts were removed' + expect(page).not_to have_link 'Keep' end end end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index c07832a4b5..35554e1e0c 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -415,12 +415,14 @@ describe Ci::Build, models: true do context 'is expired' do before { build.update(artifacts_expire_at: Time.now - 7.days) } - it { is_expected.to be_falsy } + + it { is_expected.to be_truthy } end context 'is not expired' do before { build.update(artifacts_expire_at: Time.now + 7.days) } - it { is_expected.to be_truthy } + + it { is_expected.to be_falsey } end end diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb index eb8afb2027..e3827cae9a 100644 --- a/spec/workers/expire_build_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -11,37 +11,45 @@ describe ExpireBuildArtifactsWorker do subject! { worker.perform } context 'with expired artifacts' do - let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } + let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now - 7.days) } it 'does expire' do expect(build.reload.artifacts_expired?).to be_truthy end + + it 'does remove files' do + expect(build.reload.artifacts_file.exists?).to be_falsey + end end context 'with not yet expired artifacts' do - let!(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days) } - - it 'does not expire' do - expect(build.reload.artifacts_expired?).to be_truthy - end - end - - context 'without expire date' do - let!(:build) { create(:ci_build, :artifacts) } + let(:build) { create(:ci_build, :artifacts, artifacts_expire_at: Time.now + 7.days) } it 'does not expire' do expect(build.reload.artifacts_expired?).to be_falsey end + + it 'does not remove files' do + expect(build.reload.artifacts_file.exists?).to be_truthy + end + end + + context 'without expire date' do + let(:build) { create(:ci_build, :artifacts) } + + it 'does not expire' do + expect(build.reload.artifacts_expired?).to be_falsey + end + + it 'does not remove files' do + expect(build.reload.artifacts_file.exists?).to be_truthy + end end context 'for expired artifacts' do - let!(:build) { create(:ci_build, artifacts_expire_at: Time.now - 7.days) } + let(:build) { create(:ci_build, artifacts_expire_at: Time.now - 7.days) } - it 'does not erase artifacts' do - expect_any_instance_of(Ci::Build).not_to have_received(:erase_artifacts!) - end - - it 'does expire' do + it 'is still expired' do expect(build.reload.artifacts_expired?).to be_truthy end end From 2b5449b96d1c08eafc3e874f28dd9f85a6b09535 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 14 Jun 2016 14:51:09 +0200 Subject: [PATCH 28/28] Fix Ci::Build#artifacts_expire_in= when assigning invalid duration --- spec/models/build_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 35554e1e0c..5d1fa8226e 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -474,7 +474,7 @@ describe Ci::Build, models: true do end it 'when assigning invalid duration' do - expect { build.artifacts_expire_in = '7 elephants' }.not_to raise_error + expect { build.artifacts_expire_in = '7 elephants' }.to raise_error(ChronicDuration::DurationParseError) is_expected.to be_nil end