feat: prerequisites check step as Step 1 in start#51
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces a dedicated prerequisites check step to validate that all required binaries and Docker images are available before starting the cluster. This improves the fail-fast behavior and provides clearer error messages to users when components are missing.
Changes:
- Adds
PrerequisitesCheckStepas the first step in the parallel execution startup sequence - Centralizes binary and image checks using new constants
REQUIRED_BINARIESandREQUIRED_DOCKER_IMAGES - Removes duplicate image and binary checking logic from the Lotus step
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/constants.rs |
Adds REQUIRED_BINARIES and REQUIRED_DOCKER_IMAGES constants to centralize the list of required components for cluster startup |
src/commands/status/build_status.rs |
Updates to use the shared REQUIRED_BINARIES constant instead of maintaining a separate list |
src/commands/start/prerequisites_check.rs |
New module implementing the prerequisites check step with binary and Docker image validation |
src/commands/start/mod.rs |
Adds PrerequisitesCheckStep as Epoch 1 in the parallel execution pipeline and updates epoch documentation |
src/commands/start/lotus/prerequisites.rs |
Removes the now-redundant check_image_and_binary() function |
src/commands/start/lotus/lotus_step.rs |
Removes the call to the deleted check_image_and_binary() function |
| if !missing_binaries.is_empty() { | ||
| let missing_list = missing_binaries.join("', '"); | ||
| return Err(format!( | ||
| "Missing required binaries: '{}'\n\nPlease run 'foc-devnet build ...' to build all binaries, \ |
There was a problem hiding this comment.
The error message says "Please run 'foc-devnet build ...' to build all binaries" but this is incorrect syntax. Based on the rest of the message, it should either say "foc-devnet build" (without "...") to build all binaries, or provide the correct command. The ellipsis "..." is confusing as it's not a valid shell command.
| "Missing required binaries: '{}'\n\nPlease run 'foc-devnet build ...' to build all binaries, \ | |
| "Missing required binaries: '{}'\n\nPlease run 'foc-devnet build' to build all binaries, \ |
There was a problem hiding this comment.
@redpanda-f why was this closed? An all would be great but I don't think we have that, and ... isn't a thing as far as I can tell. Or maybe this error msg could be rephrased? I read it as "run this to build all together, or run this to build them individually".
There was a problem hiding this comment.
Issue is I don't want this to be confused with "foc-devnet build" being a command itself.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
rvagg
left a comment
rvagg
left a comment
There was a problem hiding this comment.
approving but assuming that the confusing message will be fixed before merge
github-project-automation
bot
moved this from 📌 Triage
to ✔️ Approved by reviewer
in FOC
| pub const PORTAINER_DOCKER_IMAGE: &str = "foc-portainer"; | ||
|
|
||
| /// Required binaries for cluster startup | ||
| pub const REQUIRED_BINARIES: &[&str] = &[ |
There was a problem hiding this comment.
Should we also check for Docker as part of #29 ?
There was a problem hiding this comment.
Not necessary. Docker sock failure is pretty evident in later part of the program anyways.
| /// - Epoch 4: MockUSDFC Deploy + MultiCall3 Deploy + FOC Deploy (can be parallelized) | ||
| /// - Epoch 5: MockUSDFC Funding + Curio daemons (can be parallelized, needs FOC Deploy) | ||
| /// - Epoch 6: PDP SP Registration (needs Curio daemons started) | ||
| /// - Epoch 1: Prerequisites check (binaries & Docker images - must run first) |
There was a problem hiding this comment.
Should the README_ADVANCED.md also get updated?
There was a problem hiding this comment.
It is not necessary, this is an internal check/guardrail, that is optional. It is done for better UX.
Co-authored-by: Rod Vagg <rod@vagg.org>
3 participants
Created a distinct step
PrerequisitesCheckStepbeforestartactually does anything meaningful.