Add latest changes from gitlab-org/gitlab@master

app/models/deployment.rb

@@ -10,6 +10,10 @@ class Deployment < ApplicationRecord
   belongs_to :cluster, class_name: 'Clusters::Cluster', optional: true
   belongs_to :user
   belongs_to :deployable, polymorphic: true, optional: true # rubocop:disable Cop/PolymorphicAssociations
+  has_many :deployment_merge_requests
+
+  has_many :merge_requests,
+    through: :deployment_merge_requests
   has_internal_id :iid, scope: :project, init: ->(s) do
     Deployment.where(project: s.project).maximum(:iid) if s&.project
@@ -149,6 +153,18 @@ class Deployment < ApplicationRecord
       project.deployments.joins(:environment)
       .where(environments: { name: self.environment.name }, ref: self.ref)
       .where.not(id: self.id)
+      .order(id: :desc)
+      .take
+  end
+
+  def previous_environment_deployment
+    project
+      .deployments
+      .success
+      .joins(:environment)
+      .where(environments: { name: environment.name })
+      .where.not(id: self.id)
+      .order(id: :desc)
       .take
   end
@@ -181,6 +197,18 @@ class Deployment < ApplicationRecord
     deployable&.user || user
   end
+  def link_merge_requests(relation)
+    select = relation.select(['merge_requests.id', id]).to_sql
+
+    # We don't use `Gitlab::Database.bulk_insert` here so that we don't need to
+    # first pluck lots of IDs into memory.
+    DeploymentMergeRequest.connection.execute(<<~SQL)
+      INSERT INTO #{DeploymentMergeRequest.table_name}
+      (merge_request_id, deployment_id)
+      #{select}
+    SQL
+  end
+
   private
   def ref_path

app/models/deployment_merge_request.rb

+# frozen_string_literal: true
+
+class DeploymentMergeRequest < ApplicationRecord
+  belongs_to :deployment, optional: false
+  belongs_to :merge_request, optional: false
+end

app/models/environment.rb

@@ -10,7 +10,11 @@ class Environment < ApplicationRecord
   has_many :successful_deployments, -> { success }, class_name: 'Deployment'
   has_one :last_deployment, -> { success.order('deployments.id DESC') }, class_name: 'Deployment'
+  has_one :last_deployable, through: :last_deployment, source: 'deployable', source_type: 'CommitStatus'
+  has_one :last_pipeline, through: :last_deployable, source: 'pipeline'
   has_one :last_visible_deployment, -> { visible.distinct_on_environment }, class_name: 'Deployment'
+  has_one :last_visible_deployable, through: :last_visible_deployment, source: 'deployable', source_type: 'CommitStatus'
+  has_one :last_visible_pipeline, through: :last_visible_deployable, source: 'pipeline'
   before_validation :nullify_external_url
   before_validation :generate_slug, if: ->(env) { env.slug.blank? }

app/models/merge_request.rb

@@ -205,6 +205,9 @@ class MergeRequest < ApplicationRecord
   scope :by_commit_sha, ->(sha) do
     where('EXISTS (?)', MergeRequestDiff.select(1).where('merge_requests.latest_merge_request_diff_id = merge_request_diffs.id').by_commit_sha(sha)).reorder(nil)
   end
