Just use the url from options, not saving it as a column

10 files changed, 10 insertions, 34 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 754b1047644..79edacc02f3 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -26,10 +26,6 @@ module Ci
 
     validates :coverage, numericality: true, allow_blank: true
     validates :ref, presence: true
-    validates :environment_url,
-              length: { maximum: 255 },
-              allow_nil: true,
-              addressable_url: true
 
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :ignore_failures, ->() { where(allow_failure: false) }
@@ -147,6 +143,10 @@ module Ci
         environment_url
     end
 
+    def environment_url
+      options.dig(:environment, :url)
+    end
+
     def has_environment?
       environment.present?
     end
diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index 8e406d22f15..f51e9fd1d54 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -2,8 +2,8 @@ module Ci
   class RetryBuildService < ::BaseService
     CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
                          allow_failure stage stage_idx trigger_request
-                         yaml_variables when environment environment_url
-                         coverage_regex description tag_list].freeze
+                         yaml_variables when environment coverage_regex
+                         description tag_list].freeze
 
     def execute(build)
       reprocess!(build).tap do |new_build|
diff --git a/db/migrate/20170524195203_add_environment_url_to_ci_builds.rb b/db/migrate/20170524195203_add_environment_url_to_ci_builds.rb
deleted file mode 100644
index 191e7768693..00000000000
--- a/db/migrate/20170524195203_add_environment_url_to_ci_builds.rb
+++ /dev/null
@@ -1,9 +0,0 @@
-class AddEnvironmentUrlToCiBuilds < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  def change
-    add_column(:ci_builds, :environment_url, :string)
-  end
-end
diff --git a/db/schema.rb b/db/schema.rb
index 255da64d1a1..bac8f95ce3b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -233,7 +233,6 @@ ActiveRecord::Schema.define(version: 20170525174156) do
     t.string "coverage_regex"
     t.integer "auto_canceled_by_id"
     t.boolean "retried"
-    t.string "environment_url"
   end
 
   add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree
@@ -1493,4 +1492,4 @@ ActiveRecord::Schema.define(version: 20170525174156) do
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
   add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade
-end
+end
\ No newline at end of file
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index d49c6c05b50..b06474cda7f 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -61,7 +61,6 @@ module Ci
         allow_failure: job[:ignore],
         when: job[:when] || 'on_success',
         environment: job[:environment_name],
-        environment_url: job[:environment_url],
         coverage_regex: job[:coverage],
         yaml_variables: yaml_variables(name),
         options: {
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 0891d6ac8bf..176301bcca1 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -141,7 +141,6 @@ module Gitlab
               variables: variables_defined? ? variables_value : nil,
               environment: environment_defined? ? environment_value : nil,
               environment_name: environment_defined? ? environment_value[:name] : nil,
-              environment_url: environment_defined? ? environment_value[:url] : nil,
               coverage: coverage_defined? ? coverage_value : nil,
               artifacts: artifacts_value,
               after_script: after_script_value,
diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index aeabb57c512..0bb5a86d9b9 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -63,7 +63,6 @@ FactoryGirl.define do
 
     trait :teardown_environment do
       environment 'staging'
-      environment_url 'http://staging.example.com/$CI_JOB_NAME'
       options environment: { name: 'staging',
                              action: 'stop',
                              url: 'http://staging.example.com/$CI_JOB_NAME' }
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 177b31aba33..fe2c00bb2ca 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -105,7 +105,6 @@ module Ci
           allow_failure: false,
           when: "on_success",
           environment: nil,
-          environment_url: nil,
           yaml_variables: []
         })
       end
@@ -524,7 +523,6 @@ module Ci
           allow_failure: false,
           when: "on_success",
           environment: nil,
-          environment_url: nil,
           yaml_variables: []
         })
       end
@@ -554,7 +552,6 @@ module Ci
           allow_failure: false,
           when: "on_success",
           environment: nil,
-          environment_url: nil,
           yaml_variables: []
         })
       end
@@ -801,7 +798,6 @@ module Ci
           when: "on_success",
           allow_failure: false,
           environment: nil,
-          environment_url: nil,
           yaml_variables: []
         })
       end
@@ -853,7 +849,6 @@ module Ci
         it 'does return production and URL' do
           expect(builds.size).to eq(1)
           expect(builds.first[:environment]).to eq(environment[:name])
-          expect(builds.first[:environment_url]).to eq(environment[:url])
           expect(builds.first[:options]).to include(environment: environment)
         end
 
@@ -866,7 +861,6 @@ module Ci
           it 'allows a variable for the port' do
             expect(builds.size).to eq(1)
             expect(builds.first[:environment]).to eq(environment[:name])
-            expect(builds.first[:environment_url]).to eq(environment[:url])
             expect(builds.first[:options]).to include(environment: environment)
           end
         end
@@ -1007,7 +1001,6 @@ module Ci
             when: "on_success",
             allow_failure: false,
             environment: nil,
-            environment_url: nil,
             yaml_variables: []
           })
         end
@@ -1054,7 +1047,6 @@ module Ci
             when: "on_success",
             allow_failure: false,
             environment: nil,
-            environment_url: nil,
             yaml_variables: []
           })
           expect(subject.second).to eq({
@@ -1068,7 +1060,6 @@ module Ci
             when: "on_success",
             allow_failure: false,
             environment: nil,
-            environment_url: nil,
             yaml_variables: []
           })
         end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 958784d8382..54ce8051f30 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -225,7 +225,6 @@ CommitStatus:
 - erased_at
 - artifacts_expire_at
 - environment
-- environment_url
 - artifacts_size
 - when
 - yaml_variables
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 2df439819ef..141a2741ced 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -20,7 +20,6 @@ describe Ci::Build, :models do
   it { is_expected.to validate_presence_of(:ref) }
   it { is_expected.to respond_to(:has_trace?) }
   it { is_expected.to respond_to(:trace) }
-  it { is_expected.to validate_length_of(:environment_url).is_at_most(255) }
 
   describe '#actionize' do
     context 'when build is a created' do
@@ -435,7 +434,7 @@ describe Ci::Build, :models do
         let(:build) do
           create(:ci_build,
                  ref: 'master',
-                 environment_url: 'http://review/$CI_COMMIT_REF_NAME')
+                 options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } })
         end
 
         it { is_expected.to eq('http://review/master') }
@@ -445,7 +444,7 @@ describe Ci::Build, :models do
         let(:build) do
           create(:ci_build,
                  yaml_variables: [{ key: :APP_HOST, value: 'host' }],
-                 environment_url: 'http://review/$APP_HOST')
+                 options: { environment: { url: 'http://review/$APP_HOST' } })
         end
 
         it { is_expected.to eq('http://review/host') }
@@ -1244,7 +1243,7 @@ describe Ci::Build, :models do
 
         context 'when the URL was set from the job' do
           before do
-            build.update(environment_url: 'http://host/$CI_JOB_NAME')
+            build.update(options: { environment: { url: 'http://host/$CI_JOB_NAME' } })
           end
 
           it_behaves_like 'containing environment variables'