imbajin commented on code in PR #2952:
URL: https://github.com/apache/hugegraph/pull/2952#discussion_r2878259267


##########
hugegraph-pd/hg-pd-dist/docker/docker-entrypoint.sh:
##########
@@ -15,8 +15,79 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+set -euo pipefail
 
-# start hugegraph pd
-./bin/start-hugegraph-pd.sh -j "$JAVA_OPTS"
+log() { echo "[hugegraph-pd-entrypoint] $*"; }
 
+require_env() {
+    local name="$1"
+    if [[ -z "${!name:-}" ]]; then
+        echo "ERROR: missing required env '${name}'" >&2; exit 2
+    fi
+}
+
+json_escape() {
+    local s="$1"
+    s=${s//\\/\\\\}; s=${s//\"/\\\"}; s=${s//$'\n'/}
+    printf "%s" "$s"
+}
+
+migrate_env() {
+    local old_name="$1" new_name="$2"
+
+    if [[ -n "${!old_name:-}" && -z "${!new_name:-}" ]]; then
+        log "WARN: deprecated env '${old_name}' detected; mapping to 
'${new_name}'"
+        export "${new_name}=${!old_name}"
+    fi
+}
+
+migrate_env "GRPC_HOST"             "HG_PD_GRPC_HOST"
+migrate_env "RAFT_ADDRESS"          "HG_PD_RAFT_ADDRESS"
+migrate_env "RAFT_PEERS"            "HG_PD_RAFT_PEERS_LIST"
+migrate_env "PD_INITIAL_STORE_LIST" "HG_PD_INITIAL_STORE_LIST"
+
+# ── Required vars ─────────────────────────────────────────────────────
+require_env "HG_PD_GRPC_HOST"
+require_env "HG_PD_RAFT_ADDRESS"
+require_env "HG_PD_RAFT_PEERS_LIST"
+require_env "HG_PD_INITIAL_STORE_LIST"
+
+: "${HG_PD_GRPC_PORT:=8686}"
+: "${HG_PD_REST_PORT:=8620}"
+: "${HG_PD_DATA_PATH:=/hugegraph-pd/pd_data}"
+: "${HG_PD_INITIAL_STORE_COUNT:=1}"
+
+SPRING_APPLICATION_JSON="$(cat <<JSON
+{
+  "grpc":   { "host": "$(json_escape "${HG_PD_GRPC_HOST}")",
+              "port": "$(json_escape "${HG_PD_GRPC_PORT}")" },
+  "server": { "port": "$(json_escape "${HG_PD_REST_PORT}")" },
+  "raft":   { "address":    "$(json_escape "${HG_PD_RAFT_ADDRESS}")",
+              "peers-list": "$(json_escape "${HG_PD_RAFT_PEERS_LIST}")" },
+  "pd":     { "data-path":          "$(json_escape "${HG_PD_DATA_PATH}")",
+              "initial-store-list": "$(json_escape 
"${HG_PD_INITIAL_STORE_LIST}")" ,
+              "initial-store-count": ${HG_PD_INITIAL_STORE_COUNT} }
+}
+JSON
+)"
+export SPRING_APPLICATION_JSON
+
+log "effective config:"
+log "  grpc.host=${HG_PD_GRPC_HOST}"
+log "  grpc.port=${HG_PD_GRPC_PORT}"
+log "  server.port=${HG_PD_REST_PORT}"
+log "  raft.address=${HG_PD_RAFT_ADDRESS}"
+log "  raft.peers-list=${HG_PD_RAFT_PEERS_LIST}"
+log "  pd.initial-store-list=${HG_PD_INITIAL_STORE_LIST}"
+log "  pd.initial-store-count=${HG_PD_INITIAL_STORE_COUNT}"
+log "  pd.data-path=${HG_PD_DATA_PATH}"
+
+log "Patching conf/application.yml with cluster settings..."
+sed -i "s|address: 127.0.0.1:8610|address: ${HG_PD_RAFT_ADDRESS}|"             
      conf/application.yml

Review Comment:
   ‼️ **Dual config mechanism — `SPRING_APPLICATION_JSON` and `sed` both 
applied**
   
   This entrypoint sets config via `SPRING_APPLICATION_JSON` (which Spring Boot 
will use at startup), and then also patches `conf/application.yml` with `sed 
-i`. These two approaches overlap on the same keys.
   
   The `sed` commands silently do nothing if the default values in 
`application.yml` have changed (e.g. `127.0.0.1` was already replaced in a new 
image build). This makes the effective configuration hard to reason about.
   
   Consider removing the `sed` block entirely and relying solely on 
`SPRING_APPLICATION_JSON`, which is the standard Spring Boot override mechanism 
and has higher priority than the yml file anyway.



##########
hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh:
##########
@@ -15,32 +15,78 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
+set -euo pipefail
 
-# create a folder to save the docker-related file
-DOCKER_FOLDER='./docker'
-mkdir -p $DOCKER_FOLDER
-
+DOCKER_FOLDER="./docker"
 INIT_FLAG_FILE="init_complete"
