Migrate artifacts to a new storage

- Store information in database what storage is being used by artifacts,
- Artifacts are now stored as `artifacts/project_id/commit_id/job_id`.

14 files changed, 100 insertions, 57 deletions

diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index 1224e9503c9..7d156fd5e3f 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -10,7 +10,7 @@ class Projects::ArtifactsController < Projects::ApplicationController
   before_action :set_path_and_entry, only: [:file, :raw]
 
   def download
-    if artifacts_file.file_storage?
+    if artifacts_file.local_file?
       send_file artifacts_file.path, disposition: 'attachment'
     else
       redirect_to artifacts_file.url
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index b426c27afbb..9b3324b69bc 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -50,6 +50,11 @@ module Ci
     after_save :update_project_statistics, if: :artifacts_size_changed?
     after_destroy :update_project_statistics
 
+    enum artifacts_storage: { 
+      artifacts_storage_undefined: nil,
+      artifacts_storage_upgraded: 1,
+    }
+
     class << self
       def first_pending
         pending.unstarted.order('created_at ASC').first
@@ -254,38 +259,6 @@ module Ci
       Time.now - updated_at > 15.minutes.to_i
     end
 
-    ##
-    # Deprecated
-    #
-    # This contains a hotfix for CI build data integrity, see #4246
-    #
-    # This method is used by `ArtifactUploader` to create a store_dir.
-    # Warning: Uploader uses it after AND before file has been stored.
-    #
-    # This method returns old path to artifacts only if it already exists.
-    #
-    def artifacts_path
-      # We need the project even if it's soft deleted, because whenever
-      # we're really deleting the project, we'll also delete the builds,
-      # and in order to delete the builds, we need to know where to find
-      # the artifacts, which is depending on the data of the project.
-      # We need to retain the project in this case.
-      the_project = project || unscoped_project
-
-      old = File.join(created_at.utc.strftime('%Y_%m'),
-                      the_project.ci_id.to_s,
-                      id.to_s)
-
-      old_store = File.join(ArtifactUploader.artifacts_path, old)
-      return old if the_project.ci_id && File.directory?(old_store)
-
-      File.join(
-        created_at.utc.strftime('%Y_%m'),
-        the_project.id.to_s,
-        id.to_s
-      )
-    end
-
     def valid_token?(token)
       self.token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.token)
     end
diff --git a/app/uploaders/artifact_uploader.rb b/app/uploaders/artifact_uploader.rb
index 3e36ec91205..90d1252975d 100644
--- a/app/uploaders/artifact_uploader.rb
+++ b/app/uploaders/artifact_uploader.rb
@@ -1,33 +1,65 @@
 class ArtifactUploader < GitlabUploader
   storage :file
 
-  attr_accessor :build, :field
+  attr_reader :job, :field
 
-  def self.artifacts_path
+  def self.local_artifacts_store
     Gitlab.config.artifacts.path
   end
 
-  def self.artifacts_upload_path
-    File.join(self.artifacts_path, 'tmp/uploads/')
+  def initialize(job, field)
+    @job, @field = job, field
   end
 
-  def self.artifacts_cache_path
-    File.join(self.artifacts_path, 'tmp/cache/')
+  def store_dir
+    deprecated_local_path || default_local_path
   end
 
-  def initialize(build, field)
-    @build, @field = build, field
+  def cache_dir
+    File.join(self.local_artifacts_path, 'tmp/cache')
   end
 
-  def store_dir
-    File.join(self.class.artifacts_path, @build.artifacts_path)
+  def default_path
+    File.join(job.project_id.to_s, created_at.utc.strftime('%Y_%m'), id.to_s)
   end
 
-  def cache_dir
-    File.join(self.class.artifacts_cache_path, @build.artifacts_path)
+  def deprecated_local_path
+    return unless job.artifacts_storage_undefined?
+
+    @deprecated_local_path ||= deprecated_paths.find do |artifact_path|
+      File.directory?(File.Join(self.local_artifacts_store, artifact_path))
+    end
+  end
+
+  def default_local_path
+    File.Join(self.local_artifacts_store, default_path)
+  end
+
+  def migrate!
+    return unless deprecated_local_path
+    return unless default_local_path == deprecated_local_path
+
+    FileUtils.move(deprecated_local_path, default_local_path, force: true)
+  end
+
+  ##
+  # Deprecated
+  #
+  # This contains a hotfix for CI build data integrity, see #4246
+  #
+  # This method is used by `ArtifactUploader` to create a store_dir.
+  # Warning: Uploader uses it after AND before file has been stored.
+  #
+  # This method returns old path to artifacts only if it already exists.
+  #
+  def deprecated_paths
+    [
+      File.join(created_at.utc.strftime('%Y_%m'), job.project_id.to_s, id.to_s),
+      the_project&.ci_id && File.join(created_at.utc.strftime('%Y_%m'), the_project.ci_id.to_s, id.to_s),
+    ].compact
   end
 
-  def filename
-    file.try(:filename)
+  def the_project
+    job.project || job.unscoped_project
   end
 end
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index e0a6c9b4067..80e17a84f79 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -9,8 +9,16 @@ class GitlabUploader < CarrierWave::Uploader::Base
 
   delegate :base_dir, to: :class
 
-  def file_storage?
-    self.class.storage == CarrierWave::Storage::File
+  def local_file?
+    local_storage? && file&.is_a?(CarrierWave::Storage::Fog::File)
+  end
+
+  def local_storage?
+    storage.is_a?(CarrierWave::Storage::File)
+  end
+
+  def local_cache_storage?
+    cache_storage.is_a?(CarrierWave::Storage::File)
   end
 
   # Reduce disk IO
