diff --git a/cmd/erasure-server-pool-decom.go b/cmd/erasure-server-pool-decom.go index 69bab607b..16ea90b4d 100644 --- a/cmd/erasure-server-pool-decom.go +++ b/cmd/erasure-server-pool-decom.go @@ -813,10 +813,14 @@ func (z *erasureServerPools) decommissionPool(ctx context.Context, idx int, pool evt := evalActionFromLifecycle(ctx, *lc, lr, rcfg, objInfo) switch { - case evt.Action.DeleteRestored(): // if restored copy has expired, delete it synchronously - if !applyExpiryOnTransitionedObject(ctx, z, objInfo, evt, lcEventSrc_Decom) { - return evt.Action - } + case evt.Action.DeleteRestored(): + // A restored copy past its deadline is removed now, before the + // version moves, as the scanner would. When that removal is + // refused, the version moves like any other, with its tier + // metadata and its deadline, and the scanner of the pool that + // holds it then removes the copy. A version the walk neither + // moves nor expires leaves the key behind, and fails it. + applyExpiryOnTransitionedObject(ctx, z, objInfo, evt, lcEventSrc_Decom) return lifecycle.NoneAction case evt.Action == lifecycle.DeleteAction && versioned: // Expiring the latest version of a versioned bucket only puts diff --git a/cmd/erasure-server-pool-rebalance.go b/cmd/erasure-server-pool-rebalance.go index 1a46b4be7..81160fb0e 100644 --- a/cmd/erasure-server-pool-rebalance.go +++ b/cmd/erasure-server-pool-rebalance.go @@ -600,6 +600,12 @@ func (z *erasureServerPools) rebalanceBucket(ctx context.Context, bucket string, evt := evalActionFromLifecycle(ctx, *lc, lr, rcfg, objInfo) switch { + case evt.Action.DeleteRestored(): + // A restored copy expires by its own deadline, in the pool + // that holds the version; a rebalance does not remove it. + // Such a version reads as remote and the walk leaves it, as + // the decommission walk moves it: neither counts it as deleted. + return lifecycle.NoneAction case evt.Action == lifecycle.DeleteAction && versioned: // Expiring the latest version of a versioned bucket only puts // a delete marker on top of it. The version itself survives as diff --git a/cmd/erasure-server-pool-tier-ownership_test.go b/cmd/erasure-server-pool-tier-ownership_test.go index 62415025f..7dabd6e5e 100644 --- a/cmd/erasure-server-pool-tier-ownership_test.go +++ b/cmd/erasure-server-pool-tier-ownership_test.go @@ -4,11 +4,14 @@ package cmd import ( + "context" "errors" "slices" "testing" "github.com/minio/minio/internal/bucket/lifecycle" + objectlock "github.com/minio/minio/internal/bucket/object/lock" + xhttp "github.com/minio/minio/internal/http" ) func rewriteTierTestMeta(t *testing.T, disk StorageAPI, bucket, key string, change func(*xlMetaV2)) { @@ -216,3 +219,160 @@ func TestDecommissionMovesTieredDeleteAll(t *testing.T) { }) } } + +// expireRestoredFaultDisk fails the removal of the restored copy of one key, +// as a drive that refuses that write does. Every other operation passes. +type expireRestoredFaultDisk struct { + StorageAPI + bucket, object string +} + +func (d expireRestoredFaultDisk) DeleteVersion(ctx context.Context, volume, path string, fi FileInfo, forceDelMarker bool, opts DeleteOptions) error { + if volume == d.bucket && path == d.object && fi.ExpireRestored { + return errDiskFull + } + return d.StorageAPI.DeleteVersion(ctx, volume, path, fi, forceDelMarker, opts) +} + +// restoredPastDeadline turns the version oi of key in pool 0 into a tiered +// version with a restored copy whose restore deadline has passed: its data +// stays on the drives, its metadata records the completed transition and the +// deadline. Past the deadline the version reads as remote. It returns the +// name of the remote object. +func (f *freeVersionTest) restoredPastDeadline(key string, oi ObjectInfo) string { + f.t.Helper() + ctx := f.t.Context() + label := f.labels[oi.VersionID] + remote := f.remote(key, label) + infos, errs := readAllFileInfo(ctx, f.z.serverPools[0].getHashedSet(key).getDisks(), "", f.bucket, key, oi.VersionID, false, false) + if errs[0] != nil { + f.t.Fatal(errs[0]) + } + fi := infos[0] + fi.TransitionStatus, fi.TransitionTier, fi.TransitionedObjName = lifecycle.TransitionComplete, "WARM", remote + fi.Metadata[xhttp.AmzRestore] = completedRestoreObj(daysAgo(1)).String() + f.tier.objects[remote] = []byte(key + " " + label) + if err := f.z.serverPools[0].DecomTieredObject(ctx, f.bucket, key, fi, ObjectOptions{}); err != nil { + f.t.Fatal(err) + } + cur, err := f.z.GetObjectInfo(ctx, f.bucket, key, ObjectOptions{VersionID: oi.VersionID}) + if err != nil || cur.RestoreExpires.IsZero() || !UTCNow().After(cur.RestoreExpires) { + f.t.Fatalf("fixture: restore expires %v, %v", cur.RestoreExpires, err) + } + lc, err := globalLifecycleSys.Get(f.bucket) + if err != nil { + f.t.Fatal(err) + } + if evt := evalActionFromLifecycle(ctx, *lc, objectlock.Retention{}, nil, cur); !evt.Action.DeleteRestored() { + f.t.Fatalf("fixture: lifecycle action %v, want the restored copy to expire", evt.Action) + } + return remote +} + +// tieredVersion fails the test unless pool holds the version oi of key with +// its tier metadata of remote and its restore deadline, and returns it. +func (f *freeVersionTest) tieredVersion(pool int, key, remote string, oi ObjectInfo) ObjectInfo { + f.t.Helper() + got, err := f.z.serverPools[pool].GetObjectInfo(f.t.Context(), f.bucket, key, ObjectOptions{VersionID: oi.VersionID}) + if err != nil { + f.t.Fatalf("pool %d: %v", pool, err) + } + if tr := got.TransitionedObject; tr.Status != lifecycle.TransitionComplete || tr.Tier != "WARM" || tr.Name != remote { + f.t.Errorf("pool %d: the version lost its tier metadata: %+v", pool, tr) + } + if got.RestoreExpires.IsZero() || !UTCNow().After(got.RestoreExpires) { + f.t.Errorf("pool %d: the version lost its restore deadline: %v", pool, got.RestoreExpires) + } + if slices.Contains(f.tier.removed, remote) { + f.t.Errorf("remote object removed: %v", f.tier.removed) + } + return got +} + +// expireRestoredCopy removes the restored copy of oi, as the scanner of the +// pool that holds the version does once its deadline has passed, and fails +// the test unless the version then reads as remote without a deadline and +// the remote object stays. +func (f *freeVersionTest) expireRestoredCopy(pool int, key, remote string, oi ObjectInfo) { + f.t.Helper() + ctx := f.t.Context() + lc, err := globalLifecycleSys.Get(f.bucket) + if err != nil { + f.t.Fatal(err) + } + evt := evalActionFromLifecycle(ctx, *lc, objectlock.Retention{}, nil, oi) + if !evt.Action.DeleteRestored() { + f.t.Fatalf("lifecycle action in pool %d: %v", pool, evt.Action) + } + if !applyExpiryOnTransitionedObject(ctx, f.z, oi, evt, lcEventSrc_Scanner) { + f.t.Fatalf("the restored copy did not expire in pool %d", pool) + } + after, err := f.z.serverPools[pool].GetObjectInfo(ctx, f.bucket, key, ObjectOptions{VersionID: oi.VersionID}) + if err != nil || !after.IsRemote() || !after.RestoreExpires.IsZero() || after.TransitionedObject.Name != remote { + f.t.Fatalf("version after the restored copy expired: remote=%v expires=%v %+v, %v", after.IsRemote(), after.RestoreExpires, after.TransitionedObject, err) + } + if slices.Contains(f.tier.removed, remote) { + f.t.Errorf("remote object removed with the restored copy: %v", f.tier.removed) + } +} + +// A decommission removes a restored copy that is past its deadline before it +// moves the version. When the drives refuse that removal, the version moves +// like any other tiered version, with its tier metadata and its deadline: the +// remote object stays, the source copy retires with the key, and the scanner +// of the pool that holds the version removes the copy by that deadline. +// Skipping the version instead leaves the key behind with a +// PreConditionFailed and fails the decommission on every retry. +func TestDecommissionMovesRestoredCopyPastDeadline(t *testing.T) { + f := newFreeVersionTest(t, freeVersionNoncurrent) + const key = "obj-restored" + oi := f.put(key, "v1", daysAgo(4)) + remote := f.restoredPastDeadline(key, oi) + + set := f.z.serverPools[0].getHashedSet(key) + getDisks := set.getDisks + faulty := slices.Clone(getDisks()) + for i := range faulty { + faulty[i] = expireRestoredFaultDisk{StorageAPI: faulty[i], bucket: f.bucket, object: key} + } + set.getDisks = func() []StorageAPI { return faulty } + t.Cleanup(func() { set.getDisks = getDisks }) + + info := f.decommission() + if !info.Complete || info.Failed || info.ItemsDecommissionFailed != 0 { + t.Fatalf("decommission with a refused restored-copy removal: complete=%v failed=%v items failed=%d, pool 0 holds %v, pool 1 holds %v", + info.Complete, info.Failed, info.ItemsDecommissionFailed, f.stored(0, key), f.stored(1, key)) + } + if got := f.stored(0, key); len(got) != 0 { + t.Errorf("pool 0 still holds %v", got) + } + if got := f.stored(1, key); !slices.Equal(got, []string{"v1"}) { + t.Fatalf("pool 1 holds %v, want [v1]", got) + } + moved := f.tieredVersion(1, key, remote, oi) + t.Logf("the moved version reads as remote: %v", moved.IsRemote()) + f.expireRestoredCopy(1, key, remote, moved) +} + +// Control: a rebalance moves no tiered version, and a restored copy past its +// deadline reads as remote. The walk leaves the version in the source pool +// without counting it as deleted, so the key is not retired, and the scanner +// of that pool removes the copy by its deadline. +func TestRebalanceLeavesRestoredCopyPastDeadline(t *testing.T) { + f := newFreeVersionTest(t, freeVersionNoncurrent) + const key = "obj-restored" + oi := f.put(key, "v1", daysAgo(4)) + remote := f.restoredPastDeadline(key, oi) + + if failed := migrationTestWalk(t, f.z, f.bucket, 0, "rebalance"); failed != 0 { + t.Fatalf("%d versions failed to move", failed) + } + if got := f.stored(0, key); !slices.Equal(got, []string{"v1"}) { + t.Fatalf("pool 0 holds %v, want [v1]", got) + } + if got := f.stored(1, key); len(got) != 0 { + t.Errorf("pool 1 holds %v", got) + } + left := f.tieredVersion(0, key, remote, oi) + f.expireRestoredCopy(0, key, remote, left) +}