Change environment intial state to created instead of available27644-environment-state-created

Because we create environments too early (and rely on this) they may not
have successfull deployments yet but are mark as `available`. Introduce
new `created` intitial state so that it is easier to differentiate them.

6 files changed, 105 insertions, 26 deletions

diff --git a/app/models/environment.rb b/app/models/environment.rb
index 392481ea0cc..14000ae7bb0 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -59,15 +59,16 @@ class Environment < ApplicationRecord
   scope :for_project, -> (project) { where(project_id: project) }
   scope :with_deployment, -> (sha) { where('EXISTS (?)', Deployment.select(1).where('deployments.environment_id = environments.id').where(sha: sha)) }
 
-  state_machine :state, initial: :available do
+  state_machine :state, initial: :created do
     event :start do
-      transition stopped: :available
+      transition [:created, :stopped] => :available
     end
 
     event :stop do
-      transition available: :stopped
+      transition [:created, :available] => :stopped
     end
 
+    state :created
     state :available
     state :stopped
 
@@ -138,7 +139,7 @@ class Environment < ApplicationRecord
   end
 
   def stop_with_action!(current_user)
-    return unless available?
+    return unless available? || created?
 
     stop!
     stop_action&.play(current_user)
diff --git a/db/migrate/20190731141037_change_environments_state_default.rb b/db/migrate/20190731141037_change_environments_state_default.rb
new file mode 100644
index 00000000000..b58315b5980
--- /dev/null
+++ b/db/migrate/20190731141037_change_environments_state_default.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class ChangeEnvironmentsStateDefault < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    change_column_default(:environments, :state, 'created')
+  end
+
+  def down
+    change_column_default(:environments, :state, 'available')
+  end
+end
diff --git a/db/post_migrate/20190801154748_update_state_for_environments_without_deployments.rb b/db/post_migrate/20190801154748_update_state_for_environments_without_deployments.rb
new file mode 100644
index 00000000000..ebd26d4d012
--- /dev/null
+++ b/db/post_migrate/20190801154748_update_state_for_environments_without_deployments.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class UpdateStateForEnvironmentsWithoutDeployments < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    sql = <<~SQL
+      WITH no_deployments AS (
+          SELECT
+              environment_id,
+              MAX(status)
+          FROM
+              deployments
+          GROUP BY
+              environment_id
+          HAVING
+              max(status) = 0)
+      UPDATE
+          environments
+      SET
+          state = 'created'
+      WHERE
+          EXISTS (
+              SELECT
+                  1
+              FROM
+                  no_deployments
+              WHERE
+                  environment_id = environments.id)
+          AND state = 'available'
+    SQL
+
+    execute(sql)
+  end
+
+  def down
+    execute "UPDATE environments SET state='available' WHERE state='created'"
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 6f5fc6c65eb..13bdfc2b66c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_07_29_090456) do
+ActiveRecord::Schema.define(version: 2019_08_01_154748) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -1169,7 +1169,7 @@ ActiveRecord::Schema.define(version: 2019_07_29_090456) do
     t.datetime "updated_at"
     t.string "external_url"
     t.string "environment_type"
-    t.string "state", default: "available", null: false
+    t.string "state", default: "created", null: false
     t.string "slug", null: false
     t.index ["name"], name: "index_environments_on_name_varchar_pattern_ops", opclass: :varchar_pattern_ops
     t.index ["project_id", "name"], name: "index_environments_on_project_id_and_name", unique: true
diff --git a/spec/features/projects/environments/environments_spec.rb b/spec/features/projects/environments/environments_spec.rb
index 1a2302b3d0c..c7f3856e517 100644
--- a/spec/features/projects/environments/environments_spec.rb
+++ b/spec/features/projects/environments/environments_spec.rb
@@ -122,32 +122,28 @@ describe 'Environments page', :js do
   end
 
   describe 'environments table' do
-    let!(:environment) do
-      create(:environment, project: project, state: :available)
-    end
-
     context 'when there are no deployments' do
+      let!(:environment) do
+        create(:environment, project: project, state: :created)
+      end
+
       before do
         visit_environments(project)
       end
 
