From b6433acaa2ca8f93d6eeb089210b052d65308a0a Mon Sep 17 00:00:00 2001 From: davidgisbey Date: Tue, 8 Oct 2024 13:56:49 +0100 Subject: [PATCH 1/2] Update SidekiqRedis to work with Sidekiq 7 This healthcheck has been failing for applications that have upgraded to Sidekiq 7. Sidekiq 7 uses 'redis-client' which has a different API to 'redis' which was used in Sidekiq 6. To access redis_info you now have to call ``` Sidekiq.default_configration.redis_info ``` while in Sidekiq 6 you call ``` Sidekiq.redis_info ``` This updates the healthcheck to work with both versions of Sidekiq checking if Sidekiq.responds to :redis_info. If it does it calls that, otherwise it calls Sidekiq.default_configration.redis_info. It's not missively clear these healthchecks are actually being used for anything since this has been failing on production applications for a couple of weeks without anyone noticing. We're in the process of speaking to the platform team to see if they're actually being used for anything and if not whether they should be removed. But for now i'm just updating it to work with Sidekiq 7. --- .../govuk_healthcheck/sidekiq_redis.rb | 9 ++- .../govuk_healthcheck/sidekiq_redis_spec.rb | 65 ++++++++++++++----- 2 files changed, 57 insertions(+), 17 deletions(-) diff --git a/lib/govuk_app_config/govuk_healthcheck/sidekiq_redis.rb b/lib/govuk_app_config/govuk_healthcheck/sidekiq_redis.rb index 5ba4d9be..f67cbf79 100644 --- a/lib/govuk_app_config/govuk_healthcheck/sidekiq_redis.rb +++ b/lib/govuk_app_config/govuk_healthcheck/sidekiq_redis.rb @@ -5,7 +5,14 @@ def name end def status - Sidekiq.redis_info ? OK : CRITICAL + # Sidekiq 7 introduced a default_configuration object which has .redis_info + # for querying Redis information. If the default_configuration object isn't present, + # we can fall back to the old method of querying it using 'Sidekiq.redis_info'. + if Sidekiq.respond_to?(:default_configuration) + Sidekiq.default_configuration.redis_info ? OK : CRITICAL + else + Sidekiq.redis_info ? OK : CRITICAL + end rescue StandardError # One would expect a Redis::BaseConnectionError, but this should be # critical if any exception is raised when making a call to redis. diff --git a/spec/lib/govuk_healthcheck/sidekiq_redis_spec.rb b/spec/lib/govuk_healthcheck/sidekiq_redis_spec.rb index d1ef6e91..efebde93 100644 --- a/spec/lib/govuk_healthcheck/sidekiq_redis_spec.rb +++ b/spec/lib/govuk_healthcheck/sidekiq_redis_spec.rb @@ -3,35 +3,68 @@ require_relative "shared_interface" RSpec.describe GovukHealthcheck::SidekiqRedis do - let(:redis_info) { double(:redis_info) } - let(:sidekiq) { double(:sidekiq, redis_info:) } before { stub_const("Sidekiq", sidekiq) } - context "when the database is connected" do + context "when Sidekiq responds to '.default_configuration'" do let(:redis_info) { double(:redis_info) } + let(:default_configuration) { double(:default_configuration, redis_info:) } + let(:sidekiq) { double(:sidekiq, default_configuration:) } - it_behaves_like "a healthcheck" + context "and the database is connected" do + it_behaves_like "a healthcheck" - it "returns OK" do - expect(subject.status).to eq(GovukHealthcheck::OK) + it "returns OK" do + expect(subject.status).to eq(GovukHealthcheck::OK) + end end - end - context "when the database is not connected" do - let(:redis_info) { nil } + context "when the database is not connected" do + let(:redis_info) { nil } + + it_behaves_like "a healthcheck" + + it "returns CRITICAL" do + expect(subject.status).to eq(GovukHealthcheck::CRITICAL) + end + end - it_behaves_like "a healthcheck" + context "and redis raises a connection error" do + it "returns CRITICAL" do + allow(default_configuration).to receive(:redis_info).and_raise StandardError - it "returns CRITICAL" do - expect(subject.status).to eq(GovukHealthcheck::CRITICAL) + expect(subject.status).to eq(GovukHealthcheck::CRITICAL) + end end end - context "when redis raises a connection error" do - it "returns CRITICAL" do - allow(sidekiq).to receive(:redis_info).and_raise StandardError + context "when Sidekiq doesn't respond to '.default_configuration'" do + let(:redis_info) { double(:redis_info) } + let(:sidekiq) { double(:sidekiq, redis_info:) } + + context "and the database is connected" do + it_behaves_like "a healthcheck" + + it "returns OK" do + expect(subject.status).to eq(GovukHealthcheck::OK) + end + end + + context "when the database is not connected" do + let(:redis_info) { nil } + + it_behaves_like "a healthcheck" + + it "returns CRITICAL" do + expect(subject.status).to eq(GovukHealthcheck::CRITICAL) + end + end + + context "and redis raises a connection error" do + it "returns CRITICAL" do + allow(sidekiq).to receive(:redis_info).and_raise StandardError - expect(subject.status).to eq(GovukHealthcheck::CRITICAL) + expect(subject.status).to eq(GovukHealthcheck::CRITICAL) + end end end end From d6f52f6e73fb29b82208e2877c0152b4257cc046 Mon Sep 17 00:00:00 2001 From: davidgisbey Date: Tue, 8 Oct 2024 14:11:44 +0100 Subject: [PATCH 2/2] Bump gem version to 9.14.3 - Update SidekiqRedis healthcheck to work with Sidekiq 7 [#399](https://github.com/alphagov/govuk_app_config/pull/399) --- CHANGELOG.md | 4 ++++ lib/govuk_app_config/version.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c896ce8..3f93533c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 9.14.3 + +* Update SidekiqRedis healthcheck to work with Sidekiq 7 [#399](https://github.com/alphagov/govuk_app_config/pull/399) + # 9.14.2 * Update dependencies diff --git a/lib/govuk_app_config/version.rb b/lib/govuk_app_config/version.rb index e266af30..d00957db 100644 --- a/lib/govuk_app_config/version.rb +++ b/lib/govuk_app_config/version.rb @@ -1,3 +1,3 @@ module GovukAppConfig - VERSION = "9.14.2".freeze + VERSION = "9.14.3".freeze end