+  scope :by_merge_commit_sha, -> (sha) do
+    where(merge_commit_sha: sha)
+  end
   scope :join_project, -> { joins(:target_project) }
   scope :references_project, -> { references(:target_project) }
   scope :with_api_entity_associations, -> {

app/services/deployments/after_create_service.rb

@@ -33,12 +33,21 @@ module Deployments
         if environment.save && !environment.stopped?
           deployment.update_merge_request_metrics!
+          link_merge_requests(deployment)
         end
       end
     end
     private
+    def link_merge_requests(deployment)
+      unless Feature.enabled?(:deployment_merge_requests, deployment.project)
+        return
+      end
+
+      LinkMergeRequestsService.new(deployment).execute
+    end
+
     def environment_options
       options&.dig(:environment) || {}
     end

app/services/deployments/link_merge_requests_service.rb

+# frozen_string_literal: true
+
+module Deployments
+  # Service class for linking merge requests to deployments.
+  class LinkMergeRequestsService
+    attr_reader :deployment
+
+    # The number of commits per query for which to find merge requests.
+    COMMITS_PER_QUERY = 5_000
+
+    def initialize(deployment)
+      @deployment = deployment
+    end
+
+    def execute
+      return unless deployment.success?
+
+      if (prev = deployment.previous_environment_deployment)
+        link_merge_requests_for_range(prev.sha, deployment.sha)
+      else
+        # When no previous deployment is found we fall back to linking all merge
+        # requests merged into the deployed branch. This will not always be
+        # accurate, but it's better than having no data.
+        #
+        # We can't use the first commit in the repository as a base to compare
+        # to, as this will not scale to large repositories. For example, GitLab
+        # itself has over 150 000 commits.
+        link_all_merged_merge_requests
+      end
+    end
+
+    def link_merge_requests_for_range(from, to)
+      commits = project
+        .repository
+        .commits_between(from, to)
+        .map(&:id)
+
+      # For some projects the list of commits to deploy may be very large. To
+      # ensure we do not end up running SQL queries with thousands of WHERE IN
+      # values, we run one query per a certain number of commits.
+      #
+      # In most cases this translates to only a single query. For very large
+      # deployment we may end up running a handful of queries to get and insert
+      # the data.
+      commits.each_slice(COMMITS_PER_QUERY) do |slice|
+        merge_requests =
+          project.merge_requests.merged.by_merge_commit_sha(slice)
+
+        deployment.link_merge_requests(merge_requests)
+      end
+    end
+
+    def link_all_merged_merge_requests
+      merge_requests =
+        project.merge_requests.merged.by_target_branch(deployment.ref)
+
+      deployment.link_merge_requests(merge_requests)
+    end
+
+    private
+
+    def project
+      deployment.project
+    end
+  end
+end

app/services/deployments/update_service.rb

@@ -10,7 +10,22 @@ module Deployments
     end
     def execute
-      deployment.update(status: params[:status])
+      # A regular update() does not trigger the state machine transitions, which
+      # we need to ensure merge requests are linked when changing the status to
+      # success. To work around this we use this case statment, using the right
+      # event methods to trigger the transition hooks.
+      case params[:status]
+      when 'running'
+        deployment.run
+      when 'success'
+        deployment.succeed
+      when 'failed'
+        deployment.drop
+      when 'canceled'
+        deployment.cancel
+      else
+        false
+      end
     end
   end
 end

changelogs/unreleased/deployment-commit-tracking.yml

+---
+title: Add deployment_merge_requests table
+merge_request: 18755
+author:
+type: other

changelogs/unreleased/jc-add-internal-socket-dir-to-setup.yml

+---
+title: Add internal_socket_dir to gitaly config in setup helper
+merge_request: 19170
+author:
+type: other

db/migrate/20191017134513_add_deployment_merge_requests.rb

+# frozen_string_literal: true
+
+class AddDeploymentMergeRequests < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :deployment_merge_requests, id: false do |t|
+      t.references(
+        :deployment,
+        foreign_key: { on_delete: :cascade },
+        type: :integer,
+        index: false,
+        null: false
+      )
+
+      t.references(
+        :merge_request,
+        foreign_key: { on_delete: :cascade },
+        type: :integer,
+        index: true,
+        null: false
+      )
+
+      t.index(
+        [:deployment_id, :merge_request_id],
+        unique: true,
+        name: 'idx_deployment_merge_requests_unique_index'
+      )
+    end
+  end
+end

db/schema.rb

@@ -1257,6 +1257,13 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do
     t.index ["token_encrypted"], name: "index_deploy_tokens_on_token_encrypted", unique: true
   end
+  create_table "deployment_merge_requests", id: false, force: :cascade do |t|
+    t.integer "deployment_id", null: false
+    t.integer "merge_request_id", null: false
+    t.index ["deployment_id", "merge_request_id"], name: "idx_deployment_merge_requests_unique_index", unique: true
+    t.index ["merge_request_id"], name: "index_deployment_merge_requests_on_merge_request_id"
+  end
+
   create_table "deployments", id: :serial, force: :cascade do |t|
     t.integer "iid", null: false
     t.integer "project_id", null: false
@@ -4191,6 +4198,8 @@ ActiveRecord::Schema.define(version: 2019_11_05_094625) do
   add_foreign_key "dependency_proxy_blobs", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "dependency_proxy_group_settings", "namespaces", column: "group_id", on_delete: :cascade
   add_foreign_key "deploy_keys_projects", "projects", name: "fk_58a901ca7e", on_delete: :cascade
+  add_foreign_key "deployment_merge_requests", "deployments", on_delete: :cascade
+  add_foreign_key "deployment_merge_requests", "merge_requests", on_delete: :cascade
   add_foreign_key "deployments", "clusters", name: "fk_289bba3222", on_delete: :nullify
   add_foreign_key "deployments", "projects", name: "fk_b9a3851b82", on_delete: :cascade
   add_foreign_key "description_versions", "epics", on_delete: :cascade

lib/gitlab/data_builder/deployment.rb

@@ -6,11 +6,17 @@ module Gitlab
       extend self
       def build(deployment)
+        # Deployments will not have a deployable when created using the API.
+        deployable_url =
+          if deployment.deployable
+            Gitlab::UrlBuilder.build(deployment.deployable)
+          end
+
         {
           object_kind: 'deployment',
           status: deployment.status,
           deployable_id: deployment.deployable_id,
-          deployable_url: Gitlab::UrlBuilder.build(deployment.deployable),
+          deployable_url: deployable_url,
           environment: deployment.environment.name,
           project: deployment.project.hook_attrs,
           short_sha: deployment.short_sha,

lib/gitlab/setup_helper.rb

@@ -40,6 +40,11 @@ module Gitlab
         config = { socket_path: address.sub(/\Aunix:/, ''), storage: storages }
         config[:auth] = { token: 'secret' } if Rails.env.test?
+
+        internal_socket_dir = File.join(gitaly_dir, 'internal_sockets')
+        FileUtils.mkdir(internal_socket_dir) unless File.exist?(internal_socket_dir)
+        config[:internal_socket_dir] = internal_socket_dir
+
         config[:'gitaly-ruby'] = { dir: File.join(gitaly_dir, 'ruby') } if gitaly_ruby
         config[:'gitlab-shell'] = { dir: Gitlab.config.gitlab_shell.path }
         config[:bin_dir] = Gitlab.config.gitaly.client_path

spec/factories/deployments.rb

@@ -51,6 +51,10 @@ FactoryBot.define do
       finished_at { Time.now }
     end
+    trait :created do
+      status { :created }
+    end
+
     # This trait hooks the state maechine's events
     trait :succeed do
       after(:create) do |deployment, evaluator|

spec/lib/gitlab/data_builder/deployment_spec.rb

@@ -35,5 +35,12 @@ describe Gitlab::DataBuilder::Deployment do
       expect(data[:commit_url]).to eq(expected_commit_url)
       expect(data[:commit_title]).to eq(commit.title)
     end
+
+    it 'does not include the deployable URL when there is no deployable' do
+      deployment = create(:deployment, status: :failed, deployable: nil)
+      data = described_class.build(deployment)
+
+      expect(data[:deployable_url]).to be_nil
+    end
   end
 end

spec/models/deployment_merge_request_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DeploymentMergeRequest do
+  let(:mr) { create(:merge_request, :merged) }
+  let(:deployment) { create(:deployment, :success, project: project) }
+  let(:project) { mr.project }
+
+  subject { described_class.new(deployment: deployment, merge_request: mr) }
+
+  it { is_expected.to belong_to(:deployment).required }
+  it { is_expected.to belong_to(:merge_request).required }
+end

spec/models/deployment_spec.rb

@@ -10,6 +10,8 @@ describe Deployment do
   it { is_expected.to belong_to(:cluster).class_name('Clusters::Cluster') }
   it { is_expected.to belong_to(:user) }
   it { is_expected.to belong_to(:deployable) }
+  it { is_expected.to have_many(:deployment_merge_requests) }
+  it { is_expected.to have_many(:merge_requests).through(:deployment_merge_requests) }
   it { is_expected.to delegate_method(:name).to(:environment).with_prefix }
   it { is_expected.to delegate_method(:commit).to(:project) }
@@ -361,4 +363,82 @@ describe Deployment do
         .to raise_error(ActiveRecord::RecordNotFound)
     end
   end
+
+  describe '#previous_deployment' do
+    it 'returns the previous deployment' do
+      deploy1 = create(:deployment)
+      deploy2 = create(
+        :deployment,
+        project: deploy1.project,
+        environment: deploy1.environment
+      )
+
+      expect(deploy2.previous_deployment).to eq(deploy1)
+    end
+  end
+
+  describe '#link_merge_requests' do
+    it 'links merge requests with a deployment' do
+      deploy = create(:deployment)
+      mr1 = create(
+        :merge_request,
+        :merged,
+        target_project: deploy.project,
+        source_project: deploy.project
+      )
+
+      mr2 = create(
+        :merge_request,
+        :merged,
+        target_project: deploy.project,
+        source_project: deploy.project
+      )
+
+      deploy.link_merge_requests(deploy.project.merge_requests)
+
+      expect(deploy.merge_requests).to include(mr1, mr2)
+    end
+  end
+
+  describe '#previous_environment_deployment' do
+    it 'returns the previous deployment of the same environment' do
+      deploy1 = create(:deployment, :success, ref: 'v1.0.0')
+      deploy2 = create(
+        :deployment,
+        :success,
+        project: deploy1.project,
+        environment: deploy1.environment,
+        ref: 'v1.0.1'
+      )
+
+      expect(deploy2.previous_environment_deployment).to eq(deploy1)
+    end
+
+    it 'ignores deployments that were not successful' do
+      deploy1 = create(:deployment, :failed, ref: 'v1.0.0')
+      deploy2 = create(
+        :deployment,
+        :success,
+        project: deploy1.project,
+        environment: deploy1.environment,
+        ref: 'v1.0.1'
+      )
+
+      expect(deploy2.previous_environment_deployment).to be_nil
+    end
+
+    it 'ignores deployments for different environments' do
+      deploy1 = create(:deployment, :success, ref: 'v1.0.0')
+      preprod = create(:environment, project: deploy1.project, name: 'preprod')
+      deploy2 = create(
+        :deployment,
+        :success,
+        project: deploy1.project,
+        environment: preprod,
+        ref: 'v1.0.1'
+      )
+
+      expect(deploy2.previous_environment_deployment).to be_nil
+    end
+  end
 end

spec/models/environment_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 describe Environment, :use_clean_rails_memory_store_caching do
   include ReactiveCachingHelpers
   using RSpec::Parameterized::TableSyntax
+  include RepoHelpers
   let(:project) { create(:project, :stubbed_repository) }
   subject(:environment) { create(:environment, project: project) }
@@ -505,6 +506,14 @@ describe Environment, :use_clean_rails_memory_store_caching do
         end
       end
+      context 'when there is a deployment record with failed status' do
+        let!(:deployment) { create(:deployment, :failed, environment: environment) }
+
+        it 'returns the previous deployment' do
+          is_expected.to eq(previous_deployment)
+        end
+      end
+
       context 'when there is a deployment record with success status' do
         let!(:deployment) { create(:deployment, :success, environment: environment) }
@@ -557,6 +566,89 @@ describe Environment, :use_clean_rails_memory_store_caching do
     end
   end
+  describe '#last_visible_pipeline' do
+    let(:user) { create(:user) }
+    let_it_be(:project) { create(:project, :repository) }
+    let(:environment) { create(:environment, project: project) }
+    let(:commit) { project.commit }
+
+    let(:success_pipeline) do
+      create(:ci_pipeline, :success, project: project, user: user, sha: commit.sha)
+    end
+
+    let(:failed_pipeline) do
+      create(:ci_pipeline, :failed, project: project, user: user, sha: commit.sha)
+    end
+
+    it 'uses the last deployment even if it failed' do
+      pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
+      ci_build = create(:ci_build, project: project, pipeline: pipeline)
+      create(:deployment, :failed, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
+
+      last_pipeline = environment.last_visible_pipeline
+
+      expect(last_pipeline).to eq(pipeline)
+    end
+
+    it 'returns nil if there is no deployment' do
+      create(:ci_build, project: project, pipeline: success_pipeline)
+
+      expect(environment.last_visible_pipeline).to be_nil
+    end
+
+    it 'does not return an invisible pipeline' do
+      failed_pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
+      ci_build_a = create(:ci_build, project: project, pipeline: failed_pipeline)
+      create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_a, sha: commit.sha)
+      pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
+      ci_build_b = create(:ci_build, project: project, pipeline: pipeline)
+      create(:deployment, :created, project: project, environment: environment, deployable: ci_build_b, sha: commit.sha)
+
+      last_pipeline = environment.last_visible_pipeline
+
+      expect(last_pipeline).to eq(failed_pipeline)
+    end
+
+    context 'for the environment' do
+      it 'returns the last pipeline' do
+        pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
+        ci_build = create(:ci_build, project: project, pipeline: pipeline)
+        create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
+
+        last_pipeline = environment.last_visible_pipeline
+
+        expect(last_pipeline).to eq(pipeline)
+      end
+
+      context 'with multiple deployments' do
+        it 'returns the last pipeline' do
+          pipeline_a = create(:ci_pipeline, project: project, user: user)
+          pipeline_b = create(:ci_pipeline, project: project, user: user)
+          ci_build_a = create(:ci_build, project: project, pipeline: pipeline_a)
+          ci_build_b = create(:ci_build, project: project, pipeline: pipeline_b)
+          create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a)
+          create(:deployment, :success, project: project, environment: environment, deployable: ci_build_b)
+
+          last_pipeline = environment.last_visible_pipeline
+
+          expect(last_pipeline).to eq(pipeline_b)
+        end
+      end
+
+      context 'with multiple pipelines' do
+        it 'returns the last pipeline' do
+          create(:ci_build, project: project, pipeline: success_pipeline)
+          ci_build_b = create(:ci_build, project: project, pipeline: failed_pipeline)
+          create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b, sha: commit.sha)
+
+          last_pipeline = environment.last_visible_pipeline
+
+          expect(last_pipeline).to eq(failed_pipeline)
+        end
+      end
+    end
+  end
+
   describe '#has_terminals?' do
     subject { environment.has_terminals? }

spec/models/merge_request_spec.rb

@@ -283,6 +283,16 @@ describe MergeRequest do
     end
   end
+  describe '.by_merge_commit_sha' do
+    it 'returns merge requests that match the given merge commit' do
+      mr = create(:merge_request, :merged, merge_commit_sha: '123abc')
+
+      create(:merge_request, :merged, merge_commit_sha: '123def')
+
+      expect(described_class.by_merge_commit_sha('123abc')).to eq([mr])
+    end
+  end
+
   describe '.in_projects' do
     it 'returns the merge requests for a set of projects' do
       expect(described_class.in_projects(Project.all)).to eq([subject])

spec/requests/api/deployments_spec.rb

@@ -137,14 +137,42 @@ describe API::Deployments do
         expect(response).to have_gitlab_http_status(500)
       end
+
+      it 'links any merged merge requests to the deployment' do
+        mr = create(
+          :merge_request,
+          :merged,
+          target_project: project,
+          source_project: project,
+          target_branch: 'master',
+          source_branch: 'foo'
+        )
+
+        post(
+          api("/projects/#{project.id}/deployments", user),
+          params: {
+            environment: 'production',
+            sha: sha,
+            ref: 'master',
+            tag: false,
+            status: 'success'
+          }
+        )
+
+        deploy = project.deployments.last
+
+        expect(deploy.merge_requests).to eq([mr])
+      end
     end
     context 'as a developer' do
