[Core feature] Support default container configuration in default PodTemplates

[sethmz]

Motivation: Why do you think this is important?

At my company, all of our containers have some commonly required and default configurations (e.g. environment variables, volume mounts, etc). Flyte task pods should not be an exception. Although there is no k8s concept of a ContainerTemplate, we would like to put this common configuration into the default PodTemplate used by Flyte. Currently, containers defined in a default PodTemplate are thrown out by flyte-propeller when generating task pods. This means we cannot use the PythonTask type out-of-the-box in our k8s environment, as the task containers that are generated will be missing configuration needed by our task code.

Goal: What should the final outcome look like, ideally?

We would like the "placeholder" or "noop" container that must be present in the default PodTemplate to serve as a template for all containers in the pod. That way things like default images, custom environment variables, and volume mounts can be configured in the namespace's default PodTemplate.

For example:

kind: PodTemplate
metadata:
  name: default-flyte-pod
template:
  spec:
    containers:
      - name: container-template
        image: default/image:tag
        env:
          - name: FOO
            value: bar
        volumeMounts:
          - name: default-mount
            mountPath: /default
    volumes:
      - name: default-volume
        emptyDir: {}
...

Instead of throwing out the container-template container or merging it with another container definition with the same name, its configuration (image, env, and volumeMounts in this example) would be the base for all generated containers in the pod.

Describe alternatives you've considered

We are currently able to arbitrarily configure task containers by using PodTasks. (Other users could also use a ContainerTask, but we are unable to as we do not enable PTRACE for security reasons.) Although this works, it requires every one of our tasks to specify the full PodSpec in their task config. We can hide this with our own abstractions, but we would like to be able to use simple PythonTasks as much as possible.

We are also able to set some things like environment variables in the k8s plugin config read by flyte-propeller, but this older interface is more limited and does not seem to work alongside default PodTemplates.

Propose: Link/Inline OR Additional context

No response

Are you sure this issue hasn't been raised already?

• Yes

Have you read the Code of Conduct?

• Yes

[welcome]

Thank you for opening your first issue here! 🛠

[pingsutw]

@sethmz You want to add a default pod template that has image, volume, and env to the propeller configmap, and the pod template of the python task or raw container task will include those settings, right? I think it's a good idea.

[pingsutw]

It looks like we already support it. check this PR.

[sethmz]

@pingsutw thanks for taking a look. What I'm proposing is an extension to the feature introduced in that PR.

From the PR:

the containers field in any default PodTemplate, while required to be set, is ignored completely when building a Pod. This functionally is open for discussion and will likely be revisited.

I want to use the first container in the containers field as a base for all containers in the pod.

[hamersaw]

@sethmz great that you brought this up! Have you seen this issue? We are currently working on a PR as well. Would love your input on this, it seems that we have covered your use-case, but maybe there is something we overlooked. If it does indeed cover, we can close this issue as a duplicate.

[hamersaw]

I think the only difference currently is that the proposed container configuration would only apply to a primary container rather than every container in the Pod. In Flyte we are defining the primary container as the only that needs to complete for the task to complete, so things like sidecars can run indefinitely and not have effect. Would this be an issue?

[sethmz]

@hamersaw ah, I didn't see that issue, sorry! I was talking with Ketan about this yesterday and he suggested I open an issue.

I think the primary container will be sufficient for most of our cases. However, one use case we have for pod tasks is to support non-Python and legacy code that can't use flytekit natively. The primary container in this case is simply an interface to flyte, whereas the real work is being done in the sidecars. So in that case, the sidecars need the custom configuration probably more so than the primary container. That use case may be too niche, though.

[hamersaw]

Oh sure, so right now we were going to apply the container configuration if the container name was "primary" (or something). I'm wondering if it makes sense to support two container names "primary" and "default" where default will serve as the base for ALL containers in the Pod and primary is only applied to the primary container. I think this would support almost all of the use cases I can envision.

The other thing we discussed was adding other containers (non "primary" or "default"). If the default PodTemplate contained a container called for example "foo", does it make sense to add this container to all Pods? Right now, I'm not sure there's a clear use case. It would be pretty easily to add this functionality in the future if desired.

[sethmz]

Supporting "primary" and "default" containers sounds good to me. 🙏

As for additional containers, it sounds like it might apply to use cases in which every pod needs some kind of proxy like in a service mesh. Although, I think such things are usually injected into pods by mutating webhooks via the k8s admission controller. So I would agree that it doesn't seem like a necessary feature at the moment.

[hamersaw]

Closing as duplicate of this issue