feat(docker): add attachable Hubble Compose add-on for the 3-node cluster - #3149
feat(docker): add attachable Hubble Compose add-on for the 3-node cluster#3149bitflicker64 wants to merge 15 commits into
Conversation
…ster Add docker-compose-hubble.yml, a Hubble-only add-on that joins the cluster's pre-created external network (HUGEGRAPH_NETWORK, default hugegraph-net) with no depends_on, so attaching, upgrading, or removing Hubble never recreates PD, Store, or Server containers. Attach flow uses an explicit project (-p hugegraph-hubble); the fresh flow brings up cluster plus Hubble in one command with both -f flags. Give the 3-node cluster the Server settings Hubble's PD mode requires: PD registration (HG_SERVER_CLUSTER/USE_PD/REST_URL per replica via a shared env anchor), a required shared auth token secret so tokens validate on every replica, and a required admin password. The Server healthcheck now probes the bound REST URL. Hubble reads the 3x3 topology from hugegraph-hubble-3x3.properties. Document the attach, fresh, and dev-override flows plus migration notes in docker/README.md, update the cluster call sites across the docs, and extend the CI compose contract checks to the cluster file and add-on. Image tags stay on latest until the 1.8.0 release publishes; pin via HUGEGRAPH_VERSION in docker/.env.
…in compose checks Render the combined cluster+Hubble topology with non-default network and version values and assert the overridden network name and all four image tags, so CI fails if any Compose file stops honoring either override. The standalone add-on render keeps asserting the defaults and now strips any runner-level overrides for hermeticity.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3149 +/- ##
============================================
- Coverage 39.30% 37.74% -1.57%
- Complexity 264 6528 +6264
============================================
Files 771 800 +29
Lines 65915 68821 +2906
Branches 8754 9127 +373
============================================
+ Hits 25910 25977 +67
- Misses 37241 39804 +2563
- Partials 2764 3040 +276 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The Compose add-on has a false-green authentication check and operational failover and attachment gaps that can make a healthy-looking deployment unusable. Evidence: exact-head static review across six independent lanes; docker-entrypoint.sh:93-98 requires a 32-byte token, Hubble LiveOperationsCollector uses one pd.server, and the add-on has no data volume.
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The cluster quickstart currently fails after generating its own credentials, and the Hubble add-on mounts the H2 database outside the path used by Hubble; additional deployment and CI gaps can leave a false-green or state-inconsistent setup. Evidence: exact-head static review across six independent lanes plus Docker Compose render and guard checks; live container startup was unavailable.
| hostname: server0 | ||
| environment: | ||
| <<: *server-env | ||
| HG_SERVER_REST_URL: http://server0:8080 |
There was a problem hiding this comment.
http://serverN:8080 with PD while the host publishes ports 8080/8081/8082. Evidence: Server registration uses restserver.url, so a PD-aware client outside the Docker network receives server0/server1/server2, names that only resolve inside hugegraph-net. Provide a separate configurable advertised address or make the external client path use addresses it can resolve.
There was a problem hiding this comment.
Agree for PD-aware clients outside Docker DNS. Hubble on hugegraph-net can use serverN names as-is. Configurable advertised addresses feel like a follow-up to external-client addressing, not required for the add-on path. What do you think?
There was a problem hiding this comment.
Fixed in c311557 rather than deferred.
Each replica's registered URL is now ${HUGEGRAPH_SERVER0_REST_URL:-http://server0:8080} and siblings, so a PD-aware client outside Docker can be given addresses it can resolve. The defaults are unchanged, so Hubble and anything else on hugegraph-net behave exactly as before.
The README explains when the container-name defaults are wrong and that widening the Server ports is part of that setup.
There was a problem hiding this comment.
Correction to my earlier reply on this thread: the fix I described does not work and has been removed in 9616037.
HG_SERVER_REST_URL becomes restserver.url, which is the address the REST server binds, not only the one registered with PD. Running the cluster with the documented override crash-looped that replica on java.net.BindException: Cannot assign requested address while the two default replicas stayed healthy.
Your original point stands: this needs a separate advertised-address setting in the Server. The README now documents the limitation and points external clients at the published ports instead of PD discovery.
There was a problem hiding this comment.
Leaving this one open deliberately, since it is the only finding I did not actually satisfy.
You asked for a configurable advertised address, or for the external client path to use addresses it can resolve. Neither is possible from the Compose files: HG_SERVER_REST_URL becomes restserver.url, which is also the address the REST server binds, so an externally resolvable value crash-loops the replica. Splitting bind from advertise needs a Server-side setting.
What shipped is the limitation documented, with external clients pointed at the published ports instead of PD discovery. Your call whether that is an acceptable close or whether it should become a Server issue.
Read docker/.env as data instead of sourcing it as shell, and take the quoted value rather than the optional export capture when validating generated credentials. Point Hubble H2 at a file inside the /hubble/db volume, give attach and combined flows the same explicit volume names, and assert PD/auth env on every Server replica plus pd.enabled=true.
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: Four previously reported deployment blockers remain on this head: incompatible cached images can leave authentication false-green, PD/Store control-plane ports remain exposed without real authentication, PD-registered Server URLs are not resolvable outside Docker DNS, and CI remains render-only. Evidence: exact head 950f645; 21 GitHub checks passed; fresh Docker Compose render, credential guard, topology, and dotenv-parser checks passed locally; six independent review lanes were unavailable because the local review gateway repeatedly failed.
Hubble supports auth.enabled=false, so the cluster file no longer requires PASSWORD. Match Hubble to the anonymous Servers and keep PD registration for discovery.
The 3-node cluster is anonymous. Remove agent-memory edits that still required docker/.env, and describe Hubble auth.enabled=false without citing toolchain fork PRs.
…ach" The 3-node cluster is the quickstart most people copy, so it keeps authentication on by default. Running it anonymously is still supported, but as an explicit opt-in documented alongside the default flow rather than as the default itself. This restores the required admin password and JWT token secret, the credential setup and validation steps in the docker README, and the matching CI assertions. The agent-memory files removed by the follow-up commit stay out of the diff.
The 3-node cluster could report healthy while running unauthenticated. The images default to a floating tag with pull_policy: missing, so an older cached image was never refreshed, and that image ignores PASSWORD and the token secret while still answering /versions. Readiness now proves the image enforces authentication: an unauthenticated graph request must return 401 and an authenticated one 200, and all four images pull by default. An incompatible image no longer becomes healthy, so up -d --wait fails instead of handing back a false green. PD and Store publish REST, gRPC and Raft ports, and neither has real authentication. Together with the well-known external network that exposed an unauthenticated control plane on every host interface. Those ports and the Server REST ports now bind to 127.0.0.1, with HUGEGRAPH_CONTROL_PLANE_HOST and HUGEGRAPH_SERVER_PUBLISH_HOST to widen them deliberately. Each Server registers its own REST URL with PD, so a PD-aware client outside Docker received container names it cannot resolve. The registered URLs are configurable through HUGEGRAPH_SERVER0_REST_URL and its siblings, and the README explains when the defaults are wrong. Compose rendering cannot catch any of this, so CI now starts the cluster, attaches Hubble, and asserts the 401/200 pair on all three replicas, PD registration of all three Servers, that attaching Hubble recreates no cluster container, and that the H2 database survives recreation through the shared volumes. Running without authentication stays possible through an explicit opt-in, docker-compose-3x3.non-auth.yml with a matching Hubble properties file, documented next to the default flow along with a prompt for assistants that keeps the authenticated path as the default.
…t proves Running the smoke test against a real cluster showed two of its assertions were wrong. PD's /v1/cluster returns the PD peers, not the graph servers, so grepping it for registered Server addresses always found nothing; /v1/registry answers 405 to GET and 500 to POST, so there is no simple REST proof of Server registration. Assert what PD does report and what actually matters for a distributed deployment: three PD peers and three Stores in state Up. The persistence check only listed the H2 database file, which passes even if the attach and combined flows use different volumes. Write a marker through the attach flow and read it back through the combined flow after Hubble has been recreated, which is the property the explicit volume names exist for.
…eholders The Port Reference section still said cluster ports bind every host interface, which contradicted both the quickstart section and the compose file after the loopback change. It now describes the loopback default and what widening it actually exposes. The non-auth override documented a placeholder token secret that was long enough to be accepted as a real one. Anyone who copied the two placeholder lines into docker/.env and then started the default stack would have got a cluster that looked authenticated while signing tokens with a key published in this repository. Both placeholders are now obvious non-values and the token is deliberately shorter than the 32 bytes the Server requires, so it aborts startup instead. The README and the override header both say to pass them inline and never store them. Also lists the two new files in the file table and documents HUBBLE_DB_VOLUME and HUBBLE_UPLOAD_VOLUME, which were usable but undocumented.
…irst The override shared one YAML anchor across the three Server services. The plain healthcheck override propagates through the alias, but the !reset tags do not on every Compose version: on 5.1.2 only the anchored service loses PASSWORD and HG_SERVER_AUTH_TOKEN_SECRET, so server1 and server2 keep them and come up authenticated while server0 does not. The result is a cluster that is half authenticated, which is worse than either mode on its own, and readiness still passes on all three because the healthcheck override does propagate. The same file renders correctly on 5.1.4, so this depends on the Compose version rather than failing everywhere, which is how it survived a live run. Spell the three services out instead of aliasing them; the CI render check already asserts all three, so a regression fails there.
|
Pushed
The cluster also goes back to being authenticated by default, per your point that this is the file people copy. Running without auth is now an explicit opt-in, Tested on a clean Docker host, from zero images: 9/9 healthy, 401/200 on all three replicas, ports on loopback, three PD peers and three Stores Up, Hubble attached without recreating a single cluster container, H2 state surviving recreation across both flows, and the non-auth path returning 200 with no credentials anywhere in the container environment. Testing caught three bugs that rendering never would have, one of them a half-authenticated cluster where only |
bitflicker64
left a comment
There was a problem hiding this comment.
Reviewed the Compose files, the CI additions, and the docs end to end, and ran the render
checks and several of the README shell blocks locally.
The shape of this is good: an add-on file rather than a forked topology, an external
network so attach never recreates the cluster, and readiness that proves authentication
instead of trusting a tag. The CI render checks are unusually thorough.
I think it needs another pass before merge. Most of what I found is in the seams around
that core: teardown and the non-auth flow leave the operator without a working path, two
documented overrides cannot work as described, and one paragraph promises a safety net
across three files that only one of them has. Details inline.
One thing that has no inline anchor: the Healthcheck Endpoints table further down (Server
row) still says GET /versions -> 200 OK. The cluster Server healthcheck is now the
401/200 pair, so that row needs a separate touch; it sits outside the diff.
Minor, take or leave: the combined render's jq is a 21-clause conjunction that fails with
a bare exit 1 naming no clause, while the same function already has named-error helpers
(assert_guard, assert_props); and the token_fixture length check guards a literal
two lines above it, so it can only fire if someone edits that literal.
Self-review found the advertised-address option cannot work. HG_SERVER_REST_URL becomes restserver.url, which is the address the REST server binds, not only the one registered with PD. A Server given an externally reachable address exits with java.net.BindException and restarts forever, confirmed by running the cluster with the documented override: that replica crash-looped while the two defaults stayed healthy. Separating bind from advertise needs a Server-side setting, so the three overrides are removed and the limitation is documented instead. The Server healthcheck now asserts only that an unauthenticated graph request returns 401. That is what proves the image enforces authentication, and unlike the authenticated half it stays correct after an operator rotates the admin password through the API, which would otherwise leave every replica permanently unhealthy on a working cluster. The shared Hubble volumes are external now. A fixed name is not enough: Compose removes a fixed-name non-external volume on `down -v` from any project that declares the name, so leaving the attach flow destroyed the combined flow's H2 database. The setup block creates them alongside the network, and the H2 path is absolute so it cannot drift outside the mount. Also: the image pull policy is overridable, since `always` with no escape made the cluster unusable offline even with every image cached; the non-auth flow documents the network, volumes and its own teardown, which the guards demand on `down` too; the dotenv reader rejects unquoted values instead of returning an empty password, and its block runs in a subshell so a failure cannot close an interactive shell; CI unsets the operator-facing variables it asserts defaults for, covers the non-auth properties with the same topology contract, and runs the live smoke test only when this contract changes, since it boots published images and should not redden unrelated pull requests.
|
Fixed in 9616037, verified against a real cluster. The advertised-address point is the important one and you were right: Also fixed: healthcheck no longer pins to the seeded password (401-only, so password rotation cannot brick readiness), Hubble volumes are external so |
The version-pinning paragraph opened by naming the cluster, the add-on and the single-node quickstart, then concluded that an image which ignores PASSWORD never reports healthy. Only the 3-node cluster has that check. The single-node files probe /versions, which stays open whether authentication works or not, so a reader who pinned a version exactly as instructed got no protection there and no warning about it. The same paragraph also still described the old healthcheck, which required an authenticated 200 alongside the 401. It asserts only the 401 now.
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The documented no-credentials attach flow still requires external Hubble volumes to exist in advance. Evidence: exact-head review of the Compose file and README; the attach path is actionable but approval is withheld because the required independent lanes were unavailable and a latest-head CI job is failing.
Making the volumes external fixed a data-loss bug but broke the flow that needs them most. Compose refuses to start a service whose external volume is absent, and the attach flow exists for the case where someone else owns the cluster, so the reader never ran the setup block that creates them: external volume "hugegraph-hubble-upload-files" not found The section now creates both volumes before attaching, honouring HUBBLE_DB_VOLUME and HUBBLE_UPLOAD_VOLUME when the cluster owner renamed them. The block is inspect-then-create, so it is safe to repeat and never touches an existing volume. Verified against a real cluster: the attach fails without it, and Hubble reaches healthy with it.
|
Pushed
Everything above was verified by running it, not by reading. On the current head: ten containers healthy, 401 unauthenticated and 200 authenticated on all three replicas, wrong password 401, three PD peers and three Stores One note on CI: |
|
@imbajin one open question before this is mergeable, on the PD-registered Server addresses. The inline thread is marked outdated (the lines it pointed at are gone), so raising it here where it is visible: #3149 (comment) Your ask was a configurable advertised address, or an external client path that resolves. I could not do either from Compose: So, which way do you want it closed? A. Accept it as a documented limitation. External clients use the published Server ports; PD discovery stays correct for anything on B. I open a Server issue for a real advertised-address option and link it here. I lean A for this PR, since B is a Server change that would hold up the add-on. Happy to file B regardless if you want it tracked. |
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The documented Hubble no-auth flow does not disable Hubble's own API authentication, and the pull-policy and volume-management instructions need correction. Evidence: exact-head static review of the Hubble configuration and README; Compose configuration rendering passed; latest ordinary CI completed with only the non-blocking codecov/project failure; required independent review lane evidence is incomplete.
| pd.enabled=true | ||
| # Hubble's code default is auth.enabled=true. Without this line the UI would | ||
| # demand a login against Servers that have no users configured. | ||
| auth.enabled=false |
There was a problem hiding this comment.
| | `HUBBLE_DB_VOLUME` | add-on | `hugegraph-hubble-db` | Volume holding Hubble's H2 database; the explicit name is what lets the attach and combined flows share state, so change it only to run a second independent Hubble | | ||
| | `HUBBLE_UPLOAD_VOLUME` | add-on | `hugegraph-hubble-upload-files` | Volume holding Hubble's uploaded files; same naming caveat as `HUBBLE_DB_VOLUME` | | ||
| | `HUGEGRAPH_NETWORK` | cluster, add-on | `hugegraph-net` | Pre-created external Docker network shared by the 3-node cluster and the Hubble add-on; the single-node files use their own project bridge instead | | ||
| | `HUGEGRAPH_PULL_POLICY` | cluster | `always` | Pull policy for the PD, Store, and Server images. The default refreshes a stale cached `latest`; set it to `missing` to run offline or against locally built tags | |
There was a problem hiding this comment.
🧹 missing does not guarantee offline use. Evidence: Docker Compose pulls an image when it is absent under pull_policy: missing, while this table says HUGEGRAPH_PULL_POLICY=missing runs offline; the same guide uses never for local images. Document never for strict offline or local use and reserve missing for pull-if-absent.
| # Hubble's volumes are external for the same reason the network is: both | ||
| # documented flows share them, and an external volume is not destroyed by a | ||
| # `down -v` from either one. | ||
| for vol in "${HUBBLE_DB_VOLUME:-hugegraph-hubble-db}" \ |
There was a problem hiding this comment.
| # Hubble's volumes are external, so no `down` removes them. Drop them | ||
| # explicitly when you want the H2 database and uploaded files gone, or a | ||
| # later deployment reattaches the old state. | ||
| docker volume rm hugegraph-hubble-db hugegraph-hubble-upload-files |
There was a problem hiding this comment.
🧹 This teardown only removes the default volume names. Evidence: the add-on supports HUBBLE_DB_VOLUME and HUBBLE_UPLOAD_VOLUME, but this command hard-codes the defaults, so a customized deployment retains its H2 and upload data despite the instruction to drop them. Use the same variables with defaults or say custom volumes must be removed separately.
What this PR does
Adds an attachable Hubble deployment for the 3-node Compose cluster, and gives that cluster the authenticated defaults Hubble needs.
docker/docker-compose-hubble.yml(new): Hubble only. Joins the pre-created cluster network and has nodepends_onon cluster services, so attaching, upgrading, or removing Hubble never recreates PD, Store, or Server containers. Its H2 database and uploaded files live in explicitly named volumes, so the attach and combined flows share the same state.docker/docker-compose-3pd-3store-3server.yml: the Server PD-registration settings Hubble needs (cluster name, PD mode, per-replica REST URL), an admin password and one shared JWT secret across all three replicas, readiness that proves authentication is enforced, and host ports bound to loopback.docker/hugegraph-hubble-3x3.properties(new): Hubble topology for the 3-node cluster withpd.enabled=true.docker/docker-compose-3x3.non-auth.ymlanddocker/hugegraph-hubble-3x3.non-auth.properties(new): the opt-in way to run the same cluster without authentication.docker/README.md: credential setup, cluster and Hubble quickstarts, attach and combined flows, the non-auth opt-out, and a copy-paste prompt for people who hand this to an assistant..github/workflows/server-ci.yml: compose contract checks plus a live smoke job that starts the cluster, attaches Hubble, and asserts the behaviour the render checks cannot see.Design
-ffile, not a copy of the nine-node topology.hugegraph-net, override viaHUGEGRAPH_NETWORK) so attach and combined flows share one network. Compose will not adopt an unlabeled project network asexternal.-p hugegraph-hubblefor attach so the add-on does not collide with the cluster project.PASSWORDnever becomes healthy andup -d --waitfails, instead of handing back a cluster that looks fine and is wide open.Review findings addressed
All four items previously listed here as follow-ups are now in this PR:
127.0.0.1.HUGEGRAPH_CONTROL_PLANE_HOSTandHUGEGRAPH_SERVER_PUBLISH_HOSTwiden them deliberately.HUGEGRAPH_SERVER0_REST_URLand its siblings, and the README explains when the container-name defaults are wrong.Breaking change
Existing 3-node deployments: create
docker/.envwith an admin password and a token secret before running any Compose command against an older stack,downincluded, because the:?guards fire on every subcommand. The firstup -dafter this change recreates all nine containers (network move; named volumes survive). Graph APIs now require credentials. To keep an unauthenticated cluster, use the non-auth override described in the README.Validation
Run on a 16 GB Linux host from a clean Docker state (no images, no containers), against
latest, which currently resolves to 1.7.0.docker/.envat mode 600, validates both values, creates the network, and renders the cluster.up -d --wait: 9/9 healthy, which by itself proves the auth-enforcing readiness check passes on a real image.127.0.0.1:8080,127.0.0.1:8500, and127.0.0.1:8620, not0.0.0.0.Up./aboutreportshugegraph-hubble, and the H2 database is inside the mounted volume at/hubble/db/hubble.mv.db.PASSWORDor token secret in any of the three container environments, and Hubble attaches withauth.enabled=false.One bug found this way was mine. The non-auth override first shared a YAML anchor across the three Server services, and
!resetdoes not survive an anchor reference on every Compose version: on 5.1.2 onlyserver0lost its credentials, so the cluster came up half authenticated, while 5.1.4 reset all three. The live run passed because it happened to use 5.1.4. The services are now spelled out separately and the render check asserts all three.