Skip to content

feat(recovery): reconcile persistent deployment intent - #101

Open
qinyue0508-cmd wants to merge 2 commits into
developfrom
codex/deployment-recovery-clean
Open

feat(recovery): reconcile persistent deployment intent#101
qinyue0508-cmd wants to merge 2 commits into
developfrom
codex/deployment-recovery-clean

Conversation

@qinyue0508-cmd

Copy link
Copy Markdown

Summary

  • persist deployment desired state and recovery policy in SQLite
  • run a single AIMA reconciler for retry, backoff, circuit breaking, quarantine, and audit
  • keep K3S/Docker lifecycle ownership in their runtimes while adding Native recovery
  • mark Native health-check timeouts as failed/stalled so recovery can act
  • baseline historical Docker/K3S restart counts before evaluating the current failure window
  • reject recovery when sanitized secret values cannot be safely reconstructed
  • retry quarantine deletion failures without redeploying the workload

Scope

This PR is based on the latest develop branch and contains only generic deployment intent, reconciler, and runtime recovery work. It does not include AMD395 binaries, vendor branches, or engine inventory/lifecycle changes.

Regression coverage

  • Native process remains alive but never healthy, then enters recovery
  • healthy Docker/K3S deployment with historical restarts is not quarantined
  • sanitized API key/token values are never replayed as [REDACTED]
  • transient quarantine delete failures are retried and never call Deploy
  • explicit user stop remains stopped

Validation

  • key package tests passed
  • key package race tests passed
  • go vet ./... passed
  • CGO_ENABLED=0 builds passed for windows/amd64, darwin/arm64, linux/amd64, and linux/arm64
  • go test ./... has one environment-only failure in internal/external because this workstation exposes two real inference services; the same test fails identically on unmodified origin/develop

@qinyue0508-cmd
qinyue0508-cmd requested a review from rjckkkkk August 4, 2026 06:16

@rjckkkkk rjckkkkk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I found two blocking behavioral issues. go test ./..., go vet ./..., and targeted race tests pass, but the missing-container reconciliation path and persisted numeric config path are not covered by those tests.

if restartDelta > 0 || next.ConsecutiveFailureCount >= next.Policy.ConsecutiveFailures {
next.RecoveryState = StateWaiting
}
return Decision{Intent: next, Action: ActionNone}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P1] Recover container runtimes when the runtime object is absent. This branch returns ActionNone for every Docker/K3S failure. When observation.Exists == false, no restart delta can ever be observed, so AttemptCount remains zero and the intent sits in waiting forever. Docker's unless-stopped cannot recreate a removed container, and K3S deploys a standalone kind: Pod, whose restartPolicy: Always only restarts containers inside an existing Pod; it does not recreate a deleted Pod object. Thus an externally removed container/Pod violates the persisted desired_state=running indefinitely. Please distinguish confirmed absence from an unhealthy-but-existing container and recreate (or otherwise terminate/quarantine) the missing object.

Comment thread internal/sqlite.go

func decodeDeploymentIntentConfigJSON(value string) (map[string]any, error) {
decoder := json.NewDecoder(bytes.NewBufferString(value))
decoder.UseNumber()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Keep persisted numeric overrides compatible with the fit checks. UseNumber makes every recovered numeric config value a json.Number, and RecoveryApply feeds this map back through resolveDeployment. However, knowledge.CheckFit converts gpu_memory_utilization, mem_fraction_static, and tensor_parallel_size through toFloat64, which handles only Go floats/ints and turns json.Number into zero. As a result, an automatic recovery silently skips the unified-memory reserve, current-free-VRAM adjustment, and GPU-count validation that the original deploy received. A focused test with json.Number("0.95") and json.Number("2") returns Fit:true with no adjustments on a one-GPU unified-memory host. Please normalize decoded numeric values or extend all relevant numeric consumers before replaying the persisted config.

@qinyue0508-cmd
qinyue0508-cmd requested a review from rjckkkkk August 4, 2026 15:02
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