volume: let evacuation proceed on a server in maintenance mode (#11145)

Maintenance mode exists to fence a volume server so it can be evacuated
without taking new writes (#7977), but the gate added in #8115 also
rejected the RPCs evacuation issues against the source: VolumeMarkReadonly
(the first step of every move, and the failure reported in #11066),
VolumeDelete (the last step), and VolumeEcShardsDelete (the last step for
EC shards). volumeServer.evacuate, volume.move and ec.balance therefore
all failed on exactly the server they were meant to drain.

Those three RPCs only remove data or restrict the server further, the same
class as DeleteCollection and the unmount RPCs that were never gated, so
they are exempted from the maintenance check in both the Go and Rust
volume servers. Everything that adds data or reopens the server for
writes (AllocateVolume, WriteNeedleBlob, BatchDelete, VolumeCopy,
ReceiveFile, EC generate/copy/rebuild, vacuum, tiering, VolumeMarkWritable)
stays blocked. A side effect is that scrub can now fence broken volumes
readonly on a server already in maintenance.

Fixes #11066

Generated with [Devin](https://devin.ai)

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

seaweed-volume/src/server/grpc_server.rs

@@ -297,18 +297,18 @@ impl VolumeGrpcService {
     }
 
     /// Shared helper matching Go's `makeVolumeReadonly(ctx, v, canDelete, persist)`.
-    /// 1. Check maintenance mode
+    /// Not gated on maintenance mode: marking a volume readonly only restricts a
-    /// 2. Notify master (readonly=true)
+    /// server that is already meant to be read-only, and it is the first step of
-    /// 3. Mark local volume readonly
+    /// moving a volume off a server under evacuation (issue #11066).
-    /// 4. Notify master again (cover heartbeat race)
+    /// 1. Notify master (readonly=true)
+    /// 2. Mark local volume readonly
+    /// 3. Notify master again (cover heartbeat race)
     async fn make_volume_readonly(
         &self,
         vid: VolumeId,
         can_delete: bool,
         persist: bool,
     ) -> Result<(), Status> {
-        self.state.check_maintenance()?;
-
         let info = {
             let store = self.state.store.read().unwrap();
             let (loc_idx, vol) = store
@@ -1020,12 +1020,14 @@ impl VolumeServer for VolumeGrpcService {
         ))
     }
 
+    /// Allowed in maintenance mode: it removes data from the server rather than
+    /// adding any, and evacuating a server in maintenance mode ends each move by
+    /// deleting the source copy (issue #11066).
     async fn volume_delete(
         &self,
         request: Request<volume_server_pb::VolumeDeleteRequest>,
     ) -> Result<Response<volume_server_pb::VolumeDeleteResponse>, Status> {
         self.check_grpc_admin_auth(&request)?;
-        self.state.check_maintenance()?;
         let req = request.into_inner();
         let vid = VolumeId(req.volume_id);
         let mut store = self.state.store.write().unwrap();
@@ -1059,7 +1061,7 @@ impl VolumeServer for VolumeGrpcService {
         self.check_grpc_admin_auth(&request)?;
         let req = request.into_inner();
         let vid = VolumeId(req.volume_id);
-        // Go: volume lookup (L239-241) happens before maintenance check (L166 in makeVolumeReadonly)
+        // Go: VolumeMarkReadonly looks the volume up before calling makeVolumeReadonly
         {
             let store = self.state.store.read().unwrap();
             store
@@ -3014,12 +3016,13 @@ impl VolumeServer for VolumeGrpcService {
         ))
     }
 
+    /// Allowed in maintenance mode: like `volume_delete` it only removes data, and
+    /// evacuating EC shards off a server in maintenance mode ends here (issue #11066).
     async fn volume_ec_shards_delete(
         &self,
         request: Request<volume_server_pb::VolumeEcShardsDeleteRequest>,
     ) -> Result<Response<volume_server_pb::VolumeEcShardsDeleteResponse>, Status> {
         self.check_grpc_admin_auth(&request)?;
-        self.state.check_maintenance()?;
         let req = request.into_inner();
         let vid = VolumeId(req.volume_id);
 

test/volume_server/grpc/admin_lifecycle_test.go

@@ -179,7 +179,9 @@ func TestAllocateDuplicateAndMountUnmountMissingVariants(t *testing.T) {
 	}
 }
 
-func TestMaintenanceModeRejectsVolumeDelete(t *testing.T) {
+// Evacuating a server in maintenance mode ends each move by deleting the source
+// volume, so VolumeDelete must stay available in that mode (issue #11066).
+func TestMaintenanceModeAllowsVolumeDelete(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping integration test in short mode")
 	}
@@ -194,22 +196,14 @@ func TestMaintenanceModeRejectsVolumeDelete(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	stateResp, err := client.GetState(ctx, &volume_server_pb.GetStateRequest{})
+	framework.EnableMaintenanceMode(t, ctx, client)
-	if err != nil {
+
-		t.Fatalf("GetState failed: %v", err)
+	if _, err := client.VolumeDelete(ctx, &volume_server_pb.VolumeDeleteRequest{VolumeId: volumeID, OnlyEmpty: true}); err != nil {
-	}
+		t.Fatalf("VolumeDelete should succeed in maintenance mode, got: %v", err)
-	_, err = client.SetState(ctx, &volume_server_pb.SetStateRequest{
-		State: &volume_server_pb.VolumeServerState{Maintenance: true, Version: stateResp.GetState().GetVersion()},
-	})
-	if err != nil {
-		t.Fatalf("SetState maintenance=true failed: %v", err)
 	}
 
-	_, err = client.VolumeDelete(ctx, &volume_server_pb.VolumeDeleteRequest{VolumeId: volumeID, OnlyEmpty: true})
+	_, err := client.VolumeStatus(ctx, &volume_server_pb.VolumeStatusRequest{VolumeId: volumeID})
 	if err == nil {
-		t.Fatalf("VolumeDelete should fail when maintenance mode is enabled")
+		t.Fatalf("VolumeStatus should fail once the volume is deleted")
-	}
-	if !strings.Contains(err.Error(), "maintenance mode") {
-		t.Fatalf("expected maintenance mode error, got: %v", err)
 	}
 }

test/volume_server/grpc/admin_readonly_collection_test.go

@@ -117,19 +117,27 @@ func TestVolumeMarkReadonlyWritableErrorPaths(t *testing.T) {
 	// enter maintenance mode
 	framework.EnableMaintenanceMode(t, ctx, grpcClient)
 
-	// existing volume in maintenance mode should return "maintenance mode" error
+	// marking readonly only restricts the server further and is the first step
+	// of evacuating it, so maintenance mode lets it through (issue #11066)
 	_, err = grpcClient.VolumeMarkReadonly(ctx, &volume_server_pb.VolumeMarkReadonlyRequest{VolumeId: volumeID, Persist: true})
-	if err == nil || !strings.Contains(err.Error(), "maintenance mode") {
+	if err != nil {
-		t.Fatalf("VolumeMarkReadonly maintenance error mismatch: %v", err)
+		t.Fatalf("VolumeMarkReadonly should succeed in maintenance mode, got: %v", err)
+	}
+	statusResp, err := grpcClient.VolumeStatus(ctx, &volume_server_pb.VolumeStatusRequest{VolumeId: volumeID})
+	if err != nil {
+		t.Fatalf("VolumeStatus after readonly in maintenance failed: %v", err)
+	}
+	if !statusResp.GetIsReadOnly() {
+		t.Fatalf("VolumeStatus expected readonly=true after VolumeMarkReadonly in maintenance mode")
 	}
 
+	// reopening a volume for writes is still refused on a read-only server
 	_, err = grpcClient.VolumeMarkWritable(ctx, &volume_server_pb.VolumeMarkWritableRequest{VolumeId: volumeID})
 	if err == nil || !strings.Contains(err.Error(), "maintenance mode") {
 		t.Fatalf("VolumeMarkWritable maintenance error mismatch: %v", err)
 	}
 
 	// non-existent volume in maintenance mode should still return "not found"
-	// (volume lookup happens before maintenance check)
 	_, err = grpcClient.VolumeMarkReadonly(ctx, &volume_server_pb.VolumeMarkReadonlyRequest{VolumeId: 98773, Persist: true})
 	if err == nil || !strings.Contains(err.Error(), "not found") {
 		t.Fatalf("VolumeMarkReadonly missing-volume in maintenance error mismatch: %v", err)

test/volume_server/grpc/erasure_coding_test.go

@@ -27,24 +27,27 @@ func TestEcMaintenanceModeRejections(t *testing.T) {
 	conn, grpcClient := framework.DialVolumeServer(t, clusterHarness.VolumeGRPCAddress())
 	defer conn.Close()
 
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	// Encode a volume before entering maintenance mode so the shard delete
+	// below has real shards to remove.
+	const ecVolumeID = uint32(1)
+	framework.AllocateVolume(t, grpcClient, ecVolumeID, "")
+	httpClient := framework.NewHTTPClient()
+	uploadResp := framework.UploadBytes(t, httpClient, clusterHarness.VolumeAdminURL(), framework.NewFileID(ecVolumeID, 990001, 0x11223344), []byte("ec-maintenance-content"))
+	_ = framework.ReadAllAndClose(t, uploadResp)
+	if uploadResp.StatusCode != http.StatusCreated {
+		t.Fatalf("upload expected 201, got %d", uploadResp.StatusCode)
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
-	stateResp, err := grpcClient.GetState(ctx, &volume_server_pb.GetStateRequest{})
+	if _, err := grpcClient.VolumeEcShardsGenerate(ctx, &volume_server_pb.VolumeEcShardsGenerateRequest{VolumeId: ecVolumeID, Collection: ""}); err != nil {
-	if err != nil {
+		t.Fatalf("VolumeEcShardsGenerate before maintenance failed: %v", err)
-		t.Fatalf("GetState failed: %v", err)
-	}
-	_, err = grpcClient.SetState(ctx, &volume_server_pb.SetStateRequest{
-		State: &volume_server_pb.VolumeServerState{
-			Maintenance: true,
-			Version:     stateResp.GetState().GetVersion(),
-		},
-	})
-	if err != nil {
-		t.Fatalf("SetState maintenance=true failed: %v", err)
 	}
 
-	_, err = grpcClient.VolumeEcShardsGenerate(ctx, &volume_server_pb.VolumeEcShardsGenerateRequest{VolumeId: 1, Collection: ""})
+	framework.EnableMaintenanceMode(t, ctx, grpcClient)
+
+	_, err := grpcClient.VolumeEcShardsGenerate(ctx, &volume_server_pb.VolumeEcShardsGenerateRequest{VolumeId: 1, Collection: ""})
 	if err == nil || !strings.Contains(err.Error(), "maintenance mode") {
 		t.Fatalf("VolumeEcShardsGenerate maintenance error mismatch: %v", err)
 	}
@@ -59,13 +62,23 @@ func TestEcMaintenanceModeRejections(t *testing.T) {
 		t.Fatalf("VolumeEcShardsCopy maintenance error mismatch: %v", err)
 	}
 
+	// Deleting shards removes data rather than adding it, and is how evacuating
+	// EC shards off a server in maintenance mode finishes (issue #11066).
 	_, err = grpcClient.VolumeEcShardsDelete(ctx, &volume_server_pb.VolumeEcShardsDeleteRequest{
-		VolumeId:   1,
+		VolumeId:   ecVolumeID,
 		Collection: "",
 		ShardIds:   []uint32{0},
 	})
-	if err == nil || !strings.Contains(err.Error(), "maintenance mode") {
+	if err != nil {
-		t.Fatalf("VolumeEcShardsDelete maintenance error mismatch: %v", err)
+		t.Fatalf("VolumeEcShardsDelete should succeed in maintenance mode, got: %v", err)
+	}
+	_, err = grpcClient.VolumeEcShardsMount(ctx, &volume_server_pb.VolumeEcShardsMountRequest{
+		VolumeId:   ecVolumeID,
+		Collection: "",
+		ShardIds:   []uint32{0},
+	})
+	if err == nil {
+		t.Fatalf("VolumeEcShardsMount should fail once shard 0 has been deleted")
 	}
 
 	_, err = grpcClient.VolumeEcBlobDelete(ctx, &volume_server_pb.VolumeEcBlobDeleteRequest{

test/volume_server/grpc/scrub_query_test.go

@@ -520,14 +520,25 @@ func TestScrubVolumeMarkBrokenReadonlyInMaintenanceMode(t *testing.T) {
 
 	framework.EnableMaintenanceMode(t, ctx, grpcClient)
 
-	// scrub with the flag in maintenance mode: makeVolumeReadonly should fail
+	// marking a volume readonly only restricts a server that is already
-	// and ScrubVolume should propagate the error
+	// read-only, so maintenance mode does not block fencing broken volumes
-	_, err := grpcClient.ScrubVolume(ctx, &volume_server_pb.ScrubVolumeRequest{
+	resp, err := grpcClient.ScrubVolume(ctx, &volume_server_pb.ScrubVolumeRequest{
 		VolumeIds:                 []uint32{volumeID},
 		Mode:                      volume_server_pb.VolumeScrubMode_INDEX,
 		MarkBrokenVolumesReadonly: true,
 	})
-	if err == nil || !strings.Contains(err.Error(), "maintenance mode") {
+	if err != nil {
-		t.Fatalf("ScrubVolume with MarkBrokenVolumesReadonly in maintenance mode error mismatch: %v", err)
+		t.Fatalf("ScrubVolume with MarkBrokenVolumesReadonly in maintenance mode failed: %v", err)
+	}
+	if len(resp.GetBrokenVolumeIds()) == 0 {
+		t.Fatalf("expected broken volume after corruption")
+	}
+
+	statusResp, err := grpcClient.VolumeStatus(ctx, &volume_server_pb.VolumeStatusRequest{VolumeId: volumeID})
+	if err != nil {
+		t.Fatalf("VolumeStatus after scrub in maintenance mode failed: %v", err)
+	}
+	if !statusResp.GetIsReadOnly() {
+		t.Fatalf("broken volume should be read-only after MarkBrokenVolumesReadonly scrub in maintenance mode")
 	}
 }

weed/server/volume_grpc_admin.go

@@ -186,6 +186,9 @@ func (vs *VolumeServer) VolumeConsolidateIndex(ctx context.Context, req *volume_
 
 }
 
+// VolumeDelete is allowed in maintenance mode: it removes data from the server
+// rather than adding any, and evacuating a server in maintenance mode ends each
+// move by deleting the source copy (issue #11066).
 func (vs *VolumeServer) VolumeDelete(ctx context.Context, req *volume_server_pb.VolumeDeleteRequest) (*volume_server_pb.VolumeDeleteResponse, error) {
 	resp := &volume_server_pb.VolumeDeleteResponse{}
 
@@ -193,10 +196,6 @@ func (vs *VolumeServer) VolumeDelete(ctx context.Context, req *volume_server_pb.
 		return resp, err
 	}
 
-	if err := vs.CheckMaintenanceMode(); err != nil {
-		return resp, err
-	}
-
 	err := vs.store.DeleteVolume(needle.VolumeId(req.VolumeId), req.OnlyEmpty, req.KeepRemoteData)
 
 	if err != nil {
@@ -271,11 +270,10 @@ func (vs *VolumeServer) VolumeConfigure(ctx context.Context, req *volume_server_
 
 }
 
+// makeVolumeReadonly is not gated on maintenance mode: marking a volume readonly
+// only restricts a server that is already meant to be read-only, and it is the
+// first step of moving a volume off a server under evacuation (issue #11066).
 func (vs *VolumeServer) makeVolumeReadonly(ctx context.Context, v *storage.Volume, canDelete bool, persist bool) error {
-	if err := vs.CheckMaintenanceMode(); err != nil {
-		return err
-	}
-
 	// step 1: stop master from redirecting traffic here
 	if err := vs.notifyMasterVolumeReadonly(ctx, v, true); err != nil {
 		return err

weed/server/volume_grpc_erasure_coding.go

@@ -464,13 +464,12 @@ func (vs *VolumeServer) VolumeEcShardsCopy(ctx context.Context, req *volume_serv
 
 // VolumeEcShardsDelete local delete the .ecx and some ec data slices if not needed
 // the shard should not be mounted before calling this.
+// Allowed in maintenance mode: like VolumeDelete it only removes data, and
+// evacuating EC shards off a server in maintenance mode ends here (issue #11066).
 func (vs *VolumeServer) VolumeEcShardsDelete(ctx context.Context, req *volume_server_pb.VolumeEcShardsDeleteRequest) (*volume_server_pb.VolumeEcShardsDeleteResponse, error) {
 	if err := vs.checkGrpcAdminAuth(ctx); err != nil {
 		return nil, err
 	}
-	if err := vs.CheckMaintenanceMode(); err != nil {
-		return nil, err
-	}
 
 	bName := erasure_coding.EcShardBaseFileName(req.Collection, int(req.VolumeId))
 

weed/server/volume_grpc_maintenance_test.go

@@ -0,0 +1,111 @@
+package weed_server
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
+	"github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb"
+	"github.com/seaweedfs/seaweedfs/weed/storage"
+	"github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding"
+	"github.com/seaweedfs/seaweedfs/weed/storage/needle"
+	"github.com/seaweedfs/seaweedfs/weed/storage/types"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// newMaintenanceModeServer builds a volume server whose store holds volume vid
+// in collection and has maintenance mode switched on.
+func newMaintenanceModeServer(t *testing.T, vid needle.VolumeId, collection string) (*VolumeServer, string) {
+	t.Helper()
+	dir := t.TempDir()
+	store := newTraversalTestStore(dir)
+	t.Cleanup(store.Close)
+	require.NoError(t, store.AddVolume(vid, collection, storage.NeedleMapInMemory, "000", "", 0, needle.GetCurrentVersion(), 0, types.HardDriveType, 0))
+	require.NoError(t, store.State.Update(&volume_server_pb.VolumeServerState{Maintenance: true}))
+
+	vs := &VolumeServer{
+		store:          store,
+		grpcDialOption: grpc.WithTransportCredentials(insecure.NewCredentials()),
+	}
+	require.True(t, vs.MaintenanceMode())
+	return vs, dir
+}
+
+type fakeReadonlyAcceptingMaster struct {
+	master_pb.UnimplementedSeaweedServer
+}
+
+func (s *fakeReadonlyAcceptingMaster) VolumeMarkReadonly(context.Context, *master_pb.VolumeMarkReadonlyRequest) (*master_pb.VolumeMarkReadonlyResponse, error) {
+	return &master_pb.VolumeMarkReadonlyResponse{}, nil
+}
+
+// Evacuating a server in maintenance mode marks each volume readonly on the
+// source, copies it, then deletes the source (issue #11066). Those source-side
+// RPCs remove data or restrict the server further, so maintenance mode must
+// let them through — otherwise the mode defeats the evacuation it exists for.
+func TestMaintenanceModeAllowsVolumeMarkReadonly(t *testing.T) {
+	vid := needle.VolumeId(1)
+	vs, _ := newMaintenanceModeServer(t, vid, "")
+	vs.setCurrentMaster(startFakeMasterServerForLeaderLookup(t, &fakeReadonlyAcceptingMaster{}))
+
+	_, err := vs.VolumeMarkReadonly(context.Background(), &volume_server_pb.VolumeMarkReadonlyRequest{VolumeId: uint32(vid)})
+	require.NoError(t, err)
+	assert.True(t, vs.store.GetVolume(vid).IsReadOnly())
+}
+
+func TestMaintenanceModeAllowsVolumeDelete(t *testing.T) {
+	vid := needle.VolumeId(2)
+	vs, _ := newMaintenanceModeServer(t, vid, "")
+
+	_, err := vs.VolumeDelete(context.Background(), &volume_server_pb.VolumeDeleteRequest{VolumeId: uint32(vid)})
+	require.NoError(t, err)
+	assert.Nil(t, vs.store.GetVolume(vid), "volume should be gone from the store")
+}
+
+func TestMaintenanceModeAllowsVolumeEcShardsDelete(t *testing.T) {
+	const collection = "ec-maint"
+	vid := needle.VolumeId(3)
+	vs, dir := newMaintenanceModeServer(t, needle.VolumeId(99), "")
+
+	base := erasure_coding.EcShardFileName(collection, dir, int(vid))
+	require.NoError(t, os.WriteFile(base+".ecx", make([]byte, 16), 0o644))
+	for _, id := range []int{0, 1} {
+		require.NoError(t, os.WriteFile(base+erasure_coding.ToExt(id), []byte("s"), 0o644))
+	}
+
+	_, err := vs.VolumeEcShardsDelete(context.Background(), &volume_server_pb.VolumeEcShardsDeleteRequest{
+		VolumeId:   uint32(vid),
+		Collection: collection,
+		ShardIds:   []uint32{0, 1},
+	})
+	require.NoError(t, err)
+	assert.False(t, util.FileExists(base+erasure_coding.ToExt(0)))
+	assert.False(t, util.FileExists(base+erasure_coding.ToExt(1)))
+}
+
+// Maintenance mode keeps rejecting RPCs that add data to the server or reopen
+// it for writes; only the removal/restriction path above is exempt.
+func TestMaintenanceModeStillBlocksWrites(t *testing.T) {
+	vid := needle.VolumeId(4)
+	vs, _ := newMaintenanceModeServer(t, vid, "")
+	wantErr := vs.CheckMaintenanceMode().Error()
+	ctx := context.Background()
+
+	_, err := vs.AllocateVolume(ctx, &volume_server_pb.AllocateVolumeRequest{VolumeId: 5, Replication: "000", DiskType: string(types.HardDriveType)})
+	assert.EqualError(t, err, wantErr, "AllocateVolume")
+
+	_, err = vs.WriteNeedleBlob(ctx, &volume_server_pb.WriteNeedleBlobRequest{VolumeId: uint32(vid), NeedleId: 1, Size: 1, NeedleBlob: []byte{0}})
+	assert.EqualError(t, err, wantErr, "WriteNeedleBlob")
+
+	_, err = vs.BatchDelete(ctx, &volume_server_pb.BatchDeleteRequest{FileIds: []string{"4,01637037d6"}})
+	assert.EqualError(t, err, wantErr, "BatchDelete")
+
+	_, err = vs.VolumeMarkWritable(ctx, &volume_server_pb.VolumeMarkWritableRequest{VolumeId: uint32(vid)})
+	assert.EqualError(t, err, wantErr, "VolumeMarkWritable")
+}