Repository navigation
Delegate hummingbot-api setup, resolve tailnet ownership, bind the dashboard reachably - #231
Merged
Merged
Conversation
Condor hand-wrote hummingbot-api's .env and ran `docker compose up -d` directly. Two authors meant two schemas, and the deploy skipped the Makefile prerequisite that seeds the broker. - Never overwrite an existing install. The .env write had no existence guard, and api_health_check only sees a *live* API on :8000, so a stopped-but-installed stack fell through to the deploy branch. A smoke test of that path replaced a generated BROKER_PASSWORD with `password` and dropped BROKER_DASHBOARD_PASSWORD, while the broker's own bootstrap file kept the old credential -- a mismatch that needs `make emqx-auth-reset` to repair, not a re-run. The prompt now defaults to n and the existing .env becomes read-only input. - Run hummingbot-api's setup.sh non-interactively instead of writing its .env here. Condor shipped BROKER_PASSWORD=password (the default that repo deliberately stopped shipping), omitted BROKER_DASHBOARD_PASSWORD so compose fell back to another well-known default, and wrote API_BIND_HOST -- a key nothing reads, since docker-compose.yml wants API_BIND. - Deploy with `make deploy`. It depends on emqx-auth, which generates .emqx/auth-bootstrap.csv. Skipping it left the bind-mount source absent, so Docker created it as a root-owned DIRECTORY, EMQX logged boostrap_authn_built_in_database_failed and came up with auth enabled and zero accounts -- every MQTT connection refused while the HTTP health check passed and the installer reported success. - Drop the generated Tailscale overlay and the Makefile string-patch. Against current main all three patch replacements match nothing and silently no-op while still printing "Makefile patched"; upstream already ships both the overlay and a Tailscale-aware deploy target. - Point a co-located pre-existing API at localhost rather than the hummingbot-api MagicDNS name, which routed back to the same machine by a longer path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The deploy branch installed Tailscale and ran `tailscale up --hostname=condor` unconditionally. A smoke test with a daemon already running produced output byte-identical to one without, because nothing ever looked -- the remote-API branch has always checked, this one never did. On a co-located install that meant a second kernel-mode daemon contending for tailscale0, which is fatal and silent. - tailnet_state(): sidecar | native | none, mirroring hummingbot-api's tailnet-state.sh. Duplicated deliberately -- Condor installs on machines where that checkout does not exist. Detects the resource, not the sibling product, so it also sees a Tailscale the admin installed for their own SSH access. - Join only when nothing here already has. Otherwise reuse the node. - No auth key prompt when this machine is already on the tailnet; there is no new node to register. A key is still requested if the user asks for a separate node for hummingbot-api. - The second-node question now picks a mode rather than a doomed sidecar: yes -> TAILSCALE_MODE=sidecar (hummingbot-api's deploy runs it in userspace mode, which coexists), no -> TAILSCALE_MODE=host, serving port 8000 on the node this host already has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…back
Three deployments, three answers:
local machine, no Tailscale 127.0.0.1 -> localhost:8088
VPS, no Tailscale 0.0.0.0 -> VPS_IP:8088
Tailscale on <tailnet ip> -> reachable on the tailnet,
and on no public interface
Previously the Tailscale case bound 127.0.0.1 and relied on
`tailscale serve` to proxy. That never worked: the two-positional
serve syntax is rejected by current clients ("invalid argument
format", 1.102.3), and ensure_serve runs unprivileged while the daemon
socket refuses non-root outright -- nothing in the installer ever ran
`tailscale set --operator`. The dashboard was reachable from nowhere,
and WEB_URL still advertised localhost:8088.
A bind needs no privilege and no daemon round-trip. Serve remains only
as the fallback for when the tailnet address cannot be resolved, with
its syntax corrected and the privilege requirement named in the error.
resolve_web_host fails CLOSED: an unresolvable tailnet address narrows
to loopback, never widens to 0.0.0.0. Falling back to a public bind
would invert the guarantee the setting exists to provide.
Also:
- WEB_URL now follows the bind. The rewrite was guarded on SERVER_IP
AND the tailnet IP, but SERVER_IP is only prompted in the
non-Tailscale branch -- so on the Tailscale path it was never set,
the rewrite never ran, and the installer's closing summary printed
"Web dashboard URL: http://localhost:8088" on a VPS.
- doctor: a loopback bind under USE_TAILSCALE is no longer reported OK.
It is the fail-closed fallback and means "reachable from this machine
only" -- calling it success is what hid the broken serve. A tailnet
bind is OK; anything else warns and says what to check.
- doctor: hummingbot-api (and this node's own tailnet address) count as
local, so _local_stack_diagnosis() runs. A co-located API that was
down reported "check the host is reachable -- firewall, Tailscale
status, or the server is down" instead of naming `docker compose ps`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only Telegram mode takes the tailnet bind. Local mode has no login at all, and a tailnet is a smaller audience than the internet, not an authenticated one -- "everyone on the tailnet" is still more than "nobody". So the mode check now runs BEFORE the Tailscale check in resolve_web_host, and localhost:8088 keeps working on a local install regardless of whether a tailnet exists. local mode (any) 127.0.0.1 -> localhost:8088 telegram, no Tailscale 0.0.0.0 -> VPS_IP:8088 telegram + Tailscale <tailnet ip> -> tailnet only Downstream of that: - main.py treats a loopback bind in local mode as the intended end state: no `tailscale serve` fallback and no warning, because nothing failed. The fallback path still fires for Telegram mode, where a loopback bind means the tailnet address could not be resolved. - doctor applies the Tailscale bind expectations only outside local mode, so a correct local-mode install no longer reads as a warning. - A public bind in local mode is now FAIL rather than WARN: it can only happen by setting WEB_HOST explicitly, and it exposes a dashboard with no login. The remedy text no longer says "Enable Tailscale" when Tailscale is already on. - setup-environment.sh leaves WEB_URL (and its closing summary) at localhost:8088 in local mode instead of rewriting them to the tailnet address. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
check_dashboard_port now branches on LOCAL_MODE, so a test that set only USE_TAILSCALE inherited the mode from whatever .env happened to be on disk -- passing on a fresh clone and failing once a local-mode .env existed. Pin it, and cover the branches the change added: a public bind in local mode fails rather than warns, loopback under Tailscale is OK only in local mode, and a tailnet bind is the goal state. Verified against a local-mode .env, a telegram-mode .env, and no .env. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Greptile SummaryThe PR delegates Hummingbot API configuration and deployment to its upstream setup, resolves host-versus-sidecar tailnet ownership, and binds the dashboard according to its configured exposure policy.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| setup-environment.sh | Delegates API setup, separates configured and healthy deployment states, removes the unsafe raw Compose fallback, and bases tailnet decisions on host usability and device occupancy. |
| utils/config.py | Resolves dashboard binds to loopback, public interfaces, or the node’s tailnet address according to mode and explicit overrides. |
| utils/tailscale.py | Adds tailnet-address discovery and retains fail-closed loopback forwarding behavior. |
| main.py | Starts the dashboard with the resolved bind and uses Tailscale Serve only as a loopback fallback. |
| condor/doctor.py | Diagnoses dashboard bind-policy violations and recognizes co-located Hummingbot API hosts more accurately. |
| tests/test_doctor.py | Adds coverage for dashboard exposure policy and local API diagnosis. |
| tests/test_local_mode.py | Adds assertions for mode-dependent and Tailscale-dependent dashboard bind resolution. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
Setup[Condor setup] --> Existing{Hummingbot API .env exists?}
Existing -->|Yes| Preserve[Preserve upstream environment]
Existing -->|No| Delegate[Run upstream setup.sh]
Preserve --> Deploy{make available?}
Delegate -->|Success| Deploy
Delegate -->|Failure| Stop[Skip deployment and credential sync]
Deploy -->|Yes| MakeDeploy[Run make deploy with broker bootstrap]
Deploy -->|No| Configured[Report configured but not running]
MakeDeploy --> Health{HTTP API healthy?}
Health -->|Yes| Running[Report API running]
Health -->|No| Configured
Setup --> Tailnet{Usable host Tailscale node?}
Tailnet -->|Yes| Reuse[Reuse host node]
Tailnet -->|No| Device{tailscale0 occupied?}
Device -->|Yes| Disable[Do not attempt conflicting enrollment]
Device -->|No| Enroll[Enroll native host node]
Reviews (9): Last reviewed commit: "doctor: a remote API is not local just b..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
From the PR review. With only a hummingbot-api sidecar on the machine, the reuse branch treated it as this host's own tailnet node and skipped native enrollment. It is not: a sidecar is a separate tailnet identity whose socket lives inside its container, so the following `tailscale ip -4` returned nothing. Setup then selected host mode and reported "Tailscale connected" with no address -- a dashboard reachable from nowhere, announced as working. - Only a NATIVE daemon counts as reusable. tailnet_has_native asks the host's tailscaled socket, which a sidecar cannot answer for; `ip link` and `pgrep` are consulted only when no sidecar is running, since a kernel-mode one creates tailscale0 in the host's own namespace and would otherwise read as native. - A kernel-mode sidecar OWNS tailscale0, so Condor cannot enroll natively alongside it either -- walking into that is the same collision this change exists to prevent. Detected (sidecar running AND tailscale0 present) and reported with the two-line fix in ../hummingbot-api, rather than attempted and failed. - "Tailscale connected" is now claimed only when an address was actually obtained. Without one there is no tailnet bind and no working /web link, so announcing success moved the failure somewhere harder to diagnose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
507 commits landed on main since this branch was cut. Only setup-environment.sh conflicted; main.py auto-merged cleanly. The conflict was substantive rather than textual. main added backtesting tuning to the hand-written hummingbot-api .env block that this branch deletes in favour of delegating to that repo's own setup.sh: BACKTESTING_MAX_CONCURRENT=4 BACKTESTING_TIMEOUT_SECONDS=1800 Taking this branch's side wholesale would have silently dropped them and serialised mm_optimizer_cycle's sweeps behind hummingbot-api's default of one concurrent worker. They are preserved by passing them through the non-interactive contract instead of writing them here, which keeps .env with a single author -- the property this branch exists to establish. Verified on the merged tree: 5089 passed, 23 skipped; the bind matrix and doctor branches behave as before; and a condor-driven install carries both backtesting values through to hummingbot-api's .env with the broker password still generated there.
Two review findings. 1. When the delegated setup.sh failed or was absent, both branches only logged the error and fell through into `make deploy` and an unconditional hb_api_deployed=true. That deployed a stack with no .env -- containers that cannot authenticate to their own broker -- and then reported "credentials saved" and "Hummingbot API is running", sending anyone debugging it to the wrong place. The deploy and the bookkeeping are now gated on the API actually being configured, and DEPLOY_HUMMINGBOT_API is recorded false so the config.yml credential sync does not run against credentials that were never written. 2. A tailscaled belonging to the sidecar could be classified as native when the installing user cannot run `docker ps`. Detection now tells them apart by cgroup rather than by whether the container is visible, which needs no docker permissions. See the matching change in hummingbot-api for why the cgroup, and not pgrep alone, is the portable discriminator. Belt and braces: reusing an existing node now also requires `tailscale ip -4` to actually return an address. Detection can be fooled, and the failure mode of guessing wrong is an install that reports success with no address and a dashboard on localhost. The address is the thing that is actually needed, so it is required rather than inferred.
hb_api_deployed conflated two different facts. When configuration succeeded but `make deploy` failed, Docker was unavailable, or the API never answered, it still became true -- and the closing summary said "Hummingbot API is running" for a stack that had not started. Configured and running are now tracked separately. The config.yml credential sync still keys on configured, which is correct: those credentials are valid whether or not the containers came up, and the API can be started later. The summary keys on the health check actually passing, and otherwise says the API is configured but not answering yet, with the command to check it. Also mirrors hummingbot-api's tailnet ownership fix: a foreign host-networked Tailscale container is contention, not our own sidecar.
…owns what A sidecar's tailscaled could still be classified as native: the setup user may be unable to query Docker, and its cgroup may be unreadable or carry no recognised container marker. With no host-side address, setup then fell through to enrolling this machine -- running `tailscale up` against the tailscale0 the sidecar was already holding, which fails and leaves both the dashboard and the API unreachable. The requirement that reuse yield a real address made that worse rather than better: it converted a wrong reuse into a wrong enrollment. The decision no longer attributes a process to a host or a container at all. It asks two questions that cannot be got wrong: can we USE a host daemon? tailscale status answers AND yields an address is the device taken? tailscale0 already exists Reuse needs the first. Enrolling needs the absence of the second. When neither holds -- something owns the device and it is not ours to use -- setup says so, names the two-line fix in ../hummingbot-api, and continues without Tailscale rather than attempting a join that cannot succeed. The tailnet predicate is removed from this script as a result: nothing here called it any more, and leaving it would imply a contract this file no longer has. It stays in hummingbot-api, where `make deploy` and doctor genuinely need to tell a host daemon from a sidecar in order to choose userspace mode -- and where guessing wrong is merely an unnecessary downgrade rather than a failed enrollment.
…ng out The collision guard depended on `ip link show tailscale0`. With iproute2 absent that read as "device free", so a kernel-mode sidecar holding the device went unseen and setup enrolled straight into it. Rather than special-case `ip`, every external command these decisions depend on was audited for what its absence implies. Two failed open -- this one and pgrep, fixed in hummingbot-api's shared helper. The rest (docker, tailscale, curl, ss/lsof) already biased toward the safe answer. tailnet_device_present asks /sys/class/net first, which is present on every Linux kernel and needs no tools at all, keeping ip and ifconfig as fallbacks. A host with neither /sys nor those tools is not Linux, and a network_mode: host container cannot hold the host's device there, so "absent" is correct rather than a guess. Also from the review: bailing out of Tailscale left USE_TAILSCALE=true in .env from Step 1. main.py would then attempt a tailnet bind, fail closed to loopback, and doctor would report the dashboard unreachable -- each correct alone, and all confusing right after "continuing without Tailscale". The flag is now cleared with the decision. And an explicit WEB_HOST is no longer logged as a failed tailnet lookup: it is a deliberate opt-out, not an address we could not find.
When make was missing, the deploy fell back to raw `docker compose up -d` -- bypassing the emqx-auth prerequisite and producing a broker with authentication enabled and no accounts. That is precisely the failure this PR exists to fix, reintroduced five lines below the comment explaining why it must not happen, and invisible to the HTTP health check that follows. make is a hard requirement for a correct deploy, so a missing one now says so and stops. The honest outcome is a configured API that has not been started, which the summary already reports accurately, rather than a started one that cannot authenticate. The two remediation hints that also suggested raw `docker compose up -d` now say `make deploy` for the same reason. `docker compose ps` is left alone, being read-only.
Three faults found driving the install flow through a live tailnet. **The MagicDNS name was assumed, never resolved.** Tailscale suffixes a hostname that is already taken, so on a tailnet that already has a `hummingbot-api` the machine we deployed is `hummingbot-api-1` and the bare name is somebody else's. Theirs is very likely running the same software, so it answers, and the install ends up pointed at it -- surfacing as "401 Incorrect username or password", which sends people to check a password that was never wrong. Driven for real, Condor configured a colleague's node and doctor reported exactly that 401. The remote branch now searches the tailnet once it is on it: one match is used, several are listed to choose from (they are different machines, which is the whole point), none asks rather than guessing. The co-located branch asks the sidecar what name it got, after deploy, when there is something to ask. **The hand-started tailscaled did not survive the installer.** The WSL2 branch backgrounded `sudo tailscaled &` with no service, no setsid and no nohup, and it was gone the moment `make setup` returned -- taking tailscale0 and the serve config with it, while setup had already printed "Tailscale connected" and written a tailnet dashboard URL. On a systemd host it also left tailscaled.service inactive and never enabled, so nothing brought it back on reboot either; /proc/version says microsoft on WSL2, which has shipped with systemd on by default since 0.67, so the branch fired on hosts that had a perfectly good service manager sitting there. Ask the service manager first, always, and enable the unit so it survives a reboot. Only where there is no init to ask do we start it ourselves, now detached, and say out loud that it will not come back on its own. The redirection moves inside sudo: written as `sudo tailscaled ... >/var/log/...` the calling shell opens that path as the invoking user and fails before sudo runs at all, which is the silent non-start this function exists to prevent. **Nothing checked that the API answers.** Everything the remote branch writes is a claim about another machine. Probe it before reporting success, and give each failure the diagnosis it needs: 401 on a tailnet is the name collision above; a refused connection is almost always API_BIND, since hummingbot-api publishes 8000 on 127.0.0.1 and a completely healthy remote install is unreachable until that is widened -- which nothing said at the point you chose a remote API. A local host gets local advice instead, and a box without curl is never blocked.
…t-api
`hummingbot-api` is the MagicDNS name a co-located API answers to when it
has its own tailnet node, which is why it was added to _LOCAL_HOSTS: without
it a co-located API that was down reported "check the host is reachable"
instead of the one-line `docker compose ps` answer.
But it is equally the host Condor writes for a *remote* API over Tailscale
-- that is this installer's own default for that path -- and there the
local-stack diagnosis names a directory that is not there:
✗ API 'local' (hummingbot-api:8000) 401, 'Incorrect username or password'
✗ hummingbot-api container not running — start it with
cd ../hummingbot-api && make deploy
on a machine that was never meant to have the stack. The name alone cannot
tell the two apart. Whether the install is on this disk can, so ask that.
david-hummingbot
added a commit
that referenced
this pull request
Sep 29, 2026
…isions Two things the widened suffix in this branch exposed. Greptile's point first: widening `-[0-9]+` to `-.+` widens what can reach the count==1 branch, and that branch selects silently. A lone `hummingbot-api-staging`, a node someone rebuilt, another desk's API -- each is alone on a tailnet where the intended one has not joined yet, and each is now accepted and written into config.yml with only a "Found it" to show for it. The wrong node is very likely running the same software, so it answers, and the mistake arrives as "401 Incorrect username or password" against a password that was never wrong -- the exact failure #231 set out to stop. Being the only match is not evidence of being the right match. So count==1 splits. The unsuffixed `hummingbot-api` is still taken silently: it is the name hummingbot-api's own setup asks for, and there is nothing to choose between. A suffixed lone match is shown and confirmed, pre-filled, so Enter accepts it when it is right -- which is the common case and the whole point of the search. It costs a keystroke and it means no name the operator never chose lands in config.yml unseen. Second, the pick-one message asserted "Tailscale adds -1, -2 ... when a name is taken, so these are different machines. Pick the one you just deployed." Running several APIs on purpose -- hummingbot-api-1, hummingbot-api-2 alongside the default -- is an ordinary deployment, and there those suffixes are chosen names, not collision artifacts. The old wording told that operator their own naming was an accident, and "the one you just deployed" is the wrong instruction anyway when Condor is being pointed at an instance that has been up for weeks. It now covers both origins and asks which node this Condor should talk to. Plus the assertions over the helper this branch described but never committed: 16 of them, sourcing the real function out of the script with a stubbed `tailscale` rather than restating the regex in Python (a copy of the regex asserts only that the copy matches itself). Four fail against the old `-[0-9]+`. Full suite 5109 passed, 23 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Condor reimplemented hummingbot-api's setup instead of delegating to it, and the two implementations had drifted far enough apart that every Condor-driven deploy shipped a broker with no accounts. This delegates instead, stops joining the tailnet twice on one machine, and puts the dashboard on an address that can actually reach it.
The
.envproblemThe write at
setup-environment.shhad no existence guard, andapi_health_checkonly sees a live API on:8000— so a stopped-but-installed stack fell straight through to the deploy branch. Driving that path against a healthy standalone install:Not repairable by re-running setup; it needs
make emqx-auth-reset.Condor now defaults that prompt to n when an install exists, reads its credentials, and never writes. On a fresh install it runs hummingbot-api's own
setup.shnon-interactively and deploys withmake deploy, which carries theemqx-authprerequisite that seeds the broker.The generated Tailscale overlay and the Makefile string-patch are gone: against current
mainall three patch replacements match nothing and silently no-op while still printing "Makefile patched" — upstream already ships both.Joining the tailnet twice
The deploy branch installed Tailscale and ran
tailscale up --hostname=condorunconditionally. A smoke test with a daemon already running produced output byte-identical to one without, because nothing ever looked — the remote-API branch has always checked, this one never did. Two kernel-mode daemons cannot sharetailscale0; the loser dies withdevice or resource busyinside a container that keeps reporting running.Condor now reuses an existing node, and the second-node question picks a mode (
sidecar→ userspace, which coexists;host→ serve on the node this machine already has).Port 8088
Three deployments, three answers:
127.0.0.1localhost:80880.0.0.0VPS_IP:8088Previously the Tailscale case bound loopback and relied on
tailscale serve, which needs the daemon socket and refuses unprivileged callers — and used a syntax current clients reject. The dashboard was reachable from nowhere whileWEB_URLadvertisedlocalhost:8088, because the rewrite was guarded onSERVER_IP, which is only prompted in the non-Tailscale branch.resolve_web_hostfails closed: an unresolvable tailnet address narrows to loopback, never widens to0.0.0.0. Local mode is checked first — a tailnet is a smaller audience than the internet, not an authenticated one.Doctor
A loopback bind under
USE_TAILSCALEis no longer reported OK outside local mode; a public bind in local mode is now a failure; andhummingbot-apicounts as a local host so_local_stack_diagnosis()fires instead of "check the host is reachable" for a service on the same machine.Verification
Full suite: 2965 passed, 5 skipped, re-run against a local-mode
.env, a telegram-mode.env, and no.env— one existing test was inheriting its mode from whatever was on disk. 11 new bind-policy assertions plus doctor branch coverage.Wizard scenarios driven end to end through a pty in a container with
docker/tailscale/curlstubbed. Parses under bash 3.2 (macOS default); the tailnet predicate degrades correctly whereipdoes not exist.Not verified: MagicDNS resolution and ACL behaviour, which need a live multi-host tailnet.
Requires hummingbot/hummingbot-api#229 for the non-interactive contract.
🤖 Generated with Claude Code