Store when and yaml variables in builds table

CHANGELOG

@@ -15,6 +15,7 @@ v 8.10.0 (unreleased)
   - Align flash messages with left side of page content !4959 (winniehell)
   - Display tooltip for "Copy to Clipboard" button !5164 (winniehell)
   - Use default cursor for table header of project files !5165 (winniehell)
+  - Store when and yaml variables in builds table
   - Display last commit of deleted branch in push events !4699 (winniehell)
   - Escape file extension when parsing search results !5141 (winniehell)
   - Apply the trusted_proxies config to the rack request object for use with rack_attack

app/models/ci/build.rb

@@ -5,6 +5,7 @@ module Ci
     belongs_to :erased_by, class_name: 'User'
 
     serialize :options
+    serialize :yaml_variables
 
     validates :coverage, numericality: true, allow_blank: true
     validates_presence_of :ref
@@ -52,6 +53,8 @@ module Ci
         new_build.stage = build.stage
         new_build.stage_idx = build.stage_idx
         new_build.trigger_request = build.trigger_request
+        new_build.yaml_variables = build.yaml_variables
+        new_build.when = build.when
         new_build.user = user
         new_build.save
         MergeRequests::AddTodoWhenBuildFailsService.new(build.project, nil).close(new_build)
@@ -117,7 +120,12 @@ module Ci
     end
 
     def variables
-      predefined_variables + yaml_variables + project_variables + trigger_variables
+      variables = []
+      variables += predefined_variables
+      variables += yaml_variables if yaml_variables
+      variables += project_variables
+      variables += trigger_variables
+      variables
     end
 
     def merge_request
@@ -394,30 +402,6 @@ module Ci
       self.update(erased_by: user, erased_at: Time.now, artifacts_expire_at: nil)
     end
 
-    def yaml_variables
-      global_yaml_variables + job_yaml_variables
-    end
-
-    def global_yaml_variables
-      if pipeline.config_processor
-        pipeline.config_processor.global_variables.map do |key, value|
-          { key: key, value: value, public: true }
-        end
-      else
-        []
-      end
-    end
-
-    def job_yaml_variables
-      if pipeline.config_processor
-        pipeline.config_processor.job_variables(name).map do |key, value|
-          { key: key, value: value, public: true }
-        end
-      else
-        []
-      end
-    end
-
     def project_variables
       project.variables.map do |variable|
         { key: variable.key, value: variable.value, public: false }

app/services/ci/create_builds_service.rb

@@ -36,7 +36,9 @@ module Ci
                            :allow_failure,
                            :stage,
                            :stage_idx,
-                           :environment)
+                           :environment,
+                           :when,
+                           :yaml_variables)
 
         build_attrs.merge!(pipeline: @pipeline,
                            ref: @pipeline.ref,

db/migrate/20160716115710_add_when_and_yaml_variables_to_ci_builds.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddWhenAndYamlVariablesCiBuilds < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    add_column :ci_builds, :when, :string
+    add_column :ci_builds, :yaml_variables, :text
+  end
+end

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: 20160712171823) do
+ActiveRecord::Schema.define(version: 20160716115710) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -168,6 +168,8 @@ ActiveRecord::Schema.define(version: 20160712171823) do
     t.datetime "artifacts_expire_at"
     t.string   "environment"
     t.integer  "artifacts_size"
+    t.string   "when"
+    t.text     "yaml_variables"
   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

lib/ci/gitlab_ci_yaml_processor.rb

@@ -31,28 +31,34 @@ module Ci
       raise ValidationError, e.message
     end
 
-    def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
-      builds.select do |build|
-        build[:stage] == stage &&
-          process?(build[:only], build[:except], ref, tag, trigger_request)
+    def jobs_for_ref(ref, tag = false, trigger_request = nil)
+      @jobs.select do |_, job|
+        process?(job[:only], job[:except], ref, tag, trigger_request)
       end
     end
 
-    def builds
-      @jobs.map do |name, job|
-        build_job(name, job)
+    def jobs_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
+      jobs_for_ref(ref, tag, trigger_request).select do |_, job|
+        job[:stage] == stage
       end
     end
 
-    def global_variables
-      @variables
+    def builds_for_ref(ref, tag = false, trigger_request = nil)
+      jobs_for_ref(ref, tag, trigger_request).map do |name, job|
+        build_job(name, job)
+      end
     end
 
-    def job_variables(name)
-      job = @jobs[name.to_sym]
-      return [] unless job
+    def builds_for_stage_and_ref(stage, ref, tag = false, trigger_request = nil)
+      jobs_for_stage_and_ref(stage, ref, tag, trigger_request).map do |name, job|
+        build_job(name, job)
+      end
+    end
 
-      job[:variables] || []
+    def builds
+      @jobs.map do |name, job|
+        build_job(name, job)
+      end
     end
 
     private
