manifests: stop pinning the control plane and DNS to IPv4 - #805
manifests: stop pinning the control plane and DNS to IPv4#805Yuan Gao (ygao-g) wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
a4b869c to
bc751db
Compare
| image: ko://github.com/agent-substrate/substrate/cmd/ateapi | ||
| args: | ||
| - "--grpc-listen-addr=0.0.0.0:443" | ||
| # No --grpc-listen-addr: cmd/ateapi/main.go already defaults to ":443", |
There was a problem hiding this comment.
we can just remove it, instead of adding a 4 line comment here. the commit history will point back to this PR
There was a problem hiding this comment.
Done — flag line just removed, no comment.
|
|
||
| // installDir is the manifest tree hack/install-ate.sh applies, relative to this | ||
| // package. | ||
| const installDir = "../../manifests/ate-install" |
There was a problem hiding this comment.
I don't think we need a regression test for the manifests. Better to e2e test with IPv6 Regularly.
There was a problem hiding this comment.
Done — dropped internal/installmanifests entirely (both files).
There was a problem hiding this comment.
Agreed on the e2e point, and that half is already written — it just needs to go up as its own PR, which I'll do next.
It adds an ip-family axis to the e2e-test-matrix workflow (main has no such axis today, so nothing runs against IPv6 regularly right now) plus TestDataPathServicesAreDualStack in the networking suite. That test reads the cluster's families off the node pod CIDRs and fails rather than skips when a data-path Service has fewer families than the cluster can give it — the rest of the IPv6 suite gates itself on the router Service having two ClusterIPs, so a SingleStack regression would make those tests skip and the suite would still report green.
With installmanifests gone that PR is the only thing guarding this one, so I'll prioritise it over the rest of the IPv6 queue.
Two of the three Services on the actor request path were reachable over IPv4 only, for two different reasons. ate-api-server passed "--grpc-listen-addr=0.0.0.0:443", overriding the ":443" that cmd/ateapi/main.go:63 already defaults to. The bare form binds every family the host has; the IPv4 wildcard binds one. The override dates to the initial commit and `git log -S` turns up no rationale for it, so this drops the flag rather than rewriting it. The dns and api Services carried no ipFamilyPolicy, which the API server defaults to SingleStack -- an IPv4 ClusterIP and nothing else on a dual-stack cluster. PreferDualStack rather than RequireDualStack, since Require fails Service creation outright on the single-stack clusters CI runs. api is headless, so there it governs EndpointSlice families and therefore the per-pod DNS records, not a VIP. The atenet-router Service needs the same treatment and is handled separately, alongside the Envoy listener change it is useless without. Neither invariant is unit-testable: the binary is fine either way and only the deployed cluster breaks. Coverage comes from running the e2e suite against a dual-stack cluster, which is added separately.
bc751db to
adc22e1
Compare
2 participants
Two of the three Services on the actor request path were reachable over IPv4 only, for two different reasons.
ate-api-serverpinned its listener to the IPv4 wildcard. The Deployment passed--grpc-listen-addr=0.0.0.0:443, overriding the":443"thatcmd/ateapi/main.goalready defaults to. The bare form binds every family the host has; the IPv4 wildcard binds one. The override dates to the initial commit andgit log -Sturns up no rationale for it, so this drops the flag rather than rewriting it to[::]:443.The
dnsandapiServices carried noipFamilyPolicy, which the API server defaults toSingleStack— an IPv4 ClusterIP and nothing else on a dual-stack cluster. Both now setPreferDualStack, notRequireDualStack, sinceRequirefails Service creation outright on the single-stack clusters CI runs.apiis headless, so there the policy governs EndpointSlice families and therefore the per-pod DNS records that client-side gRPC load balancing depends on, not a VIP.The
atenet-routerService needs the same treatment and is handled separately, alongside the Envoy listener change it is useless without.Testing
No unit tests here, deliberately. Neither invariant is visible to a unit test of the affected binary — the binary is fine either way and only the deployed cluster breaks. An earlier revision asserted them by parsing the shipped YAML in a test-only
internal/installmanifestspackage; that was dropped at review request in favour of exercising the real thing.Coverage comes from running the e2e suite against a dual-stack cluster, which is a separate change: it adds an
ip-familyaxis to thee2e-test-matrixworkflow and aTestDataPathServicesAreDualStackthat reads the cluster'''s families off the node pod CIDRs and fails, rather than skips, when a data-path Service has fewer families than the cluster can give it.Locally on this branch:
go build ./...,go vet, andhack/verify/{boilerplate,gofmt,go-modules}.shpass.Compatibility
No behavior change on an IPv4-only cluster:
PreferDualStackdegrades silently, and":443"and"0.0.0.0:443"bind the same socket when the host has no IPv6.