fix(reaper): deliver pre-reap warnings for real — and never fake a delivery
The §18 warning path had no delivery channel at all: no Warner implementation existed, `felis reaper` passed nil, and maybeWarn still stamped warned_3d_at/ warned_1d_at and counted `warned=N`. So every owned server was silently reaped 15 days after its last join with no notice, and the operator's only feedback said warnings were sent. Two changes close that: - Honest stamps: warned_* now records a DELIVERED notice. A nil Warner logs `warning suppressed — no warner wired` and does NOT stamp; a delivery error logs and retries on the next daily run (bounded by the warning window). The stamps are no longer burned by notices nobody received. - A real channel: mail.SendNotice (the second and last message shape the mail package sends) plus a mailWarner that resolves the owner's VERIFIED email and mails the notice through the configured [smtp] relay. `felis reaper` wires it when [smtp] is set (same password_ref convention as felis-api) and prints exactly what happens when it is not. Plumbing so the in-cluster CronJob can actually reach the relay: the reaper pod gets the optional FELIS_SMTP_PASSWORD env (same Secret as felis-api), and the "configure email" screen now refreshes the minecraft-namespace mirrors of felis-smtp AND felis-config (a secretKeyRef is namespace-local, and the config mirror is what carries [smtp] into the reaper's own config). `felis setup`'s replica list gains felis-smtp for fresh installs. Tests: the delivered/retried/suppressed matrix in internal/reaper (the old "stamp advances on failure" contract is deliberately replaced), the notice message shape, the warner's resolve/send/failure paths, and the CronJob's optional-secret env. docs/troubleshooting.md §10 now states the real semantics.
This commit is contained in:
12 files changed
+375
-27
No files matched your search
@@ -203,7 +203,10 @@ type Cluster interface {
|
||||
}
|
||||
|
||||
// Warner delivers an impending-reap notice. It is optional and best-effort: a
|
||||
// nil Warner or a delivery error never blocks a reap (red line ⑤).
|
||||
// nil Warner or a delivery error never blocks a reap (red line ⑤). Warn returns
|
||||
// nil only when the notice was handed to the delivery channel; an error (or a
|
||||
// nil Warner) leaves warned_* unstamped, so the next daily run retries instead
|
||||
// of silently burning the owner's only warning.
|
||||
type Warner interface {
|
||||
Warn(ctx context.Context, ownerID, server, remaining string) error
|
||||
}
|
||||
@@ -433,9 +436,11 @@ func (r *Reaper) ensureCapacity(ctx context.Context, now time.Time, sum *Summary
|
||||
|
||||
// maybeWarn sends at most one impending-reap notice per run, honoring §18's
|
||||
// elif precedence (earliest unsent warning first). Unowned servers are never
|
||||
// warned but are still reaped at the deadline (red line ⑤). A warner delivery
|
||||
// failure is logged but the warned_* stamp still advances so the notice is not
|
||||
// retried forever; a real join (RecordJoin) is what clears the stamps.
|
||||
// warned but are still reaped at the deadline (red line ⑤). The warned_* stamp
|
||||
// records a DELIVERED notice: a nil Warner or a delivery error is logged and
|
||||
// leaves the stamp untouched, so the next run retries — bounded by the warning
|
||||
// window, since the reap itself removes the candidate. A real join (RecordJoin)
|
||||
// clears the stamps when a player renews.
|
||||
func (r *Reaper) maybeWarn(ctx context.Context, now time.Time, idle time.Duration, offs []time.Duration, c Candidate, sum *Summary) {
|
||||
if c.OwnerID == "" {
|
||||
return
|
||||
@@ -449,10 +454,21 @@ func (r *Reaper) maybeWarn(ctx context.Context, now time.Time, idle time.Duratio
|
||||
if !c.warnedAt(tier).IsZero() {
|
||||
continue // already sent this tier
|
||||
}
|
||||
if r.Warner != nil {
|
||||
if err := r.Warner.Warn(ctx, c.OwnerID, c.Name, formatRemaining(offs[i])); err != nil {
|
||||
r.log().Warn("reaper: warn delivery failed (best-effort)", "server", c.Name, "err", err)
|
||||
}
|
||||
if r.Warner == nil {
|
||||
// No delivery channel is wired at all. Do not stamp: an operator who
|
||||
// wires one later must still be able to warn, and a stamp here would
|
||||
// have recorded a notice nobody received. Logged every run so silence
|
||||
// is never mistaken for delivery.
|
||||
r.log().Warn("reaper: warning suppressed — no warner wired",
|
||||
"server", c.Name, "owner", c.OwnerID, "remaining", formatRemaining(offs[i]))
|
||||
return
|
||||
}
|
||||
if err := r.Warner.Warn(ctx, c.OwnerID, c.Name, formatRemaining(offs[i])); err != nil {
|
||||
// Best-effort: the reap still proceeds on schedule, but the stamp
|
||||
// stays empty so the next daily run retries the delivery instead of
|
||||
// permanently suppressing the owner's only notice.
|
||||
r.log().Warn("reaper: warn delivery failed; will retry next run", "server", c.Name, "err", err)
|
||||
return
|
||||
}
|
||||
if err := r.Store.MarkWarned(ctx, c.Name, tier, now); err != nil {
|
||||
r.log().Error("reaper: mark warned failed", "server", c.Name, "err", err)
|
||||
|
||||
@@ -472,6 +472,8 @@ func TestWarningsDerivedFromNonDefaultDeadline(t *testing.T) {
|
||||
// past 7d but unowned -> never warned (red line ⑤)
|
||||
Candidate{Name: "e", OwnerID: "", LastActiveAt: idleBy(8 * Day)},
|
||||
)
|
||||
rw := &recordingWarner{}
|
||||
r.Warner = rw
|
||||
|
||||
sum := mustRun(t, r)
|
||||
if sum.WorldsReaped != 0 {
|
||||
@@ -480,6 +482,9 @@ func TestWarningsDerivedFromNonDefaultDeadline(t *testing.T) {
|
||||
if sum.Warned != 2 {
|
||||
t.Fatalf("Warned = %d, want 2 (a:3d, b:1d)", sum.Warned)
|
||||
}
|
||||
if len(rw.sent) != 2 {
|
||||
t.Fatalf("deliveries = %d, want 2 (a:3d, b:1d)", len(rw.sent))
|
||||
}
|
||||
if cl.deletePVCCalls != 0 {
|
||||
t.Fatalf("a warning path deleted a PVC")
|
||||
}
|
||||
@@ -494,20 +499,48 @@ func TestWarningsDerivedFromNonDefaultDeadline(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// Red line ⑤ (best-effort): a Warner delivery error does not abort the run, and
|
||||
// the warned_* stamp still advances (a real join, not a failed warn, is what
|
||||
// resets the clock).
|
||||
func TestWarningBestEffortOnDeliveryFailure(t *testing.T) {
|
||||
// Red line ⑤ (best-effort) with delivery honesty: a Warner failure does not
|
||||
// abort the run, and it does NOT stamp — the stamp records a DELIVERED notice,
|
||||
// so the next daily run retries (the warning window bounds the retries, and the
|
||||
// reap clears the candidate either way).
|
||||
func TestWarningDeliveryRetriedAfterFailure(t *testing.T) {
|
||||
r, st, _, _ := newReaper(DefaultConfig(),
|
||||
Candidate{Name: "h", OwnerID: "u-h", LastActiveAt: idleBy(13 * Day)})
|
||||
r.Warner = failWarner{}
|
||||
|
||||
sum := mustRun(t, r)
|
||||
if sum.Warned != 1 {
|
||||
t.Fatalf("Warned = %d, want 1 despite delivery failure", sum.Warned)
|
||||
if sum.Warned != 0 {
|
||||
t.Fatalf("Warned = %d, want 0 (nothing was delivered)", sum.Warned)
|
||||
}
|
||||
if !st.byName["h"].Warned3dAt.IsZero() {
|
||||
t.Fatal("a failed delivery must not stamp warned_3d_at")
|
||||
}
|
||||
|
||||
// Next run with a working channel: the SAME warning goes out and stamps.
|
||||
rw := &recordingWarner{}
|
||||
r.Warner = rw
|
||||
sum = mustRun(t, r)
|
||||
if sum.Warned != 1 || len(rw.sent) != 1 {
|
||||
t.Fatalf("retry: Warned=%d sent=%d, want 1/1", sum.Warned, len(rw.sent))
|
||||
}
|
||||
if st.byName["h"].Warned3dAt.IsZero() {
|
||||
t.Fatalf("warned_3d_at not stamped after best-effort warn")
|
||||
t.Fatal("a delivered warning must stamp warned_3d_at")
|
||||
}
|
||||
}
|
||||
|
||||
// A nil Warner suppresses the warning WITHOUT stamping it: nothing was sent, so
|
||||
// nothing is recorded as sent — and the day a channel is wired, the owner can
|
||||
// still be warned.
|
||||
func TestWarningSuppressedWithoutWarner(t *testing.T) {
|
||||
r, st, _, _ := newReaper(DefaultConfig(),
|
||||
Candidate{Name: "n", OwnerID: "u-n", LastActiveAt: idleBy(13 * Day)})
|
||||
|
||||
sum := mustRun(t, r)
|
||||
if sum.Warned != 0 {
|
||||
t.Fatalf("Warned = %d, want 0 with no warner wired", sum.Warned)
|
||||
}
|
||||
if !st.byName["n"].Warned3dAt.IsZero() || !st.byName["n"].Warned1dAt.IsZero() {
|
||||
t.Fatal("a suppressed warning must not stamp either tier")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -517,6 +550,15 @@ func (failWarner) Warn(context.Context, string, string, string) error {
|
||||
return errors.New("smtp unavailable")
|
||||
}
|
||||
|
||||
// recordingWarner captures deliveries so the threshold tests exercise the real
|
||||
// deliver-then-stamp path.
|
||||
type recordingWarner struct{ sent []string }
|
||||
|
||||
func (w *recordingWarner) Warn(_ context.Context, ownerID, server, remaining string) error {
|
||||
w.sent = append(w.sent, ownerID+"/"+server+"/"+remaining)
|
||||
return nil
|
||||
}
|
||||
|
||||
// §26 capacity: when the store is over its cap, the oldest backup is evicted
|
||||
// early (destructive — audited) to make room, then the reap proceeds.
|
||||
func TestCapacityEvictsOldestThenReaps(t *testing.T) {
|
||||
|
||||
Reference in new issue
Block a user