Skip to content

fix: Require TRITON_BUILD_CONTAINER instead of synthesizing a py3-min image - #125

Merged
mc-nv merged 1 commit into
mainfrom
mchornyi/TRI-1930/inference-image
Oct 5, 2026
Merged

mc-nv merged 1 commit into
mainfrom
mchornyi/TRI-1930/inference-image

Conversation

@mc-nv

@mc-nv mc-nv commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What does the PR do?

  • Building with only TRITON_BUILD_CONTAINER_VERSION invented
    tritonserver:${VERSION}-py3-min, an image being retired — configure
    succeeded and the build died later on a docker pull 404.
  • Requires TRITON_BUILD_CONTAINER instead, making it a configure-time error.
    TRITON_BUILD_CONTAINER_VERSION only fed that synthesis, so it goes too, and
    two guards collapse into one.

⚠️ Breaking change

Standalone cmake builds passing only -DTRITON_BUILD_CONTAINER_VERSION now
fail with TRITON_BUILD_CONTAINER is required. This is the intent — loud
and immediate, rather than an opaque 404 once py3-min is withdrawn. The README
example is updated in the same commit so the documented path never references
a removed variable.

Builds driven by server/build.py are unaffected: it always passes a
resolved image.

Why requiring it is safe here

tools/gen_openvino_dockerfile.py emits only FROM ${BASE_IMAGE} and
WORKDIR /workspace before apt-installing its own dependencies — zero
references to tritonserver or /opt/tritonserver. CMakeLists.txt also
forces TRITON_ENABLE_GPU OFF, so this backend does not even need CUDA.
Nothing is lost by making the caller name the image.

Where should the reviewer start?

  • CMakeLists.txt — the removed synthesis and the single FATAL_ERROR guard.

Test plan

Verified with cmake 4.4.3:

Case Result
no TRITON_BUILD_CONTAINER fatal, "TRITON_BUILD_CONTAINER is required"
old -DTRITON_BUILD_CONTAINER_VERSION=25.11 only same fatal — the documented break
new -DTRITON_BUILD_CONTAINER=<image> clears the guard, proceeds to FetchContent

Also ./build.py --dryrun --enable-gpu --backend openvino in server: the
generated cmake_build carries
TRITON_BUILD_CONTAINER=nvcr.io/nvidia/cuda-dl-base:26.09-cuda13.4-devel-ubuntu24.04,
confirming build.py never relied on the removed fallback.

  • CI Pipeline ID:

Related PRs:

Related Issues:

  • Resolves: TRI-1930

… image

When only TRITON_BUILD_CONTAINER_VERSION was given, the build container was
invented as nvcr.io/nvidia/tritonserver:${VERSION}-py3-min. That image is
being retired, so the fallback is a latent failure: configure succeeds and the
build dies much later on a docker pull 404 for an image nobody asked for.

Stop inventing one. TRITON_BUILD_CONTAINER is now required on all platforms,
which turns a missing image into a configure-time error that names exactly
what to supply. TRITON_BUILD_CONTAINER_VERSION had no other use in this repo,
so it goes with it, and the WIN32 and either-or guards collapse into one.

Nothing is lost by making the caller name the image: tools/gen_openvino_dockerfile.py
emits only FROM ${BASE_IMAGE} and WORKDIR before apt-installing its own
dependencies, with no reference to tritonserver paths, and this backend forces
TRITON_ENABLE_GPU OFF so it does not need CUDA either.

Breaking for standalone cmake builds: configuring with only
-DTRITON_BUILD_CONTAINER_VERSION now fails with "TRITON_BUILD_CONTAINER is
required". The README example is updated in the same commit so the documented
path does not point at a removed variable. Builds driven by server/build.py
are unaffected -- it always passes a resolved image.
@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[High risk] Build system now requires explicit container image instead of inferring it.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR removes inference of a retiring py3-min build image, requires callers to provide TRITON_BUILD_CONTAINER, and updates the standalone build example accordingly.

Reviews (1) · Last reviewed commit: "fix: Require TRITON_BUILD_CONTAINER inst..."

@mc-nv

mc-nv commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

FYI : @dtrawins

@mc-nv
mc-nv requested review from Vinya567, nv-rinig and whoisj October 2, 2026 18:57
@mc-nv
mc-nv merged commit 1ba7cc3 into main Oct 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fix (fix: PRs)

Development

Successfully merging this pull request may close these issues.

2 participants