fix(compose): keep build-only services when Up starts a subset#3732
Open
davireis wants to merge 1 commit into
Open
fix(compose): keep build-only services when Up starts a subset#3732davireis wants to merge 1 commit into
davireis wants to merge 1 commit into
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
Summary by CodeRabbit
WalkthroughThe ChangesRunServices build context scoping fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
`Up(RunServices(...))` (any partial-service Up) rebuilt `project.Services` down
to just the selected services before calling the compose service. When a
selected service builds from a `service:` additional build context
(`build.additional_contexts.<name>: service:<dep>`), the referenced build-only
service was dropped too, so the build failed with:
service "X" declares unknown service "Y" as additional contexts
even though `docker compose up X` on the same project builds the context and
starts only X.
Use `project.WithSelectedServices(services, IgnoreDependencies)` instead: it
disables (not deletes) the non-selected services, keeping them in the project's
disabled set so compose can re-enable build dependencies for the build phase
while still only creating/starting the selected ones. `IgnoreDependencies`
preserves the existing behavior of starting only the explicitly requested
services. The returned project is reassigned to the up options so create and
start operate on the same scoped project.
Adds a regression test with a runtime service that builds from a `service:`
additional context: it fails on the previous code ("no such service") and
passes now.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
davireis
force-pushed
the
fix/compose-runservices-service-build-context
branch
from
ce84aa5 to
ba4213b
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes partial-service compose.Up(..., RunServices(...)) so that build-only services referenced via build.additional_contexts: service:<dep> remain resolvable during the build phase, matching docker compose up <service> behavior.
Changes:
- Replace manual
project.Servicesfiltering withproject.WithSelectedServices(..., types.IgnoreDependencies)and ensure the returned project is used consistently for create/start. - Add a regression test and new compose/Dockerfile testdata covering
service:additional build contexts in a partial-serviceUp. - Add new test compose project assets (compose YAML + two Dockerfiles) to exercise the build-context scenario.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/compose/compose_api.go | Switches partial-service scoping to WithSelectedServices and updates up options to use the new project. |
| modules/compose/compose_api_test.go | Adds regression test ensuring build-only service: build context works while only the selected runtime service is started. |
| modules/compose/testdata/docker-compose-service-build-context.yml | New compose fixture defining a runtime service that builds from a build-only service context. |
| modules/compose/testdata/service-build-context-app.Dockerfile | New app Dockerfile consuming the service: additional build context. |
| modules/compose/testdata/service-build-context-dep.Dockerfile | New build-only dependency Dockerfile producing an artifact consumed by the app build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -321,17 +320,16 @@ func (d *DockerCompose) Up(ctx context.Context, opts ...StackUpOption) (err erro | |||
| } | |||
|
|
|||
| if len(upOptions.Services) != len(d.project.Services) { | |||
Comment on lines
+3
to
+7
| build-dep: | ||
| build: | ||
| context: . | ||
| dockerfile: service-build-context-dep.Dockerfile | ||
| image: testcontainers/compose-service-build-dep:latest |
Comment on lines
+8
to
+14
| app: | ||
| build: | ||
| context: . | ||
| dockerfile: service-build-context-app.Dockerfile | ||
| additional_contexts: | ||
| build-dep: service:build-dep | ||
| image: testcontainers/compose-service-build-app:latest |
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.
Problem
Up(RunServices(...))(any partial-serviceUp) rebuiltproject.Servicesdown to just the selected services before calling the compose service. When a selected service builds from aservice:additional build context (build.additional_contexts.<name>: service:<dep>), the referenced build-only service was dropped too, so the build failed with:…even though
docker compose up Xon the same project builds the context and starts onlyX.Repro
A compose project where a runtime service builds from a
service:additional context:compose.Up(ctx, RunServices("app"))→compose up: no such service: build-dep.Fix
Use
project.WithSelectedServices(services, types.IgnoreDependencies)instead of rebuildingproject.Services. It disables (not deletes) the non-selected services, keeping them in the project's disabled set, so compose can re-enable build dependencies for the build phase (addBuildDependencies+WithServicesEnabled) while still only creating/starting the selected services.IgnoreDependenciespreserves the existing behavior of starting only the explicitly requested services (not their runtimedepends_on). The returned (new) project is reassigned to the up options so create and start operate on the same scoped project.Test
Adds
TestDockerComposeAPIWithRunServicesAndServiceBuildContextwith a runtime service that builds from aservice:additional context. It fails on the previous code (no such service) and passes with the fix. The existingTestDockerComposeAPIWithRunServicesstill passes, and the up-all path is unchanged.