Skip to content

Finish the Bazel module consolidation in reviewable steps #608

Description

@balajinvda

Tracks finishing the Bazel module consolidation after #593 was closed
as too large to review (1608 files). The work was correct and is preserved on
the preserve/consolidation-full branch; this issue re-lands it in reviewable
pieces.

Where things stand

tools/ci/nested-modules.txt lists 17 services that still carry their own
MODULE.bazel. Everything else already builds from the root module.

Two things are already done and merged:

Why the previous attempt was too large

Of the 1608 files in #593:

what files
nvca vendored BUILD relabels 1113
nvca's own BUILD files 119
the other 16 services 376

nvca is the whole reviewability problem, and for a reason worth fixing on its
own merits.

Step 1: move nvca off vendored BUILD files

nvca is the only service that resolves Go dependencies through //vendor/...
labels. That requires a committed BUILD.bazel beside every vendored package:
1261 of them today. Adding a dependency means committing a BUILD file per
package in its transitive closure, so a single library bump churns hundreds of
files.

Every other service resolves through go_deps from go.mod, with vendor/
kept on disk for the non-Bazel path and listed in .bazelignore.
image-credential-helper already does exactly this.

Scope:

  • delete 1261 vendored BUILD.bazel files
  • rewrite 119 of nvca's own BUILD files from //vendor/x to @go_deps labels
  • add nvca/vendor to .bazelignore, add nvca to go.work.bazel

Two facts established before proposing this:

  1. Only one vendored BUILD file does anything a generated one would not
    (x_defs stamping a version into the DRA driver's info package). It is
    redundant: the same stamp is applied from nvca's own targets using the
    fully-qualified import path, which works regardless of how the dependency
    resolves.
  2. Joining nvca's dependencies to the shared graph moves 10 versions, all minor
    or patch: golang.org/x/{sync,term,net,sys}, two otel exporters, otelgrpc,
    genproto, klauspost/compress, samber/lo. No majors, nothing in the
    containerd, moby or k8s families, and otel itself does not move, so the
    semconv schema conflict seen in feat(bazel): consolidate every service into the root Bazel module #593 does not arise.

This lands independently of consolidation and is worth doing regardless.

Step 2: shared rules parity

Root's rules/oci is a strict subset of what the services use. Every migration
fails at load time until it is a superset. Additive, touches no service.

Needs: binary_path (used by 15 of 17), extra_layers, env,
extra_registries, cmd, go_oci_multi_binary_image, rust_oci_image, and
layer visibility following the image so entrypoint-mode tests can inspect it.

Step 3: migrate the remaining 16 services

About 376 files, in batches of four. Each batch: delete the nested module and
its duplicated scaffold, join go.work.bazel, leave .bazelignore, rewrite
subtree-relative labels, convert the CI matrix row to root-scoped.

The first batch must also carry the CI partition fix, because that is when
.bazelignore stops separating the root row from the service rows.

Step 4: paired release-config change

The private release repo builds each service from a source.subtree with
subtree-relative target labels. Those only resolve inside a nested module. The
matching change is written and open, and must land in the same window as the
migration: on its own it points release builds at labels that do not exist, and
the migration without it points them at a subtree with no module. Neither
repository's CI catches the mismatch, because the failure is a broken release
rather than a failed check.

Findings that must carry forward

Recorded because each cost real time to find and none is obvious:

  1. A replace directive is not scoped to its own module once that module joins
    the root go.work. Two services carried 19 between them for modules they
    never import, which re-pinned the whole workspace and broke unrelated
    subtrees with missing-sum errors.
  2. Bazel honours the manual tag only when expanding wildcard patterns. CI
    resolves an explicit label list, so ten deliberately quarantined tests run in
    CI while bazel test //... skips them locally.
  3. .bazelignore implicitly enforces the one-row-per-target CI partition.
    Removing entries makes the root row a superset of every service row, which
    doubles the work and lets 68 fixed-port test packages contend on one machine.
  4. cargo_build_script targets carry no edition and take the toolchain default.
    One root module registers one default, so a crate whose build script needs a
    newer edition fails.
  5. Relative paths in build_script_env break when a package moves deeper.
    Nothing in analysis catches it; the build script fails at execution.
  6. Nested .bazelignore files carry load-bearing exclusions that must be lifted
    with a path prefix.

Known defects, not in scope here

  • 68 test packages bind literal ports, overlapping heavily across services.
    Safe while each has a CI row to itself. Should move to ephemeral binding.
  • image-push-manual and the matrix build key their Bazel cache per row, while
    the cached content is identical for every root-module row. Fixed on the
    preserved branch; needs re-landing with step 3.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions