Revert add action column changes

app/controllers/projects/deployments_controller.rb

@@ -6,7 +6,7 @@ class Projects::DeploymentsController < Projects::ApplicationController
   # rubocop: disable CodeReuse/ActiveRecord
   def index
-    deployments = environment.deployments.deployed.reorder(created_at: :desc)
+    deployments = environment.deployments.reorder(created_at: :desc)
     deployments = deployments.where('created_at > ?', params[:after].to_time) if params[:after]&.to_time
     render json: { deployments: DeploymentSerializer.new(project: project)
@@ -47,7 +47,7 @@ def additional_metrics
   # rubocop: disable CodeReuse/ActiveRecord
   def deployment
-    @deployment ||= environment.deployments.deployed.find_by(iid: params[:id])
+    @deployment ||= environment.deployments.find_by(iid: params[:id])
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/controllers/projects/environments_controller.rb

@@ -58,7 +58,7 @@ def folder
   # rubocop: disable CodeReuse/ActiveRecord
   def show
-    @deployments = environment.deployments.deployed.order(id: :desc).page(params[:page])
+    @deployments = environment.deployments.order(id: :desc).page(params[:page])
   end
   # rubocop: enable CodeReuse/ActiveRecord

app/finders/environments_finder.rb

@@ -9,7 +9,7 @@ def initialize(project, current_user, params = {})
   # rubocop: disable CodeReuse/ActiveRecord
   def execute
-    deployments = project.deployments.deployed
+    deployments = project.deployments.success
     deployments =
       if ref
         deployments_query = params[:with_tags] ? 'ref = :ref OR tag IS TRUE' : 'ref = :ref'

app/models/concerns/deployable.rb

@@ -7,7 +7,7 @@ module Deployable
     after_create :create_deployment
     def create_deployment
-      return unless has_environment? && !has_deployment?
+      return unless starts_environment? && !has_deployment?
       environment = project.environments.find_or_create_by(
         name: expanded_environment_name
@@ -21,8 +21,7 @@ def create_deployment
         sha: sha,
         user: user,
         deployable: self,
-        on_stop: on_stop,
-        action: environment_action).tap do |_|
+        on_stop: on_stop).tap do |_|
         self.reload # Reload relationships
       end
     end

app/models/deployment.rb

@@ -10,7 +10,9 @@ class Deployment < ActiveRecord::Base
   belongs_to :user
   belongs_to :deployable, polymorphic: true # rubocop:disable Cop/PolymorphicAssociations
-  has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.deployments&.maximum(:iid) }
+  has_internal_id :iid, scope: :project, init: ->(s) do
+    Deployment.where(project: s.project).maximum(:iid) if s&.project
+  end
   validates :sha, presence: true
   validates :ref, presence: true
@@ -18,8 +20,6 @@ class Deployment < ActiveRecord::Base
   delegate :name, to: :environment, prefix: true
   scope :for_environment, -> (environment) { where(environment_id: environment) }
-  scope :deployed, -> { success.start }
-  scope :stopped, -> { success.stop }
   state_machine :status, initial: :created do
     event :run do
@@ -57,11 +57,6 @@ class Deployment < ActiveRecord::Base
     canceled: 4
   }
-  enum action: {
-    start: 1,
-    stop: 2
-  }
-
   def self.last_for_environment(environment)
     ids = self
       .for_environment(environment)
@@ -137,7 +132,7 @@ def update_merge_request_metrics!
   def previous_deployment
     @previous_deployment ||=
-      project.deployments.deployed.joins(:environment)
+      project.deployments.success.joins(:environment)
       .where(environments: { name: self.environment.name }, ref: self.ref)
       .where.not(id: self.id)
       .take
@@ -182,14 +177,6 @@ def additional_metrics
     metrics&.merge(deployment_time: finished_at.to_i) || {}
   end
-  def deployed?
-    success? && start?
-  end
-
-  def stopped?
-    success? && stop?
-  end
-
   private
   def prometheus_adapter

app/models/environment.rb

@@ -8,9 +8,9 @@ class Environment < ActiveRecord::Base
   belongs_to :project, required: true
-  has_many :deployments, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-  has_one :last_deployment, -> { deployed.order('deployments.id DESC') }, class_name: 'Deployment'
+  has_many :deployments, -> { success }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  has_one :last_deployment, -> { success.order('deployments.id DESC') }, class_name: 'Deployment'
   before_validation :nullify_external_url
   before_validation :generate_slug, if: ->(env) { env.slug.blank? }
