Merge branch 'stateful_deployments' into 'master'

Change life cycle of `deployments` records in order to make it a stateful object

See merge request gitlab-org/gitlab-ce!22380

app/models/ci/build.rb

@@ -9,19 +9,18 @@ class Build < CommitStatus
     include Presentable
     include Importable
     include Gitlab::Utils::StrongMemoize
+    include Deployable
     belongs_to :project, inverse_of: :builds
     belongs_to :runner
     belongs_to :trigger_request
     belongs_to :erased_by, class_name: 'User'
-    has_many :deployments, as: :deployable
-
     RUNNER_FEATURES = {
       upload_multiple_artifacts: -> (build) { build.publishes_artifacts_reports? }
     }.freeze
-    has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment'
+    has_one :deployment, as: :deployable, class_name: 'Deployment'
     has_many :trace_sections, class_name: 'Ci::BuildTraceSection'
     has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id
@@ -195,6 +194,8 @@ def retry(build, current_user)
       end
       after_transition pending: :running do |build|
+        build.deployment&.run
+
         build.run_after_commit do
           BuildHooksWorker.perform_async(id)
         end
@@ -207,14 +208,18 @@ def retry(build, current_user)
       end
       after_transition any => [:success] do |build|
+        build.deployment&.succeed
+
         build.run_after_commit do
-          BuildSuccessWorker.perform_async(id)
           PagesWorker.perform_async(:deploy, id) if build.pages_generator?
         end
       end
       before_transition any => [:failed] do |build|
         next unless build.project
+
+        build.deployment&.drop
+
         next if build.retries_max.zero?
         if build.retries_count < build.retries_max
@@ -233,6 +238,10 @@ def retry(build, current_user)
       after_transition running: any do |build|
         Ci::BuildRunnerSession.where(build: build).delete_all
       end
+
+      after_transition any => [:skipped, :canceled] do |build|
+        build.deployment&.cancel
+      end
     end
     def ensure_metadata
@@ -342,8 +351,12 @@ def environment_action
       self.options.fetch(:environment, {}).fetch(:action, 'start') if self.options
     end
+    def has_deployment?
+      !!self.deployment
+    end
+
     def outdated_deployment?
-      success? && !last_deployment.try(:last?)
+      success? && !deployment.try(:last?)
     end
     def depends_on_builds
@@ -358,6 +371,10 @@ def triggered_by?(current_user)
       user == current_user
     end
+    def on_stop
+      options&.dig(:environment, :on_stop)
+    end
+
     # A slugified version of the build ref, suitable for inclusion in URLs and
     # domain names. Rules:
     #
@@ -725,7 +742,7 @@ def deployment_status
       if success?
         return successful_deployment_status
-      elsif complete? && !success?
+      elsif failed?
         return :failed
       end
@@ -742,13 +759,11 @@ def erase_old_artifacts!
     end
     def successful_deployment_status
-      if success? && last_deployment&.last?
-        return :last
-      elsif success? && last_deployment.present?
-        return :out_of_date
+      if deployment&.last?
+        :last
+      else
+        :out_of_date
       end
-
-      :creating
     end
     def each_report(report_types)

app/models/concerns/deployable.rb

+# frozen_string_literal: true
+
+module Deployable
+  extend ActiveSupport::Concern
+
+  included do
+    after_create :create_deployment
+
+    def create_deployment
+      return unless starts_environment? && !has_deployment?
+
+      environment = project.environments.find_or_create_by(
+        name: expanded_environment_name
+      )
+
+      environment.deployments.create!(
+        project_id: environment.project_id,
+        environment: environment,
+        ref: ref,
+        tag: tag,
+        sha: sha,
+        user: user,
+        deployable: self,
+        on_stop: on_stop).tap do |_|
+        self.reload # Reload relationships
+      end
+    end
+  end
+end

app/models/deployment.rb

@@ -3,6 +3,7 @@
 class Deployment < ActiveRecord::Base
   include AtomicInternalId
   include IidRoutes
+  include AfterCommitQueue
   belongs_to :project, required: true
   belongs_to :environment, required: true
@@ -16,11 +17,44 @@ class Deployment < ActiveRecord::Base
   delegate :name, to: :environment, prefix: true
