Fix modular mode deployment issues (#1045)

- Fix Postgres version check failing with restricted DB users (use $POSTGRES_DB instead of hardcoded 'postgres')
- Fix DVR recording broken in modular mode (respect DISPATCHARR_PORT instead of hardcoding 9191)
- Remove flushdb() from wait_for_redis.py to prevent Redis data loss on container restart
- Add DISPATCHARR_PORT to celery environment in docker-compose.yml
- Add depends_on health conditions for proper service startup ordering
- Add extra_hosts for host.docker.internal resolution on Linux
- Harden celery entrypoint with timeouts for JWT wait (120s) and migration wait (300s)
- Replace fragile showmigrations grep with migrate --check
- Add unit tests for DVR port resolution and flushdb removal regression

apps/channels/tasks.py

@@ -1486,6 +1486,33 @@ def _build_output_paths(channel, program, start_time, end_time):
     return final_path, temp_ts_path, os.path.basename(final_path)
 
 
+def build_dvr_candidates():
+    """Build ordered list of candidate base URLs for DVR TS streaming.
+
+    Reads environment variables to determine which URLs to try:
+    - DISPATCHARR_INTERNAL_TS_BASE_URL: explicit override (first priority)
+    - DISPATCHARR_PORT: the external port (default 9191)
+    - DISPATCHARR_ENV/DISPATCHARR_DEBUG/REDIS_HOST: dev-mode detection
+    - DISPATCHARR_INTERNAL_API_BASE: override for the docker service URL
+    """
+    explicit = os.environ.get('DISPATCHARR_INTERNAL_TS_BASE_URL')
+    dispatcharr_port = os.environ.get('DISPATCHARR_PORT', '9191')
+    is_dev = (os.environ.get('DISPATCHARR_ENV', '').lower() == 'dev') or \
+             (os.environ.get('DISPATCHARR_DEBUG', '').lower() == 'true') or \
+             (os.environ.get('REDIS_HOST', 'redis') in ('localhost', '127.0.0.1'))
+    candidates = []
+    if explicit:
+        candidates.append(explicit)
+    if is_dev:
+        # Debug container typically exposes API on 5656 (uwsgi internal port)
+        candidates.extend(['http://127.0.0.1:5656', f'http://127.0.0.1:{dispatcharr_port}'])
+    # Docker service name fallback — use DISPATCHARR_PORT so modular mode works with custom ports
+    candidates.append(os.environ.get('DISPATCHARR_INTERNAL_API_BASE', f'http://web:{dispatcharr_port}'))
+    # Last-resort localhost ports
+    candidates.extend(['http://localhost:5656', f'http://localhost:{dispatcharr_port}'])
+    return candidates
+
+
 @shared_task
 def run_recording(recording_id, channel_id, start_time_str, end_time_str):
     """
@@ -1790,22 +1817,7 @@ def run_recording(recording_id, channel_id, start_time_str, end_time_str):
 
     from requests.exceptions import ReadTimeout, ConnectionError as ReqConnectionError, ChunkedEncodingError
 
-    # Determine internal base URL(s) for TS streaming
-    # Prefer explicit override, then try common ports for debug and docker
-    explicit = os.environ.get('DISPATCHARR_INTERNAL_TS_BASE_URL')
-    is_dev = (os.environ.get('DISPATCHARR_ENV', '').lower() == 'dev') or \
-             (os.environ.get('DISPATCHARR_DEBUG', '').lower() == 'true') or \
-             (os.environ.get('REDIS_HOST', 'redis') in ('localhost', '127.0.0.1'))
-    candidates = []
-    if explicit:
-        candidates.append(explicit)
-    if is_dev:
-        # Debug container typically exposes API on 5656
-        candidates.extend(['http://127.0.0.1:5656', 'http://127.0.0.1:9191'])
-    # Docker service name fallback
-    candidates.append(os.environ.get('DISPATCHARR_INTERNAL_API_BASE', 'http://web:9191'))
-    # Last-resort localhost ports
-    candidates.extend(['http://localhost:5656', 'http://localhost:9191'])
+    candidates = build_dvr_candidates()
 
     chosen_base = None
     last_error = None

apps/channels/tests/test_dvr_port_resolution.py

@@ -0,0 +1,59 @@
+import os
+from django.test import SimpleTestCase
+from unittest.mock import patch
+
+from apps.channels.tasks import build_dvr_candidates
+
+
+class DVRPortResolutionTests(SimpleTestCase):
+    """
+    Tests that DVR recording candidate URLs respect the DISPATCHARR_PORT
+    environment variable instead of hardcoding port 9191.
+    """
+
+    @patch.dict(os.environ, {'REDIS_HOST': 'redis'}, clear=True)
+    def test_default_port_uses_9191(self):
+        """Without DISPATCHARR_PORT set, candidates default to 9191."""
+        candidates = build_dvr_candidates()
+        self.assertIn('http://web:9191', candidates)
+        self.assertIn('http://localhost:9191', candidates)
+
+    @patch.dict(os.environ, {'DISPATCHARR_PORT': '8080', 'REDIS_HOST': 'redis'}, clear=True)
+    def test_custom_port_reflected_in_candidates(self):
+        """DISPATCHARR_PORT=8080 replaces all hardcoded 9191 references."""
+        candidates = build_dvr_candidates()
+        self.assertIn('http://web:8080', candidates)
+        self.assertIn('http://localhost:8080', candidates)
+        self.assertNotIn('http://web:9191', candidates)
+        self.assertNotIn('http://localhost:9191', candidates)
+
+    @patch.dict(os.environ, {
+        'DISPATCHARR_PORT': '7777',
+        'DISPATCHARR_ENV': 'dev',
+        'REDIS_HOST': 'redis',
+    }, clear=True)
+    def test_dev_mode_includes_5656_and_custom_port(self):
+        """Dev mode includes both uwsgi internal port (5656) and custom port."""
+        candidates = build_dvr_candidates()
+        self.assertIn('http://127.0.0.1:5656', candidates)
+        self.assertIn('http://127.0.0.1:7777', candidates)
+
+    @patch.dict(os.environ, {
+        'DISPATCHARR_INTERNAL_TS_BASE_URL': 'http://custom:1234',
+        'REDIS_HOST': 'redis',
+    }, clear=True)
+    def test_explicit_override_is_first(self):
+        """DISPATCHARR_INTERNAL_TS_BASE_URL should be the first candidate."""
+        candidates = build_dvr_candidates()
+        self.assertEqual(candidates[0], 'http://custom:1234')
+
+    @patch.dict(os.environ, {
+        'DISPATCHARR_PORT': '3000',
+        'DISPATCHARR_INTERNAL_API_BASE': 'http://myhost:4000',
+        'REDIS_HOST': 'redis',
+    }, clear=True)
+    def test_internal_api_base_overrides_web_fallback(self):
+        """DISPATCHARR_INTERNAL_API_BASE replaces the http://web:{port} default."""
+        candidates = build_dvr_candidates()
+        self.assertIn('http://myhost:4000', candidates)
+        self.assertNotIn('http://web:3000', candidates)

docker/docker-compose.yml

@@ -15,8 +15,12 @@ services:
     volumes:
       - ./data:/data
     depends_on:
-      - db
-      - redis
+      db:
+        condition: service_healthy
+      redis:
+        condition: service_healthy
+    extra_hosts:
+      - "host.docker.internal:host-gateway"
 
     # --- Environment Configuration ---
     environment:
@@ -83,9 +87,12 @@ services:
     container_name: dispatcharr_celery
     restart: unless-stopped
     depends_on:
-      - db
-      - redis
-      - web
+      db:
+        condition: service_healthy
+      redis:
+        condition: service_healthy
+      web:
+        condition: service_started
     volumes:
       - ./data:/data
     extra_hosts:
@@ -97,6 +104,10 @@ services:
       # Deployment Mode
       - DISPATCHARR_ENV=modular
 
+      # Internal Service Communication
+      # Must match the web service port for DVR recording and internal API calls
+      - DISPATCHARR_PORT=9191
+
       # PostgreSQL Connection
       - POSTGRES_HOST=db
       - POSTGRES_PORT=5432

docker/entrypoint.celery.sh

@@ -9,9 +9,19 @@ echo_with_timestamp() {
   echo "$(date '+%Y-%m-%d %H:%M:%S') - $1"
 }
 
-# Wait for Django secret key
+# Wait for Django secret key (generated by the web container on startup)
+JWT_TIMEOUT=120
+JWT_WAITED=0
 echo 'Waiting for Django secret key...'
-while [ ! -f /data/jwt ]; do sleep 1; done
+while [ ! -f /data/jwt ]; do
+    if [ $JWT_WAITED -ge $JWT_TIMEOUT ]; then
+        echo "❌ ERROR: Timed out waiting for /data/jwt after ${JWT_TIMEOUT}s."
+        echo "   Is the web container running? Does it have the /data volume mounted?"
+        exit 1
+    fi
+    sleep 1
+    JWT_WAITED=$((JWT_WAITED + 1))
+done
 export DJANGO_SECRET_KEY="$(tr -d '\r\n' < /data/jwt)"
 
 # --- NumPy version switching for legacy hardware ---
@@ -26,11 +36,21 @@ if [ "$USE_LEGACY_NUMPY" = "true" ]; then
     fi
 fi
 
-# Wait for migrations to complete (check that NO unapplied migrations remain)
+# Wait for migrations to complete
+# Uses 'migrate --check' which exits 0 only when all migrations are applied,
+# and exits 1 on unapplied migrations OR connection errors (safe either way)
+MIG_TIMEOUT=300
+MIG_WAITED=0
 echo 'Waiting for migrations to complete...'
-until ! python manage.py showmigrations 2>&1 | grep -q '\[ \]'; do
+until python manage.py migrate --check >/dev/null 2>&1; do
+    if [ $MIG_WAITED -ge $MIG_TIMEOUT ]; then
+        echo "❌ ERROR: Timed out waiting for migrations after ${MIG_TIMEOUT}s."
+        echo "   Check web container logs for migration errors."
+        exit 1
+    fi
     echo_with_timestamp 'Migrations not ready yet, waiting...'
     sleep 2
+    MIG_WAITED=$((MIG_WAITED + 2))
 done
 
 # Start Celery

docker/init/02-postgres.sh

@@ -204,14 +204,19 @@ check_external_postgres_version() {
     MIN_REQUIRED_VERSION=$PG_VERSION
 
     # Query external PostgreSQL version
-    EXTERNAL_VERSION=$(PGPASSWORD="$POSTGRES_PASSWORD" psql -w -h "$POSTGRES_HOST" -p "$POSTGRES_PORT" -U "$POSTGRES_USER" -d "postgres" -tAc "SHOW server_version;" 2>/dev/null | grep -oE '^[0-9]+')
+    # Use $POSTGRES_DB — restricted users may not have access to the default 'postgres' database
+    PG_VERSION_ERR=$(mktemp)
+    EXTERNAL_VERSION=$(PGPASSWORD="$POSTGRES_PASSWORD" psql -w -h "$POSTGRES_HOST" -p "$POSTGRES_PORT" -U "$POSTGRES_USER" -d "$POSTGRES_DB" -tAc "SHOW server_version;" 2>"$PG_VERSION_ERR" | grep -oE '^[0-9]+')
 
     if [ -z "$EXTERNAL_VERSION" ]; then
         echo "❌ ERROR: Unable to determine external PostgreSQL version"
-        echo "   Could not connect to database at ${POSTGRES_HOST}:${POSTGRES_PORT}"
+        echo "   Could not connect to database '$POSTGRES_DB' at ${POSTGRES_HOST}:${POSTGRES_PORT} as user '$POSTGRES_USER'"
+        echo "   Error: $(cat "$PG_VERSION_ERR")"
         echo "   Please verify your database connection settings."
+        rm -f "$PG_VERSION_ERR"
         return 1
     fi
+    rm -f "$PG_VERSION_ERR"
 
     # Compare versions
     if [[ "$EXTERNAL_VERSION" -lt "$MIN_REQUIRED_VERSION" ]]; then

scripts/wait_for_redis.py

@@ -31,7 +31,6 @@ def wait_for_redis(host='localhost', port=6379, db=0, password='', username='',
                 socket_connect_timeout=2
             )
             redis_client.ping()
-            redis_client.flushdb()  # Flush the database to ensure it's clean
             logger.info(f"✅ Redis at {host}:{port}/{db} is now available!")
             return True
         except (redis.exceptions.ConnectionError, redis.exceptions.TimeoutError) as e:

tests/test_wait_for_redis.py

@@ -0,0 +1,81 @@
+import sys
+import os
+import importlib
+from django.test import SimpleTestCase
+from unittest.mock import patch, MagicMock
+
+import redis as redis_module
+
+# Ensure the scripts directory is importable
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..', 'scripts'))
+
+
+def _import_wait_for_redis():
+    """Import (or reimport) the wait_for_redis function from scripts/."""
+    import wait_for_redis as module
+    importlib.reload(module)
+    return module.wait_for_redis
+
+
+class WaitForRedisTests(SimpleTestCase):
+    """
+    Tests for scripts/wait_for_redis.py.
+
+    The critical regression test is test_successful_connection_does_not_call_flushdb
+    which ensures the removed flushdb() call is never re-added.
+    """
+
+    @patch('wait_for_redis.redis.Redis')
+    def test_successful_connection_does_not_call_flushdb(self, mock_redis_cls):
+        """After connecting successfully, flushdb must NOT be called."""
+        mock_client = MagicMock()
+        mock_client.ping.return_value = True
+        mock_redis_cls.return_value = mock_client
+
+        wait_for_redis = _import_wait_for_redis()
+        result = wait_for_redis(max_retries=1, retry_interval=0)
+
+        self.assertTrue(result)
+        mock_client.ping.assert_called_once()
+        mock_client.flushdb.assert_not_called()
+
+    @patch('wait_for_redis.redis.Redis')
+    def test_retries_on_connection_error(self, mock_redis_cls):
+        """Should retry on ConnectionError and eventually succeed."""
+        mock_client = MagicMock()
+        mock_client.ping.side_effect = [
+            redis_module.exceptions.ConnectionError("refused"),
+            redis_module.exceptions.ConnectionError("refused"),
+            True,
+        ]
+        mock_redis_cls.return_value = mock_client
+
+        wait_for_redis = _import_wait_for_redis()
+        result = wait_for_redis(max_retries=5, retry_interval=0)
+
+        self.assertTrue(result)
+        self.assertEqual(mock_client.ping.call_count, 3)
+
+    @patch('wait_for_redis.redis.Redis')
+    def test_returns_false_after_max_retries(self, mock_redis_cls):
+        """Should return False when max retries are exhausted."""
+        mock_client = MagicMock()
+        mock_client.ping.side_effect = redis_module.exceptions.ConnectionError("refused")
+        mock_redis_cls.return_value = mock_client
+
+        wait_for_redis = _import_wait_for_redis()
+        result = wait_for_redis(max_retries=2, retry_interval=0)
+
+        self.assertFalse(result)
+
+    @patch('wait_for_redis.redis.Redis')
+    def test_unexpected_error_returns_false(self, mock_redis_cls):
+        """Generic exceptions should return False immediately."""
+        mock_client = MagicMock()
+        mock_client.ping.side_effect = RuntimeError("unexpected")
+        mock_redis_cls.return_value = mock_client
+
+        wait_for_redis = _import_wait_for_redis()
+        result = wait_for_redis(max_retries=5, retry_interval=0)
+
+        self.assertFalse(result)