@@ -50,7 +50,7 @@ class Environment < ActiveRecord::Base
   scope :in_review_folder, -> { where(environment_type: "review") }
   scope :for_name, -> (name) { where(name: name) }
   scope :for_project, -> (project) { where(project_id: project) }
-  scope :with_deployment, -> (sha) { where('EXISTS (?)', Deployment.select(1).start.where('deployments.environment_id = environments.id').where(sha: sha)) }
+  scope :with_deployment, -> (sha) { where('EXISTS (?)', Deployment.select(1).where('deployments.environment_id = environments.id').where(sha: sha)) }
   state_machine :state, initial: :available do
     event :start do
@@ -103,6 +103,15 @@ def update_merge_request_metrics?
     folder_name == "production"
   end
+  def first_deployment_for(commit_sha)
+    ref = project.repository.ref_name_for_sha(ref_path, commit_sha)
+
+    return nil unless ref
+
+    deployment_iid = ref.split('/').last
+    deployments.find_by(iid: deployment_iid)
+  end
+
   def ref_path
     "refs/#{Repository::REF_ENVIRONMENTS}/#{slug}"
   end

app/models/environment_status.rb

@@ -28,7 +28,7 @@ def initialize(environment, merge_request, sha)
   def deployment
     strong_memoize(:deployment) do
-      environment.deployments.start.find_by_sha(sha)
+      Deployment.where(environment: environment).find_by_sha(sha)
     end
   end

app/models/project.rb

@@ -254,7 +254,7 @@ class Project < ActiveRecord::Base
   has_many :variables, class_name: 'Ci::Variable'
   has_many :triggers, class_name: 'Ci::Trigger'
   has_many :environments
-  has_many :deployments
+  has_many :deployments, -> { success }
   has_many :pipeline_schedules, class_name: 'Ci::PipelineSchedule'
   has_many :project_deploy_tokens
   has_many :deploy_tokens, through: :project_deploy_tokens

app/services/stop_environment_service.rb

-# frozen_string_literal: true
-
-class StopEnvironmentService
-  attr_reader :deployment
-
-  delegate :environment, to: :deployment
-
-  def initialize(deployment)
-    @deployment = deployment
-  end
-
-  def execute
-    return unless deployment.stopped?
-
-    environment.fire_state_event(:stop)
-    environment.expire_etag_cache
-  end
-end

app/services/start_environment_service.rb → app/services/update_deployment_service.rb

 # frozen_string_literal: true
-class StartEnvironmentService
+class UpdateDeploymentService
   attr_reader :deployment
   attr_reader :deployable
@@ -13,8 +13,6 @@ def initialize(deployment)
   end
   def execute
-    return unless deployment.deployed?
-
     deployment.create_ref
     deployment.invalidate_cache
@@ -22,9 +20,10 @@ def execute
       environment.external_url = expanded_environment_url if
         expanded_environment_url
-      environment.fire_state_event(:start)
+      environment.fire_state_event(action)
       break unless environment.save
+      break if environment.stopped?
       deployment.tap(&:update_merge_request_metrics!)
     end
@@ -47,4 +46,8 @@ def expanded_environment_url
   def environment_url
     environment_options[:url]
   end
+
+  def action
+    environment_options[:action] || 'start'
+  end
 end

app/workers/build_success_worker.rb

@@ -10,6 +10,7 @@ class BuildSuccessWorker
   def perform(build_id)
     Ci::Build.find_by(id: build_id).try do |build|
       create_deployment(build) if build.has_environment?
+      stop_environment(build) if build.stops_environment?
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord
@@ -26,4 +27,10 @@ def create_deployment(build)
       deployment.succeed
     end
   end
+
+  ##
+  # TODO: This should be processed in DeploymentSuccessWorker once we started storing `action` value in `deployments` records
+  def stop_environment(build)
+    build.persisted_environment.fire_state_event(:stop)
+  end
 end

app/workers/deployments/success_worker.rb

@@ -8,11 +8,9 @@ class SuccessWorker
     def perform(deployment_id)
       Deployment.find_by_id(deployment_id).try do |deployment|
