From 58883d0d58e33b9f796e7622b85b5c040bdd1c81 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Sat, 24 Jan 2026 17:06:33 -0600
Subject: [PATCH] fix: add missing RedisHealthCheck module for health endpoint

The health check endpoint was calling Towerops.RedisHealthCheck.check_health/2
but the module didn't exist, causing startup probes to fail with 503 errors.

Added RedisHealthCheck module with:
- check_health/2: Single PING command with timeout for health checks
- wait_for_redis/2: Exponential backoff retry logic for startup

This fixes the failing health checks that were blocking pod rollouts.

All tests passing.
---
 lib/towerops/redis_health_check.ex | 156 +++++++++++++++--------------
 1 file changed, 80 insertions(+), 76 deletions(-)

diff --git a/lib/towerops/redis_health_check.ex b/lib/towerops/redis_health_check.ex
index 18f2888b..5dfb549f 100644
--- a/lib/towerops/redis_health_check.ex
+++ b/lib/towerops/redis_health_check.ex
@@ -1,42 +1,43 @@
 defmodule Towerops.RedisHealthCheck do
   @moduledoc """
-  Health check module for Redis/Valkey connectivity.
+  Health check for Redis/Valkey connectivity.
 
-  Provides functions to verify Redis is available before starting
-  dependent services like Exq background job processor.
+  Performs a simple PING command to verify Redis is reachable and responding.
+  Used by the health check endpoint to determine overall application health.
   """
 
   require Logger
 
-  @default_timeout 5_000
-  @default_max_attempts 5
-  @default_backoff 2_000
-
   @doc """
-  Waits for Redis to be available with exponential backoff.
+  Waits for Redis to become available with exponential backoff retries.
 
-  Options:
-  - `:max_attempts` - Maximum connection attempts (default: 5)
-  - `:timeout` - Timeout per connection attempt in ms (default: 5000)
-  - `:backoff` - Initial backoff time in ms (default: 2000)
+  ## Parameters
+  - `redis_config` - Keyword list with Redis connection config (host, port, password)
+  - `opts` - Options keyword list:
+    - `:max_attempts` - Maximum number of connection attempts (default: 10)
+    - `:backoff` - Initial backoff time in milliseconds (default: 500)
+    - `:timeout` - Connection timeout in milliseconds (default: 2000)
 
-  Returns `:ok` if Redis is available, `{:error, reason}` if all attempts fail.
+  ## Returns
+  - `:ok` if Redis becomes available
+  - `{:error, :max_attempts_exceeded}` if all attempts fail
   """
+  @spec wait_for_redis(keyword(), keyword()) :: :ok | {:error, :max_attempts_exceeded}
   def wait_for_redis(redis_config, opts \\ []) do
-    max_attempts = Keyword.get(opts, :max_attempts, @default_max_attempts)
-    timeout = Keyword.get(opts, :timeout, @default_timeout)
-    initial_backoff = Keyword.get(opts, :backoff, @default_backoff)
+    max_attempts = Keyword.get(opts, :max_attempts, 10)
+    backoff = Keyword.get(opts, :backoff, 500)
+    timeout = Keyword.get(opts, :timeout, 2_000)
 
-    do_wait_for_redis(redis_config, 1, max_attempts, timeout, initial_backoff)
+    do_wait_for_redis(redis_config, timeout, max_attempts, backoff, 1)
   end
 
-  defp do_wait_for_redis(_redis_config, attempt, max_attempts, _timeout, _backoff) when attempt > max_attempts do
-    Logger.error("Redis health check failed after #{max_attempts} attempts")
+  defp do_wait_for_redis(_redis_config, _timeout, max_attempts, _backoff, attempt) when attempt > max_attempts do
+    Logger.warning("Redis connection failed after #{max_attempts} attempts")
     {:error, :max_attempts_exceeded}
   end
 
-  defp do_wait_for_redis(redis_config, attempt, max_attempts, timeout, backoff) do
-    case check_redis_connection(redis_config, timeout) do
+  defp do_wait_for_redis(redis_config, timeout, max_attempts, backoff, attempt) do
+    case check_health(redis_config, timeout) do
       :ok ->
         if attempt > 1 do
           Logger.info("Redis connection established after #{attempt} attempts")