@@ -95,11 +101,10 @@ module Ci
         commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
         tag_list: job[:tags] || [],
         name: name,
-        only: job[:only],
-        except: job[:except],
         allow_failure: job[:allow_failure] || false,
         when: job[:when] || 'on_success',
         environment: job[:environment],
+        yaml_variables: yaml_variables(name),
         options: {
           image: job[:image] || @image,
           services: job[:services] || @services,
@@ -111,6 +116,24 @@ module Ci
       }
     end
 
+    def yaml_variables(name)
+      variables = global_variables.merge(job_variables(name))
+      variables.map do |key, value|
+        { key: key, value: value, public: true }
+      end
+    end
+
+    def global_variables
+      @variables || {}
+    end
+
+    def job_variables(name)
+      job = @jobs[name.to_sym]
+      return {} unless job
+
+      job[:variables] || {}
+    end
+
     def validate!
       @jobs.each do |name, job|
         validate_job!(name, job)

spec/factories/ci/builds.rb

@@ -15,6 +15,11 @@ FactoryGirl.define do
         services: ["postgres"]
       }
     end
+    yaml_variables do
+      [
+        { key: :DB_NAME, value: 'postgres', public: true }
+      ]
+    end
 
     pipeline factory: :ci_pipeline
 

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -19,15 +19,14 @@ module Ci
         expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({
           stage: "test",
           stage_idx: 1,
-          except: nil,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {},
           allow_failure: false,
           when: "on_success",
           environment: nil,
+          yaml_variables: {}
         })
       end
 
@@ -432,11 +431,9 @@ module Ci
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
-          except: nil,
           stage: "test",
           stage_idx: 1,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -446,6 +443,7 @@ module Ci
           allow_failure: false,
           when: "on_success",
           environment: nil,
+          yaml_variables: {}
         })
       end
 
@@ -461,11 +459,9 @@ module Ci
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
-          except: nil,
           stage: "test",
           stage_idx: 1,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -475,6 +471,7 @@ module Ci
           allow_failure: false,
           when: "on_success",
           environment: nil,
+          yaml_variables: []
         })
       end
     end
@@ -681,11 +678,9 @@ module Ci
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
-          except: nil,
           stage: "test",
           stage_idx: 1,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -701,6 +696,7 @@ module Ci
           when: "on_success",
           allow_failure: false,
           environment: nil,
+          yaml_variables: {}
         })
       end
 
@@ -819,17 +815,16 @@ module Ci
         it "doesn't create jobs that start with dot" do
           expect(subject.size).to eq(1)
           expect(subject.first).to eq({
-            except: nil,
             stage: "test",
             stage_idx: 1,
             name: :normal_job,
-            only: nil,
             commands: "test",
             tag_list: [],
             options: {},
             when: "on_success",
             allow_failure: false,
             environment: nil,
+            yaml_variables: {}
           })
         end
       end
@@ -865,30 +860,28 @@ module Ci
         it "is correctly supported for jobs" do
           expect(subject.size).to eq(2)
           expect(subject.first).to eq({
-            except: nil,
             stage: "build",
             stage_idx: 0,
             name: :job1,
-            only: nil,
             commands: "execute-script-for-job",
             tag_list: [],
             options: {},
             when: "on_success",
             allow_failure: false,
             environment: nil,
+            yaml_variables: {}
           })
           expect(subject.second).to eq({
-            except: nil,
             stage: "build",
             stage_idx: 0,
             name: :job2,
-            only: nil,
             commands: "execute-script-for-job",
             tag_list: [],
             options: {},
             when: "on_success",
             allow_failure: false,
             environment: nil,
+            yaml_variables: {}
           })
         end
       end

spec/models/build_spec.rb

@@ -208,7 +208,7 @@ describe Ci::Build, models: true do
       end
 
       before do
-        build.update_attributes(stage: 'stage')
+        build.update_attributes(stage: 'stage', yaml_variables: yaml_variables)
       end
 
       it { is_expected.to eq(predefined_variables + yaml_variables) }
@@ -260,22 +260,6 @@ describe Ci::Build, models: true do
 
           it { is_expected.to eq(predefined_variables + predefined_trigger_variable + yaml_variables + secure_variables + trigger_variables) }
         end
-
-        context 'when job variables are defined' do
-          ##
-          # Job-level variables are defined in gitlab_ci.yml fixture
-          #
-          context 'when job variables are unique' do
-            let(:build) { create(:ci_build, name: 'staging') }
-
-            it 'includes job variables' do
-              expect(subject).to include(
-                { key: :KEY1, value: 'value1', public: true },
-                { key: :KEY2, value: 'value2', public: true }
-              )
-            end
-          end
-        end
       end
     end
   end