From ffd54f17f8fff7a659fd4ca116eabc338a5acd95 Mon Sep 17 00:00:00 2001 From: Krasimir Angelov Date: Fri, 2 Aug 2019 09:02:05 +0300 Subject: Change environment intial state to created instead of available 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. --- app/models/environment.rb | 9 +++-- ...0731141037_change_environments_state_default.rb | 15 ++++++++ ...e_state_for_environments_without_deployments.rb | 44 ++++++++++++++++++++++ db/schema.rb | 4 +- .../projects/environments/environments_spec.rb | 34 ++++++++--------- spec/models/environment_spec.rb | 25 ++++++++++-- 6 files changed, 105 insertions(+), 26 deletions(-) create mode 100644 db/migrate/20190731141037_change_environments_state_default.rb create mode 100644 db/post_migrate/20190801154748_update_state_for_environments_without_deployments.rb 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 -- cgit v1.2.1