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 12 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.
dosubot
Bot
added
size:L
…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% 32.53% -6.77%
Complexity 264 264
============================================
Files 771 771
Lines 65915 65962 +47
Branches 8754 8766 +12
============================================
- Hits 25910 21463 -4447
- Misses 37241 42023 +4782
+ Partials 2764 2476 -288 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
imbajin
left a comment
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
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.
| printf "HUGEGRAPH_AUTH_TOKEN_SECRET='%s'\n" "${token_secret}" >> .env | ||
| unset token_secret | ||
| fi | ||
| admin_value="$(sed -nE "s/${pat}HUGEGRAPH_ADMIN_PASSWORD='([^']*)'[[:space:]]*$/\\1/p" .env | tail -n1)" |
There was a problem hiding this comment.
export capture before the value, but both sed commands substitute capture group 1, so HUGEGRAPH_ADMIN_PASSWORD='...' yields an empty value (and an export form yields the word export); lines 216/220 then fail. Use the value capture group and parse or explicitly reject all supported Compose dotenv formats consistently.
There was a problem hiding this comment.
Agreed and also in scope. The optional export group made \1 capture the wrong text. I’ll fix the setup parser to read the value group (and keep parsing as data, not by sourcing .env).
| - "${HUBBLE_PUBLISH_HOST:-127.0.0.1}:8088:8088" | ||
| volumes: | ||
| - ./hugegraph-hubble-3x3.properties:/hubble/conf/hugegraph-hubble.properties:ro | ||
| - hg-hubble-db:/hubble/db |
There was a problem hiding this comment.
/hubble and its datasource is jdbc:h2:file:./db; mounting a volume at /hubble/db leaves the actual /hubble/db.mv.db outside that volume (or makes startup fail because /hubble/db is a directory). Point H2 at a file inside the mounted directory or mount the parent directory, then verify data survives container recreation.
There was a problem hiding this comment.
I’ll fix the persistence boundary so Hubble state survives recreate.
|
|
||
| # ── Shared service defaults ────────────────────────────────────────── | ||
| x-pd-common: &pd-common | ||
| # Pin a release via HUGEGRAPH_VERSION in docker/.env; unset, the image |
There was a problem hiding this comment.
latest with pull_policy: missing; an already-cached older image can ignore PASSWORD and the token secret, while the healthcheck probes public /versions. Pin or fail closed on incompatible images, or add an authenticated readiness smoke check that proves graph requests are rejected without credentials and succeed with them.
There was a problem hiding this comment.
Agree this is a real gap for the auth-required 3-node quickstart. I’d like to keep #3149 scoped to the Hubble add-on and the minimum cluster contract Hubble needs. Changing Server readiness to prove 401/200 and changing default pull policy feels like follow-up cluster hardening rather than the add-on itself. README already documents the manual 401/200 check for incompatible images.
There was a problem hiding this comment.
Fixed in c311557, and you were right that this belonged here rather than in a follow-up.
Both halves of your suggestion are in: all four images now use pull_policy: always, so a stale cached latest is refreshed, and Server readiness no longer probes public /versions. It requires an unauthenticated graph request to return 401 and an authenticated one to return 200. An image that ignores PASSWORD never reports healthy, so up -d --wait fails instead of handing back an open cluster.
Verified on a clean Docker host: 9/9 healthy against latest (1.7.0), 401/200 on all three replicas, wrong password 401.
| networks: | ||
| hg-net: | ||
| driver: bridge | ||
| external: true |
There was a problem hiding this comment.
hugegraph-net is external and the file publishes PD/Store REST, gRPC, and Raft ports on all host interfaces; Store has no authentication and PD only checks the internal service username, not the password. Bind these ports to loopback/remove them by default, or require real authentication/TLS and explicit network ACLs before shipping this as the default quickstart.
There was a problem hiding this comment.
Agree on the exposure concern. Default loopback binds / ACL+TLS requirements are a cluster-quickstart security redesign; I’d track that as a follow-up so this PR stays reviewable as a Hubble add-on.
There was a problem hiding this comment.
Fixed in c311557.
PD, Store, and Server host ports now bind to 127.0.0.1 by default, so the unauthenticated control plane is not reachable from outside the host. HUGEGRAPH_CONTROL_PLANE_HOST and HUGEGRAPH_SERVER_PUBLISH_HOST widen them, and the README says to do that only behind a network ACL or TLS.
I had asked to defer this as a quickstart redesign. It was not one: the services already talk over hugegraph-net, so nothing needed those host bindings.
Verified live: ss -ltn shows 127.0.0.1:8080, 127.0.0.1:8500, 127.0.0.1:8620.
| name: ${HUGEGRAPH_NETWORK:-hugegraph-net} | ||
|
|
||
| volumes: | ||
| hg-hubble-db: |
There was a problem hiding this comment.
hugegraph-hubble, while the combined flow runs under hugegraph-3x3; Compose therefore creates different project-prefixed H2 and upload volumes. Switching flows makes existing state and uploads appear to disappear. Give both volumes stable explicit names (with a multi-cluster override if needed) or document a supported volume migration.
There was a problem hiding this comment.
I’ll give the Hubble DB/upload volumes explicit stable names so attach and combined share the same physical volumes.
| # And a signed-in read must succeed. Compose reads docker/.env by itself, but | ||
| # your shell does not — load it first. Passing the credential through | ||
| # --config keeps it out of argv, where `ps` would expose it to other users. | ||
| set -a; . ./.env; set +a |
There was a problem hiding this comment.
set -a; . ./.env; set +a evaluates command substitutions and other shell syntax before the curl; a value such as HUGEGRAPH_ADMIN_PASSWORD=$(...) runs on the operator host. Parse only the required key with a dotenv-aware parser or avoid sourcing .env directly.
There was a problem hiding this comment.
I’ll replace set -a; . ./.env; set +a with a parse‑only read of the required key, so the verify block does not execute dotenv as a shell.
| 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.
| .services.store0.image == "hugegraph/store:ci-test-tag" and | ||
| .services.server0.image == "hugegraph/server:ci-test-tag" and | ||
| .services.hubble.image == "hugegraph/hubble:ci-test-tag" and | ||
| .services.server0.environment.HG_SERVER_USE_PD == "true" and |
There was a problem hiding this comment.
server0 for HG_SERVER_USE_PD, HG_SERVER_CLUSTER, HG_SERVER_INIT_STORE_ENABLED, and PASSWORD; server1/server2 only check REST URL and token. A per-replica override can therefore pass CI while breaking discovery or initialization. Apply the required-value assertions to all three Server services.
There was a problem hiding this comment.
Agreed — in scope. This PR introduced the PD/auth Server env for Hubble discovery, so CI should assert the required values on server0/server1/server2, not only server0.
| | select(.value.hostname == $h) | ||
| | .value.hostname + ":" + .value.environment.HG_PD_REST_PORT' \ | ||
| "$rendered")" | ||
| assert_props "cluster=${cluster_name}" "matches HG_SERVER_CLUSTER" |
There was a problem hiding this comment.
assert_props validates cluster, pd.peers, pd.server, and Store targets, but never asserts pd.enabled=true; the current server.direct_url is only meaningful when PD mode is false. Add the explicit mode assertion and a runtime smoke check so Hubble cannot silently fall back to one direct Server.
There was a problem hiding this comment.
I’ll add an explicit assert_props "pd.enabled=true" so the Hubble properties check cannot pass with PD mode disabled.
| HUGEGRAPH_NETWORK=ci-test-net \ | ||
| HUGEGRAPH_VERSION=ci-test-tag \ | ||
| env -u HUBBLE_IMAGE -u HUBBLE_PULL_POLICY -u HUBBLE_PUBLISH_HOST \ | ||
| docker compose --env-file /dev/null -f "$cluster" -f "$addon" \ |
There was a problem hiding this comment.
docker compose config plus jq/grep and never creates the external network, starts the services, probes Hubble /about, verifies PD registration, or checks authenticated graph access. Add a Docker-capable smoke job for the combined and attach flows so entrypoint, DNS, registration, and auth failures cannot remain green.
There was a problem hiding this comment.
Render‑only cannot catch runtime auth/DNS failures. For this PR I’ll keep and strengthen the compose contract assertions (including the in‑scope ones above). A full combined or attached Docker smoke job is useful but adds heavier CI overhead; I’d prefer a follow‑up rather than blocking the add‑on here.
There was a problem hiding this comment.
Fixed in c311557 and corrected in 10c6c6b.
CI now starts the cluster, checks 401 unauthenticated and 200 authenticated on all three replicas, checks three PD peers and three Stores Up, attaches Hubble, confirms no cluster container is recreated, and proves Hubble state survives recreation by writing through the attach flow and reading back through the combined flow.
Writing it against a real cluster caught two bad assertions of my own: /v1/cluster returns PD peers, not graph servers, and the H2 check passed even when the two flows used different volumes. Both fixed in 10c6c6b, which is the argument for the job existing.
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
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.
dosubot
Bot
added
size:XL
|
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 |
2 participants
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.