Merge pull request #185 from alphagov/redis-healthcheck-lock

Redis healthcheck lock

CHANGELOG.md

@@ -1,6 +1,10 @@
+# 2.8.4
+
+* Ensure Redis healthcheck avoids potential race condition ([#185](https://github.com/alphagov/govuk_app_config/pull/185))
+
 # 2.8.3
 
-* Add new Redis healthcheck and relevant tests (https://github.com/alphagov/govuk_app_config/pull/183)
+* Add new Redis healthcheck and relevant tests ([#183](https://github.com/alphagov/govuk_app_config/pull/183))
 
 # 2.8.2
 

lib/govuk_app_config/govuk_healthcheck/redis.rb

@@ -1,15 +1,19 @@
+require "securerandom"
+
 module GovukHealthcheck
   class Redis
     def name
-      :redis
+      :redis_connectivity
     end
 
     def status
       client = ::Redis.new
 
-      client.set("healthcheck", "val")
-      client.get("healthcheck")
-      client.del("healthcheck")
+      key = "healthcheck-#{SecureRandom.hex}"
+
+      client.set(key, "val")
+      client.get(key)
+      client.del(key)
 
       client.close
 

lib/govuk_app_config/version.rb

@@ -1,3 +1,3 @@
 module GovukAppConfig
-  VERSION = "2.8.3".freeze
+  VERSION = "2.8.4".freeze
 end

spec/lib/govuk_healthcheck/redis_spec.rb

@@ -9,20 +9,31 @@
   before do
     stub_const("Redis", redis)
     allow(redis).to receive(:new).and_return(redis_client)
+    allow(SecureRandom).to receive(:hex).and_return("abc")
   end
 
   context "when the database is connected" do
-    before { allow(redis_client).to receive(:set) }
+    before do
+      allow(redis_client)
+        .to receive(:set).with("healthcheck-abc", anything)
+    end
+
     it_behaves_like "a healthcheck"
+
     it "returns OK" do
-      expect(redis_client).to receive(:set).with(anything, anything)
       expect(subject.status).to eq(GovukHealthcheck::OK)
     end
   end
 
   context "when the database is not connected" do
-    before { allow(redis_client).to receive(:set).with(anything, anything).and_raise("error") }
+    before do
+      allow(redis_client)
+        .to receive(:set).with("healthcheck-abc", anything)
+        .and_raise("error")
+    end
+
     it_behaves_like "a healthcheck"
+
     it "returns CRITICAL" do
       expect(subject.status).to eq(GovukHealthcheck::CRITICAL)
     end