From ac962fc8331069b4230eac611d92a1b8a74067c7 Mon Sep 17 00:00:00 2001 From: pingqiu Date: Thu, 2 Apr 2026 21:12:38 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20CP13-2=20=E2=80=94=20relax=20contract=20?= =?UTF-8?q?to=20host:port,=20add=20BlockService-level=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: 1. Rename advertisedIP → advertisedHost throughout, relax contract from "always a real IP" to "routable host from -ip flag (IP or resolvable hostname)". This matches the actual -ip flag semantics which accepts both IP addresses and server names. 2. Add TestCP13_2_BlockService_AdvertisedHost_NotOpaqueID that hits the actual production wiring: BlockService with opaque localServerID + routable advertisedHost → setupReplicaReceiver → verify exported addresses use the routable host, not the opaque ID. Co-Authored-By: Claude Opus 4.6 (1M context) --- weed/command/volume.go | 2 +- weed/server/qa_block_soak_test.go | 75 +++++++++++++++++++++++++++++- weed/server/volume_server_block.go | 23 ++++----- 3 files changed, 87 insertions(+), 13 deletions(-) diff --git a/weed/command/volume.go b/weed/command/volume.go index 73dc0ec80..6cf1ff57b 100644 --- a/weed/command/volume.go +++ b/weed/command/volume.go @@ -347,7 +347,7 @@ func (v VolumeServerOptions) startVolumeServer(volumeFolders, maxVolumeCounts, v // volume server. One identity truth across VS, block service, control // bridge, and runtime ownership. blockService.SetServerID(volumeServerId) - blockService.SetAdvertisedIP(*v.ip) // CP13-2: routable IP, not opaque ID + blockService.SetAdvertisedHost(*v.ip) // CP13-2: routable host (-ip flag), not opaque ID (-id flag) volumeServer.SetBlockService(blockService) } diff --git a/weed/server/qa_block_soak_test.go b/weed/server/qa_block_soak_test.go index dcec41653..501abc9c9 100644 --- a/weed/server/qa_block_soak_test.go +++ b/weed/server/qa_block_soak_test.go @@ -84,7 +84,7 @@ func newSoakSetup(t *testing.T) *soakSetup { blockDir: filepath.Join(dir, "vs1_9333"), listenAddr: "127.0.0.1:3260", localServerID: "vs1:9333", - advertisedIP: "127.0.0.1", + advertisedHost: "127.0.0.1", v2Bridge: v2bridge.NewControlBridge(), v2Orchestrator: engine.NewRecoveryOrchestrator(), replStates: make(map[string]*volReplState), @@ -110,6 +110,79 @@ func (s *soakSetup) deliver(server string) int { return len(goAssignments) } +// --- CP13-2: BlockService-level advertisedHost wiring test --- + +func TestCP13_2_BlockService_AdvertisedHost_NotOpaqueID(t *testing.T) { + // Prove that the production wiring uses advertisedHost (routable), + // not localServerID (potentially opaque), for replica canonicalization. + // + // This test creates a BlockService with: + // localServerID = "my-opaque-id" (opaque, NOT routable) + // advertisedHost = "10.0.0.42" (routable) + // Then calls setupReplicaReceiver and verifies the exported addresses + // use the routable host, not the opaque ID. + + dir := t.TempDir() + store := storage.NewBlockVolumeStore() + + volPath := filepath.Join(dir, "cp13-2-test.blk") + vol, err := blockvol.CreateBlockVol(volPath, blockvol.CreateOptions{ + VolumeSize: 1 * 1024 * 1024, + BlockSize: 4096, + WALSize: 256 * 1024, + }) + if err != nil { + t.Fatal(err) + } + vol.Close() + if _, err := store.AddBlockVolume(volPath, ""); err != nil { + t.Fatal(err) + } + defer store.Close() + + bs := &BlockService{ + blockStore: store, + blockDir: dir, + listenAddr: "127.0.0.1:3260", + localServerID: "my-opaque-id", // NOT routable + advertisedHost: "10.0.0.42", // routable + v2Bridge: v2bridge.NewControlBridge(), + v2Orchestrator: engine.NewRecoveryOrchestrator(), + replStates: make(map[string]*volReplState), + } + + // Call the production wiring path. + bs.setupReplicaReceiver(volPath, ":0", ":0") + + // Check that exported addresses use advertisedHost, not localServerID. + bs.replMu.Lock() + state := bs.replStates[volPath] + bs.replMu.Unlock() + + if state == nil { + t.Fatal("replStates entry missing after setupReplicaReceiver") + } + + // Must contain the routable host. + if !strings.HasPrefix(state.replicaDataAddr, "10.0.0.42:") { + t.Fatalf("replicaDataAddr %q does not use advertisedHost 10.0.0.42", state.replicaDataAddr) + } + if !strings.HasPrefix(state.replicaCtrlAddr, "10.0.0.42:") { + t.Fatalf("replicaCtrlAddr %q does not use advertisedHost 10.0.0.42", state.replicaCtrlAddr) + } + + // Must NOT contain the opaque server ID. + if strings.Contains(state.replicaDataAddr, "my-opaque-id") { + t.Fatalf("replicaDataAddr %q leaked opaque localServerID", state.replicaDataAddr) + } + if strings.Contains(state.replicaCtrlAddr, "my-opaque-id") { + t.Fatalf("replicaCtrlAddr %q leaked opaque localServerID", state.replicaCtrlAddr) + } + + t.Logf("CP13-2: advertisedHost=10.0.0.42, exported data=%s ctrl=%s (opaque ID not leaked)", + state.replicaDataAddr, state.replicaCtrlAddr) +} + // --- Repeated create/failover/recover cycles with end-of-cycle truth checks --- func TestP12P2_RepeatedCycles_NoDrift(t *testing.T) { diff --git a/weed/server/volume_server_block.go b/weed/server/volume_server_block.go index 3aa8a5820..905c2bef5 100644 --- a/weed/server/volume_server_block.go +++ b/weed/server/volume_server_block.go @@ -63,10 +63,11 @@ type BlockService struct { // NOT guaranteed to be a routable address — do not use for transport endpoints. localServerID string - // advertisedIP: routable IP for this volume server (from -ip flag or auto-detected). + // advertisedHost: routable host for this volume server (from -ip flag or auto-detected). // Used by CP13-2 to canonicalize wildcard-bind replica listener addresses to - // routable ip:port. This is always a real IP, never an opaque identity string. - advertisedIP string + // routable host:port. This is the -ip value (IP or resolvable hostname), + // never an opaque server identity from -id. + advertisedHost string } // V2Orchestrator returns the V2 engine orchestrator for inspection/testing. @@ -80,11 +81,11 @@ func (bs *BlockService) SetServerID(id string) { bs.localServerID = id } -// SetAdvertisedIP sets the routable IP for replica endpoint canonicalization. -// This must be a real IP address (from -ip flag or auto-detected), never an -// opaque server identity. Called at startup from volume.go. -func (bs *BlockService) SetAdvertisedIP(ip string) { - bs.advertisedIP = ip +// SetAdvertisedHost sets the routable host for replica endpoint canonicalization. +// This is the -ip flag value (IP address or resolvable hostname), never an +// opaque server identity from -id. Called at startup from volume.go. +func (bs *BlockService) SetAdvertisedHost(host string) { + bs.advertisedHost = host } // WireStateChangeNotify sets up shipper state change callbacks on all @@ -539,10 +540,10 @@ func (bs *BlockService) setupReplicaReceiver(path, dataAddr, ctrlAddr string) { // CP13-2: Pass the routable advertisedIP (from -ip flag, NOT from -id/serverID) // so wildcard-bind listeners resolve to a real IP, not an opaque identity string. var canonDataAddr, canonCtrlAddr string - advIP := bs.advertisedIP + advHost := bs.advertisedHost if err := bs.blockStore.WithVolume(path, func(vol *blockvol.BlockVol) error { - if advIP != "" { - if err := vol.StartReplicaReceiver(dataAddr, ctrlAddr, advIP); err != nil { + if advHost != "" { + if err := vol.StartReplicaReceiver(dataAddr, ctrlAddr, advHost); err != nil { return err } } else {