From 0663a41bd5a816362eb6d8bef65f14d4355f3c1b Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 12 May 2026 20:55:34 -0700 Subject: [PATCH] fix(s3/lifecycle): WalkerDispatcher uses entry.Path for ABORT_MPU + shell announces load Two CI-surfaced bugs caught by PR #9471's S3 Lifecycle Tests run on master after PRs #9475 + #9466: 1. Walker dispatch for ABORT_MPU was sending entry.DestKey as req.ObjectPath. The server's ABORT_MPU handler (weed/s3api/s3api_internal_lifecycle.go) strips the .uploads/ prefix to extract the upload id and reads the init record from that directory, so it expects the .uploads/ path verbatim. DestKey looks like a regular object path; the server's prefix check fails and the dispatch returns BLOCKED with "FATAL_EVENT_ERROR: ABORT_MPU object_path missing .uploads/ prefix". The test fix renames TestWalkerDispatcher_MPUInitUsesDestKey to ...UsesUploadsPath and inverts the assertion to match the actual server contract. DestKey is still used for the WalkBuckets shard predicate and for rule-prefix matching in bootstrap.walker; both surfaces want the user's intended path, while DISPATCH wants the .uploads/ directory. The bootstrap test (TestLifecycleAbortIncompleteMultipartUpload) caught this when the walker's BLOCKED error surfaced as FATAL output. 2. test/s3/lifecycle/s3_lifecycle_empty_bucket_test.go asserts the shell command logs "loaded lifecycle for N bucket(s)" so a regression that produces half-shaped output (no load summary) is caught. The restored shell command (PR #9475) didn't print that line; add it back on the first pass that finds non-zero inputs. --- .../s3lifecycle/dailyrun/walker_dispatcher.go | 16 +++++++++------- .../dailyrun/walker_dispatcher_test.go | 10 ++++++---- weed/shell/command_s3_lifecycle_run_shard.go | 5 +++++ 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher.go b/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher.go index 65fdd03ce..d54e7bd81 100644 --- a/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher.go +++ b/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher.go @@ -35,16 +35,18 @@ func (d *WalkerDispatcher) Delete(ctx context.Context, action *engine.CompiledAc } objectPath := entry.Path if entry.IsMPUInit { - // Rule-prefix matching used DestKey; the server takes the - // canonical object path for the LifecycleDelete RPC, which - // is also DestKey. The walker hits the .uploads/ - // directory itself only when ActionKind=ABORT_MPU, and the - // server resolves the upload from (bucket, object_path) + - // the init record's metadata. + // Rule-prefix matching uses DestKey (the user's intended + // object key); dispatch uses entry.Path (.uploads/), + // which is what the server's ABORT_MPU handler expects in + // req.ObjectPath — it strips the .uploads/ prefix to get + // the upload id and reads the init record from that + // directory. DestKey is the dispatch ANTI-pattern here: it + // looks like a regular object path, the server's check for + // the .uploads/ prefix fails, and the dispatch comes back + // as BLOCKED FATAL_EVENT_ERROR. if entry.DestKey == "" { return fmt.Errorf("walker dispatch: MPU init entry with empty DestKey: %s", entry.Path) } - objectPath = entry.DestKey } rh := action.Key.RuleHash req := &s3_lifecycle_pb.LifecycleDeleteRequest{ diff --git a/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher_test.go b/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher_test.go index a826ee51b..68c067cd6 100644 --- a/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher_test.go +++ b/weed/s3api/s3lifecycle/dailyrun/walker_dispatcher_test.go @@ -72,9 +72,11 @@ func TestWalkerDispatcher_VersionedPassesVersionID(t *testing.T) { assert.Equal(t, "v-abc", c.lastReq.VersionId) } -func TestWalkerDispatcher_MPUInitUsesDestKey(t *testing.T) { - // Rule-prefix matching used DestKey; the RPC ObjectPath must match - // so the server resolves the upload from the user's intended key. +func TestWalkerDispatcher_MPUInitUsesUploadsPath(t *testing.T) { + // Rule-prefix matching uses DestKey to decide IF this MPU init + // matches; dispatch uses Path (.uploads/) because the server's + // ABORT_MPU handler strips the .uploads/ prefix to get the upload + // id. Sending DestKey here would BLOCK with FATAL_EVENT_ERROR. c := &walkerStubClient{outcome: s3_lifecycle_pb.LifecycleDeleteOutcome_DONE} d := &WalkerDispatcher{Client: c} a := sampleAction(t, s3lifecycle.ActionKindAbortMPU) @@ -84,7 +86,7 @@ func TestWalkerDispatcher_MPUInitUsesDestKey(t *testing.T) { IsMPUInit: true, }) require.NoError(t, err) - assert.Equal(t, "user/path/object", c.lastReq.ObjectPath) + assert.Equal(t, ".uploads/abc123", c.lastReq.ObjectPath) } func TestWalkerDispatcher_MPUInitEmptyDestKeyErrors(t *testing.T) { diff --git a/weed/shell/command_s3_lifecycle_run_shard.go b/weed/shell/command_s3_lifecycle_run_shard.go index 51c383a18..1ad1a4205 100644 --- a/weed/shell/command_s3_lifecycle_run_shard.go +++ b/weed/shell/command_s3_lifecycle_run_shard.go @@ -122,6 +122,7 @@ func (c *commandS3LifecycleRunShard) Do(args []string, env *CommandEnv, writer i fmt.Fprintf(writer, "running shards %s (event budget=%d, runtime=%s, refresh=%s)…\n", formatShardLabel(shards), *eventBudget, *runtime, *cadence) + announcedLoad := false runPass := func() error { eng := engine.New() inputs, parseErrors, err := scheduler.LoadCompileInputs(ctx, filerClient, bucketsPath) @@ -140,6 +141,10 @@ func (c *commandS3LifecycleRunShard) Do(args []string, env *CommandEnv, writer i if len(inputs) == 0 { return nil } + if !announcedLoad { + fmt.Fprintf(writer, "loaded lifecycle for %d bucket(s)\n", len(inputs)) + announcedLoad = true + } buckets := make([]string, 0, len(inputs)) for _, in := range inputs { if in.Bucket != "" {