-      it 'shows environments names and counters' do
-        expect(page).to have_link(environment.name)
-
-        expect(page.find('.js-environments-tab-available .badge').text).to eq('1')
+      it 'does not shows environments names and counters' do
+        expect(page).not_to have_link(environment.name)
+        expect(page).not_to have_content('No deployments yet')
+        expect(page.find('.js-environments-tab-available .badge').text).to eq('0')
         expect(page.find('.js-environments-tab-stopped .badge').text).to eq('0')
       end
-
-      it 'does not show deployments' do
-        expect(page).to have_content('No deployments yet')
-      end
-
-      it 'does not show stip button when environment is not stoppable' do
-        expect(page).not_to have_selector(stop_button_selector)
-      end
     end
 
     context 'when there are successful deployments' do
+      let!(:environment) do
+        create(:environment, project: project, state: :available)
+      end
+
       let(:project) { create(:project, :repository) }
 
       let!(:deployment) do
@@ -215,7 +211,7 @@ describe 'Environments page', :js do
         end
 
         context 'with external_url' do
-          let(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') }
+          let(:environment) { create(:environment, project: project, state: :available, external_url: 'https://git.gitlab.com') }
           let(:build) { create(:ci_build, pipeline: pipeline) }
           let(:deployment) { create(:deployment, :success, environment: environment, deployable: build) }
 
@@ -339,6 +335,10 @@ describe 'Environments page', :js do
     context 'when there is a failed deployment' do
       let(:project) { create(:project, :repository) }
 
+      let!(:environment) do
+        create(:environment, project: project, state: :available)
+      end
+
       let!(:deployment) do
         create(:deployment, :failed,
                             environment: environment,
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index ce0681c4331..bbe0fdada50 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -517,7 +517,15 @@ describe Environment, :use_clean_rails_memory_store_caching do
   describe '#has_terminals?' do
     subject { environment.has_terminals? }
 
+    context 'when the environment is created' do
+      it { is_expected.to be_falsy }
+    end
+
     context 'when the environment is available' do
+      before do
+        environment.start
+      end
+
       context 'with a deployment service' do
         context 'when user configured kubernetes from CI/CD > Clusters' do
           let!(:cluster) { create(:cluster, :project, :provided_by_gcp) }
@@ -525,6 +533,7 @@ describe Environment, :use_clean_rails_memory_store_caching do
 
           context 'with deployment' do
             let!(:deployment) { create(:deployment, :success, environment: environment) }
+
             it { is_expected.to be_truthy }
           end
 
@@ -539,7 +548,7 @@ describe Environment, :use_clean_rails_memory_store_caching do
       end
     end
 
-    context 'when the environment is unavailable' do
+    context 'when the environment is stopped' do
       before do
         environment.stop
       end
@@ -609,7 +618,7 @@ describe Environment, :use_clean_rails_memory_store_caching do
   describe '#calculate_reactive_cache' do
     let(:cluster) { create(:cluster, :project, :provided_by_user) }
     let(:project) { cluster.project }
-    let(:environment) { create(:environment, project: project) }
+    let(:environment) { create(:environment, project: project, state: :available) }
     let!(:deployment) { create(:deployment, :success, environment: environment) }
 
     subject { environment.calculate_reactive_cache }
@@ -641,7 +650,17 @@ describe Environment, :use_clean_rails_memory_store_caching do
   describe '#has_metrics?' do
     subject { environment.has_metrics? }
 
+    context 'when the environment is created' do
+      let(:project) { create(:prometheus_project) }
+
+      it { is_expected.to be_falsy }
+    end
+
     context 'when the environment is available' do
+      before do
+        environment.start
+      end
+
       context 'with a deployment service' do
         let(:project) { create(:prometheus_project) }
 
@@ -660,7 +679,7 @@ describe Environment, :use_clean_rails_memory_store_caching do
       end
     end
 
-    context 'when the environment is unavailable' do
+    context 'when the environment is stopped' do
       let(:project) { create(:prometheus_project) }
 
       before do