Repository navigation
fix(tier): prepare and verify MQTT dependencies before setup - #142
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Recovery can target the wrong Docker daemon and leave the broker running, while non-atomic probe writes and a mismatched MQTT probe topic make readiness checks unreliable.
3 open issues
What changed in this PR
Moves MQTT dependency preparation into the Tier suite so direct runs and CI check broker readiness before destructive node setup.
Changes:
- Adds broker reuse or managed Docker startup, publish/subscribe readiness checks, and node connectivity checks.
- Adds targeted cleanup, cancellation recovery, and retained startup diagnostics.
- Documents MQTT configuration and integrates 14 regression tests into the Tier contract runner.
| File | Description |
|---|---|
test_rustfs_tier.sh |
Runs the new MQTT regression tests. |
test_rustfs_tier_mqtt.py |
Tests MQTT readiness, ownership, cleanup, and diagnostics using command fixtures. |
rustfs-tier-test.sh |
Implements MQTT preflight, managed broker lifecycle, and failure diagnostics. |
README.md |
Documents dependencies, configuration, and diagnostic artifacts. |
.github/workflows/rustfs-tier-test.yml |
Delegates broker setup to the suite and adds receipt-based recovery. |
Review effort: Balanced
| TIER_SOURCE_ONLY=1 source ./rustfs-tier-test.sh | ||
| MQTT_CIDFILE="${TIER_ARTIFACTS_DIR}/mqtt-container-id" | ||
| RESULT_LOG_DIR="${TIER_ARTIFACTS_DIR}/logs" | ||
| MQTT_DOCKER=(sudo -n docker) |
Comment on lines
+1097
to
+1102
| local probe_topic="rustfs/tier-probe/${RUN_TOKEN}" deadline=$((SECONDS + 15)) | ||
| local diagnostic="${RESULT_LOG_DIR}/mqtt-${CURRENT_TOPOLOGY:-preflight}.log" | ||
| : > "${MQTT_EVENTS}" | ||
| : > "${diagnostic}" | ||
| mosquitto_sub -h "${HEAL_IP}" -p "${MQTT_PORT}" -t "${MQTT_TOPIC}" -t "${probe_topic}" -v \ | ||
| >> "${MQTT_EVENTS}" 2>> "${diagnostic}" & |
| import json, os, pathlib, sys | ||
| root = pathlib.Path(os.environ['MQTT_FIXTURE']) | ||
| args = sys.argv[1:] | ||
| (root / 'probe').write_text(json.dumps([args[args.index('-t') + 1], args[args.index('-m') + 1]])) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Direct Tier suite runs could enable notify without a running MQTT broker. The subscriber helper discarded stderr and reported success after a fixed sleep even when the subscriber had already exited, allowing setup to reach
module switches persisted but runtime convergence failed: local notifywithout proving its dependencies were ready.Move MQTT preparation into the suite so direct VM runs and CI share the same behavior. Reuse an existing broker, or create a uniquely named Docker broker only when the endpoint is local and has no listener. Require a publish/subscribe round trip and connectivity from the source and target nodes before destructive suite setup. External-only mode and a configurable MQTT port are documented.
Preserve subscriber, broker, and credential-filtered notification startup diagnostics. Cleanup targets only the owned subscriber and container; a persistent container receipt and the shared cleanup function allow CI to recover after cancellation or an earlier cleanup failure. Add 14 regression tests to the existing Tier contract runner.
Related to rustfs/backlog#3148. This addresses the missing MQTT prerequisite and misleading readiness result; the original VM runtime-convergence failure still needs a focused retest.
Validation
PYTHONDONTWRITEBYTECODE=1 bash test_rustfs_tier.sh, including all 14 new MQTT tests and the existing Tier contract tests.bash test_validation_tier.shand all 7test_rustfs_tier_report.pytests.git diff --check.actionlintreports the pre-existing unrecognizedsmoke-testingrunner label on both the base and modified workflow. No suppression or gate relaxation was added.