-        if deployment.deployed?
-          StartEnvironmentService.new(deployment).execute
-        elsif deployment.stopped?
-          StopEnvironmentService.new(deployment).execute
-        end
+        break unless deployment.success?
+
+        UpdateDeploymentService.new(deployment).execute
       end
     end
   end

db/fixtures/development/14_pipelines.rb

@@ -43,14 +43,12 @@ class Gitlab::Seeder::Pipelines
     # deploy stage
     { name: 'staging', stage: 'deploy', environment: 'staging', status_event: :success,
-      options: { environment: { name: 'staging', action: 'start', on_stop: 'stop staging' } },
+      options: { environment: { action: 'start', on_stop: 'stop staging' } },
       queued_at: 7.hour.ago, started_at: 6.hour.ago, finished_at: 4.hour.ago },
     { name: 'stop staging', stage: 'deploy', environment: 'staging',
-      when: 'manual', status: :skipped,
-      options: { environment: { name: 'staging', action: 'stop' } } },
+      when: 'manual', status: :skipped },
     { name: 'production', stage: 'deploy', environment: 'production',
-      when: 'manual', status: :skipped,
-      options: { environment: { name: 'production' } } },
+      when: 'manual', status: :skipped },
     # notify stage
     { name: 'slack', stage: 'notify', when: 'manual', status: :success },

db/fixtures/development/19_environments.rb

@@ -45,18 +45,14 @@ def create_merge_request_review_deployments!
   end
   def create_deployment!(project, name, ref, sha)
-    find_deployable(project, name).try do |deployable|
-      environment = find_or_create_environment!(project, name)
-      environment.deployments.create!(
-        project: project,
-        ref: ref,
-        sha: sha,
-        tag: false,
-        deployable: deployable
-      ).tap do |deployment|
-        deployment.succeed!
-      end
-    end
+    environment = find_or_create_environment!(project, name)
+    environment.deployments.create!(
+      project: project,
+      ref: ref,
+      sha: sha,
+      tag: false,
+      deployable: find_deployable(project, name)
+    )
   end
   def find_or_create_environment!(project, name)

db/migrate/20181030150739_add_action_to_deployments.rb

-# frozen_string_literal: true
-
-class AddActionToDeployments < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DEPLOYMENT_ACTION_START = 1 # Equivalent to Deployment.actions['start']
-
-  DOWNTIME = false
-
-  disable_ddl_transaction!
-
-  def up
-    add_column_with_default(:deployments,
-      :action,
-      :integer,
-      limit: 2,
-      default: DEPLOYMENT_ACTION_START,
-      allow_null: false)
-  end
-
-  def down
-    remove_column(:deployments, :action)
-  end
-end

db/migrate/20181106135939_add_index_to_deployments.rb

@@ -4,27 +4,26 @@ class AddIndexToDeployments < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
   DOWNTIME = false
-  SHORT_INDEX_NAME_FOR_ENVIRONMENT_ID = 'index_deployments_on_env_deployed_for_id'
-  SHORT_INDEX_NAME_FOR_ENVIRONMENT_IID = 'index_deployments_on_env_deployed_for_iid'
-  SHORT_INDEX_NAME_FOR_PROJECT_FINISHED_AT = 'index_deployments_on_prj_deployed_for_finished_at'
   disable_ddl_transaction!
   def up
-    add_concurrent_index :deployments, [:project_id, :action, :status, :id]
-    add_concurrent_index :deployments, [:project_id, :action, :status, :iid]
-    add_concurrent_index :deployments, [:project_id, :action, :status, :finished_at], name: SHORT_INDEX_NAME_FOR_PROJECT_FINISHED_AT
-    add_concurrent_index :deployments, [:environment_id, :action, :status, :id], name: SHORT_INDEX_NAME_FOR_ENVIRONMENT_ID
-    add_concurrent_index :deployments, [:environment_id, :action, :status, :iid], name: SHORT_INDEX_NAME_FOR_ENVIRONMENT_IID
-    add_concurrent_index :deployments, [:environment_id, :action, :sha]
+    remove_concurrent_index :deployments, [:project_id, :status]
+    remove_concurrent_index :deployments, [:environment_id, :status]
+    add_concurrent_index :deployments, [:project_id, :status, :id]
+    add_concurrent_index :deployments, [:project_id, :status, :iid]
+    add_concurrent_index :deployments, [:environment_id, :status, :id]
+    add_concurrent_index :deployments, [:environment_id, :status, :iid]
+    add_concurrent_index :deployments, [:environment_id, :sha]
   end
   def down
