hubcio commented on code in PR #4035:
URL: https://github.com/apache/iggy/pull/4035#discussion_r3914475615
##########
helm/charts/iggy/README.md.gotmpl:
##########
@@ -101,6 +101,153 @@ If Prometheus Operator is installed and you want
monitoring, set
`server.serviceMonitor.enabled=true` in `custom-values.yaml` or pass it on the
command line with `--set server.serviceMonitor.enabled=true`.
+## Cluster Mode
+
+The server runs a Viewstamped Replication cluster when `cluster.enabled` is set
+and each node is told which roster entry it is. The chart models this as **one
+release per node**: every release is handed the same roster and overrides only
+its own identity.
+
+`helm/charts/iggy/examples/cluster-3-node.yaml` is a ready three-node roster.
+Point the `ip` values at the nodes you will pin the releases to, then:
+
+```bash
+for i in 0 1 2; do
+ helm upgrade --install "iggy-n$i" ./helm/charts/iggy \
+ -f ./helm/charts/iggy/examples/cluster-3-node.yaml \
+ --set "server.cluster.selfReplicaId=$i" \
+ --set "server.nodeSelector.kubernetes\.io/hostname=node-$i"
+done
+```
+
+The chart turns `server.cluster` into the server's `IGGY_CLUSTER_*` environment
+variables and passes `--replica-id`, so no configuration file is mounted. The
+values mirror the server's `[[cluster.nodes]]` config one for one.
+
+### Why hostNetwork and node pinning are required
+
+The server's consensus listener binds `cluster.nodes[*].ip` **verbatim**,
unlike
+the client listeners, which keep their own bind address and take only the port
+from the roster. A pod therefore has to own the address its roster entry names.
+Pod IPs are neither stable nor known before install, and a Service ClusterIP is
+not an address a pod can bind, so the workable layout today is `hostNetwork:
+true` with each release pinned to a node and its roster `ip` set to that node's
+IP. Node IPs are known up front and survive a pod restart, which is what lets a
+replica come back and rejoin.
+
+The cost is real: the server pod shares the node's network namespace and its
+ports. Weigh it before running this in a shared cluster.
+
+If a release lands on a node whose IP is not the one in its roster entry, the
+server refuses to start rather than misbehaving quietly:
+
+```text
+Error: ShardJoinFailures { failures: [ShardJoinFailure { shard_id: 0,
+ kind: Error(Iggy(CannotBindToSocket("10.0.1.11:9090"))) }] }
+```
+
+Check the `nodeSelector` on that release against the `ip` in its roster entry.
+
+### Upgrades
+
+Host ports make a rolling update impossible: the replacement pod cannot bind
+ports the outgoing pod still holds, so it stays `Pending` while the Deployment
+waits for it to become ready. The chart therefore switches the server to the
+`Recreate` strategy whenever `hostNetwork` is set, which takes the node down
+for the length of the restart. Roll **one release at a time** and let the
+cluster regain quorum before starting the next.
+
+Turning `hostNetwork` on for a release that already exists moves the Deployment
+from `RollingUpdate` to `Recreate`. Under Helm 4's server-side apply that is
+rejected, because the patch cannot drop the `rollingUpdate` block the API
server
+defaulted in:
+
+```text
+Deployment.apps "iggy-n0" is invalid: spec.strategy.rollingUpdate: Forbidden:
+ may not be specified when strategy `type` is 'Recreate'
+```
+
+Run that one upgrade with `helm upgrade --server-side=false`, which replaces
the
+strategy in place. Set `server.strategy` explicitly if you want a different
+strategy.
+
+### Roster rules
+
+* Every node runs the **identical** `server.cluster.nodes` list. Only
+ `selfReplicaId` differs between releases.
+* `replicaId` values are unique and cover `0..N-1` for an `N`-node roster.
+* `ports.tcpReplica` is required on every entry. In cluster mode the server
+ takes every listener port from the roster and will not fall back to defaults,
+ because two nodes on one host would otherwise race for the same socket. The
+ remaining ports default to `server.ports`.
+* `server.cluster.name` is hashed into the on-disk cluster id on first boot.
+ Changing it later makes the server refuse to start against existing data.
+* `ip` must be a literal IP. Hostnames are rejected. Use
+ `advertisedAddress` for the name clients dial, which does accept DNS.
+
+### Secrets
+
+Four settings have to carry the byte-identical value on every node, so they
+belong in one Secret that all releases reference rather than in each release's
+values. Create it in the release namespace before the first install:
+
+```bash
+JWT="$(head -c 32 /dev/urandom | base64)"
+kubectl create secret generic iggy-cluster-secrets \
+ --from-literal=username=iggy \
+ --from-literal=password="$(head -c 24 /dev/urandom | base64)" \
+ --from-literal=clusterSharedSecret="$(head -c 32 /dev/urandom | base64)" \
+ --from-literal=jwtEncodingSecret="$JWT" \
+ --from-literal=jwtDecodingSecret="$JWT" \
+ --from-literal=encryptionKey="$(head -c 32 /dev/urandom | base64)"
+```
+
+`examples/cluster-3-node.yaml` points `server.users.root`, `server.encryption`,
+`server.jwt` and `server.cluster.auth` at that one Secret. The last three also
+accept an inline value, which the chart turns into its own Secret; that is
+convenient for a single node and a poor fit for a cluster, since the value then
+lives in every release's stored values.
+
+* **`server.users.root`** seeds the root user. Every node creates it locally
+ from its own `IGGY_ROOT_USERNAME` and `IGGY_ROOT_PASSWORD`, so a password
that
+ differs on one release logs in on that node only, and a first cluster boot
+ refuses to start without both.
+* **`server.cluster.auth`** makes every replica connection complete an
+ authenticated handshake or be rejected. The key is at least 32 bytes of
+ CSPRNG output. Turning it on or off is a coordinated restart of the whole
+ cluster: a node that authenticates cannot talk to one that does not.
+* **`server.encryption`** encrypts message payloads and state commands at rest
+ with AES-256-GCM, under a 32-byte base64 key. A node holding a different key
+ cannot read what its peers wrote.
+* **`server.jwt`** makes HTTP bearer tokens valid on every node and survive a
+ restart. With `cluster.auth` enabled the server already derives a
cluster-wide
+ key from the replica PSK, so setting the JWT secrets is an alternative to
that
+ rather than an addition; setting them anyway keeps tokens working if auth is
+ later turned off.
+
+None of the encryption, JWT and replica-auth values is written to the data
+directory, and each is masked as `******` in the startup log.
+
+### Storage
+
+A replica cannot move between nodes without invalidating its roster entry, so
+give each release storage that stays on its node, and size
+`server.persistence` per node rather than for the cluster. A replica that
starts
+on an empty volume rejoins by state transfer, which is correct but re-reads the
+whole dataset from its peers. `server.persistence.enabled: false` puts the
+replica on `emptyDir` and forces exactly that on every pod replacement, so the
+cluster example enables persistence and leaves `storageClass` for you to point
+at a node-local provisioner.
+
+### What the chart refuses
+
+`server.replicaCount > 1` fails at render time. Scaling the server Deployment
+produces N independent servers behind one Service, all writing the same PVC
+subpath with no lock between them, which corrupts the data directory while
+`helm --wait` still reports success. The chart ships no HorizontalPodAutoscaler
Review Comment:
`templates/hpa.yaml` is still in the tree and its
`.Values.autoscaling.enabled` now dereferences nil, so nothing renders and
`helm lint` fails - that is the red validate job. `git rm` the template,
nothing else references autoscaling.
##########
helm/charts/iggy/README.md.gotmpl:
##########
@@ -101,6 +101,153 @@ If Prometheus Operator is installed and you want
monitoring, set
`server.serviceMonitor.enabled=true` in `custom-values.yaml` or pass it on the
command line with `--set server.serviceMonitor.enabled=true`.
+## Cluster Mode
+
+The server runs a Viewstamped Replication cluster when `cluster.enabled` is set
+and each node is told which roster entry it is. The chart models this as **one
+release per node**: every release is handed the same roster and overrides only
+its own identity.
+
+`helm/charts/iggy/examples/cluster-3-node.yaml` is a ready three-node roster.
+Point the `ip` values at the nodes you will pin the releases to, then:
+
+```bash
+for i in 0 1 2; do
+ helm upgrade --install "iggy-n$i" ./helm/charts/iggy \
+ -f ./helm/charts/iggy/examples/cluster-3-node.yaml \
+ --set "server.cluster.selfReplicaId=$i" \
+ --set "server.nodeSelector.kubernetes\.io/hostname=node-$i"
+done
+```
+
+The chart turns `server.cluster` into the server's `IGGY_CLUSTER_*` environment
+variables and passes `--replica-id`, so no configuration file is mounted. The
+values mirror the server's `[[cluster.nodes]]` config one for one.
+
+### Why hostNetwork and node pinning are required
+
+The server's consensus listener binds `cluster.nodes[*].ip` **verbatim**,
unlike
+the client listeners, which keep their own bind address and take only the port
+from the roster. A pod therefore has to own the address its roster entry names.
+Pod IPs are neither stable nor known before install, and a Service ClusterIP is
+not an address a pod can bind, so the workable layout today is `hostNetwork:
+true` with each release pinned to a node and its roster `ip` set to that node's
+IP. Node IPs are known up front and survive a pod restart, which is what lets a
+replica come back and rejoin.
+
+The cost is real: the server pod shares the node's network namespace and its
+ports. Weigh it before running this in a shared cluster.
+
+If a release lands on a node whose IP is not the one in its roster entry, the
+server refuses to start rather than misbehaving quietly:
+
+```text
+Error: ShardJoinFailures { failures: [ShardJoinFailure { shard_id: 0,
+ kind: Error(Iggy(CannotBindToSocket("10.0.1.11:9090"))) }] }
+```
+
+Check the `nodeSelector` on that release against the `ip` in its roster entry.
+
+### Upgrades
+
+Host ports make a rolling update impossible: the replacement pod cannot bind
+ports the outgoing pod still holds, so it stays `Pending` while the Deployment
+waits for it to become ready. The chart therefore switches the server to the
+`Recreate` strategy whenever `hostNetwork` is set, which takes the node down
+for the length of the restart. Roll **one release at a time** and let the
+cluster regain quorum before starting the next.
+
+Turning `hostNetwork` on for a release that already exists moves the Deployment
+from `RollingUpdate` to `Recreate`. Under Helm 4's server-side apply that is
+rejected, because the patch cannot drop the `rollingUpdate` block the API
server
+defaulted in:
+
+```text
+Deployment.apps "iggy-n0" is invalid: spec.strategy.rollingUpdate: Forbidden:
+ may not be specified when strategy `type` is 'Recreate'
+```
+
+Run that one upgrade with `helm upgrade --server-side=false`, which replaces
the
Review Comment:
`--server-side` only exists in helm 4, and the prerequisites at the top
still say helm 3.2.0+. say "on helm 4" here so a helm 3 reader does not hit an
unknown flag.
##########
helm/charts/iggy/README.md.gotmpl:
##########
@@ -101,6 +101,153 @@ If Prometheus Operator is installed and you want
monitoring, set
`server.serviceMonitor.enabled=true` in `custom-values.yaml` or pass it on the
command line with `--set server.serviceMonitor.enabled=true`.
+## Cluster Mode
+
+The server runs a Viewstamped Replication cluster when `cluster.enabled` is set
+and each node is told which roster entry it is. The chart models this as **one
+release per node**: every release is handed the same roster and overrides only
+its own identity.
+
+`helm/charts/iggy/examples/cluster-3-node.yaml` is a ready three-node roster.
+Point the `ip` values at the nodes you will pin the releases to, then:
+
+```bash
+for i in 0 1 2; do
+ helm upgrade --install "iggy-n$i" ./helm/charts/iggy \
+ -f ./helm/charts/iggy/examples/cluster-3-node.yaml \
+ --set "server.cluster.selfReplicaId=$i" \
+ --set "server.nodeSelector.kubernetes\.io/hostname=node-$i"
+done
+```
+
+The chart turns `server.cluster` into the server's `IGGY_CLUSTER_*` environment
+variables and passes `--replica-id`, so no configuration file is mounted. The
+values mirror the server's `[[cluster.nodes]]` config one for one.
+
+### Why hostNetwork and node pinning are required
+
+The server's consensus listener binds `cluster.nodes[*].ip` **verbatim**,
unlike
+the client listeners, which keep their own bind address and take only the port
+from the roster. A pod therefore has to own the address its roster entry names.
+Pod IPs are neither stable nor known before install, and a Service ClusterIP is
+not an address a pod can bind, so the workable layout today is `hostNetwork:
+true` with each release pinned to a node and its roster `ip` set to that node's
+IP. Node IPs are known up front and survive a pod restart, which is what lets a
+replica come back and rejoin.
+
+The cost is real: the server pod shares the node's network namespace and its
+ports. Weigh it before running this in a shared cluster.
+
+If a release lands on a node whose IP is not the one in its roster entry, the
+server refuses to start rather than misbehaving quietly:
+
+```text
+Error: ShardJoinFailures { failures: [ShardJoinFailure { shard_id: 0,
+ kind: Error(Iggy(CannotBindToSocket("10.0.1.11:9090"))) }] }
+```
+
+Check the `nodeSelector` on that release against the `ip` in its roster entry.
+
+### Upgrades
+
+Host ports make a rolling update impossible: the replacement pod cannot bind
+ports the outgoing pod still holds, so it stays `Pending` while the Deployment
+waits for it to become ready. The chart therefore switches the server to the
+`Recreate` strategy whenever `hostNetwork` is set, which takes the node down
+for the length of the restart. Roll **one release at a time** and let the
+cluster regain quorum before starting the next.
+
+Turning `hostNetwork` on for a release that already exists moves the Deployment
+from `RollingUpdate` to `Recreate`. Under Helm 4's server-side apply that is
+rejected, because the patch cannot drop the `rollingUpdate` block the API
server
+defaulted in:
+
+```text
+Deployment.apps "iggy-n0" is invalid: spec.strategy.rollingUpdate: Forbidden:
+ may not be specified when strategy `type` is 'Recreate'
+```
+
+Run that one upgrade with `helm upgrade --server-side=false`, which replaces
the
+strategy in place. Set `server.strategy` explicitly if you want a different
+strategy.
+
+### Roster rules
+
+* Every node runs the **identical** `server.cluster.nodes` list. Only
+ `selfReplicaId` differs between releases.
+* `replicaId` values are unique and cover `0..N-1` for an `N`-node roster.
+* `ports.tcpReplica` is required on every entry. In cluster mode the server
+ takes every listener port from the roster and will not fall back to defaults,
+ because two nodes on one host would otherwise race for the same socket. The
+ remaining ports default to `server.ports`.
+* `server.cluster.name` is hashed into the on-disk cluster id on first boot.
+ Changing it later makes the server refuse to start against existing data.
+* `ip` must be a literal IP. Hostnames are rejected. Use
+ `advertisedAddress` for the name clients dial, which does accept DNS.
+
+### Secrets
+
+Four settings have to carry the byte-identical value on every node, so they
+belong in one Secret that all releases reference rather than in each release's
+values. Create it in the release namespace before the first install:
+
+```bash
+JWT="$(head -c 32 /dev/urandom | base64)"
+kubectl create secret generic iggy-cluster-secrets \
+ --from-literal=username=iggy \
+ --from-literal=password="$(head -c 24 /dev/urandom | base64)" \
+ --from-literal=clusterSharedSecret="$(head -c 32 /dev/urandom | base64)" \
+ --from-literal=jwtEncodingSecret="$JWT" \
+ --from-literal=jwtDecodingSecret="$JWT" \
+ --from-literal=encryptionKey="$(head -c 32 /dev/urandom | base64)"
+```
+
+`examples/cluster-3-node.yaml` points `server.users.root`, `server.encryption`,
+`server.jwt` and `server.cluster.auth` at that one Secret. The last three also
Review Comment:
"the last three" is off - `server.users.root` takes inline
`username`/`password` too, and the chart turns them into
`<release>-root-credentials`. say all four, then rerun helm-docs.
##########
helm/charts/iggy/templates/deployment.yaml:
##########
@@ -27,17 +35,31 @@ metadata:
labels:
{{- include "iggy.labels" . | nindent 4 }}
spec:
- {{- if not .Values.autoscaling.enabled }}
replicas: {{ .Values.server.replicaCount }}
+ {{- if .Values.server.strategy }}
+ strategy:
+ {{- toYaml .Values.server.strategy | nindent 4 }}
+ {{- else if .Values.server.hostNetwork }}
+ strategy:
+ type: Recreate
{{- end }}
selector:
matchLabels:
{{- include "iggy.selectorLabels" . | nindent 6 }}
template:
metadata:
- {{- with .Values.podAnnotations }}
+ {{- $secretsChecksum := "" }}
+ {{- if (include "iggy.createsSecret" .) }}
+ {{- $secretsChecksum = (include (print $.Template.BasePath
"/server-secrets.yaml") . | sha256sum) }}
Review Comment:
this hashes `server-secrets.yaml` only, so a root password change through
`helm upgrade` does not roll the pod, and root is rebuilt from `IGGY_ROOT_*` on
every boot - the old password stays valid until someone restarts it by hand.
hash `root-user-credentials.yaml` too.
##########
helm/charts/iggy/values.yaml:
##########
@@ -31,15 +32,20 @@ server:
repository: apache/iggy
# -- Image pull policy
pullPolicy: Always
- # -- Server image tag (overrides chart appVersion)
- tag: "0.7.0"
+ # -- Server image tag. Empty uses the chart appVersion.
+ tag: ""
ports:
# -- HTTP API port
http: 3000
- # -- QUIC protocol port
+ # -- QUIC protocol port (UDP)
quic: 8080
# -- TCP protocol port
tcp: 8090
+ # -- WebSocket protocol port
+ websocket: 8092
+ # -- Replica-to-replica consensus port. Only published when
Review Comment:
this key never wins - the roster entry's `tcpReplica` always overrides it
(`--set server.ports.tcpReplica=1234` renders identically), and the service
port it was "published" on is gone. drop it or say the roster owns the port.
##########
helm/charts/iggy/templates/_helpers.tpl:
##########
@@ -97,3 +97,234 @@ Create the name of the service account to use
{{- default "default" .Values.serviceAccount.name }}
{{- end }}
{{- end }}
+
+{{/*
+Validate the cluster roster and fail the render with an actionable message.
+Every check here is one the server would otherwise only reject at boot, after
+the pod is already scheduled.
+*/}}
+{{- define "iggy.validateCluster" -}}
+ {{- $cluster := .Values.server.cluster }}
+ {{- if not $cluster.nodes }}
+ {{- fail "server.cluster.enabled is true but server.cluster.nodes is
empty. Every node runs the identical roster; see the Cluster Mode section of
the chart README." }}
+ {{- end }}
+ {{- if and $cluster.requireHostNetwork (not .Values.server.hostNetwork) }}
+ {{- fail "server.cluster.enabled is true but server.hostNetwork is false.
The replica listener binds the roster ip verbatim, which is not an address a
pod owns on the cluster network, so the pod dies at boot with
CannotBindToSocket. Set server.hostNetwork to true, or
server.cluster.requireHostNetwork to false if the roster ip really is one this
pod owns." }}
+ {{- end }}
+ {{- $root := .Values.server.users.root }}
+ {{- if and (not $root.createSecret) (not $root.existingSecret.name) }}
+ {{- fail "server.users.root has neither createSecret nor
existingSecret.name, so the container receives no IGGY_ROOT_USERNAME or
IGGY_ROOT_PASSWORD. A first cluster boot refuses to start without both, because
every node creates root locally and the credentials have to come out identical
on all of them." }}
+ {{- end }}
+ {{- $count := len $cluster.nodes }}
+ {{- $seen := dict }}
+ {{- range $cluster.nodes }}
+ {{- if not .name }}
+ {{- fail "every server.cluster.nodes entry needs a name" }}
+ {{- end }}
+ {{- if not .ip }}
+ {{- fail (printf "server.cluster.nodes entry %q has no ip. The replica
listener binds this address verbatim, so it must be a literal IP the pod owns."
.name) }}
+ {{- end }}
+ {{- if not (regexMatch "^[0-9a-fA-F.:]+$" .ip) }}
Review Comment:
a numeric-looking ip (`--set 'server.cluster.nodes[1].ip=2001'` or an
unquoted `ip: 2001`) arrives as a number and this aborts with `wrong type for
value; expected string; got int64`. use `(toString .ip)` here and in the two
`eq` checks below.
--
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]
