add more dangling heal related tests (#13140)

also make sure that HealObject() never returns
'ObjectNotFound' or 'VersionNotFound' errors,
as those are meaningless and not useful for the
caller.

cmd/admin-heal-ops.go

@@ -720,14 +720,6 @@ func (h *healSequence) queueHealTask(source healSource, healType madmin.HealItem
 			if errors.Is(res.err, errSkipFile) { // this is only sent usually by nopHeal
 				return nil
 			}
-			// Object might have been deleted, by the time heal
-			// was attempted, we should ignore this object and
-			// return the error and not calculate this object
-			// as part of the metrics.
-			if isErrObjectNotFound(res.err) || isErrVersionNotFound(res.err) {
-				// Return the error so that caller can handle it.
-				return res.err
-			}
 
 			h.mutex.Lock()
 			defer h.mutex.Unlock()
@@ -751,11 +743,6 @@ func (h *healSequence) queueHealTask(source healSource, healType madmin.HealItem
 		}
 		res.result.Type = healType
 		if res.err != nil {
-			// Object might have been deleted, by the time heal
-			// was attempted, we should ignore this object and return success.
-			if isErrObjectNotFound(res.err) || isErrVersionNotFound(res.err) {
-				return nil
-			}
 			// Only report object error
 			if healType != madmin.HealItemObject {
 				return res.err
@@ -812,11 +799,6 @@ func (h *healSequence) healMinioSysMeta(objAPI ObjectLayer, metaPrefix string) f
 				object:    object,
 				versionID: versionID,
 			}, madmin.HealItemBucketMetadata)
-			// Object might have been deleted, by the time heal
-			// was attempted we ignore this object an move on.
-			if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
-				return nil
-			}
 			return err
 		})
 	}
@@ -865,9 +847,7 @@ func (h *healSequence) healBuckets(objAPI ObjectLayer, bucketsOnly bool) error {
 // healBucket - traverses and heals given bucket
 func (h *healSequence) healBucket(objAPI ObjectLayer, bucket string, bucketsOnly bool) error {
 	if err := h.queueHealTask(healSource{bucket: bucket}, madmin.HealItemBucket); err != nil {
-		if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+		return err
-			return err
-		}
 	}
 
 	if bucketsOnly {
@@ -881,9 +861,6 @@ func (h *healSequence) healBucket(objAPI ObjectLayer, bucket string, bucketsOnly
 			oi, err := objAPI.GetObjectInfo(h.ctx, bucket, h.object, ObjectOptions{})
 			if err == nil {
 				if err = h.healObject(bucket, h.object, oi.VersionID); err != nil {
-					if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
-						return nil
-					}
 					return err
 				}
 			}
@@ -893,11 +870,7 @@ func (h *healSequence) healBucket(objAPI ObjectLayer, bucket string, bucketsOnly
 	}
 
 	if err := objAPI.HealObjects(h.ctx, bucket, h.object, h.settings, h.healObject); err != nil {
-		// Object might have been deleted, by the time heal
+		return errFnHealFromAPIErr(h.ctx, err)
-		// was attempted we ignore this object an move on.
-		if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
-			return errFnHealFromAPIErr(h.ctx, err)
-		}
 	}
 	return nil
 }

cmd/data-scanner.go

@@ -699,9 +699,7 @@ func (f *folderScanner) scanFolder(ctx context.Context, folder cachedFolder, int
 							object:    entry.name,
 							versionID: "",
 						}, madmin.HealItemObject)
-						if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+						logger.LogIf(ctx, err)
-							logger.LogIf(ctx, err)
-						}
 						foundObjs = foundObjs || err == nil
 						return
 					}
@@ -716,9 +714,7 @@ func (f *folderScanner) scanFolder(ctx context.Context, folder cachedFolder, int
 							object:    fiv.Name,
 							versionID: ver.VersionID,
 						}, madmin.HealItemObject)
-						if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+						logger.LogIf(ctx, err)
-							logger.LogIf(ctx, err)
-						}
 						foundObjs = foundObjs || err == nil
 					}
 				},
@@ -872,9 +868,6 @@ func (i *scannerItem) applyHealing(ctx context.Context, o ObjectLayer, oi Object
 		ScanMode: globalHealConfig.ScanMode(),
 	}
 	res, err := o.HealObject(ctx, i.bucket, i.objectPath(), oi.VersionID, healOpts)
