test(ci): require dependency-aware readiness (#6491)

* test(e2e): fail closed on runner readiness

* test(ci): require dependency-aware readiness
This commit is contained in:
Zhengchao An
2026-08-24 14:31:38 +08:00
committed by GitHub
parent 20a9c12f86
commit 9681f19bec
4 changed files with 222 additions and 22 deletions
+1 -1
View File
@@ -311,7 +311,7 @@ jobs:
- name: Wait for RustFS ready - name: Wait for RustFS ready
run: | run: |
for _ in {1..120}; do for _ in {1..120}; do
if curl -sf "http://${S3_HOST}:${S3_PORT}/health" >/dev/null 2>&1; then if curl -sf "http://${S3_HOST}:${S3_PORT}/health/ready" >/dev/null 2>&1; then
echo "RustFS is ready" echo "RustFS is ready"
exit 0 exit 0
fi fi
+1 -1
View File
@@ -155,7 +155,7 @@ jobs:
- name: Wait for RustFS ready - name: Wait for RustFS ready
run: | run: |
for _ in {1..60}; do for _ in {1..60}; do
if curl -sf http://127.0.0.1:9000/health >/dev/null 2>&1; then if curl -sf http://127.0.0.1:9000/health/ready >/dev/null 2>&1; then
echo "RustFS is ready" echo "RustFS is ready"
exit 0 exit 0
fi fi
+211 -4
View File
@@ -228,9 +228,120 @@ def check_runner_selection(root: Path) -> list[str]:
def check_s3_tests_runner(root: Path) -> list[str]: def check_s3_tests_runner(root: Path) -> list[str]:
runner = (root / "scripts/s3-tests/run.sh").read_text() runner = (root / "scripts/s3-tests/run.sh").read_text()
errors: list[str] = []
if "--showlocals" in runner: if "--showlocals" in runner:
return ["scripts/s3-tests/run.sh: pytest failure diagnostics must not dump local values"] errors.append("scripts/s3-tests/run.sh: pytest failure diagnostics must not dump local values")
return [] readiness = re.search(
r"test_s3_api_ready\(\) \{(?P<body>.*?)\n\}\n\n# First, wait",
runner,
re.DOTALL,
)
readiness_body = readiness.group("body") if readiness else ""
if (
'"http://${S3_HOST}:${S3_PORT}/health/ready"' not in readiness_body
or re.search(r"/health(?=[\"'\s])", readiness_body)
or '[ "${READY_CODE}" != "200" ]' not in readiness_body
):
errors.append("scripts/s3-tests/run.sh: startup must require the ready endpoint to return HTTP 200")
signed_probe = re.search(
r"if command -v awscurl\b[^\n]*; then(?P<body>.*?)\n\s*fi\s*"
r"(?:#[^\n]*\n\s*)*return 0\s*$",
readiness_body,
re.DOTALL,
)
signed_body = signed_probe.group("body") if signed_probe else ""
readiness_code = "\n".join(line.split("#", 1)[0] for line in readiness_body.splitlines())
signed_code = "\n".join(line.split("#", 1)[0] for line in signed_body.splitlines())
signed_commands = [
command
for line in signed_body.splitlines()
if (command := line.split("#", 1)[0].strip())
]
signed_success = re.search(
r'if echo "\$\{RESPONSE\}" \| grep -q "<ListAllMyBucketsResult"; then\s*'
r"(?:#[^\n]*\n\s*)*return 0\s*\n\s*fi",
signed_body,
)
response_capture = re.search(
r"^\s*RESPONSE=\$\((?P<command>.*?)\)\s*$",
signed_code,
re.DOTALL | re.MULTILINE,
)
response_command = response_capture.group("command") if response_capture else ""
response_command = response_command.replace("\\\n", " ").strip()
response_operators = re.search(
r";|\|\||&&|(?<![>|])\|(?!\|)|(?<![>&])&(?![>&0-9])|\$\(|[<>]\(|`",
response_command,
)
if (
len(re.findall(r"\breturn\s+0\b", readiness_code)) != 2
or len(re.findall(r"\breturn\s+0\b", signed_code)) != 1
or len(re.findall(r"(?<![A-Za-z0-9_])RESPONSE=", signed_code)) != 1
or not response_command.startswith("awscurl ")
or "\n" in response_command
or response_operators
or not signed_success
or not signed_commands
or signed_commands[-1] != "return 1"
):
errors.append("scripts/s3-tests/run.sh: readiness must bind success to the signed probe")
return errors
def check_workflow_readiness(root: Path) -> list[str]:
errors: list[str] = []
for relative in (".github/workflows/e2e-s3tests.yml", ".github/workflows/mint.yml"):
path = root / relative
try:
lines = path.read_text().splitlines()
except FileNotFoundError:
errors.append(f"{relative}: missing workflow")
continue
start = next(
(index for index, line in enumerate(lines) if line.strip() == "- name: Wait for RustFS ready"),
None,
)
if start is None:
errors.append(f"{relative}: missing RustFS readiness step")
continue
indent = len(lines[start]) - len(lines[start].lstrip())
end = next(
(
index
for index in range(start + 1, len(lines))
if lines[index].strip().startswith("- name:")
and len(lines[index]) - len(lines[index].lstrip()) <= indent
),
len(lines),
)
readiness_step = "\n".join(lines[start:end])
ready_branch = re.search(
r"if curl [^\n]*/health/ready[^\n]*; then(?P<body>.*?)\n\s*fi",
readiness_step,
re.DOTALL,
)
ready_body = ready_branch.group("body") if ready_branch else ""
ready_condition = ready_branch.group(0).splitlines()[0].rsplit("; then", 1)[0] if ready_branch else ""
step_commands = [
command
for line in readiness_step.splitlines()
if (command := line.split("#", 1)[0].strip())
]
step_code = "\n".join(step_commands)
ready_code = "\n".join(line.split("#", 1)[0] for line in ready_body.splitlines())
if (
"/health/ready" not in readiness_step
or re.search(r"/health(?=[\"'\s])", readiness_step)
or not re.search(r"curl [^\n]*(?:-sf|-fs|--fail)", readiness_step)
or not ready_branch
or re.search(r"\|\||&&|(?<![>|])\|(?!\|)|;|(?<![>&])&(?![>&])", ready_condition)
or len(re.findall(r"\bexit\s+0\b", step_code)) != 1
or not re.search(r"\bexit\s+0\b", ready_code)
or not step_commands
or step_commands[-1] != "exit 1"
):
errors.append(f"{relative}: RustFS readiness step must fail closed on /health/ready")
return errors
def profile_selection_entries(root: Path, profile: str) -> tuple[Path, list[str], dict[str, str]]: def profile_selection_entries(root: Path, profile: str) -> tuple[Path, list[str], dict[str, str]]:
@@ -607,6 +718,7 @@ def validate(root: Path) -> list[str]:
errors.extend(check_fuzz_targets(root)) errors.extend(check_fuzz_targets(root))
errors.extend(check_runner_selection(root)) errors.extend(check_runner_selection(root))
errors.extend(check_s3_tests_runner(root)) errors.extend(check_s3_tests_runner(root))
errors.extend(check_workflow_readiness(root))
errors.extend(check_profile_definitions(root)) errors.extend(check_profile_definitions(root))
errors.extend(check_scheduled_alerts(root)) errors.extend(check_scheduled_alerts(root))
return errors return errors
@@ -715,19 +827,114 @@ class SelfTests(unittest.TestCase):
root = Path(tmp) root = Path(tmp)
runner = root / "scripts/s3-tests/run.sh" runner = root / "scripts/s3-tests/run.sh"
runner.parent.mkdir(parents=True) runner.parent.mkdir(parents=True)
runner.write_text("tox -- -vv -ra --tb=long\n") valid_runner = (
"test_s3_api_ready() {\n"
' READY_CODE=$(curl "http://${S3_HOST}:${S3_PORT}/health/ready")\n'
' if [ "${READY_CODE}" != "200" ]; then\n'
" return 1\n"
" fi\n"
" if command -v awscurl; then\n"
" RESPONSE=$(awscurl --service s3)\n"
' if echo "${RESPONSE}" | grep -q "<ListAllMyBucketsResult"; then\n'
" return 0\n"
" fi\n"
" return 1\n"
" fi\n"
" return 0\n"
"}\n\n# First, wait\n"
"tox -- -vv -ra --tb=long\n"
)
runner.write_text(valid_runner)
self.assertEqual(check_s3_tests_runner(root), []) self.assertEqual(check_s3_tests_runner(root), [])
runner.write_text("tox -- -vv -ra --showlocals --tb=long\n") runner.write_text(valid_runner.replace("--tb=long", "--showlocals --tb=long"))
self.assertEqual(len(check_s3_tests_runner(root)), 1)
runner.write_text(valid_runner.replace("/health/ready", "/health"))
self.assertEqual(len(check_s3_tests_runner(root)), 1)
runner.write_text(valid_runner.replace(" return 0\n}\n", " return 0\n return 0\n}\n"))
self.assertEqual(len(check_s3_tests_runner(root)), 1)
runner.write_text(
valid_runner.replace(
" return 1\n fi\n return 0\n",
" return 0\n fi\n return 1\n",
)
)
self.assertEqual(len(check_s3_tests_runner(root)), 1)
runner.write_text(valid_runner.replace(" return 1\n fi", " false || return 0\n return 1\n fi"))
self.assertEqual(len(check_s3_tests_runner(root)), 1)
runner.write_text(valid_runner.replace('echo "${RESPONSE}"', 'echo "<ListAllMyBucketsResult"'))
self.assertEqual(len(check_s3_tests_runner(root)), 1)
runner.write_text(
valid_runner.replace(
"RESPONSE=$(awscurl --service s3)",
'RESPONSE=$(awscurl --service s3 || echo "<ListAllMyBucketsResult")',
)
)
self.assertEqual(len(check_s3_tests_runner(root)), 1)
runner.write_text(
valid_runner.replace(
"RESPONSE=$(awscurl --service s3)",
'RESPONSE=$(awscurl --service s3; echo "<ListAllMyBucketsResult")',
)
)
self.assertEqual(len(check_s3_tests_runner(root)), 1) self.assertEqual(len(check_s3_tests_runner(root)), 1)
with ( with (
mock.patch(__name__ + ".check_e2e_modules", return_value=[]), mock.patch(__name__ + ".check_e2e_modules", return_value=[]),
mock.patch(__name__ + ".check_fuzz_targets", return_value=[]), mock.patch(__name__ + ".check_fuzz_targets", return_value=[]),
mock.patch(__name__ + ".check_runner_selection", return_value=[]), mock.patch(__name__ + ".check_runner_selection", return_value=[]),
mock.patch(__name__ + ".check_workflow_readiness", return_value=[]),
mock.patch(__name__ + ".check_profile_definitions", return_value=[]), mock.patch(__name__ + ".check_profile_definitions", return_value=[]),
mock.patch(__name__ + ".check_scheduled_alerts", return_value=[]), mock.patch(__name__ + ".check_scheduled_alerts", return_value=[]),
): ):
self.assertEqual(len(validate(root)), 1) self.assertEqual(len(validate(root)), 1)
def test_workflow_readiness_requires_dependency_probe(self) -> None:
with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp)
workflows = root / ".github/workflows"
workflows.mkdir(parents=True)
valid_workflow = (
"jobs:\n"
" test:\n"
" steps:\n"
" - name: Wait for RustFS ready\n"
" run: |\n"
" for _ in {1..60}; do\n"
" if curl -sf http://127.0.0.1:9000/health/ready; then\n"
" exit 0\n"
" fi\n"
" done\n"
" exit 1\n"
" - name: Run tests\n"
" run: true\n"
)
for name in ("e2e-s3tests.yml", "mint.yml"):
(workflows / name).write_text(valid_workflow)
self.assertEqual(check_workflow_readiness(root), [])
(workflows / "mint.yml").write_text(valid_workflow.replace("/health/ready", "/health"))
self.assertEqual(len(check_workflow_readiness(root)), 1)
(workflows / "mint.yml").write_text(
valid_workflow.replace(
"if curl -sf http://127.0.0.1:9000/health/ready; then",
"if curl -sf http://127.0.0.1:9000/health/ready || true; then",
)
)
self.assertEqual(len(check_workflow_readiness(root)), 1)
(workflows / "mint.yml").write_text(
valid_workflow.replace(
"if curl -sf http://127.0.0.1:9000/health/ready; then",
"if curl -sf http://127.0.0.1:9000/health/ready || :; then",
)
)
self.assertEqual(len(check_workflow_readiness(root)), 1)
(workflows / "mint.yml").write_text(
valid_workflow.replace(
"if curl -sf http://127.0.0.1:9000/health/ready; then\n exit 0\n fi",
"curl -sf http://127.0.0.1:9000/health/ready || true\n exit 0",
)
)
self.assertEqual(len(check_workflow_readiness(root)), 1)
def test_profile_listing_enforces_selection(self) -> None: def test_profile_listing_enforces_selection(self) -> None:
with tempfile.TemporaryDirectory() as tmp: with tempfile.TemporaryDirectory() as tmp:
root = Path(tmp) root = Path(tmp)
+9 -16
View File
@@ -584,19 +584,17 @@ check_server_ready_from_log() {
# Test S3 API readiness # Test S3 API readiness
test_s3_api_ready() { test_s3_api_ready() {
# Step 1: Check if server is responding using /health endpoint # Step 1: Require the dependency-aware readiness endpoint.
# /health is a probe path that bypasses readiness gate, so it can be used READY_CODE=$(curl -s -o /dev/null -w "%{http_code}" \
# to check if the server is up and running, even if readiness gate is not ready yet
HEALTH_CODE=$(curl -s -o /dev/null -w "%{http_code}" \
-X GET \ -X GET \
"http://${S3_HOST}:${S3_PORT}/health" \ "http://${S3_HOST}:${S3_PORT}/health/ready" \
--max-time 5 2>/dev/null || echo "000") --max-time 5 2>/dev/null || echo "000")
if [ "${HEALTH_CODE}" = "000" ]; then if [ "${READY_CODE}" = "000" ]; then
# Connection failed - server might not be running or not listening yet # Connection failed - server might not be running or not listening yet
return 1 return 1
elif [ "${HEALTH_CODE}" != "200" ]; then elif [ "${READY_CODE}" != "200" ]; then
# Health endpoint returned non-200 status, server might have issues # The service is live but its storage/IAM dependencies are not ready.
return 1 return 1
fi fi
@@ -617,16 +615,11 @@ test_s3_api_ready() {
if echo "${RESPONSE}" | grep -q "503\|Service not ready"; then if echo "${RESPONSE}" | grep -q "503\|Service not ready"; then
return 1 # Not ready yet (readiness gate is blocking S3 API) return 1 # Not ready yet (readiness gate is blocking S3 API)
fi fi
# Other errors from awscurl - might be auth issues or other problems # Authentication, server, and transport failures are not readiness.
# But server is up, so we'll consider it ready (S3 API might have other issues) return 1
return 0
fi fi
# Step 3: Fallback - if /health returns 200, server is up and readiness gate is ready # /health/ready is authoritative when the optional signed probe is absent.
# Since /health is a probe path and returns 200, and we don't have awscurl to test S3 API,
# we can assume the server is ready. The readiness gate would have blocked /health if not ready.
# Note: Root path "/" with HEAD method returns 501 Not Implemented (S3 doesn't support HEAD on root),
# so we can't use it as a reliable test. Since /health already confirmed readiness, we return success.
return 0 return 0
} }