-    remove_concurrent_index :deployments, [:project_id, :action, :status, :id]
-    remove_concurrent_index :deployments, [:project_id, :action, :status, :iid]
-    remove_concurrent_index_by_name(:deployments, SHORT_INDEX_NAME_FOR_PROJECT_FINISHED_AT)
-    remove_concurrent_index_by_name(:deployments, SHORT_INDEX_NAME_FOR_ENVIRONMENT_ID)
-    remove_concurrent_index_by_name(:deployments, SHORT_INDEX_NAME_FOR_ENVIRONMENT_IID)
-    remove_concurrent_index :deployments, [:environment_id, :action, :sha]
+    add_concurrent_index :deployments, [:project_id, :status]
+    add_concurrent_index :deployments, [:environment_id, :status]
+    remove_concurrent_index :deployments, [:project_id, :status, :id]
+    remove_concurrent_index :deployments, [:project_id, :status, :iid]
+    remove_concurrent_index :deployments, [:environment_id, :status, :id]
+    remove_concurrent_index :deployments, [:environment_id, :status, :iid]
+    remove_concurrent_index :deployments, [:environment_id, :sha]
   end
 end

db/schema.rb

@@ -827,23 +827,19 @@
     t.string "on_stop"
     t.integer "status", limit: 2, default: 2, null: false
     t.datetime_with_timezone "finished_at"
-    t.integer "action", limit: 2, default: 1, null: false
   end
   add_index "deployments", ["created_at"], name: "index_deployments_on_created_at", using: :btree
   add_index "deployments", ["deployable_type", "deployable_id"], name: "index_deployments_on_deployable_type_and_deployable_id", using: :btree
-  add_index "deployments", ["environment_id", "action", "sha"], name: "index_deployments_on_environment_id_and_action_and_sha", using: :btree
-  add_index "deployments", ["environment_id", "action", "status", "id"], name: "index_deployments_on_env_deployed_for_id", using: :btree
-  add_index "deployments", ["environment_id", "action", "status", "iid"], name: "index_deployments_on_env_deployed_for_iid", using: :btree
   add_index "deployments", ["environment_id", "id"], name: "index_deployments_on_environment_id_and_id", using: :btree
   add_index "deployments", ["environment_id", "iid", "project_id"], name: "index_deployments_on_environment_id_and_iid_and_project_id", using: :btree
-  add_index "deployments", ["environment_id", "status"], name: "index_deployments_on_environment_id_and_status", using: :btree
+  add_index "deployments", ["environment_id", "sha"], name: "index_deployments_on_environment_id_and_sha", using: :btree
+  add_index "deployments", ["environment_id", "status", "id"], name: "index_deployments_on_environment_id_and_status_and_id", using: :btree
+  add_index "deployments", ["environment_id", "status", "iid"], name: "index_deployments_on_environment_id_and_status_and_iid", using: :btree
   add_index "deployments", ["id"], name: "partial_index_deployments_for_legacy_successful_deployments", where: "((finished_at IS NULL) AND (status = 2))", using: :btree
-  add_index "deployments", ["project_id", "action", "status", "finished_at"], name: "index_deployments_on_prj_deployed_for_finished_at", using: :btree
-  add_index "deployments", ["project_id", "action", "status", "id"], name: "index_deployments_on_project_id_and_action_and_status_and_id", using: :btree
-  add_index "deployments", ["project_id", "action", "status", "iid"], name: "index_deployments_on_project_id_and_action_and_status_and_iid", using: :btree
   add_index "deployments", ["project_id", "iid"], name: "index_deployments_on_project_id_and_iid", unique: true, using: :btree
-  add_index "deployments", ["project_id", "status"], name: "index_deployments_on_project_id_and_status", using: :btree
+  add_index "deployments", ["project_id", "status", "id"], name: "index_deployments_on_project_id_and_status_and_id", using: :btree
+  add_index "deployments", ["project_id", "status", "iid"], name: "index_deployments_on_project_id_and_status_and_iid", using: :btree
   create_table "emails", force: :cascade do |t|
     t.integer "user_id", null: false
