[Spec] Reconcile hydrated state against reality on rad startup - #12871
Draft
nithyatsu wants to merge 18 commits into
Draft
[Spec] Reconcile hydrated state against reality on rad startup#12871nithyatsu wants to merge 18 commits into
rad startup#12871nithyatsu wants to merge 18 commits into
Conversation
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Contributor
There was a problem hiding this comment.
Pull request overview
Spec-only PR proposing a rad startup reconciliation pass that invokes a new application-scoped reconcile custom action (mirroring getGraph) to bring hydrated state back in sync with underlying reality, preventing rad app delete from looping indefinitely on 409 Conflict when hydration resurrects a stranded non-terminal provisioningState.
Changes:
- Add a feature spec defining the reconciliation goal, endpoint shape, orchestration model (corerp via UCP), and acceptance criteria.
- Add an implementation plan outlining phased delivery (app action stub → container reconcile → functional coverage) and key risks/constraints.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| specs/006-state-restoration/spec.md | New feature spec for app-scoped + per-resource reconcile actions invoked from a new rad startup stage. |
| specs/006-state-restoration/plan.md | New implementation plan detailing phases, testing approach, and rollout constraints for the reconciliation feature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
…Spec toolchain No schema change; picks up emitter drift accumulated in typespec-go since the last regen. Isolated from the reconcile action addition that follows so each commit is independently reviewable. Verified with 'go build ./...' and existing corerp application-controller tests. Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
…spec + regen)
Adds a synchronous reconcile action to Radius.Core/applications/{name} mirroring
the shape of getGraph:
POST .../providers/Radius.Core/applications/{app}/reconcile
Adds ReconcileRequest / ReconcileResponse / ReconcileResourceOutcome models to
carry the per-resource outcome report.
This commit is TypeSpec + emitter output only; the corerp handler and 'rad
startup' wiring land in follow-up commits (Phase 0 of specs/006-state-restoration).
No callers are registered yet, so the action is currently unreachable.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Wires the reconcile custom action registered in the previous commit into a
concrete handler and registers it alongside getGraph on the Radius.Core preview
applications resource. The handler validates the application exists (404 if
not) and returns an empty ReconcileResponse.
Phase 0 of specs/006-state-restoration: this end-to-end path lets a client
POST /planes/.../applications/{app}/reconcile without error, so 'rad startup'
can be wired up next before the orchestrator that walks children and the
dynamic-rp per-resource reality-check are added.
The sdk.Connection is threaded through the constructor now so future commits
that add the child walk (which fans out through UCP) do not need to change the
handler's signature or its registration in setup.go.
Tests cover the empty-report happy path, 404 when the application does not
exist, and error-propagation on database failures.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Adds a fifth stage to rad startup that runs after ScaleUp: lists every application in the workspace's plane and POSTs the Radius.Core/applications reconcile custom action on each. Aggregates per-application outcomes into a report that is logged to the workflow log. The stage is best-effort per the spec's acceptance criterion: * individual per-application reconcile failures are recorded in the report and do not halt the pass; * a failure to even begin the pass (unreachable workspace, LIST failure) is logged and rad startup still returns success. Since Phase 0's server-side handler returns an empty report, the log will show 'reconciled 0 resource(s)' per application until the child walk and the dynamic-rp per-resource handler land in Phase 1. Nothing else about the existing startup flow changes. Tests extend the fakeStateRestoreClient to record the reconcile call and add coverage for the happy path (order includes reconcile after scaleup), the best-effort contract (reconcile error does not fail startup), and workspace plumbing (the runner's active workspace is what the reconcile stage receives). Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Adds a POST /{resourceName}/reconcile route to the dynamic-rp resource-group
scope and a stub controller that resolves the target resource (404 if
missing) and returns an empty ReconcileResponse.
Purpose: give the corerp app-scoped orchestrator a stable endpoint to
dispatch to before the reality-check logic exists. Every dynamic resource
type served by dynamic-rp gets the route automatically, since the router
adds it once inside the resource-group / provider-namespace / resource-type
subrouter shared by all dynamic types. No per-type registration needed.
The reality-check that walks properties.status.outputResources, GETs each
Kubernetes object, and PATCHes provisioningState to match reality lands in
the next commit. The response shape (ReconcileResponse / ReconcileResourceOutcome)
is finalised here so the orchestrator's response-decoding path does not
need to change when the handler starts populating the array.
Tests cover the empty-report happy path and the 404 for a missing resource.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Replace the reconcile placeholder with the Phase 1 reality check. For a
non-terminal dynamic resource the handler walks properties.status.outputResources,
does a Kubernetes GET on each output via the runtime client the RP already
holds, and aggregates the outcomes to decide the new provisioningState:
* all outputs gone (404 in k8s) -> transition to Failed
* all outputs settled (GET succeeded) -> transition to Succeeded
* any output skipped (cloud output, unresolved API version, transient GET
failure) -> leave provisioningState unchanged, because we refuse to lie
about state we could not verify
The new state is persisted through the frontend controller's SaveResource
path (no direct database writes). Cloud outputs served by Terraform / cloud
providers are reported as 'skipped: cloud output not yet reality-checked'
so the caller sees exactly which resources were and were not verified.
Full cloud output reality checking is a follow-up.
The frontend service now fetches the Kubernetes runtime client and the
discovery client from the KubernetesClientProvider and threads them into
the reconcile controller. The runtime client rides in controller.Options
alongside the other controllers; the discovery client is passed directly
to NewReconcile because no other frontend controller needs it. The test
host was updated to plant an empty fake DiscoveryClient so plane bring-up
still succeeds without a real cluster.
Unit tests cover the four aggregation branches: 404 -> Failed, present ->
Succeeded, cloud output -> skipped + state unchanged, terminal state ->
empty report. A separate test still asserts the 404-on-missing-resource
behavior.
The corerp orchestrator on Radius.Core/applications still returns an empty
report; the child walk lands in the next commit.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Replace the Phase 0 stub on Radius.Core/applications/reconcile with the
Phase 1 orchestrator. When 'rad startup' invokes this action it now:
1. Looks up the application (404 if missing).
2. Enumerates the resource-type registry via UCP, filters out the built-in
namespaces that don't implement /reconcile (Applications.Core / Dapr /
Datastores / Messaging, Radius.Core, Microsoft.Resources), and lists
every remaining resource associated with the application. This mirrors
the walk getGraph already uses so the two custom actions stay
consistent.
3. Skips children whose provisioningState is already terminal.
4. Fans out reconcile POSTs concurrently through the shared UCP connection,
bounded to 8 in flight and capped at 15 s per child so one unresponsive
RP cannot hang the whole reconcile (spec §Risks).
5. Aggregates each RP's per-resource outcomes into one ReconcileResponse.
A dispatch failure (timeout, transport error, non-2xx) does not fail
the reconcile: it is recorded as a per-child skipped outcome with the
provisioningState left unchanged, so the caller always gets a report.
6. Returns 404 or 405 from a child RP as 'skipped: RP does not implement
reconcile', so the orchestrator stays forward-compatible with RPs
that haven't wired reconcile yet.
The child walk and per-child dispatch are exposed as injectable hooks on
the controller so the orchestration logic can be unit-tested without
standing up a live UCP. The public constructor keeps its existing
signature and installs the UCP-backed defaults.
Tests cover: 404 on missing application, database error on lookup, empty
child list, terminal children skipped, multi-child fan-out with mixed
outcomes, and per-child dispatch failure recorded as skipped without
failing the reconcile.
Phase 1 is now feature-complete. An httptest-backed integration test that
exercises the end-to-end path lands in the next commit.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12871 +/- ##
==========================================
+ Coverage 59.81% 59.89% +0.08%
==========================================
Files 776 778 +2
Lines 46042 46370 +328
==========================================
+ Hits 27541 27775 +234
- Misses 18501 18595 +94 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Adds an integration test that exercises the reconcile orchestrator against
a real HTTP server that impersonates UCP + downstream RPs. This covers the
parts pure unit tests cannot: URL construction for each child's /reconcile
endpoint, the resource-provider summary lookup that resolves each child's
API version, request headers, request body, and JSON response decoding
into the aggregated ReconcileResponse.
The child walk is still stubbed with a static list — standing up a fake
UCP resource-listing surface would balloon the test without adding
coverage of anything that isn't already unit-tested.
Two scenarios:
* end-to-end: two-container application, one child's RP reports Failed
('kubernetes object not found') and the other reports Succeeded. The
orchestrator must aggregate both outcomes verbatim and dispatch to
each child's specific /reconcile path.
* forward-compat: a child's RP returns 404 for /reconcile (the route is
not registered). The orchestrator must record a skipped outcome with
provisioningState unchanged, not fail the reconcile.
Phase 1 is now feature-complete and covered end to end. Phase 2 (a
functional test against a real k3d cluster in test/functional/) remains
out of scope for this PR per the plan.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Fixes the copilot-pull-request-reviewer feedback that pointed out drift
between the spec doc and the plan/implementation actually shipped:
* Purpose paragraph (spec.md): the reconciler does not remove state-store
entries when the underlying resource is gone. It PATCHes provisioningState
to Failed and keeps the row so the next normal 'rad app delete' cleans it
up through the standard state machine. Updated wording to match.
* Reality table (spec.md): the 'not found' row now describes the same
'PATCH Failed, keep row' action as the plan / implementation, not
'delete the state-store entry'.
* Terminal set (spec.md): 'terminal' is not just Succeeded or Failed --
v1.ProvisioningState.IsTerminal() also treats Canceled and the empty
string (synchronous resources that already settled) as terminal. Spec
now references the helper so it cannot drift again.
* Endpoint casing (spec.md): the example route now uses lowercase
'resourcegroups' to match the canonical casing in the getGraph route
comment and elsewhere in the codebase.
* Go version (plan.md): updated 1.26.5 to 1.27.0 to match go.mod.
The tables in both files are already well-formed markdown -- the reviewer
flagged them as having a leading '||' but they use standard '|' cells and
render correctly on GitHub. No table syntax change needed.
No implementation change: plan and code already match the corrected spec.
Signed-off-by: Nithya Subramanian <nithyasu@microsoft.com>
Contributor
Author
|
Addressed the 6 review threads in a6407e3d3:
No implementation change: the plan and the shipped code already matched the corrected spec. |
willtsai
reviewed
Aug 31, 2026
| 3. Reads the resource's `properties.status.outputResources` — the concrete backing objects the recipe engine recorded when the resource was deployed. | ||
| 4. For each output resource, queries its underlying provider: | ||
| - Kubernetes objects → GET via the target-cluster Kubernetes client the RP already holds. | ||
| - Terraform-backed cloud outputs (Azure/AWS resource IDs) → **out of scope for the prototype**; record `skipped: cloud output not yet reality-checked` in the per-output report. Follow-up work adds the cloud-SDK branches inside the same handler. |
Contributor
There was a problem hiding this comment.
Does this mean the initial phase of this reconciler will not be able to reconcile against cloud provider resources other than Kubernetes? e.g. if a mismatch exists for an Azure Postgres database, it will not be caught?
Contributor
There was a problem hiding this comment.
Also, the description should not be scoped down to "Terraform-backed" only as I believe this would also apply to Bicep-backed resources.
Contributor
Author
There was a problem hiding this comment.
I will work on adding that support too to the PR. But for a review of approach, this scope sounded OK.
willtsai
reviewed
Aug 31, 2026
| This feature adds a reconciliation pass triggered by `rad startup` and executed against the running control plane: for every application in the plane, an application-scoped `reconcile` action asks each resource's owning resource provider to check its actual current state and rewrite the state store to match reality — including marking entries as `Failed` when the underlying resource does not exist, so the next normal `rad app delete` cleans them up through the standard state machine. | ||
|
|
||
| The scope is deliberately narrow: reconcile hydrated state so operations that follow see reality. | ||
|
|
Contributor
There was a problem hiding this comment.
would be good to define what is a terminal vs. non-terminal state
kachawla
reviewed
Aug 31, 2026
|
|
||
| ## Non-goals | ||
|
|
||
| - **A `rad app delete --force` flag was considered and explicitly rejected.** A force option that bypasses state can convert an in-progress happy-path delete into a broken one by overwriting the state store while the first delete is still driving to a terminal state. Fixing hydration is the right approach. |
Member
There was a problem hiding this comment.
rad app delete --force already exists. Can you share details on why passing --force from the delete workflow does not already fix this, before we add a new API?
|
|
||
| That is not sufficient when the previous run was interrupted while a resource was mid-operation. The archive can preserve a resource in a non-terminal state — for example `provisioningState: "Updating"` — that never actually completed. On the next run, the control plane accepts that state as authoritative, so every subsequent operation against the resource is blocked with `409 Conflict / target resource is in progress`. The delete workflow loops on that 409 forever and the application becomes undeletable through Radius. | ||
|
|
||
| This feature adds a reconciliation pass triggered by `rad startup` and executed against the running control plane: for every application in the plane, an application-scoped `reconcile` action asks each resource's owning resource provider to check its actual current state and rewrite the state store to match reality — including marking entries as `Failed` when the underlying resource does not exist, so the next normal `rad app delete` cleans them up through the standard state machine. |
Member
There was a problem hiding this comment.
A customer restoring a Radius database backup into a rebuilt cluster gets the rows without the queue, which is the same stuck state. How do we address this for them?
Also, do we delete database on rad uninstall? If not uninstalling and reinstalling Radius is another scenaior where this could happen.
| reports := make([]ApplicationReconcileReport, 0, len(names)) | ||
| for _, name := range names { | ||
| report := ApplicationReconcileReport{Name: name} | ||
| resp, err := applications.Reconcile(ctx, workspace.Scope, name, corerpv20250801preview.ReconcileRequest{}, nil) |
Member
There was a problem hiding this comment.
issue(operations,blocking): Bound each application reconcile request
This call inherits the command context without a deadline. A stalled Core RP enumeration or application request can prevent rad startup from returning, contrary to the bounded per-application timeout required by spec.md. Apply a per-application timeout that covers the complete action.
| if err != nil { | ||
| report.Err = err | ||
| } else { | ||
| report.ResourceCount = len(resp.Resources) |
Member
There was a problem hiding this comment.
issue(operations,blocking): Preserve skipped and unchanged outcomes in startup output
The CLI keeps only len(resp.Resources) and later reports every entry as reconciled. Cloud skips, query failures, and unchanged records therefore look successful. Preserve and log from, to, and reason, or summarize changed, unchanged, skipped, and failed outcomes separately.
| ``` | ||
|
|
||
| in parallel (bounded fan-out), through the UCP-fronted connection the handler already has. | ||
| 5. After every child response returns, reconciles the application record itself: if all children are now terminal, transition the application accordingly; if any child remains non-terminal, leave the application in its hydrated state. |
Member
There was a problem hiding this comment.
todo(non-blocking): Remove or implement application-record reconciliation
The spec and plan say Core RP updates the application after reconciling its children, but Reconcilev20250801preview.Run only returns the aggregated child outcomes. Align this step with the implemented scope, or add the stated application transition.
| } | ||
| scaledBackUp = true | ||
|
|
||
| // ReconcileHydratedState is best-effort: it POSTs the reconcile custom action per application |
Member
There was a problem hiding this comment.
todo(non-blocking): Update the living architecture documentation
This post-restore pass changes the state-archive lifecycle and adds a generic state-mutating Dynamic RP action. Update docs/architecture/state-archive.md with the reconciliation stage and docs/architecture/dynamic-rp.md with the /reconcile path, Kubernetes clients, and failure behavior.
| 3. Reads the resource's `properties.status.outputResources` — the concrete backing objects the recipe engine recorded when the resource was deployed. | ||
| 4. For each output resource, queries its underlying provider: | ||
| - Kubernetes objects → GET via the target-cluster Kubernetes client the RP already holds. | ||
| - Terraform-backed cloud outputs (Azure/AWS resource IDs) → **out of scope for the prototype**; record `skipped: cloud output not yet reality-checked` in the per-output report. Follow-up work adds the cloud-SDK branches inside the same handler. |
Contributor
There was a problem hiding this comment.
it says cloud output not reality-checked here, are non-tf cloud deployments in scope?
Radius functional test overviewClick here to see the test run details
Test Status⌛ Building Radius and pushing container images for functional tests... |
nithyatsu
marked this pull request as draft
September 1, 2026 17:04
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.
Reconcile hydrated state against reality on
rad startupImplements the design in
specs/006-state-restoration/. Fixes the delete-workflow 409 loop whererad app deleteretries forever because a state archive preserved a resource in a non-terminalprovisioningStatethat no longer exists (or has settled) underneath.What lands
A new sync custom action
reconcileonRadius.Core/applications/{name}, plus a/reconcileroute on every dynamic type served by dynamic-rp, plus a new best-effortReconcileHydratedStatestage onrad startupthat invokes the app-scoped action for every hydrated application.Flow
rad startupfinishes hydrating the state archive, then POSTs/planes/radius/local/resourcegroups/{rg}/providers/Radius.Core/applications/{app}/reconcileper application.pkg/corerp/frontend/controller/applications/v20250801preview/reconcile.go) walks the application's children (same walkgetGraphuses, restricted to dynamic-rp namespaces), skips terminal children, and fans out/reconcilePOSTs concurrently through the UCP connection with per-child timeout + bounded concurrency.pkg/dynamicrp/frontend/reconcile.go) walksproperties.status.outputResourcesfor each non-terminal child, does a Kubernetes GET on each output, aggregates: all gone → PATCHFailed; all settled → PATCHSucceeded; any skipped (cloud output, unresolved version, transient failure) → leaveprovisioningStateunchanged.ReconcileResponse. Individual failures never fail the whole reconcile; the whole reconcile never failsrad startup.Files
typespec/Radius.Core/applications.tsp+ regeneratedpkg/corerp/api/v20250801preview/zz_generated_*.gopkg/corerp/frontend/controller/applications/v20250801preview/reconcile.go+ tests +pkg/corerp/setup/setup.goregistrationpkg/dynamicrp/frontend/reconcile.go+routes.go+service.goK8s client wiring + tests +pkg/dynamicrp/testhost/host.gofake discoverypkg/cli/cmd/startup/stateclient.go+startup.go+ testsspecs/006-state-restoration/spec.md+plan.mdTesting
rad startupunit tests cover the new stage's ordering + best-effort semantics.httptest-backed integration test on the corerp orchestrator exercises the real HTTP dispatch path against a fake UCP + downstream RPs — including URL construction, API-version lookup, request body, response decoding, and the forward-compat behavior when a child RP does not implement/reconcileyet.Design decisions (unchanged from the spec-only PR)
getGraph. corerp orchestrates; UCP is the proxy layer.--forceflag onrad app delete— rejected because it force-updates the database and can break the state machine.Failed, keep the row — not delete — so the next normalrad app deletecleans it up through the standard state machine.rad startupstill returns success.skippedin the response.Related
rad app deletebehavior tracked separately in #12870.