Repository navigation
Conversation
Images built from current master run as UID/GID 10001. Add a chown step to the PD and Store bind-mount examples, a troubleshooting entry for volumes written by older root images, and the Helm fsGroup upgrade step, in English and Chinese. Pairs with apache/hugegraph#3381.
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The Docker, Compose and Helm statements match apache/hugegraph#3381 at 305b4f53, and the EN and CN pages agree; one Minor finding: the new Helm fsGroup block splits an existing paragraph, so its lifecycle warning now reads as part of the chown advice. This should merge only after apache/hugegraph#3381, which is still open: on current master 86c05ef9 the four Dockerfiles have no USER and pd.podSecurityContext / store.podSecurityContext are still {}, so "images built from current master run as UID/GID 10001" and "the chart defaults ... to fsGroup: 10001" are not true until it lands. Evidence: read the full diff at 38af8df (10 files, EN and CN) and checked each statement against #3381: hugegraph-pd/Dockerfile, hugegraph-store/Dockerfile, hugegraph-server/Dockerfile and Dockerfile-hstore (HG_UID/HG_GID 10001, useradd hugegraph, install -d of pd_data, storage, rocksdb-data and logs, USER ${HG_UID}:${HG_GID}), helm/hugegraph/values.yaml (PD and Store fsGroup: 10001 with OnRootMismatch, image tag: latest, pullPolicy: Always, Server has no PVC), helm/hugegraph/docs/operations.md and docker/README.md (same chown command, --renew-anon-volumes caveat, NFS and hostPath note). The docker run bind-mount targets (/hugegraph-pd/pd_data, /hugegraph-store/storage) match the examples, the preload script only needs to be readable because start-hugegraph.sh copies it inside the image-owned scripts/ directory, and both anchors (#5-upgrade-and-verification, #5-升级和验证) match the ## 5 headings. No other page mounts host paths into these images. Not built with Hugo; site CI (prepare) was still queued.
| ``` | ||
|
|
||
| If a Pod still reports `Permission denied`, set `fsGroupChangePolicy: Always` for one rollout or change the owner once with `chown -R 10001:10001`; | ||
| volumes whose CSI driver ignores `fsGroup` (for example many NFS drivers) and `hostPath` volumes need that `chown` too. |
There was a problem hiding this comment.
Minor: This block is inserted between "Adopt new defaults deliberately ..." and "Do not treat PD membership, storage or credential changes as ordinary independent overrides; consult the lifecycle reference.", with no blank line before the latter. That sentence used to close the --reuse-values paragraph; it now renders as the last sentence of this Permission denied / chown paragraph, so it reads as advice about fsGroup. The CN page has the same split (PD 成员、持久存储和共享凭据变更需要按生命周期说明协调处理。 now follows the chown paragraph). Please move the new text, in both languages, below that sentence so the original paragraph stays intact.
apache/hugegraph#3381 applies the PD and Store fsGroup in the chart templates, so an upgrade with --reuse-values no longer needs explicit --set flags. Drop them from the English and Chinese upgrade notes.
|
The earlier review comments below still apply at 744478b; this head does not fix them.
|
Mirror apache/hugegraph#3381: switch PD and Store to the 10001 images in the same rollout that first applies fsGroup, and use a one-time fsGroupChangePolicy=Always rollout when root images already ran with it.
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The new commit mirrors apache/hugegraph#3381 at caeaa37e: switch PD and Store to the 10001 images in the same rollout that first applies fsGroup, pin pd.image / store.image instead of latest, and use a one-time fsGroupChangePolicy=Always rollout when root images already ran with fsGroup; the EN and CN pages agree with each other and with the chart. One new Minor finding: the bind-mount upgrade guidance only asks for a writable directory, while a directory an older root image wrote needs a recursive chown. The earlier Minor comment on the split Helm paragraph still applies at this head. As before, this should merge only after apache/hugegraph#3381, which is still open. Evidence: read the full diff at 93c1f6d (10 files, EN and CN) and the 744478b..93c1f6d delta; checked it against the #3381 diff at caeaa37e: hugegraph.dataPodSecurityContext in _helpers.tpl sets fsGroup 10001 when unset and keeps an explicit fsGroupChangePolicy, so --reuse-values --set *.podSecurityContext.fsGroupChangePolicy=Always on a release with an empty context renders fsGroup: 10001 plus Always (also asserted in tests/data_pod_security_context_test.yaml); values.yaml image tag: latest; helm/hugegraph/README.md and docs/operations.md carry the same rollout text; the four Dockerfiles create pd_data, storage, rocksdb-data and logs owned by 10001 and keep VOLUME on the application directory; start-hugegraph.sh copies the mounted example.groovy into the image-owned scripts/, so read access is enough; the docker run mount targets match the new chown lines; both #5 anchors match the headings. Not built with Hugo; site CI prepare and Build group 0 passed, deploy queued.
The earlier review comments below still apply at 93c1f6d; this head does not fix them.
- Minor: This block is inserted between "Adopt new defaults deliberately ..." and "Do not treat PD membership, storage or credential changes as ordinary independent overrides; consult the lifecycle reference.", with no blank line before the latter. That sentence used to close the
--reuse-valuesparagraph; it now renders as the last sentence of thisPermission denied/chownparagraph, so it reads as advice about fsGroup. The CN page has the same split (PD 成员、持久存储和共享凭据变更需要按生命周期说明协调处理。now follows thechownparagraph). Please move the new text, in both languages, below that sentence so the original paragraph stays intact. (#544 (comment))
| ``` | ||
|
|
||
| Do not upgrade running containers with a plain `up -d`: Compose reattaches the anonymous volume holding the previous image's root-owned application | ||
| directory. Run `down` first, or use `up -d --renew-anon-volumes`. A host directory used as a bind mount must be writable by UID `10001`. On Kubernetes, |
There was a problem hiding this comment.
Minor: "A host directory used as a bind mount must be writable by UID 10001" sits in the entry for upgrading data written by older root images, but unlike the volume case above it gives no command, and the quickstarts this points back to use a non-recursive sudo chown 10001:10001 /path/to/data (hugegraph-pd.md, hugegraph-hstore.md). Someone who previously ran the master-built PD or Store with -v /path/to/data:/hugegraph-pd/pd_data (or /hugegraph-store/storage) has root-owned RocksDB and Raft files inside that directory; mkdir -p is a no-op and the non-recursive chown changes only the top directory, so the 10001 image still fails with Permission denied on those files. Please say that a host directory written by an older image needs sudo chown -R 10001:10001 <dir>, here and in the CN entry (作为 bind mount 的宿主机目录必须对 UID 10001 可写).
Mirror apache/hugegraph#3381: set the 10001 image tags in the one-time fsGroupChangePolicy=Always upgrade, and show the follow-up upgrade that sets the policy back to OnRootMismatch.
68ce476 to
f777f3a
Compare
Purpose of the PR
apache/hugegraph#3381 makes the PD, Store and Server images run as the non-root user
hugegraph(UID/GID10001). The website quickstarts bind-mount hostdirectories and upgrade existing Helm releases in ways that now need an extra step, so this is the paired documentation PR for it. Changes apply to the
latest(master) docs only; released 1.7 images still run as root.Main Changes
English and Chinese:
quickstart/hugegraph/hugegraph-pd.md,hugegraph-hstore.md: the master-builtdocker runexamples create the bind-mounted data directory andchownit to10001:10001first.quickstart/hugegraph/hugegraph-server.md: a mounted preload script must be readable by UID10001with master-built images.guides/hugegraph-docker-cluster.md: new troubleshooting entry forPermission deniedafter upgrading, with the one-timechownfor volumes writtenby older root images and the Compose
down/up -d --renew-anon-volumescaveat.quickstart/hugegraph/hugegraph-helm.md(5 Upgrade and verification):--reuse-valueskeeps an existing release's emptypodSecurityContext, so setfsGroup: 10001for PD and Store explicitly;OnRootMismatch, NFS andhostPathcaveats.Verifying these changes
bash dist/validate-links.shpasses;git diff --checkis clean; new prose is wrapped at 160 characters.🤖 Generated with Claude Code