+GRAPH_CONF="./conf/graphs/hugegraph.properties"
+
+mkdir -p "${DOCKER_FOLDER}"
+
+log() { echo "[hugegraph-server-entrypoint] $*"; }
+
+set_prop() {
+    local key="$1" val="$2" file="$3"
+    local esc_key esc_val
+
+    esc_key=$(printf '%s' "$key" | sed -e 's/[][(){}.^$*+?|\\/]/\\&/g')
+    esc_val=$(printf '%s' "$val" | sed -e 's/[&|\\]/\\&/g')
 
-if [ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]; then
-    # wait for storage backend
-    ./bin/wait-storage.sh
-    if [ -z "$PASSWORD" ]; then
-        echo "init hugegraph with non-auth mode"
+    if grep -qE "^[[:space:]]*${esc_key}[[:space:]]*=" "${file}"; then
+        sed -ri "s|^([[:space:]]*${esc_key}[[:space:]]*=).*|\\1${esc_val}|" 
"${file}"
+    else
+        printf '%s=%s\n' "$key" "$val" >> "${file}"
+    fi
+}
+
+migrate_env() {
+    local old_name="$1" new_name="$2"
+
+    if [[ -n "${!old_name:-}" && -z "${!new_name:-}" ]]; then
+        log "WARN: deprecated env '${old_name}' detected; mapping to 
'${new_name}'"
+        export "${new_name}=${!old_name}"
+    fi
+}
+
+migrate_env "BACKEND"  "HG_SERVER_BACKEND"
+migrate_env "PD_PEERS" "HG_SERVER_PD_PEERS"
+
+# ── Map env → properties file ─────────────────────────────────────────
+[[ -n "${HG_SERVER_BACKEND:-}"  ]] && set_prop "backend"  
"${HG_SERVER_BACKEND}"  "${GRAPH_CONF}"
+[[ -n "${HG_SERVER_PD_PEERS:-}" ]] && set_prop "pd.peers" 
"${HG_SERVER_PD_PEERS}" "${GRAPH_CONF}"
+
+# ── Build wait-storage env ─────────────────────────────────────────────
+WAIT_ENV=()
+[[ -n "${HG_SERVER_BACKEND:-}"  ]] && 
WAIT_ENV+=("hugegraph.backend=${HG_SERVER_BACKEND}")
+[[ -n "${HG_SERVER_PD_PEERS:-}" ]] && 
WAIT_ENV+=("hugegraph.pd.peers=${HG_SERVER_PD_PEERS}")
+
+# ── Init store (once) ─────────────────────────────────────────────────
+if [[ ! -f "${DOCKER_FOLDER}/${INIT_FLAG_FILE}" ]]; then
+    if (( ${#WAIT_ENV[@]} > 0 )); then
+        env "${WAIT_ENV[@]}" ./bin/wait-storage.sh
+    else
+        ./bin/wait-storage.sh
+    fi
+
+    if [[ -z "${PASSWORD:-}" ]]; then
+        log "init hugegraph with non-auth mode"
         ./bin/init-store.sh
     else
-        echo "init hugegraph with auth mode"
+        log "init hugegraph with auth mode"
         ./bin/enable-auth.sh
-        echo "$PASSWORD" | ./bin/init-store.sh
+        echo "${PASSWORD}" | ./bin/init-store.sh
     fi
-    # create a flag file to avoid re-init when restarting
-    touch ${DOCKER_FOLDER}/${INIT_FLAG_FILE}
+    touch "${DOCKER_FOLDER}/${INIT_FLAG_FILE}"
 else
-    echo "Hugegraph Initialization already done. Skipping re-init..."
+    log "HugeGraph initialization already done. Skipping re-init..."
 fi
 
-# start hugegraph-server
-# remove "-g zgc" now, which is only available on ARM-Mac with java > 13 
-./bin/start-hugegraph.sh -j "$JAVA_OPTS" 
+./bin/start-hugegraph.sh -j "${JAVA_OPTS:-}" -t 120
+
+STORE_REST="${STORE_REST:-hg-store:8520}"

Review Comment:
   ⚠️ **`STORE_REST` default is set after `start-hugegraph.sh`, breaking 
single-node deployments**
   
   `STORE_REST` is assigned its default on line 86, but `start-hugegraph.sh` is 
called on line 84. More importantly, `docker-compose.yml` (single-node) does 
not pass `STORE_REST` as an environment variable, so when `wait-partition.sh` 
runs it will hit:
   
   ```
   : "${STORE_REST:?STORE_REST not set}"
   ```
   
   ...and exit 1, crashing the container.
   
   The fix is to move the default assignment and export to **before** 
`start-hugegraph.sh`:
   ```suggestion
   STORE_REST="${STORE_REST:-store:8520}"
   export STORE_REST
   
   ./bin/start-hugegraph.sh -j "${JAVA_OPTS:-}" -t 120
   ```



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