Merge branch 'use-publish-to-post-notifications' into 'master'

Use publish channel to post notifications changes

See merge request !9574

4 files changed, 76 insertions, 8 deletions

diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 4863c34a6a6..edd21f984c8 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -127,18 +127,15 @@ module Ci
 
     def tick_runner_queue
       SecureRandom.hex.tap do |new_update|
-        Gitlab::Redis.with do |redis|
-          redis.set(runner_queue_key, new_update, ex: RUNNER_QUEUE_EXPIRY_TIME)
-        end
+        ::Gitlab::Workhorse.set_key_and_notify(runner_queue_key, new_update,
+          expire: RUNNER_QUEUE_EXPIRY_TIME, overwrite: true)
       end
     end
 
     def ensure_runner_queue_value
-      Gitlab::Redis.with do |redis|
-        value = SecureRandom.hex
-        redis.set(runner_queue_key, value, ex: RUNNER_QUEUE_EXPIRY_TIME, nx: true)
-        redis.get(runner_queue_key)
-      end
+      new_value = SecureRandom.hex
+      ::Gitlab::Workhorse.set_key_and_notify(runner_queue_key, new_value,
+        expire: RUNNER_QUEUE_EXPIRY_TIME, overwrite: false)
     end
 
     def is_runner_queue_value_latest?(value)
diff --git a/changelogs/unreleased/use-redis-channel-to-post-runner-notifcations.yml b/changelogs/unreleased/use-redis-channel-to-post-runner-notifcations.yml
new file mode 100644
index 00000000000..ff5a58f6232
--- /dev/null
+++ b/changelogs/unreleased/use-redis-channel-to-post-runner-notifcations.yml
@@ -0,0 +1,4 @@
+---
+title: Use redis channel to post notifications
+merge_request:
+author:
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index 3ff9f9eb5e7..eae1a0abf06 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -8,6 +8,7 @@ module Gitlab
     VERSION_FILE = 'GITLAB_WORKHORSE_VERSION'.freeze
     INTERNAL_API_CONTENT_TYPE = 'application/vnd.gitlab-workhorse+json'.freeze
     INTERNAL_API_REQUEST_HEADER = 'Gitlab-Workhorse-Api-Request'.freeze
+    NOTIFICATION_CHANNEL = 'workhorse:notifications'.freeze
 
     # Supposedly the effective key size for HMAC-SHA256 is 256 bits, i.e. 32
     # bytes https://tools.ietf.org/html/rfc4868#section-2.6
@@ -154,6 +155,18 @@ module Gitlab
         Rails.root.join('.gitlab_workhorse_secret')
       end
 
+      def set_key_and_notify(key, value, expire: nil, overwrite: true)
+        Gitlab::Redis.with do |redis|
+          result = redis.set(key, value, ex: expire, nx: !overwrite)
+          if result
+            redis.publish(NOTIFICATION_CHANNEL, "#{key}=#{value}")
+            value
+          else
+            redis.get(key)
+          end
+        end
+      end
+
       protected
 
       def encode(hash)
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index a32c6131030..8e5e8288c49 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -199,4 +199,58 @@ describe Gitlab::Workhorse, lib: true do
       end
     end
   end
+
+  describe '.set_key_and_notify' do
+    let(:key) { 'test-key' }
+    let(:value) { 'test-value' }
+
+    subject { described_class.set_key_and_notify(key, value, overwrite: overwrite) }
+
+    shared_examples 'set and notify' do
+      it 'set and return the same value' do
+        is_expected.to eq(value)
+      end
+
+      it 'set and notify' do
+        expect_any_instance_of(Redis).to receive(:publish)
+          .with(described_class::NOTIFICATION_CHANNEL, "test-key=test-value")
+
+        subject
+      end
+    end
+
+    context 'when we set a new key' do
+      let(:overwrite) { true }
+
+      it_behaves_like 'set and notify'
+    end
+
+    context 'when we set an existing key' do
+      let(:old_value) { 'existing-key' }
+
+      before do
+        described_class.set_key_and_notify(key, old_value, overwrite: true)
+      end
+
+      context 'and overwrite' do
+        let(:overwrite) { true }
+
+        it_behaves_like 'set and notify'
+      end
+
+      context 'and do not overwrite' do
+        let(:overwrite) { false }
+
+        it 'try to set but return the previous value' do
+          is_expected.to eq(old_value)
+        end
+
+        it 'does not notify' do
+          expect_any_instance_of(Redis).not_to receive(:publish)
+
+          subject
+        end
+      end
+    end
+  end
 end