@@ -931,35 +927,6 @@
   add_index "forked_project_links", ["forked_to_project_id"], name: "index_forked_project_links_on_forked_to_project_id", unique: true, using: :btree
-  create_table "gcp_clusters", force: :cascade do |t|
-    t.integer "project_id", null: false
-    t.integer "user_id"
-    t.integer "service_id"
-    t.integer "status"
-    t.integer "gcp_cluster_size", null: false
-    t.datetime_with_timezone "created_at", null: false
-    t.datetime_with_timezone "updated_at", null: false
-    t.boolean "enabled", default: true
-    t.text "status_reason"
-    t.string "project_namespace"
-    t.string "endpoint"
-    t.text "ca_cert"
-    t.text "encrypted_kubernetes_token"
-    t.string "encrypted_kubernetes_token_iv"
-    t.string "username"
-    t.text "encrypted_password"
-    t.string "encrypted_password_iv"
-    t.string "gcp_project_id", null: false
-    t.string "gcp_cluster_zone", null: false
-    t.string "gcp_cluster_name", null: false
-    t.string "gcp_machine_type"
-    t.string "gcp_operation_id"
-    t.text "encrypted_gcp_token"
-    t.string "encrypted_gcp_token_iv"
-  end
-
-  add_index "gcp_clusters", ["project_id"], name: "index_gcp_clusters_on_project_id", unique: true, using: :btree
-
   create_table "gpg_key_subkeys", force: :cascade do |t|
     t.integer "gpg_key_id", null: false
     t.binary "keyid"
@@ -1860,6 +1827,7 @@
   end
   add_index "redirect_routes", ["path"], name: "index_redirect_routes_on_path", unique: true, using: :btree
+  add_index "redirect_routes", ["path"], name: "index_redirect_routes_on_path_text_pattern_ops", using: :btree, opclasses: {"path"=>"varchar_pattern_ops"}
   add_index "redirect_routes", ["source_type", "source_id"], name: "index_redirect_routes_on_source_type_and_source_id", using: :btree
   create_table "releases", force: :cascade do |t|
@@ -2443,9 +2411,6 @@
   add_foreign_key "fork_network_members", "projects", on_delete: :cascade
   add_foreign_key "fork_networks", "projects", column: "root_project_id", name: "fk_e7b436b2b5", on_delete: :nullify
   add_foreign_key "forked_project_links", "projects", column: "forked_to_project_id", name: "fk_434510edb0", on_delete: :cascade
-  add_foreign_key "gcp_clusters", "projects", on_delete: :cascade
-  add_foreign_key "gcp_clusters", "services", on_delete: :nullify
-  add_foreign_key "gcp_clusters", "users", on_delete: :nullify
   add_foreign_key "gpg_key_subkeys", "gpg_keys", on_delete: :cascade
   add_foreign_key "gpg_keys", "users", on_delete: :cascade
   add_foreign_key "gpg_signatures", "gpg_key_subkeys", on_delete: :nullify

lib/api/deployments.rb

@@ -24,7 +24,7 @@ class Deployments < Grape::API
       get ':id/deployments' do
         authorize! :read_deployment, user_project
-        present paginate(user_project.deployments.deployed.order(params[:order_by] => params[:sort])), with: Entities::Deployment
+        present paginate(user_project.deployments.order(params[:order_by] => params[:sort])), with: Entities::Deployment
       end
       # rubocop: enable CodeReuse/ActiveRecord
@@ -38,7 +38,7 @@ class Deployments < Grape::API
       get ':id/deployments/:deployment_id' do
         authorize! :read_deployment, user_project
-        deployment = user_project.deployments.deployed.find(params[:deployment_id])
+        deployment = user_project.deployments.find(params[:deployment_id])
         present deployment, with: Entities::Deployment
       end

lib/gitlab/cycle_analytics/summary/deploy.rb

@@ -7,7 +7,7 @@ def title
         end
         def value
-          @value ||= @project.deployments.deployed.where("finished_at > ?", @from).count
+          @value ||= @project.deployments.success.where("created_at > ?", @from).count
         end
       end
     end

spec/factories/deployments.rb

@@ -47,13 +47,5 @@
         deployment.succeed!
       end
     end
-
-    trait :start do
-      action :start
-    end
-
-    trait :stop do
-      action :stop
-    end
   end
 end