Pull Request Test Coverage Report for Build 21140900008

Details

• 8 of 50 (16.0%) changed or added relevant lines in 2 files are covered.
• 3 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.04%) to 36.795%

Totals
Change from base Build 21132355090:	-0.04%
Covered Lines:	5258
Relevant Lines:	14290

💛 - Coveralls

The only comment I had was addressed with a recent commit update.

Note, as currently implemented in the operator, setting RUNTIME=none in the toolkit will not work. We process user-supplied envvars here but we call transformForRuntime() right after which unconditionally sets the RUNTIME variable (this will override what the user set). I believe we decided to call transformForRuntime last since any envvars configured in that method are considered to be tightly-coupled with the runtime, and therefore, we do not expect users to set these themselves. I am open to revisiting the behavior here though.

cc @tariq1890

Note, as currently implemented in the operator, setting RUNTIME=none in the toolkit will not work. We process user-supplied envvars here but we call transformForRuntime() right after which unconditionally sets the RUNTIME variable (this will override what the user set). I believe we decided to call transformForRuntime last since any envvars configured in that method are considered to be tightly-coupled with the runtime, and therefore, we do not expect users to set these themselves. I am open to revisiting the behavior here though.

cc @tariq1890

Thanks for calling this out @cdesiniotis. You're correct then that this is only a piece of the required functionality.

@just1not2 it may be good to expand #1513 with additional requirements / details on how you would like the operator to behave and also how you are currently configuring it.

Converting this to a draft until we have clarified things.

Update: Created NVIDIA/gpu-operator#2041 to add basic support for this to the operator. The thinking is that if a user explicitly sets RUNTIME=none in the toolkit enf, this disables the injection of sockets and config files so that no runtime configuration is performed.

@elezar I lean towards using a flag to trigger a certain behaviour as opposed to relying on a specific value (RUNTIME=none) to trigger said behaviour. Moreover the notion of RUNTIME=none is rather ambiguous and could be interpreted in multiple ways by users.

I don't want to block on this if others are okay with the proposed approach, but my preference right is now if we used a flag like --no-runtime-config (as discussed offline) instead

@elezar I lean towards using a flag to trigger a certain behaviour as opposed to relying on a specific value (RUNTIME=none) to trigger said behaviour. Moreover the notion of RUNTIME=none is rather ambiguous and could be interpreted in multiple ways by users.

I don't want to block on this if others are okay with the proposed approach, but my preference right is now if we used a flag like --no-runtime-config (as discussed offline) instead

I think a dedicated flag makes more sense too. Abusing the RUNTIME envvar was motivated by my (incorrect) assumption that this would not require changes to the GPU Operator. Since these are required anyway, let me rework this.

I have also updated NVIDIA/gpu-operator#2041 for this specific implementation. (It's not strictly needed to function correctly, but does mean that the mounts are cleaner).