Skip to content

VOIP-1279-Asterisk-proxy-fail-fast-listen-handler - #1156

Merged
pchero merged 1 commit into
mainfrom
VOIP-1279-Asterisk-proxy-fail-fast-listen-handler
Aug 2, 2026
Merged

VOIP-1279-Asterisk-proxy-fail-fast-listen-handler#1156
pchero merged 1 commit into
mainfrom
VOIP-1279-Asterisk-proxy-fail-fast-listen-handler

Conversation

@pchero

@pchero pchero commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Make asterisk-proxy exit non-zero when its RabbitMQ listen handler cannot establish or permanently loses its consumer topology at startup, so restart policy heals the instance instead of it running indefinitely with zero consumers. Found during VOIP-1275 live internal-mode verification, where a missing delayed-message exchange plugin left three proxy instances "Up" but unable to service any RPC request.

  • voip-asterisk-proxy: ListenHandler.Run() now declares all listen queues (permanent + volatile) synchronously with no retry, validates the combined queue-name list for empty or duplicate entries before any broker call, and returns an error immediately on the first declare failure
  • voip-asterisk-proxy: Run() returns a buffered error channel (sized to the number of listen queues) that consumer goroutines use to report a ConsumeRPC failure after a successful start, without ever blocking on send
  • voip-asterisk-proxy: main() now calls log.Fatalf when getAsteriskIDAddress fails or when the listen handler cannot be established at startup or reports a consumer failure via the error channel; Redis address registration, Kubernetes annotation patching, and the ARI/AMI event handler intentionally keep their existing soft-failure (log and continue) behavior — see the design doc for the per-path reasoning
  • voip-asterisk-proxy: add unit tests covering queue-name validation, permanent/volatile declare failure paths, successful consumer startup, single and concurrent consumer-error delivery on the buffered channel
  • voip-asterisk-proxy: update docs/architecture.md, docs/operations.md, docs/subsystems.md to describe the new fail-fast startup sequence, the resulting operational signature (crash-loop instead of a silently-idle proxy), and the residual single-container deployment risk
  • docs: add the design doc and implementation plan for this change (docs/plans/), both went through multiple rounds of independent review

Design trade-offs accepted (detailed in the design doc):

  • log.Fatalf also stops the ARI/AMI event relay, in sidecar mode as well as single-container — in-progress calls lose their terminal-event path during the crash-loop window, in exchange for visibility and self-healing over today's silent, unbounded outage
  • The observed trigger (missing broker plugin) is fleet-correlated, so all proxy instances would crash-loop together rather than one instance silently degrading
  • No in-process retry/backoff was added; backoff between restart attempts is left entirely to the container runtime's restart policy
  • Post-startup consumer loss (bin-common-handler's reconsumerAll exhausting its bounded retry after a reconnect) is a related but separate gap requiring a bin-common-handler change with a 38-service blast radius — out of scope here, follow-up ticket to be filed after this merges

Still pending before merge (implementation plan steps 5b/6/7, need live infra access):

  • Confirm deployed instances' --rabbitmq_queue_listen config has no empty/duplicate/colliding queue names (the new validation turns that into a permanent crash loop)
  • Sandbox replay: break the delayed-message plugin, start the stack, confirm proxies crash-loop, fix the plugin, confirm consumers recover without manual intervention — this design's stated merge gate
  • File the bin-common-handler follow-up ticket for the post-startup consumer-loss gap noted above

Test plan:

  • go mod tidy && go mod vendor && go generate ./... && go test -race ./... && golangci-lint run — all clean (35 tests passing)
  • Design doc: 5 rounds of independent review, 2 consecutive approvals
  • Implementation plan: 4 rounds of independent review, 2 consecutive approvals
  • Code: 3 rounds of independent review (functional correctness, security/concurrency, quality), all approved
  • Sandbox replay of the VOIP-1279 scenario (gates merge per design doc)
  • Rollout-safety confirmation of deployed queue-name config

Fail fast when the RabbitMQ listen handler cannot establish its
consumer topology at startup, so restart policy can heal a broken
proxy instead of it running indefinitely with zero consumers.

- voip-asterisk-proxy: ListenHandler.Run() now declares all listen
  queues synchronously (no retry) and validates the combined
  permanent+volatile queue name list for empty/duplicate entries
  before any broker call, returning an error immediately on failure
- voip-asterisk-proxy: Run() returns a buffered error channel that
  consumer goroutines use to report ConsumeRPC failures after a
  successful start
- voip-asterisk-proxy: main() now exits non-zero (log.Fatalf) when
  getAsteriskIDAddress fails or when the listen handler cannot be
  established at startup or loses a consumer permanently; Redis
  registration, Kubernetes annotation patching, and the ARI/AMI event
  handler keep their existing soft-failure behavior by design
- voip-asterisk-proxy: add unit tests for queue validation, declare
  failure paths, and consumer error delivery on the buffered channel
- voip-asterisk-proxy: update docs/architecture.md, docs/operations.md,
  docs/subsystems.md to describe the new fail-fast startup behavior,
  operational signature, and the single-container deployment risk
- docs: add design and implementation plan documents for this change
@pchero
pchero merged commit 8e6b520 into main Aug 2, 2026
2 of 3 checks passed
@pchero
pchero deleted the VOIP-1279-Asterisk-proxy-fail-fast-listen-handler branch August 2, 2026 00:10
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.

1 participant