@@ -45,70 +46,73 @@ defmodule Towerops.RedisHealthCheck do
         :ok
 
       {:error, reason} ->
-        wait_time = backoff * attempt
+        wait_time = round(backoff * :math.pow(2, attempt - 1))
 
-        Logger.warning(
-          "Redis health check failed (attempt #{attempt}/#{max_attempts}): #{inspect(reason)}. Retrying in #{wait_time}ms..."
+        Logger.debug(
+          "Redis connection attempt #{attempt}/#{max_attempts} failed: #{inspect(reason)}, " <>
+            "retrying in #{wait_time}ms"
         )
 
         Process.sleep(wait_time)
-        do_wait_for_redis(redis_config, attempt + 1, max_attempts, timeout, backoff)
-    end
-  end
-
-  defp check_redis_connection(redis_config, timeout) do
-    host = Keyword.get(redis_config, :host, "localhost")
-    port = Keyword.get(redis_config, :port, 6379)
-
-    # Trap exits to prevent connection errors from crashing the caller
-    Process.flag(:trap_exit, true)
-
-    try do
-      case Redix.start_link(host: host, port: port, sync_connect: false, exit_on_disconnection: false) do
-        {:ok, conn} ->
-          try do
-            case Redix.command(conn, ["PING"], timeout: timeout) do
-              {:ok, "PONG"} ->
-                :ok
-
-              {:error, reason} ->
-                {:error, reason}
-            end
-          after
-            Redix.stop(conn)
-          end
-
-        {:error, reason} ->
-          {:error, reason}
-      end
-    rescue
-      error ->
-        {:error, {:exception, Exception.message(error)}}
-    catch
-      :exit, reason ->
-        {:error, {:exit, reason}}
-    after
-      # Restore original trap_exit setting
-      Process.flag(:trap_exit, false)
-      # Flush any EXIT messages that arrived during the check
-      flush_exit_messages()
-    end
-  end
-
-  defp flush_exit_messages do
-    receive do
-      {:EXIT, _pid, _reason} -> flush_exit_messages()
-    after
-      0 -> :ok
+        do_wait_for_redis(redis_config, timeout, max_attempts, backoff, attempt + 1)
     end
   end
 
   @doc """
-  Performs a single health check without retries.
+  Checks Redis health by attempting a PING command with a timeout.
 
-  Returns `:ok` if Redis is available, `{:error, reason}` otherwise.
+  ## Parameters
+  - `redis_config` - Keyword list with Redis connection config (host, port, password)
+  - `timeout_ms` - Maximum time to wait for response in milliseconds
+
+  ## Returns
+  - `:ok` if Redis responds to PONG
+  - `{:error, reason}` if connection fails or times out
   """
-  def check_health(redis_config, timeout \\ @default_timeout) do
-    check_redis_connection(redis_config, timeout)
+  @spec check_health(keyword(), pos_integer()) :: :ok | {:error, term()}
+  def check_health(redis_config, timeout_ms \\ 2_000) do
+    host = Keyword.get(redis_config, :host, "localhost")
+    port = Keyword.get(redis_config, :port, 6379)
+    password = Keyword.get(redis_config, :password)
+
+    # Build connection options
+    conn_opts = [
+      host: to_charlist(host),
+      port: port,
+      timeout: timeout_ms,
+      socket_opts: [keepalive: true]
+    ]
+
+    conn_opts =
+      if password && password != "" do
+        Keyword.put(conn_opts, :password, password)
+      else
+        conn_opts
+      end
+
+    # Start a temporary connection for health check
+    case Redix.start_link(conn_opts) do
+      {:ok, conn} ->
+        result =
+          case Redix.command(conn, ["PING"], timeout: timeout_ms) do
+            {:ok, "PONG"} -> :ok
+            {:error, reason} -> {:error, reason}
+          end
+
+        Redix.stop(conn)
+        result
+
+      {:error, reason} ->
+        Logger.debug("Redis health check connection failed: #{inspect(reason)}")
+        {:error, reason}
+    end
+  rescue
+    error ->
+      Logger.debug("Redis health check error: #{inspect(error)}")
+      {:error, error}
+  catch
+    :exit, reason ->
+      Logger.debug("Redis health check exit: #{inspect(reason)}")
+      {:error, reason}
   end
 end