-  after_create :create_ref
-  after_create :invalidate_cache
-
   scope :for_environment, -> (environment) { where(environment_id: environment) }
+  state_machine :status, initial: :created do
+    event :run do
+      transition created: :running
+    end
+
+    event :succeed do
+      transition any - [:success] => :success
+    end
+
+    event :drop do
+      transition any - [:failed] => :failed
+    end
+
+    event :cancel do
+      transition any - [:canceled] => :canceled
+    end
+
+    before_transition any => [:success, :failed, :canceled] do |deployment|
+      deployment.finished_at = Time.now
+    end
+
+    after_transition any => :success do |deployment|
+      deployment.run_after_commit do
+        Deployments::SuccessWorker.perform_async(id)
+      end
+    end
+  end
+
+  enum status: {
+    created: 0,
+    running: 1,
+    success: 2,
+    failed: 3,
+    canceled: 4
+  }
+
   def self.last_for_environment(environment)
     ids = self
       .for_environment(environment)
@@ -69,15 +103,15 @@ def includes_commit?(commit)
   end
   def update_merge_request_metrics!
-    return unless environment.update_merge_request_metrics?
+    return unless environment.update_merge_request_metrics? && success?
     merge_requests = project.merge_requests
                      .joins(:metrics)
                      .where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil })
-                     .where("merge_request_metrics.merged_at <= ?", self.created_at)
+                     .where("merge_request_metrics.merged_at <= ?", finished_at)
     if previous_deployment
-      merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at)
+      merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.finished_at)
     end
     # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table
@@ -91,7 +125,7 @@ def update_merge_request_metrics!
     MergeRequest::Metrics
       .where(merge_request_id: merge_request_ids, first_deployed_to_production_at: nil)
-      .update_all(first_deployed_to_production_at: self.created_at)
+      .update_all(first_deployed_to_production_at: finished_at)
   end
   def previous_deployment
@@ -109,8 +143,18 @@ def stop_action
     @stop_action ||= manual_actions.find_by(name: on_stop)
   end
+  def finished_at
+    read_attribute(:finished_at) || legacy_finished_at
+  end
+
+  def deployed_at
+    return unless success?
+
+    finished_at
+  end
+
   def formatted_deployment_time
-    created_at.to_time.in_time_zone.to_s(:medium)
+    deployed_at&.to_time&.in_time_zone&.to_s(:medium)
   end
   def has_metrics?
@@ -118,21 +162,17 @@ def has_metrics?
   end
   def metrics
-    return {} unless has_metrics?
+    return {} unless has_metrics? && success?
     metrics = prometheus_adapter.query(:deployment, self)
-    metrics&.merge(deployment_time: created_at.to_i) || {}
+    metrics&.merge(deployment_time: finished_at.to_i) || {}
   end
   def additional_metrics
-    return {} unless has_metrics?
+    return {} unless has_metrics? && success?
     metrics = prometheus_adapter.query(:additional_metrics_deployment, self)
-    metrics&.merge(deployment_time: created_at.to_i) || {}
-  end
-
-  def status
-    'success'
+    metrics&.merge(deployment_time: finished_at.to_i) || {}
   end
   private
@@ -144,4 +184,8 @@ def prometheus_adapter
   def ref_path
     File.join(environment.ref_path, 'deployments', iid.to_s)
   end
+
+  def legacy_finished_at
+    self.created_at if success? && !read_attribute(:finished_at)
+  end
 end

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, -> { 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? }

app/models/environment_status.rb

@@ -8,8 +8,8 @@ class EnvironmentStatus
   delegate :id, to: :environment
   delegate :name, to: :environment
   delegate :project, to: :environment
+  delegate :status, to: :deployment, allow_nil: true
   delegate :deployed_at, to: :deployment, allow_nil: true
-  delegate :status, to: :deployment
   def self.for_merge_request(mr, user)
     build_environments_status(mr, user, mr.head_pipeline)
@@ -33,10 +33,6 @@ def deployment
     end
   end
-  def deployed_at
-    deployment&.created_at
-  end
-
   def changes
     return [] if project.route_map_for(sha).nil?

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/create_deployment_service.rb → app/services/update_deployment_service.rb

 # frozen_string_literal: true
