Merge branch 'message-in-danger-when-sidekiq-queues-change' into 'master'

Add a Danger message when Sidekiq queues change

See merge request gitlab-org/gitlab!34841

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?
 

danger/plugins/sidekiq_queues.rb

+# 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

danger/sidekiq_queues/Dangerfile

+# 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

lib/gitlab/danger/sidekiq_queues.rb

+# 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

lib/gitlab_danger.rb

@@ -21,6 +21,7 @@ class GitlabDanger
     specs
     roulette
     ce_ee_vue_templates
+    sidekiq_queues
   ].freeze
 
   MESSAGE_PREFIX = '==>'.freeze

spec/lib/gitlab/danger/sidekiq_queues_spec.rb

+# 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