Skip to content

feat(compose): validate hostRequirements in Docker Compose path#247

Merged
skevetter merged 1 commit into
mainfrom
1f90-61ff-ws-compose-host-req
May 6, 2026
Merged

feat(compose): validate hostRequirements in Docker Compose path#247
skevetter merged 1 commit into
mainfrom
1f90-61ff-ws-compose-host-req

Conversation

@skevetter

@skevetter skevetter commented May 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds ValidateHostRequirements() call to the Docker Compose container creation path (pkg/devcontainer/compose.go), matching the existing behavior in the single-container path (single.go:225-229).
  • Warnings are advisory only and never block container creation — they propagate through setupContainerParams.hostWarnings into the JSON result envelope.
  • Adds E2E test (up-docker-compose-host-requirements label) that verifies CPU warnings appear in the JSON envelope when using Docker Compose with hostRequirements.cpus set above host capacity.

Reference: https://containers.dev/implementors/reference/ (hostRequirements section)

Summary by CodeRabbit

  • New Features

    • Docker Compose setup now validates host requirements and surfaces warnings (including CPU requirements) during container initialization to inform users of system prerequisites.
  • Tests

    • Added end-to-end test validating host requirements warnings are properly displayed in Docker Compose workflows.

The Docker Compose container creation path never called
ValidateHostRequirements(), unlike the single-container path.
Add the advisory validation call before setupContainer so that
host requirement warnings propagate through the JSON envelope.
@netlify

netlify Bot commented May 6, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 560fd4c
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/69fb4141b0bc7a0008879888

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 40e4bed8-ad4a-4af5-92d3-10fd4e3a9d25

📥 Commits

Reviewing files that changed from the base of the PR and between c9bca5d and 560fd4c.

📒 Files selected for processing (4)
  • e2e/tests/up-docker-compose/host_requirements.go
  • e2e/tests/up-docker-compose/testdata/compose-host-requirements/.devcontainer.json
  • e2e/tests/up-docker-compose/testdata/compose-host-requirements/docker-compose.yaml
  • pkg/devcontainer/compose.go

📝 Walkthrough

Walkthrough

Integrates host requirement warnings validation into the Docker Compose setup flow. When running the up docker-compose command, host requirements from the dev container configuration are now validated, and any warnings are captured and passed to container setup before initialization. A new e2e test verifies this integration.

Changes

Host Requirements Warning Integration for Docker Compose

Layer / File(s) Summary
Test Configuration & Data
e2e/tests/up-docker-compose/testdata/compose-host-requirements/.devcontainer.json, e2e/tests/up-docker-compose/testdata/compose-host-requirements/docker-compose.yaml
New test workspace with a devcontainer specifying hostRequirements with cpus: 128 and a corresponding Docker Compose service running a Go image.
Core Implementation
pkg/devcontainer/compose.go
In runDockerCompose, host requirements validation is performed via config.ValidateHostRequirements before container setup, and the computed hostWarnings are passed to setupContainer through setupContainerParams.
E2E Test
e2e/tests/up-docker-compose/host_requirements.go
Build-tagged test (linux/darwin/unix) that exercises the docker-compose flow with host requirements, captures the JSON envelope output, and asserts the presence of a cpus warning with a successful outcome.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • devsy-org/devsy#173: Introduces the ValidateHostRequirements function that this PR integrates into the docker-compose setup flow.

Suggested labels

size/l

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding host requirements validation to the Docker Compose path, matching existing behavior in the single-container path.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the size/m label May 6, 2026
@skevetter
skevetter marked this pull request as ready for review May 6, 2026 13:34
@coderabbitai coderabbitai Bot added the size/l label May 6, 2026
@skevetter
skevetter merged commit 7cd689f into main May 6, 2026
54 checks passed
@skevetter
skevetter deleted the 1f90-61ff-ws-compose-host-req branch May 6, 2026 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant