Allow to store artifacts on two storages, internal and external

app/models/ci/build.rb

@@ -52,11 +52,6 @@ module Ci
     after_commit :update_project_statistics_after_save, on: [:create, :update]
     after_commit :update_project_statistics, on: :destroy
 
-    enum artifacts_storage: { 
-      artifacts_storage_undefined: nil,
-      artifacts_storage_upgraded: 1,
-    }
-
     class << self
       # This is needed for url_for to work,
       # as the controller is JobsController

app/uploaders/artifact_uploader.rb

-class ArtifactUploader < GitlabUploader
-  include ObjectStoreable
+class ArtifactUploader < ObjectStoreUploader
+  attr_reader :subject, :field
 
-  attr_reader :job, :field
+  storage_options Gitlab.config.artifacts
 
   def self.local_artifacts_store
     Gitlab.config.artifacts.path
@@ -11,8 +11,9 @@ class ArtifactUploader < GitlabUploader
     File.join(self.local_artifacts_store, 'tmp/uploads/')
   end
 
-  def initialize(job, field)
-    @job, @field = job, field
+  def initialize(subject, field)
+    @subject = subject
+    @field = field
   end
 
   def store_dir
@@ -25,16 +26,12 @@ class ArtifactUploader < GitlabUploader
 
   def cache_dir
     if file_cache_storage?
-      File.join(self.local_artifacts_store, 'tmp/cache')
+      File.join(self.class.local_artifacts_store, 'tmp/cache')
     else
       'tmp/cache'
     end
   end
 
-  def migrate!
-    # TODO
-  end
-
   private
 
   def default_local_path
@@ -42,6 +39,6 @@ class ArtifactUploader < GitlabUploader
   end
 
   def default_path
-    File.join(job.created_at.utc.strftime('%Y_%m'), job.project_id.to_s, job.id.to_s)
+    File.join(subject.created_at.utc.strftime('%Y_%m'), subject.project_id.to_s, subject.id.to_s)
   end
 end

app/uploaders/concerns/object_storeable.rb

-module ObjectStoreable
-  extend ActiveSupport::Concern
-
-  module ClassMethods
-    def use_object_store?
-      @storage_options.object_store.enabled
-    end
-
-    def storage_options(options)
-      @storage_options = options
-
-      class_eval do
-        storage use_object_store? ? :fog : :file
-      end
-    end
-  end
-
-  def fog_directory
-    return super unless use_object_store?
-
-    @storage_options.bucket
-  end
-
-  # Override the credentials
-  def fog_credentials
-    return super unless use_object_store?
-
-    {
-      provider:              @storage_options.provider,
-      aws_access_key_id:     @storage_options.access_key_id,
-      aws_secret_access_key: @storage_options.secret_access_key,
-      region:                @storage_options.region,
-      endpoint:              @storage_options.endpoint,
-      path_style:            true
-    }
-  end
-
-  def fog_public
-    false
-  end
-
-  def use_object_store?
-    @storage_options.object_store.enabled
-  end
-
-  def move_to_store
-    !use_object_store?
-  end
-
-  def move_to_cache
-    !use_object_store?
-  end
-
-  def use_file
-    if use_object_store?
-      return yield path
-    end
-
-    begin
-      cache_stored_file!
-      yield cache_path
-    ensure
-      cache_storage.delete_dir!(cache_path(nil))
-    end
-  end
-
-  def upload_authorize
-    self.cache_id = CarrierWave.generate_cache_id
-    self.original_filename = SecureRandom.hex
-
-    result = { TempPath: cache_path }
-
-    use_cache_object_storage do
-      expire_at = ::Fog::Time.now + fog_authenticated_url_expiration
-      result[:ObjectStore] = {
-        ObjectID: cache_name,
-        StoreURL: storage.connection.put_object_url(
-          fog_directory, cache_path, expire_at)
-      }
-    end
-
-    result
-  end
-
-  def cache!(new_file = nil)
-    unless retrive_uploaded_file!(new_file&.object_id, new_file.original_filename)
-      super
-    end
-  end
-
-  def cache_storage
-    if @use_storage_for_cache || cached? && remote_file?
-      storage
-    else
-      super
-    end
-  end
-
-  def retrive_uploaded_file!(identifier, filename)
-    return unless identifier
-    return unless filename
-    return unless use_object_store?
-
-    @use_storage_for_cache = true
-
-    retrieve_from_cache!(identifier)
-    @filename = filename
-  ensure
-    @use_storage_for_cache = false
-  end
-end

