envoyproxy / envoyproxy/envoy

Panic mode routes traffic to hosts that were excluded from the panic calculation (EDS `DRAINING`)

Open
#46,800 1 comment 0 reactions 0 assignees View on GitHub
area/eds area/load balancing bug
Dominant language
C++
Stars
28.9k
Forks
5.6k
Avg merge
1d 22h
Merged PRs (30d)
430

Description

## Title

Panic mode routes traffic to hosts that were excluded from the panic calculation (EDS `DRAINING`)

---

## Body

### Description

Envoy subtracts "excluded" hosts from the panic *threshold* calculation, but once panic engages it
selects from `host_set.hosts()` — the unfiltered membership — so those same excluded hosts do
receive traffic. For `EDS_STATUS_DRAINING` this means an endpoint the control plane explicitly
marked as draining receives **new** requests.

The two decisions contradict each other: if a host is not capacity for the purpose of deciding
whether we are in panic, it should not become capacity once we are in panic.

**Code path** (paths/lines from `main` at time of writing):

1. `LoadBalancerBase::isHostSetInPanic` — excluded hosts are removed from the denominator:
```cpp
const auto host_count = host_set.hosts().size() - host_set.excludedHosts().size();
```
`source/extensions/load_balancing_policies/common/load_balancer_impl.cc`

2. `ZoneAwareLoadBalancerBase::hostSourceToUse` — on panic, selects `AllHosts`:
```cpp
if (per_priority_panic_[hosts_source.priority_]) {
stats_.lb_healthy_panic_.inc();
if (fail_traffic_on_panic_) { return std::nullopt; }
else { hosts_source.source_type_ = HostsSource::SourceType::AllHosts; return hosts_source; }
}
```

3. `ZoneAwareLoadBalancerBase::hostSourceToHosts` — `AllHosts` is the raw membership, **not**
filtered by `excludedHosts()`:
```cpp
case HostsSource::SourceType::AllHosts:
return host_set.hosts();
```

4. `excludeBasedOnHealthFlag` includes draining (`source/common/upstream/upstream_impl.cc`):
```cpp
return host.healthFlagGet(Host::HealthFlag::PENDING_ACTIVE_HC) ||
host.healthFlagGet(Host::HealthFlag::EXCLUDED_VIA_IMMEDIATE_HC_FAIL) ||
host.healthFlagGet(Host::HealthFlag::EDS_STATUS_DRAINING);
```

`ClusterImplBase::partitionHostList` builds `healthy`/`degraded`/`excluded` as derived views while
`hosts()` keeps everything, so nothing downstream of `AllHosts` re-applies the exclusion.

Envoy's own documentation of the exclusion concept says these hosts are **not meant to be routed
to** (comment on `EXCLUDED_VIA_IMMEDIATE_HC_FAIL`, `envoy/upstream/upstream.h`):

> The host should be excluded from panic, spillover, etc. calculations because it was explicitly
> taken out of rotation via protocol signal and **is not meant to be routed to.**

### Repro

1. Cluster with two EDS endpoints, `common_lb_config.healthy_panic_threshold = 50%`, round robin.
2. Endpoint **A**: normal, but failing its active health check.
3. Endpoint **B**: `health_status: DRAINING` (and passing its health check).
4. Send requests with **no** host override (no `override_host_status` match, no session cookie).

Panic engages because A is the only endpoint in the denominator and it is unhealthy
(`0 / (2 - 1) = 0% < 50%`).

**Observed:** roughly half the requests are served by the **DRAINING** endpoint B, returning 200.

```
cluster..membership_total: 2
cluster..membership_excluded: 1
cluster..membership_healthy: 0
cluster..lb_healthy_panic: 12
cluster..upstream_cx_none_healthy: 0
```
(12 requests sent; 6 reached the draining endpoint.)

**Expected:** a DRAINING endpoint should not receive new load-balanced traffic. It stays reachable
through an explicit host override (`common_lb_config.override_host_status` including `DRAINING`),
which is the documented mechanism for letting existing sessions finish.

### This is not limited to "all endpoints down"

The repro above uses zero healthy endpoints for simplicity, but that is not required. With `L`
non-excluded endpoints and `H` healthy ones, panic engages whenever `H/L` is below the configured
threshold (and below the overprovisioning cap of ~71.4%). Healthy endpoints can therefore be
available and serving while a DRAINING endpoint is *also* receiving new traffic.

Example: 4 endpoints + 1 DRAINING, `healthy_panic_threshold = 75%`, 2 healthy and 2 unhealthy.
Healthy is 50%, so panic engages and `AllHosts` spreads new requests across all five — including
the draining one — despite two healthy endpoints being available to take the load.

### Why this matters

Panic's rationale is *"most hosts look unhealthy — the health signal may be wrong, so ignore health
and try everything rather than fail."* That reasoning holds for a failed health check, which is an
inference. It does not hold for `DRAINING`, which is an explicit statement from the control plane.
A draining endpoint is not draining by mistake.

Practical consequence: a proxy answers 200 by using an endpoint the operator has already removed,
so the fact that every remaining endpoint is down produces no 5xx and no alert. If a session
affinity mechanism is in play, new clients can additionally be pinned to an endpoint that is about
to be shut down.

### Design question (not asserting a single answer)

`excludeBasedOnHealthFlag` groups three flags with different meanings:

| Flag | Meaning | Use in panic? |
|---|---|---|
| `PENDING_ACTIVE_HC` | Not yet health checked — status *unknown* | Probably **yes**; excluding these could make panic useless at startup |
| `EXCLUDED_VIA_IMMEDIATE_HC_FAIL` | Explicitly taken out of rotation via protocol signal | Probably **no** |
| `EDS_STATUS_DRAINING` | Explicit administrative removal | Probably **no** |

So "panic should skip `excludedHosts()`" may be too blunt; the narrower change is to skip only the
hosts that were *administratively* removed. Happy to prepare a PR once there is agreement on which
of the three should be excluded from panic selection.

Note `fail_traffic_on_panic` is not a workaround: it is all-or-nothing and also disables the
legitimate panic fallback when no draining hosts are present.

### Version

Observed on 1.38.3. The four code paths above are unchanged on `main` as of 2026-08-19.

Contributor guide

Open the contributing guide

Research direction

Start with LoadBalancerBase::isHostSetInPanic and ZoneAwareLoadBalancerBase::hostSourceToUse/hostSourceToHosts in source/extensions/load_balancing_policies/common/load_balancer_impl.cc, then inspect excludeBasedOnHealthFlag in source/common/upstream/upstream_impl.cc and the exclusion comment in envoy/upstream/upstream.h. Clarify which flags should remain eligible during panic, especially EDS_STATUS_DRAINING, and validate that draining endpoints receive no new traffic except through the documented host override.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, networking
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.