diff --git a/connito/test/test_telemetry_port_resolution.py b/connito/test/test_telemetry_port_resolution.py new file mode 100644 index 0000000..fd8a13a --- /dev/null +++ b/connito/test/test_telemetry_port_resolution.py @@ -0,0 +1,86 @@ +"""Tests for `resolve_telemetry_port`. + +`CONNITO_TELEMETRY_PORT` shipped in the validator image for months but was +never read by any code — the exporter port was hardcoded to `8200 + rank`. +An operator whose host already had 8200 occupied would set the variable, +see no effect, and end up with a validator that ran fine on chain while the +exporter failed to bind ("Address already in use") and served nothing. + +The variable is a **base** port, not an absolute one: the effective port is +`base + rank`, matching the semantics of the 8200 default it replaces, so a +multi-rank deployment on one host stays collision-free. +""" +from __future__ import annotations + +from connito.validator.run import DEFAULT_TELEMETRY_BASE_PORT, resolve_telemetry_port + + +# --------------------------------------------------------------------------- +# Default / fallback behaviour +# --------------------------------------------------------------------------- + +def test_default_when_env_unset(): + assert resolve_telemetry_port(0, env={}) == 8200 + + +def test_default_applies_rank_offset(): + assert resolve_telemetry_port(1, env={}) == 8201 + assert resolve_telemetry_port(3, env={}) == 8203 + + +def test_blank_and_whitespace_are_treated_as_unset(): + assert resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": ""}) == 8200 + assert resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": " "}) == 8200 + + +# --------------------------------------------------------------------------- +# Override behaviour — the case this exists for +# --------------------------------------------------------------------------- + +def test_override_is_used_verbatim_at_rank_zero(): + # The validator always runs rank 0, so an operator setting 8201 gets 8201. + assert resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": "8201"}) == 8201 + + +def test_override_is_a_base_not_an_absolute_port(): + # Multi-rank stays collision-free: an absolute override would point every + # rank at the same port and all but one would fail to bind. + env = {"CONNITO_TELEMETRY_PORT": "9100"} + assert resolve_telemetry_port(0, env=env) == 9100 + assert resolve_telemetry_port(1, env=env) == 9101 + assert resolve_telemetry_port(2, env=env) == 9102 + + +def test_override_tolerates_surrounding_whitespace(): + assert resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": " 8201 "}) == 8201 + + +# --------------------------------------------------------------------------- +# Invalid input must never take the validator down +# --------------------------------------------------------------------------- + +def test_non_numeric_falls_back_to_default(): + assert resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": "not-a-port"}) == 8200 + + +def test_out_of_range_falls_back_to_default(): + for bad in ("0", "-1", "65536", "999999"): + assert resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": bad}) == 8200 + + +def test_base_plus_rank_overflow_falls_back_to_default(): + # 65535 is a legal base but overflows once rank is added. + assert resolve_telemetry_port(1, env={"CONNITO_TELEMETRY_PORT": "65535"}) == ( + DEFAULT_TELEMETRY_BASE_PORT + 1 + ) + + +def test_float_string_falls_back_rather_than_truncating(): + # Silently truncating "8201.9" to 8201 would be a surprising success. + assert resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": "8201.9"}) == 8200 + + +def test_resolution_never_raises_on_hostile_input(): + for bad in ("", " ", "abc", "8201;rm -rf /", "0x2009", "١٢٣"): + port = resolve_telemetry_port(0, env={"CONNITO_TELEMETRY_PORT": bad}) + assert isinstance(port, int) and 1 <= port <= 65535 diff --git a/connito/validator/docker/Dockerfile b/connito/validator/docker/Dockerfile index bdd5f4e..c080549 100644 --- a/connito/validator/docker/Dockerfile +++ b/connito/validator/docker/Dockerfile @@ -33,13 +33,20 @@ ENV DEBIAN_FRONTEND=noninteractive \ PIP_DISABLE_PIP_VERSION_CHECK=1 \ LANG=C.UTF-8 \ LC_ALL=C.UTF-8 \ - # Default Hivemind/DHT + telemetry + state-API ports — overridable at - # runtime. Note: connito.validator.run currently hardcodes the telemetry - # and state-API ports as `8200 + rank` / `8300 + rank`, so these env - # vars are documentation-only until run.py is updated to read them. - CONNITO_DHT_PORT=6000 \ + # Base port for the Prometheus exporter. Read by + # `connito.validator.run.resolve_telemetry_port`; the effective port is + # `CONNITO_TELEMETRY_PORT + rank` (rank is 0 for the validator). Override + # via the compose `.env` when 8200 is already taken on the host — and + # remember to open the new port in your firewall. + # + # NOTE: the Hivemind/DHT port is NOT an env var. It is configured by + # `dht.port` in validator.yaml, which is the single source of truth — + # that port is announced to peers, so a second competing setting could + # make a validator advertise one port while listening on another. The + # former CONNITO_DHT_PORT / CONNITO_STATE_API_PORT vars were never read + # by any code and have been removed (the /v1/state.json API they + # referenced no longer exists). CONNITO_TELEMETRY_PORT=8200 \ - CONNITO_STATE_API_PORT=8300 \ CONNITO_GIT_SHA=${GIT_SHA} \ CONNITO_GIT_VERSION=${GIT_VERSION} @@ -91,9 +98,10 @@ ENV CHECKPOINTS_DIR=/data/checkpoints \ # 6000/tcp + 6000/udp: hivemind DHT (config.dht.port) # 8000/tcp: chain serve port (config.chain.port) -# 8200/tcp: prometheus telemetry (8200 + rank) -> /metrics -# 8300/tcp: validator state API (8300 + rank) -> /v1/state.json, /healthz -EXPOSE 6000/tcp 6000/udp 8000/tcp 8200/tcp 8300/tcp +# 8200/tcp: prometheus telemetry (CONNITO_TELEMETRY_PORT + rank) -> /metrics +# Documentation only — the compose file uses `network_mode: host`, so these +# are not published mappings. 8300 (the removed /v1/state.json API) is gone. +EXPOSE 6000/tcp 6000/udp 8000/tcp 8200/tcp # Default command — override `--path` to point at your config in compose. ENTRYPOINT ["python", "-m", "connito.validator.run"] diff --git a/connito/validator/docker/docker-compose.yml b/connito/validator/docker/docker-compose.yml index 18f641f..a24595a 100644 --- a/connito/validator/docker/docker-compose.yml +++ b/connito/validator/docker/docker-compose.yml @@ -94,6 +94,12 @@ services: HF_TOKEN: ${HF_TOKEN:-} HF_HOME: /data/hf TRANSFORMERS_CACHE: /data/hf + # Base port for the Prometheus exporter (effective port is this + rank; + # rank is 0 for the validator). Set CONNITO_TELEMETRY_PORT in .env when + # 8200 is already taken on the host, and open the new port in your + # firewall. Must be passed through explicitly — a value in .env alone + # only interpolates into this file, it does not reach the container. + CONNITO_TELEMETRY_PORT: ${CONNITO_TELEMETRY_PORT:-8200} volumes: # Wallets — read-only is safest; the validator only signs with the hotkey. @@ -123,7 +129,9 @@ services: # headroom and added curl --max-time so the probe itself can't hang past # the docker timeout. healthcheck: - test: ["CMD", "curl", "-fsS", "--max-time", "30", "http://localhost:8200/metrics"] + # Follows CONNITO_TELEMETRY_PORT so overriding the port doesn't leave + # the container permanently unhealthy probing a port nothing binds. + test: ["CMD", "curl", "-fsS", "--max-time", "30", "http://localhost:${CONNITO_TELEMETRY_PORT:-8200}/metrics"] interval: 60s timeout: 60s retries: 10 diff --git a/connito/validator/run.py b/connito/validator/run.py index 1f3ffd2..925b80a 100644 --- a/connito/validator/run.py +++ b/connito/validator/run.py @@ -201,6 +201,64 @@ def validate_hf_distribution_config(config: ValidatorConfig) -> tuple[str | None from connito.shared.memory import cleanup, release_cpu_ram +# Default base port for the Prometheus exporter. Overridable per host via +# the `CONNITO_TELEMETRY_PORT` env var — see `resolve_telemetry_port`. +DEFAULT_TELEMETRY_BASE_PORT = 8200 + + +def resolve_telemetry_port(rank: int, env: dict[str, str] | None = None) -> int: + """Resolve the port for the Prometheus exporter. + + `CONNITO_TELEMETRY_PORT` is a **base** port, not an absolute one: the + effective port is `base + rank`. That preserves the semantics of the + 8200 default it replaces, and keeps a multi-rank deployment on one host + collision-free — an absolute override would point every rank at the same + port and all but one would fail to bind, which is exactly the bug this + override exists to fix. For the validator `rank` is always 0, so + `CONNITO_TELEMETRY_PORT=8201` yields 8201. + + Operators need this when the default 8200 is already taken on the host: + before this was wired up the env var existed in the image but was never + read, so the exporter kept trying 8200, failed with "Address already in + use", and the validator ran on with no telemetry at all. + + Invalid input falls back to the default with a warning rather than + raising — a typo in an operator's `.env` must not take a validator off + chain over a telemetry setting. + """ + env = os.environ if env is None else env + raw = str(env.get("CONNITO_TELEMETRY_PORT", "") or "").strip() + base = DEFAULT_TELEMETRY_BASE_PORT + if raw: + try: + parsed = int(raw) + if not (1 <= parsed <= 65535): + raise ValueError(f"port out of range: {parsed}") + base = parsed + except ValueError as e: + logger.warning( + "Invalid CONNITO_TELEMETRY_PORT — falling back to default base port", + value=raw, + default_base=DEFAULT_TELEMETRY_BASE_PORT, + error=str(e), + ) + port = base + int(rank) + if not (1 <= port <= 65535): + logger.warning( + "Resolved telemetry port out of range — falling back to default", + base=base, rank=rank, resolved=port, + default_base=DEFAULT_TELEMETRY_BASE_PORT, + ) + base = DEFAULT_TELEMETRY_BASE_PORT + port = base + int(rank) + if base != DEFAULT_TELEMETRY_BASE_PORT: + logger.info( + "Telemetry port overridden via CONNITO_TELEMETRY_PORT", + base=base, rank=rank, port=port, + ) + return port + + @track_metagraph_sync_latency() def _sync_lite_metagraph(subtensor, netuid: int): """Validator-side metagraph fetch via lite_subtensor. @@ -700,8 +758,7 @@ def run(rank: int, world_size: int, config: ValidatorConfig, pkg_version: str = None """ # Start the integrated Prometheus telemetry server - # Port 8200+rank to avoid conflicts with other services on this host - telemetry_port = 8200 + rank + telemetry_port = resolve_telemetry_port(rank) TelemetryManager().start_server(port=telemetry_port) if rank == 0: