hubcio commented on code in PR #68:
URL: https://github.com/apache/iggy-website/pull/68#discussion_r3893008068


##########
content/docs/server/docker.mdx:
##########
@@ -10,6 +10,7 @@ Two properties of the published image matter for any 
deployment:
 
 - The working directory is `/app` and the `iggy-server` and `iggy` binaries 
are on `PATH` (`/usr/local/bin`). The default data directory `local_data` 
therefore resolves to `/app/local_data` - **mount your volume there**, or set 
`IGGY_SYSTEM_PATH` and mount that path instead.
 - The image bakes in no address overrides, so the server binds the loopback 
defaults (`127.0.0.1`) and is **unreachable from outside the container** even 
with published ports. Set `IGGY_TCP_ADDRESS=0.0.0.0:8090` (and the equivalent 
for every other transport you expose) alongside the `-p` flags.
+- The wildcard says nothing about where clients reach the container, so the 
server refuses to start until `IGGY_NODE_ADVERTISED_ADDRESS` supplies that 
address. Use `localhost` when the ports are published to the host, the compose 
service name when the clients are containers on the same network, and the 
external hostname or load balancer name when they are further away. The value 
reaches clients through cluster metadata, which they read to follow redirects 
and to learn where to reconnect.

Review Comment:
   `node.advertised_address` is only read when cluster.enabled is false, and a 
single-node roster never redirects. drop "to follow redirects" - the value only 
feeds the endpoint list clients reconnect through.



##########
content/docs/server/docker.mdx:
##########
@@ -84,7 +87,7 @@ The repository root carries its own `Dockerfile` and 
`docker-compose.yml` for bu
 docker build -t iggy .
 ```
 
-Or use `docker compose up` directly from the repository root. That image 
**differs from the published one**: it sets the `0.0.0.0` listener addresses 
via `ENV` and keeps its data at `/local_data`.
+Or use `docker compose up` directly from the repository root. That image 
**differs from the published one**: it sets the `0.0.0.0` listener addresses 
via `ENV` and keeps its data at `/local_data`. Its compose file publishes the 
ports to the host, so it declares `IGGY_NODE_ADVERTISED_ADDRESS=localhost`.

Review Comment:
   the repo dockerfile ENVs all four listeners to `0.0.0.0`, so `docker build 
-t iggy .` plus a plain `docker run` also refuses to boot, not just compose. 
say so.



##########
content/docs/server/configuration.mdx:
##########
@@ -35,6 +35,7 @@ Every configuration key can be overridden with an `IGGY_` 
variable. The name is
 
 ```bash
 IGGY_TCP_ADDRESS=0.0.0.0:8090          # [tcp] address
+IGGY_NODE_ADVERTISED_ADDRESS=iggy-1    # [node] advertised_address

Review Comment:
   the minimal config example at lines 9-18 binds tcp and http to `0.0.0.0` 
with no `[node]` advertised_address, so that exact file now refuses to boot. 
add `advertised_address` to it.



##########
content/docs/server/configuration.mdx:
##########
@@ -410,6 +411,14 @@ Tunables for the internal bus that ships consensus traffic 
between replicas and
 |-----|---------|-------------|
 | `rebalancing_timeout` | `"30s"` | Maximum time a partition can remain in 
pending revocation before being force-transferred to the target member. |
 
+### `[node]`
+
+This node's own client-facing identity, read only while `cluster.enabled = 
false`. In cluster mode the roster answers the same question per node, so a 
value here takes no effect and the server says so at startup.

Review Comment:
   `node.advertised_address` is validated in every mode, so a malformed value 
refuses boot even with `cluster.enabled = true`. "takes no effect" should say 
the address is ignored, not the whole key.



##########
content/docs/server/docker.mdx:
##########
@@ -120,6 +123,9 @@ helm install iggy ./helm/charts/iggy
 # values.yaml (excerpt, chart defaults)
 server:
   replicaCount: 1
+  # The address clients dial, published in cluster metadata. Empty falls

Review Comment:
   the upstream values.yaml comment also warns that setting 
`IGGY_NODE_ADVERTISED_ADDRESS` in server.env and `server.advertisedAddress` 
together fails at helm render. restore that sentence, since the bullet above 
points operators at server.env.



-- 
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]

Reply via email to