fix(decom): count lifecycle removals as scan progress

Reuse the source-version lookup to count versions removed by ILM before migration cleanup so nested keys are discovered on the next pass. Keep delete-marker-only expiry from reporting physical progress, and cover both the decommission regression and the removal-count boundary.

Signed-off-by: Feng Ruohang <rh@vonng.com>

cmd/erasure-server-pool-consistency.go

@@ -272,7 +272,7 @@ func (z *erasureServerPools) applyMigrationExpiry(ctx context.Context, bucket, o
 // owns replication state and tier cleanup, and it re-evaluates the current
 // lifecycle rule. A migration cleanup must never turn an asynchronous ILM
 // decision into an unconditional physical purge. It returns how many source
-// versions it deleted.
+// versions were removed, including versions deleted by ILM before cleanup.
 func (z *erasureServerPools) removeMovedVersions(ctx context.Context, bucket, object string, source int, versions []FileInfo, expired []migrationExpiredVersion) (removed int, err error) {
 	if err := z.applyMigrationExpiry(ctx, bucket, object, source, expired); err != nil {
 		return 0, err
@@ -290,8 +290,14 @@ func (z *erasureServerPools) removeMovedVersions(ctx context.Context, bucket, ob
 		if vid == "" {
 			vid = nullVersionID
 		}
+		expiredVersion := isExpiredVersion(expired, version)
 		copies, err := z.objectPoolInfos(ctx, bucket, object, ObjectOptions{VersionID: vid, NoAuditLog: true})
 		if isErrObjectNotFound(err) || isErrVersionNotFound(err) {
+			// ILM can remove the parent key before cleanup. Count that as
+			// progress so the next walk discovers keys below it.
+			if expiredVersion {
+				removed++
+			}
 			continue
 		}
 		if err != nil {
@@ -307,12 +313,15 @@ func (z *erasureServerPools) removeMovedVersions(ctx context.Context, bucket, ob
 			}
 		}
 		if src == nil {
+			if expiredVersion {
+				removed++
+			}
 			continue
 		}
 		if !src.ModTime.Equal(version.ModTime) || src.DeleteMarker != version.Deleted {
 			return removed, PreConditionFailed{}
 		}
-		if isExpiredVersion(expired, version) {
+		if expiredVersion {
 			// The logical expiry path owns this version. Keeping the source is
 			// also the retry point if that path failed or the pool check races it.
 			continue

cmd/erasure-server-pool-decom-keys-below_test.go

@@ -17,7 +17,10 @@
 
 package cmd
 
-import "testing"
+import (
+	"testing"
+	"time"
+)
 
 // A key below another key ("k/b" under "k") is a directory inside the
 // directory of that key. The listing treats the directory of an object as a
@@ -64,3 +67,36 @@ func TestDecommissionMovesKeysBelowAKey(t *testing.T) {
 		}
 	}
 }
+
+// An ILM expiry can remove the parent key during a decommission walk. That
+// removal must count as progress, or the next key below the parent remains
+// hidden from the listing and the source pool is left non-empty.
+func TestDecommissionContinuesAfterLifecycleRemovesParent(t *testing.T) {
+	z, _ := migrationTestPools(t)
+	const bucket = "decommission-ilm-keys-below"
+	if err := z.MakeBucket(t.Context(), bucket, MakeBucketOptions{}); err != nil {
+		t.Fatal(err)
+	}
+	meta, err := globalBucketMetadataSys.Get(bucket)
+	if err != nil {
+		t.Fatal(err)
+	}
+	meta.LifecycleConfigXML = []byte(`<LifecycleConfiguration><Rule><ID>expire-old</ID><Status>Enabled</Status><Filter><Prefix></Prefix></Filter><Expiration><Days>1</Days></Expiration></Rule></LifecycleConfiguration>`)
+	if err := meta.Save(t.Context(), z); err != nil {
+		t.Fatal(err)
+	}
+	globalBucketMetadataSys.Set(bucket, meta)
+
+	putConsistencyObject(t, z, bucket, "k", 0, "parent", ObjectOptions{MTime: UTCNow().Add(-72 * time.Hour)})
+	putConsistencyObject(t, z, bucket, "k/b", 0, "child", ObjectOptions{MTime: UTCNow()})
+
+	if info := keysBelowDecommission(t, z); !info.Complete || info.Failed {
+		t.Fatalf("decommission: complete %v, failed %v; want complete", info.Complete, info.Failed)
+	}
+	if _, err := z.serverPools[0].GetObjectInfo(t.Context(), bucket, "k/b", ObjectOptions{}); !isErrObjectNotFound(err) {
+		t.Fatalf("child still in source pool: %v", err)
+	}
+	if body := migrationTestBody(t, z.GetObjectNInfo, bucket, "k/b", ""); body != "child" {
+		t.Fatalf("child reads %q", body)
+	}
+}

cmd/erasure-server-pool-migration-ilm_test.go

@@ -414,6 +414,46 @@ func testMigrationLifecycleExpiry(t *testing.T, kind string) {
 	})
 }
 
+// Physical expiry advances the walk; creating a delete marker does not remove
+// the addressed version and must not make a walk repeat without progress.
+func TestPoolsMigrationLifecycleRemovalProgress(t *testing.T) {
+	for _, versioned := range []bool{false, true} {
+		status := versioning.State("")
+		name, wantRemoved := "physical-delete", 1
+		if versioned {
+			status = versioning.Enabled
+			name, wantRemoved = "delete-marker-only", 0
+		}
+		t.Run(name, func(t *testing.T) {
+			m := newMigrationILM(t, status, false, migrationILMExpireCurrent())
+			oi := m.put("obj-01", "v1", ObjectOptions{Versioned: versioned, MTime: UTCNow().Add(-72 * time.Hour)})
+			_, updatedAt, err := globalBucketMetadataSys.GetLifecycleConfig(m.bucket)
+			if err != nil {
+				t.Fatal(err)
+			}
+			version := FileInfo{Name: oi.Name, VersionID: oi.VersionID, ModTime: oi.ModTime}
+			removed, err := m.z.removeMovedVersions(t.Context(), m.bucket, oi.Name, 0, []FileInfo{version}, []migrationExpiredVersion{{
+				version:            version,
+				event:              lifecycle.Event{Action: lifecycle.DeleteAction},
+				lifecycleUpdatedAt: updatedAt,
+				source:             lcEventSrc_Decom,
+			}})
+			if err != nil || removed != wantRemoved {
+				t.Fatalf("removed %d versions, err %v; want %d, nil", removed, err, wantRemoved)
+			}
+			if versioned {
+				if body := migrationTestBody(t, m.z.GetObjectNInfo, m.bucket, oi.Name, oi.VersionID); body != "obj-01 v1" {
+					t.Fatalf("noncurrent version reads %q", body)
+				}
+			}
+			current, err := m.z.GetObjectInfo(t.Context(), m.bucket, oi.Name, ObjectOptions{})
+			if !isErrObjectNotFound(err) || current.DeleteMarker != versioned {
+				t.Fatalf("after expiry: delete marker %t, err %v", current.DeleteMarker, err)
+			}
+		})
+	}
+}
+
 // A lifecycle action found during the migration walk must not be applied
 // after the bucket's lifecycle configuration changes. The source copy stays
 // available for a fresh scan under the new policy.