fix(monitoring): repair silent-false-positive update badge + auto-update scheduling
Customer container status checks looked up containers by an exact expected name. When a docker compose recreate got interrupted (e.g. a hung command previously killed the whole update-all batch on timeout), Compose could leave the old container renamed with a random hash prefix instead of removed. The exact-name lookup then found nothing, returned None, and that silently counted as "up to date" (green "Aktuell") instead of surfacing as unknown — affecting 5 customers on the appliance whose containers were actually still running under orphaned names. - _run_cmd no longer raises on subprocess timeout, so one stuck customer can't abort the rest of a batch update - repair_container_naming() self-heals orphaned hash-renamed containers by renaming them back before every status check and before recreate - update-all loop now catches per-customer exceptions instead of aborting - status responses expose "unknown" separately from "needs_update" so the UI shows a distinct grey badge instead of a false-positive green one - new settings: automatic daily update check (on/off + time), with an independent toggle for whether it also auto-recreates customer containers
This commit is contained in:
@@ -122,6 +122,11 @@ def _run_migrations() -> None:
|
||||
("system_config", "git_repo_url", "TEXT"),
|
||||
("system_config", "git_branch", "TEXT DEFAULT 'main'"),
|
||||
("system_config", "git_token_encrypted", "TEXT"),
|
||||
# Automatic NetBird image update check/apply
|
||||
("system_config", "auto_update_check_enabled", "BOOLEAN DEFAULT 0"),
|
||||
("system_config", "auto_update_check_time", "TEXT DEFAULT '03:00'"),
|
||||
("system_config", "auto_update_apply_enabled", "BOOLEAN DEFAULT 0"),
|
||||
("system_config", "auto_update_last_run_at", "TEXT"),
|
||||
]
|
||||
for table, column, col_type in migrations:
|
||||
if not _has_column(table, column):
|
||||
|
||||
@@ -13,6 +13,7 @@ from slowapi.errors import RateLimitExceeded
|
||||
from app.database import init_db
|
||||
from app.limiter import limiter
|
||||
from app.routers import auth, customers, deployments, monitoring, settings, users
|
||||
from app.services import scheduler_service
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Logging
|
||||
@@ -140,3 +141,10 @@ async def startup_event():
|
||||
logger.info("Starting NetBird MSP Appliance...")
|
||||
init_db()
|
||||
logger.info("Database initialized.")
|
||||
scheduler_service.start()
|
||||
|
||||
|
||||
@app.on_event("shutdown")
|
||||
async def shutdown_event():
|
||||
"""Stop background tasks on shutdown."""
|
||||
scheduler_service.stop()
|
||||
|
||||
@@ -199,6 +199,12 @@ class SystemConfig(Base):
|
||||
git_branch: Mapped[Optional[str]] = mapped_column(String(100), default="main")
|
||||
git_token_encrypted: Mapped[Optional[str]] = mapped_column(Text, nullable=True)
|
||||
|
||||
# Automatic NetBird image update check/apply
|
||||
auto_update_check_enabled: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||
auto_update_check_time: Mapped[Optional[str]] = mapped_column(String(5), default="03:00")
|
||||
auto_update_apply_enabled: Mapped[bool] = mapped_column(Boolean, default=False)
|
||||
auto_update_last_run_at: Mapped[Optional[datetime]] = mapped_column(DateTime, nullable=True)
|
||||
|
||||
created_at: Mapped[datetime] = mapped_column(DateTime, default=datetime.utcnow)
|
||||
updated_at: Mapped[datetime] = mapped_column(
|
||||
DateTime, default=datetime.utcnow, onupdate=datetime.utcnow
|
||||
@@ -253,6 +259,12 @@ class SystemConfig(Base):
|
||||
"git_repo_url": self.git_repo_url or "",
|
||||
"git_branch": self.git_branch or "main",
|
||||
"git_token_set": bool(self.git_token_encrypted),
|
||||
"auto_update_check_enabled": bool(self.auto_update_check_enabled),
|
||||
"auto_update_check_time": self.auto_update_check_time or "03:00",
|
||||
"auto_update_apply_enabled": bool(self.auto_update_apply_enabled),
|
||||
"auto_update_last_run_at": (
|
||||
self.auto_update_last_run_at.isoformat() if self.auto_update_last_run_at else None
|
||||
),
|
||||
"created_at": self.created_at.isoformat() if self.created_at else None,
|
||||
"updated_at": self.updated_at.isoformat() if self.updated_at else None,
|
||||
}
|
||||
|
||||
@@ -159,6 +159,7 @@ async def check_image_updates(
|
||||
"subdomain": customer.subdomain,
|
||||
"container_prefix": dep.container_prefix,
|
||||
"needs_update": cs["needs_update"],
|
||||
"unknown": cs.get("unknown", False),
|
||||
"services": cs["services"],
|
||||
})
|
||||
|
||||
@@ -231,7 +232,7 @@ async def customers_local_update_status(
|
||||
|
||||
async def _check(dep: Deployment) -> dict[str, Any]:
|
||||
cs = await image_service.get_customer_container_image_status_async(dep.container_prefix, config)
|
||||
return {"customer_id": dep.customer_id, "needs_update": cs["needs_update"]}
|
||||
return {"customer_id": dep.customer_id, "needs_update": cs["needs_update"], "unknown": cs.get("unknown", False)}
|
||||
|
||||
results = await asyncio.gather(*[_check(dep) for dep in deployments])
|
||||
results = list(results)
|
||||
@@ -274,19 +275,27 @@ async def update_all_customers(
|
||||
if not to_update:
|
||||
return {"message": "All customers are already up to date.", "updated": 0, "results": []}
|
||||
|
||||
# Update customers sequentially — one at a time
|
||||
# Update customers sequentially — one at a time. A failure for one
|
||||
# customer (e.g. a hung docker compose call) must not abort the rest of
|
||||
# the batch, otherwise later customers silently never get updated.
|
||||
update_results = []
|
||||
for entry in to_update:
|
||||
res = await image_service.update_customer_containers(
|
||||
entry["instance_dir"], entry["project_name"]
|
||||
)
|
||||
ok = res["success"]
|
||||
logger.info("Updated %s: %s", entry["project_name"], "OK" if ok else res.get("error"))
|
||||
try:
|
||||
res = await image_service.update_customer_containers(
|
||||
entry["instance_dir"], entry["project_name"]
|
||||
)
|
||||
ok = res["success"]
|
||||
error = res.get("error")
|
||||
except Exception as exc:
|
||||
logger.exception("Unexpected error updating %s", entry["project_name"])
|
||||
ok = False
|
||||
error = str(exc)
|
||||
logger.info("Updated %s: %s", entry["project_name"], "OK" if ok else error)
|
||||
update_results.append({
|
||||
"customer_name": entry["customer_name"],
|
||||
"customer_id": entry["customer_id"],
|
||||
"success": ok,
|
||||
"error": res.get("error"),
|
||||
"error": error,
|
||||
})
|
||||
|
||||
success_count = sum(1 for r in update_results if r["success"])
|
||||
|
||||
@@ -19,13 +19,38 @@ logger = logging.getLogger(__name__)
|
||||
NETBIRD_SERVICES = ["management", "signal", "relay", "dashboard"]
|
||||
|
||||
|
||||
class _TimeoutResult:
|
||||
"""Stand-in for subprocess.CompletedProcess when a command times out.
|
||||
|
||||
A hung `docker compose up -d` used to raise TimeoutExpired straight out of
|
||||
_run_cmd, which killed the whole update-all loop mid-recreate and left the
|
||||
old container renamed-but-not-removed (orphaned with a hash-prefixed name).
|
||||
Returning a failed result instead lets callers handle it gracefully and
|
||||
keeps the batch loop going for the remaining customers.
|
||||
"""
|
||||
|
||||
def __init__(self, cmd: list[str], timeout: int):
|
||||
self.returncode = -1
|
||||
self.stdout = ""
|
||||
self.stderr = f"Command timed out after {timeout}s: {' '.join(cmd)}"
|
||||
|
||||
|
||||
async def _run_cmd(cmd: list[str], timeout: int = 300) -> subprocess.CompletedProcess:
|
||||
"""Run a subprocess command without blocking the event loop."""
|
||||
"""Run a subprocess command without blocking the event loop.
|
||||
|
||||
Never raises on timeout — returns a failed CompletedProcess-like result
|
||||
instead, so a single hung docker/compose call can't abort a batch of
|
||||
otherwise-independent operations (e.g. updating multiple customers).
|
||||
"""
|
||||
loop = asyncio.get_event_loop()
|
||||
return await loop.run_in_executor(
|
||||
None,
|
||||
lambda: subprocess.run(cmd, capture_output=True, text=True, timeout=timeout),
|
||||
)
|
||||
try:
|
||||
return await loop.run_in_executor(
|
||||
None,
|
||||
lambda: subprocess.run(cmd, capture_output=True, text=True, timeout=timeout),
|
||||
)
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.error("Command timed out after %ds: %s", timeout, " ".join(cmd))
|
||||
return _TimeoutResult(cmd, timeout)
|
||||
|
||||
|
||||
def _parse_image_name(image: str) -> tuple[str, str]:
|
||||
@@ -123,6 +148,60 @@ def get_container_image_id(container_name: str) -> str | None:
|
||||
return None
|
||||
|
||||
|
||||
def repair_container_naming(container_prefix: str, services: list[str] = NETBIRD_SERVICES) -> list[str]:
|
||||
"""Rename orphaned containers back to their expected compose name.
|
||||
|
||||
When a `docker compose up -d` is interrupted mid-recreate (e.g. a timeout
|
||||
killing the process), Compose can leave the *old* container renamed with a
|
||||
random hash prefix (e.g. "4e45e71fcb7b_netbird-acme-management") instead
|
||||
of removing it, while never creating the correctly-named replacement. The
|
||||
container itself keeps running fine — it's just invisible to every lookup
|
||||
that expects the exact name, which used to silently read as "no container
|
||||
found" and get reported as "up to date" instead of "unknown".
|
||||
|
||||
This finds any such orphan (a container whose name *contains* the expected
|
||||
name but isn't an exact match) and, only when no container already holds
|
||||
the exact expected name, renames it back. Safe no-op otherwise.
|
||||
|
||||
Returns the list of service names that were repaired.
|
||||
"""
|
||||
repaired = []
|
||||
for svc in services:
|
||||
expected_name = f"{container_prefix}-{svc}"
|
||||
exact = subprocess.run(
|
||||
["docker", "inspect", expected_name, "--format", "{{.Id}}"],
|
||||
capture_output=True, text=True, timeout=10,
|
||||
)
|
||||
if exact.returncode == 0:
|
||||
continue # already correctly named
|
||||
|
||||
found = subprocess.run(
|
||||
["docker", "ps", "-a", "--filter", f"name={expected_name}", "--format", "{{.Names}}"],
|
||||
capture_output=True, text=True, timeout=10,
|
||||
)
|
||||
candidates = [n for n in found.stdout.strip().splitlines() if n and n != expected_name]
|
||||
if not candidates:
|
||||
continue # container genuinely doesn't exist (not deployed / not running)
|
||||
|
||||
orphan = candidates[0]
|
||||
rename = subprocess.run(
|
||||
["docker", "rename", orphan, expected_name],
|
||||
capture_output=True, text=True, timeout=10,
|
||||
)
|
||||
if rename.returncode == 0:
|
||||
logger.warning(
|
||||
"Repaired orphaned container naming for %s: '%s' -> '%s'",
|
||||
container_prefix, orphan, expected_name,
|
||||
)
|
||||
repaired.append(svc)
|
||||
else:
|
||||
logger.error(
|
||||
"Failed to repair orphaned container '%s' -> '%s': %s",
|
||||
orphan, expected_name, rename.stderr,
|
||||
)
|
||||
return repaired
|
||||
|
||||
|
||||
def get_local_image_id(image: str) -> str | None:
|
||||
"""Get the full image ID (sha256:...) of a locally stored image."""
|
||||
try:
|
||||
@@ -231,6 +310,13 @@ async def get_customer_container_image_status_async(container_prefix: str, confi
|
||||
}
|
||||
loop = asyncio.get_event_loop()
|
||||
|
||||
# Self-heal any container left orphaned under a hash-prefixed name by a
|
||||
# previously interrupted recreate, so the lookups below find it by its
|
||||
# real, expected name instead of silently returning "not found".
|
||||
await loop.run_in_executor(
|
||||
None, repair_container_naming, container_prefix, list(service_images.keys())
|
||||
)
|
||||
|
||||
async def _check(svc: str, image: str) -> tuple[str, dict[str, Any]]:
|
||||
container_name = f"{container_prefix}-{svc}"
|
||||
container_id, local_id = await asyncio.gather(
|
||||
@@ -246,7 +332,8 @@ async def get_customer_container_image_status_async(container_prefix: str, confi
|
||||
pairs = await asyncio.gather(*[_check(svc, image) for svc, image in service_images.items()])
|
||||
services = dict(pairs)
|
||||
needs_update = any(s["up_to_date"] is False for s in services.values())
|
||||
return {"services": services, "needs_update": needs_update}
|
||||
unknown = any(s["up_to_date"] is None for s in services.values())
|
||||
return {"services": services, "needs_update": needs_update, "unknown": unknown}
|
||||
|
||||
|
||||
def get_customer_container_image_status(container_prefix: str, config) -> dict[str, Any]:
|
||||
@@ -265,6 +352,11 @@ def get_customer_container_image_status(container_prefix: str, config) -> dict[s
|
||||
"relay": config.netbird_relay_image,
|
||||
"dashboard": config.netbird_dashboard_image,
|
||||
}
|
||||
|
||||
# Self-heal any container left orphaned under a hash-prefixed name by a
|
||||
# previously interrupted recreate (see repair_container_naming docstring).
|
||||
repair_container_naming(container_prefix, list(service_images.keys()))
|
||||
|
||||
services: dict[str, Any] = {}
|
||||
for svc, image in service_images.items():
|
||||
container_name = f"{container_prefix}-{svc}"
|
||||
@@ -280,7 +372,8 @@ def get_customer_container_image_status(container_prefix: str, config) -> dict[s
|
||||
"up_to_date": up_to_date,
|
||||
}
|
||||
needs_update = any(s["up_to_date"] is False for s in services.values())
|
||||
return {"services": services, "needs_update": needs_update}
|
||||
unknown = any(s["up_to_date"] is None for s in services.values())
|
||||
return {"services": services, "needs_update": needs_update, "unknown": unknown}
|
||||
|
||||
|
||||
async def update_customer_containers(instance_dir: str, project_name: str) -> dict[str, Any]:
|
||||
@@ -292,6 +385,13 @@ async def update_customer_containers(instance_dir: str, project_name: str) -> di
|
||||
compose_file = os.path.join(instance_dir, "docker-compose.yml")
|
||||
if not os.path.isfile(compose_file):
|
||||
return {"success": False, "error": f"docker-compose.yml not found at {compose_file}"}
|
||||
|
||||
# Repair any container still orphaned under a hash-prefixed name from a
|
||||
# previous interrupted recreate before Compose tries to touch it again —
|
||||
# otherwise Compose keeps colliding with the same stuck rename.
|
||||
loop = asyncio.get_event_loop()
|
||||
await loop.run_in_executor(None, repair_container_naming, project_name)
|
||||
|
||||
cmd = [
|
||||
"docker", "compose",
|
||||
"-f", compose_file,
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
"""Background scheduler for automatic NetBird image update checks.
|
||||
|
||||
No external scheduler dependency (APScheduler etc.) — a single asyncio task
|
||||
started at app startup wakes up once a minute, and only actually does
|
||||
anything once per day at the configured HH:MM, controlled entirely by
|
||||
SystemConfig.auto_update_check_enabled / auto_update_check_time.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
from datetime import datetime
|
||||
|
||||
from app.database import SessionLocal
|
||||
from app.models import Deployment, SystemConfig
|
||||
from app.services import image_service
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_POLL_INTERVAL_SECONDS = 60
|
||||
_task: asyncio.Task | None = None
|
||||
|
||||
|
||||
def start() -> None:
|
||||
"""Start the background polling task. Safe to call once at app startup."""
|
||||
global _task
|
||||
if _task is None or _task.done():
|
||||
_task = asyncio.create_task(_poll_loop())
|
||||
logger.info("Automatic update scheduler started.")
|
||||
|
||||
|
||||
def stop() -> None:
|
||||
"""Cancel the background polling task."""
|
||||
global _task
|
||||
if _task is not None:
|
||||
_task.cancel()
|
||||
_task = None
|
||||
|
||||
|
||||
async def _poll_loop() -> None:
|
||||
while True:
|
||||
try:
|
||||
await _tick()
|
||||
except Exception:
|
||||
logger.exception("Scheduler tick failed")
|
||||
await asyncio.sleep(_POLL_INTERVAL_SECONDS)
|
||||
|
||||
|
||||
async def _tick() -> None:
|
||||
db = SessionLocal()
|
||||
try:
|
||||
config = db.query(SystemConfig).filter(SystemConfig.id == 1).first()
|
||||
if not config or not config.auto_update_check_enabled:
|
||||
return
|
||||
|
||||
now = datetime.now()
|
||||
target_time = config.auto_update_check_time or "03:00"
|
||||
current_hhmm = now.strftime("%H:%M")
|
||||
if current_hhmm != target_time:
|
||||
return
|
||||
|
||||
last_run = config.auto_update_last_run_at
|
||||
if last_run and last_run.date() == now.date():
|
||||
return # already ran today
|
||||
|
||||
# Claim this run immediately so a slow run can't overlap the next tick.
|
||||
config.auto_update_last_run_at = now
|
||||
db.commit()
|
||||
|
||||
apply_enabled = bool(config.auto_update_apply_enabled)
|
||||
logger.info(
|
||||
"Running scheduled NetBird image update check (auto-apply=%s)...", apply_enabled
|
||||
)
|
||||
await _run_check_and_optionally_apply(config, apply_enabled)
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
async def _run_check_and_optionally_apply(config: SystemConfig, apply_enabled: bool) -> None:
|
||||
hub_status = await image_service.check_all_images(config)
|
||||
if not hub_status["any_update_available"]:
|
||||
logger.info("Scheduled check: all NetBird images already up to date.")
|
||||
return
|
||||
|
||||
logger.info("Scheduled check: new NetBird image(s) available — pulling.")
|
||||
pull_result = await image_service.pull_all_images(config)
|
||||
if not pull_result["all_success"]:
|
||||
logger.error("Scheduled image pull had failures: %s", pull_result["results"])
|
||||
|
||||
if not apply_enabled:
|
||||
logger.info("Auto-apply disabled — images pulled, customer containers left untouched.")
|
||||
return
|
||||
|
||||
db = SessionLocal()
|
||||
try:
|
||||
deployments = db.query(Deployment).all()
|
||||
to_update = []
|
||||
for dep in deployments:
|
||||
cs = image_service.get_customer_container_image_status(dep.container_prefix, config)
|
||||
if cs["needs_update"]:
|
||||
customer = dep.customer
|
||||
to_update.append({
|
||||
"instance_dir": f"{config.data_dir}/{customer.subdomain}",
|
||||
"project_name": dep.container_prefix,
|
||||
"customer_name": customer.name,
|
||||
})
|
||||
logger.info("Scheduled auto-apply: updating %d customer(s)...", len(to_update))
|
||||
for entry in to_update:
|
||||
try:
|
||||
res = await image_service.update_customer_containers(
|
||||
entry["instance_dir"], entry["project_name"]
|
||||
)
|
||||
logger.info(
|
||||
"Scheduled update for %s: %s",
|
||||
entry["customer_name"], "OK" if res["success"] else res.get("error"),
|
||||
)
|
||||
except Exception:
|
||||
logger.exception("Scheduled update failed for %s", entry["customer_name"])
|
||||
finally:
|
||||
db.close()
|
||||
@@ -158,6 +158,21 @@ class SystemConfigUpdate(BaseModel):
|
||||
git_repo_url: Optional[str] = Field(None, max_length=500)
|
||||
git_branch: Optional[str] = Field(None, max_length=100)
|
||||
git_token: Optional[str] = None # plaintext, encrypted before storage
|
||||
# Automatic NetBird image update check/apply
|
||||
auto_update_check_enabled: Optional[bool] = None
|
||||
auto_update_check_time: Optional[str] = Field(None, max_length=5)
|
||||
auto_update_apply_enabled: Optional[bool] = None
|
||||
|
||||
@field_validator("auto_update_check_time")
|
||||
@classmethod
|
||||
def validate_auto_update_check_time(cls, v: Optional[str]) -> Optional[str]:
|
||||
"""Must be HH:MM in 24h format."""
|
||||
if v is None:
|
||||
return v
|
||||
import re
|
||||
if not re.fullmatch(r"([01]\d|2[0-3]):[0-5]\d", v):
|
||||
raise ValueError("auto_update_check_time must be in HH:MM 24h format")
|
||||
return v
|
||||
|
||||
@field_validator("ssl_mode")
|
||||
@classmethod
|
||||
|
||||
Reference in New Issue
Block a user