Is there any way to define a `ContainerTask` with ...
# ask-the-community
e
Is there any way to define a
ContainerTask
with a toleration or affinity? It looks like I may have define a custom
Task
that overrides the pod spec construction method right now?
The main issue seems to be that
ContainerTask
forces
task_config=None
in its
super().__init__
call, which prevents callers from forwarding a
task_config
up to
PythonTask
n
seems like a reasonable thing to forward a user-defined config into
super().__init__
, unless I’m missing something @Eduardo Apolinario (eapolinario) @Ketan (kumare3)?
d
@Eli Bixby have you seen the issue on adding PodTemplates to tasks? I believe this will cover your use-case right? Something like:
Copy code
t = ContainerTask(
    name="foo",
    image="bar:latest",
    command=[
        "echo",
        "hello",
        "{{.inputs.b}}",
        "/var/outputs",
    ],
    pod_template=PodTemplate(
        spec=PodSpec(
            tolerations={...}
        )
    )
)
This have been pretty high priority for us, because many users are interested. We're planning on having a beta release out later this week.
e
@Dan Rammer (hamersaw) yes I think that's a good idea but not relevant here. I want to define a task without a python function (rather with a container image, and a command)
It seems like the pod plugin almost does what we need except it needs a mixin pattern rather than assuming the user wants to inherit the whole python function stack
d
So we plan to support
ContainerTask
. We'll have a beta out later this week - but unfortunately this support will not be a part of it. It looks like you've dove into this a little bit, would you be interested in contributed support for specifying
pod_template
for
ContainerTask
?
The plan is to deprecate the current pod plugin at some point with this new
pod_template
support. It does everything the pod plugin does, and more.
e
Sounds good. I'll have to look into the design to see how easy it is to adapt to ContainerTask, but it doesn't seem like it would be too difficult.
d
Yeah, once we get the rest of the infrastructure to support this feature it should be trivial.
e
Yeah looks really straight forward, especially with the other MR already doing the heavy lifting on the SDK side. AFAICT
ContainerTask
becomes basically a no-op once you have a pod template? I'm assuming the sidecar container gets injected server-side for
raw-container
type tasks? because I don't see its definition anywhere in python.
d
I'm assuming the sidecar container gets injected server-side
absolutley, actually happens here
e
• Cool. Then the only question I have is how a primary container defined as part of a template should be merged with the one generated by the
ContainerTask
definition; ◦ Fail if both are defined at all. ◦ Deep merge failing on mismatch ◦ Deep merge warning/log on mismatch My understanding of how people use PodTemplates is incomplete, but I'm assuming there are compelling reasons to have primary container definitions included (e.g. volume mounts), so I would think the 3rd option is best.
f
e
@Felix Ruess my understanding from @Dan Rammer (hamersaw) is that the pod plugin is going to be deprecated in favor of the
pod_template
argument, so this would solve that issue indirectly
f
yes, indeed
I also need this ASAP to set node affinity per task, the rest I could do with per project/domain pod templates so far
d
@Eli Bixby I think you're right. A deep merge with logging may be the best approach. Currently we support a "default" PodTemplate as a k8s resource that gets applied to all Pods created by Flyte. This uses the PodTemplate as base configuration and then anything the user specifies overrides it. I think it makes sense to emulate this functionality - of course open to discussion here.
e
ContainerTask also seems be broken for local execution 😞
Or isn't intended to work more likely looking at the code.
d
Yeah, unfortunately I'm not sure if it's necessarily "broken" as much as "not supported". Running a raw container means there needs to be a dependency on some CRI, so it can't really be run as a raw python func.
e
Taking an optional path to a CRI binary, and allowing string outputs or directory outputs to be output path or captured stdout from a subprocess seems reasonable to me? At the very least it should raise a NotImplemented or something
d
That would make sense to me! cc @Eduardo Apolinario (eapolinario) ^^ updating ContainerTask local execution.
f
Yeah, I actually wanted to start implementing local execution (assuming docker) via subprocess, but quickly realized that this would be of no help to me, as I need GPU support for all workflows anyway and I don't have it locally... So I immediately went with the remote cluster.
k
@Eli Bixby / @Felix Ruess you can mock today - But I think some one added a dependency to docker. That is not acceptable to many people because of docker license issues - If you think you can add support and upstream highly welcome to do this. The core team currently does not have a plan to work on it. Caution use oci client
e
I wouldn't add a dependency, I would just ask users to pass a path to a client binary in their local environment, if they wanted local execution. Probably best set in config somewhere globally.
k
That sounds good
How about contributing 😍
Flytekit has the code and flytesnacks the example
159 Views