[Feature] Recover from panics in the reconcile loop and background goroutines to preserve observability

Open
  • Is something wrong?
#44 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
68/100
Issue type
Feature
Clarity
Clearly specified
Activity status
Active
Tech stack
go, redis
Domain
backend, observability, testing-qa

Research direction

Start with internal/reconciler/reconciler.go and trace Start() through reconcile(), then inspect fan-out goroutines in internal/reconciler/scale_reconciler.go and the three goroutines in cmd/main.go. Run the existing unit tests before adding coverage for recovered panics, continued scheduling, server availability, logging, and the panic counter. Done means one shared recovery helper protects every listed goroutine without changing normal error backoff.

Written by the indexing model from the issue text.

Description

enhancement

Motivation

The primary goal of this change is to preserve observability when something goes wrong.

Today, any unrecovered panic crashes the entire Robin process. Because the metrics/health HTTP server runs in the same process, a panic takes down /metrics (and the future /healthz) exactly when an operator most needs them to diagnose the problem. On top of that, repeated panics drive the pod into CrashLoopBackOff (backoff up to ~5 min), during which the cluster is not managed and no telemetry is being served.

Robin is effectively stateless between cycles — every reconcile rebuilds its state from the live cluster (re-reads config, recreates Redis clients, rediscovers nodes). So the usual "don't continue after a panic, the in-memory state may be corrupt" argument does not apply here: there is no cross-cycle state to corrupt. This makes in-process recovery both safe and clearly beneficial.

Problem

Robin has no panic recovery anywhere in internal/:

  • The reconciliation loop (internal/reconciler/reconciler.go, Start() → reconcile()) calls reconcile logic with no recover().
  • Several paths fan out into background goroutines (e.g. concurrent master checks in internal/reconciler/scale_reconciler.go) with no recover(). A panic in a goroutine cannot be recovered by its caller and unconditionally kills the process.
  • The three goroutines launched in cmd/main.go (reconciler, metrics collector, metrics server) are likewise unprotected.

Unlike the operator — whose controller-runtime manager recovers reconcile panics by default (RecoverPanic defaults to true) — Robin is a plain polling loop and inherits no such protection.

Proposed solution

Add defensive panic recovery so a single bad reconcile can neither crash the process nor kill observability:

  1. Reconcile loop: wrap each reconcile() invocation so a panic is caught and converted into a failed cycle that reschedules on the existing error interval (intervalOnError) — keeping the process (and the metrics/health server) alive.
  2. Background goroutines: add a shared recover() helper used at the entry point of every goroutine running reconcile/Redis logic (fan-outs in scale_reconciler.go, and the three goroutines in cmd/main.go). A panicking goroutine fails its unit of work, it does not abort the process.
  3. Keep panics loud (non-negotiable): every recovered panic MUST be logged at error level with the recovered value and a full stack trace (runtime/debug.Stack()), and increment a robin_reconcile_panics_total (or similar) counter. Recovery must not hide bugs — it only prevents them from destroying visibility.

Acceptance criteria

  • A panic inside reconcile() is recovered: logged with stack trace, counted, and the loop continues (reschedules on intervalOnError) instead of exiting.
  • A panic inside any background goroutine (fan-out checks, collector, metrics server, reconciler goroutine in main.go) is recovered and does not crash the process.
  • The metrics/health HTTP server stays up across a recovered reconcile panic (observability preserved).
  • A ..._panics_total counter is incremented on each recovered panic, and each panic is logged at error with value + stack.
  • Recovery does not alter behavior for normal error returns: existing scheduling/backoff is unchanged.
  • Unit tests: a panicking reconcile leaves the loop alive, keeps the server serving, and increments the metric; a panicking goroutine is contained.
  • A single shared helper (e.g. recoverPanic(logger, metric)) is used consistently instead of ad-hoc recovers.

Implementation notes

  • Loop: internal/reconciler/reconciler.go (Start() / reconcile()) — a defer func(){ if r := recover(); r != nil { ... ; onError = true } }() around the reconcile call is enough to convert a panic into a throttled failed cycle.
  • Goroutines: add the helper in a shared package and apply it at each go func(){...}() entry (notably scale_reconciler.go fan-outs and the three goroutines in cmd/main.go).
  • Throttling comes for free from intervalOnError, so a persistently panicking condition does not become a hot loop.
  • Log stacks with runtime/debug.Stack().

Scope / non-goals

  • This change targets panics only. It does not detect a wedged/deadlocked loop that never panics — that is covered by the health-endpoint feature (issue A: /healthz liveness based on "last successful reconcile").
  • The recovered-panic state may later feed the /healthz liveness signal (repeated panics → unhealthy → Kubernetes restart) as a follow-up.

Related

  • Complements redkey-robin issue A (/healthz health endpoint): recovery keeps observability up; liveness catches non-panic hangs.
  • Mirrors the operator's built-in behavior from controller-runtime's default RecoverPanic.
Dominant language
Go
Stars
18
Forks
0
Avg merge
11h 16m
Merged PRs (30d)
11

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from InditexTech/redkey-robin

All issues in InditexTech/redkey-robin

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.