feat(sandbox-manager): add network interface binding and reliable peer discovery - #920
Open
AiRanthem wants to merge 5 commits into
Open
Conversation
…r discovery - Introduce optional --network-interface flag to bind Sandbox Manager to one user network IPv4 address - Validate interface existence, status, and single global unicast IPv4 address on startup; fail if invalid - Use resolved address uniformly for control API, peer route service, memberlist binding/advertisement, and non-Sandbox proxy return - Preserve existing non-hosted behavior when flag is empty, using POD_IP or first non-loopback address - Reuse existing controller-runtime cache scoped by namespace and peer label selector for peer discovery; avoid separate client/cache - Establish trusted seed addresses from cached Pods with filtering on memberlist-url annotation and duplicates - Implement background, non-blocking lifecycle for peer discovery and joining with retries and network deadline aware shutdown - Enforce one lifecycle owner for graceful Leave and Shutdown on stop requests; prevent concurrent cleanup races - Ensure startup fails on invalid network scope/configuration before serving requests; seed discovery not a startup blocker - Maintain compatibility by adding only CLI flag without CRD, HTTP model, or protocol changes and preserving existing user cluster config - Document risks including resource use for Pod informer, delayed join cancellation, stale member presence on forced termination, and address change requiring restart Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
… discovery - Add --network-interface flag to specify network interface for user-cluster traffic - Resolve and validate the network interface IP address at startup - Introduce --disable-envoy-ext-proc flag to disable Envoy ext-proc gRPC listener - Pass resolved bind address and ext-proc disable flag to SandboxManagerOptions - Update sandbox controller Run method to accept stop channel for graceful shutdown - Refactor peer discovery to use a live, uncached Kubernetes client with namespace and label selector - Implement bounded Pod listing with retries until joining memberlist succeeds - Ensure peer discovery stops retrying after successful join to avoid cache overhead - Add lifecycle management for peer discovery and memberlist join with once-only stop semantics - Enhance memberlist peers start/stop logic with context handling and IP binding - Add comprehensive unit tests covering start/stop, join lifecycle, peer leave, seed address trust, retry behavior, and namespace/label filtering in peer discovery - Update proposal documents to reflect design and implementation details for peer discovery and network interface usage Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
- Add detailed join seed recording in fakeMemberlistHandle for better verification - Refactor peerlist tests to assert joined seeds with namespace and label selector filtering - Simplify and enhance synchronization in join retry and join order tests using atomic counters - Add startBlockedJoin helper to test Join call blocking and graceful Stop handling - Replace deprecated test patterns with improved concurrency control and clearer assertions test(ext_proc): consolidate server run lifecycle tests - Replace separate tests with table-driven TestServerRunLifecycle covering - synchronous bind failures for grpc listener - normal run and stop releasing both listeners - skipping grpc listener when ext-proc disabled - stopping before run prevents late start - Verify listener release after stop in all scenarios test(sandbox-manager): ensure cache start before peers and pass bind address - Merge tests to check cache starts before peers and peers receive bind address and port - Remove redundant assertions and simplify setup test(servers/e2b): add newStartupController helper and improve startup signal handling tests - Extract newStartupController to enable testing controller start behavior with custom run functions - Adjust TestControllerSignalDuringStartupExitsCleanly to use new helper and verify expected zero exit semantics - Update TestControllerRunPropagatesStartupFailure to use newStartupController and check synchronous startup error surfacing test(utils/network): combine ResolveNetworkInterfaceAddress tests - Unify empty and missing interface tests into a table-driven test for ResolveNetworkInterfaceAddress - Confirm error messages and returned addresses for different interface names correctly Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #920 +/- ##
==========================================
+ Coverage 83.08% 83.30% +0.21%
==========================================
Files 257 259 +2
Lines 22536 22691 +155
==========================================
+ Hits 18724 18902 +178
+ Misses 3097 3056 -41
- Partials 715 733 +18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
4 tasks
- Use atomic bool flag to mark memberlist start completion - Gate GetPeers, GetAllMembers, LocalAddr, LocalPort on started flag instead of list nil-check - Add test to simulate concurrent getters racing with Start - Publish list before the started flag is set, so getters never observe a nil list - Prevent nil dereference and stale reads on memberlist data before start completion - Keep a single English proposal (drop the CN copy, rename the EN file) and align the --network-interface flag description Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
…proposal Address review findings on the network-interface peer discovery branch. Proposal: - Document the startup crash model: a termination signal during startup exits without graceful cleanup, while a signal observed together with a completed startup runs the graceful Stop path. - Document Start exactly once and Stop at most once for the peer lifecycle instead of idempotent repeated Stop. - Bring --disable-envoy-ext-proc into scope with its contract. - Exclude terminal-phase Pods from the seed set; describe single-replica listing behavior, the Gateway PEER_NAMESPACE/PEER_LABEL_SELECTOR requirement, and where the peer client and interface resolution live. Code: - peers: drop the speculative Stop-before-Start guard and reject a second Start instead; share the lifecycle tail between Start and tests; wait a full retry interval after each attempt; log an empty seed set; always advertise the bound address; skip Succeeded/Failed Pods. - proxy: drop the stopped flag and the redundant listener fields; the servers own their listeners. - sandbox-manager: start the proxy before memberlist so the peer route listener serves before this replica is advertised; replace the elector runOnce with an atomic claim so Stop keeps honoring its context. - e2b: prefer an already-available startup result when a signal arrives so a fully started controller shuts down gracefully. - Makefile: run test packages serially while several test binaries bind the fixed route-refresh and ext-proc ports. Tests: table-driven seed selection, shared lifecycle wiring, startup ordering against an occupied port, awaitStartup both-ready rows, and the controller startup tests moved to run_test.go. Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
AiRanthem
force-pushed
the
feature/network-interface-peer-discovery-260821
branch
from
September 4, 2026 11:03
534bed2 to
6921fc6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ⅰ. Describe what this PR does
This PR adds an optional
--network-interfaceflag for Sandbox Manager and uses its validated IPv4 address consistently for the control API, peer route listener, memberlist bind/advertise address, and non-Sandbox proxy return path.It also makes peer discovery resilient to startup ordering and temporary Kubernetes API failures:
The design and lifecycle contract are documented in the accompanying English and Chinese proposals.
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Describe how to verify it
go test -mod=mod ./pkg/peers ./pkg/proxy ./pkg/sandbox-gateway/server ./pkg/sandbox-manager ./pkg/servers/e2b ./pkg/utils/network -count=1go build -mod=mod ./cmd/sandbox-managergit diff --check upstream/master...HEADⅣ. Special notes for reviews
--network-interfacepreserves the existing non-hosted behavior.