fix(heal): send the dry-run flag to remote bucket heal

remotePeerS3Client.HealBucket sent only the bucket name and the remove
flag, and HealBucketHandler built its heal options from those two
values. During a dry-run bucket heal every remote node therefore ran a
real one: it created the bucket directory on drives that lacked it, or
removed empty bucket directories when the bucket was missing from
every pool. Only the coordinating node honored DryRun.

Add a "dry-run" key to the peer message and read it into
HealOpts.DryRun on the receiving side, so healBucketLocal returns
before MakeVol and DeleteVol on every node. The message is a string
map: an older node ignores the key, and a missing key reads as false,
so a mixed-version cluster behaves as before until every node is
upgraded.

The regression calls the real client over a test grid connection to
the real handler. It fails on main, and it fails if either side of the
change is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Feng Ruohang <rh@vonng.com>
This commit is contained in:
Feng Ruohang
2026-11-05 01:21:13 +08:00
parent c8dd5678bb
commit dd0a2da55e
4 changed files with 121 additions and 4 deletions
+2
View File
@@ -1291,8 +1291,10 @@ func (s *peerRESTServer) HealBucketHandler(mss *grid.MSS) (np *grid.JSON[madmin.
}
bucketDeleted := mss.Get(peerS3BucketDeleted) == "true"
dryRun := mss.Get(peerS3BucketDryRun) == "true"
res, err := healBucketLocal(context.Background(), bucket, madmin.HealOpts{
Remove: bucketDeleted,
DryRun: dryRun,
})
if err != nil {
return np, grid.NewRemoteErr(err)
+11 -4
View File
@@ -354,10 +354,7 @@ func (client *remotePeerS3Client) HealBucket(ctx context.Context, bucket string,
return madmin.HealResultItem{}, nil
}
mss := grid.NewMSSWith(map[string]string{
peerS3Bucket: bucket,
peerS3BucketDeleted: strconv.FormatBool(opts.Remove),
})
mss := healBucketPeerMSS(bucket, opts)
ctx, cancel := context.WithTimeout(ctx, globalDriveConfig.GetMaxTimeout())
defer cancel()
@@ -366,6 +363,16 @@ func (client *remotePeerS3Client) HealBucket(ctx context.Context, bucket string,
return resp.ValueOrZero(), toStorageErr(err)
}
// healBucketPeerMSS encodes the bucket heal options a peer applies to its
// local drives. Peers that predate the dry-run key ignore it.
func healBucketPeerMSS(bucket string, opts madmin.HealOpts) *grid.MSS {
return grid.NewMSSWith(map[string]string{
peerS3Bucket: bucket,
peerS3BucketDeleted: strconv.FormatBool(opts.Remove),
peerS3BucketDryRun: strconv.FormatBool(opts.DryRun),
})
}
// GetBucketInfo returns bucket stat info from a peer
func (client *remotePeerS3Client) GetBucketInfo(ctx context.Context, bucket string, opts BucketOptions) (BucketInfo, error) {
conn := client.gridConn()
+107
View File
@@ -0,0 +1,107 @@
// Copyright (c) 2026 Feng Ruohang
//
// This file is part of Silo Object Storage stack
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.
package cmd
import (
"os"
"path/filepath"
"testing"
"github.com/minio/madmin-go/v3"
"github.com/minio/minio/internal/grid"
)
// A remote node must apply the caller's dry run to its local bucket heal.
// The request goes from remotePeerS3Client.HealBucket over a grid connection
// to the peer handler, as it does between the nodes of a deployment.
func TestPeerHealBucketDryRun(t *testing.T) {
obj, roots, err := prepareErasure(t.Context(), 4)
if err != nil {
t.Fatal(err)
}
setObjectLayer(obj)
defer removeRoots(roots)
defer obj.Shutdown(t.Context())
tg, err := grid.SetupTestGrid(2)
if err != nil {
t.Fatal(err)
}
defer tg.Cleanup()
if err = healBucketRPC.Register(tg.Managers[1], (&peerRESTServer{}).HealBucketHandler); err != nil {
t.Fatal(err)
}
conn := func() *grid.Connection { return tg.Managers[0].Connection(tg.Hosts[1]) }
client := &remotePeerS3Client{gridConn: conn}
const bucket = "peer-heal-dry-run"
if err = obj.MakeBucket(t.Context(), bucket, MakeBucketOptions{}); err != nil {
t.Fatal(err)
}
dirs := make([]string, len(roots))
for i, root := range roots {
dirs[i] = filepath.Join(root, bucket)
}
if err = os.Remove(dirs[0]); err != nil {
t.Fatal(err)
}
exists := func(dir string) bool {
_, err := os.Stat(dir)
return err == nil
}
heal := func(opts madmin.HealOpts) {
t.Helper()
if _, err := client.HealBucket(t.Context(), bucket, opts); err != nil {
t.Fatal(err)
}
}
// Dry run must not recreate the missing bucket directory.
heal(madmin.HealOpts{DryRun: true})
if exists(dirs[0]) {
t.Fatal("dry-run peer bucket heal created a missing bucket directory")
}
// Dry run must not remove the empty bucket directories.
heal(madmin.HealOpts{DryRun: true, Remove: true})
for _, dir := range dirs[1:] {
if !exists(dir) {
t.Fatalf("dry-run peer bucket heal removed %s", dir)
}
}
// Controls. A message without the dry-run key, as sent by an older node,
// still heals: the fixture reaches MakeVol.
if _, err = healBucketRPC.Call(t.Context(), conn(), grid.NewMSSWith(map[string]string{
peerS3Bucket: bucket,
peerS3BucketDeleted: "false",
})); err != nil {
t.Fatal(err)
}
if !exists(dirs[0]) {
t.Fatal("control: peer bucket heal did not recreate the missing directory")
}
// A real heal with Remove reaches DeleteVol for the empty directories.
heal(madmin.HealOpts{Remove: true})
for _, dir := range dirs {
if exists(dir) {
t.Fatalf("control: peer bucket heal kept empty directory %s", dir)
}
}
}
+1
View File
@@ -29,6 +29,7 @@ import (
const (
peerS3Bucket = "bucket"
peerS3BucketDeleted = "bucket-deleted"
peerS3BucketDryRun = "dry-run"
peerS3BucketForceCreate = "force-create"
peerS3BucketForceDelete = "force-delete"
)