-      it 'creates a new deployment' do
-        developer = create(:user)
+      let(:developer) { create(:user) }
+      before do
         project.add_developer(developer)
+      end
+      it 'creates a new deployment' do
         post(
           api("/projects/#{project.id}/deployments", developer),
           params: {
@@ -161,6 +189,32 @@ describe API::Deployments do
         expect(json_response['sha']).to eq(sha)
         expect(json_response['ref']).to eq('master')
       end
+
+      it 'links any merged merge requests to the deployment' do
+        mr = create(
+          :merge_request,
+          :merged,
+          target_project: project,
+          source_project: project,
+          target_branch: 'master',
+          source_branch: 'foo'
+        )
+
+        post(
+          api("/projects/#{project.id}/deployments", developer),
+          params: {
+            environment: 'production',
+            sha: sha,
+            ref: 'master',
+            tag: false,
+            status: 'success'
+          }
+        )
+
+        deploy = project.deployments.last
+
+        expect(deploy.merge_requests).to eq([mr])
+      end
     end
     context 'as non member' do
@@ -182,7 +236,7 @@ describe API::Deployments do
   end
   describe 'PUT /projects/:id/deployments/:deployment_id' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project, :repository) }
     let(:build) { create(:ci_build, :failed, project: project) }
     let(:environment) { create(:environment, project: project) }
     let(:deploy) do
@@ -191,7 +245,8 @@ describe API::Deployments do
         :failed,
         project: project,
         environment: environment,
-        deployable: nil
+        deployable: nil,
+        sha: project.commit.sha
       )
     end
@@ -216,6 +271,26 @@ describe API::Deployments do
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['status']).to eq('success')
       end
+
+      it 'links merge requests when the deployment status changes to success', :sidekiq_inline do
+        mr = create(
+          :merge_request,
+          :merged,
+          target_project: project,
+          source_project: project,
+          target_branch: 'master',
+          source_branch: 'foo'
+        )
+
+        put(
+          api("/projects/#{project.id}/deployments/#{deploy.id}", user),
+          params: { status: 'success' }
+        )
+
+        deploy = project.deployments.last
+
+        expect(deploy.merge_requests).to eq([mr])
+      end
     end
     context 'as a developer' do