Merge branch 'kamil-refactor-ci-builds-v5' into 'master'

Use BuildMetadata to store build configuration in JSONB form

See merge request gitlab-org/gitlab-ce!21499

4 files changed, 63 insertions, 12 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index aeb35538d67..dc6f8ae1a7f 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -8,10 +8,15 @@ module Ci
     include ObjectStorage::BackgroundMove
     include Presentable
     include Importable
+    include IgnorableColumn
     include Gitlab::Utils::StrongMemoize
     include Deployable
     include HasRef
 
+    BuildArchivedError = Class.new(StandardError)
+
+    ignore_column :commands
+
     belongs_to :project, inverse_of: :builds
     belongs_to :runner
     belongs_to :trigger_request
@@ -31,7 +36,7 @@ module Ci
       has_one :"job_artifacts_#{key}", -> { where(file_type: value) }, class_name: 'Ci::JobArtifact', inverse_of: :job, foreign_key: :job_id
     end
 
-    has_one :metadata, class_name: 'Ci::BuildMetadata'
+    has_one :metadata, class_name: 'Ci::BuildMetadata', autosave: true
     has_one :runner_session, class_name: 'Ci::BuildRunnerSession', validate: true, inverse_of: :build
 
     accepts_nested_attributes_for :runner_session
@@ -273,11 +278,14 @@ module Ci
 
     # degenerated build is one that cannot be run by Runner
     def degenerated?
-      self.options.nil?
+      self.options.blank?
     end
 
     def degenerate!
-      self.update!(options: nil, yaml_variables: nil, commands: nil)
+      Build.transaction do
+        self.update!(options: nil, yaml_variables: nil)
+        self.metadata&.destroy
+      end
     end
 
     def archived?
@@ -624,11 +632,23 @@ module Ci
     end
 
     def when
-      read_attribute(:when) || build_attributes_from_config[:when] || 'on_success'
+      read_attribute(:when) || 'on_success'
+    end
+
+    def options
+      read_metadata_attribute(:options, :config_options, {})
     end
 
     def yaml_variables
-      read_attribute(:yaml_variables) || build_attributes_from_config[:yaml_variables] || []
+      read_metadata_attribute(:yaml_variables, :config_variables, [])
+    end
+
+    def options=(value)
+      write_metadata_attribute(:options, :config_options, value)
+    end
+
+    def yaml_variables=(value)
+      write_metadata_attribute(:yaml_variables, :config_variables, value)
     end
 
     def user_variables
@@ -904,8 +924,11 @@ module Ci
     # have the old integer only format. This method returns the retry option
     # normalized as a hash in 11.5+ format.
     def normalized_retry
-      value = options&.dig(:retry)
-      value.is_a?(Integer) ? { max: value } : value.to_h
+      strong_memoize(:normalized_retry) do
+        value = options&.dig(:retry)
+        value = value.is_a?(Integer) ? { max: value } : value.to_h
+        value.with_indifferent_access
+      end
     end
 
     def build_attributes_from_config
@@ -929,5 +952,20 @@ module Ci
     def project_destroyed?
       project.pending_delete?
     end
+
+    def read_metadata_attribute(legacy_key, metadata_key, default_value = nil)
+      read_attribute(legacy_key) || metadata&.read_attribute(metadata_key) || default_value
+    end
+
+    def write_metadata_attribute(legacy_key, metadata_key, value)
+      # save to metadata or this model depending on the state of feature flag
+      if Feature.enabled?(:ci_build_metadata_config)
+        ensure_metadata.write_attribute(metadata_key, value)
+        write_attribute(legacy_key, nil)
+      else
+        write_attribute(legacy_key, value)
+        metadata&.write_attribute(metadata_key, nil)
+      end
+    end
   end
 end
diff --git a/app/models/ci/build_metadata.rb b/app/models/ci/build_metadata.rb
index 9d588b862bd..38390f49217 100644
--- a/app/models/ci/build_metadata.rb
+++ b/app/models/ci/build_metadata.rb
@@ -13,8 +13,12 @@ module Ci
     belongs_to :build, class_name: 'Ci::Build'
     belongs_to :project
 
+    before_create :set_build_project
+
     validates :build, presence: true
-    validates :project, presence: true
+
+    serialize :config_options, Serializers::JSON # rubocop:disable Cop/ActiveRecordSerialize
+    serialize :config_variables, Serializers::JSON # rubocop:disable Cop/ActiveRecordSerialize
 
     chronic_duration_attr_reader :timeout_human_readable, :timeout
 
@@ -33,5 +37,11 @@ module Ci
 
       update(timeout: timeout, timeout_source: timeout_source)
     end
+
+    private
+
+    def set_build_project
+      self.project_id ||= self.build.project_id
+    end
   end
 end
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index 218f1e63d08..fab8a179843 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -2,7 +2,7 @@
 
 module Ci
   class RetryBuildService < ::BaseService
-    CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
+    CLONE_ACCESSORS = %i[pipeline project ref tag options name
                          allow_failure stage stage_id stage_idx trigger_request
                          yaml_variables when environment coverage_regex
                          description tag_list protected].freeze
diff --git a/app/views/projects/ci/lints/_create.html.haml b/app/views/projects/ci/lints/_create.html.haml
index 30bf1384b22..b4c18374220 100644
--- a/app/views/projects/ci/lints/_create.html.haml
+++ b/app/views/projects/ci/lints/_create.html.haml
@@ -13,20 +13,23 @@
       %tbody
         - @stages.each do |stage|
           - @builds.select { |build| build[:stage] == stage }.each do |build|
+            - job = @jobs[build[:name].to_sym]
             %tr
               %td #{stage.capitalize} Job - #{build[:name]}
               %td
-                %pre= build[:commands]
+                %pre= job[:before_script].to_a.join('\n')
+                %pre= job[:script].to_a.join('\n')
+                %pre= job[:after_script].to_a.join('\n')
 
                 %br
                 %b Tag list:
                 = build[:tag_list].to_a.join(", ")
                 %br
                 %b Only policy:
-                = @jobs[build[:name].to_sym][:only].to_a.join(", ")
+                = job[:only].to_a.join(", ")
                 %br
                 %b Except policy:
-                = @jobs[build[:name].to_sym][:except].to_a.join(", ")
+                = job[:except].to_a.join(", ")
                 %br
                 %b Environment:
                 = build[:environment]