fix(core): пункты 5-10 аудита — UPID-синк, порт, бэкап all, IPAM, креды
5. UPID-синк: транзитная ошибка опроса статуса больше не помечает живую
задачу failed — get_task_status(raise_on_error=True) + счётчик из 3
последовательных неудач; в exit_status пишется реальная причина.
6. Дашборд и бэкапы для серверов на порту != 8006: metrics_broadcaster и
backup_scheduler строят клиент через ProxmoxClient.from_server (учёт
порта и token/password) вместо ручной сборки по ip_address.
7. Бэкап "все VM": create_backup при vmid in (None, 0, "all") передаёт
vzdump параметр all=1 — раньше уходил vmid=0 и PVE отвергал задание.
8. IPAM: get_next_available_ip собирает занятые IP глобально (как проверяет
allocate_ip), а не только по pool_id — авто-выдача больше не предлагает
занятый адрес и не срывает создание инстанса.
9. IPAM: fallback в release_ip_by_vmid ищет только записи без
proxmox_server_id — удаление VM не освобождает IP одноимённого vmid
на другом сервере.
10. _get_proxmox_client: сервер без валидных кредов теперь возвращает
понятную 503-ошибку вместо молчаливо пустых списков; перечисление
серверов в метриках защищено per-server try/except.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -74,7 +74,22 @@ def require_vm_access(db: Session, current_user: User, server_id: int, vmid: int
|
||||
|
||||
|
||||
def _get_proxmox_client(server: ProxmoxServer) -> ProxmoxClient:
|
||||
"""Build a ProxmoxClient from a ProxmoxServer model (password or token auth)."""
|
||||
"""Build a ProxmoxClient from a ProxmoxServer model (password or token auth).
|
||||
|
||||
Raises HTTPException 503, если у сервера нет валидных кредов: раньше
|
||||
from_server молча создавал клиент с proxmox=None, и все методы возвращали
|
||||
пустые списки — пользователь видел «пустой сервер» вместо ошибки настройки.
|
||||
"""
|
||||
has_password = bool(getattr(server, "use_password", False) and getattr(server, "password", ""))
|
||||
has_token = bool(getattr(server, "api_token_name", "") and getattr(server, "api_token_value", ""))
|
||||
if not (has_password or has_token):
|
||||
raise HTTPException(
|
||||
status_code=503,
|
||||
detail=(
|
||||
f"Server '{server.name}' has no valid Proxmox credentials configured "
|
||||
f"(API token or password). Edit the server settings."
|
||||
),
|
||||
)
|
||||
return ProxmoxClient.from_server(server)
|
||||
|
||||
|
||||
|
||||
@@ -186,26 +186,27 @@ class IPAMService:
|
||||
# Also exclude gateway
|
||||
gateway_ip = network.gateway if network.gateway else None
|
||||
|
||||
# Search through pools for available IP — query only per-pool allocated IPs
|
||||
# Занятые IP собираем ГЛОБАЛЬНО (без фильтра по pool_id/статусу) —
|
||||
# ровно так же проверяет allocate_ip (ip_address уникален на всю БД).
|
||||
# Фильтр по pool_id пропускал аллокации с pool_id=NULL или из
|
||||
# пересекающегося пула: сервис предлагал занятый IP, allocate_ip
|
||||
# отвечал "already allocated", и создание инстанса срывалось,
|
||||
# хотя свободные адреса были.
|
||||
occupied = set(
|
||||
row[0] for row in self.db.query(IPAMAllocation.ip_address).all()
|
||||
)
|
||||
if gateway_ip:
|
||||
occupied.add(gateway_ip)
|
||||
|
||||
# Search through pools for available IP
|
||||
for pool in pools:
|
||||
try:
|
||||
start_ip = ipaddress.ip_address(pool.range_start)
|
||||
end_ip = ipaddress.ip_address(pool.range_end)
|
||||
|
||||
# Load only allocated IPs for this specific pool (much smaller set)
|
||||
pool_allocated = set(
|
||||
a.ip_address for a in
|
||||
self.db.query(IPAMAllocation.ip_address).filter(
|
||||
IPAMAllocation.pool_id == pool.id,
|
||||
IPAMAllocation.status.in_(['allocated', 'reserved'])
|
||||
).all()
|
||||
)
|
||||
if gateway_ip:
|
||||
pool_allocated.add(gateway_ip)
|
||||
|
||||
for ip_int in range(int(start_ip), int(end_ip) + 1):
|
||||
ip = str(ipaddress.ip_address(ip_int))
|
||||
if ip not in pool_allocated:
|
||||
if ip not in occupied:
|
||||
logger.info(f"Found available IP: {ip} in pool {pool.name}")
|
||||
return ip
|
||||
|
||||
@@ -450,10 +451,14 @@ class IPAMService:
|
||||
# First try to find by proxmox_server_id and proxmox_vmid
|
||||
allocation = self.find_allocation_by_resource(proxmox_server_id, proxmox_vmid)
|
||||
|
||||
# Fallback: search by resource_id if proxmox fields are not set
|
||||
# Fallback: search by resource_id if proxmox fields are not set.
|
||||
# ВАЖНО: только среди записей БЕЗ proxmox_server_id — иначе удаление
|
||||
# VM с vmid=100 на сервере A освобождало IP виртуалки с тем же vmid
|
||||
# на сервере B (записи с заполненным server_id находит основной поиск).
|
||||
if not allocation:
|
||||
allocation = self.db.query(IPAMAllocation).filter(
|
||||
IPAMAllocation.resource_id == proxmox_vmid,
|
||||
IPAMAllocation.proxmox_server_id.is_(None),
|
||||
IPAMAllocation.status.in_(['allocated', 'reserved'])
|
||||
).first()
|
||||
if allocation:
|
||||
|
||||
@@ -1187,14 +1187,24 @@ class ProxmoxClient(VmMixin, LxcMixin, ClusterMixin, StorageMixin, NetworkMixin,
|
||||
except Exception as e:
|
||||
return str(e)
|
||||
|
||||
def get_task_status(self, node: str, upid: str) -> Dict:
|
||||
"""Get status of a Proxmox task by UPID"""
|
||||
def get_task_status(self, node: str, upid: str, raise_on_error: bool = False) -> Dict:
|
||||
"""Get status of a Proxmox task by UPID.
|
||||
|
||||
raise_on_error=True — пробросить исключение вместо возврата {}.
|
||||
Нужно вызывающим, которым важно отличать «задача не найдена» от
|
||||
транзитной сетевой ошибки (см. run_upid_task_sync): пустой словарь
|
||||
эту разницу стирает.
|
||||
"""
|
||||
if not self.proxmox:
|
||||
if raise_on_error:
|
||||
raise RuntimeError(f"Proxmox client for {self.host} is not connected")
|
||||
return {}
|
||||
try:
|
||||
status = self.proxmox.nodes(node).tasks(upid).status.get()
|
||||
return status
|
||||
except Exception as e:
|
||||
if raise_on_error:
|
||||
raise
|
||||
logger.error(f"Error getting task status {upid}: {e}")
|
||||
return {}
|
||||
|
||||
@@ -2726,12 +2736,16 @@ class ProxmoxClient(VmMixin, LxcMixin, ClusterMixin, StorageMixin, NetworkMixin,
|
||||
return {"success": False, "error": "Not connected"}
|
||||
try:
|
||||
params = {
|
||||
"vmid": vmid,
|
||||
"storage": storage,
|
||||
"mode": mode,
|
||||
"compress": compress,
|
||||
"remove": int(bool(remove)),
|
||||
}
|
||||
# «Все VM»: vzdump требует all=1, а не vmid (vmid=0 — ошибка PVE)
|
||||
if vmid in (None, 0, "all"):
|
||||
params["all"] = 1
|
||||
else:
|
||||
params["vmid"] = vmid
|
||||
if keep_last and int(keep_last) > 0:
|
||||
params["prune-backups"] = f"keep-last={int(keep_last)}"
|
||||
if notes:
|
||||
|
||||
@@ -152,7 +152,9 @@ async def run_backup_job(job_id: int) -> None:
|
||||
try:
|
||||
result = client.create_backup(
|
||||
node=job.node,
|
||||
vmid=vmid if vmid != "all" else 0,
|
||||
# "all" пробрасываем как есть: create_backup превратит его в
|
||||
# параметр vzdump all=1 (vmid=0 PVE отвергает с ошибкой)
|
||||
vmid=vmid,
|
||||
storage=job.storage,
|
||||
mode=job.mode,
|
||||
compress=job.compress,
|
||||
@@ -196,21 +198,13 @@ async def run_backup_job(job_id: int) -> None:
|
||||
|
||||
|
||||
def _build_client(server: ProxmoxServer) -> ProxmoxClient:
|
||||
"""Build a ProxmoxClient from a ProxmoxServer model"""
|
||||
if server.use_password:
|
||||
return ProxmoxClient(
|
||||
host=server.ip_address,
|
||||
user=server.api_user,
|
||||
password=server.password,
|
||||
verify_ssl=server.verify_ssl,
|
||||
)
|
||||
return ProxmoxClient(
|
||||
host=server.ip_address,
|
||||
user=server.api_user,
|
||||
token_name=server.api_token_name,
|
||||
token_value=server.api_token_value,
|
||||
verify_ssl=server.verify_ssl,
|
||||
)
|
||||
"""Build a ProxmoxClient from a ProxmoxServer model.
|
||||
|
||||
Идём через from_server — он учитывает нестандартный порт (!= 8006) и
|
||||
корректно выбирает token/password. Ручная сборка по ip_address ломала
|
||||
бэкапы для серверов на кастомном порту.
|
||||
"""
|
||||
return ProxmoxClient.from_server(server)
|
||||
|
||||
|
||||
def _update_job_status(db, job: BackupJob, status: str,
|
||||
|
||||
@@ -97,7 +97,6 @@ def _fetch_server_resources_with_nodes(server) -> dict:
|
||||
Pull VMs + containers + per-node CPU/RAM/Disk status + cached RRD slices.
|
||||
Runs in a thread pool — safe to block.
|
||||
"""
|
||||
from ..proxmox import get_proxmox_resources
|
||||
from ..api.proxmox._helpers import _get_proxmox_client
|
||||
|
||||
base: dict = {
|
||||
@@ -111,23 +110,18 @@ def _fetch_server_resources_with_nodes(server) -> dict:
|
||||
"nodes": [],
|
||||
}
|
||||
|
||||
# Клиент строим через from_server — учитывает нестандартный порт (!= 8006)
|
||||
# и выбор token/password. Ручная сборка по ip_address ломала дашборд для
|
||||
# серверов на кастомном порту.
|
||||
try:
|
||||
client = _get_proxmox_client(server)
|
||||
except Exception as exc:
|
||||
base["error"] = str(exc)
|
||||
return _merge_server_cache(server.id, base)
|
||||
|
||||
# ── VMs + containers ──────────────────────────────────────────────────
|
||||
try:
|
||||
if server.use_password:
|
||||
resources = get_proxmox_resources(
|
||||
host=server.ip_address,
|
||||
user=server.api_user,
|
||||
password=server.password,
|
||||
verify_ssl=server.verify_ssl,
|
||||
)
|
||||
else:
|
||||
resources = get_proxmox_resources(
|
||||
host=server.ip_address,
|
||||
user=server.api_user,
|
||||
token_name=server.api_token_name,
|
||||
token_value=server.api_token_value,
|
||||
verify_ssl=server.verify_ssl,
|
||||
)
|
||||
resources = client.get_all_resources()
|
||||
vms = resources.get("vms", [])
|
||||
containers = resources.get("containers", [])
|
||||
base["vms"] = vms
|
||||
@@ -141,7 +135,6 @@ def _fetch_server_resources_with_nodes(server) -> dict:
|
||||
|
||||
# ── Node status + RRD mini-slices ─────────────────────────────────────
|
||||
try:
|
||||
client = _get_proxmox_client(server)
|
||||
if not client.is_connected():
|
||||
return _merge_server_cache(server.id, base)
|
||||
nodes_raw = client.get_nodes()
|
||||
@@ -457,8 +450,15 @@ def _enumerate_instances_for_list(db_factory):
|
||||
db = db_factory()
|
||||
try:
|
||||
servers = db.query(ProxmoxServer).filter(ProxmoxServer.is_online == True).all()
|
||||
# Build clients while the session is open (from_server reads server attrs)
|
||||
server_clients = [(s.id, _get_proxmox_client(s)) for s in servers]
|
||||
# Build clients while the session is open (from_server reads server attrs).
|
||||
# Per-server guard: один сервер с битой конфигурацией не должен
|
||||
# останавливать метрики для всех остальных.
|
||||
server_clients = []
|
||||
for s in servers:
|
||||
try:
|
||||
server_clients.append((s.id, _get_proxmox_client(s)))
|
||||
except Exception as exc:
|
||||
logger.debug(f"[metrics_broadcaster] skip server {s.id} ({s.name}): {exc}")
|
||||
finally:
|
||||
db.close()
|
||||
|
||||
|
||||
@@ -93,11 +93,16 @@ def run_async(coro):
|
||||
class MonitoringWorker:
|
||||
"""Background worker for monitoring and generating notifications"""
|
||||
|
||||
# Сколько подряд неудачных опросов статуса UPID терпим, прежде чем пометить
|
||||
# задачу failed. Транзитный сетевой сбой не должен «убивать» живую задачу.
|
||||
UPID_FAIL_THRESHOLD = 3
|
||||
|
||||
def __init__(self):
|
||||
self.last_vm_states: Dict[str, str] = {} # vm_id -> status
|
||||
self.last_resource_alerts: Dict[str, float] = {} # resource_id -> last_alert_time
|
||||
self.last_server_states: Dict[int, bool] = {} # server_id -> is_online
|
||||
self.last_server_alerts: Dict[int, float] = {} # server_id -> last_alert_time
|
||||
self.upid_fail_counts: Dict[int, int] = {} # ProxmoxTask.id -> consecutive fetch failures
|
||||
|
||||
def _create_proxmox_client(self, server: ProxmoxServer) -> ProxmoxClient:
|
||||
"""Create ProxmoxClient from server model, raising if auth is missing."""
|
||||
@@ -964,20 +969,43 @@ class MonitoringWorker:
|
||||
continue
|
||||
|
||||
# --- status -----------------------------------------------
|
||||
proxmox_status = client.get_task_status(task.node, task.upid)
|
||||
# If status is empty, it means Proxmox API rejected the UPID
|
||||
# Транзитный сбой (сеть, перезагрузка ноды) не должен
|
||||
# помечать живую задачу как failed: считаем ПОДРЯД идущие
|
||||
# неудачи и финализируем только после UPID_FAIL_THRESHOLD.
|
||||
try:
|
||||
proxmox_status = client.get_task_status(task.node, task.upid, raise_on_error=True)
|
||||
fetch_error = None
|
||||
except Exception as fe:
|
||||
proxmox_status = None
|
||||
fetch_error = str(fe)
|
||||
|
||||
if not proxmox_status:
|
||||
logger.warning(f"[UPID SYNC] Cannot fetch status for task #{task.id} (UPID={task.upid[:30]}...) — marking as failed")
|
||||
fails = self.upid_fail_counts.get(task.id, 0) + 1
|
||||
self.upid_fail_counts[task.id] = fails
|
||||
if fails < self.UPID_FAIL_THRESHOLD:
|
||||
logger.debug(
|
||||
f"[UPID SYNC] Task #{task.id}: status fetch failed "
|
||||
f"({fails}/{self.UPID_FAIL_THRESHOLD}), will retry: {fetch_error}"
|
||||
)
|
||||
continue
|
||||
logger.warning(
|
||||
f"[UPID SYNC] Cannot fetch status for task #{task.id} "
|
||||
f"(UPID={task.upid[:30]}...) after {fails} attempts — marking as failed: {fetch_error}"
|
||||
)
|
||||
task.status = "failed"
|
||||
task.exit_status = "Invalid UPID"
|
||||
task.exit_status = (fetch_error or "Invalid UPID")[:200]
|
||||
task.completed_at = utcnow()
|
||||
db.commit()
|
||||
self.upid_fail_counts.pop(task.id, None)
|
||||
try:
|
||||
broadcast_task_update(task.user_id, "task_update", task.to_dict())
|
||||
except Exception:
|
||||
pass
|
||||
continue
|
||||
|
||||
# Успешный опрос — сбрасываем счётчик неудач
|
||||
self.upid_fail_counts.pop(task.id, None)
|
||||
|
||||
pve_status = proxmox_status.get("status", "running")
|
||||
|
||||
# --- log --------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user