summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2017-08-08 01:47:48 +0000
committerStan Hu <stanhu@gmail.com>2017-08-08 01:47:48 +0000
commitfd40bce9ccad94c3f8fe522a51a771335d95539f (patch)
tree0fb76c147480326f17fb77271628c24794255572
parent4490af8cf22416f12c8cf2e00f8b7c293be08167 (diff)
parenta0c22d1edafbd87d59dbf01acd610da97ec87912 (diff)
downloadgitlab-ce-fd40bce9ccad94c3f8fe522a51a771335d95539f.tar.gz
Merge branch '31207-clean-locked-merge-requests' into 'master'
Resolve "Store MergeWorker JID on merge request, and clean up stuck merges" Closes #31207 See merge request !13207
-rw-r--r--.flayignore1
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merging.js (renamed from app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_locked.js)4
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/dependencies.js2
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js4
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js9
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js4
-rw-r--r--app/controllers/projects/merge_requests_controller.rb5
-rw-r--r--app/models/merge_request.rb23
-rw-r--r--app/serializers/merge_request_entity.rb2
-rw-r--r--app/workers/merge_worker.rb2
-rw-r--r--app/workers/stuck_merge_jobs_worker.rb34
-rw-r--r--changelogs/unreleased/31207-clean-locked-merge-requests.yml4
-rw-r--r--config/initializers/1_settings.rb4
-rw-r--r--db/migrate/20170731183033_add_merge_jid_to_merge_requests.rb7
-rw-r--r--db/post_migrate/20170807160457_remove_locked_at_column_from_merge_requests.rb11
-rw-r--r--db/schema.rb4
-rw-r--r--doc/user/project/integrations/webhooks.md1
-rw-r--r--lib/gitlab/import_export/import_export.yml1
-rw-r--r--spec/features/merge_requests/widget_spec.rb13
-rw-r--r--spec/fixtures/api/schemas/entities/merge_request.json4
-rw-r--r--spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js10
-rw-r--r--spec/javascripts/vue_mr_widget/mock_data.js1
-rw-r--r--spec/javascripts/vue_mr_widget/mr_widget_options_spec.js2
-rw-r--r--spec/lib/gitlab/import_export/project.json9
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/models/merge_request_spec.rb26
-rw-r--r--spec/serializers/merge_request_entity_spec.rb2
-rw-r--r--spec/workers/merge_worker_spec.rb11
-rw-r--r--spec/workers/stuck_merge_jobs_worker_spec.rb50
29 files changed, 196 insertions, 55 deletions
diff --git a/.flayignore b/.flayignore
index e2d0a2e50c5..b63ce4c4df0 100644
--- a/.flayignore
+++ b/.flayignore
@@ -3,4 +3,5 @@ lib/gitlab/sanitizers/svg/whitelist.rb
lib/gitlab/diff/position_tracer.rb
app/policies/project_policy.rb
app/models/concerns/relative_positioning.rb
+app/workers/stuck_merge_jobs_worker.rb
lib/gitlab/redis/*.rb
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_locked.js b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merging.js
index a12f418e1af..f6d1a4feeb2 100644
--- a/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_locked.js
+++ b/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_merging.js
@@ -1,7 +1,7 @@
import statusIcon from '../mr_widget_status_icon';
export default {
- name: 'MRWidgetLocked',
+ name: 'MRWidgetMerging',
props: {
mr: { type: Object, required: true },
},
@@ -13,7 +13,7 @@ export default {
<status-icon status="loading" />
<div class="media-body">
<h4>
- This merge request is in the process of being merged, during which time it is locked and cannot be closed
+ This merge request is in the process of being merged
</h4>
<section class="mr-info-list">
<p>
diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
index 546a3f625c7..49340c232c8 100644
--- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js
+++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
@@ -19,7 +19,7 @@ export { default as WidgetRelatedLinks } from './components/mr_widget_related_li
export { default as MergedState } from './components/states/mr_widget_merged';
export { default as FailedToMerge } from './components/states/mr_widget_failed_to_merge';
export { default as ClosedState } from './components/states/mr_widget_closed';
-export { default as LockedState } from './components/states/mr_widget_locked';
+export { default as MergingState } from './components/states/mr_widget_merging';
export { default as WipState } from './components/states/mr_widget_wip';
export { default as ArchivedState } from './components/states/mr_widget_archived';
export { default as ConflictsState } from './components/states/mr_widget_conflicts';
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
index 577d77f09a6..0042c48816f 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
@@ -8,7 +8,7 @@ import {
WidgetRelatedLinks,
MergedState,
ClosedState,
- LockedState,
+ MergingState,
WipState,
ArchivedState,
ConflictsState,
@@ -212,7 +212,7 @@ export default {
'mr-widget-related-links': WidgetRelatedLinks,
'mr-widget-merged': MergedState,
'mr-widget-closed': ClosedState,
- 'mr-widget-locked': LockedState,
+ 'mr-widget-merging': MergingState,
'mr-widget-failed-to-merge': FailedToMerge,
'mr-widget-wip': WipState,
'mr-widget-archived': ArchivedState,
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index fddafb0ddfa..fbea764b739 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -73,6 +73,7 @@ export default class MergeRequestStore {
this.canCancelAutomaticMerge = !!data.cancel_merge_when_pipeline_succeeds_path;
this.hasSHAChanged = this.sha !== data.diff_head_sha;
this.canBeMerged = data.can_be_merged || false;
+ this.mergeOngoing = data.merge_ongoing;
// Cherry-pick and Revert actions related
this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;
@@ -94,6 +95,11 @@ export default class MergeRequestStore {
}
setState(data) {
+ if (this.mergeOngoing) {
+ this.state = 'merging';
+ return;
+ }
+
if (this.isOpen) {
this.state = getStateKey.call(this, data);
} else {
@@ -104,9 +110,6 @@ export default class MergeRequestStore {
case 'closed':
this.state = 'closed';
break;
- case 'locked':
- this.state = 'locked';
- break;
default:
this.state = null;
}
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
index 605dd3a1ff4..9074a064a6d 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
@@ -1,7 +1,7 @@
const stateToComponentMap = {
merged: 'mr-widget-merged',
closed: 'mr-widget-closed',
- locked: 'mr-widget-locked',
+ merging: 'mr-widget-merging',
conflicts: 'mr-widget-conflicts',
missingBranch: 'mr-widget-missing-branch',
workInProgress: 'mr-widget-wip',
@@ -20,7 +20,7 @@ const stateToComponentMap = {
};
const statesToShowHelpWidget = [
- 'locked',
+ 'merging',
'conflicts',
'workInProgress',
'readyToMerge',
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index d361e661d0e..4de814d0ca8 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -67,11 +67,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@noteable = @merge_request
@commits_count = @merge_request.commits_count
- if @merge_request.locked_long_ago?
- @merge_request.unlock_mr
- @merge_request.close
- end
-
labels
set_pipeline_variables
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 8ca850b6d96..e83b11f7668 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -8,6 +8,7 @@ class MergeRequest < ActiveRecord::Base
include CreatedAtFilterable
ignore_column :position
+ ignore_column :locked_at
belongs_to :target_project, class_name: "Project"
belongs_to :source_project, class_name: "Project"
@@ -61,16 +62,6 @@ class MergeRequest < ActiveRecord::Base
transition locked: :opened
end
- after_transition any => :locked do |merge_request, transition|
- merge_request.locked_at = Time.now
- merge_request.save
- end
-
- after_transition locked: (any - :locked) do |merge_request, transition|
- merge_request.locked_at = nil
- merge_request.save
- end
-
state :opened
state :closed
state :merged
@@ -392,6 +383,12 @@ class MergeRequest < ActiveRecord::Base
'Source project is not a fork of the target project'
end
+ def merge_ongoing?
+ return false unless merge_jid
+
+ Gitlab::SidekiqStatus.num_running([merge_jid]) > 0
+ end
+
def closed_without_fork?
closed? && source_project_missing?
end
@@ -725,12 +722,6 @@ class MergeRequest < ActiveRecord::Base
end
end
- def locked_long_ago?
- return false unless locked?
-
- locked_at.nil? || locked_at < (Time.now - 1.day)
- end
-
def has_ci?
has_ci_integration = source_project.try(:ci_service)
uses_gitlab_ci = all_pipelines.any?
diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb
index 7f17f2bf604..07650ce6f20 100644
--- a/app/serializers/merge_request_entity.rb
+++ b/app/serializers/merge_request_entity.rb
@@ -2,7 +2,6 @@ class MergeRequestEntity < IssuableEntity
include RequestAwareEntity
expose :in_progress_merge_commit_sha
- expose :locked_at
expose :merge_commit_sha
expose :merge_error
expose :merge_params
@@ -32,6 +31,7 @@ class MergeRequestEntity < IssuableEntity
expose :head_pipeline, with: PipelineDetailsEntity, as: :pipeline
# Booleans
+ expose :merge_ongoing?, as: :merge_ongoing
expose :work_in_progress?, as: :work_in_progress
expose :source_branch_exists?, as: :source_branch_exists
expose :mergeable_discussions_state?, as: :mergeable_discussions_state
diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb
index 48e2da338f6..c3b58df92c1 100644
--- a/app/workers/merge_worker.rb
+++ b/app/workers/merge_worker.rb
@@ -7,6 +7,8 @@ class MergeWorker
current_user = User.find(current_user_id)
merge_request = MergeRequest.find(merge_request_id)
+ merge_request.update_column(:merge_jid, jid)
+
MergeRequests::MergeService.new(merge_request.target_project, current_user, params)
.execute(merge_request)
end
diff --git a/app/workers/stuck_merge_jobs_worker.rb b/app/workers/stuck_merge_jobs_worker.rb
new file mode 100644
index 00000000000..7843179d77c
--- /dev/null
+++ b/app/workers/stuck_merge_jobs_worker.rb
@@ -0,0 +1,34 @@
+class StuckMergeJobsWorker
+ include Sidekiq::Worker
+ include CronjobQueue
+
+ def perform
+ stuck_merge_requests.find_in_batches(batch_size: 100) do |group|
+ jids = group.map(&:merge_jid)
+
+ # Find the jobs that aren't currently running or that exceeded the threshold.
+ completed_jids = Gitlab::SidekiqStatus.completed_jids(jids)
+
+ if completed_jids.any?
+ completed_ids = group.select { |merge_request| completed_jids.include?(merge_request.merge_jid) }.map(&:id)
+
+ apply_current_state!(completed_jids, completed_ids)
+ end
+ end
+ end
+
+ private
+
+ def apply_current_state!(completed_jids, completed_ids)
+ merge_requests = MergeRequest.where(id: completed_ids)
+
+ merge_requests.where.not(merge_commit_sha: nil).update_all(state: :merged)
+ merge_requests.where(merge_commit_sha: nil).update_all(state: :opened)
+
+ Rails.logger.info("Updated state of locked merge jobs. JIDs: #{completed_jids.join(', ')}")
+ end
+
+ def stuck_merge_requests
+ MergeRequest.select('id, merge_jid').with_state(:locked).where.not(merge_jid: nil).reorder(nil)
+ end
+end
diff --git a/changelogs/unreleased/31207-clean-locked-merge-requests.yml b/changelogs/unreleased/31207-clean-locked-merge-requests.yml
new file mode 100644
index 00000000000..1f52987baef
--- /dev/null
+++ b/changelogs/unreleased/31207-clean-locked-merge-requests.yml
@@ -0,0 +1,4 @@
+---
+title: Unlock stuck merge request and set the proper state
+merge_request: 13207
+author:
diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb
index e24cf33adb5..2699173fc61 100644
--- a/config/initializers/1_settings.rb
+++ b/config/initializers/1_settings.rb
@@ -395,6 +395,10 @@ Settings.cron_jobs['remove_old_web_hook_logs_worker'] ||= Settingslogic.new({})
Settings.cron_jobs['remove_old_web_hook_logs_worker']['cron'] ||= '40 0 * * *'
Settings.cron_jobs['remove_old_web_hook_logs_worker']['job_class'] = 'RemoveOldWebHookLogsWorker'
+Settings.cron_jobs['stuck_merge_jobs_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['stuck_merge_jobs_worker']['cron'] ||= '0 */2 * * *'
+Settings.cron_jobs['stuck_merge_jobs_worker']['job_class'] = 'StuckMergeJobsWorker'
+
#
# GitLab Shell
#
diff --git a/db/migrate/20170731183033_add_merge_jid_to_merge_requests.rb b/db/migrate/20170731183033_add_merge_jid_to_merge_requests.rb
new file mode 100644
index 00000000000..a7d8f2f3604
--- /dev/null
+++ b/db/migrate/20170731183033_add_merge_jid_to_merge_requests.rb
@@ -0,0 +1,7 @@
+class AddMergeJidToMergeRequests < ActiveRecord::Migration
+ DOWNTIME = false
+
+ def change
+ add_column :merge_requests, :merge_jid, :string
+ end
+end
diff --git a/db/post_migrate/20170807160457_remove_locked_at_column_from_merge_requests.rb b/db/post_migrate/20170807160457_remove_locked_at_column_from_merge_requests.rb
new file mode 100644
index 00000000000..ea3d1fb3e02
--- /dev/null
+++ b/db/post_migrate/20170807160457_remove_locked_at_column_from_merge_requests.rb
@@ -0,0 +1,11 @@
+class RemoveLockedAtColumnFromMergeRequests < ActiveRecord::Migration
+ DOWNTIME = false
+
+ def up
+ remove_column :merge_requests, :locked_at
+ end
+
+ def down
+ add_column :merge_requests, :locked_at, :datetime_with_timezone
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index dc5640ad2ca..ed3cf70bcdd 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20170803130232) do
+ActiveRecord::Schema.define(version: 20170807160457) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -850,7 +850,6 @@ ActiveRecord::Schema.define(version: 20170803130232) do
t.integer "target_project_id", null: false
t.integer "iid"
t.text "description"
- t.datetime "locked_at"
t.integer "updated_by_id"
t.text "merge_error"
t.text "merge_params"
@@ -868,6 +867,7 @@ ActiveRecord::Schema.define(version: 20170803130232) do
t.integer "last_edited_by_id"
t.integer "head_pipeline_id"
t.boolean "ref_fetched"
+ t.string "merge_jid"
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md
index c03a2df9a72..47eb0b34f66 100644
--- a/doc/user/project/integrations/webhooks.md
+++ b/doc/user/project/integrations/webhooks.md
@@ -438,7 +438,6 @@ X-Gitlab-Event: Note Hook
"iid": 1,
"description": "Et voluptas corrupti assumenda temporibus. Architecto cum animi eveniet amet asperiores. Vitae numquam voluptate est natus sit et ad id.",
"position": 0,
- "locked_at": null,
"source":{
"name":"Gitlab Test",
"description":"Aut reprehenderit ut est.",
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index c8ad3a7a5e0..c5c05bfe2fb 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -101,6 +101,7 @@ excluded_attributes:
merge_requests:
- :milestone_id
- :ref_fetched
+ - :merge_jid
award_emoji:
- :awardable_id
statuses:
diff --git a/spec/features/merge_requests/widget_spec.rb b/spec/features/merge_requests/widget_spec.rb
index 69e31c7481f..fd991293ee9 100644
--- a/spec/features/merge_requests/widget_spec.rb
+++ b/spec/features/merge_requests/widget_spec.rb
@@ -219,4 +219,17 @@ describe 'Merge request', :js do
expect(page).to have_field('remove-source-branch-input', disabled: true)
end
end
+
+ context 'ongoing merge process' do
+ it 'shows Merging state' do
+ allow_any_instance_of(MergeRequest).to receive(:merge_ongoing?).and_return(true)
+
+ visit project_merge_request_path(project, merge_request)
+
+ wait_for_requests
+
+ expect(page).not_to have_button('Merge')
+ expect(page).to have_content('This merge request is in the process of being merged')
+ end
+ end
end
diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request.json
index 7ffa82fc4bd..2f12b671dec 100644
--- a/spec/fixtures/api/schemas/entities/merge_request.json
+++ b/spec/fixtures/api/schemas/entities/merge_request.json
@@ -19,7 +19,6 @@
"human_time_estimate": { "type": ["integer", "null"] },
"human_total_time_spent": { "type": ["integer", "null"] },
"in_progress_merge_commit_sha": { "type": ["string", "null"] },
- "locked_at": { "type": ["string", "null"] },
"merge_error": { "type": ["string", "null"] },
"merge_commit_sha": { "type": ["string", "null"] },
"merge_params": { "type": ["object", "null"] },
@@ -94,7 +93,8 @@
"commit_change_content_path": { "type": "string" },
"remove_wip_path": { "type": "string" },
"commits_count": { "type": "integer" },
- "remove_source_branch": { "type": ["boolean", "null"] }
+ "remove_source_branch": { "type": ["boolean", "null"] },
+ "merge_ongoing": { "type": "boolean" }
},
"additionalProperties": false
}
diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js
index fb2ef606604..237035648cf 100644
--- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js
+++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_locked_spec.js
@@ -1,10 +1,10 @@
import Vue from 'vue';
-import lockedComponent from '~/vue_merge_request_widget/components/states/mr_widget_locked';
+import mergingComponent from '~/vue_merge_request_widget/components/states/mr_widget_merging';
-describe('MRWidgetLocked', () => {
+describe('MRWidgetMerging', () => {
describe('props', () => {
it('should have props', () => {
- const { mr } = lockedComponent.props;
+ const { mr } = mergingComponent.props;
expect(mr.type instanceof Object).toBeTruthy();
expect(mr.required).toBeTruthy();
@@ -13,7 +13,7 @@ describe('MRWidgetLocked', () => {
describe('template', () => {
it('should have correct elements', () => {
- const Component = Vue.extend(lockedComponent);
+ const Component = Vue.extend(mergingComponent);
const mr = {
targetBranchPath: '/branch-path',
targetBranch: 'branch',
@@ -24,7 +24,7 @@ describe('MRWidgetLocked', () => {
}).$el;
expect(el.classList.contains('mr-widget-body')).toBeTruthy();
- expect(el.innerText).toContain('it is locked');
+ expect(el.innerText).toContain('This merge request is in the process of being merged');
expect(el.innerText).toContain('changes will be merged into');
expect(el.querySelector('.label-branch a').getAttribute('href')).toEqual(mr.targetBranchPath);
expect(el.querySelector('.label-branch a').textContent).toContain(mr.targetBranch);
diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js
index ad2f28b24f0..0795d0aaa82 100644
--- a/spec/javascripts/vue_mr_widget/mock_data.js
+++ b/spec/javascripts/vue_mr_widget/mock_data.js
@@ -20,7 +20,6 @@ export default {
"human_time_estimate": null,
"human_total_time_spent": null,
"in_progress_merge_commit_sha": null,
- "locked_at": null,
"merge_commit_sha": "53027d060246c8f47e4a9310fb332aa52f221775",
"merge_error": null,
"merge_params": {
diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
index 3a0c50b750f..669ee248bf1 100644
--- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
+++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
@@ -342,7 +342,7 @@ describe('mrWidgetOptions', () => {
expect(comps['mr-widget-related-links']).toBeDefined();
expect(comps['mr-widget-merged']).toBeDefined();
expect(comps['mr-widget-closed']).toBeDefined();
- expect(comps['mr-widget-locked']).toBeDefined();
+ expect(comps['mr-widget-merging']).toBeDefined();
expect(comps['mr-widget-failed-to-merge']).toBeDefined();
expect(comps['mr-widget-wip']).toBeDefined();
expect(comps['mr-widget-archived']).toBeDefined();
diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json
index 469a014e4d2..4e631e13410 100644
--- a/spec/lib/gitlab/import_export/project.json
+++ b/spec/lib/gitlab/import_export/project.json
@@ -2534,7 +2534,6 @@
"iid": 9,
"description": null,
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -2983,7 +2982,6 @@
"iid": 8,
"description": null,
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -3267,7 +3265,6 @@
"iid": 7,
"description": "Et commodi deserunt aspernatur vero rerum. Ut non dolorum alias in odit est libero. Voluptatibus eos in et vitae repudiandae facilis ex mollitia.",
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -3551,7 +3548,6 @@
"iid": 6,
"description": "Dicta magnam non voluptates nam dignissimos nostrum deserunt. Dolorum et suscipit iure quae doloremque. Necessitatibus saepe aut labore sed.",
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -4241,7 +4237,6 @@
"iid": 5,
"description": "Est eaque quasi qui qui. Similique voluptatem impedit iusto ratione reprehenderit. Itaque est illum ut nulla aut.",
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -4789,7 +4784,6 @@
"iid": 4,
"description": "Nam magnam odit velit rerum. Sapiente dolore sunt saepe debitis. Culpa maiores ut ad dolores dolorem et.",
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -5288,7 +5282,6 @@
"iid": 3,
"description": "Libero nesciunt mollitia quis odit eos vero quasi. Iure voluptatem ut sint pariatur voluptates ut aut. Laborum possimus unde illum ipsum eum.",
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -5548,7 +5541,6 @@
"iid": 2,
"description": "Ut dolor quia aliquid dolore et nisi. Est minus suscipit enim quaerat sapiente consequatur rerum. Eveniet provident consequatur dolor accusantium reiciendis.",
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
@@ -6238,7 +6230,6 @@
"iid": 1,
"description": "Eveniet nihil ratione veniam similique qui aut sapiente tempora. Sed praesentium iusto dignissimos possimus id repudiandae quo nihil. Qui doloremque autem et iure fugit.",
"position": 0,
- "locked_at": null,
"updated_by_id": null,
"merge_error": null,
"merge_params": {
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index 11f4c16ff96..4dce48f8079 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -145,7 +145,6 @@ MergeRequest:
- iid
- description
- position
-- locked_at
- updated_by_id
- merge_error
- merge_params
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 3402c260f27..a1a3e70a7d2 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1369,6 +1369,32 @@ describe MergeRequest do
end
end
+ describe '#merge_ongoing?' do
+ it 'returns true when merge process is ongoing for merge_jid' do
+ merge_request = create(:merge_request, merge_jid: 'foo')
+
+ allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(1)
+
+ expect(merge_request.merge_ongoing?).to be(true)
+ end
+
+ it 'returns false when no merge process running for merge_jid' do
+ merge_request = build(:merge_request, merge_jid: 'foo')
+
+ allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(0)
+
+ expect(merge_request.merge_ongoing?).to be(false)
+ end
+
+ it 'returns false when merge_jid is nil' do
+ merge_request = build(:merge_request, merge_jid: nil)
+
+ expect(Gitlab::SidekiqStatus).not_to receive(:num_running)
+
+ expect(merge_request.merge_ongoing?).to be(false)
+ end
+ end
+
describe "#closed_without_fork?" do
let(:project) { create(:project) }
let(:fork_project) { create(:project, forked_from_project: project) }
diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb
index 18cd9e9c006..a2fd5b7daae 100644
--- a/spec/serializers/merge_request_entity_spec.rb
+++ b/spec/serializers/merge_request_entity_spec.rb
@@ -47,7 +47,7 @@ describe MergeRequestEntity do
:cancel_merge_when_pipeline_succeeds_path,
:create_issue_to_resolve_discussions_path,
:source_branch_path, :target_branch_commits_path,
- :target_branch_tree_path, :commits_count)
+ :target_branch_tree_path, :commits_count, :merge_ongoing)
end
it 'has email_patches_path' do
diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb
index 303193bab9b..ee51000161a 100644
--- a/spec/workers/merge_worker_spec.rb
+++ b/spec/workers/merge_worker_spec.rb
@@ -27,4 +27,15 @@ describe MergeWorker do
expect(source_project.repository.branch_names).not_to include('markdown')
end
end
+
+ it 'persists merge_jid' do
+ merge_request = create(:merge_request, merge_jid: nil)
+ user = create(:user)
+ worker = described_class.new
+
+ allow(worker).to receive(:jid) { '999' }
+
+ expect { worker.perform(merge_request.id, user.id, {}) }
+ .to change { merge_request.reload.merge_jid }.from(nil).to('999')
+ end
end
diff --git a/spec/workers/stuck_merge_jobs_worker_spec.rb b/spec/workers/stuck_merge_jobs_worker_spec.rb
new file mode 100644
index 00000000000..a5ad78393c9
--- /dev/null
+++ b/spec/workers/stuck_merge_jobs_worker_spec.rb
@@ -0,0 +1,50 @@
+require 'spec_helper'
+
+describe StuckMergeJobsWorker do
+ describe 'perform' do
+ let(:worker) { described_class.new }
+
+ context 'merge job identified as completed' do
+ it 'updates merge request to merged when locked but has merge_commit_sha' do
+ allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(%w(123 456))
+ mr_with_sha = create(:merge_request, :locked, merge_jid: '123', state: :locked, merge_commit_sha: 'foo-bar-baz')
+ mr_without_sha = create(:merge_request, :locked, merge_jid: '123', state: :locked, merge_commit_sha: nil)
+
+ worker.perform
+
+ expect(mr_with_sha.reload).to be_merged
+ expect(mr_without_sha.reload).to be_opened
+ end
+
+ it 'updates merge request to opened when locked but has not been merged' do
+ allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(%w(123))
+ merge_request = create(:merge_request, :locked, merge_jid: '123', state: :locked)
+
+ worker.perform
+
+ expect(merge_request.reload).to be_opened
+ end
+
+ it 'logs updated stuck merge job ids' do
+ allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return(%w(123 456))
+
+ create(:merge_request, :locked, merge_jid: '123')
+ create(:merge_request, :locked, merge_jid: '456')
+
+ expect(Rails).to receive_message_chain(:logger, :info).with('Updated state of locked merge jobs. JIDs: 123, 456')
+
+ worker.perform
+ end
+ end
+
+ context 'merge job not identified as completed' do
+ it 'does not change merge request state when job is not completed yet' do
+ allow(Gitlab::SidekiqStatus).to receive(:completed_jids).and_return([])
+
+ merge_request = create(:merge_request, :locked, merge_jid: '123')
+
+ expect { worker.perform }.not_to change { merge_request.reload.state }.from('locked')
+ end
+ end
+ end
+end