diff --git a/Dangerfile b/Dangerfile index cc6ebc27d4e9cf4f150e7649339e891c90231df4..cba7226d4b9aa4b9dd72c126d71cfb7af12ae37a 100644 --- a/Dangerfile +++ b/Dangerfile @@ -6,6 +6,7 @@ require_relative 'lib/gitlab/danger/request_helper' danger.import_plugin('danger/plugins/helper.rb') danger.import_plugin('danger/plugins/roulette.rb') danger.import_plugin('danger/plugins/changelog.rb') +danger.import_plugin('danger/plugins/sidekiq_queues.rb') return if helper.release_automation? diff --git a/danger/plugins/sidekiq_queues.rb b/danger/plugins/sidekiq_queues.rb new file mode 100644 index 0000000000000000000000000000000000000000..1edeb6da3d59ccc22a86bd3707c30d01fd289cf5 --- /dev/null +++ b/danger/plugins/sidekiq_queues.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require_relative '../../lib/gitlab/danger/sidekiq_queues' + +module Danger + class SidekiqQueues < Plugin + # Put the helper code somewhere it can be tested + include Gitlab::Danger::SidekiqQueues + end +end diff --git a/danger/sidekiq_queues/Dangerfile b/danger/sidekiq_queues/Dangerfile new file mode 100644 index 0000000000000000000000000000000000000000..b40be7a486ea5fc897d530347ed01c115fa11c3c --- /dev/null +++ b/danger/sidekiq_queues/Dangerfile @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +SCALABILITY_REVIEW_MESSAGE = <<~MSG +## Sidekiq queue changes + +This merge request contains changes to Sidekiq queues. Please follow the [documentation on changing a queue's urgency](https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#changing-a-queues-urgency). +MSG + +ADDED_QUEUES_MESSAGE = <<~MSG +These queues were added: +MSG + +CHANGED_QUEUES_MESSAGE = <<~MSG +These queues had their attributes changed: +MSG + +if sidekiq_queues.added_queue_names.any? || sidekiq_queues.changed_queue_names.any? + markdown(SCALABILITY_REVIEW_MESSAGE) + + if sidekiq_queues.added_queue_names.any? + markdown(ADDED_QUEUES_MESSAGE + helper.markdown_list(sidekiq_queues.added_queue_names)) + end + + if sidekiq_queues.changed_queue_names.any? + markdown(CHANGED_QUEUES_MESSAGE + helper.markdown_list(sidekiq_queues.changed_queue_names)) + end +end diff --git a/lib/gitlab/danger/sidekiq_queues.rb b/lib/gitlab/danger/sidekiq_queues.rb new file mode 100644 index 0000000000000000000000000000000000000000..5bf6057d5bae330bf4d038a43ec258eefc42f332 --- /dev/null +++ b/lib/gitlab/danger/sidekiq_queues.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Gitlab + module Danger + module SidekiqQueues + def changed_queue_files + @changed_queue_files ||= git.modified_files.grep(%r{\A(ee/)?app/workers/all_queues\.yml}) + end + + def added_queue_names + @added_queue_names ||= new_queues.keys - old_queues.keys + end + + def changed_queue_names + @changed_queue_names ||= + (new_queues.values_at(*old_queues.keys) - old_queues.values) + .map { |queue| queue[:name] } + end + + private + + def old_queues + @old_queues ||= queues_for(gitlab.base_commit) + end + + def new_queues + @new_queues ||= queues_for(gitlab.head_commit) + end + + def queues_for(branch) + changed_queue_files + .flat_map { |file| YAML.safe_load(`git show #{branch}:#{file}`, permitted_classes: [Symbol]) } + .to_h { |queue| [queue[:name], queue] } + end + end + end +end diff --git a/lib/gitlab_danger.rb b/lib/gitlab_danger.rb index 1c1763454a5a75ef6002b7f6e4d9c4c040cb6069..a98ac9200da90aa3dd3c79e9abf84409d660fb54 100644 --- a/lib/gitlab_danger.rb +++ b/lib/gitlab_danger.rb @@ -21,6 +21,7 @@ class GitlabDanger specs roulette ce_ee_vue_templates + sidekiq_queues ].freeze MESSAGE_PREFIX = '==>'.freeze diff --git a/spec/lib/gitlab/danger/sidekiq_queues_spec.rb b/spec/lib/gitlab/danger/sidekiq_queues_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..afae22eda200db8502437315d65483134a4d3972 --- /dev/null +++ b/spec/lib/gitlab/danger/sidekiq_queues_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' +require_relative 'danger_spec_helper' + +require 'gitlab/danger/sidekiq_queues' + +RSpec.describe Gitlab::Danger::SidekiqQueues do + using RSpec::Parameterized::TableSyntax + include DangerSpecHelper + + let(:fake_git) { double('fake-git') } + let(:fake_danger) { new_fake_danger.include(described_class) } + + subject(:sidekiq_queues) { fake_danger.new(git: fake_git) } + + describe '#changed_queue_files' do + where(:modified_files, :changed_queue_files) do + %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml foo) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) + %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) | %w(app/workers/all_queues.yml ee/app/workers/all_queues.yml) + %w(app/workers/all_queues.yml foo) | %w(app/workers/all_queues.yml) + %w(ee/app/workers/all_queues.yml foo) | %w(ee/app/workers/all_queues.yml) + %w(foo) | %w() + %w() | %w() + end + + with_them do + it do + allow(fake_git).to receive(:modified_files).and_return(modified_files) + + expect(sidekiq_queues.changed_queue_files).to match_array(changed_queue_files) + end + end + end + + describe '#added_queue_names' do + it 'returns queue names added by this change' do + old_queues = { post_receive: nil } + + allow(sidekiq_queues).to receive(:old_queues).and_return(old_queues) + allow(sidekiq_queues).to receive(:new_queues).and_return(old_queues.merge(merge: nil, process_commit: nil)) + + expect(sidekiq_queues.added_queue_names).to contain_exactly(:merge, :process_commit) + end + end + + describe '#changed_queue_names' do + it 'returns names for queues whose attributes were changed' do + old_queues = { + merge: { name: :merge, urgency: :low }, + post_receive: { name: :post_receive, urgency: :high }, + process_commit: { name: :process_commit, urgency: :high } + } + + new_queues = old_queues.merge(mailers: { name: :mailers, urgency: :high }, + post_receive: { name: :post_receive, urgency: :low }, + process_commit: { name: :process_commit, urgency: :low }) + + allow(sidekiq_queues).to receive(:old_queues).and_return(old_queues) + allow(sidekiq_queues).to receive(:new_queues).and_return(new_queues) + + expect(sidekiq_queues.changed_queue_names).to contain_exactly(:post_receive, :process_commit) + end + end +end