-class CreateDeploymentService
-  attr_reader :job
-  delegate :expanded_environment_name,
-           :variables,
-           :project,
-           to: :job
-  def initialize(job)
-    @job = job
+class UpdateDeploymentService
+  attr_reader :deployment
+  attr_reader :deployable
+  delegate :environment, to: :deployment
+  delegate :variables, to: :deployable
+  def initialize(deployment)
+    @deployment = deployment
+    @deployable = deployment.deployable
   end
   def execute
-    return unless executable?
+    deployment.create_ref
+    deployment.invalidate_cache
     ActiveRecord::Base.transaction do
       environment.external_url = expanded_environment_url if
@@ -24,50 +25,28 @@ def execute
       break unless environment.save
       break if environment.stopped?
-      deploy.tap(&:update_merge_request_metrics!)
+      deployment.tap(&:update_merge_request_metrics!)
     end
   end
   private
-  def executable?
-    project && job.environment.present? && environment
-  end
-
-  def deploy
-    project.deployments.create(
-      environment: environment,
-      ref: job.ref,
-      tag: job.tag,
-      sha: job.sha,
-      user: job.user,
-      deployable: job,
-      on_stop: on_stop)
-  end
-
-  def environment
-    @environment ||= job.persisted_environment
-  end
-
   def environment_options
-    @environment_options ||= job.options&.dig(:environment) || {}
+    @environment_options ||= deployable.options&.dig(:environment) || {}
   end
   def expanded_environment_url
     return @expanded_environment_url if defined?(@expanded_environment_url)
+    return unless environment_url
     @expanded_environment_url =
-      ExpandVariables.expand(environment_url, variables) if environment_url
+      ExpandVariables.expand(environment_url, variables)
   end
   def environment_url
     environment_options[:url]
   end
-  def on_stop
-    environment_options[:on_stop]
-  end
-
   def action
     environment_options[:action] || 'start'
   end

app/workers/all_queues.yml

@@ -73,6 +73,8 @@
 - pipeline_processing:update_head_pipeline_for_merge_request
 - pipeline_processing:ci_build_schedule
+- deployment:deployments_success
+
 - repository_check:repository_check_clear
 - repository_check:repository_check_batch
 - repository_check:repository_check_single_repository

app/workers/build_success_worker.rb

@@ -10,13 +10,27 @@ 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
   private
+  ##
+  # Deprecated:
+  # As of 11.5, we started creating a deployment record when ci_builds record is created.
+  # Therefore we no longer need to create a deployment, after a build succeeded.
+  # We're leaving this code for the transition period, but we can remove this code in 11.6.
   def create_deployment(build)
-    CreateDeploymentService.new(build).execute
+    build.create_deployment.try do |deployment|
+      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

+# frozen_string_literal: true
+
+module Deployments
+  class SuccessWorker
+    include ApplicationWorker
+
+    queue_namespace :deployment
+
+    def perform(deployment_id)
+      Deployment.find_by_id(deployment_id).try do |deployment|
+        break unless deployment.success?
+
+        UpdateDeploymentService.new(deployment).execute
+      end
+    end
+  end
+end

changelogs/unreleased/stateful_deployments.yml

+---
+title: Add status to Deployment
+merge_request: 22380
+author:
+type: changed

config/sidekiq_queues.yml

@@ -29,6 +29,7 @@
   - [pipeline_creation, 4]
   - [pipeline_default, 3]
   - [pipeline_cache, 3]
+  - [deployment, 3]
   - [pipeline_hooks, 2]
   - [gitlab_shell, 2]
   - [email_receiver, 2]

db/fixtures/development/17_cycle_analytics.rb

@@ -180,11 +180,8 @@ def run_builds(merge_requests)
                                               ref: "refs/heads/#{merge_request.source_branch}")
       pipeline = service.execute(:push, ignore_skip_ci: true, save_on_errors: false)
-      pipeline.run!
-      Timecop.travel rand(1..6).hours.from_now
-      pipeline.succeed!
-
-      PipelineMetricsWorker.new.perform(pipeline.id)
+      pipeline.builds.map(&:run!)
+      pipeline.update_status
     end
   end
@@ -204,7 +201,8 @@ def deploy_to_production(merge_requests)
       job = merge_request.head_pipeline.builds.where.not(environment: nil).last
