Merging to release-5.13: [TT-17486] fix: stop liveness probe blocking on node re-registration (#8314) by probelabs[bot] · Pull Request #8328 · TykTechnologies/tyk

probelabs · 2026-06-17T11:55:23Z

TT-17486 fix: stop liveness probe blocking on node re-registration (#8314)

Description

/hello and /ready served a stale "pass" status forever when Redis
failed before the Dashboard (regression in 5.8.14 / 5.13.0).

Root cause: the liveness probe Ping() shared sendHeartBeat, whose
403 branch calls DashService.Register(ctx) — since #8188 an unbounded
5s retry loop — with context.Background(). With Redis down and the
Dashboard up, the Dashboard answers the heartbeat with 403 and every
registration attempt with 409 + Status:"Error", so Register() never
returned, the dashboard probe goroutine never released
gatherHealthChecks' wg.Wait(), the cached health map was never
refreshed, and the synchronous ticker loop wedged for the duration of
the outage. Kubernetes kept routing traffic to Gateways without Redis.

The fix, in three layers (none of which reverts the TT-16865 behaviour —
startup registration retry is unchanged):

The probe no longer triggers re-registration. The heartbeat HTTP
exchange is extracted into doHeartBeat(), returning a sentinel on 403.
sendHeartBeat() (heartbeat loop) keeps 403 → Register(ctx); Ping()
treats 403 as "dashboard reachable → healthy", preserving v5.8.13
endpoint payloads exactly. Re-registration is owned by the
StartBeating loop.
The probe is bounded. Ping() runs under a context derived from
gw.ctx, bounded at half the configured liveness_check.check_duration
(so the probe always reports its own error before the round's barrier
expires), so no probe request can outlive a check interval.
The health-check barrier is wedge-proof. gatherHealthChecks()
waits for its probes only up to the check interval, commits a race-safe
snapshot of what completed, and reports any unfinished component as
fail / "health check timed out" — no single hung dependency can ever
freeze /hello and /ready again, whatever the failure order.

Related Issue

https://tyktech.atlassian.net/browse/TT-17486

Motivation and Context

In Tyk Pro deployments on Kubernetes the readiness probe is the only
signal gating traffic to Gateway Pods. With this regression, a Gateway
that lost Redis kept answering /ready with 200/pass indefinitely, so
traffic continued to be routed to instances that could not serve it. The
failure was order-dependent (only when Redis died before the Dashboard),
which is what made it hard to diagnose; the storage-library theory was
ruled out (all good/bad tags pin storage v1.3.1; health_check.go is
byte-identical across them — only the #8188 Register() rewrite tracks
the good/bad split).

How This Has Been Tested

TDD throughout — every regression test was watched failing on the
unfixed code first:

TestPing_RedisDownDashboardUp_DoesNotBlockOrReRegister — stubs the
Dashboard's Redis-down wire behaviour (heartbeat 403, registration `409

Status:"Error"). Before: Ping()blocked >10s retrying registration; after: returnsnil` in ~20ms with zero registration attempts.

TestPing_HangingDashboard_BoundedByTimeout — unresponsive Dashboard
with the production-default 30s client; probe now fails at ~5s.
TestGatherHealthChecks_HungProbeDoesNotWedgeTheLoop — injected
DashService whose Ping blocks forever; the round completes within
the interval with redis: pass, dashboard: fail/"health check timed out".
TestSendHeartBeat_Forbidden_ReRegisters — pins the recovery path:
the heartbeat loop still re-registers on 403 (node identity and nonce
refreshed).
Characterization tests written before the refactor pin preserved
behaviour: 200 heartbeat updates the nonce; transport errors still
report "dashboard is down? Heartbeat is failing".
Full dashboard-register + health-check suites green, including the
TT-16865 tests (TestRegister_Retries,
TestRegister_DuplicateSession409); -race clean on the new tests; go vet/gofmt clean.
Differential validation of the root cause: the Redis-down probe test
passes on a v5.8.13 worktree unmodified and fails on 5.8.14+ code.

Not covered here (tracked in the ticket's test plan for QA/e2e):
full-stack failure-order matrix, recovery/flapping soak, and the
Kubernetes readiness scenario.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing
functionality to change)
Refactoring or add test (improvements in base code or adds test
coverage to functionality)

Checklist

I ensured that the documentation is up to date
I explained why this PR updates go.mod in detail with reasoning
why it's required (go.mod untouched)
I would like a code coverage CI quality gate exception and have
explained why (not needed)

Co-authored-by: Kofo Okesola [email protected]
Co-authored-by: Ilija Bojanovic [email protected]

…8314) ## Description `/hello` and `/ready` served a stale "pass" status forever when Redis failed before the Dashboard (regression in 5.8.14 / 5.13.0). Root cause: the liveness probe `Ping()` shared `sendHeartBeat`, whose 403 branch calls `DashService.Register(ctx)` — since #8188 an unbounded 5s retry loop — with `context.Background()`. With Redis down and the Dashboard up, the Dashboard answers the heartbeat with 403 and every registration attempt with `409 + Status:"Error"`, so `Register()` never returned, the dashboard probe goroutine never released `gatherHealthChecks`' `wg.Wait()`, the cached health map was never refreshed, and the synchronous ticker loop wedged for the duration of the outage. Kubernetes kept routing traffic to Gateways without Redis. The fix, in three layers (none of which reverts the TT-16865 behaviour — startup registration retry is unchanged): 1. **The probe no longer triggers re-registration.** The heartbeat HTTP exchange is extracted into `doHeartBeat()`, returning a sentinel on 403. `sendHeartBeat()` (heartbeat loop) keeps 403 → `Register(ctx)`; `Ping()` treats 403 as "dashboard reachable → healthy", preserving v5.8.13 endpoint payloads exactly. Re-registration is owned by the `StartBeating` loop. 2. **The probe is bounded.** `Ping()` runs under a context derived from `gw.ctx`, bounded at half the configured `liveness_check.check_duration` (so the probe always reports its own error before the round's barrier expires), so no probe request can outlive a check interval. 3. **The health-check barrier is wedge-proof.** `gatherHealthChecks()` waits for its probes only up to the check interval, commits a race-safe snapshot of what completed, and reports any unfinished component as `fail` / `"health check timed out"` — no single hung dependency can ever freeze `/hello` and `/ready` again, whatever the failure order. ## Related Issue https://tyktech.atlassian.net/browse/TT-17486 ## Motivation and Context In Tyk Pro deployments on Kubernetes the readiness probe is the only signal gating traffic to Gateway Pods. With this regression, a Gateway that lost Redis kept answering `/ready` with `200/pass` indefinitely, so traffic continued to be routed to instances that could not serve it. The failure was order-dependent (only when Redis died before the Dashboard), which is what made it hard to diagnose; the storage-library theory was ruled out (all good/bad tags pin `storage v1.3.1`; `health_check.go` is byte-identical across them — only the #8188 `Register()` rewrite tracks the good/bad split). ## How This Has Been Tested TDD throughout — every regression test was watched failing on the unfixed code first: - `TestPing_RedisDownDashboardUp_DoesNotBlockOrReRegister` — stubs the Dashboard's Redis-down wire behaviour (heartbeat 403, registration `409 + Status:"Error"`). Before: `Ping()` blocked >10s retrying registration; after: returns `nil` in ~20ms with **zero** registration attempts. - `TestPing_HangingDashboard_BoundedByTimeout` — unresponsive Dashboard with the production-default 30s client; probe now fails at ~5s. - `TestGatherHealthChecks_HungProbeDoesNotWedgeTheLoop` — injected `DashService` whose `Ping` blocks forever; the round completes within the interval with `redis: pass`, `dashboard: fail/"health check timed out"`. - `TestSendHeartBeat_Forbidden_ReRegisters` — pins the recovery path: the heartbeat loop still re-registers on 403 (node identity and nonce refreshed). - Characterization tests written before the refactor pin preserved behaviour: 200 heartbeat updates the nonce; transport errors still report `"dashboard is down? Heartbeat is failing"`. - Full dashboard-register + health-check suites green, including the TT-16865 tests (`TestRegister_Retries`, `TestRegister_DuplicateSession409`); `-race` clean on the new tests; `go vet`/`gofmt` clean. - Differential validation of the root cause: the Redis-down probe test passes on a v5.8.13 worktree unmodified and fails on 5.8.14+ code. Not covered here (tracked in the ticket's test plan for QA/e2e): full-stack failure-order matrix, recovery/flapping soak, and the Kubernetes readiness scenario. ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist - [x] I ensured that the documentation is up to date - [x] I explained why this PR updates go.mod in detail with reasoning why it's required (go.mod untouched) - [x] I would like a code coverage CI quality gate exception and have explained why (not needed) --------- Co-authored-by: Kofo Okesola <[email protected]> Co-authored-by: Ilija Bojanovic <[email protected]> (cherry picked from commit 9f70226)

sentinelone-cnapp-eu1 · 2026-06-17T11:55:29Z

SentinelOne CNS Hardcoded Secret Detector
✅ Congratulations, your code is safe

SentinelOne CNS is a cloud-agnostic, agentless CSPM & CWPP solution that continuously detects and prevents vulnerabilities that have the highest probability of being exploited in Azure, AWS, Google Cloud, and Kubernetes.

github-actions · 2026-06-17T11:56:05Z

🚨 Jira Linter Failed

Commit: 044d215
Failed at: 2026-06-17 11:56:04 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details

failed to get Jira issue: failed to fetch Jira issue TT-17486: Issue does not exist or you do not have permission to see it.: request failed. Please analyze the request body for more details. Status code: 404

Next Steps

Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
Verify your PR title matches the branch's Jira ticket ID
Check that the Jira ticket exists and is accessible

This comment will be automatically deleted once the linter passes.

probelabs · 2026-06-17T11:56:43Z

This PR fixes a critical bug where the Gateway's liveness and readiness probes (/hello, /ready) could become stuck, indefinitely reporting a stale pass status even when a critical dependency like Redis was unavailable. This issue, a regression in recent versions, prevented container orchestrators like Kubernetes from correctly identifying and removing unhealthy Gateway pods from service rotation.

Files Changed Analysis

The changes are focused on the Gateway's health checking and Dashboard registration logic:

gateway/dashboard_register.go: The core logic is refactored. The Ping() function, used by health checks, is modified to no longer trigger a node re-registration on a 403 Forbidden response from the Dashboard. The heartbeat logic is split into doHeartBeat() (for simple reachability checks) and sendHeartBeat() (for the background loop that handles re-registration).
gateway/health_check.go: The gatherHealthChecks() function is now time-bounded. It waits for all component probes to complete, but only up to the configured check interval. Any probe that fails to respond in time is marked as fail, preventing a single hung component from freezing the entire health status.
gateway/dashboard_register_test.go & gateway/health_check_test.go: Extensive new tests have been added to verify the fix. These include tests for the specific Redis-down failure scenario, behavior with a slow or hung Dashboard, and confirmation that the background heartbeat loop still correctly triggers re-registration when needed.

Architecture & Impact Assessment

What this PR accomplishes

This PR makes the Gateway's health checking mechanism significantly more robust. It resolves a failure mode where a Gateway instance could appear healthy to an orchestrator while being unable to process traffic, thereby improving the overall reliability of a Tyk deployment.

Key Technical Changes

Decoupling Probing from Re-registration: The liveness probe's responsibility is now strictly to check the Dashboard's reachability. The responsibility for session recovery and re-registration remains with the background heartbeat process.
Time-bounding Health Checks: Both the individual Dashboard probe and the aggregate health check collection are now protected by timeouts. This ensures that a slow or unresponsive dependency cannot block the health check from reporting a timely and accurate status.

Affected System Components

Gateway Health Probes: The /hello (liveness) and /ready (readiness) endpoints.
Gateway-Dashboard Communication: The heartbeat and node registration mechanisms.
Service Discovery & Load Balancing: In environments like Kubernetes, this change directly impacts how traffic is routed to Gateway pods, ensuring unhealthy pods are correctly removed from the load balancer's pool.

Visualization of the Fix

Previous Blocking Behavior (Redis Down):

sequenceDiagram
    participant K8s as Kubernetes
    participant GW as "Gateway (/hello)"
    participant Probe as Dashboard Probe
    participant Dash as Dashboard

    K8s->>GW: GET /hello
    GW->>Probe: Ping()
    Probe->>Dash: Heartbeat
    Dash-->>Probe: "403 Forbidden (No Redis session)"
    Probe->>Probe: "Call Register() in unbounded loop"
    Probe--xGW: BLOCKS INDEFINITELY
    Note over GW,K8s: Returns stale 'pass' status forever

New Non-Blocking Behavior (Hung Component):

sequenceDiagram
    participant K8s as Kubernetes
    participant GW as "Gateway (/hello)"
    participant HC as HealthCheck Aggregator
    participant DashP as Dashboard Probe
    participant Dash as Dashboard

    K8s->>GW: GET /hello
    GW->>HC: gatherHealthChecks()
    HC->>DashP: Ping()
    DashP->>Dash: Heartbeat (Hangs)
    Note right of HC: Wait for probes with timeout
    HC->>HC: Timeout expires!
    Note right of HC: Mark Dashboard as 'fail'
    HC-->>GW: Return current status (e.g., redis:pass, dash:fail)
    GW-->>K8s: 503 Service Unavailable

Scope Discovery & Context Expansion

The impact of this change is primarily on the operational stability of the Gateway in production environments. By preventing health checks from deadlocking, it ensures that automated systems can reliably determine the Gateway's status.

The changes in health_check.go improve the resilience of all registered health checks (Redis, Dashboard, RPC), not just the Dashboard probe that triggered the original issue.
The refactoring in dashboard_register.go creates a clearer separation of concerns between active liveness probing and the passive, background task of maintaining a valid session with the Dashboard.
This fix is crucial for any Tyk Pro user running in a high-availability, orchestrated environment where automated health checks are the primary mechanism for maintaining service uptime.

Metadata

Review Effort: 3 / 5
Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-06-17T11:58:36.321Z | Triggered by: pr_opened | Commit: 044d215

💡 TIP: You can chat with Visor using /visor ask <your question>

probelabs · 2026-06-17T11:57:40Z

\n\n

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

Performance Issues (1)

Severity	Location	Issue
🟡 Warning	`gateway/health_check.go:114-127`	The timeout mechanism added to `gatherHealthChecks` prevents the health check from being blocked by a slow dependency. However, it abandons the goroutines running the individual health probes if they time out. These abandoned goroutines will continue to run until their underlying I/O operations (like the `gw.DashService.Ping()` call) complete or time out on their own. If a dependency is permanently unresponsive in a way that underlying network calls don't time out (e.g. TCP connection established but no response), this will lead to a goroutine leak, with a new goroutine being leaked on each health check interval. 💡 Suggestion To prevent goroutine leaks, the probes themselves should be cancellable. The idiomatic way to do this in Go is to have probe functions accept a `context.Context`. In `gatherHealthChecks`, a `context.WithTimeout` should be created with a timeout shorter than the health check interval, and this context should be passed to each probe. This ensures that if the health check times out, the underlying operations are also cancelled, allowing the goroutines to exit cleanly. If changing the `Ping()` method's interface signature is not feasible, ensure that the HTTP client used within the probe has a timeout set that is shorter than the health check interval.

Quality Issues (1)

Severity	Location	Issue
🟠 Error	`gateway/dashboard_register_test.go:150-185`	The test `TestPing_SlowDashboard_RedisDown_ReportsPass` contains a logical contradiction regarding timeouts, undermining its validity. The PR description states the liveness probe's timeout is half of `liveness_check.check_duration`. The test sets `checkDuration` to 4 seconds (implying a 2-second timeout) but simulates a 2.5-second delay and asserts that the operation succeeds. This should fail with a timeout error. The test's success implies the actual timeout is longer than specified, meaning the test relies on implementation-specific timing rather than validating the documented requirement. The hardcoded duration and delay values appear to be magic numbers that make the test pass under current conditions, but do not properly verify the timeout logic. 💡 Suggestion Align the test with the documented timeout behavior. Either adjust the test to verify the `checkDuration / 2` timeout by testing delays both shorter and longer than the expected timeout and asserting success/failure accordingly, or if the timeout logic is different, update the PR description and add comments to the test to clarify the expected timing.

Powered by Visor from Probelabs

Last updated: 2026-06-17T11:58:23.860Z | Triggered by: pr_opened | Commit: 044d215

💡 TIP: You can chat with Visor using /visor ask <your question>

sonarqubecloud · 2026-06-17T12:11:37Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

probelabs Bot mentioned this pull request Jun 17, 2026

[TT-17486] fix: stop liveness probe blocking on node re-registration #8314

Merged

7 tasks

kofoworola enabled auto-merge (squash) June 17, 2026 12:42

kofoworola merged commit 4ad9813 into release-5.13 Jun 17, 2026
82 of 88 checks passed

kofoworola deleted the merge/release-5.13/9f70226dee6aac4277a6928ca10d7fcdb88455e9/TT-17486 branch June 17, 2026 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merging to release-5.13: [TT-17486] fix: stop liveness probe blocking on node re-registration (#8314)#8328

Merging to release-5.13: [TT-17486] fix: stop liveness probe blocking on node re-registration (#8314)#8328
kofoworola merged 1 commit into
release-5.13from
merge/release-5.13/9f70226dee6aac4277a6928ca10d7fcdb88455e9/TT-17486

probelabs Bot commented Jun 17, 2026 •

edited by atlassian Bot

Loading

Uh oh!

sentinelone-cnapp-eu1 Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

probelabs Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

probelabs Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented Jun 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

probelabs Bot commented Jun 17, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How This Has Been Tested

Types of changes

Checklist

Uh oh!

sentinelone-cnapp-eu1 Bot commented Jun 17, 2026

SentinelOne CNS is a cloud-agnostic, agentless CSPM & CWPP solution that continuously detects and prevents vulnerabilities that have the highest probability of being exploited in Azure, AWS, Google Cloud, and Kubernetes.

Uh oh!

github-actions Bot commented Jun 17, 2026

🚨 Jira Linter Failed

Next Steps

Uh oh!

probelabs Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Files Changed Analysis

Architecture & Impact Assessment

What this PR accomplishes

Key Technical Changes

Affected System Components

Visualization of the Fix

Scope Discovery & Context Expansion

Uh oh!

probelabs Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Architecture Check Passed

Performance Issues (1)

Quality Issues (1)

Uh oh!

sonarqubecloud Bot commented Jun 17, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

probelabs Bot commented Jun 17, 2026 •

edited by atlassian Bot

Loading

probelabs Bot commented Jun 17, 2026 •

edited

Loading

probelabs Bot commented Jun 17, 2026 •

edited

Loading