v0.263.1: the undo asks the old probe even on an app marked unhealthy (R-637)
gates / gates (push) Successful in 27s
gates / gates (push) Successful in 27s
Found live on 9202: the periodic probe (current .felhom.yml, new port) flips the app to unhealthy, and the update's health wait probed only 'running' apps - so the undo's old probe was never asked and a serving old version was judged "did not start". With the undo's override, an unhealthy app is probed and the old check decides; never settled on container state. New seam probeRunFn; the test drives the real wait loop and reproduces the live message when the fix is switched off. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS
This commit is contained in:
@@ -1,3 +1,22 @@
|
|||||||
|
## v0.263.1 — the undo asks the OLD probe even when the new one has marked the app unhealthy (2026-09-23, R-637)
|
||||||
|
|
||||||
|
**MinAgent: 0.131.0** (unchanged). No new strings.
|
||||||
|
|
||||||
|
**Caught by the live proof on 9202, not by a test.** v0.263.0's undo put docmost's data and old
|
||||||
|
definition back, and then judged the old version "did not start" after the full 90 s — while it was
|
||||||
|
serving. The periodic health probe judges an app with its CURRENT `.felhom.yml`; the new version had
|
||||||
|
brought a probe the old one does not answer (the drill's wrong port, and a real case whenever a probe
|
||||||
|
moves with a version), so it flipped the app to `unhealthy` every 10 s — and the update's health wait
|
||||||
|
only probes an app whose state reads `running`. The old probe was never asked (`last: state unhealthy`).
|
||||||
|
|
||||||
|
**Fix:** with the undo's override probe, an `unhealthy` app IS probed, and the old check decides; it
|
||||||
|
is never settled on container state (the settle path still requires `running`). The normal update
|
||||||
|
path is unchanged. **Why the unit tests missed it:** `TestUndo_UsesTheOldProbe` injects the undo's
|
||||||
|
whole health wait, so it never reached the state gate. The new test drives the real wait loop with only
|
||||||
|
the network probe faked (new seam `Manager.probeRunFn`) and reproduces the live message verbatim when
|
||||||
|
the fix is switched off. The first live attempt ended in an honest HOLD (data put back, old version
|
||||||
|
not judged healthy) — kept as evidence.
|
||||||
|
|
||||||
## v0.263.0 — a failed update puts the app back by itself (2026-09-23, R-637)
|
## v0.263.0 — a failed update puts the app back by itself (2026-09-23, R-637)
|
||||||
|
|
||||||
**MinAgent: 0.131.0** (unchanged). New strings, all born as bundle keys (hu + en): `app_info.update_undone`,
|
**MinAgent: 0.131.0** (unchanged). New strings, all born as bundle keys (hu + en): `app_info.update_undone`,
|
||||||
|
|||||||
@@ -242,6 +242,7 @@ type Manager struct {
|
|||||||
// which receives the probe it must use (nil ⇒ waitUpdateHealthyMeta).
|
// which receives the probe it must use (nil ⇒ waitUpdateHealthyMeta).
|
||||||
undoCopier volumeCopier
|
undoCopier volumeCopier
|
||||||
updateUndoHealthFn func(ctx context.Context, name string, timeout time.Duration, meta *Metadata) (bool, string)
|
updateUndoHealthFn func(ctx context.Context, name string, timeout time.Duration, meta *Metadata) (bool, string)
|
||||||
|
probeRunFn func(t probeTarget) *HealthProbeResult // the health wait's network probe; nil ⇒ runChecks
|
||||||
updateMemoryFn func(newReqMB, newLimitMB, releasedReqMB, releasedLimitMB int) (refusal error, warning string)
|
updateMemoryFn func(newReqMB, newLimitMB, releasedReqMB, releasedLimitMB int) (refusal error, warning string)
|
||||||
updateDiskFreeFn func() (freeGiB float64, ok bool)
|
updateDiskFreeFn func() (freeGiB float64, ok bool)
|
||||||
updateNowFn func() time.Time
|
updateNowFn func() time.Time
|
||||||
|
|||||||
@@ -415,3 +415,66 @@ func TestUndo_RemovalDeletesKeptCopies(t *testing.T) {
|
|||||||
t.Errorf("removed %d, left %d", n, fc.nCopies())
|
t.Errorf("removed %d, left %d", n, fc.nCopies())
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestUndo_OldProbeRunsOnAnAppTheCurrentProbeMarkedUnhealthy drives the REAL wait loop
|
||||||
|
// (waitUpdateHealthyMeta → RefreshStatus → `docker ps` → the health-probe override → the state gate →
|
||||||
|
// findProbeContainerMeta) with only the network probe faked. The app's CURRENT .felhom.yml probe has
|
||||||
|
// failed, so the refresh reads it Unhealthy; the undo's OLD probe answers.
|
||||||
|
//
|
||||||
|
// FOUND LIVE, NOT BY A TEST: v0.263.0's first build gated on StateRunning, so the old probe was never
|
||||||
|
// asked and docmost's undo on 9202 was reported "did not start" after the full 90 s while the old
|
||||||
|
// version served. TestUndo_UsesTheOldProbe could not see it — it injects updateUndoHealthFn and so
|
||||||
|
// never reaches this loop (the seam-boundary class: a test proves only what is behind its seam).
|
||||||
|
//
|
||||||
|
// COMPANION RED-PROOF (REPORT.md): drop `|| undoProbe` from the state case — the wait times out with
|
||||||
|
// `last: state unhealthy` and this test fails.
|
||||||
|
func TestUndo_OldProbeRunsOnAnAppTheCurrentProbeMarkedUnhealthy(t *testing.T) {
|
||||||
|
m, dir, _, _, _ := newUndoManager(t)
|
||||||
|
mustWrite(t, filepath.Join(dir, ".felhom.yml"), undoMetaNew) // the catalog's probe (3999) is current
|
||||||
|
m.mu.Lock()
|
||||||
|
m.stacks["nextcloud"].Meta = LoadMetadata(dir)
|
||||||
|
m.stacks["nextcloud"].HealthProbe = &HealthProbeResult{Healthy: false} // the periodic probe failed on 3999
|
||||||
|
m.mu.Unlock()
|
||||||
|
m.execFn = func(name string, args ...string) (string, error) {
|
||||||
|
if name == "docker" && len(args) > 0 && args[0] == "ps" {
|
||||||
|
return "nextcloud\tnextcloud:31.0.14-apache\trunning\tUp 30 seconds\tnextcloud\n", nil
|
||||||
|
}
|
||||||
|
return "", nil
|
||||||
|
}
|
||||||
|
m.inspectRestartPolicyFn = func(string) (string, error) { return "unless-stopped", nil }
|
||||||
|
m.updateNowFn = time.Now // the shared setup freezes the clock; this loop's deadline needs it moving
|
||||||
|
var probed []int
|
||||||
|
m.probeRunFn = func(pt probeTarget) *HealthProbeResult {
|
||||||
|
probed = append(probed, pt.checks[0].Port)
|
||||||
|
return &HealthProbeResult{Healthy: pt.checks[0].Port == 3000}
|
||||||
|
}
|
||||||
|
if err := m.RefreshStatus(); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if st, _ := m.GetStack("nextcloud"); st.State != StateUnhealthy {
|
||||||
|
t.Fatalf("setup: the refresh must read the app Unhealthy (the current probe failed), got %q", st.State)
|
||||||
|
}
|
||||||
|
old, err := savePreUpdateMeta(dir) // what the job kept — but it holds the NEW file here, so write the OLD one
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
mustWrite(t, filepath.Join(old, ".felhom.yml"), undoMetaOld)
|
||||||
|
oldMeta := LoadMetadata(old)
|
||||||
|
|
||||||
|
ok, detail := m.waitUpdateHealthyMeta(context.Background(), "nextcloud", time.Second, &oldMeta)
|
||||||
|
if !ok {
|
||||||
|
t.Fatalf("the old version answers its own probe and must be judged healthy; got %q, probed ports %v", detail, probed)
|
||||||
|
}
|
||||||
|
if len(probed) == 0 || probed[0] != 3000 {
|
||||||
|
t.Errorf("the undo must probe the OLD port (3000), probed %v", probed)
|
||||||
|
}
|
||||||
|
// And without an override the same Unhealthy app is NOT probed or settled — the normal update path
|
||||||
|
// is unchanged.
|
||||||
|
probed = nil
|
||||||
|
m.mu.Lock()
|
||||||
|
m.stacks["nextcloud"].HealthProbe = &HealthProbeResult{Healthy: false} // the first call stored its healthy result
|
||||||
|
m.mu.Unlock()
|
||||||
|
if ok, _ := m.waitUpdateHealthyMeta(context.Background(), "nextcloud", time.Second, nil); ok || len(probed) != 0 {
|
||||||
|
t.Errorf("without an override an Unhealthy app must stay unhealthy and unprobed; ok=%v probed=%v", ok, probed)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -918,11 +918,22 @@ func (m *Manager) waitUpdateHealthyMeta(ctx context.Context, name string, timeou
|
|||||||
for {
|
for {
|
||||||
_ = m.RefreshStatus()
|
_ = m.RefreshStatus()
|
||||||
st, ok := m.GetStack(name)
|
st, ok := m.GetStack(name)
|
||||||
|
// v0.263.0 — THE UNDO'S PROBE MUST BE ALLOWED TO RUN. The periodic health probe judges the app
|
||||||
|
// with the CURRENT .felhom.yml and flips a running app to StateUnhealthy when that check fails —
|
||||||
|
// which is exactly the undo's situation when the new version brought a probe the old one does
|
||||||
|
// not answer. Gating on StateRunning alone meant the old probe was never asked and the undo was
|
||||||
|
// reported "did not start" after the full timeout. MEASURED LIVE on 9202 2026-09-23 (docmost,
|
||||||
|
// `last: state unhealthy` for 90 s while the old version served). So with an override that
|
||||||
|
// declares checks, an Unhealthy app is PROBED — and the override's own check decides. It never
|
||||||
|
// settles an Unhealthy app on container state (below: the settle path requires StateRunning).
|
||||||
|
// Pinned by TestUndo_OldProbeRunsOnAnAppTheCurrentProbeMarkedUnhealthy.
|
||||||
|
undoProbe := ok && override != nil && st.State == StateUnhealthy &&
|
||||||
|
override.HealthCheck != nil && len(override.HealthCheck.Checks) > 0
|
||||||
switch {
|
switch {
|
||||||
case !ok:
|
case !ok:
|
||||||
last = "stack vanished"
|
last = "stack vanished"
|
||||||
runningSince = time.Time{}
|
runningSince = time.Time{}
|
||||||
case st.State == StateRunning:
|
case st.State == StateRunning || undoProbe:
|
||||||
// A declared health check is only usable if it resolves to a container. When it does
|
// A declared health check is only usable if it resolves to a container. When it does
|
||||||
// not, the app is judged the same way an app with NO declared check is judged —
|
// not, the app is judged the same way an app with NO declared check is judged —
|
||||||
// settling on container state — and the log says so.
|
// settling on container state — and the log says so.
|
||||||
@@ -942,7 +953,7 @@ func (m *Manager) waitUpdateHealthyMeta(ctx context.Context, name string, timeou
|
|||||||
c, candidates := findProbeContainerMeta(name, &meta, st.Containers)
|
c, candidates := findProbeContainerMeta(name, &meta, st.Containers)
|
||||||
if c != "" {
|
if c != "" {
|
||||||
usable = true
|
usable = true
|
||||||
res := m.runChecks(probeTarget{stackName: name, containerName: c, checks: hc.Checks})
|
res := m.probeRun(probeTarget{stackName: name, containerName: c, checks: hc.Checks})
|
||||||
m.mu.Lock()
|
m.mu.Lock()
|
||||||
if s, ok := m.stacks[name]; ok {
|
if s, ok := m.stacks[name]; ok {
|
||||||
s.HealthProbe = res
|
s.HealthProbe = res
|
||||||
@@ -958,7 +969,9 @@ func (m *Manager) waitUpdateHealthyMeta(ctx context.Context, name string, timeou
|
|||||||
name, name, candidates)
|
name, name, candidates)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if !usable {
|
if !usable && st.State != StateRunning {
|
||||||
|
last = "unhealthy, and the check resolves to no container"
|
||||||
|
} else if !usable {
|
||||||
if runningSince.IsZero() {
|
if runningSince.IsZero() {
|
||||||
runningSince = m.now()
|
runningSince = m.now()
|
||||||
}
|
}
|
||||||
@@ -1213,3 +1226,13 @@ func (m *Manager) ResumeInterruptedUpdates(ctx context.Context) int {
|
|||||||
}
|
}
|
||||||
return len(names)
|
return len(names)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// probeRun is the network half of the update's health wait — its own seam, so a test can drive the
|
||||||
|
// REAL wait loop (the state gate, the container resolution, the settle rule) with only the HTTP/TCP
|
||||||
|
// probe faked. nil ⇒ runChecks.
|
||||||
|
func (m *Manager) probeRun(t probeTarget) *HealthProbeResult {
|
||||||
|
if m.probeRunFn != nil {
|
||||||
|
return m.probeRunFn(t)
|
||||||
|
}
|
||||||
|
return m.runChecks(t)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user