bitflicker64 commented on code in PR #3149:
URL: https://github.com/apache/hugegraph/pull/3149#discussion_r3839620289


##########
docker/docker-compose-3pd-3store-3server.yml:
##########
@@ -187,16 +207,25 @@ services:
     <<: *server-common
     container_name: hg-server0
     hostname: server0
+    environment:
+      <<: *server-env
+      HG_SERVER_REST_URL: http://server0:8080

Review Comment:
   Fixed in c3115579 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.



##########
.github/workflows/server-ci.yml:
##########
@@ -138,6 +140,189 @@ jobs:
           check_compose docker/docker-compose.yml always always
           check_compose docker/docker-compose.dev.yml build missing
 
+          check_cluster_compose() {
+            local cluster="docker/docker-compose-3pd-3store-3server.yml"
+            local addon="docker/docker-compose-hubble.yml"
+            local rendered
+            local token_fixture=ci-test-token-secret-32-bytes-long
+            rendered="$(mktemp)"
+            if [ "${#token_fixture}" -lt 32 ]; then
+              echo "CI token fixture must be at least 32 bytes" >&2
+              return 1
+            fi
+
+            # RETURN only: an EXIT trap would fire after this function's
+            # `local rendered` has gone out of scope, which `set -u` turns
+            # into an "unbound variable" error. A hard errexit abort can
+            # therefore still leak one temp file, which is acceptable on an
+            # ephemeral runner.
+            trap 'rm -f "$rendered"' RETURN
+
+            # --env-file /dev/null on every invocation: Compose otherwise reads
+            # docker/.env automatically, and the quickstart tells operators to
+            # create one holding exactly the credentials these assertions
+            # control. Without it the checks pass in CI (which has no .env) but
+            # report false failures for anyone running them locally after
+            # following the quickstart, and the add-on render below — which
+            # deliberately unsets the variables to assert their defaults — 
would
+            # read whatever HUGEGRAPH_NETWORK/HUGEGRAPH_VERSION that .env 
holds.
+            # Pinning an empty env file makes both renders depend only on what
+            # each invocation sets explicitly.
+
+            # Both cluster credentials are required and may not be empty.
+            # Each case asserts the guard fired for the *intended* variable:
+            # a bare non-zero exit would also be produced by a YAML error, a
+            # renamed file, or a missing docker binary.
+            assert_guard() { # assert_guard <blamed-var> <description>
+              local var="$1" desc="$2" err
+              if err="$(docker compose --env-file /dev/null -f "$cluster" \
+                          config -q 2>&1)"; then
+                echo "$cluster accepted $desc" >&2
+                return 1
+              fi
+              case "$err" in
+                *"$var"*) : ;;
+                *) echo "$cluster rejected $desc, but not because of $var: 
$err" >&2
+                   return 1 ;;
+              esac
+            }
+
+            ( unset HUGEGRAPH_ADMIN_PASSWORD
+              export HUGEGRAPH_AUTH_TOKEN_SECRET="${token_fixture}"
+              assert_guard HUGEGRAPH_ADMIN_PASSWORD "an unset admin password" )
+            ( export HUGEGRAPH_ADMIN_PASSWORD=
+              export HUGEGRAPH_AUTH_TOKEN_SECRET="${token_fixture}"
+              assert_guard HUGEGRAPH_ADMIN_PASSWORD "an empty admin password" )
+            ( unset HUGEGRAPH_AUTH_TOKEN_SECRET
+              export HUGEGRAPH_ADMIN_PASSWORD=ci-test-password
+              assert_guard HUGEGRAPH_AUTH_TOKEN_SECRET "an unset token secret" 
)
+            ( export HUGEGRAPH_ADMIN_PASSWORD=ci-test-password
+              export HUGEGRAPH_AUTH_TOKEN_SECRET=
+              assert_guard HUGEGRAPH_AUTH_TOKEN_SECRET "an empty token secret" 
)
+            # The add-on alone must define Hubble and nothing else, join the
+            # shared external network with its default name, and need no
+            # credentials or overrides.
+            env -u HUGEGRAPH_ADMIN_PASSWORD -u HUGEGRAPH_AUTH_TOKEN_SECRET \
+                -u HUGEGRAPH_NETWORK -u HUGEGRAPH_VERSION \
+                -u HUBBLE_IMAGE -u HUBBLE_PULL_POLICY -u HUBBLE_PUBLISH_HOST \
+              docker compose --env-file /dev/null -f "$addon" config --format 
json > "$rendered"
+            jq -e '
+                (.services | keys) == ["hubble"] and
+                .networks."hg-net".external == true and
+                .networks."hg-net".name == "hugegraph-net" and
+                (.services.hubble.networks | has("hg-net")) and
+                (.services.hubble | has("depends_on") | not) and
+                any(.services.hubble.volumes[];
+                    .target == "/hubble/conf/hugegraph-hubble.properties" and
+                    (.source | endswith("hugegraph-hubble-3x3.properties")))
+                and any(.services.hubble.volumes[];
+                    .source == "hg-hubble-db" and
+                    .target == "/hubble/db")
+                and any(.services.hubble.volumes[];
+                    .source == "hg-hubble-upload-files" and
+                    .target == "/hubble/upload-files")
+              ' "$rendered" >/dev/null
+
+            # The cluster's own default network name must match the add-on's,
+            # or the attach flow and the cluster land on different networks.
+            # The combined render below pins an override, so it cannot catch a
+            # drifting default.
+            HUGEGRAPH_ADMIN_PASSWORD=ci-test-password \
+            HUGEGRAPH_AUTH_TOKEN_SECRET="${token_fixture}" \
+              env -u HUGEGRAPH_NETWORK -u HUBBLE_IMAGE -u HUBBLE_PULL_POLICY \
+                  -u HUBBLE_PUBLISH_HOST \
+              docker compose --env-file /dev/null -f "$cluster" \
+                config --format json > "$rendered"
+            jq -e '.networks."hg-net".name == "hugegraph-net"' \
+              "$rendered" >/dev/null
+
+            # The combined render carries the PD-registration and auth
+            # settings on every server replica and keeps Hubble on loopback.
+            # Rendered with non-default HUGEGRAPH_NETWORK/HUGEGRAPH_VERSION so
+            # CI fails if any file stops honoring the overrides (the add-on
+            # render above covers the defaults).
+            HUGEGRAPH_ADMIN_PASSWORD=ci-test-password \
+            HUGEGRAPH_AUTH_TOKEN_SECRET="${token_fixture}" \
+            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" \

Review Comment:
   Fixed in c3115579 and corrected in 10c6c6b5.
   
   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 10c6c6b5, which is the 
argument for the job existing.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to