Skip to content

refactor: split create() into preparePlan + execute - #14289

Open
ndeloof wants to merge 1 commit into
docker:mainfrom
ndeloof:split-create-prepare-execute
Open

ndeloof wants to merge 1 commit into
docker:mainfrom
ndeloof:split-create-prepare-execute

Conversation

@ndeloof

@ndeloof ndeloof commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

What this PR does, in one sentence

create()'s planning half — resolving the project, ensuring images/models and external resources, observing the daemon, and reconciling a Plan — becomes its own function, preparePlan, so a future caller can get the plan and snapshot without also running it.

Context

create() (pkg/compose/create.go) currently does two very different things in one unbroken call chain: it figures out what needs to happen (resolve the project model, ensure images/models exist, check external networks/volumes, observe the daemon, reconcile all of it into a Plan), then immediately does it (executePlan). That's fine as long as every caller wants both halves back to back — which is true today, but won't stay true once up converges onto the plan engine (epic #14081): interactive up needs to prepare the plan once, run its Create phase, set up the attach/printer/monitor session, and only then run the Start phase under a different context — it needs the plan and the daemon snapshot in hand between those two steps, not buried inside a single function that also executes everything.

What the PR brings

preparePlan(ctx, project, options) now returns exactly what a caller needs to decide how to run a plan: the canonical project (after optional-dependency pruning and use_api_socket resolution), the observed daemon state, and the reconciled Plan itself — without creating, starting, or otherwise touching a single container, network, or volume. create() is now just preparePlan + the existing emitRunningEvents + the existing executePlan, in the same order as before.

This is a pure extraction: every step, every error short-circuit, and every side effect (the orphan-containers warning, the unmanaged-network/volume warnings) keeps its exact original position and order. No caller-visible behavior changes — verified against the existing unit suite and the create/up/down e2e scenarios (TestLocalComposeUp, TestCreateIdempotentDefaultPlatform, TestCreateIdempotentSharedImageMixedPlatforms, TestCreateRefusesJob, TestCreateLocalMultiPlatformImage, TestUpWithAllResources).

This closes the last item of epic #14081's Lot 1 — inert code, no consumer yet: nothing calls preparePlan directly outside create() until Lot 2 wires the interactive-up phase split in.

Part of #14081.

🤖 Generated with Claude Code

Pure extraction, no behavior change: create() resolved the project
model, ensured images/models and external networks/volumes, observed
the daemon, and reconciled a Plan, all inline before calling
executePlan. That decision-making half is now its own function,
preparePlan, returning the canonical project, the daemon snapshot, and
the Plan — nothing is created, started, or otherwise touched by it.

This is Lot 1's last item (epic docker#14081): interactive up's eventual
create/start phase boundary needs to prepare the plan once, run the
Create phase, set up attach/printer/monitor, then run the Start phase
— which needs the plan and snapshot available between those steps,
not buried inside create()'s single call chain. create() itself is
unchanged in behavior, just now a two-line call into preparePlan
followed by the existing emitRunningEvents + executePlan.

Verified with the existing unit suite plus the create/up/down e2e
scenarios (TestLocalComposeUp, TestCreateIdempotentDefaultPlatform,
TestCreateIdempotentSharedImageMixedPlatforms, TestCreateRefusesJob,
TestCreateLocalMultiPlatformImage, TestUpWithAllResources) against a
locally built binary.

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof
ndeloof requested review from a team as code owners October 6, 2026 12:18
@ndeloof
ndeloof requested review from docker-agent and glours October 6, 2026 12:18

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

@codecov

codecov Bot commented Oct 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/compose/create.go 66.66% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The extraction preserves the existing operation order and error handling without introducing behavioral changes.

Review effort: Balanced
Findings: None

What changed in this PR

Extracts create planning into preparePlan, enabling future lifecycle phase separation without changing current behavior.

Changes:

  • Adds preparePlan to resolve resources, observe daemon state, and reconcile a plan.
  • Refactors create to prepare, emit running events, then execute.
File Description
pkg/​compose/​create.go Separates plan preparation from execution.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The extraction preserves operation order, error propagation, and existing execution behavior.

Review effort: Balanced
Findings: None

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants