Support multiple containers in the Kubernetes Transport implementation#5306
Conversation
oheger-bosch
force-pushed
the
oheger-bosch/kubernetes_multiple_containers
branch
from
c8c2395 to
076d2ae
Compare
|
@mnonnenmacher, do you have an idea about the failing build health check? This may be related to the introduction of the fluent API client dependency in c170f77, but I have no idea why it now suddenly pops up. Copilot says that According to the AI, there is no artifact available containing only the fluent builder classes; so no easy way to work around the duplicated classes issues except for avoiding the fluent API. It therefore suggests to rewrite the Do you agree to this approach or do you have a better proposal? |
The warnings just are just noise that makes it harder to find the actual problem, I didn't find a way to suppress them:
You can also auto-fix such errors using the The duplicate class issue was reported upstream but was auto-closed, so I don't expect that it will be fixed. But IIUC the classes are generated and identical so technically it should be fine. |
Great, this did the trick! Many thanks. |
oheger-bosch
requested review from
MarcelBochtler,
bs-ondem,
mnonnenmacher,
nnobelis,
sschuberth and
wkl3nk
as code owners
| imageName = System.getenv("${prefix}IMAGE_NAME") ?: mainContainer.imageName, | ||
| imagePullPolicy = System.getenv("${prefix}IMAGE_PULL_POLICY") ?: mainContainer.imagePullPolicy, | ||
| commands = System.getenv("${prefix}COMMANDS")?.splitAtWhitespace() ?: mainContainer.commands, | ||
| args = System.getenv("${prefix}ARGS")?.splitAtWhitespace() ?: mainContainer.args, | ||
| cpuLimit = System.getenv("${prefix}CPU_LIMIT"), | ||
| cpuRequest = System.getenv("${prefix}CPU_REQUEST"), | ||
| memoryLimit = System.getenv("${prefix}MEMORY_LIMIT"), | ||
| memoryRequest = System.getenv("${prefix}MEMORY_REQUEST"), | ||
| volumeMounts = System.getenv("${prefix}VOLUME_MOUNTS").splitAt(splitCommaListRegex) |
There was a problem hiding this comment.
Shouldn't this also use the Config abstraction for consistency and more importantly it would allow to configure this in other means than env variables.
There was a problem hiding this comment.
With the Config interface, there is no mechanism I am aware of that could handle lists of objects with an arbitrary number of elements, which would be needed to define the additional containers and their properties. Therefore, I went for the approach to have one property with a list of container names, and the properties of these containers then have to be looked up via environment variables.
oheger-bosch
force-pushed
the
oheger-bosch/kubernetes_multiple_containers
branch
from
0cfff75 to
f35968e
Compare
Extract a common interface for volume mount declarations and move the logic to generate the Kubernetes manifest to the concrete implementations. This simplifies the handling of such mounts. It is also a preparation for an upcoming extension of the Kubernetes transport implementation to support the creation of multiple containers in a pod. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
The declaration of a volume mount can now include an optional name assignment. This is going to be used later to configure which volume mount should be added to which container in the resulting pod. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
This internal helper function from the `:utils:config` module is useful in general. Therefore, convert it to an extension function for easy usage and make it public. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
oheger-bosch
force-pushed
the
oheger-bosch/kubernetes_multiple_containers
branch
from
f35968e to
e10de3d
Compare
Collect the properties for a container to create in a separate data class. This is a preparation to support the declaration of multiple containers. To make this possible, slightly rework the mechanism for variable substitution. This is no longer done by the configuration, but by the sender when creating the Kubernetes manifest. This simplifies the code a bit because no copy of the configuration with variables for interpolation has to be created. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
It is now possible to declare additional containers with all supported properties. This enables some advanced use cases like adding side-car containers to the ORT Server workers. Extend the declaration of volume mounts, so that named volume mounts can be added to a subset of containers only. This makes it possible for instance, to mount secrets only into specific containers. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
Allow additional containers to be declared as init containers. Handle them correctly when setting up the Kubernetes manifest. This makes it possible to run some initialization tasks before an ORT Server worker gets active. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
Describe the newly added functionality for additional containers and named volume mounts. Restructure the documentation to distinguish between pod-related and container-related configuration properties. Order the properties alphabetically. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
This is necessary to fix a failed build-health check. Signed-off-by: Oliver Heger <oliver.heger@bosch.com>
oheger-bosch
force-pushed
the
oheger-bosch/kubernetes_multiple_containers
branch
from
e10de3d to
ba1ad98
Compare
e629e1c
3 participants
This PR extends the Kubernetes Transport to allow the declaration of multiple containers. This enables advanced use cases like defining init containers or sidecar containers. Refer to the single commits for further information.