Repository navigation
Conversation
Add a DualStack field to ServerConfigs that binds server processes to the IPv6 wildcard address instead of 0.0.0.0, so they also accept IPv4 clients on dual-stack or IPv6-only clusters. Online and offline servers use the bracketed [::] form required by gunicorn and Arrow Flight; ui, lineage, and registry keep their existing behavior or use the bare :: form uvicorn expects. The registry server always binds dual-stack and ignores this setting. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
Mutation testing found survivors in withBindHost's loop bounds: an -h flag as the very last argument (no value to replace) and one as the first argument were both untested edge cases. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6887 +/- ##
==========================================
- Coverage 49.02% 48.63% -0.39%
==========================================
Files 433 427 -6
Lines 54308 53855 -453
Branches 7910 7846 -64
==========================================
- Hits 26625 26195 -430
+ Misses 25799 25792 -7
+ Partials 1884 1868 -16
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
The registry's REST server binds dual-stack by default in the SDK CLI. Render -h 0.0.0.0 only when the shared DualStack field is explicitly set to false, so nil/true keep today's dual-stack default instead of silently going IPv4-only. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
Only the generated_at metadata field diverged from master, which was enough for GitHub to report this PR as unmergeable and apparently skip queuing CI. No scan content changed. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
|
@dbbvitor I see asymmetry in default behavior, which is confusing:
I think it's better if one field, one meaning, everywhere. |
Make unset or false dualStack render an explicit 0.0.0.0 host flag. Only dualStack=true selects the bare IPv6 wildcard, aligning registry REST defaults with other host-flag servers. Signed-off-by: dbbvitor <vitor.diniz@gympass.com>
40d8d19 to
c2267df
Compare
Agreed. I've changed the code to remove the asymmetry in the design here: c2267df |
What this PR does / why we need it:
Merge after #6886.
FeastServicesrenders every service container's command with a hardcoded IPv4 host flag (-h 0.0.0.0for online/offline, or the ui/lineage servers' equivalent), with no CRD field to change it. On an IPv6-only or dual-stack cluster, none of these servers bind an address that anything can reach; the Operator half of the same gap #6886 already fixed on the SDK side.This adds a
DualStack *boolfield to [ServerConfigs] https://github.com/dbbvitor/feast/blob/feat/operator-dual-stack/infra/feast-operator/api/v1/featurestore_types.go#L904). When set, a newwithBindHost()helper rewrites the rendered-hargument pair to the IPv6 wildcard address instead of leaving the hardcoded IPv4 literal:"[::]"), both reject a bare"::"as a host argument."::"instead.--hostflag at all and is left untouched; it already always binds dual-stack.withBindHost()is applied both insidegetContainerCommand(), which builds each service Deployment's container args, and insetLineageDeployment(), which builds the lineage container'sCommandslice directly rather than going through the sharedArgspath.Which issue(s) this PR fixes:
Part of #6862 (together with #6886 on the SDK side; don't let this auto-close the issue on merge; #6886 should merge first or alongside, since
dualStackdoesn't fully work for ui/lineage without it)Checks
git commit -s)Testing Strategy
Misc
DualStackis opt-in (nil/false preserves today's0.0.0.0behavior exactly), no change for clusters that don't set it.-h ::for the ui/lineage (uvicorn-based) servers only works, end-to-end because fix: Bind metrics, REST registry, ui, and lineage servers dual-stack #6886 now also fixesui_server.py'sstart_server()andlineage_server.py'sstart_lineage_server(): both previously calleduvicorn.run(host=host, ...)directly, which setsIPV6_V6ONLY=1via asyncio'sloop.create_server()and would have made those two servers IPv6-only under this option, a silent IPv4 regression, not the dual-stack fix this PR promises.