-	if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
-		return 0
-	}
 	if err != nil && !errors.Is(err, NotImplemented{}) {
 		logger.LogIf(ctx, err)
 		return 0

cmd/erasure-healing.go

@@ -885,6 +885,12 @@ func isObjectDangling(metaArr []FileInfo, errs []error, dataErrs []error) (valid
 
 // HealObject - heal the given object, automatically deletes the object if stale/corrupted if `remove` is true.
 func (er erasureObjects) HealObject(ctx context.Context, bucket, object, versionID string, opts madmin.HealOpts) (hr madmin.HealResultItem, err error) {
+	defer func() {
+		if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
+			err = nil
+		}
+	}()
+
 	// Create context that also contains information about the object and bucket.
 	// The top level handler might not have this information.
 	reqInfo := logger.GetReqInfo(ctx)

cmd/erasure-healing_test.go

@@ -288,6 +288,61 @@ func TestHealingDanglingObject(t *testing.T) {
 	if fileInfoPostHeal.NumVersions != 2 {
 		t.Fatalf("Expected versions 2, got %d", fileInfoPreHeal.NumVersions)
 	}
+
+	rd = mustGetPutObjReader(t, bytes.NewReader(data), int64(len(data)), "", "")
+	objInfo, err = objLayer.PutObject(ctx, bucket, object, rd, ObjectOptions{
+		Versioned: true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, fsDir := range fsDirs[:4] {
+		if err = os.Chmod(fsDir, 0400); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// Create delete marker under quorum.
+	_, err = objLayer.DeleteObject(ctx, bucket, object, ObjectOptions{
+		Versioned: true,
+		VersionID: objInfo.VersionID,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for _, fsDir := range fsDirs[:4] {
+		if err = os.Chmod(fsDir, 0755); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	fileInfoPreHeal, err = disks[0].ReadVersion(context.Background(), bucket, object, "", false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if fileInfoPreHeal.NumVersions != 3 {
+		t.Fatalf("Expected versions 3, got %d", fileInfoPreHeal.NumVersions)
+	}
+
+	if err = objLayer.HealObjects(ctx, bucket, "", madmin.HealOpts{Remove: true},
+		func(bucket, object, vid string) error {
+			_, err := objLayer.HealObject(ctx, bucket, object, vid, madmin.HealOpts{Remove: true})
+			return err
+		}); err != nil {
+		t.Fatal(err)
+	}
+
+	fileInfoPostHeal, err = disks[0].ReadVersion(context.Background(), bucket, object, "", false)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if fileInfoPostHeal.NumVersions != 2 {
+		t.Fatalf("Expected versions 2, got %d", fileInfoPreHeal.NumVersions)
+	}
 }
 
 func TestHealObjectCorrupted(t *testing.T) {

cmd/erasure-server-pool.go

@@ -1738,9 +1738,6 @@ func (z *erasureServerPools) HealObject(ctx context.Context, bucket, object, ver
 		result, err := pool.HealObject(ctx, bucket, object, versionID, opts)
 		result.Object = decodeDirObject(result.Object)
 		if err != nil {
-			if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
-				continue
-			}
 			return result, err
 		}
 		return result, nil

cmd/global-heal.go

@@ -192,9 +192,7 @@ func (er *erasureObjects) healErasureSet(ctx context.Context, buckets []BucketIn
 		if _, err := er.HealBucket(ctx, bucket.Name, madmin.HealOpts{
 			ScanMode: scanMode,
 		}); err != nil {
-			if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+			logger.LogIf(ctx, err)
-				logger.LogIf(ctx, err)
-			}
 		}
 
 		if serverDebugLog {
@@ -242,9 +240,7 @@ func (er *erasureObjects) healErasureSet(ctx context.Context, buckets []BucketIn
 					object:    entry.name,
 					versionID: "",
 				}, madmin.HealItemObject)
-				if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+				logger.LogIf(ctx, err)
-					logger.LogIf(ctx, err)
-				}
 				return
 			}
 
@@ -254,12 +250,10 @@ func (er *erasureObjects) healErasureSet(ctx context.Context, buckets []BucketIn
 						ScanMode: scanMode,
 						Remove:   healDeleteDangling,
 					}); err != nil {
-					if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+					// If not deleted, assume they failed.
-						// If not deleted, assume they failed.
+					tracker.ItemsFailed++
-						tracker.ItemsFailed++
+					tracker.BytesFailed += uint64(version.Size)
-						tracker.BytesFailed += uint64(version.Size)
+					logger.LogIf(ctx, err)
-						logger.LogIf(ctx, err)
-					}
 				} else {
 					tracker.ItemsHealed++
 					tracker.BytesDone += uint64(version.Size)

cmd/mrf.go

@@ -218,15 +218,8 @@ func (m *mrfState) healRoutine() {
 			// Heal objects
 			for _, u := range mrfOperations {
 				if _, err := m.objectAPI.HealObject(m.ctx, u.bucket, u.object, u.versionID, mrfHealingOpts); err != nil {
-					if !isErrObjectNotFound(err) && !isErrVersionNotFound(err) {
+					// If not deleted, assume they failed.
-						// If not deleted, assume they failed.
+					logger.LogIf(m.ctx, err)
-						logger.LogIf(m.ctx, err)
-					} else {
-						m.mu.Lock()
-						m.itemsHealed++
-						m.pendingItems--
-						m.mu.Unlock()
-					}
 				} else {
 					m.mu.Lock()
 					m.itemsHealed++