mirror of
https://github.com/blakeblackshear/frigate.git
synced 2026-07-29 07:09:03 +03:00
fix watchdog process restarts reverting to the boot config
/api/config/set parses a new FrigateConfig and swaps the API and dispatcher onto it, but FrigateApp.config was never rebound, so the watchdog factories rebuilt a crashed process from the config as of startup. Fix is to read through a ConfigHolder that the swap updates.
This commit is contained in:
@@ -16,6 +16,10 @@ def swap_runtime_config(app: FastAPI, config: FrigateConfig) -> None:
|
||||
camera the user turned off would silently come back on.
|
||||
"""
|
||||
app.frigate_config = config
|
||||
|
||||
if app.config_holder is not None:
|
||||
app.config_holder.set(config)
|
||||
|
||||
app.genai_manager.update_config(config)
|
||||
|
||||
if app.profile_manager is not None:
|
||||
|
||||
@@ -35,6 +35,7 @@ from frigate.comms.event_metadata_updater import (
|
||||
)
|
||||
from frigate.config import FrigateConfig
|
||||
from frigate.config.camera.updater import CameraConfigUpdatePublisher
|
||||
from frigate.config.holder import ConfigHolder
|
||||
from frigate.config.profile_manager import ProfileManager
|
||||
from frigate.debug_replay import DebugReplayManager, debug_replay_auto_stop_watchdog
|
||||
from frigate.embeddings import EmbeddingsContext
|
||||
@@ -74,6 +75,7 @@ def create_fastapi_app(
|
||||
dispatcher: Dispatcher | None = None,
|
||||
profile_manager: ProfileManager | None = None,
|
||||
enforce_default_admin: bool = True,
|
||||
config_holder: ConfigHolder | None = None,
|
||||
):
|
||||
logger.info("Starting FastAPI app")
|
||||
app = FastAPI(
|
||||
@@ -162,6 +164,7 @@ def create_fastapi_app(
|
||||
app.replay_manager = replay_manager
|
||||
app.dispatcher = dispatcher
|
||||
app.profile_manager = profile_manager
|
||||
app.config_holder = config_holder
|
||||
|
||||
if frigate_config.auth.enabled:
|
||||
secret = get_jwt_secret()
|
||||
|
||||
+14
-1
@@ -30,6 +30,7 @@ from frigate.comms.ws import WebSocketClient
|
||||
from frigate.comms.zmq_proxy import ZmqProxy
|
||||
from frigate.config.camera.updater import CameraConfigUpdatePublisher
|
||||
from frigate.config.config import FrigateConfig
|
||||
from frigate.config.holder import ConfigHolder
|
||||
from frigate.config.profile_manager import ProfileManager
|
||||
from frigate.const import (
|
||||
CACHE_DIR,
|
||||
@@ -122,7 +123,18 @@ class FrigateApp:
|
||||
self.processes: dict[str, int] = {}
|
||||
self.embeddings: EmbeddingsContext | None = None
|
||||
self.profile_manager: ProfileManager | None = None
|
||||
self.config = config
|
||||
self.config_holder = ConfigHolder(config)
|
||||
|
||||
@property
|
||||
def config(self) -> FrigateConfig:
|
||||
"""The current config, not the one Frigate booted with.
|
||||
|
||||
Read through the holder so the deferred watchdog factories below build
|
||||
a replacement process from the config as it is now. There is no setter
|
||||
on purpose: a plain attribute would let a caller pin this back to a
|
||||
single object and reintroduce the staleness.
|
||||
"""
|
||||
return self.config_holder.config
|
||||
|
||||
def ensure_dirs(self) -> None:
|
||||
dirs = [
|
||||
@@ -645,6 +657,7 @@ class FrigateApp:
|
||||
self.replay_manager,
|
||||
self.dispatcher,
|
||||
self.profile_manager,
|
||||
config_holder=self.config_holder,
|
||||
),
|
||||
host="127.0.0.1",
|
||||
port=5001,
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
"""Shared handle on the config object that is current for this instance."""
|
||||
|
||||
from .config import FrigateConfig
|
||||
|
||||
__all__ = ["ConfigHolder"]
|
||||
|
||||
|
||||
class ConfigHolder:
|
||||
"""Indirection for the most recently parsed config.
|
||||
|
||||
/api/config/set re-parses yaml into a brand new FrigateConfig instead of
|
||||
mutating the old one, so any reference captured during startup goes stale
|
||||
the first time a user saves. Anything that has to build something after
|
||||
startup, most importantly the watchdog factories that rebuild a crashed
|
||||
process, must read through a holder rather than close over a config
|
||||
object, or the rebuilt process comes back with the config as it was at
|
||||
boot and silently discards every change made since.
|
||||
|
||||
There is deliberately no setter on the read side: the swap runs in exactly
|
||||
one place (frigate.api.config_util.swap_runtime_config) and everyone else
|
||||
only reads.
|
||||
"""
|
||||
|
||||
def __init__(self, config: FrigateConfig) -> None:
|
||||
self._config = config
|
||||
|
||||
@property
|
||||
def config(self) -> FrigateConfig:
|
||||
"""The config as of the most recent successful save."""
|
||||
return self._config
|
||||
|
||||
def set(self, config: FrigateConfig) -> None:
|
||||
"""Install a freshly parsed config as the current one."""
|
||||
self._config = config
|
||||
@@ -13,6 +13,7 @@ from frigate.config.camera.updater import (
|
||||
CameraConfigUpdatePublisher,
|
||||
CameraConfigUpdateTopic,
|
||||
)
|
||||
from frigate.config.holder import ConfigHolder
|
||||
from frigate.models import Event, Recordings, ReviewSegment
|
||||
from frigate.test.http_api.base_http_test import AuthTestClient, BaseTestHttp
|
||||
|
||||
@@ -373,6 +374,72 @@ class TestConfigSetWildcardPropagation(BaseTestHttp):
|
||||
finally:
|
||||
os.unlink(config_path)
|
||||
|
||||
@patch("frigate.api.app.find_config_file")
|
||||
def test_save_updates_the_config_holder(self, mock_find_config):
|
||||
"""A save must move the holder onto the freshly parsed config.
|
||||
|
||||
FrigateApp reads the holder when the watchdog rebuilds a crashed
|
||||
process; if the save leaves it on the boot config, that process comes
|
||||
back having lost every change made since Frigate started.
|
||||
"""
|
||||
from fastapi import Request
|
||||
|
||||
from frigate.api.auth import get_allowed_cameras_for_filter, get_current_user
|
||||
from frigate.api.fastapi_app import create_fastapi_app
|
||||
|
||||
config_path = self._write_config_file()
|
||||
mock_find_config.return_value = config_path
|
||||
|
||||
mock_publisher = Mock(spec=CameraConfigUpdatePublisher)
|
||||
mock_publisher.publisher = MagicMock()
|
||||
boot_config = FrigateConfig(**self.minimal_config)
|
||||
holder = ConfigHolder(boot_config)
|
||||
|
||||
try:
|
||||
app = create_fastapi_app(
|
||||
boot_config,
|
||||
self.db,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
mock_publisher,
|
||||
None,
|
||||
enforce_default_admin=False,
|
||||
config_holder=holder,
|
||||
)
|
||||
|
||||
async def mock_get_current_user(request: Request):
|
||||
return {"username": "admin", "role": "admin"}
|
||||
|
||||
async def mock_get_allowed_cameras_for_filter(request: Request):
|
||||
return list(self.minimal_config.get("cameras", {}).keys())
|
||||
|
||||
app.dependency_overrides[get_current_user] = mock_get_current_user
|
||||
app.dependency_overrides[get_allowed_cameras_for_filter] = (
|
||||
mock_get_allowed_cameras_for_filter
|
||||
)
|
||||
|
||||
with AuthTestClient(app) as client:
|
||||
resp = client.put(
|
||||
"/config/set",
|
||||
json={
|
||||
"config_data": {"birdseye": {"inactivity_threshold": 5}},
|
||||
"update_topic": "config/birdseye",
|
||||
"requires_restart": 0,
|
||||
},
|
||||
)
|
||||
|
||||
self.assertEqual(resp.status_code, 200)
|
||||
|
||||
self.assertIsNot(holder.config, boot_config)
|
||||
self.assertIs(holder.config, app.frigate_config)
|
||||
self.assertEqual(holder.config.birdseye.inactivity_threshold, 5)
|
||||
finally:
|
||||
os.unlink(config_path)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -4,6 +4,7 @@ import unittest
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from frigate.api.config_util import swap_runtime_config
|
||||
from frigate.config.holder import ConfigHolder
|
||||
|
||||
|
||||
class TestSwapRuntimeConfig(unittest.TestCase):
|
||||
@@ -12,6 +13,7 @@ class TestSwapRuntimeConfig(unittest.TestCase):
|
||||
def _make_app(self) -> MagicMock:
|
||||
app = MagicMock()
|
||||
app.dispatcher.comms = [MagicMock(), MagicMock()]
|
||||
app.config_holder = ConfigHolder(MagicMock(name="boot_config"))
|
||||
return app
|
||||
|
||||
def test_rebinds_all_references(self) -> None:
|
||||
@@ -37,11 +39,40 @@ class TestSwapRuntimeConfig(unittest.TestCase):
|
||||
# the swap rebuilds cameras from yaml, so overrides must be re-layered
|
||||
app.dispatcher.reapply_runtime_state_to_config.assert_called_once_with()
|
||||
|
||||
def test_updates_the_config_holder(self) -> None:
|
||||
app = self._make_app()
|
||||
holder = app.config_holder
|
||||
config = MagicMock(name="new_config")
|
||||
|
||||
swap_runtime_config(app, config)
|
||||
|
||||
self.assertIs(holder.config, config)
|
||||
|
||||
def test_deferred_factory_builds_from_the_swapped_config(self) -> None:
|
||||
"""A watchdog-style factory must not rebuild from the boot config.
|
||||
|
||||
The factories in FrigateApp are lambdas evaluated when a process is
|
||||
restarted, long after a user may have saved. Reading through the
|
||||
holder is what keeps a rebuilt process from reverting every change
|
||||
made since Frigate started.
|
||||
"""
|
||||
app = self._make_app()
|
||||
holder = app.config_holder
|
||||
boot_config = holder.config
|
||||
factory = lambda: holder.config # noqa: E731
|
||||
self.assertIs(factory(), boot_config)
|
||||
|
||||
config = MagicMock(name="new_config")
|
||||
swap_runtime_config(app, config)
|
||||
|
||||
self.assertIs(factory(), config)
|
||||
|
||||
def test_tolerates_missing_optional_collaborators(self) -> None:
|
||||
app = MagicMock()
|
||||
app.profile_manager = None
|
||||
app.stats_emitter = None
|
||||
app.dispatcher = None
|
||||
app.config_holder = None
|
||||
config = MagicMock(name="new_config")
|
||||
|
||||
# must not raise when the optional collaborators are absent
|
||||
|
||||
Reference in New Issue
Block a user