Skip to content

re-implement compose logic - #13641

Closed
ndeloof wants to merge 13 commits into
docker:mainfrom
ndeloof:reconciliation
Closed

re-implement compose logic#13641
ndeloof wants to merge 13 commits into
docker:mainfrom
ndeloof:reconciliation

Conversation

@ndeloof

Copy link
Copy Markdown
Contributor

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
ndeloofforce-pushed the reconciliation branch 2 times, most recently from 6129bb0 to 53c93f1CompareMarch 17, 2026 08:51
@codecov

codecovBot commented Mar 17, 2026

Copy link
Copy Markdown

@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 ownerMarch 17, 2026 09:10
@ndeloof
ndeloof requested review from Copilot and gloursMarch 17, 2026 09:10

CopilotAI 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.

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
FileDescription
pkg/compose/reconcile.goNew pure reconciliation logic computing operation plans from observed vs desired state
pkg/compose/plan_executor.goNew DAG-based concurrent plan executor and display utilities
pkg/compose/observed_state.goNew types and Docker API queries for capturing current project state
pkg/compose/create.goRewired create() to use InspectState → Reconcile → ExecutePlan pipeline
pkg/compose/convergence.goRemoved old convergence struct, mustRecreate, recreateContainer, etc.
pkg/compose/run.goUpdated to use executionState instead of old convergence
pkg/compose/reconcile_test.goComprehensive unit tests for reconciliation logic
pkg/compose/observed_state_test.goTests for observed state types and plan utilities
pkg/e2e/*.goAdded TODO comments cross-referencing new unit tests
pkg/compose/publish.goMinor fmt.Fprintf cleanup
pkg/compose/progress.goFixed misleading comment
cmd/compose/compose.goUnrelated: use consts.ComposeProfiles for default profile env var
AI_POLICY.mdNew 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.

Comment threadpkg/compose/reconcile.go Outdated
@ndeloof
ndeloofforce-pushed the reconciliation branch 4 times, most recently from 428b020 to f1743e1CompareMarch 23, 2026 13:40
@ndeloof
ndeloof requested a review from CopilotMarch 23, 2026 15:13

CopilotAI 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.

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
FileDescription
pkg/compose/reconcile.goNew reconciliation planner (ops, dependency edges, recreate/scale logic).
pkg/compose/plan_executor.goNew plan executor + execution-state used for resolving service references during execution.
pkg/compose/observed_state.goNew “observed state” snapshot builder from Docker engine (containers/networks/volumes/orphans).
pkg/compose/observed_state_test.goUnit tests for observed-state types and plan helpers (Roots, String, ContainerTouched).
pkg/compose/reconcile_test.goLarge new unit test suite for reconcile scenarios (scale, recreate, networks/volumes/orphans, dependency edges).
pkg/compose/create.goSwitch create flow to InspectState → Reconcile → ExecutePlan; add external network validation; emit events for untouched containers (when plan non-empty).
pkg/compose/convergence.goRemove old convergence/apply logic (scale/recreate/start logic) and related helpers.
pkg/compose/run.goReplace convergence-based service-reference resolution with executionState.
pkg/compose/publish.goMinor strings.Builder formatting change (fmt.Fprintf instead of WriteString(fmt.Sprintf(...))).
pkg/compose/progress.goComment tweak.
cmd/compose/compose.goDefault --profile values now come from env (COMPOSE_PROFILES) via compose-go consts.
pkg/e2e/volumes_test.goAdd TODO linking e2e volume recreate coverage to new reconcile unit tests.
pkg/e2e/up_test.goAdd TODO linking e2e scale/no-recreate coverage to new reconcile unit tests.
pkg/e2e/scale_test.goAdd TODOs linking e2e scale behaviors to reconcile unit tests.
pkg/e2e/recreate_no_deps_test.goAdd TODO linking force-recreate/no-deps coverage to reconcile unit tests.
pkg/e2e/orphans_test.goAdd TODO linking orphan removal coverage to reconcile unit tests.
pkg/e2e/networks_test.goAdd 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.

Comment threadpkg/compose/reconcile.go
Comment threadpkg/compose/plan_executor.go
Comment threadpkg/compose/create.go Outdated
Comment threadpkg/compose/create.go Outdated
Comment threadpkg/compose/reconcile.go Outdated
Comment threadpkg/compose/reconcile.go Outdated
Comment threadpkg/compose/reconcile.go
Comment threadpkg/compose/reconcile.go Outdated
@ndeloofndeloof 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
@ndeloofndeloof reopened this Apr 13, 2026
- 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>
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>
Simplification des créations réseau/volume
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>
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>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloofndeloof mentioned this pull request Apr 20, 2026
@gloursglours mentioned this pull request Jun 8, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ndeloof