feat(docker-compose): Add Docker Compose deployment. - #427
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdded a Docker Compose deployment for Spider with MariaDB, storage, scheduler, and worker services. Added service configurations, environment defaults, lifecycle tasks, persistent storage, health checks, and YAML lint coverage. ChangesSpider Compose deployment
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant DockerCompose
participant MariaDB
participant Storage
participant Scheduler
participant Worker
DockerCompose->>MariaDB: Start database and wait for health check
DockerCompose->>Storage: Start storage with database dependency
Storage->>MariaDB: Connect to database
DockerCompose->>Scheduler: Start scheduler with storage dependency
Scheduler->>Storage: Connect to storage endpoint
DockerCompose->>Worker: Start workers with scheduler and storage dependencies
Worker->>Scheduler: Connect to scheduler endpoint
Worker->>Storage: Connect to storage endpoint
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@taskfiles/docker.yaml`:
- Around line 38-40: Add a separate compose:clean task alongside compose:down,
preserving the same G_SPIDER_COMPOSE_DIR working directory and compose.yaml file
while invoking docker compose down with --volumes to remove persistent database
volumes. Leave compose:down unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c7c05e6-082b-4bc6-a792-c698c6d2142a
📒 Files selected for processing (6)
taskfiles/docker.yamltaskfiles/lint.yamltools/deployment/spider-compose/compose.yamltools/deployment/spider-compose/config/scheduler.yamltools/deployment/spider-compose/config/storage.yamltools/deployment/spider-compose/config/worker.yaml
sitaowang1998
changed the title
|
Nice work — I ran this end-to-end and it genuinely works, including executing real jobs. Details on the validation below. A couple of small things to fix, one question, and one follow-up suggestion. Blocking
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
taskfiles/docker.yaml (1)
26-28: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake
compose:validaterunnable from a clean checkout.
.gitignoreexcludestools/deployment/spider-compose/.env, butcompose:validatestill requires the database credential variables used by required${VAR:?…}interpolations, sodocker compose config --quietfails unless the user copies the example env file or exports those variables. Direct the command at a documented syntax-only default env file instead.Suggested change
- cmd: "docker compose --file compose.yaml config --quiet" + cmd: "docker compose --env-file .env.example --file compose.yaml config --quiet"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@taskfiles/docker.yaml` around lines 26 - 28, Update the compose:validate task to pass Docker Compose the documented syntax-only default environment file when validating compose.yaml. Ensure the command uses that tracked default env file so required variable interpolation succeeds from a clean checkout without relying on ignored .env files or exported credentials.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@taskfiles/docker.yaml`:
- Line 32: Restore the bounded startup timeout for tcompose:up: add
SPIDER_COMPOSE_WAIT_TIMEOUT=120 to tools/deployment/spider-compose/.env.example,
load that variable in the Taskfile, and update the docker compose command to
pass --wait-timeout using the variable with 120 as its fallback.
In `@tools/deployment/spider-compose/.env.example`:
- Line 10: Update the SPIDER_PULL_POLICY example in the environment template to
a local-image-friendly default that allows locally tagged images to be used
without pulling from the registry, or document the required explicit override
for local builds. Keep it consistent with the image references produced by the
deployment taskfiles and consumed by the Compose configuration.
---
Outside diff comments:
In `@taskfiles/docker.yaml`:
- Around line 26-28: Update the compose:validate task to pass Docker Compose the
documented syntax-only default environment file when validating compose.yaml.
Ensure the command uses that tracked default env file so required variable
interpolation succeeds from a clean checkout without relying on ignored .env
files or exported credentials.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f68a5675-8b3a-4623-8bf7-fc6dc41080cc
📒 Files selected for processing (4)
.gitignoretaskfiles/docker.yamltools/deployment/spider-compose/.env.exampletools/deployment/spider-compose/compose.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/deployment/spider-compose/compose.yaml
|
|
||
| compose:up: | ||
| dir: "{{.G_SPIDER_COMPOSE_DIR}}" | ||
| cmd: "docker compose --file compose.yaml up --detach --wait" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate docker taskfiles =="
fd -a 'docker\.ya?ml$' . || true
echo "== taskfiles/docker.yaml excerpt =="
if [ -f taskfiles/docker.yaml ]; then
nl -ba taskfiles/docker.yaml | sed -n '1,120p'
fi
echo "== env examples and wait references =="
fd -a '(\.env\.example|example.*\.env|.*env.*)' . | sed -n '1,80p'
rg -n "SPIDER_COMPOSE_WAIT_TIMEOUT|wait-timeout|docker compose|120|wait" -S . --glob '!node_modules' --glob '!dist' --glob '!build' 2>/dev/null | sed -n '1,200p'Repository: y-scope/spider
Length of output: 299
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== taskfiles/docker.yaml excerpt =="
if [ -f taskfiles/docker.yaml ]; then
awk '{printf "%6d\t%s\n", NR, $0}' taskfiles/docker.yaml | sed -n '1,120p'
fi
echo "== tracked relevant files (sample) =="
git ls-files | grep -E '(^|/)(\.env\.example|example.*\.env|.*\.env|taskfiles/docker\.yaml|compose\.ya?ml|README\.md)$' | sed -n '1,120p'
echo "== wait and variable references =="
grep -RIn -E "SPIDER_COMPOSE_WAIT_TIMEOUT|wait-timeout|docker compose|120|wait" --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build . 2>/dev/null | sed -n '1,240p' || trueRepository: y-scope/spider
Length of output: 29282
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Taskfile references =="
for f in $(git ls-files 'Taskfile*' 'taskfiles/*' 'pyproject.toml'); do
echo "--- $f"
grep -nIIn -E "dotenv|envfile|env_file|SPIDER_COMPOSE_WAIT_TIMEOUT|compose:up|docker" "$f" 2>/dev/null || true
done
echo "== spider-compose env and compose metadata =="
if [ -f tools/deployment/spider-compose/.env.example ]; then
awk '{printf "%6d\t%s\n", NR, $0}' tools/deployment/spider-compose/.env.example
fi
if [ -f tools/deployment/spider-compose/compose.yaml ]; then
sed -n '1,80p' tools/deployment/spider-compose/compose.yaml
fi
echo "== repo-wide 120/wait references excluding generated deps =="
grep -RIn -E "SPIDER_COMPOSE_WAIT_TIMEOUT|wait-timeout|compose:up|120.*second|second.*120|startup timeout|bounded|timeout" --exclude-dir=node_modules --exclude-dir=dist --exclude-dir=build --exclude-dir=.git . 2>/dev/null | sed -n '1,160p' || trueRepository: y-scope/spider
Length of output: 23494
🌐 Web query:
Docker Compose up --wait-timeout documentation
💡 Result:
The --wait-timeout option in Docker Compose is used with the --wait flag to specify a maximum duration, in seconds, to wait for the services in a project to reach a running or healthy state [1][2][3]. Key details regarding this option: Purpose: It prevents the docker compose up command from waiting indefinitely when services fail to start or reach a healthy state within a reasonable timeframe [4][5]. Usage: It must be used in conjunction with the --wait flag, which signifies that the command should wait for services to be running or healthy [1][2]. Because the --wait flag implies detached mode, --wait-timeout is typically used when running services in the background [1][3]. Syntax: You can specify the timeout in seconds as follows: docker compose up -d --wait --wait-timeout Note: This option is distinct from the --timeout (or -t) flag, which is used to set the timeout for container shutdown [1][6]. While the --wait-timeout feature was introduced to address issues where processes would hang, users should be aware of ongoing maintenance and potential bugs in specific edge cases, such as complex dependency conditions (e.g., service_completed_successfully) [7][4].
Citations:
- 1: https://docs.docker.com/reference/cli/docker/compose/up/
- 2: https://manpages.debian.org/unstable/docker-compose/docker-compose-up.1.en.html
- 3: https://manpages.debian.org/testing/docker-compose/docker-compose-up.1.en.html
- 4: Add timeout to --wait option docker/compose#10269
- 5: introduce --wait-timeout on
updocker/compose#10276 - 6: https://github.com/docker/compose/blob/28120148f4e1380096407626f846eda8f6970f36/docs/reference/up.md
- 7: [BUG] --wait-timeout option not working with 'service_completed_successfully' dependency docker/compose#12134
Restore the bounded startup timeout.
tcompose:up uses docker compose up --detach --wait without --wait-timeout, so startup waits are no longer bounded by the documented 120-second limit. Add SPIDER_COMPOSE_WAIT_TIMEOUT=120 to tools/deployment/spider-compose/.env.example, load the variable in the Taskfile, and pass --wait-timeout ${.SPIDER_COMPOSE_WAIT_TIMEOUT:-120} to the compose command.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@taskfiles/docker.yaml` at line 32, Restore the bounded startup timeout for
tcompose:up: add SPIDER_COMPOSE_WAIT_TIMEOUT=120 to
tools/deployment/spider-compose/.env.example, load that variable in the
Taskfile, and update the docker compose command to pass --wait-timeout using the
variable with 120 as its fallback.
|
|
||
| # Optional deployment settings and their defaults. | ||
| SPIDER_IMAGE_TAG=main | ||
| SPIDER_PULL_POLICY=always |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)(\.env\.example|docker\.yaml|compose\.ya?ml)$' || true
echo
echo "== spider compose env example =="
if [ -f tools/deployment/spider-compose/.env.example ]; then
cat -n tools/deployment/spider-compose/.env.example
fi
echo
echo "== taskfiles/docker.yaml relevant references =="
if [ -f taskfiles/docker.yaml ]; then
rg -n "SPIDER_|spider|compose|pull|build|task" taskfiles/docker.yaml -C 3
fi
echo
echo "== compose files referencing SPIDER_PULL_POLICY or spider pull_policy =="
for f in $(git ls-files | rg '(^|/)(docker|docker-compose|compose)\.ya?ml$' -i); do
echo "-- $f --"
rg -n "SPIDER_PULL_POLICY|pull_policy|services:" "$f" -C 3 || true
doneRepository: y-scope/spider
Length of output: 2845
🌐 Web query:
Docker Compose pull_policy missing cached image when available
💡 Result:
If Docker Compose is failing to use a locally cached image when you expect it to, it is typically due to a mismatch between the expected image tag/digest or the configuration of the pull_policy. The pull_policy attribute controls how Docker Compose handles image retrieval [1][2]. When you believe an image should be found locally but it is not, check the following: 1. Verify Image Existence and Tags: Ensure the image name and tag (or digest) exactly match what is defined in your docker-compose.yml file. You can verify this by running the command docker images [1]. If the tag in your local cache does not match the tag defined in the compose file, Compose will treat it as missing [1][3]. 2. Check the pull_policy Setting: - missing (or if_not_present): This is the default policy if no build section is defined [2]. It uses the local image if it exists; otherwise, it attempts to pull from the registry [1][4]. - never: This tells Compose to use the local image only and never attempt to pull from a registry [3][4]. If the image is not in your local cache, the service will fail to start [5][3]. - build: If a build section is present, Compose may prioritize building the image from source over using an existing local image [2][6]. 3. Interaction with the build Directive: If your service includes a build section, Docker Compose's behavior changes [6]. Even if an image exists locally, Compose might trigger a rebuild or attempt to pull if not configured correctly [6][7]. If you want to force the use of a local image while a build section is present, setting pull_policy: never can prevent accidental pulls, though you may still need to manage the build behavior explicitly [2][7]. Troubleshooting Tips: - Verify with docker images: Confirm that the exact image name and tag reside in the Docker daemon's image store [1][3]. - Overriding policies: You can force a specific behavior during execution by using the --pull flag with the docker compose up command (e.g., --pull always or --pull never) to override the file configuration for a single run [2]. - Clean up: If you suspect issues with local cache integrity, ensure that there are no dangling images or conflicting tags that might be confusing the resolution process [3].
Citations:
- 1: https://www.baeldung.com/ops/docker-compose-local-image
- 2: https://lours.me/posts/compose-tip-067-pull-policy/
- 3: https://adhdecode.com/debugging/docker-compose/pull-policy-never-image-missing/
- 4: https://hostim.dev/learn/docker/local-images-with-compose/
- 5: https://latchkey.dev/learn/docker/docker-pull-policy-never-image-not-present
- 6: https://docs.docker.com/reference/compose-file/build/
- 7: Adding a build context to support docker compose watch changes the behavior of other commands docker/compose#11198
🌐 Web query:
Docker Compose service pull_policy always local image pull policy always documentation
💡 Result:
In Docker Compose, the pull_policy attribute defines the behavior for fetching container images from a registry [1][2]. The pull_policy: always setting instructs Docker Compose to pull the image from the registry every time the service starts, regardless of whether the image already exists in the local platform cache [1][2]. This is useful for tags that are frequently updated (such as:latest or development branch tags) to ensure you are always using the most recent version available on the registry [2]. The following pull_policy options are available in the Compose Specification [1]: - always: Pulls the image from the registry every time [1][2]. - missing (or if_not_present): The default behavior. Compose pulls the image only if it is not already available in the local platform cache [1][2]. Note that the:latest tag is always checked/pulled even with this policy [1][2]. - never: Does not attempt to pull the image from a registry. It relies entirely on the locally cached image and will report a failure if the image is missing [1][2]. - build: Compose builds the image from source, even if it is already present [1][2]. - daily: Checks the registry if the last pull occurred more than 24 hours ago [1][2]. - weekly: Checks the registry if the last pull occurred more than 7 days ago [1][2]. - every_: Checks for updates if the last pull took place before the specified duration (e.g., every_12h, every_30m) [1][2]. You can also override the configuration file setting using command-line flags. For example, using docker compose up --pull always will force a pull for all services regardless of their individual pull_policy settings [2]. Likewise, the docker compose pull command respects the pull_policy defined in your file unless a different policy is specified via the --policy flag [3][2].
Citations:
- 1: https://compose-spec.github.io/compose-spec/05-services.html
- 2: https://lours.me/posts/compose-tip-067-pull-policy/
- 3: https://docs.docker.com/reference/cli/docker/compose/pull/
🌐 Web query:
site:docs.docker.com reference/compose-file/services pull_policy always missing never
💡 Result:
In Docker Compose, the pull_policy attribute is used within a service definition to control when and how the service's image is pulled from a registry [1][2]. While command-line flags like --pull (available in commands such as docker compose up and docker compose create) explicitly accept values such as "always", "missing", "never", and sometimes "build" [3][4], the pull_policy attribute within the compose.yaml file itself functions based on the Compose Specification's defined behavior for image resolution [5][6]. When pull_policy is configured in a service definition, it determines the image pulling strategy: - always: Compose always attempts to pull the image from the registry, even if it is already present locally [5]. - missing: Compose pulls the image only if it is not already available in the local image store [1][2]. - never: Compose does not attempt to pull the image from the registry and relies entirely on the local image. If pull_policy is omitted, the default behavior of Compose is to attempt to pull the image if it is missing, and in scenarios where a build section is also defined, Compose manages the precedence of pulling versus building from source according to the Compose Build Specification [5][1][2]. Note that the --pull command-line flag provides additional control during runtime execution, separate from the persistent configuration set in the Compose file [7][3][4].
Citations:
- 1: https://docs.docker.com/reference/compose-file/services/
- 2: https://docs.docker.com/reference/compose-file/services
- 3: https://docs.docker.com/reference/cli/docker/compose/up/
- 4: https://docs.docker.com/reference/cli/docker/compose/create/
- 5: https://docs.docker.com/reference/compose-file/build/
- 6: https://docs.docker.com/reference/compose-file/
- 7: https://docs.docker.com/reference/cli/docker/compose/pull/
🌐 Web query:
"SPIDER_WORKER_CONTAINER_IMAGE_REF" "tools/docker/build.sh" "build.sh"
💡 Result:
The term SPIDER_WORKER_CONTAINER_IMAGE_REF is not a standard tool or universal environment variable, but rather an application-specific configuration identifier. It is primarily associated with projects using containerized worker architectures, such as those found in custom CI/CD pipelines or distributed systems [1]. In the context of the requested path tools/docker/build.sh, this script is a common pattern used in various open-source projects (such as Apache TVM, Apache Karaf, and Microsoft Playwright) to automate the building and tagging of Docker images [2][3][4]. While build.sh scripts generally perform the same function—compiling source and wrapping it in a container—they are specific to the repository where they reside [2][3][4]. If you are encountering SPIDER_WORKER_CONTAINER_IMAGE_REF in a configuration file or environment, it is likely being used as a reference to define which Docker image tag or registry path the worker service should pull or utilize for a given task [1]. To effectively use a tools/docker/build.sh script, you should: 1. Navigate to the directory containing the script. 2. Check the script's usage by running./tools/docker/build.sh --help (or equivalent), as most of these scripts require arguments like --tag, --platform, or specific environment variables to function correctly [3][4]. 3. Ensure any required dependencies (e.g., node_modules, cargo builds, or specific build-time binaries) are installed or prepared as indicated by the script's internal logic [2][4]. If you are working with a specific project, consult that project's documentation or repository root README for how their build system handles image references [5][6].
Citations:
- 1: https://github.com/diggerhq/opencomputer/blob/main/proto/worker/worker.pb.go
- 2: https://github.com/apache/tvm/blob/main/docker/build.sh
- 3: https://github.com/apache/karaf/blob/main/assemblies/docker/build.sh
- 4: https://github.com/microsoft/playwright/blob/main/utils/docker/build.sh
- 5: https://github.com/spider-rs/spider/blob/d2caaa9cd7a89b03d0ec51235d6c8119548ae4ea/spider_worker/README.md
- 6: https://github.com/dlstreamer/pipeline-zoo
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files under tools =="
git ls-files tools | sed -n '1,200p'
echo
echo "== tools/docker/build.sh references if present =="
if [ -f tools/docker/build.sh ]; then
wc -l tools/docker/build.sh
rg -n "SPIDER_IMAGE_TAG|SPIDER_WORKER_CONTAINER_IMAGE_REF|IMAGE_TAG|tag|build|DOCKER|docker" tools/docker/build.sh -C 2 || true
fi
echo
echo "== compose image references =="
cat -n tools/deployment/spider-compose/compose.yamlRepository: y-scope/spider
Length of output: 7466
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== taskfiles/docker.yaml referenced build images and compose default =="
cat -n taskfiles/docker.yaml
echo
echo "== tools/docker/build.sh =="
cat -n tools/docker/build.sh
echo
echo "== tools/docker/Dockerfile stages =="
sed -n '1,220p' tools/docker/Dockerfile | rg -n "FROM|AS " -C 2 || sed -n '1,220p' tools/docker/DockerfileRepository: y-scope/spider
Length of output: 4176
Use a local-image-friendly default pull policy.
taskfiles/docker.yaml builds images with docker tag using spider-${service}:dev-..., while tools/deployment/spider-compose/compose.yaml expects ghcr.io/y-scope/spider/${SPIDER_IMAGE_TAG:-main} by default. With SPIDER_PULL_POLICY=always, Compose pulls that registry reference every start, so local dev builds are ignored unless SPIDER_WORKER_CONTAINER_IMAGE_REF is overridden.
If local builds are meant to work with this Compose setup, use an example value matching the local tag or document that local builds need an explicit override.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/deployment/spider-compose/.env.example` at line 10, Update the
SPIDER_PULL_POLICY example in the environment template to a local-image-friendly
default that allows locally tagged images to be used without pulling from the
registry, or document the required explicit override for local builds. Keep it
consistent with the image references produced by the deployment taskfiles and
consumed by the Compose configuration.
|
I have fixed:
To answer the question:
|
jackluo923
left a comment
jackluo923
left a comment
There was a problem hiding this comment.
Changes look good to me, thanks for the quick turnaround.
Nit: I'd rename SPIDER_WORKER_CONTAINER_IMAGE_REF to SPIDER_WORKER_IMAGE_REF — it's the only variable carrying a CONTAINER, and the rest already follow SPIDER_<COMPONENT>_<SETTING>. Worth doing before the CLP integration hardcodes the name on its side.
2 participants
Description
This PR adds Docker Compose deployment:
Checklist
breaking change.
Validation performed
Verified using Docker Compose v5.4.0.
The Compose deployment does not provide default database credentials. Before running any Docker Compose tasks, set them explicitly:
Worker image selection
Verified all supported worker image-selection paths.
With neither override set:
The worker resolves to:
With only a Spider image tag:
SPIDER_IMAGE_TAG="sha-test" \ docker compose \ --file tools/deployment/spider-compose/compose.yaml \ config --imagesThe worker resolves to:
With a complete worker image reference:
The complete reference takes precedence:
Runtime startup
With the required database variables still exported, started the cluster using:
The resulting deployment contained:
All seven containers had a restart count of zero.
Port exposure
Verified the container port bindings:
Therefore, storage is the only service published to the host.
Network isolation
Started a temporary container attached only to the consumer-facing spider_default network:
The check passed:
Also verified that a worker retained outbound connectivity:
Cleanup
Removed the deployment and persistent database data using:
After cleanup, no Compose resources with project name spider remained:
Summary by CodeRabbit
New Features
Chores