-      CreateDeploymentService.new(job).execute
+      job.success!
+      pipeline.update_status
     end
   end
 end

db/migrate/20181015155839_add_finished_at_to_deployments.rb

+# frozen_string_literal: true
+
+class AddFinishedAtToDeployments < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    add_column :deployments, :finished_at, :datetime_with_timezone
+  end
+
+  def down
+    remove_column :deployments, :finished_at, :datetime_with_timezone
+  end
+end

db/migrate/20181016141739_add_status_to_deployments.rb

+# frozen_string_literal: true
+
+class AddStatusToDeployments < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DEPLOYMENT_STATUS_SUCCESS = 2 # Equivalent to Deployment.state_machine.states['success'].value
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  ##
+  # NOTE:
+  # Ideally, `status` column should not have default value because it should be leveraged by state machine (i.e. application level).
+  # However, we have to use the default value for avoiding `NOT NULL` violation during the transition period.
+  # The default value should be removed in the future release.
+  def up
+    add_column_with_default(:deployments,
+      :status,
+      :integer,
+      limit: 2,
+      default: DEPLOYMENT_STATUS_SUCCESS,
+      allow_null: false)
+  end
+
+  def down
+    remove_column(:deployments, :status)
+  end
+end

db/migrate/20181022135539_add_index_on_status_to_deployments.rb

+# frozen_string_literal: true
+
+class AddIndexOnStatusToDeployments < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :deployments, [:project_id, :status]
+    add_concurrent_index :deployments, [:environment_id, :status]
+  end
+
+  def down
+    remove_concurrent_index :deployments, [:project_id, :status]
+    remove_concurrent_index :deployments, [:environment_id, :status]
+  end
+end

db/migrate/20181023144439_add_partial_index_for_legacy_successful_deployments.rb

+# frozen_string_literal: true
+
+class AddPartialIndexForLegacySuccessfulDeployments < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'partial_index_deployments_for_legacy_successful_deployments'.freeze
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:deployments, :id, where: "finished_at IS NULL AND status = 2", name: INDEX_NAME)
+  end
+
+  def down
+    remove_concurrent_index_by_name(:deployments, INDEX_NAME)
+  end
+end

db/post_migrate/20181030135124_fill_empty_finished_at_in_deployments.rb

+# frozen_string_literal: true
+
+class FillEmptyFinishedAtInDeployments < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  DEPLOYMENT_STATUS_SUCCESS = 2 # Equivalent to Deployment.statuses[:success]
+
+  class Deployments < ActiveRecord::Base
+    self.table_name = 'deployments'
+
+    include EachBatch
+  end
+
+  def up
+    FillEmptyFinishedAtInDeployments::Deployments
+      .where('finished_at IS NULL')
+      .where('status = ?', DEPLOYMENT_STATUS_SUCCESS)
+      .each_batch(of: 10_000) do |relation|
+      relation.update_all('finished_at=created_at')
+    end
+  end
+
+  def down
+    # no-op
+  end
+end

db/schema.rb

@@ -825,13 +825,18 @@
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string "on_stop"
+    t.integer "status", limit: 2, default: 2, null: false
+    t.datetime_with_timezone "finished_at"
   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", "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", ["id"], name: "partial_index_deployments_for_legacy_successful_deployments", where: "((finished_at IS NULL) AND (status = 2))", 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
   create_table "emails", force: :cascade do |t|
     t.integer "user_id", null: false

lib/gitlab/ci/pipeline/chain/create.rb

@@ -7,26 +7,11 @@ module Chain
         class Create < Chain::Base
           include Chain::Helpers
-          # rubocop: disable CodeReuse/ActiveRecord
           def perform!
-            ::Ci::Pipeline.transaction do
-              pipeline.save!
-
-              ##
-              # Create environments before the pipeline starts.
-              #
-              pipeline.builds.each do |build|
-                if build.has_environment?
-                  project.environments.find_or_create_by(
-                    name: build.expanded_environment_name
-                  )
-                end
-              end
-            end
+            pipeline.save!
           rescue ActiveRecord::RecordInvalid => e
             error("Failed to persist the pipeline: #{e}")
           end
-          # rubocop: enable CodeReuse/ActiveRecord
           def break?
             !pipeline.persisted?