worker: balance pre-move check compares addresses in http form (#10360)

The pre-move stale check compared the master's location urls (host:port)
literally against the proposal's node addresses, which carry the grpc
suffix (host:port.grpcPort). Every move failed as "stale move: volume no
longer on source" even though the volume was still there. Normalize both
sides through pb.ServerAddress before comparing.
This commit is contained in:
Chris Lu
2026-07-18 13:52:34 -07:00
committed by GitHub
parent 564803becd
commit fba01ae2dd
2 changed files with 21 additions and 11 deletions
+9 -3
View File
@@ -12,6 +12,7 @@ import (
"github.com/seaweedfs/seaweedfs/weed/admin/topology"
"github.com/seaweedfs/seaweedfs/weed/glog"
"github.com/seaweedfs/seaweedfs/weed/pb"
"github.com/seaweedfs/seaweedfs/weed/pb/plugin_pb"
"github.com/seaweedfs/seaweedfs/weed/pb/worker_pb"
pluginworker "github.com/seaweedfs/seaweedfs/weed/plugin/worker"
@@ -790,12 +791,17 @@ func (h *VolumeBalanceHandler) checkMoveStillValid(ctx context.Context, masterAd
}
func checkMovePreconditions(locations []string, volumeID uint32, sourceNode, targetNode string) error {
// Proposal nodes carry the grpc suffix (host:port.grpcPort) while the
// master reports plain host:port urls, so compare in http form.
sourceAddress := pb.ServerAddress(strings.TrimSpace(sourceNode))
targetAddress := pb.ServerAddress(strings.TrimSpace(targetNode))
sourceFound := false
for _, location := range locations {
switch strings.TrimSpace(location) {
case targetNode:
locationAddress := pb.ServerAddress(strings.TrimSpace(location))
switch {
case locationAddress.Equals(targetAddress):
return fmt.Errorf("stale move: volume %d already has a replica on target %s", volumeID, targetNode)
case sourceNode:
case locationAddress.Equals(sourceAddress):
sourceFound = true
}
}
@@ -798,18 +798,22 @@ func (r *recordingDetectionSender) SendActivity(event *plugin_pb.ActivityEvent)
func TestCheckMovePreconditions(t *testing.T) {
tests := []struct {
name string
locations []string
wantErr string
name string
locations []string
sourceNode string
targetNode string
wantErr string
}{
{"valid move", []string{"10.0.0.1:8080", "10.0.0.3:8080"}, ""},
{"volume left the source", []string{"10.0.0.3:8080"}, "no longer on source"},
{"volume gone entirely", nil, "no longer on source"},
{"target already has a replica", []string{"10.0.0.1:8080", "10.0.0.2:8080"}, "already has a replica on target"},
{"valid move", []string{"10.0.0.1:8080", "10.0.0.3:8080"}, "10.0.0.1:8080", "10.0.0.2:8080", ""},
{"volume left the source", []string{"10.0.0.3:8080"}, "10.0.0.1:8080", "10.0.0.2:8080", "no longer on source"},
{"volume gone entirely", nil, "10.0.0.1:8080", "10.0.0.2:8080", "no longer on source"},
{"target already has a replica", []string{"10.0.0.1:8080", "10.0.0.2:8080"}, "10.0.0.1:8080", "10.0.0.2:8080", "already has a replica on target"},
{"proposal nodes carry grpc suffix", []string{"10.0.0.1:8080", "10.0.0.3:8080"}, "10.0.0.1:8080.18080", "10.0.0.2:8080.18080", ""},
{"grpc-suffixed target already has a replica", []string{"10.0.0.1:8080", "10.0.0.2:8080"}, "10.0.0.1:8080.18080", "10.0.0.2:8080.18080", "already has a replica on target"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := checkMovePreconditions(tt.locations, 5, "10.0.0.1:8080", "10.0.0.2:8080")
err := checkMovePreconditions(tt.locations, 5, tt.sourceNode, tt.targetNode)
if tt.wantErr == "" {
if err != nil {
t.Fatalf("unexpected error: %v", err)