imbajin commented on code in PR #3105:
URL: https://github.com/apache/hugegraph/pull/3105#discussion_r3666488631
##########
hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh:
##########
@@ -185,16 +366,31 @@ function wait_for_startup() {
return 1
fi
- status=$(curl -I -sS -k -w "%{http_code}" -o /dev/null "$server_url"
2> "$error_file_name")
- if [[ $status -eq 200 || $status -eq 401 ]]; then
+ # Bound each probe by the time left in the overall deadline: without
+ # --max-time a single blackholed request blocks past ${timeout_s}s.
+ local remain_s=$((stop_s - now_s))
+ [ "$remain_s" -lt 1 ] && remain_s=1
Review Comment:
⚠️ The overall deadline can still be exceeded at its boundary. The inclusive
loop permits another iteration when `now_s == stop_s`, then this line converts
the expired `remain_s=0` into a fresh one-second curl budget. Two independent
exact-head lanes reproduced two probes for `timeout_s=1`; when the
deadline-edge probe consumed its advertised budget, elapsed time was about two
seconds. Please refresh the clock before every curl and stop when no positive
budget remains instead of clamping zero to one, then add a regression proving
no probe begins at the deadline.
##########
.github/workflows/docker-build-ci.yml:
##########
@@ -47,6 +47,28 @@ jobs:
run: |
IMAGE_ID=$(docker build -q -f ${{ matrix.dockerfile }} .)
echo "Built: $IMAGE_ID"
+ echo "IMAGE_ID=$IMAGE_ID" >> "$GITHUB_ENV"
HC=$(docker inspect --format='{{json .Config.Healthcheck}}'
"$IMAGE_ID")
echo "Healthcheck: $HC"
[[ "$HC" != "null" ]] || { echo "ERROR: HEALTHCHECK missing in ${{
matrix.dockerfile }}"; exit 1; }
+
+ # The startup preflight needs a socket-table tool, and the base image
+ # ships none of its own. Without one every start reports "unknown" and
+ # a duplicate start is no longer refused, so assert the image can
+ # actually answer. Only the server images run check_port.
+ # TODO(docker-ci): this pins the probe dependency, not the behaviour it
+ # protects. A full duplicate-start/stop check needs a booted server with
+ # a backend, which belongs with the e2e job rather than the image build.
+ - name: Port preflight can answer inside ${{ matrix.dockerfile }}
Review Comment:
⚠️ This image-level port-preflight check executes the changed `bin/util.sh`,
but the workflow's PR filter still includes only Dockerfiles and
`.dockerignore`. A later PR changing only the probed utility will skip the only
check that proves the built image has a compatible socket-table tool. Please
add `hugegraph-server/hugegraph-dist/src/assembly/static/bin/util.sh` to
`pull_request.paths`.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-check-port.sh:
##########
@@ -0,0 +1,395 @@
+#!/bin/bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# Contract tests for the startup port preflight in bin/util.sh.
+#
+# The preflight is best effort: the server's own bind is authoritative. These
+# tests pin the three-state contract (busy / free / unknown) rather than the
+# internals of any one probe.
+#
+# TODO(test-check-port): the Linux and BSD detection branches are both driven
+# by mocked tool output, so on any one runner only the host's own branch is
+# ever exercised against a real kernel. The single real-listener case covers
+# whichever OS the job runs on. Closing this needs the suite to run on both
+# a Linux and a macOS runner, which CI already does for the server job.
+
+set -u
+
+STATIC_DIR="${1:-hugegraph-server/hugegraph-dist/src/assembly/static}"
+UTIL_SH="$STATIC_DIR/bin/util.sh"
+
+if [[ ! -f "$UTIL_SH" ]]; then
+ echo "SKIP: util.sh not found at $UTIL_SH"
+ exit 0
Review Comment:
⚠️ Both Linux and macOS CI invoke this required contract test with an
explicit production path, yet a missing or moved `util.sh` exits successfully
without running any assertion. That can turn a broken test wiring change into a
green check. Please fail when the explicitly selected utility is absent;
reserve a skip result for a clearly optional invocation mode.
##########
hugegraph-store/hg-store-dist/src/assembly/static/bin/util.sh:
##########
@@ -286,26 +311,37 @@ download_and_verify() {
local url=$1
local filepath=$2
local expected_md5=$3
+ local actual_md5
- if [[ -f $filepath ]]; then
+ if [[ -f "$filepath" ]]; then
echo "File $filepath exists. Verifying MD5 checksum..."
- actual_md5=$(md5sum $filepath | awk '{ print $1 }')
- if [[ $actual_md5 != $expected_md5 ]]; then
+ actual_md5=$(md5sum -- "$filepath" | awk '{ print $1 }')
+ if [[ "$actual_md5" != "$expected_md5" ]]; then
echo "MD5 checksum verification failed for $filepath. Expected:
$expected_md5, but got: $actual_md5"
echo "Deleting $filepath..."
- rm -f $filepath
+ rm -f -- "$filepath"
Review Comment:
⚠️ The checksum repair is still racy across concurrent starts. Two callers
can both verify the same corrupt destination; one can download and atomically
install a valid replacement at line 342, after which the other executes this
stale removal and deletes the valid file. Please leave the destination in place
while downloading and verifying a private temporary file, atomically replace it
only after success, and use a per-destination lock if callers require stable
success semantics; add a coordinated two-caller regression.
--
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]
