Conversation
6129bb0 to
53c93f1
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
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(), andReconciliationPlantypes to separate state inspection, diff computation, and execution - Replaces the
convergencestruct withexecutionStatefor 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.
428b020 to
f1743e1
Compare
There was a problem hiding this comment.
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.gosuite 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.
- 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>
re-implement compose logic as a reconciliation between observed and desired state, producing a plan with pending operations.
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