app/uploaders/gitlab_uploader.rb

@@ -38,10 +38,6 @@ class GitlabUploader < CarrierWave::Uploader::Base
     self.file.path.sub("#{root}/", '')
   end
 
-  def use_file
-    yield path
-  end
-
   def exists?
     file.try(:exists?)
   end

app/uploaders/object_store_uploader.rb

+class ObjectStoreUploader < GitlabUploader
+  before :store, :set_default_local_store
+
+  LOCAL_STORE = 1
+  REMOTE_STORE = 2
+
+  def object_store
+    subject.public_send(:"#{field}_store")
+  end
+
+  def object_store=(value)
+    @storage = nil
+    subject.public_send(:"#{field}_store=", value)
+  end
+
+  def self.storage_options(options)
+    @storage_options = options
+  end
+
+  def self.object_store_options
+    @storage_options&.object_store
+  end
+
+  def self.object_store_enabled?
+    object_store_options&.enabled
+  end
+
+  def use_file
+    unless object_store == REMOTE_STORE
+      return yield path
+    end
+
+    begin
+      cache_stored_file!
+      yield cache_path
+    ensure
+      cache_storage.delete_dir!(cache_path(nil))
+    end
+  end
+
+  def migrate!(new_store)
+    raise 'Undefined new store' unless new_store
+
+    return unless object_store != new_store
+
+    # retrive file from current storage
+    current_file = file
+
+    # change storage
+    self.object_store = new_store
+
+    # store file on a new storage
+    new_file = storage.store!(current_file)
+
+    # since we change storage store the new storage
+    # in case of failure delete new file
+    begin
+      subject.save!
+    rescue
+      new_file.delete
+    end
+
+    # since we migrated file we can delete it now
+    current_file.delete
+  end
+
+  def move_to_store
+    object_store != REMOTE_STORE
+  end
+
+  def move_to_cache
+    false
+  end
+
+  def fog_directory
+    self.class.object_store_options.bucket
+  end
+
+  def fog_credentials
+    object_store_options = self.class.object_store_options
+    {
+      provider:              object_store_options.provider,
+      aws_access_key_id:     object_store_options.access_key_id,
+      aws_secret_access_key: object_store_options.secret_access_key,
+      region:                object_store_options.region,
+      endpoint:              object_store_options.endpoint,
+      path_style:            true
+    }
+  end
+
+  def fog_public
+    false
+  end
+
+  private
+
+  def set_default_local_store(new_file)
+    object_store ||= LOCAL_STORE
+  end
+
+  def storage
+    @storage ||=
+      if object_store == REMOTE_STORE
+        remote_storage
+      else
+        local_storage
+      end
+  end
+
+  def remote_storage
+    raise 'Object Storage is not enabled' unless self.class.object_store_enabled?
+
+    CarrierWave::Storage::Fog.new(self)
+  end
+
+  def local_storage
+    CarrierWave::Storage::File.new(self)
+  end
+end

db/migrate/20170504203205_add_file_storage_to_ci_build.rb

-class AddArtifactsFileStorageToCiBuild < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  def change
-    add_column :ci_builds, :artifacts_storage, :integer
-  end
-end

db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb

+class AddArtifactsStoreToCiBuild < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def change
+    add_column_with_default(:ci_builds, :artifacts_file_store, :integer, default: 1)
+    add_column_with_default(:ci_builds, :artifacts_metadata_store, :integer, default: 1)
+  end
+end

db/schema.rb

@@ -284,7 +284,8 @@ ActiveRecord::Schema.define(version: 20170602003304) do
     t.string "coverage_regex"
     t.integer "auto_canceled_by_id"
     t.boolean "retried"
-    t.integer "artifacts_storage"
+    t.integer "artifacts_file_store", default: 1, null: false
+    t.integer "artifacts_metadata_store", default: 1, null: false
   end
 
   add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree

lib/api/helpers.rb

@@ -290,7 +290,7 @@ module API
 
     # file helpers
 
-    def uploaded_file(uploader, field)
+    def uploaded_file(field, uploads_path)
       if params[field]
         bad_request!("#{field} is not a file") unless params[field].respond_to?(:filename)
         return params[field]
@@ -301,15 +301,14 @@ module API
       # sanitize file paths
       # this requires all paths to exist
       required_attributes! %W(#{field}.path)
-      artifacts_store = File.realpath(uploader.local_artifacts_store)
+      uploads_path = File.realpath(uploads_path)
       file_path = File.realpath(params["#{field}.path"])
-      bad_request!('Bad file path') unless file_path.start_with?(artifacts_store)
+      bad_request!('Bad file path') unless file_path.start_with?(uploads_path)
 
       UploadedFile.new(
         file_path,
         params["#{field}.name"],
-        params["#{field}.type"] || 'application/octet-stream',
-        params["#{field}.object_id"]
+        params["#{field}.type"] || 'application/octet-stream'
       )
     end
 

lib/api/runner.rb

@@ -181,7 +181,7 @@ module API
 
         status 200
         content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
-        job.artifacts_file.upload_authorize
+        Gitlab::Workhorse.artifact_upload_ok
       end
 
       desc 'Upload artifacts for job' do
@@ -199,7 +199,6 @@ module API
         optional :file, type: File, desc: %q(Artifact's file)
         optional 'file.path', type: String, desc: %q(path to locally stored body (generated by Workhorse))
         optional 'file.name', type: String, desc: %q(real filename as send in Content-Disposition (generated by Workhorse))
-        optional 'file.object_id', type: String, desc: %q(object_id as send by authorize (generated by Workhorse))
         optional 'file.type', type: String, desc: %q(real content type as send in Content-Type (generated by Workhorse))
         optional 'metadata.path', type: String, desc: %q(path to locally stored body (generated by Workhorse))
         optional 'metadata.name', type: String, desc: %q(filename (generated by Workhorse))
@@ -211,13 +210,13 @@ module API
         job = authenticate_job!
         forbidden!('Job is not running!') unless job.running?
 
-        artifacts = uploaded_file(job.artifacts_file, :file)
-        metadata = uploaded_file(job.artifacts_metadata, :metadata)
+        artifacts_upload_path = ArtifactUploader.artifacts_upload_path
+        artifacts = uploaded_file(:file, artifacts_upload_path)
+        metadata = uploaded_file(:metadata, artifacts_upload_path)
 
         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'] ||

lib/ci/api/builds.rb

@@ -124,7 +124,7 @@ module Ci
 
           status 200
           content_type Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE
-          build.artifacts_file.upload_authorize
+          Gitlab::Workhorse.artifact_upload_ok
         end
 
         # Upload artifacts to build - Runners only
@@ -153,13 +153,13 @@ module Ci
           build = authenticate_build!
           forbidden!('Build is not running!') unless build.running?
 
-          artifacts = uploaded_file(build.artifacts_file, :file)
-          metadata = uploaded_file(build.artifacts_metadata, :metadata)
+          artifacts_upload_path = ArtifactUploader.artifacts_upload_path
+          artifacts = uploaded_file(:file, artifacts_upload_path)
+          metadata = uploaded_file(:metadata, artifacts_upload_path)
 
           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 =

lib/tasks/migrate/migrate_artifacts.rake

 desc "GitLab | Migrate files for artifacts to comply with new storage format"
 task migrate_artifacts: :environment do
   puts 'Artifacts'.color(:yellow)
-  Ci::Build.joins(:project)
-    .with_artifacts
-    .where(artifacts_file_migrated: nil)
+  Ci::Build.joins(:project).with_artifacts
+    .where(artifacts_file_store: ArtifactUploader::LOCAL_STORE)
     .find_each(batch_size: 100) do |issue|
     begin
-      build.artifacts_file.migrate!
-      build.artifacts_metadata.migrate!
-      build.save! if build.changed?
+      build.artifacts_file.migrate!(ArtifactUploader::REMOTE_STORE)
+      build.artifacts_metadata.migrate!(ArtifactUploader::REMOTE_STORE)
       print '.'
     rescue
       print 'F'

lib/uploaded_file.rb

@@ -9,19 +9,15 @@ class UploadedFile
   # The tempfile
   attr_reader :tempfile
 
-  # The object_id for asynchronous uploads
-  attr_reader :object_id
-
   # The content type of the "uploaded" file
   attr_accessor :content_type
 
-  def initialize(path, filename, content_type = "text/plain", object_id = nil)
+  def initialize(path, filename, content_type = "text/plain")
     raise "#{path} file does not exist" unless ::File.exist?(path)
 
     @content_type = content_type
     @original_filename = filename || ::File.basename(path)
     @tempfile = File.new(path, 'rb')
-    @object_id = object_id
   end
 
   def path