Replace shell wrapper with a Go wrapper #700
Conversation
|
An alternative to the side files would be to embed the arguments and environments in custom ELF sections. It felt more opaque than side files, but let me know if you'd prefer that approach and I'll implement it. |
There was a problem hiding this comment.
@jfroy one thought I would have here is whether it makes sense to rather pull this functionality into the main runtime and feature gate it or obfuscate it behind a mode flag. We already produce mode-specific binaries for cdi and legacy for use on the operator and this could be an extension of that.
Not sure what @klueska's thoughts are on this though.
|
The nvidia module detection logic seems like a natural fit to be moved in the runtime program. It's less clear that the argv and envv stuff would fit there -- the purpose of that indirection is to allow the gpu-operator to customize the installation of the toolkit to the cluster, and to its parameters (e.g. host-provided driver vs driver container, etc). -- As a third alternative implementation, the wrapper could look for a dot-config file recursively up the directory tree, à la .gitconfig. This would be a good fit as the wrappers mostly all have the same argv and envv computed by the toolkit installer. |
It may not have to be up the tree. We could create a single By the way, I have created #763 which should allow us to simplify this implementation further since we would not have to deal with command line arguments in addition to envvironment variables. |
| if err := scanner.Err(); err != nil { | ||
| log.Fatalf("failed to read argv file: %v", err) | ||
| } | ||
| return append(env, os.Environ()...) |
There was a problem hiding this comment.
Don't later envvars override earlier ones? Should we not reverse the ordering of this?
Assuming all the tools can use a single set of environment variables, that would work,
If I remember correctly, this means the patch could drop the ability to set arguments and only handle environment variables. |
jfroy
force-pushed
the
go-wrapper
branch
2 times, most recently
from
753b731
to
79a3a99
Compare
Some platforms and Kubernetes distributions do not include a shell. This patch replaces the shell wrapper scripts with a small Go program. Signed-off-by: Jean-Francois Roy <[email protected]>
This patch modifies the the container toolkit installer, used by the GPU operator, to use the new Go wrapper program. Signed-off-by: Jean-Francois Roy <[email protected]>
Signed-off-by: Jean-Francois Roy <[email protected]>
This will be handled by the runtime itself. Signed-off-by: Jean-Francois Roy <[email protected]>
This PR replaces the shell wrapper used by the container toolkit installed with a Go program. This allows the GPU operator to install the toolkit on distributions that do not have a shell, such as Talos Linux.
The implementation of the Go wrapper uses side files (.argv, .envv) to provide injected arguments and environment variables, which are prepended to those supplied to the wrapper.
For environment variables, special prefixes can be used to prepend or appent to a colon-delimited list variable, such a
PATH. '<' is used to prepend and '>' is used to append. For example,<PATH=foowill prependfootoPATH.Finally, the wrapper includes special logic to detect that it is wrapping one of the runtime programs and if so check if the nvidia kernel modules are loaded. If not, the wrapper will execute
runcinstead.