Skip to content

fix: Service mode should handle buckets lazily instead of checking/creating a predefined bucket #182

@tinswzy

Description

@tinswzy

Background

Woodpecker service mode can be used as a multi-tenant service. Every data-plane
RPC carries its own bucket_name + root_path (proto/logstore.proto), and the
server routes each request to that bucket/path (server/service.go: AddEntry /
GetBatchEntriesAdv / FenceSegment / ...). The object-storage client takes
bucketName per call, so a single client can serve many buckets/tenants. So in
service mode the node does not necessarily belong to any single global bucket —
different requests may target different callers' buckets/rootPaths, each of which
the caller owns and pre-provisions.

Problem

At startup, NewObjectStorageNewMinioHandlernewMinioClient
(common/minio/minio_client_helper.go:143) runs checkBucketFn unconditionally:

  • it calls BucketExists(cfg.Minio.BucketName);
  • if the bucket is missing: createBucket=trueMakeBucket(cfg.Minio.BucketName);
    createBucket=false → returns ErrConfigError: "bucket X not Existed" and
    startup fails (after 20 retries, CheckBucketRetryAttempts).

This runs for both minio and service modes (server/service.go:112). So
service-mode startup is coupled to one configured bucket (minio.bucketName,
default a-bucket): the node either fails when it's missing, or auto-creates a
stray global bucket unrelated to any caller. When service mode is used this way
(callers managing their own bucket/rootPath), Woodpecker should not interfere with
the user's bucket lifecycle.

Proposal

Unify bucket-existence handling under the existing createBucket flag:

  • createBucket=true → check (BucketExists) and create if missing (current behavior).
  • createBucket=false → skip both check and create, do not error. Assume the
    bucket/rootPath is pre-provisioned and owned by the caller.

For service-mode deployments where callers manage their own buckets, ship
minio.createBucket: false so the node never touches a global bucket. Keep the Go
default true, so embed / single-tenant minio behavior is unchanged.

Scope / notes

  • Gate the checkBucketFn block in newMinioClient
    (common/minio/minio_client_helper.go:141-166) on cfg.Minio.CreateBucket;
    drop the else { return ErrConfigError("not Existed") } branch.
  • Semantics change: createBucket=false goes from "bucket must already exist (else
    fail fast at boot)" to "don't touch buckets". Trade-off: loses boot-time
    reachability/credential fail-fast; failures surface on first per-request op
    instead.
  • Default unchanged: createBucket stays true (common/config/configuration.go:885).
  • Deprecated newMinioClientFromConfig (:171) has the same block — update or
    remove for consistency.
  • Config: set createBucket: false in service-mode deploy config
    (deployments/operator/config/samples/*, chart/values); update the bucketName
    comment in config/woodpecker.yaml to document the new false semantics.
  • Tests: common/minio/minio_client_helper_test.go doesn't lock this branch;
    verify green, optionally add a "createBucket=false ⇒ no BucketExists call" case.

Acceptance criteria

  • createBucket=false: server starts with no BucketExists/MakeBucket call and
    no error, even if minio.bucketName is absent/placeholder.
  • createBucket=true: behavior unchanged (check + create-if-missing).
  • Per-request read/write paths unchanged.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions