Remove "update" mode, replace with flag to pull images, move docker prune function to Taskfile - #190
Conversation
WalkthroughReplaced the service-level Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant L as labctl (CLI)
participant PS as process_services
participant DC as docker_command
participant DP as docker_pull
participant D as Docker/Compose/Bake
U->>L: labctl config apply --pull-before-start
L->>PS: process_services(pull_before_start=true)
loop per service
PS->>DC: docker_command(..., pull_before_start=true, action="up")
DC->>DC: check compose file exists
alt pull_before_start
DC->>DP: docker_pull(stack_dir, service, compose_file, env)
alt build directive present & BAKE enabled
DP->>D: buildx bake ... --pull
else
DP->>D: docker compose pull
end
end
DC->>D: docker compose up -d
end
sequenceDiagram
autonumber
actor U as User
participant L as labctl (CLI)
participant DC as docker_command
participant DP as docker_pull
participant D as Docker/Compose
U->>L: labctl service pull <service>
L->>DC: docker_command(..., action="pull")
DC->>DP: docker_pull(...)
DP->>D: compose pull OR buildx bake --pull
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
scripts/task-mcp/server.py (1)
123-151: Expose --pull-before-start via the MCP tool (optional)Let callers pass the new flag through Task MCP, so they don’t need two calls (pull + up).
-@mcp.tool(name="control-container-service") -def control_container_service(operation: str, service_name: str) -> str: +@mcp.tool(name="control-container-service") +def control_container_service(operation: str, service_name: str, pull_before_start: bool = False) -> str: """ - Execute one operation ('pull', 'up', 'down', 'restart', 'recreate', 'config') on the specified service and return the output + Execute one operation ('pull', 'up', 'down', 'restart', 'recreate', 'config') on the specified service and return the output + Args: + pull_before_start: When true, pass '--pull-before-start' through to labctl. """ @@ cmd = [ sys.executable, os.path.join(repository_root_path, "scripts", "labctl.py"), "service", operation, service_name ] + if pull_before_start: + cmd.append("--pull-before-start")docker/guidelines.md (1)
136-138: Docs reflect docker:pull-all; consider listing apply-update, tooNice rename. Suggest adding the new combined task for discoverability.
task docker:apply # Deploy all containers task docker:update # Update and restart containers task docker:pull-all # Pull latest container images +task docker:apply-update # Pull-before-start update, prune old images, show restarts task docker:stop # Stop configured containersscripts/labctl.py (2)
93-103: Centralized pulling is good; coerce Path to str to match type hintsSubprocess args are typed as list[str]; pass str(compose_file) for consistency.
- docker(["compose", "-f", compose_file, *env_file_args, "build", "--pull"], env=env) + docker(["compose", "-f", str(compose_file), *env_file_args, "build", "--pull"], env=env) else: - docker(["compose", "-f", compose_file, *env_file_args, "pull"]) + docker(["compose", "-f", str(compose_file), *env_file_args, "pull"])
132-139: Optionally honor --pull-before-start for recreate/restart as wellKeeps semantics uniform for “actions that (re)start containers.”
case "restart": - logger.info(f">>> Restarting {stack_dir}/{service_name}") + if pull_before_start: + docker_pull(stack_dir, service_name, compose_file, env_file_args) + logger.info(f">>> Restarting {stack_dir}/{service_name}") docker(["compose", "-f", compose_file, *env_file_args, "restart"]) case "recreate": - logger.info(f">>> Recreating {stack_dir}/{service_name}") + if pull_before_start: + docker_pull(stack_dir, service_name, compose_file, env_file_args) + logger.info(f">>> Recreating {stack_dir}/{service_name}") docker(["compose", "-f", compose_file, *env_file_args, "up", "--detach", "--force-recreate"])docker/Taskfile.docker.yaml (1)
19-23: pull-all task — good; clarify description (optional)Make it explicit it pulls for all services defined in config, regardless of their configured state.
- pull-all: - desc: Pull all (even not enabled) container images + pull-all: + desc: Pull images for all services defined in config (ignores per-service state)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
docker/Taskfile.docker.yaml(2 hunks)docker/guidelines.md(1 hunks)scripts/labctl.py(8 hunks)scripts/task-mcp/server.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
scripts/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python code (e.g., labctl.py) must pass Ruff linting
Files:
scripts/task-mcp/server.pyscripts/labctl.py
**/*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
YAML files must be linted and validated
Files:
docker/Taskfile.docker.yaml
🧠 Learnings (1)
📚 Learning: 2025-08-30T09:49:06.951Z
Learnt from: CR
PR: bubacoder/infra#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-30T09:49:06.951Z
Learning: Use the scripts/labctl.py tool to manage services (up, down, restart, recreate, update, pull, config)
Applied to files:
docker/Taskfile.docker.yamlscripts/labctl.py
🧬 Code graph analysis (2)
docker/Taskfile.docker.yaml (1)
docker/labctl.py (1)
cmd_config_apply(189-211)
scripts/labctl.py (2)
docker/labctl.py (5)
docker_command(92-135)main(234-261)cmd_config_apply(189-211)process_services(149-179)cmd_service(214-231)docker/backup.sh (1)
start_selected_containers(13-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (9)
scripts/task-mcp/server.py (1)
126-136: Removal of 'update' and tightened allowed ops — good alignmentDocstring, param help, and runtime validation now consistently allow only: pull, up, down, restart, recreate, config.
scripts/labctl.py (5)
121-127: Up honors --pull-before-start — niceThis matches the new flag’s intent.
156-156: process_services signature change — OKThreading pull_before_start through is clear.
186-186: Call site updated with pull_before_start — OK
212-212: config apply now forwards --pull-before-start — OK
232-232: service forwards --pull-before-start — OKdocker/Taskfile.docker.yaml (3)
17-17: Update uses pull-before-start — goodTask now maps to the new flag; no separate “update” mode needed.
24-30: apply-update flow aligns with new modelComposition looks correct (update → prune → show-restarts).
42-42: Prune description now states 21-day filter — goodMatches the actual command.
2bce318 to
8d7592d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/labctl.py (1)
1-269: Add YAML document start to docker/Taskfile.docker.yaml
The file is missing the required ‘---’ document start (yamllint warning at line 1); labctl.py passes all Ruff checks.
♻️ Duplicate comments (1)
scripts/labctl.py (1)
23-23: ALLOWED_STATES update looks correct; verify no lingering 'update' usages.
Good removal of 'update' from the CLI surface.Run to find stale references to the removed "update" operation in code/docs:
#!/bin/bash set -euo pipefail rg -nP -C2 '(labctl\.py\s+service\s+update\b)|(--mode=update\b)|\b(operation|state)\s*[:=]\s*["'\'']?update\b'
🧹 Nitpick comments (4)
scripts/labctl.py (2)
105-113: Signature change is fine; consider argument order for readability.
Placing action before the flag reads more naturally: docker_command(..., action, pull_before_start). Low-impact.Apply:
-def docker_command(host_config_dir: Path, stack_dir: Path, service_name: str, pull_before_start: bool, action: str) -> None: +def docker_command(host_config_dir: Path, stack_dir: Path, service_name: str, action: str, pull_before_start: bool) -> None:And update call sites accordingly (see lines 186 and 232 diffs below).
186-186: Call-site matches new signature; if you adopt action-first, update here.Apply if adopting prior suggestion:
- docker_command(host_config_dir, docker_stacks_dir / category, name, pull_before_start, state) + docker_command(host_config_dir, docker_stacks_dir / category, name, state, pull_before_start)docker/Taskfile.docker.yaml (2)
17-17: Update task now pulls-before-start: good alignment with CLI.Option: ensure services not in 'up' state still refresh images by forcing recreate:
- - scripts/labctl.py config apply --pull-before-start + - scripts/labctl.py config apply --pull-before-start --mode=recreate(Only if your intended update flow should recreate everything.)
24-29: apply-update description vs steps: ensure images are pulled for restart/recreate services.
Current steps rely on pull-before-start within 'update', which only affects 'up' and (after suggested change) 'recreate' flows. If you have services with 'state: restart', they won't pull. Either:
- Prepend pull-all, or
- Clarify the description, or
- Switch update to mode=recreate as suggested above.
Apply one:
apply-update: - desc: Pull, update and restart containers, then show restart events + desc: Update containers (pull before start), then prune and show restart events cmds: - - task: update + - task: update - task: prune - task: show-restartsOr prepend pull-all:
apply-update: cmds: + - task: pull-all - task: update - task: prune - task: show-restarts
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
CLAUDE.md(1 hunks)docker/Taskfile.docker.yaml(2 hunks)docker/guidelines.md(1 hunks)scripts/labctl.py(8 hunks)scripts/task-mcp/server.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/task-mcp/server.py
- docker/guidelines.md
🧰 Additional context used
📓 Path-based instructions (2)
scripts/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python code (e.g., labctl.py) must pass Ruff linting
Files:
scripts/labctl.py
**/*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
YAML files must be linted and validated
Files:
docker/Taskfile.docker.yaml
🧠 Learnings (1)
📚 Learning: 2025-08-30T09:49:06.951Z
Learnt from: CR
PR: bubacoder/infra#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-30T09:49:06.951Z
Learning: Use the scripts/labctl.py tool to manage services (up, down, restart, recreate, update, pull, config)
Applied to files:
scripts/labctl.pydocker/Taskfile.docker.yaml
🧬 Code graph analysis (2)
scripts/labctl.py (2)
docker/labctl.py (5)
docker_command(92-135)cmd_config_apply(189-211)main(234-261)process_services(149-179)cmd_service(214-231)docker/backup.sh (1)
start_selected_containers(13-15)
docker/Taskfile.docker.yaml (1)
docker/labctl.py (1)
cmd_config_apply(189-211)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (8)
scripts/labctl.py (6)
93-103: Centralized image-pull logic: LGTM.
Clear separation via docker_pull; matches prior behavior for build vs image cases.
211-213: Config apply wiring: LGTM.
Flags are passed correctly to process_services.
243-248: New flag on config apply: LGTM.
Good help text and default.
253-254: New flag on service: LGTM.
Consistent with config apply.
156-186: All docker_command invocations updated to the new signature. No outdated arity calls remain.
232-232: Service command wiring: confirmed single source of truth
Backup.sh callstask docker:apply, which invokesscripts/labctl.py config apply; there is nodocker/labctl.pyfile—no drift to consolidate.docker/Taskfile.docker.yaml (2)
19-23: New pull-all task: LGTM.
Accurately maps to mode=pull to cover disabled services too.
42-42: Prune description refinement: LGTM.
Matches 504h filter used below.
Move docker prune function to Taskfile
8d7592d to
e122532
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
scripts/labctl.py (2)
137-139: Extend pull-before-start to 'recreate' — good catchThis closes the gap where recreate could reuse cached images.
250-257: Address Ruff N802 violations in HTTP handler methods
Ruff flagged four N802 errors in scripts/proxy-request-log.py for thedo_GET,do_POST,do_PUTanddo_DELETEoverrides. Since these method names must remain uppercase to override SimpleHTTPRequestHandler, add# noqa: N802to each definition or configure aper-file-ignoresrule for this file in pyproject.toml to suppress N802. YAML linting on docker/Taskfile.docker.yaml passes.
🧹 Nitpick comments (1)
scripts/labctl.py (1)
93-103: docker_pull looks good; cast compose_file to str for broader Python compatibilitySome environments don’t accept PathLike in subprocess args. Cast once and reuse.
Apply:
def docker_pull(stack_dir: Path, service_name: str, compose_file: Path, env_file_args: list[str]) -> None: """Pull Docker images for a service.""" logger.info(f">>> Pulling {stack_dir}/{service_name}") + compose_file_arg = str(compose_file) if has_build_directive(compose_file): # Bake: https://docs.docker.com/guides/compose-bake/ env = os.environ.copy() env["COMPOSE_BAKE"] = "true" - docker(["compose", "-f", compose_file, *env_file_args, "build", "--pull"], env=env) + docker(["compose", "-f", compose_file_arg, *env_file_args, "build", "--pull"], env=env) else: - docker(["compose", "-f", compose_file, *env_file_args, "pull"]) + docker(["compose", "-f", compose_file_arg, *env_file_args, "pull"])
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
CLAUDE.md(1 hunks)docker/Taskfile.docker.yaml(2 hunks)docker/guidelines.md(1 hunks)scripts/labctl.py(9 hunks)scripts/task-mcp/server.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- docker/guidelines.md
- CLAUDE.md
- scripts/task-mcp/server.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{yml,yaml}
📄 CodeRabbit inference engine (CLAUDE.md)
YAML files must be linted and validated
Files:
docker/Taskfile.docker.yaml
scripts/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python code (e.g., labctl.py) must pass Ruff linting
Files:
scripts/labctl.py
🧠 Learnings (1)
📚 Learning: 2025-08-30T09:49:06.951Z
Learnt from: CR
PR: bubacoder/infra#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-30T09:49:06.951Z
Learning: Use the scripts/labctl.py tool to manage services (up, down, restart, recreate, update, pull, config)
Applied to files:
docker/Taskfile.docker.yamlscripts/labctl.py
🧬 Code graph analysis (2)
docker/Taskfile.docker.yaml (1)
docker/labctl.py (1)
cmd_config_apply(189-211)
scripts/labctl.py (1)
docker/labctl.py (4)
docker_command(92-135)cmd_config_apply(189-211)main(234-261)process_services(149-179)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (11)
scripts/labctl.py (7)
23-23: ALLOWED_STATES cleanup aligns with PR goalRemoving "update" and adding "pull" is consistent with the new flow. LGTM.
105-105: Signature threading of pull_before_startPropagating the flag here is correct. Consider documenting that it has effect only for up/recreate.
118-124: Pull-before-start for 'up' is correctly placedNice separation via docker_pull; no behavioral regressions spotted.
159-160: process_services flag plumbing looks correctInterface stays backward-compatible with default False.
189-189: Correct flag propagation to docker_commandMatches updated signature; no issues.
215-215: config apply: flag is threaded end-to-endLGTM.
235-235: service: flag is supported for one-off operationsLGTM.
docker/Taskfile.docker.yaml (4)
14-18: pull-all task matches new ‘pull’ modeAccurately pulls regardless of configured service state. LGTM.
19-23: update now maps to pull-before-startClearer semantics; aligns with removed 'update' operation.
24-29: apply-update composes update → prune → show-restartsSensible sequencing. LGTM.
42-42: Prune description clarifiedAccurate and user-friendly. LGTM.
Summary by CodeRabbit
New Features
Refactor
Documentation