diff --git a/.github/workflows/docker_buildx_workflow.yml b/.github/workflows/docker_buildx_workflow.yml index 682f832e..d1415565 100644 --- a/.github/workflows/docker_buildx_workflow.yml +++ b/.github/workflows/docker_buildx_workflow.yml @@ -14,10 +14,10 @@ jobs: - name: Checkout code uses: actions/checkout@v4.1.1 - - name: Extract version from main.py + - name: Extract version from version.py id: get_version run: | - VERSION=$(grep -E '^VERSION *= *' main.py | head -1 | sed -E 's/^VERSION *= *["\x27]?([^"\x27]*)["\x27]?/\1/') + VERSION=$(grep -E '^VERSION *= *' version.py | head -1 | sed -E 's/^VERSION *= *["\x27]?([^"\x27]*)["\x27]?/\1/') echo "VERSION=$VERSION" >> $GITHUB_OUTPUT - name: Set up QEMU diff --git a/Dockerfile b/Dockerfile index 27e4b497..2bf04d4c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -33,7 +33,7 @@ COPY --from=builder /opt/conda/envs/hummingbot-api /opt/conda/envs/hummingbot-ap WORKDIR /hummingbot-api # Copy only necessary application files -COPY main.py config.py deps.py ./ +COPY main.py version.py config.py deps.py ./ COPY models ./models COPY routers ./routers COPY services ./services diff --git a/deps.py b/deps.py index bb4dc640..9d753edd 100644 --- a/deps.py +++ b/deps.py @@ -15,6 +15,7 @@ from services.gateway_service import GatewayService from services.gateway_swap_service import GatewaySwapService from services.market_data_service import MarketDataService +from services.self_upgrade import SelfUpgradeService from services.trading_history_service import TradingHistoryService from services.trading_service import TradingService from services.unified_connector_service import UnifiedConnectorService @@ -61,6 +62,11 @@ def get_docker_service(request: Request) -> DockerService: return request.app.state.docker_service +def get_self_upgrade_service(request: Request) -> SelfUpgradeService: + """Get SelfUpgradeService from app state.""" + return request.app.state.self_upgrade_service + + def get_gateway_service(request: Request) -> GatewayService: """Get GatewayService from app state.""" return request.app.state.gateway_service diff --git a/main.py b/main.py index c87d226a..a7d93ab2 100644 --- a/main.py +++ b/main.py @@ -13,7 +13,7 @@ # Load environment variables early load_dotenv() -VERSION = "1.0.1" +from version import VERSION # noqa: E402 (kept in its own module so routers can read it) # Monkey patch save_to_yml to prevent writes to library directory @@ -28,11 +28,10 @@ def patched_save_to_yml(yml_path, cm): config_helpers.save_to_yml = patched_save_to_yml -from fastapi import Depends, FastAPI, HTTPException, Request, status # noqa: E402 +from fastapi import Depends, FastAPI, HTTPException, status # noqa: E402 from fastapi.exceptions import RequestValidationError # noqa: E402 from fastapi.middleware.cors import CORSMiddleware # noqa: E402 from fastapi.middleware.gzip import GZipMiddleware # noqa: E402 -from fastapi.responses import JSONResponse # noqa: E402 from fastapi.security import HTTPBasic, HTTPBasicCredentials # noqa: E402 from hummingbot.client.config.client_config_map import GatewayConfigMap # noqa: E402 from hummingbot.client.config.config_crypt import ETHKeyFileSecretManger # noqa: E402 @@ -45,6 +44,7 @@ def patched_save_to_yml(yml_path, cm): archived_bots, backtesting, bot_orchestration, + bot_rate_oracle, connectors, controllers, docker, @@ -73,6 +73,7 @@ def patched_save_to_yml(yml_path, cm): from services.gateway_service import GatewayService # noqa: E402 from services.gateway_swap_service import GatewaySwapService # noqa: E402 from services.market_data_service import MarketDataService # noqa: E402 +from services.self_upgrade import SelfUpgradeService # noqa: E402 from services.trading_history_service import TradingHistoryService # noqa: E402 from services.trading_service import TradingService # noqa: E402 from services.unified_connector_service import UnifiedConnectorService # noqa: E402 @@ -80,6 +81,7 @@ def patched_save_to_yml(yml_path, cm): from utils.bot_archiver import BotArchiver # noqa: E402 from utils.core_compatibility import require_core_surface # noqa: E402 from utils.security import BackendAPISecurity # noqa: E402 +from utils.validation_errors import validation_exception_handler # noqa: E402 # Set up logging configuration logging.basicConfig( @@ -305,6 +307,18 @@ async def lifespan(app: FastAPI): else: logging.info("Gateway container not running; status monitor deferred until it is started") + # An upgrade started by the *previous* API process outlives it: the helper container + # that ran `docker compose up` is still on the box with its exit code and logs, and + # this process is the new container it created. Collect that record now, before + # anything can ask for it, and remove the helper so the next preflight is not blocked + # by its own predecessor. Never raises (FEAT-122). + self_upgrade_service = SelfUpgradeService( + docker_service=docker_service, + executor_service=executor_service, + bots_orchestrator=bots_orchestrator, + ) + self_upgrade_service.collect_on_boot() + bot_archiver = BotArchiver( settings.aws.api_key, settings.aws.secret_key, @@ -354,6 +368,7 @@ async def lifespan(app: FastAPI): app.state.backtesting_service = backtesting_service app.state.bots_orchestrator = bots_orchestrator app.state.docker_service = docker_service + app.state.self_upgrade_service = self_upgrade_service app.state.gateway_service = gateway_service app.state.bot_archiver = bot_archiver @@ -410,28 +425,7 @@ async def lifespan(app: FastAPI): app.add_middleware(GZipMiddleware, minimum_size=1000) -@app.exception_handler(RequestValidationError) -async def validation_exception_handler(request: Request, exc: RequestValidationError): - """ - Custom handler for validation errors to log detailed error messages. - """ - # Build a readable error message from validation errors - error_messages = [] - for error in exc.errors(): - loc = " -> ".join(str(part) for part in error.get("loc", [])) - msg = error.get("msg", "Validation error") - error_messages.append(f"{loc}: {msg}") - - # Log the validation error with details - logging.warning( - f"Validation error on {request.method} {request.url.path}: {'; '.join(error_messages)}" - ) - - # Return standard FastAPI validation error response - return JSONResponse( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - content={"detail": exc.errors()}, - ) +app.add_exception_handler(RequestValidationError, validation_exception_handler) logfire.configure(send_to_logfire="if-token-present", environment=settings.app.logfire_environment, service_name="hummingbot-api") @@ -476,6 +470,7 @@ def auth_user( app.include_router(controllers.router, dependencies=[Depends(auth_user)]) app.include_router(scripts.router, dependencies=[Depends(auth_user)]) app.include_router(market_data.router, dependencies=[Depends(auth_user)]) +app.include_router(bot_rate_oracle.router, dependencies=[Depends(auth_user)]) app.include_router(performance.router, dependencies=[Depends(auth_user)]) app.include_router(backtesting.router, dependencies=[Depends(auth_user)]) app.include_router(archived_bots.router, dependencies=[Depends(auth_user)]) diff --git a/models/__init__.py b/models/__init__.py index da3dae61..1b637590 100644 --- a/models/__init__.py +++ b/models/__init__.py @@ -181,6 +181,9 @@ # Script models from .scripts import Script, ScriptConfig, ScriptConfigResponse, ScriptResponse +# System models +from .system import SelfUpgradeRequest + # Trading models from .trading import ( AccountBalance, @@ -382,4 +385,6 @@ "ExecutorsSummaryResponse", "OrphanedPositionRecord", "OrphanedPositionsResponse", + # System models + "SelfUpgradeRequest", ] diff --git a/models/bot_rate_oracle.py b/models/bot_rate_oracle.py new file mode 100644 index 00000000..e65c3432 --- /dev/null +++ b/models/bot_rate_oracle.py @@ -0,0 +1,88 @@ +""" +Pydantic models for the bot client defaults endpoints. + +The API itself prices through the in-house ticker pool, but deployed bots still run +hummingbot's RateOracle, configured from the conf_client.yml copied out of their +credentials profile. These models describe that persisted configuration, plus the one +other conf_client.yml knob operators tune per profile -- rate_limits_share_pct, the share +of each exchange's API rate limit a bot instance is allowed to spend. +""" + +from typing import List, Optional + +from pydantic import BaseModel, Field, field_validator + + +class GlobalTokenConfig(BaseModel): + """Global token configuration for displaying values.""" + global_token_name: Optional[str] = Field( + default=None, + description="The token to use as global quote (e.g., USDT, USD, BTC)" + ) + global_token_symbol: Optional[str] = Field( + default=None, + description="Symbol to display for the global token" + ) + + @field_validator("global_token_name", "global_token_symbol") + @classmethod + def _reject_blank(cls, v: Optional[str], info) -> Optional[str]: + # A blank quote token would make every cross-rate lookup resolve to nothing, and a + # blank symbol renders every value in the bot's status without a unit. Omit the + # field to leave it unchanged. + if v is not None: + v = v.strip() + if not v: + raise ValueError(f"{info.field_name} must not be blank") + return v + + +class RateOracleSourceConfig(BaseModel): + """Rate oracle source configuration.""" + name: str = Field(description="The rate oracle source deployed bots use for price data") + + +class RateOracleConfig(BaseModel): + """Bot client defaults persisted in an account's conf_client.yml.""" + account_name: str = Field(description="Credentials profile the configuration belongs to") + rate_oracle_source: RateOracleSourceConfig + global_token: GlobalTokenConfig + # Bounds mirror hummingbot's own ClientConfigMap.rate_limits_share_pct (gt=0, le=100): + # the file is read back by hummingbot inside the bot container, so a value it would + # reject must not be writable here. + rate_limits_share_pct: float = Field( + default=100.0, + gt=0, + le=100, + description="Percentage of each exchange's API rate limit allocated to a bot instance" + ) + + +class RateOracleConfigResponse(RateOracleConfig): + """Response for the rate oracle configuration GET endpoint.""" + available_sources: List[str] = Field(description="Rate oracle sources that can be configured") + + +class RateOracleConfigUpdateRequest(BaseModel): + """Request model for updating the rate oracle configuration.""" + rate_oracle_source: Optional[RateOracleSourceConfig] = Field( + default=None, + description="New rate oracle source configuration (optional)" + ) + global_token: Optional[GlobalTokenConfig] = Field( + default=None, + description="New global token configuration (optional)" + ) + rate_limits_share_pct: Optional[float] = Field( + default=None, + gt=0, + le=100, + description="New share of each exchange's API rate limit for a bot instance, 1-100 (optional)" + ) + + +class RateOracleConfigUpdateResponse(BaseModel): + """Response for a bot client defaults update.""" + success: bool = Field(description="Whether the update was successful") + message: str = Field(description="Status message") + config: RateOracleConfig = Field(description="Updated configuration") diff --git a/models/system.py b/models/system.py new file mode 100644 index 00000000..fffe4eb4 --- /dev/null +++ b/models/system.py @@ -0,0 +1,21 @@ +"""System router models (FEAT-122).""" + +from pydantic import BaseModel, Field + + +class SelfUpgradeRequest(BaseModel): + """Consent to what a self-upgrade destroys. + + Restarting the API closes every RUNNING executor as SYSTEM_CLEANUP and they are not + restored, so the one field here is the acknowledgement of that -- and it defaults to + False, which means a caller that sends an empty body is refused rather than served. + Bot containers are separate and keep running. + """ + + acknowledge_executor_loss: bool = Field( + default=False, + description=( + "Confirms that every running executor will be closed as SYSTEM_CLEANUP and not " + "restored. Required when the server has any running executor." + ), + ) diff --git a/routers/bot_rate_oracle.py b/routers/bot_rate_oracle.py new file mode 100644 index 00000000..bf0237d9 --- /dev/null +++ b/routers/bot_rate_oracle.py @@ -0,0 +1,189 @@ +""" +Bot client defaults router. + +Configures deployed bots, not the API. The API prices through its own ticker pool (see +MarketDataService), but every deployed bot still runs hummingbot's RateOracle, configured +from the conf_client.yml copied out of its credentials profile at deploy time. These +endpoints read and persist that configuration so bots are not stuck with whatever source +the shipped template defaults to. + +This router is the only writer of a credentials profile's conf_client.yml, which is why +rate_limits_share_pct lives here rather than behind an endpoint of its own: a second +writer of the same file would have to repeat this validation and would silently skip the +live-quote-token switch below. The set it writes is closed and small -- rate_oracle_source, +global_token, rate_limits_share_pct -- so no caller can reach the mqtt_bridge or gateway +credentials that share the file. +""" + +import yaml +from fastapi import APIRouter, Depends, HTTPException, Query +from hummingbot.core.rate_oracle.rate_oracle import RATE_ORACLE_SOURCES + +from deps import get_market_data_service +from models.bot_rate_oracle import ( + GlobalTokenConfig, + RateOracleConfig, + RateOracleConfigResponse, + RateOracleConfigUpdateRequest, + RateOracleConfigUpdateResponse, + RateOracleSourceConfig, +) +from services.accounts_service import validate_safe_name +from services.market_data_service import MarketDataService +from utils.file_system import FileSystemUtil + +router = APIRouter(tags=["Bot Orchestration"], prefix="/bot-orchestration/rate-oracle") + +DEFAULT_ACCOUNT = "master_account" + + +def _conf_client_path(account_name: str) -> str: + """Path to an account's conf_client.yml, relative to the FileSystemUtil base path ("bots").""" + validate_safe_name(account_name, "account name") + return f"credentials/{account_name}/conf_client.yml" + + +def _read_conf_client(account_name: str) -> dict: + path = _conf_client_path(account_name) + try: + data = FileSystemUtil().read_yaml_file(path) + except FileNotFoundError: + raise HTTPException(status_code=404, detail=f"Configuration file not found: {path}") + except yaml.YAMLError: + raise HTTPException(status_code=500, detail=f"Configuration file is not valid YAML: {path}") + # A hand-edited file can hold anything at the top level, including a YAML list or a + # bare scalar; read as empty rather than handing callers something they can't `.get()`. + return data if isinstance(data, dict) else {} + + +def _clean_str(value, default: str) -> str: + """``value`` as a stripped string, or ``default`` for anything else. + + conf_client.yml is user-editable and this only reads it back, so a blank, wrong-typed + or missing value must read as the default rather than failing the whole GET -- same + principle as _share_pct below. + """ + return value.strip() if isinstance(value, str) and value.strip() else default + + +def _share_pct(config_data: dict) -> float: + """rate_limits_share_pct as a number, falling back to hummingbot's own default. + + The key is absent from older templates, and a hand-edited file can hold anything, so + an unreadable value reads as the default rather than failing the whole GET -- the + caller is asking what bots will run with, and hummingbot itself would fall back the + same way. + """ + try: + value = float(config_data.get("rate_limits_share_pct")) + except (TypeError, ValueError): + return 100.0 + return value if 0 < value <= 100 else 100.0 + + +def _to_config(account_name: str, config_data: dict) -> RateOracleConfig: + rate_oracle_source = config_data.get("rate_oracle_source") + if not isinstance(rate_oracle_source, dict): + rate_oracle_source = {} + global_token = config_data.get("global_token") + if not isinstance(global_token, dict): + global_token = {} + return RateOracleConfig( + account_name=account_name, + rate_oracle_source=RateOracleSourceConfig( + name=_clean_str(rate_oracle_source.get("name"), "gate_io") + ), + global_token=GlobalTokenConfig( + global_token_name=_clean_str(global_token.get("global_token_name"), "USDT"), + global_token_symbol=_clean_str(global_token.get("global_token_symbol"), "$"), + ), + rate_limits_share_pct=_share_pct(config_data), + ) + + +@router.get("/sources", response_model=list[str]) +async def get_bot_rate_oracle_sources(): + """List the rate oracle sources a bot can be configured with. Does not affect the API's own pricing.""" + return list(RATE_ORACLE_SOURCES.keys()) + + +@router.get("/config", response_model=RateOracleConfigResponse) +async def get_bot_rate_oracle_config(account_name: str = Query(DEFAULT_ACCOUNT)): + """ + Get the client defaults that bots deployed with this credentials profile will use. + + Args: + account_name: Credentials profile whose conf_client.yml to read (default master_account) + """ + config = _to_config(account_name, _read_conf_client(account_name)) + return RateOracleConfigResponse(**config.model_dump(), available_sources=list(RATE_ORACLE_SOURCES.keys())) + + +@router.put("/config", response_model=RateOracleConfigUpdateResponse) +async def update_bot_rate_oracle_config( + update_request: RateOracleConfigUpdateRequest, + account_name: str = Query(DEFAULT_ACCOUNT), + market_data_service: MarketDataService = Depends(get_market_data_service), +): + """ + Update the bot rate oracle source, global token and/or rate-limit share in an account's + conf_client.yml. + + Bots pick the change up on their next deploy (running bots keep their copied config). + Changing master_account's global token also switches the API's own valuation quote token. + + Args: + update_request: Configuration updates to apply + account_name: Credentials profile whose conf_client.yml to update (default master_account) + """ + config_data = _read_conf_client(account_name) + changes_made = [] + + if update_request.rate_oracle_source is not None: + source_name = update_request.rate_oracle_source.name + if source_name not in RATE_ORACLE_SOURCES: + raise HTTPException( + status_code=400, + detail=f"Invalid rate oracle source: {source_name}. " + f"Available sources: {list(RATE_ORACLE_SOURCES.keys())}" + ) + if not isinstance(config_data.get("rate_oracle_source"), dict): + config_data["rate_oracle_source"] = {} + config_data["rate_oracle_source"]["name"] = source_name + changes_made.append(f"rate_oracle_source updated to {source_name}") + + if update_request.global_token is not None: + if not isinstance(config_data.get("global_token"), dict): + config_data["global_token"] = {} + global_token = config_data["global_token"] + token_name = update_request.global_token.global_token_name + if token_name is not None: + global_token["global_token_name"] = token_name + changes_made.append(f"global_token_name updated to {token_name}") + token_symbol = update_request.global_token.global_token_symbol + if token_symbol is not None: + global_token["global_token_symbol"] = token_symbol + changes_made.append(f"global_token_symbol updated to {token_symbol}") + + if update_request.rate_limits_share_pct is not None: + # Written as a float so the file keeps the shape hummingbot's own writer produces; + # the 0 < pct <= 100 bound is enforced by the request model, which mirrors + # ClientConfigMap's, so a value the bot would reject never reaches the file. + share_pct = float(update_request.rate_limits_share_pct) + config_data["rate_limits_share_pct"] = share_pct + changes_made.append(f"rate_limits_share_pct updated to {share_pct}") + + if changes_made: + FileSystemUtil().dump_dict_to_yaml(_conf_client_path(account_name), config_data) + + # Only switch the API's live quote token once the file is written, so a failed write + # can't leave the API and future bots disagreeing on the token. + token_name = update_request.global_token and update_request.global_token.global_token_name + if token_name is not None and account_name == DEFAULT_ACCOUNT: + market_data_service.quote_token = token_name + + return RateOracleConfigUpdateResponse( + success=True, + message="; ".join(changes_made) if changes_made else "No changes made", + config=_to_config(account_name, config_data), + ) diff --git a/routers/system.py b/routers/system.py index eb9dbae7..a0f16dc1 100644 --- a/routers/system.py +++ b/routers/system.py @@ -1,8 +1,20 @@ +import logging import os +from importlib import metadata +from typing import Optional import psutil +from fastapi import APIRouter, Depends, HTTPException -from fastapi import APIRouter +from config import settings +from deps import get_docker_service, get_self_upgrade_service +from models import SelfUpgradeRequest +from services.docker_service import DockerService +from services.self_upgrade import SelfUpgradeRefused, SelfUpgradeService +from utils.compose import describe_container, own_container, pinning +from version import VERSION + +logger = logging.getLogger(__name__) router = APIRouter(tags=["System"], prefix="/system") @@ -66,3 +78,147 @@ async def get_system_resources(): "percent": disk.percent, }, } + + +def _hummingbot_version(): + """The version of the hummingbot library this API runs against, or None if absent.""" + try: + return metadata.version("hummingbot") + except metadata.PackageNotFoundError: + return None + + +@router.get("/info") +async def get_system_info(docker_service: DockerService = Depends(get_docker_service)): + """ + Get the versions this API server runs and how its own container was deployed. + + Read-only, and never raises: when docker.sock is not mounted, the daemon is + unreachable or this process cannot find its own container, the version fields are + still reported and the container block is None. + + Returns: + Dictionary with the API version, the hummingbot library version, the market-data + tunables this process is running with, whether the Docker daemon could be + reached, the API's own container (id, name, image, digest and compose labels), + and whether that image is pinned, with a short reason and the override file name + when it is. + """ + info = { + "api_version": VERSION, + "hummingbot_version": _hummingbot_version(), + # The MARKET_DATA_* knobs as this process resolved them. Reported rather than + # editable: they come from the env_file, which is not mounted into the + # container, so the API cannot write its own .env -- a dashboard shows them and + # says where to change them. Non-secret by construction (MarketDataSettings + # holds only intervals and timeouts). + "market_data": settings.market_data.model_dump(), + "docker_available": False, + "container": None, + # None rather than False: without the daemon we have not established that the + # image is unpinned, only that we cannot tell. + "pinned": None, + "pinned_reason": None, + "override_file": None, + } + + # DockerService leaves `client` unset when docker.from_env() failed at startup. + client = getattr(docker_service, "client", None) + if client is None: + return info + try: + client.ping() + except Exception as e: + logger.warning(f"Docker daemon unreachable while reporting system info: {e}") + return info + info["docker_available"] = True + + container = own_container(client) + if container is None: + logger.warning("Could not identify the API's own container while reporting system info") + return info + + try: + info["container"] = describe_container(container) + except Exception as e: + logger.warning(f"Could not inspect the API's own container while reporting system info: {e}") + return info + + info.update(pinning(info["container"])) + return info + + +# ── Self-upgrade (FEAT-122) ────────────────────────────────────────────────────── +# +# Replacing this API's own container with the published image, so a remote server can +# be upgraded from a dashboard instead of over SSH. See services/self_upgrade.py for +# why a helper container does the recreate, and for the rule these three routes exist +# to enforce: anything not established is a refusal. + + +@router.get("/upgrade/preflight") +async def get_upgrade_preflight(upgrade_service: SelfUpgradeService = Depends(get_self_upgrade_service)): + """ + Report whether this API can replace its own container, and what it would cost. + + Read-only and never raises: a daemon that cannot be reached, a container that cannot + be placed and a registry that cannot be read are all reported as can_upgrade false + with a blocked_reason, because the caller of a destructive action needs a reason far + more than it needs a 500. + + Returns: + Dictionary with image_ref, current_digest, available_digest, up_to_date, the + pinning verdict (pinned / pinned_reason / override_file), the compose project, + working dir and config files, running_executors and running_bots, and + can_upgrade with its blocked_reason (None only when the upgrade may be started). + """ + return await upgrade_service.preflight() + + +@router.post("/upgrade", status_code=202) +async def start_upgrade( + request: Optional[SelfUpgradeRequest] = None, + upgrade_service: SelfUpgradeService = Depends(get_self_upgrade_service), +): + """ + Pull the published image and hand the container recreate to a helper container. + + The preflight is re-run here, so a server that became unupgradable since the caller + last looked -- an executor started, another upgrade began, the daemon went away -- + is refused rather than upgraded on a stale reading. + + Args: + request: acknowledge_executor_loss, the caller's consent to every running + executor being closed as SYSTEM_CLEANUP. Required when any is running. + + Returns: + 202 with the run id and the initial phase. The API is replaced part-way through, + so the result is read back from /system/upgrade/status after it restarts. + + Raises: + HTTPException: 409 when the upgrade is refused. Nothing was pulled, and nothing + on the server was changed. + """ + # An absent body is a caller who acknowledged nothing, which is the refusing default. + request = request or SelfUpgradeRequest() + try: + return await upgrade_service.start(acknowledge_executor_loss=request.acknowledge_executor_loss) + except SelfUpgradeRefused as e: + raise HTTPException(status_code=409, detail=str(e)) + + +@router.get("/upgrade/status") +async def get_upgrade_status(upgrade_service: SelfUpgradeService = Depends(get_self_upgrade_service)): + """ + Report the current or last self-upgrade. + + Phases: idle (there has been none), pulling, recreating (this API is about to be + replaced, so the next answer comes from the new one), done, failed. After a + successful recreate the record is the one this API collected from the helper + container on boot, including its exit code and the tail of its output. + + Returns: + Dictionary with run_id, phase, detail, previous_digest, new_digest, exit_code + and log_tail. + """ + return upgrade_service.status() diff --git a/services/market_data_service.py b/services/market_data_service.py index 0ed839a6..8c20c769 100644 --- a/services/market_data_service.py +++ b/services/market_data_service.py @@ -1107,6 +1107,10 @@ def quote_token(self) -> str: """The global quote token everything is valued in.""" return self._quote_token + @quote_token.setter + def quote_token(self, value: str) -> None: + self._quote_token = value + @property def connector_service(self) -> "UnifiedConnectorService": """Get the connector service instance.""" diff --git a/services/self_upgrade.py b/services/self_upgrade.py new file mode 100644 index 00000000..18993d30 --- /dev/null +++ b/services/self_upgrade.py @@ -0,0 +1,619 @@ +"""Upgrade this API's own container to the published image (FEAT-122). + +Today upgrading a hummingbot-api server means ssh-ing to the host and running +``docker compose pull && docker compose up -d``. Condor's own updater only knows how to +do that in a *local* checkout, so every remote server is a manual job and servers drift. + +**Why a helper container.** A process cannot recreate its own container: ``compose up`` +stops the old container, which kills the process running compose halfway through. So the +API pulls the image in-process -- the thing it already does for every other image -- and +then hands the recreate to a one-shot ``docker:cli`` container that survives the API's +restart. That helper is also the run record: its exit code and logs say what happened, +and the *new* API collects them on boot. There is no state file and no DB table. + +**Everything here defaults to refusing.** The failure mode of this module is a trading +API server that does not come back, so ``preflight`` starts from "blocked" and only +clears that at the very end, once every fact it needs has actually been established. +Not established is the same as blocked: an unreachable Docker daemon, a container it +cannot place, missing compose labels, a registry it cannot read, an executor count it +cannot take. None of those are "probably fine". + +**What it will not do.** It never upgrades a *pinned* deployment -- an image built on the +box, or a compose override file that can repoint ``image:``. Removing an override or +swapping a hand-built tag is an operator's decision about their own host, and silently +rolling such a server onto the published build is the one outcome worse than not +upgrading at all. Those servers stay an SSH job and the reason names the override file. + +Out of scope, deliberately: switching tags or images, editing override files, rebuilding +from source, upgrading postgres/emqx/gateway, and rollback. +""" + +import logging +import threading +import time +import uuid +from typing import Any, Dict, Optional + +from utils.compose import config_files, describe_container, own_container, pinning + +logger = logging.getLogger(__name__) + +# Ships the compose plugin. Pinned to a major tag rather than :latest so the helper that +# recreates this API is not itself a moving target. +HELPER_IMAGE = "docker:27-cli" + +# Carries the run id, and is how a helper -- running or exited -- is found again after +# the API that started it has been replaced. +HELPER_LABEL = "hummingbot-api.self-upgrade" +HELPER_PREVIOUS_DIGEST_LABEL = "hummingbot-api.self-upgrade.previous-digest" +# The compose project the helper was started for. HELPER_LABEL alone is not scoped to a +# deployment -- on a host running more than one hummingbot-api stack, every stack's helper +# carries it -- so collection is only ever allowed to touch a helper carrying this API's +# own project. +HELPER_PROJECT_LABEL = "hummingbot-api.self-upgrade.project" + +# The compose service to recreate. Only this one: postgres, emqx, any tailscale sidecar +# and every bot container are left exactly as they are. +SERVICE_NAME = "hummingbot-api" + +# Lines of the helper's output kept as the run record. +LOG_TAIL_LINES = 40 + + +class SelfUpgradeRefused(Exception): + """This upgrade will not be started, and nothing has been changed. + + Raised before any pull and before any container is touched, so a caller that sees it + knows the server is exactly as it was. The router answers 409. + """ + + +def _sha(digest: Optional[str]) -> Optional[str]: + """The ``sha256:...`` part of a digest, however it was spelled. + + A container's ``RepoDigests`` entry is ``repo@sha256:...`` while the registry answers + with the bare ``sha256:...``; comparing the two as written would make every server + look out of date forever. + """ + if not digest: + return None + return digest.split("@")[-1].strip() or None + + +class SelfUpgradeService: + """Preflight, start and report on replacing this API's container. + + Args: + docker_service: The API's ``DockerService``; its ``client`` is the daemon handle + and its ``pull_image_sync`` is the pull already used everywhere else. + executor_service: Source of the running-executor count, the number that decides + whether this upgrade destroys anything. + bots_orchestrator: Source of the running-bot count, which is reported so the + operator can see that bots are *not* affected. + """ + + def __init__(self, docker_service, executor_service=None, bots_orchestrator=None): + self.docker_service = docker_service + self.executor_service = executor_service + self.bots_orchestrator = bots_orchestrator + self._lock = threading.Lock() + self._run: Optional[Dict[str, Any]] = None + + # ── Facts ──────────────────────────────────────────────────────────────────── + + @property + def client(self): + """The Docker daemon handle, or None when it was never established.""" + return getattr(self.docker_service, "client", None) + + def _daemon(self): + """The daemon handle if it answers a ping, else None. Never raises.""" + client = self.client + if client is None: + return None + try: + client.ping() + except Exception as e: + logger.warning(f"Docker daemon unreachable while preparing a self-upgrade: {e}") + return None + return client + + def _registry_digest(self, image_ref: str) -> Optional[str]: + """The digest the registry currently serves for ``image_ref``, or None. + + None is a *blocking* answer, not an optimistic one: without it there is no + evidence that a newer image exists, and pulling a tag on that basis is how a + server gets recreated for nothing. + """ + client = self.client + try: + return _sha(client.images.get_registry_data(image_ref).id) + except Exception as e: + logger.warning(f"Could not read the registry digest for {image_ref}: {e}") + return None + + def _existing_helper(self, client): + """``(established, helper)`` for a helper of this deployment still in flight. + + ``established`` is False when the daemon would not answer the question at all, + which blocks: an upgrade started while another one is mid-recreate is two + ``compose up`` runs racing for the same container, and "I could not check" is not + a reason to start the second one. + + An exited helper is not in flight -- it is either this deployment's, which + ``_collect_finished_helpers`` already turns into a run and removes before this is + reached, or another stack's on the same host, which is inert as far as this + deployment's own recreate is concerned and must not block it. + """ + try: + found = client.containers.list(all=True, filters={"label": HELPER_LABEL}) + except Exception as e: + logger.warning(f"Could not list self-upgrade helper containers: {e}") + return False, None + own_project = self._own_compose_project(client) + in_flight = [ + h for h in found + if getattr(h, "status", None) not in ("exited", "dead") + and own_project is not None + and (h.labels or {}).get(HELPER_PROJECT_LABEL) == own_project + ] + return True, (in_flight[0] if in_flight else None) + + async def _running_executors(self) -> Optional[int]: + """How many executors are live, or None when that could not be established.""" + if self.executor_service is None: + return None + try: + return int(self.executor_service.get_summary().get("total_active", 0)) + except Exception as e: + logger.warning(f"Could not count running executors before a self-upgrade: {e}") + return None + + async def _running_bots(self) -> Optional[int]: + """How many bot containers are running, or None when that is unknown.""" + if self.bots_orchestrator is None: + return None + try: + return len(await self.bots_orchestrator.get_active_containers()) + except Exception as e: + logger.warning(f"Could not count running bots before a self-upgrade: {e}") + return None + + # ── Preflight ──────────────────────────────────────────────────────────────── + + async def preflight(self) -> Dict[str, Any]: + """Everything the caller needs to decide, and whether it may proceed at all. + + Returns: + Dictionary with the image reference and its current/available digests, + ``up_to_date``, the pinning verdict, the compose coordinates, the counts of + running executors and bots, and ``can_upgrade`` with a ``blocked_reason`` + that is None only when the upgrade may actually be started. + + Never raises: this is what a dashboard polls, and a daemon hiccup has to render + as a refusal with a reason, not as a 500. + """ + info: Dict[str, Any] = { + "service": SERVICE_NAME, + "helper_image": HELPER_IMAGE, + "image_ref": None, + "current_digest": None, + "available_digest": None, + "up_to_date": None, + "pinned": None, + "pinned_reason": None, + "override_file": None, + "compose": None, + "running_executors": None, + "running_bots": None, + "can_upgrade": False, + # Refusing is the default. Every branch below either replaces this reason + # with a more specific one or falls through to the single place that clears + # it, so a path that forgets to decide still refuses. + "blocked_reason": "This server has not established that it can upgrade itself.", + } + + client = self._daemon() + if client is None: + info["blocked_reason"] = ( + "The Docker daemon is not reachable from this API, so it cannot upgrade itself. " + "Check that /var/run/docker.sock is mounted into the container." + ) + return info + + # A helper that exited while this process is still alive is a recreate that did + # not replace us. Record it and clear it here, before any other check can return, + # so neither the run status nor the helper check below is stuck on it. + self._collect_finished_helpers(client, on_boot=False) + + container = own_container(client) + if container is None: + info["blocked_reason"] = ( + "This API could not identify its own container, so it does not know what to recreate. " + "Upgrade this server over SSH." + ) + return info + + try: + described = describe_container(container) + except Exception as e: + logger.warning(f"Could not inspect the API's own container before a self-upgrade: {e}") + info["blocked_reason"] = ( + "This API could not inspect its own container, so it does not know what to recreate. " + "Upgrade this server over SSH." + ) + return info + + info["image_ref"] = described.get("image") + info["current_digest"] = described.get("digest") + info.update(pinning(described)) + + project = described.get("compose_project") + working_dir = described.get("compose_working_dir") + files = config_files(described) + if project and working_dir and files: + info["compose"] = {"project": project, "working_dir": working_dir, "config_files": files} + + # Counts first: they are reported even on a blocked preflight, because "you + # cannot upgrade here" and "17 executors are running" are both things the + # operator came to find out. + info["running_executors"] = await self._running_executors() + info["running_bots"] = await self._running_bots() + + if info["compose"] is None: + info["blocked_reason"] = ( + "This API was not started by Docker Compose (no compose labels on its container), " + "so there is no compose project to recreate it from. Upgrade this server over SSH." + ) + return info + + if info["pinned"]: + info["blocked_reason"] = ( + f"This server's image is pinned: {info['pinned_reason']}. " + "Pulling the published image would undo that, so it is refused. " + "Upgrade this server over SSH." + ) + return info + + if not info["image_ref"]: + info["blocked_reason"] = ( + "This API's container does not report the image it was started from, " + "so there is nothing to compare against the registry." + ) + return info + + available = self._registry_digest(info["image_ref"]) + info["available_digest"] = available + if available is None: + info["blocked_reason"] = ( + f"Could not read the published digest for {info['image_ref']} from the registry, " + "so there is no evidence that a newer image exists. Nothing was pulled." + ) + return info + + info["up_to_date"] = _sha(info["current_digest"]) == available + if info["up_to_date"]: + info["blocked_reason"] = f"Already running the published {info['image_ref']}." + return info + + if info["running_executors"] is None: + info["blocked_reason"] = ( + "Could not determine how many executors are running, and a restart closes every " + "running executor as SYSTEM_CLEANUP. Refusing rather than guessing." + ) + return info + + established, helper = self._existing_helper(client) + if not established: + info["blocked_reason"] = ( + "Could not check whether an upgrade is already running on this server. " + "Refusing rather than risking two recreates of the same container." + ) + return info + if helper is not None: + info["blocked_reason"] = ( + f"An upgrade helper container is still running ({helper.name}). " + "Wait for it to finish." + ) + return info + + with self._lock: + phase = (self._run or {}).get("phase") + if phase in ("pulling", "recreating"): + info["blocked_reason"] = "An upgrade is already running on this server." + return info + + # The one place the refusal is lifted. + info["can_upgrade"] = True + info["blocked_reason"] = None + return info + + # ── Start ──────────────────────────────────────────────────────────────────── + + async def start(self, acknowledge_executor_loss: bool = False) -> Dict[str, Any]: + """Pull the published image and hand the recreate to a helper container. + + The preflight is re-run here rather than trusted from the caller's earlier read: + between the dialog opening and the button being pressed, an executor can start, + another operator can begin an upgrade, or the daemon can go away. + + Args: + acknowledge_executor_loss: The caller has been told that every running + executor will be closed as ``SYSTEM_CLEANUP`` and is not coming back. + + Returns: + ``{"run_id": ..., "phase": "pulling"}``. + + Raises: + SelfUpgradeRefused: Nothing was pulled and nothing was touched. + """ + pre = await self.preflight() + if pre["blocked_reason"]: + raise SelfUpgradeRefused(pre["blocked_reason"]) + + running = pre["running_executors"] or 0 + if running > 0 and not acknowledge_executor_loss: + raise SelfUpgradeRefused( + f"{running} executor(s) are running. Restarting this API closes every one of them as " + "SYSTEM_CLEANUP and they are not restored. Confirm that loss to proceed." + ) + + run_id = uuid.uuid4().hex[:12] + compose = pre["compose"] + with self._lock: + # Re-checked under the lock: two callers can both clear the preflight before + # either has claimed the run, and the loser would start a second `compose up` + # against the container the first is already replacing. + in_flight = (self._run or {}).get("phase") + if in_flight in ("pulling", "recreating"): + raise SelfUpgradeRefused("An upgrade is already running on this server.") + self._run = { + "run_id": run_id, + "phase": "pulling", + "detail": f"Pulling {pre['image_ref']}", + "image_ref": pre["image_ref"], + "previous_digest": pre["current_digest"], + "new_digest": None, + "exit_code": None, + "log_tail": [], + "started_at": time.time(), + } + + threading.Thread( + target=self._pull_then_recreate, + args=(run_id, pre["image_ref"], compose, pre["current_digest"]), + daemon=True, + name=f"self-upgrade-{run_id}", + ).start() + + return {"run_id": run_id, "phase": "pulling"} + + def _set(self, run_id: str, **fields) -> None: + """Update the current run, ignoring a write for a run that has been replaced.""" + with self._lock: + if not self._run or self._run.get("run_id") != run_id: + return + self._run.update(fields) + + def _pull_then_recreate(self, run_id, image_ref, compose, previous_digest) -> None: + """The whole upgrade, off the event loop. Never raises out of the thread. + + Order matters and is the safety property: both images are on the box *before* + anything is stopped. A pull that fails leaves the server untouched and running, + which is why the helper image is pulled here too rather than being left to + ``containers.run``'s implicit pull -- that one would happen with the recreate + already committed. + """ + for ref in (image_ref, HELPER_IMAGE): + self._set(run_id, phase="pulling", detail=f"Pulling {ref}") + result = self.docker_service.pull_image_sync(ref) + if not (isinstance(result, dict) and result.get("success")): + # The daemon's own string carries the socket URL and API version, so it + # goes to the log; the caller gets the image that failed. + logger.error(f"Self-upgrade {run_id}: pull of {ref} failed: {result}") + self._set( + run_id, + phase="failed", + detail=f"Could not pull {ref}. Nothing was changed; see the API log for the daemon's error.", + finished_at=time.time(), + ) + return + + command = ["docker", "compose", "-p", compose["project"]] + for path in compose["config_files"]: + command += ["-f", path] + command += ["up", "-d", "--no-deps", SERVICE_NAME] + + working_dir = compose["working_dir"] + try: + helper = self.client.containers.run( + HELPER_IMAGE, + command=command, + detach=True, + labels={ + HELPER_LABEL: run_id, + HELPER_PREVIOUS_DIGEST_LABEL: previous_digest or "", + HELPER_PROJECT_LABEL: compose["project"], + }, + volumes={ + "/var/run/docker.sock": {"bind": "/var/run/docker.sock", "mode": "rw"}, + # Bound at the *same* host path so the relative paths inside the + # compose files -- ./bots, .env, the override file -- resolve to the + # same things they resolved to when the operator brought the stack up. + working_dir: {"bind": working_dir, "mode": "rw"}, + }, + working_dir=working_dir, + # auto_remove would delete the run record along with the container, and + # the record is the only thing that survives this API being replaced. + auto_remove=False, + ) + except Exception as e: + logger.error(f"Self-upgrade {run_id}: could not start the helper container: {e}") + self._set( + run_id, + phase="failed", + detail="Could not start the upgrade helper container. Nothing was changed.", + finished_at=time.time(), + ) + return + + logger.info(f"Self-upgrade {run_id}: helper {helper.name} running `{' '.join(command)}`") + self._set( + run_id, + phase="recreating", + detail=( + "Recreating the hummingbot-api container. This API is about to be replaced; " + "the new one reports the result." + ), + helper=getattr(helper, "name", None), + ) + + # ── Report ─────────────────────────────────────────────────────────────────── + + def status(self) -> Dict[str, Any]: + """The current or last run, or an idle record when there has been none. + + A run this process left at ``recreating`` is checked against its helper: if the + helper has exited and we are still here, the recreate did not replace us, and + without collecting it here the run would read ``recreating`` forever. + """ + with self._lock: + phase = (self._run or {}).get("phase") + if phase == "recreating": + client = self._daemon() + if client is not None: + self._collect_finished_helpers(client, on_boot=False) + with self._lock: + if self._run is None: + return {"run_id": None, "phase": "idle", "detail": None, "log_tail": []} + return dict(self._run) + + def collect_on_boot(self) -> None: + """Adopt the outcome of a helper that outlived the API that started it. + + This is where a successful upgrade finishes: the process that started it is gone, + and the record of what happened is the exited helper container. Never raises: a + failure to tidy up must not stop the API booting. + """ + client = self._daemon() + if client is None: + return + self._collect_finished_helpers(client, on_boot=True) + + def _own_compose_project(self, client) -> Optional[str]: + """This container's own compose project, or None when it cannot be determined. + + A helper is only ever this deployment's if it was labelled with this project at + creation time (see _pull_then_recreate). Without knowing our own project there is + nothing to compare it against, so collection is skipped rather than guessed. + """ + try: + container = own_container(client) + if container is None: + return None + return describe_container(container).get("compose_project") + except Exception as e: + logger.warning(f"Could not read this API's own compose project: {e}") + return None + + def _collect_finished_helpers(self, client, on_boot: bool) -> None: + """Turn every exited helper of this deployment into its last run, then remove it. + + On boot the helper is our predecessor's, and exit 0 means we are the new + container. Anywhere else this process is the one the helper was meant to replace + and it is still running, so the recreate did not happen whatever the exit code + says -- recorded as failed, so the operator sees it and can retry. + + A helper still running is left alone -- it may be mid-recreate, and preflight + blocks on it. A helper that does not carry this API's own compose project is + another stack's, whatever its label and exit code say, and touching it -- let + alone removing it -- would take away the only record that stack has of its own + run. When a run of ours is already tracked (self._run has a run_id), a helper for + some other run under our own project is left alone too, on the same reasoning. + Never raises. + """ + try: + helpers = client.containers.list(all=True, filters={"label": HELPER_LABEL}) + except Exception as e: + logger.warning(f"Could not look for self-upgrade helper containers: {e}") + return + + own_project = self._own_compose_project(client) + with self._lock: + expected_run_id = (self._run or {}).get("run_id") + + for helper in helpers: + # A helper that has not exited is very likely running `docker compose up` + # right now. Removing it would kill the recreate half-way, which is the one + # thing this whole module is built to avoid, so it is left alone -- and + # preflight blocks on it until it finishes. + if getattr(helper, "status", None) not in ("exited", "dead"): + logger.info(f"Self-upgrade helper {helper.name} is still {helper.status}; leaving it.") + continue + + labels = helper.labels or {} + run_id = labels.get(HELPER_LABEL) + helper_project = labels.get(HELPER_PROJECT_LABEL) + if own_project is None or helper_project != own_project: + logger.info(f"Self-upgrade helper {helper.name} belongs to another compose project; leaving it.") + continue + if expected_run_id is not None and run_id != expected_run_id: + logger.info(f"Self-upgrade helper {helper.name} is not the run this API is tracking; leaving it.") + continue + + try: + previous_digest = labels.get(HELPER_PREVIOUS_DIGEST_LABEL) or None + exit_code = (helper.attrs.get("State") or {}).get("ExitCode") + try: + raw = helper.logs(tail=LOG_TAIL_LINES) + text = raw.decode("utf-8", "replace") if isinstance(raw, bytes) else str(raw) + log_tail = [line for line in text.splitlines() if line.strip()] + except Exception as e: + logger.warning(f"Could not read logs of self-upgrade helper {helper.name}: {e}") + log_tail = [] + + succeeded = on_boot and exit_code == 0 + if succeeded: + detail = "Upgrade finished; this API is the new container." + elif on_boot: + detail = ( + f"The recreate failed (exit code {exit_code}). " + "The previous container may still be running; check " + "`docker compose logs hummingbot-api` on the host." + ) + else: + detail = ( + f"The recreate did not replace this API (exit code {exit_code}); it is still " + "running the previous image. See log_tail for compose's output, then retry." + ) + with self._lock: + self._run = { + "run_id": run_id, + "phase": "done" if succeeded else "failed", + "detail": detail, + "image_ref": None, + "previous_digest": previous_digest, + "new_digest": self._current_digest(client), + "exit_code": exit_code, + "log_tail": log_tail, + "finished_at": time.time(), + } + logger.info(f"Collected self-upgrade helper {helper.name} (exit {exit_code})") + except Exception as e: + logger.warning(f"Could not read self-upgrade helper {getattr(helper, 'name', '?')}: {e}") + finally: + try: + helper.remove(force=True) + except Exception as e: + logger.warning(f"Could not remove self-upgrade helper {getattr(helper, 'name', '?')}: {e}") + + def _current_digest(self, client) -> Optional[str]: + """The digest this API is running right now, or None when it cannot be read.""" + try: + container = own_container(client) + if container is None: + return None + return describe_container(container).get("digest") + except Exception as e: + logger.warning(f"Could not read this API's own digest after an upgrade: {e}") + return None diff --git a/test/test_bot_rate_oracle_config.py b/test/test_bot_rate_oracle_config.py new file mode 100644 index 00000000..9b3246bc --- /dev/null +++ b/test/test_bot_rate_oracle_config.py @@ -0,0 +1,266 @@ +""" +Tests for the bot rate oracle config endpoints. + +The API prices through its own ticker pool, but deployed bots still run hummingbot's +RateOracle from the conf_client.yml copied out of their credentials profile. Removing +these endpoints left every bot stuck on the template's source; pinned here: the config +is read and persisted per account, invalid sources and names are rejected, and changing +master_account's global token also moves the API's own quote token. + +Run with: pytest test/test_bot_rate_oracle_config.py -v +""" +from types import SimpleNamespace + +import pytest +import yaml +from fastapi import FastAPI +from fastapi.exceptions import RequestValidationError +from fastapi.testclient import TestClient + +from utils.file_system import FileSystemUtil +from utils.validation_errors import validation_exception_handler + + +@pytest.fixture +def env(tmp_path, monkeypatch): + from deps import get_market_data_service + from routers import bot_rate_oracle + + for account in ("master_account", "other"): + (tmp_path / "credentials" / account).mkdir(parents=True) + (tmp_path / "credentials" / account / "conf_client.yml").write_text(yaml.safe_dump({ + "instance_id": "x", + "rate_oracle_source": {"name": "gate_io"}, + "global_token": {"global_token_name": "USDT", "global_token_symbol": "$"}, + })) + monkeypatch.setattr(FileSystemUtil(), "base_path", str(tmp_path)) + + service = SimpleNamespace(quote_token="USDT") + app = FastAPI() + app.include_router(bot_rate_oracle.router) + # The real app's handler, not FastAPI's default: a blank token name must come back as + # a 422 through the handler main.py installs, which is where it once became a 500. + app.add_exception_handler(RequestValidationError, validation_exception_handler) + app.dependency_overrides[get_market_data_service] = lambda: service + return SimpleNamespace(client=TestClient(app), service=service, root=tmp_path) + + +def _conf(env, account): + return yaml.safe_load((env.root / "credentials" / account / "conf_client.yml").read_text()) + + +def test_get_config_reads_account_file(env): + resp = env.client.get("/bot-orchestration/rate-oracle/config") + assert resp.status_code == 200 + body = resp.json() + assert body["rate_oracle_source"]["name"] == "gate_io" + assert body["global_token"]["global_token_name"] == "USDT" + assert "binance" in body["available_sources"] + + +def test_update_source_persists_and_keeps_other_keys(env): + resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"rate_oracle_source": {"name": "binance"}}) + assert resp.status_code == 200 + conf = _conf(env, "master_account") + assert conf["rate_oracle_source"]["name"] == "binance" + assert conf["instance_id"] == "x" + + +def test_update_other_account_does_not_touch_api_quote_token(env): + resp = env.client.put( + "/bot-orchestration/rate-oracle/config", + params={"account_name": "other"}, + json={"global_token": {"global_token_name": "USDC"}}, + ) + assert resp.status_code == 200 + assert _conf(env, "other")["global_token"] == {"global_token_name": "USDC", "global_token_symbol": "$"} + assert _conf(env, "master_account")["global_token"]["global_token_name"] == "USDT" + assert env.service.quote_token == "USDT" + + +def test_master_global_token_updates_api_quote_token(env): + env.client.put("/bot-orchestration/rate-oracle/config", json={"global_token": {"global_token_name": "USDC"}}) + assert env.service.quote_token == "USDC" + + +def test_invalid_source_rejected(env): + resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"rate_oracle_source": {"name": "nope"}}) + assert resp.status_code == 400 + assert _conf(env, "master_account")["rate_oracle_source"]["name"] == "gate_io" + + +def test_unknown_or_unsafe_account(env): + assert env.client.get("/bot-orchestration/rate-oracle/config", params={"account_name": "missing"}).status_code == 404 + assert env.client.get("/bot-orchestration/rate-oracle/config", params={"account_name": "../x"}).status_code == 400 + + +def test_blank_global_token_name_rejected(env): + resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"global_token": {"global_token_name": " "}}) + assert resp.status_code == 422 + assert _conf(env, "master_account")["global_token"]["global_token_name"] == "USDT" + assert env.service.quote_token == "USDT" + + +def test_blank_global_token_symbol_rejected(env): + resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"global_token": {"global_token_symbol": " "}}) + assert resp.status_code == 422 + assert _conf(env, "master_account")["global_token"]["global_token_symbol"] == "$" + + +def test_failed_write_leaves_live_quote_token_untouched(env, monkeypatch): + def fail(*args, **kwargs): + raise PermissionError("read-only") + + monkeypatch.setattr(FileSystemUtil(), "dump_dict_to_yaml", fail) + client = TestClient(env.client.app, raise_server_exceptions=False) + resp = client.put("/bot-orchestration/rate-oracle/config", json={"global_token": {"global_token_name": "USDC"}}) + assert resp.status_code == 500 + assert env.service.quote_token == "USDT" + + +def test_share_pct_defaults_when_the_template_has_no_key(env): + # The fixture's conf_client.yml predates the key, which is the state every existing + # credentials profile is in. The answer is hummingbot's own default, not an error. + body = env.client.get("/bot-orchestration/rate-oracle/config").json() + + assert body["rate_limits_share_pct"] == 100.0 + + +def test_update_share_pct_persists_and_leaves_the_oracle_alone(env): + resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"rate_limits_share_pct": 40}) + + assert resp.status_code == 200 + assert resp.json()["config"]["rate_limits_share_pct"] == 40.0 + conf = _conf(env, "master_account") + assert conf["rate_limits_share_pct"] == 40.0 + # A partial update touches only what it names. + assert conf["rate_oracle_source"]["name"] == "gate_io" + assert conf["instance_id"] == "x" + assert env.client.get("/bot-orchestration/rate-oracle/config").json()["rate_limits_share_pct"] == 40.0 + + +@pytest.mark.parametrize("bad", [0, -5, 100.5, 1000]) +def test_share_pct_outside_hummingbots_own_bounds_is_rejected(env, bad): + # Anything the bot's ClientConfigMap would refuse must not reach the file: the bot + # reads it back at startup, so a bad write breaks the deploy, not this request. + resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"rate_limits_share_pct": bad}) + + assert resp.status_code == 422 + assert "rate_limits_share_pct" not in _conf(env, "master_account") + + +def test_a_hand_edited_share_pct_reads_as_the_default(env): + path = env.root / "credentials" / "master_account" / "conf_client.yml" + conf = yaml.safe_load(path.read_text()) + conf["rate_limits_share_pct"] = "not a number" + path.write_text(yaml.safe_dump(conf)) + + body = env.client.get("/bot-orchestration/rate-oracle/config").json() + + assert body["rate_limits_share_pct"] == 100.0 + + +def test_share_pct_and_oracle_change_together_in_one_write(env): + resp = env.client.put( + "/bot-orchestration/rate-oracle/config", + json={"rate_oracle_source": {"name": "binance"}, "rate_limits_share_pct": 50}, + ) + + assert resp.status_code == 200 + conf = _conf(env, "master_account") + assert conf["rate_oracle_source"]["name"] == "binance" + assert conf["rate_limits_share_pct"] == 50.0 + assert "rate_limits_share_pct" in resp.json()["message"] + + +def test_share_pct_is_scoped_to_the_account_it_was_sent_for(env): + env.client.put("/bot-orchestration/rate-oracle/config", params={"account_name": "other"}, + json={"rate_limits_share_pct": 25}) + + assert _conf(env, "other")["rate_limits_share_pct"] == 25.0 + assert "rate_limits_share_pct" not in _conf(env, "master_account") + + +# ── A hand-edited or legacy file can hold anything ────────────────────────────── + + +def _rewrite(env, account, conf): + path = env.root / "credentials" / account / "conf_client.yml" + path.write_text(yaml.safe_dump(conf)) + + +@pytest.mark.parametrize("bad_global_token", ["nonsense", ["a", "b"]]) +def test_a_global_token_of_the_wrong_shape_reads_as_the_default(env, bad_global_token): + conf = _conf(env, "master_account") + conf["global_token"] = bad_global_token + _rewrite(env, "master_account", conf) + + body = env.client.get("/bot-orchestration/rate-oracle/config").json() + + assert body["global_token"] == {"global_token_name": "USDT", "global_token_symbol": "$"} + + +@pytest.mark.parametrize("bad_global_token", ["nonsense", ["a", "b"]]) +def test_updating_a_global_token_of_the_wrong_shape_replaces_it_instead_of_crashing(env, bad_global_token): + conf = _conf(env, "master_account") + conf["global_token"] = bad_global_token + _rewrite(env, "master_account", conf) + + resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"global_token": {"global_token_name": "USDC"}}) + + assert resp.status_code == 200 + assert _conf(env, "master_account")["global_token"] == {"global_token_name": "USDC"} + + +def test_a_rate_oracle_source_of_the_wrong_shape_reads_as_the_default(env): + conf = _conf(env, "master_account") + conf["rate_oracle_source"] = "nonsense" + _rewrite(env, "master_account", conf) + + body = env.client.get("/bot-orchestration/rate-oracle/config").json() + + assert body["rate_oracle_source"]["name"] == "gate_io" + + +def test_a_null_rate_oracle_source_name_reads_as_the_default(env): + conf = _conf(env, "master_account") + conf["rate_oracle_source"]["name"] = None + _rewrite(env, "master_account", conf) + + body = env.client.get("/bot-orchestration/rate-oracle/config").json() + + assert body["rate_oracle_source"]["name"] == "gate_io" + + +@pytest.mark.parametrize("field", ["global_token_name", "global_token_symbol"]) +def test_a_blank_or_numeric_global_token_field_reads_as_the_default(env, field): + conf = _conf(env, "master_account") + conf["global_token"][field] = 12345 + _rewrite(env, "master_account", conf) + + body = env.client.get("/bot-orchestration/rate-oracle/config").json() + + assert body["global_token"][field] == ({"global_token_name": "USDT", "global_token_symbol": "$"})[field] + + +def test_invalid_yaml_is_a_clean_500_not_a_crash(env): + path = env.root / "credentials" / "master_account" / "conf_client.yml" + path.write_text("global_token: [unterminated\n") + + resp = env.client.get("/bot-orchestration/rate-oracle/config") + + assert resp.status_code == 500 + assert "not valid YAML" in resp.json()["detail"] + + +def test_a_top_level_yaml_list_reads_as_empty_and_can_be_repaired(env): + path = env.root / "credentials" / "master_account" / "conf_client.yml" + path.write_text(yaml.safe_dump(["not", "a", "mapping"])) + + get_resp = env.client.get("/bot-orchestration/rate-oracle/config") + assert get_resp.status_code == 200 + assert get_resp.json()["rate_oracle_source"]["name"] == "gate_io" + + put_resp = env.client.put("/bot-orchestration/rate-oracle/config", json={"rate_oracle_source": {"name": "binance"}}) + assert put_resp.status_code == 200 + assert _conf(env, "master_account")["rate_oracle_source"]["name"] == "binance" diff --git a/test/test_self_upgrade.py b/test/test_self_upgrade.py new file mode 100644 index 00000000..1985f4d5 --- /dev/null +++ b/test/test_self_upgrade.py @@ -0,0 +1,756 @@ +"""The API replacing its own container (FEAT-122). + +The failure mode of this feature is a trading API server that does not come back, so +almost everything here is a refusal. `preflight` starts blocked and only clears that once +every fact it needs is established, and `start` re-runs it, so the tests worth having are +the ones that prove nothing happens: + +* an unreachable daemon, a container it cannot place, a stack with no compose labels +* a *pinned* image -- built on the box, or an override file that can repoint `image:` -- + which is the moneymaker case and must name the override file +* a registry it cannot read, which is "no evidence a newer image exists", not "go ahead" +* an executor count it cannot take, because a restart closes every running executor +* running executors without the caller acknowledging that loss +* a pull that fails, which must leave the server untouched and never start the helper + +The happy path is here too, and what it asserts is the exact `docker compose` the helper +is given: the project, every config file in the order compose was given them, `--no-deps` +so postgres/emqx/bots are not touched, and the working dir bound at the *same* host path +so `./bots`, `.env` and the override file resolve to what the operator actually has. + +Run with: pytest test/test_self_upgrade.py -v +""" +import threading +from types import SimpleNamespace + +import pytest +from docker.errors import DockerException, NotFound +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from services.self_upgrade import ( + HELPER_IMAGE, + HELPER_LABEL, + HELPER_PREVIOUS_DIGEST_LABEL, + HELPER_PROJECT_LABEL, + SERVICE_NAME, + SelfUpgradeRefused, + SelfUpgradeService, +) + +CONTAINER_ID = "3f2a1b0c9d8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3b2c1d0e9f8a7b6c5d4e3f2a" # noqa: mock (a Docker container id, not a key) +WORKING_DIR = "/home/hbot/hummingbot-api" +IMAGE_REF = "hummingbot/hummingbot-api:latest" +CURRENT_SHA = "sha256:1111111111111111111111111111111111111111111111111111111111111111" # noqa: mock +NEWER_SHA = "sha256:2222222222222222222222222222222222222222222222222222222222222222" # noqa: mock +COMPOSE_FILES = f"{WORKING_DIR}/docker-compose.yml,{WORKING_DIR}/docker-compose.tailscale.yml" + + +# ── Doubles ────────────────────────────────────────────────────────────────────── + + +def _container(digests=(f"hummingbot/hummingbot-api@{CURRENT_SHA}",), config_files=COMPOSE_FILES): + """A docker-py Container as utils/compose.py reads it.""" + labels = { + "com.docker.compose.project": "hummingbot-api", + "com.docker.compose.project.working_dir": WORKING_DIR, + "com.docker.compose.service": SERVICE_NAME, + } + if config_files is not None: + labels["com.docker.compose.project.config_files"] = config_files + return SimpleNamespace( + id=CONTAINER_ID, + name="hummingbot-api", + labels=labels, + attrs={"Config": {"Image": IMAGE_REF}}, + image=SimpleNamespace( + id="sha256:deadbeef", # noqa: mock + tags=[IMAGE_REF], + attrs={"RepoDigests": list(digests)}, + ), + ) + + +def _helper(status="exited", exit_code=0, run_id="run123", logs=b"Container hummingbot-api Started\n", + project="hummingbot-api"): + """A helper container as _pull_then_recreate creates one, for this test file's own + stack by default (see _container(), whose compose project is also "hummingbot-api"). + Pass a different `project` (or None) to simulate another stack's helper. + """ + removed = [] + labels = {HELPER_LABEL: run_id, HELPER_PREVIOUS_DIGEST_LABEL: f"hummingbot/hummingbot-api@{CURRENT_SHA}"} + if project is not None: + labels[HELPER_PROJECT_LABEL] = project + return SimpleNamespace( + name=f"helper-{run_id}", + status=status, + labels=labels, + attrs={"State": {"ExitCode": exit_code}}, + logs=lambda tail=None: logs, + remove=lambda force=False: removed.append(force), + removed=removed, + ) + + +class _Containers: + def __init__(self, container, helpers=()): + self.container = container + self.helpers = list(helpers) + self.runs = [] + + def get(self, name): + if self.container is None: + raise NotFound(f"no such container: {name}") + return self.container + + def list(self, all=False, filters=None): + label = (filters or {}).get("label", "") + if label.startswith(HELPER_LABEL): + return [h for h in self.helpers if not getattr(h, "removed", None)] + # The compose-service fallback own_container() uses. + return [self.container] if self.container is not None else [] + + def run(self, image, **kwargs): + self.runs.append({"image": image, **kwargs}) + return SimpleNamespace(name="helper-new") + + +class _Images: + def __init__(self, registry_sha=NEWER_SHA): + self.registry_sha = registry_sha + + def get_registry_data(self, ref): + if self.registry_sha is None: + raise DockerException(f"error while looking up {ref}") + return SimpleNamespace(id=self.registry_sha) + + +class _Client: + def __init__(self, container=None, helpers=(), registry_sha=NEWER_SHA, reachable=True): + self.containers = _Containers(container, helpers) + self.images = _Images(registry_sha) + self.reachable = reachable + + def ping(self): + if not self.reachable: + raise DockerException("Error while fetching server API version") + return True + + +class _DockerService: + """Just the two things SelfUpgradeService uses off the real one.""" + + def __init__(self, client, pull_fails=()): + self.client = client + self.pull_fails = set(pull_fails) + self.pulled = [] + + def pull_image_sync(self, image_name): + self.pulled.append(image_name) + if image_name in self.pull_fails: + return {"success": False, "error": f"500 Server Error for http+docker://localhost/v1.55/images/create: {image_name}"} + return {"success": True, "image": image_name} + + +class _Executors: + def __init__(self, active=0, raises=False): + self.active = active + self.raises = raises + + def get_summary(self): + if self.raises: + raise RuntimeError("executor service is not up") + return {"total_active": self.active} + + +class _Bots: + def __init__(self, names=("bot-a", "bot-b")): + self.names = list(names) + + async def get_active_containers(self): + return list(self.names) + + +def _service(client=None, pull_fails=(), active=0, executor_raises=False, bots=None): + if client is None: + client = _Client(_container()) + return SelfUpgradeService( + docker_service=_DockerService(client, pull_fails), + executor_service=_Executors(active, executor_raises), + bots_orchestrator=_Bots() if bots is None else bots, + ) + + +@pytest.fixture +def run_threads_inline(monkeypatch): + """Run the upgrade's background thread on the calling thread, so tests are ordered.""" + started = [] + + class _Inline: + def __init__(self, target=None, args=(), daemon=None, name=None): + self.target, self.args = target, args + + def start(self): + started.append(self) + self.target(*self.args) + + monkeypatch.setattr(threading, "Thread", _Inline) + return started + + +# ── Preflight refuses ──────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_refuses_when_the_docker_daemon_is_unreachable(): + info = await _service(_Client(_container(), reachable=False)).preflight() + + assert info["can_upgrade"] is False + assert "docker.sock" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_when_the_docker_client_was_never_built(): + service = SelfUpgradeService(docker_service=SimpleNamespace(), executor_service=_Executors()) + + info = await service.preflight() + + assert info["can_upgrade"] is False + assert "Docker daemon is not reachable" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_when_it_cannot_identify_its_own_container(): + info = await _service(_Client(container=None)).preflight() + + assert info["can_upgrade"] is False + assert "could not identify its own container" in info["blocked_reason"] + assert "SSH" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_when_there_are_no_compose_labels(): + """Not a compose deployment: there is no `docker compose up` to replay.""" + info = await _service(_Client(_container(config_files=None))).preflight() + + assert info["can_upgrade"] is False + assert info["compose"] is None + assert "not started by Docker Compose" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_a_pinned_image_and_names_the_override_file(): + """The moneymaker case: an override file can repoint `image:` at a pinned tag.""" + files = f"{WORKING_DIR}/docker-compose.yml,{WORKING_DIR}/docker-compose.override.yml" + + info = await _service(_Client(_container(config_files=files))).preflight() + + assert info["can_upgrade"] is False + assert info["pinned"] is True + assert info["override_file"] == "docker-compose.override.yml" + assert "docker-compose.override.yml" in info["blocked_reason"] + assert "SSH" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_a_locally_built_image(): + """No RepoDigests: nothing published this image, so the published one is not it.""" + info = await _service(_Client(_container(digests=()))).preflight() + + assert info["can_upgrade"] is False + assert info["pinned"] is True + assert "built locally" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_when_the_registry_digest_cannot_be_read(): + """No evidence a newer image exists is not the same as "go ahead".""" + service = _service(_Client(_container(), registry_sha=None)) + + info = await service.preflight() + + assert info["can_upgrade"] is False + assert info["available_digest"] is None + assert "no evidence that a newer image exists" in info["blocked_reason"] + assert service.docker_service.pulled == [] + + +@pytest.mark.asyncio +async def test_refuses_when_already_running_the_published_image(): + info = await _service(_Client(_container(), registry_sha=CURRENT_SHA)).preflight() + + assert info["can_upgrade"] is False + assert info["up_to_date"] is True + assert "Already running" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_when_the_executor_count_cannot_be_taken(): + """A restart closes every running executor; not knowing how many is not "none".""" + info = await _service(executor_raises=True).preflight() + + assert info["can_upgrade"] is False + assert info["running_executors"] is None + assert "Refusing rather than guessing" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_while_a_helper_container_is_present(): + info = await _service(_Client(_container(), helpers=[_helper(status="running")])).preflight() + + assert info["can_upgrade"] is False + assert "still running" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_refuses_when_it_cannot_check_for_an_existing_helper(): + """Two `compose up` runs racing for the same container is the worst outcome here.""" + service = _service() + + def _boom(all=False, filters=None): + raise DockerException("daemon said no") + + service.docker_service.client.containers.list = _boom + + info = await service.preflight() + + assert info["can_upgrade"] is False + assert "Refusing rather than risking two recreates" in info["blocked_reason"] + + +@pytest.mark.asyncio +async def test_a_second_start_is_refused_while_the_first_is_in_flight(run_threads_inline): + service = _service() + + await service.start() + with pytest.raises(SelfUpgradeRefused) as excinfo: + await service.start() + + assert "already running" in str(excinfo.value) + # Still exactly one helper: the second call started nothing. + assert len(service.docker_service.client.containers.runs) == 1 + + +@pytest.mark.asyncio +async def test_refuses_while_a_run_is_in_flight(): + service = _service() + service._run = {"run_id": "abc", "phase": "recreating"} + + info = await service.preflight() + + assert info["can_upgrade"] is False + assert "already running" in info["blocked_reason"] + + +# ── Preflight allows ───────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_allows_an_unpinned_compose_install_with_a_newer_image(): + info = await _service(active=3).preflight() + + assert info["can_upgrade"] is True + assert info["blocked_reason"] is None + assert info["image_ref"] == IMAGE_REF + assert info["current_digest"].endswith(CURRENT_SHA) + assert info["available_digest"] == NEWER_SHA + assert info["up_to_date"] is False + assert info["pinned"] is False + assert info["running_executors"] == 3 + assert info["running_bots"] == 2 + assert info["compose"] == { + "project": "hummingbot-api", + "working_dir": WORKING_DIR, + "config_files": [f"{WORKING_DIR}/docker-compose.yml", f"{WORKING_DIR}/docker-compose.tailscale.yml"], + } + + +@pytest.mark.asyncio +async def test_a_tailscale_overlay_is_not_a_pin(): + """Only a file named *override* can repoint `image:`; an overlay is networking.""" + info = await _service().preflight() + + assert info["pinned"] is False + assert info["override_file"] is None + assert info["can_upgrade"] is True + + +@pytest.mark.asyncio +async def test_counts_are_reported_even_on_a_blocked_preflight(): + """"You cannot upgrade here" and "17 executors are running" are both the answer.""" + files = f"{WORKING_DIR}/docker-compose.yml,{WORKING_DIR}/docker-compose.override.yml" + + info = await _service(_Client(_container(config_files=files)), active=17).preflight() + + assert info["can_upgrade"] is False + assert info["running_executors"] == 17 + assert info["running_bots"] == 2 + + +# ── Start refuses ──────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_start_refuses_a_blocked_preflight_without_touching_anything(): + service = _service(_Client(_container(), registry_sha=CURRENT_SHA)) + + with pytest.raises(SelfUpgradeRefused) as excinfo: + await service.start(acknowledge_executor_loss=True) + + assert "Already running" in str(excinfo.value) + assert service.docker_service.pulled == [] + assert service.docker_service.client.containers.runs == [] + assert service.status()["phase"] == "idle" + + +@pytest.mark.asyncio +async def test_start_refuses_running_executors_without_acknowledgement(): + service = _service(active=4) + + with pytest.raises(SelfUpgradeRefused) as excinfo: + await service.start() + + assert "4 executor(s) are running" in str(excinfo.value) + assert "SYSTEM_CLEANUP" in str(excinfo.value) + assert service.docker_service.pulled == [] + assert service.docker_service.client.containers.runs == [] + + +@pytest.mark.asyncio +async def test_start_refuses_a_pinned_server_even_when_the_loss_is_acknowledged(): + """Acknowledging the executor loss does not buy past the pin.""" + files = f"{WORKING_DIR}/docker-compose.yml,{WORKING_DIR}/docker-compose.override.yml" + service = _service(_Client(_container(config_files=files)), active=1) + + with pytest.raises(SelfUpgradeRefused) as excinfo: + await service.start(acknowledge_executor_loss=True) + + assert "docker-compose.override.yml" in str(excinfo.value) + assert service.docker_service.client.containers.runs == [] + + +# ── Start proceeds ─────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_start_pulls_both_images_then_runs_the_helper(run_threads_inline): + service = _service() + + started = await service.start() + + assert started["phase"] == "pulling" + assert started["run_id"] + # Both images on the box before anything is stopped. + assert service.docker_service.pulled == [IMAGE_REF, HELPER_IMAGE] + + run = service.docker_service.client.containers.runs[0] + assert run["image"] == HELPER_IMAGE + assert run["detach"] is True + assert run["command"] == [ + "docker", "compose", "-p", "hummingbot-api", + "-f", f"{WORKING_DIR}/docker-compose.yml", + "-f", f"{WORKING_DIR}/docker-compose.tailscale.yml", + "up", "-d", "--no-deps", SERVICE_NAME, + ] + # Same host path on both sides, or ./bots, .env and the override file resolve to + # nothing inside the helper. + assert run["volumes"][WORKING_DIR]["bind"] == WORKING_DIR + assert run["volumes"]["/var/run/docker.sock"]["bind"] == "/var/run/docker.sock" + assert run["working_dir"] == WORKING_DIR + assert run["labels"][HELPER_LABEL] == started["run_id"] + assert run["labels"][HELPER_PREVIOUS_DIGEST_LABEL].endswith(CURRENT_SHA) + assert run["labels"][HELPER_PROJECT_LABEL] == "hummingbot-api" + # The record has to outlive the helper; auto_remove would delete it. + assert run["auto_remove"] is False + + status = service.status() + assert status["phase"] == "recreating" + assert status["previous_digest"].endswith(CURRENT_SHA) + + +@pytest.mark.asyncio +async def test_start_proceeds_when_the_executor_loss_is_acknowledged(run_threads_inline): + service = _service(active=2) + + await service.start(acknowledge_executor_loss=True) + + assert len(service.docker_service.client.containers.runs) == 1 + + +@pytest.mark.asyncio +async def test_no_remove_orphans_and_only_the_api_service_is_recreated(run_threads_inline): + """postgres, emqx, any sidecar and every bot container must be left alone.""" + service = _service() + + await service.start() + + command = service.docker_service.client.containers.runs[0]["command"] + assert "--remove-orphans" not in command + assert "--no-deps" in command + assert command[-1] == SERVICE_NAME + + +# ── A pull that fails changes nothing ──────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_a_failed_api_image_pull_never_starts_the_helper(run_threads_inline): + service = _service(pull_fails=[IMAGE_REF]) + + await service.start() + + assert service.docker_service.client.containers.runs == [] + status = service.status() + assert status["phase"] == "failed" + assert "Nothing was changed" in status["detail"] + # The daemon's own string names the socket and API version; it stays in the log. + assert "http+docker" not in status["detail"] + + +@pytest.mark.asyncio +async def test_a_failed_helper_image_pull_never_starts_the_helper(run_threads_inline): + """The helper image is pulled up front precisely so this failure is harmless.""" + service = _service(pull_fails=[HELPER_IMAGE]) + + await service.start() + + assert service.docker_service.client.containers.runs == [] + assert service.status()["phase"] == "failed" + + +@pytest.mark.asyncio +async def test_a_helper_that_cannot_be_started_is_reported_not_raised(run_threads_inline): + service = _service() + + def _boom(image, **kwargs): + raise DockerException("no such image") + + service.docker_service.client.containers.run = _boom + + await service.start() + + status = service.status() + assert status["phase"] == "failed" + assert "Nothing was changed" in status["detail"] + + +# ── Collecting a helper that did not replace this API ──────────────────────────── + + +@pytest.mark.asyncio +async def test_preflight_collects_a_failed_helper_instead_of_blocking_on_it(): + # The helper exited non-zero and this API was never replaced, so no boot will ever + # collect it. Blocking on it would need SSH to clear -- what this endpoint replaces. + helper = _helper(exit_code=1, logs=b"service bogus-service depends on undefined service\n") + service = _service(_Client(_container(), helpers=[helper])) + + info = await service.preflight() + + assert info["can_upgrade"] is True + assert helper.removed == [True] + status = service.status() + assert status["phase"] == "failed" + assert status["exit_code"] == 1 + assert status["log_tail"] == ["service bogus-service depends on undefined service"] + + +@pytest.mark.asyncio +async def test_status_does_not_stay_recreating_after_the_helper_fails(run_threads_inline): + client = _Client(_container()) + service = _service(client) + run_id = (await service.start())["run_id"] + assert service.status()["phase"] == "recreating" + + client.containers.helpers.append(_helper(exit_code=1, run_id=run_id)) + + status = service.status() + assert status["phase"] == "failed" + assert status["run_id"] == run_id + assert status["exit_code"] == 1 + assert "did not replace this API" in status["detail"] + + +@pytest.mark.asyncio +async def test_a_helper_that_exits_zero_without_replacing_this_api_is_not_done(): + # We are still here, so the recreate did not happen whatever compose's exit code. + helper = _helper(exit_code=0) + service = _service(_Client(_container(), helpers=[helper])) + + await service.preflight() + + assert service.status()["phase"] == "failed" + assert helper.removed == [True] + + +@pytest.mark.asyncio +async def test_status_leaves_a_helper_that_is_still_running(run_threads_inline): + client = _Client(_container()) + service = _service(client) + run_id = (await service.start())["run_id"] + helper = _helper(status="running", run_id=run_id) + client.containers.helpers.append(helper) + + assert service.status()["phase"] == "recreating" + assert helper.removed == [] + + +@pytest.mark.asyncio +async def test_an_idle_api_does_not_adopt_or_remove_another_stacks_helper(): + """A host running more than one hummingbot-api stack: an idle API -- one that has + never started an upgrade -- must not adopt a foreign helper's record as its own, nor + remove it and rob the stack that actually ran it of its result. + """ + foreign_helper = _helper(exit_code=7, run_id="FOREIGN_RUN_9999", project="some-other-stack") + service = _service(_Client(_container(), helpers=[foreign_helper])) + + info = await service.preflight() + + assert info["can_upgrade"] is True + assert foreign_helper.removed == [] + assert service.status() == {"run_id": None, "phase": "idle", "detail": None, "log_tail": []} + + +@pytest.mark.asyncio +async def test_a_tracked_run_does_not_adopt_a_helper_from_a_different_run_in_the_same_project(run_threads_inline): + """A helper carrying this API's own compose project but a different run id is not the + run this process is tracking -- e.g. a leftover from a run this process never started. + """ + client = _Client(_container()) + service = _service(client) + await service.start() + + other_helper = _helper(exit_code=0, run_id="some-other-run") + client.containers.helpers.append(other_helper) + + assert service.status()["phase"] == "recreating" + assert other_helper.removed == [] + + +# ── Collecting the run on boot ─────────────────────────────────────────────────── + + +def test_collect_on_boot_reports_a_successful_recreate_and_removes_the_helper(): + helper = _helper(exit_code=0) + client = _Client(_container(digests=(f"hummingbot/hummingbot-api@{NEWER_SHA}",)), helpers=[helper]) + service = _service(client) + + service.collect_on_boot() + + status = service.status() + assert status["phase"] == "done" + assert status["run_id"] == "run123" + assert status["exit_code"] == 0 + assert status["previous_digest"].endswith(CURRENT_SHA) + assert status["new_digest"].endswith(NEWER_SHA) + assert helper.removed == [True] + + +def test_collect_on_boot_reports_a_failed_recreate_with_the_log_tail(): + helper = _helper(exit_code=1, logs=b"service hummingbot-api: error\nvolume not found\n") + service = _service(_Client(_container(), helpers=[helper])) + + service.collect_on_boot() + + status = service.status() + assert status["phase"] == "failed" + assert status["exit_code"] == 1 + assert status["log_tail"] == ["service hummingbot-api: error", "volume not found"] + assert "docker compose logs hummingbot-api" in status["detail"] + # Still removed: a failed run must not block the next preflight forever. + assert helper.removed == [True] + + +def test_collect_on_boot_leaves_a_helper_that_is_still_running(): + helper = _helper(status="running") + service = _service(_Client(_container(), helpers=[helper])) + + service.collect_on_boot() + + assert helper.removed == [] + assert service.status()["phase"] == "idle" + + +def test_collect_on_boot_leaves_a_helper_from_another_compose_project(): + """The exited helper belongs to a different hummingbot-api deployment on the same + host; adopting it here would report the wrong stack's upgrade as this one's, and + removing it would erase the only record the other stack has of its own run. + """ + helper = _helper(exit_code=0, run_id="FOREIGN_RUN_9999", project="some-other-stack") + service = _service(_Client(_container(), helpers=[helper])) + + service.collect_on_boot() + + assert helper.removed == [] + assert service.status()["phase"] == "idle" + + +def test_collect_on_boot_survives_an_unreachable_daemon(): + service = _service(_Client(_container(), reachable=False)) + + service.collect_on_boot() + + assert service.status()["phase"] == "idle" + + +def test_status_is_idle_before_anything_has_run(): + assert _service().status() == {"run_id": None, "phase": "idle", "detail": None, "log_tail": []} + + +# ── The routes ─────────────────────────────────────────────────────────────────── + + +@pytest.fixture +def make_client(): + """A TestClient whose /system/upgrade/* talks to the given SelfUpgradeService.""" + from deps import get_self_upgrade_service + from routers import system + + def _make(service): + app = FastAPI() + app.include_router(system.router) + app.dependency_overrides[get_self_upgrade_service] = lambda: service + return TestClient(app) + + return _make + + +def test_preflight_route_reports_the_refusal_rather_than_failing(make_client): + body = make_client(_service(_Client(_container(), reachable=False))).get("/system/upgrade/preflight") + + assert body.status_code == 200 + assert body.json()["can_upgrade"] is False + assert body.json()["blocked_reason"] + + +def test_upgrade_route_answers_409_when_refused(make_client): + response = make_client(_service(active=2)).post("/system/upgrade", json={}) + + assert response.status_code == 409 + assert "2 executor(s) are running" in response.json()["detail"] + + +def test_upgrade_route_with_no_body_acknowledges_nothing(make_client): + """An empty body must be read as consent withheld, not consent given.""" + response = make_client(_service(active=2)).post("/system/upgrade") + + assert response.status_code == 409 + + +def test_upgrade_route_answers_202_when_it_starts(make_client, run_threads_inline): + service = _service(active=2) + + response = make_client(service).post("/system/upgrade", json={"acknowledge_executor_loss": True}) + + assert response.status_code == 202 + assert response.json()["phase"] == "pulling" + assert response.json()["run_id"] + + +def test_status_route_serves_the_collected_run(make_client): + service = _service(_Client(_container(), helpers=[_helper(exit_code=0)])) + service.collect_on_boot() + + body = make_client(service).get("/system/upgrade/status").json() + + assert body["phase"] == "done" + assert body["run_id"] == "run123" diff --git a/test/test_system_info.py b/test/test_system_info.py new file mode 100644 index 00000000..83017101 --- /dev/null +++ b/test/test_system_info.py @@ -0,0 +1,302 @@ +""" +Tests for the /system/info endpoint. + +The dashboard needs to tell which hummingbot-api and hummingbot version a server runs, +and whether its image is the published one or something pinned on the box. That answer +comes from the API's own container over the mounted docker.sock, which is exactly the +thing that can be missing -- so pinned here: the happy path reports the digest and the +compose labels, a locally built image and a compose override file each read as pinned, +and a daemon that cannot be reached still answers with both versions. + +Run with: pytest test/test_system_info.py -v +""" +from types import SimpleNamespace + +import pytest +from docker.errors import DockerException, NotFound +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from version import VERSION + +CONTAINER_ID = "3f2a1b0c9d8e7f6a5b4c3d2e1f0a9b8c7d6e5f4a3b2c1d0e9f8a7b6c5d4e3f2a" # noqa: mock (a Docker container id, not a key) +WORKING_DIR = "/home/hbot/hummingbot-api" + + +def _container( + digests=("hummingbot/hummingbot-api@sha256:abc123",), + config_files=f"{WORKING_DIR}/docker-compose.yml", + image="hummingbot/hummingbot-api:latest", + name="hummingbot-api", + project="hummingbot-api", +): + """A docker-py Container as this route reads it.""" + return SimpleNamespace( + id=CONTAINER_ID, + name=name, + labels={ + "com.docker.compose.project": project, + "com.docker.compose.project.working_dir": WORKING_DIR, + "com.docker.compose.project.config_files": config_files, + "com.docker.compose.service": "hummingbot-api", + }, + attrs={"Config": {"Image": image}}, + image=SimpleNamespace( + id="sha256:deadbeef", + tags=[image], + attrs={"RepoDigests": list(digests)}, + ), + ) + + +class _Containers: + def __init__(self, container, found_by_hostname=True, others=(), found_by_id=None): + self.container = container + self.found_by_hostname = found_by_hostname + self.found_by_id = found_by_id + self.others = list(others) + self.list_filters = None + + def get(self, name): + if self.found_by_hostname or (self.found_by_id and name == self.found_by_id): + return self.container + raise NotFound(f"no such container: {name}") + + def list(self, filters=None): + self.list_filters = filters + return ([self.container] if self.container is not None else []) + self.others + + +class _Client: + def __init__(self, container, found_by_hostname=True, reachable=True, others=(), found_by_id=None): + self.containers = _Containers(container, found_by_hostname, others, found_by_id) + self.reachable = reachable + + def ping(self): + if not self.reachable: + raise DockerException("Error while fetching server API version") + return True + + +@pytest.fixture(autouse=True) +def inside_a_container(monkeypatch): + """Run as the containerised API by default, with no container id in the mount table.""" + from utils import compose + + monkeypatch.setattr(compose, "_container_id_from_mountinfo", lambda: None) + monkeypatch.setattr(compose, "_in_container", lambda: True) + return monkeypatch + + +@pytest.fixture +def make_client(): + """Build a TestClient whose /system/info talks to the given fake docker client.""" + from deps import get_docker_service + from routers import system + + def _make(docker_client): + app = FastAPI() + app.include_router(system.router) + app.dependency_overrides[get_docker_service] = lambda: SimpleNamespace(client=docker_client) + return TestClient(app) + + return _make + + +def test_reports_versions_and_container(make_client): + body = make_client(_Client(_container())).get("/system/info").json() + + assert body["api_version"] == VERSION + assert body["hummingbot_version"] # the library is installed in the test environment + assert body["docker_available"] is True + assert body["container"] == { + "id": CONTAINER_ID, + "name": "hummingbot-api", + "image": "hummingbot/hummingbot-api:latest", + "image_id": "sha256:deadbeef", + "digest": "hummingbot/hummingbot-api@sha256:abc123", + "compose_project": "hummingbot-api", + "compose_working_dir": WORKING_DIR, + "compose_config_files": f"{WORKING_DIR}/docker-compose.yml", + } + assert body["pinned"] is False + assert body["pinned_reason"] is None + assert body["override_file"] is None + + +def test_locally_built_image_is_pinned(make_client): + body = make_client(_Client(_container(digests=()))).get("/system/info").json() + + assert body["container"]["digest"] is None + assert body["pinned"] is True + assert "locally" in body["pinned_reason"] + assert body["override_file"] is None + + +def test_locally_built_image_on_the_containerd_store_is_pinned(make_client): + # The containerd image store gives a local build a RepoDigest of its own; it is not a + # digest in the published repository, so it does not make the image the published one. + sha = "sha256:f84fd1ab6802c4434a8ea709c63a7a99c30b4aaa80e828fc508d1ebb2e65f806" # noqa: mock + container = _container(digests=(f"hbapi@{sha}",), image="hbapi:pr235") + body = make_client(_Client(container)).get("/system/info").json() + + assert body["container"]["digest"] is None + assert body["pinned"] is True + assert "locally" in body["pinned_reason"] + + +def test_a_fully_qualified_published_digest_is_the_published_image(make_client): + container = _container(digests=("docker.io/hummingbot/hummingbot-api@sha256:abc123",)) + body = make_client(_Client(container)).get("/system/info").json() + + assert body["container"]["digest"] == "docker.io/hummingbot/hummingbot-api@sha256:abc123" + assert body["pinned"] is False + + +def test_a_version_tag_in_the_main_compose_file_is_pinned(make_client): + # Pulled from the registry, so it has a published digest -- but upgrading it would + # pull the same tag back, not latest. + container = _container(image="hummingbot/hummingbot-api:1.0.1") + body = make_client(_Client(container)).get("/system/info").json() + + assert body["pinned"] is True + assert "1.0.1" in body["pinned_reason"] + assert body["override_file"] is None + + +def test_an_image_pinned_by_digest_is_pinned(make_client): + container = _container(image="hummingbot/hummingbot-api@sha256:abc123") + assert make_client(_Client(container)).get("/system/info").json()["pinned"] is True + + +def test_override_file_is_pinned_and_named(make_client): + container = _container( + config_files=f"{WORKING_DIR}/docker-compose.yml,{WORKING_DIR}/docker-compose.override.yml", + ) + body = make_client(_Client(container)).get("/system/info").json() + + assert body["pinned"] is True + assert body["override_file"] == "docker-compose.override.yml" + assert "docker-compose.override.yml" in body["pinned_reason"] + + +def test_extra_overlay_that_is_not_an_override_is_not_pinned(make_client): + # The Tailscale overlay repoints networking, not the image. + container = _container( + config_files=f"{WORKING_DIR}/docker-compose.yml,{WORKING_DIR}/docker-compose.tailscale.yml", + ) + assert make_client(_Client(container)).get("/system/info").json()["pinned"] is False + + +def test_falls_back_to_the_compose_service_label(make_client): + client = _Client(_container(), found_by_hostname=False) + body = make_client(client).get("/system/info").json() + + assert client.containers.list_filters == {"label": "com.docker.compose.service=hummingbot-api"} + assert body["container"]["name"] == "hummingbot-api" + + +def test_finds_itself_by_container_id_when_the_hostname_is_overridden(make_client, inside_a_container): + from utils import compose + + inside_a_container.setattr(compose, "_container_id_from_mountinfo", lambda: CONTAINER_ID) + decoy = _container(name="decoy-api", project="some-other-stack", image="alpine:latest") + client = _Client(_container(), found_by_hostname=False, found_by_id=CONTAINER_ID, others=[decoy]) + body = make_client(client).get("/system/info").json() + + assert body["container"]["name"] == "hummingbot-api" + assert client.containers.list_filters is None # never needed the label fallback + + +def test_two_candidates_for_the_service_label_report_no_container(make_client): + # Another stack on the same host runs a hummingbot-api service too. Picking one would + # report its image and its pinning verdict as this server's. + decoy = _container(name="decoy-api", project="some-other-stack", image="alpine:latest") + body = make_client(_Client(_container(), found_by_hostname=False, others=[decoy])).get("/system/info").json() + + assert body["docker_available"] is True + assert body["container"] is None + assert body["pinned"] is None + + +def test_running_from_source_does_not_claim_a_container(make_client, inside_a_container): + # From source this process is no container at all; the containerised API on the same + # host carries the service label, but it is not us. + from utils import compose + + inside_a_container.setattr(compose, "_in_container", lambda: False) + client = _Client(_container(), found_by_hostname=False) + body = make_client(client).get("/system/info").json() + + assert body["container"] is None + assert body["pinned"] is None + assert client.containers.list_filters is None + + +def test_docker_unavailable_still_reports_versions(make_client): + body = make_client(_Client(_container(), reachable=False)).get("/system/info").json() + + assert body["docker_available"] is False + assert body["container"] is None + assert body["pinned"] is None + assert body["pinned_reason"] is None + assert body["api_version"] == VERSION + assert body["hummingbot_version"] + + +def test_container_not_identified_reports_no_container(make_client): + client = _Client(None, found_by_hostname=False) + body = make_client(client).get("/system/info").json() + + assert body["docker_available"] is True + assert body["container"] is None + assert body["pinned"] is None + assert body["api_version"] == VERSION + + +def test_docker_service_without_a_client_never_raises(): + """DockerService leaves `client` unset when the daemon was absent at startup.""" + from deps import get_docker_service + from routers import system + + app = FastAPI() + app.include_router(system.router) + app.dependency_overrides[get_docker_service] = lambda: SimpleNamespace() + + body = TestClient(app).get("/system/info").json() + assert body["docker_available"] is False + assert body["container"] is None + assert body["api_version"] == VERSION + + +def test_reports_the_market_data_tunables(make_client): + # The dashboard shows these read-only: they come from the server's .env, which is not + # mounted into the container, so the API can report them but never rewrite them. + from config import settings + + body = make_client(_Client(_container())).get("/system/info").json() + + assert body["market_data"] == settings.market_data.model_dump() + assert body["market_data"]["ticker_update_interval"] == settings.market_data.ticker_update_interval + + +def test_market_data_tunables_carry_no_secret(make_client): + # MarketDataSettings is intervals and timeouts by construction. Pinned so a later + # field that does not belong in an unauthenticated-looking payload fails here first. + body = make_client(_Client(_container())).get("/system/info").json() + + assert body["market_data"] + assert all(isinstance(v, (int, float)) for v in body["market_data"].values()) + assert not any( + word in key.lower() + for key in body["market_data"] + for word in ("password", "secret", "token", "key", "user") + ) + + +def test_market_data_is_reported_without_docker(make_client): + # The versions and the tunables are the part that works from source too. + body = make_client(_Client(_container(), reachable=False)).get("/system/info").json() + + assert body["docker_available"] is False + assert body["market_data"] diff --git a/utils/compose.py b/utils/compose.py new file mode 100644 index 00000000..475e9931 --- /dev/null +++ b/utils/compose.py @@ -0,0 +1,189 @@ +"""How this API's own container was deployed, read from Docker Compose's own labels. + +Compose stamps every container it creates with the project name, the directory it was +run from and the config files it was run with. Those labels are the only record on the +box of how the deployment is put together -- there is nothing to configure and nothing +to guess -- so both things that need to know read them from here: + +* ``GET /system/info`` (FEAT-121) reports them, and reports whether the running image is + the published one or something pinned on the box. +* ``services/self_upgrade.py`` (FEAT-122) *refuses to upgrade* when it is pinned, and + rebuilds the exact ``docker compose -f ... up`` the operator would have typed. + +They share this module rather than each reading the labels themselves because the two +answers have to agree. A panel that says "not pinned" over an upgrade path that refuses +as pinned is a confusing afternoon; a panel that says "not pinned" over an upgrade path +that *proceeds* on a hand-built image is a trading API server replaced by a published +build nobody asked for. +""" + +import os +import re +import socket + +# The image this API is published as. An image from any other repository -- a local +# `docker build -t hbapi .`, a fork -- is by definition not the published one. +PUBLISHED_REPO = "hummingbot/hummingbot-api" +PUBLISHED_TAG = "latest" + +# Compose service this API is deployed as, used to find our own container when the +# hostname lookup fails (see own_container). +COMPOSE_SERVICE = "hummingbot-api" + +# Compose stamps these on every container it creates; they say which project the API +# belongs to and which files were used to bring it up. +LABEL_PROJECT = "com.docker.compose.project" +LABEL_WORKING_DIR = "com.docker.compose.project.working_dir" +LABEL_CONFIG_FILES = "com.docker.compose.project.config_files" + + +def _container_id_from_mountinfo(): + """This process's own container id, read from the files Docker bind-mounts into it. + + Docker mounts /var/lib/docker/containers//{hostname,hosts,resolv.conf} into every + container, so the id is in our own mount table whatever `hostname:` was set to. + """ + try: + with open("/proc/self/mountinfo") as f: + match = re.search(r"/containers/([0-9a-f]{64})/", f.read()) + except OSError: + return None + return match.group(1) if match else None + + +def _in_container(): + return os.path.exists("/.dockerenv") or _container_id_from_mountinfo() is not None + + +def own_container(client): + """Find the container this API process runs in. + + Docker sets a container's hostname to its own short id, which is how a process + identifies itself from the inside; when the deployment overrides `hostname:` the id + is still in the mount table. Only when both miss, and only from inside a container, + fall back to the compose service label -- and only when exactly one container carries + it. A process running from source on the host is not any container, and with two + candidates picking one would report another deployment's image and pinning as ours: + None is the honest answer in both cases. + + Returns: + A docker-py Container, or None when this process cannot be placed. + """ + for ref in (socket.gethostname(), _container_id_from_mountinfo()): + if not ref: + continue + try: + return client.containers.get(ref) + except Exception: + pass + if not _in_container(): + return None + try: + matches = client.containers.list(filters={"label": f"com.docker.compose.service={COMPOSE_SERVICE}"}) + except Exception: + return None + return matches[0] if len(matches) == 1 else None + + +def _repository(ref): + """The repository of an image reference, without tag, digest or the Docker Hub host. + + ``docker.io/hummingbot/hummingbot-api:latest`` and ``hummingbot/hummingbot-api@sha256:..`` + are both ``hummingbot/hummingbot-api``. A colon after the last slash is a tag; one + before it is a registry port. + """ + repo = (ref or "").split("@")[0] + if ":" in repo.rsplit("/", 1)[-1]: + repo = repo.rsplit(":", 1)[0] + for host in ("docker.io/", "index.docker.io/"): + if repo.startswith(host): + repo = repo[len(host):] + return repo + + +def _tag(ref): + """The tag of an image reference; an untagged reference means ``latest``.""" + name = (ref or "").split("@")[0].rsplit("/", 1)[-1] + return name.rsplit(":", 1)[1] if ":" in name else "latest" + + +def describe_container(container): + """Identity, image and compose provenance of a container. + + Returns: + Dictionary with the container's id and name, the image reference it was started + from, the image id, its digest in the published repository (None when it has + none, e.g. a locally built image) and the three compose labels that say where the + deployment lives. + """ + labels = container.labels or {} + image = container.image + # Only a digest in the published repository says the image came from it. With the + # legacy image store a local build has no RepoDigests at all, but with the containerd + # store (the default on current Docker Desktop) `docker build -t hbapi .` gets one too + # -- `hbapi@sha256:...` -- and taking that as a registry digest reads a hand-built + # image as the published one. + digests = [ + d for d in ((image.attrs.get("RepoDigests") or []) if image is not None else []) + if _repository(d) == PUBLISHED_REPO + ] + return { + "id": container.id, + "name": container.name, + "image": container.attrs.get("Config", {}).get("Image"), + "image_id": image.id if image is not None else None, + "digest": digests[0] if digests else None, + "compose_project": labels.get(LABEL_PROJECT), + "compose_working_dir": labels.get(LABEL_WORKING_DIR), + "compose_config_files": labels.get(LABEL_CONFIG_FILES), + } + + +def config_files(described): + """The compose config files of a described container, as a list of host paths. + + The label holds them comma-separated in the order they were passed to compose, and + that order is load-bearing: later files override earlier ones, so replaying it as + `-f a -f b` is the only way to recreate the container the operator actually has. + """ + raw = (described or {}).get("compose_config_files") or "" + return [f.strip() for f in raw.split(",") if f.strip()] + + +def pinning(described): + """Whether this deployment runs something other than the published image. + + Three things make an image not the one `hummingbot/hummingbot-api:latest` currently + resolves to: it was built on the box or comes from another repository (no digest in + the published one), it runs a tag other than `latest` (a published version pinned in + docker-compose.yml itself), or compose was brought up with an override file that can + repoint `image:` at a pinned tag. Only files whose name contains "override" count -- + `docker-compose.tailscale.yml` is an overlay for networking, not for the image. + + The one case this cannot see is an image built locally *as* + `hummingbot/hummingbot-api:latest` on the containerd store, which carries a digest in + the published repository like a pulled image does; nothing local tells them apart. + + Returns: + Dictionary with pinned, a short pinned_reason and the override file name (None + when that is not the reason). + """ + files = config_files(described) + override_file = next((os.path.basename(f) for f in files if "override" in os.path.basename(f).lower()), None) + + image_ref = (described or {}).get("image") + reasons = [] + if not (described or {}).get("digest"): + reasons.append(f"image was built locally or is not from {PUBLISHED_REPO} (no published digest)") + elif "@" in (image_ref or ""): + reasons.append("image is pinned to a digest") + elif _tag(image_ref) != PUBLISHED_TAG: + reasons.append(f"image is pinned to tag {_tag(image_ref)}") + if override_file: + reasons.append(f"compose override file {override_file} can pin the image") + + return { + "pinned": bool(reasons), + "pinned_reason": "; ".join(reasons) or None, + "override_file": override_file, + } diff --git a/utils/validation_errors.py b/utils/validation_errors.py new file mode 100644 index 00000000..6cf5871f --- /dev/null +++ b/utils/validation_errors.py @@ -0,0 +1,38 @@ +"""The app-wide handler for request validation errors. + +Lives outside main.py so tests can mount the handler the real app uses: a router tested +on a bare FastAPI() gets FastAPI's default handler instead, which is how a 422 that the +real app could not render went unnoticed. +""" + +import logging + +from fastapi import Request +from fastapi.encoders import jsonable_encoder +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse + + +async def validation_exception_handler(request: Request, exc: RequestValidationError): + """ + Custom handler for validation errors to log detailed error messages. + """ + # Build a readable error message from validation errors + error_messages = [] + for error in exc.errors(): + loc = " -> ".join(str(part) for part in error.get("loc", [])) + msg = error.get("msg", "Validation error") + error_messages.append(f"{loc}: {msg}") + + # Log the validation error with details + logging.warning( + f"Validation error on {request.method} {request.url.path}: {'; '.join(error_messages)}" + ) + + # Return standard FastAPI validation error response. jsonable_encoder is what makes a + # validator that raises ValueError renderable: pydantic puts the exception object + # itself in the error's ctx, and JSONResponse cannot serialize it. + return JSONResponse( + status_code=422, + content={"detail": jsonable_encoder(exc.errors())}, + ) diff --git a/version.py b/version.py new file mode 100644 index 00000000..59ac9278 --- /dev/null +++ b/version.py @@ -0,0 +1,9 @@ +"""The API server's own version number. + +It lives in its own module, with no imports of its own, because main.py imports every +router: a router that needs the version -- /system/info reports it -- cannot import it +back from main.py without a circular import. main.py re-exports it, and the Docker +build workflow reads the assignment below with grep, so keep it a plain literal. +""" + +VERSION = "1.0.1"