atenet: bind the Envoy listeners and admin sockets dual-stack - #2
Conversation
| admin: | ||
| address: | ||
| socket_address: { address: 0.0.0.0, port_value: 15000 } | ||
| # ipv4_compat because the ext-proc sidecar's drainer dials this over IPv4 |
There was a problem hiding this comment.
evaluate whether we need this comment.
| // existing Service rather than replacing the spec, so a Service created by an | ||
| // earlier router keeps whatever policy it was created with unless this field is | ||
| // copied too, and never gets the IPv6 ClusterIP the "::" listeners need. | ||
| func TestReconcileEnvoyService_IPFamilyPolicy(t *testing.T) { |
There was a problem hiding this comment.
Seems like a lot of redundancy in test setup, can we extend TestReconcileEnvoyConfigMap_AdminAddress instead to check additional fields
|
|
||
| addrs := l.GetAdditionalAddresses() | ||
| if len(addrs) == 0 { | ||
| t.Errorf("Expected additional addresses, got none") |
There was a problem hiding this comment.
Nit. return early here to avoid else branch
a0a0067 to
7b109d0
Compare
The HTTP and HTTPS ingress listeners bound 0.0.0.0 only, so on a dual-stack cluster Envoy answered on the router Service's IPv4 ClusterIP and nothing at all on its IPv6 one. Pair each primary socket with an additional "::" address on the same port. Ipv4Compat is false on the additional address. Setting it would clear IPV6_V6ONLY and collide with the primary IPv4 wildcard already bound to that port, and Envoy rejects the whole listener when an additional address fails to bind -- that would take down all ingress, not just the IPv6 half. Hoisting the literal into a helper keeps the two listeners from drifting. No behaviour change on an IPv4-only cluster: the primary address is untouched, and a host without IPv6 simply has no second socket to bind.
The Envoy admin socket bound 0.0.0.0, leaving it reachable over IPv4 only. It binds "::" with ipv4_compat now -- one socket for both families. ipv4_compat is required rather than incidental here: Envoy sets IPV6_V6ONLY without it, and dataplane.go health-checks the listener over http://127.0.0.1:9901/ready, so dropping it would take the dataplane component of /statusz unhealthy. The atenet-router Service carried no ipFamilyPolicy, which the API server defaults to SingleStack -- an IPv4 ClusterIP and nothing else, which leaves the listeners above with no IPv6 address to answer on. Prefer, not Require, so this stays valid on a single-stack cluster, where it is a no-op. spec.ipFamilies is deliberately left alone: the primary family is immutable and the API server appends the secondary one itself.
…families The gateway's two Envoy sockets bound 0.0.0.0, so on an IPv6-primary cluster the kubelet's startup probe against the admin port was refused and atenet-egress crashlooped while Envoy itself started fine and logged "admin address: 0.0.0.0:15000". The :443 listener had the same gap, leaving no v6 path for an actor's CONNECT. Both are single sockets, so they bind "::" with ipv4_compat rather than taking the additional-address pairing the ingress listeners use. On the admin socket ipv4_compat is load-bearing: the ext-proc sidecar's drainer reaches it at 127.0.0.1:15000 and envoydrain.go reads a refusal as "Envoy already exited", so a bare "::" would silently skip the drain. The Service gets PreferDualStack for the same reason the router's does -- without it a dual-stack cluster hands out one ClusterIP and the new v6 bind is unreachable.
697cd22 to
5308c09
Compare
|
Superseded by agent-substrate#911, which carries the same work rebased onto current upstream The rebase dropped the Tracked upstream as agent-substrate#910. |
…-substrate#1002) Fixes agent-substrate#1001 `TestK8sResolverEndpointSliceUpdates` intermittently fails at `resolver_test.go:215` — 8 of the 30 most recent failed `pr-workflow` runs (~27%): ``` updated state.Addresses = [{Addr: "10.0.0.1:443", ServerName: "", }], want [{Addr: "10.0.0.1:443", ServerName: "", } {Addr: "10.0.0.2:443", ServerName: "", }] ``` `Build` starts a goroutine that calls `updateState` once `WaitForCacheSync` returns. That report is deliberate: a service with no EndpointSlices never fires `AddFunc`, so without it the resolver would stay silent instead of telling gRPC the answer is an empty set. But `WaitForCacheSync` polls at `syncedPollPeriod = 100ms`, so it fires roughly 100ms after Build — and if the test has not yet created the second slice by then, that update still carries only `10.0.0.1` and sits in the channel ahead of the real one. The second `select` took whatever came next, so it asserted against the stale update. Locally Build-to-Create is 0.6ms, well ahead of the timer, which is why this only shows up on loaded runners. The resolver is not at fault — it promises eventual convergence, not that the first update after a change is final, and a duplicate update costs gRPC nothing. So both waits now go through one `waitForAddrs` helper that consumes updates until the set matches, with a timeout so a genuinely broken resolver still fails and reports the last set it saw. Note this changes the first wait as well: it asserted the *first* update equals `[10.0.0.1]`, and now waits for that set instead. The same argument applies there — nothing promises the first update is final. ## Verification `-count=N` proves nothing here: the unfixed test passes locally at any count because the window is never hit. A/B with the Build→Create delay as the only variable: | delay | old assertion | new assertion | |---|---|---| | 0ms | 5/5 pass | 5/5 pass | | 150ms | **0/5 pass** | **5/5 pass** | Measured timeline with the 150ms stall in place: ``` [ 0.5ms] update #1: [10.0.0.1] <- AddFunc for slice1 first select takes it ... 150ms stall ... [101.1ms] update #2: [10.0.0.1] <- the WaitForCacheSync goroutine test creates slice2 [151.4ms] update #3: [10.0.0.1, 10.0.0.2] ``` The probe tests used for this are not included. Rebased over agent-substrate#1013. That fixes a different bug — concurrent `updateState` calls letting an older address set win — and does not close this one: the two updates here are ~100ms apart, so the queue has nothing to coalesce. Re-measured on top of it, unchanged: at a 150ms delay the old assertion is 0/5 and the new one 5/5.
On an IPv6-primary cluster the kubelet probes a pod on its only address, which is
IPv6, and every Envoy socket in atenet was bound to the IPv4 wildcard.
atenet-egresscrashlooped for 14h on a v6-only kind cluster from exactly that —Envoy started fine, logged
admin address: 0.0.0.0:15000, and then:The ingress listeners had the same gap on the data path: on a dual-stack cluster
Envoy answered on the router Service's IPv4 ClusterIP and nothing at all on its
IPv6 one.
Two socket shapes, two fixes. Ingress listeners pair each
0.0.0.0primarywith an additional
::address on the same port,ipv4_compat: false— settingit would clear
IPV6_V6ONLYand collide with the primary already bound to thatport, and Envoy rejects the whole listener when an additional address fails to
bind, taking down all ingress rather than just the IPv6 half. The two admin
sockets and the egress
:443listener are single sockets, so they bind::with
ipv4_compat: true. That flag is load-bearing, not incidental:dataplane.gohealth-checks the router admin over
http://127.0.0.1:9901/ready, and the egressext-proc sidecar's drainer dials
127.0.0.1:15000, whereenvoydrain.goreads arefusal as "Envoy already exited" and skips the drain silently.
The atenet-router Service carried no
ipFamilyPolicy, which defaults toSingleStack — an IPv4 ClusterIP and nothing else, leaving the listeners above with
no IPv6 address to answer on. It becomes
PreferDualStack(notRequire, whichfails Service creation on a single-stack cluster), on both the shipped Service and
the one the router creates in managed mode.
No behaviour change on IPv4-only: the ingress primary is untouched, and a host
without IPv6 simply has no second socket to bind.
Not in scope:
dns_lookup_family: V4_ONLYon the egress dynamic forward proxy, sothe gateway now accepts IPv6 downstream but still forwards only to IPv4
destinations.
Testing
go test ./cmd/atenet/internal/router/...passes.TestReconcileEnvoyConfigMap_AdminAddresspins the generated bootstrap on boththe create and overwrite-a-stale-ConfigMap paths,
TestReconcileEnvoyService_IPFamilyPolicycovers create / SingleStack upgrade /already-dual-stack, and
TestXdsServer_UpdateSnapshot{,_WithHttps}assert the::additional address withipv4_compatfalse.atenet-egresswent from1/2 CrashLoopBackOff(64 restarts, startup probe refused) to
2/2 Runningwith 0 restarts, Envoylogging
admin address: [::]:15000.hack/install-ate-kind.sh --deploy-ate-systemcompletes on that cluster.Envoy's
/listenersthen reports all four sockets —ingress_http_listener::0.0.0.0:8080,ingress_http_listener::[::]:8080,ingress_https_listener::0.0.0.0:8443,ingress_https_listener::[::]:8443—and the router Service comes up
PreferDualStack.http://127.0.0.1:15000/readyandhttp://[::1]:15000/readyboth return 200, so the singleipv4_compatsocketstill serves the drainer's IPv4 loopback dial.
a v6-only kind cluster until the CoreDNS resolver fix lands.