diff --git a/app/uploaders/records_uploads.rb b/app/uploaders/records_uploads.rb
index 4c127f29250..be3a2caa60c 100644
--- a/app/uploaders/records_uploads.rb
+++ b/app/uploaders/records_uploads.rb
@@ -16,7 +16,7 @@ module RecordsUploads
   #
   # Called `after :store`
   def record_upload(_tempfile)
-    return unless file_storage?
+    return unless local_file?
     return unless file.exists?
 
     Upload.record(self)
@@ -26,7 +26,7 @@ module RecordsUploads
   #
   # Called `before :remove`
   def destroy_upload(*args)
-    return unless file_storage?
+    return unless local_file?
     return unless file
 
     Upload.remove_path(relative_path)
diff --git a/changelogs/unreleased/migrate-artifacts-to-a-new-path.yml b/changelogs/unreleased/migrate-artifacts-to-a-new-path.yml
new file mode 100644
index 00000000000..bd022a3a91b
--- /dev/null
+++ b/changelogs/unreleased/migrate-artifacts-to-a-new-path.yml
@@ -0,0 +1,4 @@
+---
+title: Migrate artifacts to a new path
+merge_request:
+author:
diff --git a/db/migrate/20170504203205_add_file_storage_to_ci_build.rb b/db/migrate/20170504203205_add_file_storage_to_ci_build.rb
new file mode 100644
index 00000000000..292ad8ed08c
--- /dev/null
+++ b/db/migrate/20170504203205_add_file_storage_to_ci_build.rb
@@ -0,0 +1,9 @@
+class AddArtifactsFileStorageToCiBuild < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_builds, :artifacts_storage, :integer
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 01c0f00c924..c5285c203c2 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: 20170502091007) do
+ActiveRecord::Schema.define(version: 20170504203205) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -230,6 +230,7 @@ ActiveRecord::Schema.define(version: 20170502091007) do
     t.integer "lock_version"
     t.string "coverage_regex"
     t.integer "auto_canceled_by_id"
+    t.integer "artifacts_storage"
   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
diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 288b03d940c..6096d9010e4 100644
--- a/lib/api/jobs.rb
+++ b/lib/api/jobs.rb
@@ -220,7 +220,7 @@ module API
       end
 
       def present_artifacts!(artifacts_file)
-        if !artifacts_file.file_storage?
+        if !artifacts_file.local_file?
           redirect_to(build.artifacts_file.url)
         elsif artifacts_file.exists?
           present_file!(artifacts_file.path, artifacts_file.filename)
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index 6fbb02cb3aa..73ea2807f0f 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -217,6 +217,7 @@ module API
         bad_request!('Missing artifacts file!') unless artifacts
         file_to_large! unless artifacts.size < max_artifacts_size
 
+        job.artifacts_storage_upgraded!
         job.artifacts_file = artifacts
         job.artifacts_metadata = metadata
         job.artifacts_expire_in = params['expire_in'] ||
@@ -242,7 +243,7 @@ module API
         job = authenticate_job!
 
         artifacts_file = job.artifacts_file
-        unless artifacts_file.file_storage?
+        unless artifacts_file.local_file?
           return redirect_to job.artifacts_file.url
         end
 
diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb
index 4dd03cdf24b..cbfa77718a5 100644
--- a/lib/api/v3/builds.rb
+++ b/lib/api/v3/builds.rb
@@ -222,7 +222,7 @@ module API
         end
 
         def present_artifacts!(artifacts_file)
-          if !artifacts_file.file_storage?
+          if !artifacts_file.local_file?
             redirect_to(build.artifacts_file.url)
           elsif artifacts_file.exists?
             present_file!(artifacts_file.path, artifacts_file.filename)
diff --git a/lib/backup/artifacts.rb b/lib/backup/artifacts.rb
index 51fa3867e67..1f4bda6f588 100644
--- a/lib/backup/artifacts.rb
+++ b/lib/backup/artifacts.rb
@@ -3,7 +3,7 @@ require 'backup/files'
 module Backup
   class Artifacts < Files
     def initialize
-      super('artifacts', ArtifactUploader.artifacts_path)
+      super('artifacts', ArtifactUploader.local_artifacts_store)
     end
 
     def create_files_dir
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index 67b269b330c..73d11385dea 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -160,6 +160,7 @@ module Ci
           bad_request!('Missing artifacts file!') unless artifacts
           file_to_large! unless artifacts.size < max_artifacts_size
 
+          build.artifacts_storage_upgraded!
           build.artifacts_file = artifacts
           build.artifacts_metadata = metadata
           build.artifacts_expire_in =
@@ -187,7 +188,7 @@ module Ci
           build = authenticate_build!
           artifacts_file = build.artifacts_file
 
-          unless artifacts_file.file_storage?
+          unless artifacts_file.local_file?
             return redirect_to build.artifacts_file.url
           end
 
diff --git a/lib/tasks/migrate/migrate_artifacts.rake b/lib/tasks/migrate/migrate_artifacts.rake
new file mode 100644
index 00000000000..ebc18fe1b59
--- /dev/null
+++ b/lib/tasks/migrate/migrate_artifacts.rake
@@ -0,0 +1,14 @@
+desc "GitLab | Migrate files for artifacts to comply with new storage format"
+task migrate_artifacts: :environment do
+  puts 'Artifacts'.color(:yellow)
+  Ci::Build.with_artifacts.where(artifacts_file_migrated: nil).find_each(batch_size: 100) do |issue|
+    begin
+      build.artifacts_file.migrate!
+      build.artifacts_metadata.migrate!
+      build.save! if build.changed?
+      print '.'
+    rescue
+      print 'F'
+    end
+  end
+end