Replaces apiserver's two connect mechanisms -- a per-peer Go loop that hardcoded nmasters=1 per attempt, and a separate direct-construction path for the legacy single-host config -- with one pipeline: buildClusterConfig() parses (via task 4.1's C parser) or synthesizes a single-candidate config, validates it, then ConfigConverter (xce_cluster_config_to_client_config) produces the client config for one xce_backend_init() call. A 3-candidate config now actually exercises the C library's own failover/standby-redirect handling on initial connect, matching worker's and scheduler's task 2.1/3.1 work, instead of a Go-level per-candidate retry loop that never built a real multi-master config. Along the way: found and fixed a genuine use-after-free. The deleted tryConnect() freed its C.CString-allocated host address immediately after its single connect call returned, but xcecli_ctx_init() stores a raw pointer to the config forever (client.c:38) and every later failover rotation re-dereferences it (client_failover.c) -- so any reconnect after the first would read freed memory. Fixed by giving the cluster/client config structs package-level, process-lifetime storage (gClusterCfg/gClientCfg), mirroring worker's and scheduler's file-scope static pattern, with host/from/key written into fixed-size C arrays instead of freed heap pointers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNB4zGPCsHyDUCDJyMbiQN
288 lines
12 KiB
Go
288 lines
12 KiB
Go
//go:build cgo
|
|
|
|
package backend
|
|
|
|
import (
|
|
"os"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"go.uber.org/zap/zaptest/observer"
|
|
)
|
|
|
|
// clib_test.go exercises RegisterToBackend/buildClusterConfig (clib.go), the
|
|
// unified candidate-list connect pipeline that replaces the former
|
|
// dual-mechanism implementation (component-failover-adoption, task 4.2,
|
|
// boundary: ApiserverBackendBridge). Kept //go:build cgo since these
|
|
// functions only exist in the CGO build -- this file itself does not
|
|
// `import "C"` (a _test.go file can never do that directly, golang/go#13938):
|
|
// it only calls the plain-Go-signature functions defined in clib.go, same
|
|
// split candidates.go/candidates_test.go already established in this
|
|
// package.
|
|
//
|
|
// Test strategy:
|
|
// - Req 1.5 (no-Peers and single-entry-Peers converge on the identical
|
|
// validated xce_cluster_config_t shape) and Req 1.1-1.3 (multi-candidate
|
|
// passthrough) are exercised via buildClusterConfig() directly -- pure
|
|
// struct-building + validation, no network I/O, so these run fast and
|
|
// avoid any socket/leak concerns from a real (failing) connect attempt.
|
|
// - "a single xce_backend_init() call carries the full candidate count"
|
|
// is exercised by calling the real RegisterToBackend() against closed
|
|
// local ports (connect() returns ECONNREFUSED near-instantly, same
|
|
// "no backoff loop inside xcecli_start()'s per-master connect scan"
|
|
// property scheduler's test_scheduler_connect.c and worker's task 2.3
|
|
// test both rely on) and inspecting gClientCfg.nmasters afterward via
|
|
// currentClientConfigNMasters() -- this is deliberately the ONLY test in
|
|
// this file that reaches xce_backend_init()/a real connect() call, to
|
|
// keep the (unavoidable, pre-existing -- see status report CONCERNS)
|
|
// per-call xcecli_ctx_init() allocation this path leaks on a failed
|
|
// connect bounded to one occurrence.
|
|
// - "no-peers and single-entry-peers produce the identical connect log
|
|
// sequence" is exercised behaviorally via captureLog() (handler_test.go,
|
|
// this package), asserting on the log MESSAGE sequence (not field
|
|
// values) RegisterToBackend emits for each source.
|
|
// - The "source config structs are package-level, not stack-local"
|
|
// regression guard is a source-text scan of clib.go, same genre as
|
|
// scheduler's/worker's structural regression tests (test_scheduler_connect.c,
|
|
// test_controller_connector.c) for the identical hazard.
|
|
|
|
// testPortBase is deliberately unlikely to have a real listener bound in
|
|
// CI/dev sandboxes -- connect() against a closed local port fails fast, so
|
|
// RegisterToBackend() calls below return quickly without needing a live
|
|
// controller (same precedent as scheduler's TEST_PORT_BASE).
|
|
const testPortBase = 28581
|
|
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
// Req 1.5 -- unified candidate-list pipeline: no branch for "single host"
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
|
|
func TestBuildClusterConfig_NoPeersAndSingleEntryPeers_IdenticalShape(t *testing.T) {
|
|
// (a) No Peers: legacy Address/Port synthesized into a single candidate.
|
|
cfgNoPeers := &BackendConfig{
|
|
Address: "127.0.0.1",
|
|
Port: testPortBase,
|
|
From: "xce_apiserver_test",
|
|
Key: "keepsecret$",
|
|
}
|
|
if err := buildClusterConfig(cfgNoPeers); err != nil {
|
|
t.Fatalf("buildClusterConfig(no peers) failed: %v", err)
|
|
}
|
|
noPeersCandidates := currentClusterCandidates()
|
|
|
|
// (b) A single-entry Peers list pointed at the exact same host:port.
|
|
cfgOnePeer := &BackendConfig{
|
|
Peers: []string{"127.0.0.1:28581"},
|
|
From: "xce_apiserver_test",
|
|
Key: "keepsecret$",
|
|
}
|
|
if err := buildClusterConfig(cfgOnePeer); err != nil {
|
|
t.Fatalf("buildClusterConfig(one peer) failed: %v", err)
|
|
}
|
|
onePeerCandidates := currentClusterCandidates()
|
|
|
|
if len(noPeersCandidates) != 1 {
|
|
t.Fatalf("no-peers: got %d candidates, want 1", len(noPeersCandidates))
|
|
}
|
|
if len(onePeerCandidates) != 1 {
|
|
t.Fatalf("single-entry-peers: got %d candidates, want 1", len(onePeerCandidates))
|
|
}
|
|
if noPeersCandidates[0].Host != onePeerCandidates[0].Host ||
|
|
noPeersCandidates[0].Port != onePeerCandidates[0].Port {
|
|
t.Errorf("no-peers and single-entry-peers candidates diverge: %+v vs %+v",
|
|
noPeersCandidates[0], onePeerCandidates[0])
|
|
}
|
|
}
|
|
|
|
func TestBuildClusterConfig_MultiCandidatePassthrough(t *testing.T) {
|
|
cfg := &BackendConfig{
|
|
Peers: []string{
|
|
"127.0.0.1:28590",
|
|
"127.0.0.1:28591",
|
|
"127.0.0.1:28592",
|
|
},
|
|
From: "xce_apiserver_test",
|
|
Key: "keepsecret$",
|
|
}
|
|
if err := buildClusterConfig(cfg); err != nil {
|
|
t.Fatalf("buildClusterConfig failed: %v", err)
|
|
}
|
|
candidates := currentClusterCandidates()
|
|
if len(candidates) != 3 {
|
|
t.Fatalf("got %d candidates, want 3", len(candidates))
|
|
}
|
|
for i, want := range []int{28590, 28591, 28592} {
|
|
if candidates[i].Host != "127.0.0.1" || candidates[i].Port != want {
|
|
t.Errorf("candidate %d = %+v, want host=127.0.0.1 port=%d", i, candidates[i], want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestBuildClusterConfig_MalformedPeersRejected(t *testing.T) {
|
|
cfg := &BackendConfig{
|
|
Peers: []string{"not-a-valid-entry"},
|
|
From: "xce_apiserver_test",
|
|
Key: "keepsecret$",
|
|
}
|
|
err := buildClusterConfig(cfg)
|
|
if err == nil {
|
|
t.Fatal("expected error for malformed --peers entry, got nil")
|
|
}
|
|
if !strings.Contains(err.Error(), "not-a-valid-entry") {
|
|
t.Errorf("error %q does not identify the malformed input", err.Error())
|
|
}
|
|
}
|
|
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
// Req 1.1-1.3 -- one xce_backend_init() call carries the full candidate list
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
|
|
func TestRegisterToBackend_MultiCandidate_SingleInitCallCarriesFullCount(t *testing.T) {
|
|
captureLog(t) // RegisterToBackend logs via utils.Log; nil otherwise (utils.InitLogger is never called in unit tests)
|
|
|
|
cfg := &BackendConfig{
|
|
Peers: []string{
|
|
"127.0.0.1:28600",
|
|
"127.0.0.1:28601",
|
|
"127.0.0.1:28602",
|
|
},
|
|
From: "xce_apiserver_test",
|
|
Key: "keepsecret$",
|
|
}
|
|
|
|
start := time.Now()
|
|
err := RegisterToBackend(cfg)
|
|
elapsed := time.Since(start)
|
|
|
|
if err == nil {
|
|
t.Fatal("expected RegisterToBackend to fail against 3 closed local ports")
|
|
}
|
|
// Generous bound: closed-port ECONNREFUSED is near-instant, well under a
|
|
// few seconds even across 3 candidates -- proves this is NOT the deleted
|
|
// per-candidate Go loop silently retrying/blocking somewhere.
|
|
if elapsed > 5*time.Second {
|
|
t.Errorf("RegisterToBackend took %v against closed ports -- too slow, "+
|
|
"suggests a retry loop is still present", elapsed)
|
|
}
|
|
|
|
// The struct xce_backend_init() was actually called with must carry the
|
|
// FULL candidate count (nmasters=3), not the hardcoded 1 the deleted
|
|
// per-peer Go loop always used -- proves ONE call, not three separate
|
|
// single-candidate calls.
|
|
if got := currentClientConfigNMasters(); got != 3 {
|
|
t.Errorf("gClientCfg.nmasters = %d, want 3 (a single call must carry "+
|
|
"the full candidate count)", got)
|
|
}
|
|
}
|
|
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
// Req 5.1/5.2 -- no-peers and single-entry-peers produce the identical
|
|
// connect log sequence (shape, not values)
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
|
|
func TestRegisterToBackend_NoPeersAndSingleEntryPeers_IdenticalLogSequence(t *testing.T) {
|
|
logs := captureLog(t)
|
|
|
|
cfgNoPeers := &BackendConfig{
|
|
Address: "127.0.0.1",
|
|
Port: testPortBase + 20,
|
|
From: "xce_apiserver_test",
|
|
Key: "keepsecret$",
|
|
}
|
|
if err := RegisterToBackend(cfgNoPeers); err == nil {
|
|
t.Fatal("expected RegisterToBackend(no peers) to fail against a closed port")
|
|
}
|
|
noPeersMessages := logMessages(logs) // drains the observer buffer
|
|
|
|
cfgOnePeer := &BackendConfig{
|
|
Peers: []string{"127.0.0.1:28601"},
|
|
From: "xce_apiserver_test",
|
|
Key: "keepsecret$",
|
|
}
|
|
if err := RegisterToBackend(cfgOnePeer); err == nil {
|
|
t.Fatal("expected RegisterToBackend(single-entry peers) to fail against a closed port")
|
|
}
|
|
onePeerMessages := logMessages(logs)
|
|
|
|
if len(noPeersMessages) == 0 {
|
|
t.Fatal("expected at least one log message from RegisterToBackend(no peers)")
|
|
}
|
|
if !equalStringSlices(noPeersMessages, onePeerMessages) {
|
|
t.Errorf("log message sequence diverges between no-peers and "+
|
|
"single-entry-peers:\n no-peers: %v\n single-entry: %v",
|
|
noPeersMessages, onePeerMessages)
|
|
}
|
|
}
|
|
|
|
func logMessages(logs *observer.ObservedLogs) []string {
|
|
entries := logs.TakeAll()
|
|
msgs := make([]string, len(entries))
|
|
for i, e := range entries {
|
|
msgs[i] = e.Message
|
|
}
|
|
return msgs
|
|
}
|
|
|
|
func equalStringSlices(a, b []string) bool {
|
|
if len(a) != len(b) {
|
|
return false
|
|
}
|
|
for i := range a {
|
|
if a[i] != b[i] {
|
|
return false
|
|
}
|
|
}
|
|
return true
|
|
}
|
|
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
// Structural regression guard: source config structs are package-level
|
|
// (source-text scan of the real, current on-disk clib.go -- same genre as
|
|
// scheduler's/worker's structural regression tests for the identical
|
|
// use-after-scope hazard).
|
|
// ════════════════════════════════════════════════════════════════════════
|
|
|
|
func TestClibSource_ClusterAndClientConfigsArePackageLevel(t *testing.T) {
|
|
src, err := os.ReadFile("clib.go")
|
|
if err != nil {
|
|
t.Fatalf("could not read clib.go source for scan: %v", err)
|
|
}
|
|
text := string(src)
|
|
|
|
if !strings.Contains(text, "var gClusterCfg C.xce_cluster_config_t") {
|
|
t.Error("gClusterCfg must be declared as a package-level var " +
|
|
"(`var gClusterCfg C.xce_cluster_config_t`) -- a function-local " +
|
|
"variable would go out of scope while xce_cluster_config_to_client_config()'s " +
|
|
"aliased pointers (and xcecli_ctx_init()'s retained ctx->conf pointer) " +
|
|
"are still in use, a use-after-scope bug (see tasks.md's Implementation " +
|
|
"Notes for task 2.1)")
|
|
}
|
|
if !strings.Contains(text, "var gClientCfg C.xce_client_config_t") {
|
|
t.Error("gClientCfg must be declared as a package-level var " +
|
|
"(`var gClientCfg C.xce_client_config_t`) -- xcecli_ctx_init() " +
|
|
"stores a raw pointer to it (ctx->conf) dereferenced for the ctx's " +
|
|
"entire runtime life, not only the initial connect")
|
|
}
|
|
|
|
// Exactly one xce_backend_init() call site: the former per-peer loop
|
|
// called it once per candidate (a real loop, not a single call), and the
|
|
// former no-Peers branch duplicated it a second time in the same
|
|
// function. A single call site proves both are gone.
|
|
first := strings.Index(text, "C.xce_backend_init(")
|
|
if first < 0 {
|
|
t.Fatal("C.xce_backend_init(...) call site not found in clib.go")
|
|
}
|
|
if strings.Index(text[first+1:], "C.xce_backend_init(") >= 0 {
|
|
t.Error("clib.go must contain exactly ONE C.xce_backend_init() call site -- " +
|
|
"more than one means the deleted per-peer loop or the separate " +
|
|
"no-Peers branch is still present")
|
|
}
|
|
|
|
if strings.Contains(text, "func tryConnect(") {
|
|
t.Error("tryConnect() must be deleted entirely -- its per-call " +
|
|
"C.CString(host)/C.CString(from)/C.CString(key) + deferred C.free " +
|
|
"immediately after a single xce_backend_init() call left ctx->conf's " +
|
|
"aliased masters[]/from/key pointers dangling for the ctx's remaining " +
|
|
"runtime life (see status report CONCERNS for the precise finding)")
|
|
}
|
|
}
|