Commit e4edc69e authored by Sean McGivern's avatar Sean McGivern

Add a Danger message when Sidekiq queues change

When Sidekiq queues are added, or existing queues are changed, this can
have some impact on our infrastructure as we may have a change in
workload that leads to us needing to make an infrastructure change.

The Scalability team can help with this, so point to them as a first
step.
parent c3caf581
......@@ -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?
......
# 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
# 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
# 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
......@@ -21,6 +21,7 @@ class GitlabDanger
specs
roulette
ce_ee_vue_templates
sidekiq_queues
].freeze
MESSAGE_PREFIX = '==>'.freeze
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment