heal: Report bucket healing result correctly (minio#20721)

klauspost · Dec 3, 2024 · eddbe6b · eddbe6b
1 parent 734d1e3
commit eddbe6b
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 16 deletions.
diff --git a/cmd/peer-rest-server.go b/cmd/peer-rest-server.go
@@ -56,6 +56,7 @@ var (
 	aoBucketInfo           = grid.NewArrayOf[*BucketInfo](func() *BucketInfo { return &BucketInfo{} })
 	aoMetricsGroup         = grid.NewArrayOf[*MetricV2](func() *MetricV2 { return &MetricV2{} })
 	madminBgHealState      = grid.NewJSONPool[madmin.BgHealState]()
+	madminHealResultItem   = grid.NewJSONPool[madmin.HealResultItem]()
 	madminCPUs             = grid.NewJSONPool[madmin.CPUs]()
 	madminMemInfo          = grid.NewJSONPool[madmin.MemInfo]()
 	madminNetInfo          = grid.NewJSONPool[madmin.NetInfo]()
@@ -97,7 +98,7 @@ var (
 	getSysErrorsRPC                = grid.NewSingleHandler[*grid.MSS, *grid.JSON[madmin.SysErrors]](grid.HandlerGetSysErrors, grid.NewMSS, madminSysErrors.NewJSON)
 	getSysServicesRPC              = grid.NewSingleHandler[*grid.MSS, *grid.JSON[madmin.SysServices]](grid.HandlerGetSysServices, grid.NewMSS, madminSysServices.NewJSON)
 	headBucketRPC                  = grid.NewSingleHandler[*grid.MSS, *VolInfo](grid.HandlerHeadBucket, grid.NewMSS, func() *VolInfo { return &VolInfo{} })
-	healBucketRPC                  = grid.NewSingleHandler[*grid.MSS, grid.NoPayload](grid.HandlerHealBucket, grid.NewMSS, grid.NewNoPayload)
+	healBucketRPC                  = grid.NewSingleHandler[*grid.MSS, *grid.JSON[madmin.HealResultItem]](grid.HandlerHealBucket, grid.NewMSS, madminHealResultItem.NewJSON)
 	listBucketsRPC                 = grid.NewSingleHandler[*BucketOptions, *grid.Array[*BucketInfo]](grid.HandlerListBuckets, func() *BucketOptions { return &BucketOptions{} }, aoBucketInfo.New)
 	loadBucketMetadataRPC          = grid.NewSingleHandler[*grid.MSS, grid.NoPayload](grid.HandlerLoadBucketMetadata, grid.NewMSS, grid.NewNoPayload).IgnoreNilConn()
 	loadGroupRPC                   = grid.NewSingleHandler[*grid.MSS, grid.NoPayload](grid.HandlerLoadGroup, grid.NewMSS, grid.NewNoPayload)
@@ -1258,21 +1259,21 @@ func (s *peerRESTServer) NetSpeedTestHandler(w http.ResponseWriter, r *http.Requ
 	peersLogIf(r.Context(), gob.NewEncoder(w).Encode(result))
 }
 
-func (s *peerRESTServer) HealBucketHandler(mss *grid.MSS) (np grid.NoPayload, nerr *grid.RemoteErr) {
+func (s *peerRESTServer) HealBucketHandler(mss *grid.MSS) (np *grid.JSON[madmin.HealResultItem], nerr *grid.RemoteErr) {
 	bucket := mss.Get(peerS3Bucket)
 	if isMinioMetaBucket(bucket) {
 		return np, grid.NewRemoteErr(errInvalidArgument)
 	}
 
 	bucketDeleted := mss.Get(peerS3BucketDeleted) == "true"
-	_, err := healBucketLocal(context.Background(), bucket, madmin.HealOpts{
+	res, err := healBucketLocal(context.Background(), bucket, madmin.HealOpts{
 		Remove: bucketDeleted,
 	})
 	if err != nil {
 		return np, grid.NewRemoteErr(err)
 	}
 
-	return np, nil
+	return madminHealResultItem.NewJSONWith(&res), nil
 }
 
 func (s *peerRESTServer) ListBucketsHandler(opts *BucketOptions) (*grid.Array[*BucketInfo], *grid.RemoteErr) {

diff --git a/cmd/peer-s3-client.go b/cmd/peer-s3-client.go
@@ -178,13 +178,24 @@ func (sys *S3PeerSys) HealBucket(ctx context.Context, bucket string, opts madmin
 		}
 	}
 
+	if healBucketErr := reduceWriteQuorumErrs(ctx, errs, bucketOpIgnoredErrs, len(errs)/2+1); healBucketErr != nil {
+		return madmin.HealResultItem{}, toObjectErr(healBucketErr, bucket)
+	}
+
+	res := madmin.HealResultItem{
+		Type:     madmin.HealItemBucket,
+		Bucket:   bucket,
+		SetCount: -1, // explicitly set an invalid value -1, for bucket heal scenario
+	}
+
 	for i, err := range errs {
 		if err == nil {
-			return healBucketResults[i], nil
+			res.Before.Drives = append(res.Before.Drives, healBucketResults[i].Before.Drives...)
+			res.After.Drives = append(res.After.Drives, healBucketResults[i].After.Drives...)
 		}
 	}
 
-	return madmin.HealResultItem{}, toObjectErr(errVolumeNotFound, bucket)
+	return res, nil
 }
 
 // ListBuckets lists buckets across all nodes and returns a consistent view:
@@ -355,14 +366,8 @@ func (client *remotePeerS3Client) HealBucket(ctx context.Context, bucket string,
 	ctx, cancel := context.WithTimeout(ctx, globalDriveConfig.GetMaxTimeout())
 	defer cancel()
 
-	_, err := healBucketRPC.Call(ctx, conn, mss)
-
-	// Initialize heal result info
-	return madmin.HealResultItem{
-		Type:     madmin.HealItemBucket,
-		Bucket:   bucket,
-		SetCount: -1, // explicitly set an invalid value -1, for bucket heal scenario
-	}, toStorageErr(err)
+	resp, err := healBucketRPC.Call(ctx, conn, mss)
+	return resp.ValueOrZero(), toStorageErr(err)
 }
 
 // GetBucketInfo returns bucket stat info from a peer

diff --git a/cmd/peer-s3-server.go b/cmd/peer-s3-server.go
@@ -101,7 +101,7 @@ func healBucketLocal(ctx context.Context, bucket string, opts madmin.HealOpts) (
 	for i := range beforeState {
 		res.Before.Drives = append(res.Before.Drives, madmin.HealDriveInfo{
 			UUID:     "",
-			Endpoint: localDrives[i].String(),
+			Endpoint: localDrives[i].Endpoint().String(),
 			State:    beforeState[i],
 		})
 	}
@@ -149,7 +149,7 @@ func healBucketLocal(ctx context.Context, bucket string, opts madmin.HealOpts) (
 	for i := range afterState {
 		res.After.Drives = append(res.After.Drives, madmin.HealDriveInfo{
 			UUID:     "",
-			Endpoint: localDrives[i].String(),
+			Endpoint: localDrives[i].Endpoint().String(),
 			State:    afterState[i],
 		})
 	}