Move keep to ArtifactsController

7 files changed, 27 insertions, 20 deletions

diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index 832d7deb57d..028e1f77119 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 74084b650cf..5eb9fe5f1f5 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 d1a0da29ef7..e1fdd7019ff 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
index 3d809d8ab6b..17b3b5f227f 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 7b37e92ed46..75e1a3c1093 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 b412d1e0981..a7320c3a0a7 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 3d092d98c8e..59724b737f6 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