Skip to content

re-implement compose logic#13641

Open
ndeloof wants to merge 4 commits intodocker:mainfrom
ndeloof:reconciliation
Open

re-implement compose logic#13641
ndeloof wants to merge 4 commits intodocker:mainfrom
ndeloof:reconciliation

Conversation

@ndeloof
Copy link
Copy Markdown
Contributor

@ndeloof ndeloof commented Mar 17, 2026

re-implement compose logic as a reconciliation between observed and desired state, producing a plan with pending operations.

  • build observed state
  • compute reconciliation plan by comparing observed vs desired state
  • execute plan

This allow to decorelate reconciliation (aka "convergence") logic from docker API, and write simpler and efficient tests to cover various scenarios

This PR was created with AI assistance

What I did

told Claude about the architecture I had in mind, and expectations regarding tests structure

@ndeloof ndeloof force-pushed the reconciliation branch 2 times, most recently from 6129bb0 to 53c93f1 Compare March 17, 2026 08:51
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 17, 2026

Codecov Report

❌ Patch coverage is 88.63275% with 143 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pkg/compose/plan_executor.go 74.29% 78 Missing and 13 partials ⚠️
pkg/compose/reconcile.go 95.16% 20 Missing and 17 partials ⚠️
pkg/compose/create.go 82.19% 8 Missing and 5 partials ⚠️
pkg/compose/observed_state.go 96.82% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@ndeloof ndeloof marked this pull request as ready for review March 17, 2026 09:10
@ndeloof ndeloof requested a review from a team as a code owner March 17, 2026 09:10
@ndeloof ndeloof requested review from Copilot and glours March 17, 2026 09:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR re-implements the Docker Compose "up/create" convergence logic as a reconciliation between observed and desired state, producing an explicit plan with pending operations. The old convergence struct is removed in favor of a pure Reconcile function that computes a ReconciliationPlan, and a separate ExecutePlan method that executes it.

Changes:

  • Introduces ObservedState, Reconcile(), and ReconciliationPlan types to separate state inspection, diff computation, and execution
  • Replaces the convergence struct with executionState for resolving service references during plan execution
  • Adds comprehensive unit tests for the reconciliation logic, with cross-references to existing e2e tests

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/compose/reconcile.go New pure reconciliation logic computing operation plans from observed vs desired state
pkg/compose/plan_executor.go New DAG-based concurrent plan executor and display utilities
pkg/compose/observed_state.go New types and Docker API queries for capturing current project state
pkg/compose/create.go Rewired create() to use InspectState → Reconcile → ExecutePlan pipeline
pkg/compose/convergence.go Removed old convergence struct, mustRecreate, recreateContainer, etc.
pkg/compose/run.go Updated to use executionState instead of old convergence
pkg/compose/reconcile_test.go Comprehensive unit tests for reconciliation logic
pkg/compose/observed_state_test.go Tests for observed state types and plan utilities
pkg/e2e/*.go Added TODO comments cross-referencing new unit tests
pkg/compose/publish.go Minor fmt.Fprintf cleanup
pkg/compose/progress.go Fixed misleading comment
cmd/compose/compose.go Unrelated: use consts.ComposeProfiles for default profile env var
AI_POLICY.md New AI usage policy document

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@ndeloof ndeloof force-pushed the reconciliation branch 4 times, most recently from 428b020 to f1743e1 Compare March 23, 2026 13:40
@ndeloof ndeloof requested a review from Copilot March 23, 2026 15:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors Compose “up”/create behavior into a pure reconciliation phase (observed vs desired state) that produces an execution plan (DAG of operations), then executes that plan, with extensive new unit tests for reconcile decision logic.

Changes:

  • Add ObservedState + Reconcile(...) to compute a deterministic reconciliation plan (networks/volumes/containers/orphans/dependency edges).
  • Add a DAG-based ExecutePlan(...) executor to run operations concurrently while respecting dependencies.
  • Add large reconcile_test.go suite and annotate some e2e tests with TODOs pointing to the new unit coverage.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
pkg/compose/reconcile.go New reconciliation planner (ops, dependency edges, recreate/scale logic).
pkg/compose/plan_executor.go New plan executor + execution-state used for resolving service references during execution.
pkg/compose/observed_state.go New “observed state” snapshot builder from Docker engine (containers/networks/volumes/orphans).
pkg/compose/observed_state_test.go Unit tests for observed-state types and plan helpers (Roots, String, ContainerTouched).
pkg/compose/reconcile_test.go Large new unit test suite for reconcile scenarios (scale, recreate, networks/volumes/orphans, dependency edges).
pkg/compose/create.go Switch create flow to InspectState → Reconcile → ExecutePlan; add external network validation; emit events for untouched containers (when plan non-empty).
pkg/compose/convergence.go Remove old convergence/apply logic (scale/recreate/start logic) and related helpers.
pkg/compose/run.go Replace convergence-based service-reference resolution with executionState.
pkg/compose/publish.go Minor strings.Builder formatting change (fmt.Fprintf instead of WriteString(fmt.Sprintf(...))).
pkg/compose/progress.go Comment tweak.
cmd/compose/compose.go Default --profile values now come from env (COMPOSE_PROFILES) via compose-go consts.
pkg/e2e/volumes_test.go Add TODO linking e2e volume recreate coverage to new reconcile unit tests.
pkg/e2e/up_test.go Add TODO linking e2e scale/no-recreate coverage to new reconcile unit tests.
pkg/e2e/scale_test.go Add TODOs linking e2e scale behaviors to reconcile unit tests.
pkg/e2e/recreate_no_deps_test.go Add TODO linking force-recreate/no-deps coverage to reconcile unit tests.
pkg/e2e/orphans_test.go Add TODO linking orphan removal coverage to reconcile unit tests.
pkg/e2e/networks_test.go Add TODOs linking network recreate/change detection coverage to reconcile unit tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ndeloof ndeloof closed this Apr 13, 2026
@ndeloof ndeloof deleted the reconciliation branch April 13, 2026 06:49
@ndeloof ndeloof restored the reconciliation branch April 13, 2026 06:49
ndeloof added 3 commits April 13, 2026 08:58
- build observed state
- compute reconciliation plan by comparing observed vs desired state
- execute plan

this decorelates reconciliation (aka "convergence") logic from docker API,
and write simpler and efficient tests to cover various scenarios

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof reopened this Apr 13, 2026
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
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.

2 participants