Introduce Testcontainers.Triton#1678
Introduce Testcontainers.Triton#1678ezsilmar wants to merge 2 commits intotestcontainers:developfrom
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughAdded a new Testcontainers.Triton module (library + tests), registered projects in solutions, and centralized gRPC package versions; implements Triton container builder, configuration, container, connection-string provider, and integration tests exercising HTTP/gRPC endpoints and model inference. Changes
Sequence DiagramsequenceDiagram
participant Test as Test Case
participant Builder as TritonBuilder
participant Container as TritonContainer
participant ConnProvider as TritonConnectionStringProvider
participant Endpoints as HTTP/gRPC Endpoints
Test->>Builder: new TritonBuilder(image)
Builder->>Builder: Init() — configure ports, wait strategy
Test->>Builder: Build()
Builder->>Container: create TritonContainer(config)
Test->>Container: Start (InitializeAsync)
Container->>Endpoints: expose mapped ports (8000,8001,8002)
Test->>Container: GetHttpEndpoint()
Container->>Endpoints: resolve mapped public port (8000)
Endpoints-->>Container: HTTP URL
Test->>Container: GetGrpcEndpoint()
Container->>Endpoints: resolve mapped public port (8001)
Endpoints-->>Container: gRPC URL
Test->>ConnProvider: GetConnectionString()
ConnProvider->>Container: GetHttpEndpoint()
Container-->>ConnProvider: HTTP URL
ConnProvider-->>Test: connection string
Test->>Container: DisposeAsync()
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/Testcontainers.Triton.Tests/Dockerfile (1)
1-1: Consider running as non-root user for production use.The static analysis tool flagged that this image runs as root. While acceptable for testing, consider adding a
USERdirective if this Dockerfile is adapted for production scenarios.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Testcontainers.Triton.Tests/Dockerfile` at line 1, The Dockerfile currently uses the NVIDIA Triton base image and runs as root; to make it safe for production add a non-root user and switch to it using a USER directive: create a dedicated user/group (e.g., triton), set ownership/permissions for any directories the server needs (models, logs, etc.) and then add a USER triton line before the container start commands so the Triton process does not run as root; update any RUN/COPY steps that need elevated rights to use root, then change ownership and switch to the non-root user before CMD/ENTRYPOINT.src/Testcontainers.Triton/TritonContainer.cs (1)
25-32: Add a dedicated metrics endpoint helper for API symmetry.
TritonContainerexposes HTTP and gRPC endpoint helpers, but metrics callers must currently reconstruct the URL manually. AddingGetMetricsEndpoint()avoids duplicated URI logic.♻️ Proposed refactor
public string GetGrpcEndpoint() { return new UriBuilder(Uri.UriSchemeHttp, Hostname, GetMappedPublicPort(TritonBuilder.TritonGrpcPort)).ToString(); } + +/// <summary> +/// Gets the Triton metrics endpoint URL. +/// </summary> +/// <returns>The Triton metrics endpoint.</returns> +public string GetMetricsEndpoint() +{ + return new UriBuilder(Uri.UriSchemeHttp, Hostname, GetMappedPublicPort(TritonBuilder.TritonMetricsPort)).ToString(); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.Triton/TritonContainer.cs` around lines 25 - 32, TritonContainer lacks a dedicated metrics endpoint helper causing callers to reconstruct the URL; add a public GetMetricsEndpoint() method analogous to GetGrpcEndpoint() that returns a Uri (string) built with Uri.UriSchemeHttp, Hostname and the mapped port from GetMappedPublicPort(TritonBuilder.TritonMetricsPort) so callers can use TritonContainer.GetMetricsEndpoint() instead of duplicating URI construction logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Testcontainers.Triton/TritonBuilder.cs`:
- Around line 72-73: Replace the brittle log-string wait in TritonBuilder (the
WithWaitStrategy call using Wait.ForUnixContainer and UntilMessageIsLogged) with
an HTTP readiness probe against Triton’s standard readiness endpoint; use the
testcontainers HTTP wait strategy to poll path /v2/health/ready on port 8000 and
wait for an OK response (e.g., use Wait.ForHttp("/v2/health/ready") configured
for port 8000 and to expect HTTP 200), then remove the UntilMessageIsLogged
usage so the container readiness relies on the HTTP readiness endpoint instead.
---
Nitpick comments:
In `@src/Testcontainers.Triton/TritonContainer.cs`:
- Around line 25-32: TritonContainer lacks a dedicated metrics endpoint helper
causing callers to reconstruct the URL; add a public GetMetricsEndpoint() method
analogous to GetGrpcEndpoint() that returns a Uri (string) built with
Uri.UriSchemeHttp, Hostname and the mapped port from
GetMappedPublicPort(TritonBuilder.TritonMetricsPort) so callers can use
TritonContainer.GetMetricsEndpoint() instead of duplicating URI construction
logic.
In `@tests/Testcontainers.Triton.Tests/Dockerfile`:
- Line 1: The Dockerfile currently uses the NVIDIA Triton base image and runs as
root; to make it safe for production add a non-root user and switch to it using
a USER directive: create a dedicated user/group (e.g., triton), set
ownership/permissions for any directories the server needs (models, logs, etc.)
and then add a USER triton line before the container start commands so the
Triton process does not run as root; update any RUN/COPY steps that need
elevated rights to use root, then change ownership and switch to the non-root
user before CMD/ENTRYPOINT.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6e235474-80da-4534-9e56-513241f3ad6a
📒 Files selected for processing (18)
Directory.Packages.propsTestcontainers.slnTestcontainers.slnxsrc/Testcontainers.Triton/.editorconfigsrc/Testcontainers.Triton/Testcontainers.Triton.csprojsrc/Testcontainers.Triton/TritonBuilder.cssrc/Testcontainers.Triton/TritonConfiguration.cssrc/Testcontainers.Triton/TritonConnectionStringProvider.cssrc/Testcontainers.Triton/TritonContainer.cssrc/Testcontainers.Triton/Usings.cstests/Testcontainers.Triton.Tests/.editorconfigtests/Testcontainers.Triton.Tests/.runs-ontests/Testcontainers.Triton.Tests/Dockerfiletests/Testcontainers.Triton.Tests/Testcontainers.Triton.Tests.csprojtests/Testcontainers.Triton.Tests/TritonContainerTest.cstests/Testcontainers.Triton.Tests/Usings.cstests/Testcontainers.Triton.Tests/models/simple/1/.gitkeeptests/Testcontainers.Triton.Tests/models/simple/config.pbtxt
* Use health check instead of log message
* Validate that the user passed something to WithCommand
Triton requires the correct command line to be set, however there's no
reasonable defaults that we could pass because it depends on the models
the user is testing. I investigated if we could use configuration or
some other tricks to make the zero-conf work but it only adds more
confusion because we must modify the command line which breaks the
intended behavior of WithCommand interface.
The minimal configuration looks something like this:
```
var testModelRepositoryPath =
Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "models");
builder
.WithCommand("tritonserver", "--model-repository=/models")
.WithResourceMapping(testModelRepositoryPath, "/models/")
```
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/Testcontainers.Triton.Tests/TritonContainerTest.cs (1)
116-119: Use structured JSON assertions for inference output.The current checks can pass on unrelated payload text (e.g., any
"42"in the response). AssertOUTPUT0and its value from parsed JSON for deterministic validation.Suggested patch
var responseBody = await httpResponse.Content.ReadAsStringAsync(TestContext.Current.CancellationToken) .ConfigureAwait(true); - Assert.Contains("OUTPUT0", responseBody); - Assert.Contains("42", responseBody); + using var responseJson = System.Text.Json.JsonDocument.Parse(responseBody); + + float? output0 = null; + foreach (var output in responseJson.RootElement.GetProperty("outputs").EnumerateArray()) + { + if (output.GetProperty("name").GetString() == "OUTPUT0") + { + output0 = output.GetProperty("data")[0].GetSingle(); + break; + } + } + + Assert.True(output0.HasValue, "Expected OUTPUT0 in inference response."); + Assert.Equal(42.0f, output0.Value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Testcontainers.Triton.Tests/TritonContainerTest.cs` around lines 116 - 119, Replace the plain-text assertions on responseBody with structured JSON assertions: parse responseBody into a JSON object (from responseBody variable obtained from httpResponse) and assert that the JSON contains the "OUTPUT0" property and that its value equals 42 (use the test framework's Assert.Equal/Assert.True on the parsed token/value). Ensure you reference the same responseBody variable and perform the parse before the assertions so the test deterministically validates the inference output rather than matching incidental substrings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/Testcontainers.Triton.Tests/TritonContainerTest.cs`:
- Around line 9-14: The test should fail fast when the host models directory is
missing: before constructing the TritonContainer, check that modelRepositoryPath
(the Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "models") variable)
exists and throw or Assert with a clear message if
Directory.Exists(modelRepositoryPath) is false; update the setup where
TritonBuilder(...).WithResourceMapping(modelRepositoryPath, "/models/") is
called so the existence check runs first (referencing modelRepositoryPath and
the WithResourceMapping call) to provide an immediate, actionable error instead
of letting container startup fail later.
---
Nitpick comments:
In `@tests/Testcontainers.Triton.Tests/TritonContainerTest.cs`:
- Around line 116-119: Replace the plain-text assertions on responseBody with
structured JSON assertions: parse responseBody into a JSON object (from
responseBody variable obtained from httpResponse) and assert that the JSON
contains the "OUTPUT0" property and that its value equals 42 (use the test
framework's Assert.Equal/Assert.True on the parsed token/value). Ensure you
reference the same responseBody variable and perform the parse before the
assertions so the test deterministically validates the inference output rather
than matching incidental substrings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6f07a3ed-9fa4-4424-9cab-9eac114e5349
📒 Files selected for processing (4)
src/Testcontainers.Triton/TritonBuilder.cssrc/Testcontainers.Triton/Usings.cstests/Testcontainers.Triton.Tests/TritonContainerTest.cstests/Testcontainers.Triton.Tests/Usings.cs
✅ Files skipped from review due to trivial changes (2)
- src/Testcontainers.Triton/Usings.cs
- tests/Testcontainers.Triton.Tests/Usings.cs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Testcontainers.Triton/TritonBuilder.cs
| var modelRepositoryPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "models"); | ||
|
|
||
| _tritonContainer = new TritonBuilder(TestSession.GetImageFromDockerfile()) | ||
| .WithCommand("tritonserver", "--model-repository=/models") | ||
| .WithResourceMapping(modelRepositoryPath, "/models/") | ||
| .Build(); |
There was a problem hiding this comment.
Fail fast when the mounted models directory is missing.
If models is not copied to the test output, the failure happens later during container startup with a less actionable error. Add an explicit guard near Line 9.
Suggested patch
public TritonContainerTest()
{
var modelRepositoryPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "models");
+ if (!Directory.Exists(modelRepositoryPath))
+ {
+ throw new DirectoryNotFoundException($"Expected models directory was not found: {modelRepositoryPath}");
+ }
_tritonContainer = new TritonBuilder(TestSession.GetImageFromDockerfile())
.WithCommand("tritonserver", "--model-repository=/models")
.WithResourceMapping(modelRepositoryPath, "/models/")
.Build();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var modelRepositoryPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "models"); | |
| _tritonContainer = new TritonBuilder(TestSession.GetImageFromDockerfile()) | |
| .WithCommand("tritonserver", "--model-repository=/models") | |
| .WithResourceMapping(modelRepositoryPath, "/models/") | |
| .Build(); | |
| var modelRepositoryPath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "models"); | |
| if (!Directory.Exists(modelRepositoryPath)) | |
| { | |
| throw new DirectoryNotFoundException($"Expected models directory was not found: {modelRepositoryPath}"); | |
| } | |
| _tritonContainer = new TritonBuilder(TestSession.GetImageFromDockerfile()) | |
| .WithCommand("tritonserver", "--model-repository=/models") | |
| .WithResourceMapping(modelRepositoryPath, "/models/") | |
| .Build(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/Testcontainers.Triton.Tests/TritonContainerTest.cs` around lines 9 -
14, The test should fail fast when the host models directory is missing: before
constructing the TritonContainer, check that modelRepositoryPath (the
Path.Combine(AppDomain.CurrentDomain.BaseDirectory, "models") variable) exists
and throw or Assert with a clear message if
Directory.Exists(modelRepositoryPath) is false; update the setup where
TritonBuilder(...).WithResourceMapping(modelRepositoryPath, "/models/") is
called so the existence check runs first (referencing modelRepositoryPath and
the WithResourceMapping call) to provide an immediate, actionable error instead
of letting container startup fail later.
| protected override void Validate() | ||
| { | ||
| base.Validate(); | ||
|
|
||
| const string message = "Triton Inference Server requires a command to be specified. " + | ||
| "Use .WithCommand(\"tritonserver\", \"--model-repository=/path/to/models\") to configure the server."; | ||
|
|
||
| _ = Guard.Argument(DockerResourceConfiguration.Command, nameof(DockerResourceConfiguration.Command)) | ||
| .ThrowIf(argument => argument.Value == null || !argument.Value.Any(), _ => new ArgumentException(message)); | ||
| } |
There was a problem hiding this comment.
I wrote the following before the recent changes, and I think it's better. WDZT about a dedicated API?
I think with the current implementation, getting started with the module is difficult. Users probably won't realize they need to pass an additional command to run the container/module.
In Testcontainers, we often take an opinionated approach to modules. I suggest we add something like WithModelRepository(...) that applies the necessary pre-configuration (including common setups like a remote host, etc.).
We could then throw an exception on validation if the Command property doesn't specify any model, with a clear error message explaining what's missing. That way, users understand what the module expects.
Testcontainers modules don't need to support every possible configuration or edge case, just the most important ones. If users need more flexibility, they can always fall back to the generic container builder or fully override the commands.
There was a problem hiding this comment.
What bothers me is that if I implement some minimal model loading, like
var defaultModelsDir = "/some/path/without/collisions"
base.Init()
.WithTmpfsMount(defaultModelsDir)
.WithCommand("tritonserver", $"--model-repository={defaultModelsDir}")It would mean that the following WithModelRepository would need to modify the command line instead of just calling WithCommand. And it may get trickier given that the user might call WithCommand for different command line parameters before/after/multiple times, override the command completely with OverwriteEnumerable, etc. Not sure how brittle these helpers would be but I agree it's possible and it would help to ease a simple usecase, like WithLocalModelRepository(string dirToMount) and WithRemoteModelRepository(string url).
Regarding the validation I feel like it could be too restrictive. We could check that the command begins with tritonserver and includes --model-repository but what if the user wants to do something before calling tritonserver (the entrypoint in this container seem to allow that). On the other hand with a working zero-conf container we could just skip the validation.
There was a problem hiding this comment.
It would mean that the following
WithModelRepositorywould need to modify the command line instead of just callingWithCommand.
What we usually do is set the binary in Init() and then append or override the command. This is a very common pattern in TC, like:
testcontainers-dotnet/src/Testcontainers.Azurite/AzuriteBuilder.cs
Lines 134 to 135 in a2f5717
And it may get trickier given that the user might call
WithCommandfor different command line parameters before/after/multiple times, override the command completely withOverwriteEnumerable, etc.
This is a general "issue" with pre-configured modules. Their purpose is to cover common use cases, not all kinds of configurations. E.g. if you look at Kafka, it pre-configures a lot, if users start to override the module configuration (entrypoint, cmd, etc.), it might not work anymore. As I said, if users need full flexibility, they can use the generic builder. Modules are pre-configured, opinionated, containing best practices and serve as a good example. Usually I expect modules to add some kind of additional value.
Does tritonserver always require a model repository? If so, I would recommend at least validating --model-repository and providing an API to apply the configuration. Otherwise, we could check whether any Command is set and, if not, throw a meaningful exception that explains what is expected from the developer (not my favorite).
WDYT?
There was a problem hiding this comment.
Hello, after looking more into it and investigating our internal usecase deeper, I believe that a generic ContainerBuilder would suit us better this time.
I realized the larger community would certainly expect to run GPU models with Testcontainers.Triton which requires significant investment to make things easy: smth like .WithGpuSupport() which will construct correct DeviceRequests (and there are driver parameters involved). Same for model repository, indeed implementing dedicated helpers for the most common options would be more aligned with the testcontainers purpose. I also learnt that a relatively common case is to use multiple repositories at the same time, with combined auth configurations, etc.
Internally, even though we use GPU models, we don't really have a usecase to test them from inside the application, i.e. mocked CPU models are enough, as well as locally mounted model repository. So I unfortunately do not have a dogfooding setup complex enough to create a good "opinionated" container wrapper. Otherwise, I'd have pushed forward with this PR (and I will if the situation changes and our container builder code starts to grow / I have something I'm sure is useful).
So sorry for taking your time to review this. I'm very grateful for the discussion and learnt a lot from it.
There was a problem hiding this comment.
No worries. Then we can close the issue and PR?
2 participants
What does this PR do?
Introduce
Testcontainers.Tritonpackage which wraps Nvidia Triton inference server:Why is it important?
Avoids setting up a custom wrapper, gives a pre-configured alternative to anyone who may need it.
Related issues
Solves #1677
Summary by CodeRabbit
New Features
Chores
Tests