Merge branch 'fix/missing-ci-build-traces' into 'master'

Hotfix for builds trace data integrity

Closes #4246

See merge request !2224

2 files changed, 57 insertions, 9 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 7b89fe069ea..3e67b2771c1 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -194,8 +194,11 @@ module Ci
     end
 
     def raw_trace
-      if File.exist?(path_to_trace)
+      if File.file?(path_to_trace)
         File.read(path_to_trace)
+      elsif project.ci_id && File.file?(old_path_to_trace)
+        # Temporary fix for build trace data integrity
+        File.read(old_path_to_trace)
       else
         # backward compatibility
         read_attribute :trace
@@ -212,8 +215,8 @@ module Ci
     end
 
     def trace=(trace)
-      unless Dir.exists? dir_to_trace
-        FileUtils.mkdir_p dir_to_trace
+      unless Dir.exists?(dir_to_trace)
+        FileUtils.mkdir_p(dir_to_trace)
       end
 
       File.write(path_to_trace, trace)
@@ -231,6 +234,55 @@ module Ci
       "#{dir_to_trace}/#{id}.log"
     end
 
+    ##
+    # Deprecated
+    #
+    # This is a hotfix for CI build data integrity, see #4246
+    # Should be removed in 8.4, after CI files migration has been done.
+    #
+    def old_dir_to_trace
+      File.join(
+        Settings.gitlab_ci.builds_path,
+        created_at.utc.strftime("%Y_%m"),
+        project.ci_id.to_s
+      )
+    end
+
+    ##
+    # Deprecated
+    #
+    # This is a hotfix for CI build data integrity, see #4246
+    # Should be removed in 8.4, after CI files migration has been done.
+    #
+    def old_path_to_trace
+      "#{old_dir_to_trace}/#{id}.log"
+    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
+      old = File.join(created_at.utc.strftime('%Y_%m'),
+                      project.ci_id.to_s,
+                      id.to_s)
+
+      old_store = File.join(ArtifactUploader.artifacts_path, old)
+      return old if project.ci_id && File.directory?(old_store)
+
+      File.join(
+        created_at.utc.strftime('%Y_%m'),
+        project.id.to_s,
+        id.to_s
+      )
+    end
+
     def token
       project.runners_token
     end
diff --git a/app/uploaders/artifact_uploader.rb b/app/uploaders/artifact_uploader.rb
index 1dccc39e7e2..1b0ae6c0056 100644
--- a/app/uploaders/artifact_uploader.rb
+++ b/app/uploaders/artifact_uploader.rb
@@ -20,16 +20,12 @@ class ArtifactUploader < CarrierWave::Uploader::Base
     @build, @field = build, field
   end
 
-  def artifacts_path
-    File.join(build.created_at.utc.strftime('%Y_%m'), build.project.id.to_s, build.id.to_s)
-  end
-
   def store_dir
-    File.join(ArtifactUploader.artifacts_path, artifacts_path)
+    File.join(self.class.artifacts_path, @build.artifacts_path)
   end
 
   def cache_dir
-    File.join(ArtifactUploader.artifacts_cache_path, artifacts_path)
+    File.join(self.class.artifacts_cache_path, @build.artifacts_path)
   end
 
   def file_storage?