Skip to content

Commit 67dc296

Browse files
popojkclaudepingsutw
authored
Preserve DRA resource claims when overriding task resources (#1324)
Signed-off-by: Alex Wu <c.alexwu@gmail.com> Signed-off-by: Kevin Su <pingsutw@apache.org> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Kevin Su <pingsutw@apache.org>
1 parent f3811a7 commit 67dc296

4 files changed

Lines changed: 103 additions & 1 deletion

File tree

src/flyte/_internal/runtime/task_serde.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,11 @@ def _get_k8s_pod(primary_container: tasks_pb2.Container, pod_template: PodTempla
418418
existing = container.resources or V1ResourceRequirements()
419419
merged_limits = {**(existing.limits or {}), **limits}
420420
merged_requests = {**(existing.requests or {}), **requests}
421-
container.resources = V1ResourceRequirements(limits=merged_limits, requests=merged_requests)
421+
container.resources = V1ResourceRequirements(
422+
limits=merged_limits,
423+
requests=merged_requests,
424+
claims=existing.claims,
425+
)
422426

423427
if primary_container.env is not None:
424428
container.env = [V1EnvVar(name=e.key, value=e.value) for e in primary_container.env] + (

src/flyte/remote/_task.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,14 @@ def _apply_overrides_to_primary_container(
7373
if proto_res is not None:
7474
requests = {_sanitize_resource_name(e): e.value for e in proto_res.requests}
7575
limits = {_sanitize_resource_name(e): e.value for e in proto_res.limits}
76+
existing = target.get("resources") or {}
7677
rr: Dict[str, Any] = {}
7778
if requests:
7879
rr["requests"] = requests
7980
if limits:
8081
rr["limits"] = limits
82+
if existing.get("claims"):
83+
rr["claims"] = existing["claims"]
8184
target["resources"] = rr
8285

8386
if env_vars:

tests/flyte/internal/runtime/test_task_serde.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@
99
from flyteidl2.core.security_pb2 import SecurityContext
1010
from flyteidl2.task import common_pb2, environment_pb2
1111
from kubernetes.client import (
12+
CoreV1ResourceClaim,
1213
V1Container,
1314
V1EnvVar,
1415
V1LocalObjectReference,
16+
V1PodResourceClaim,
1517
V1PodSpec,
18+
V1ResourceRequirements,
1619
)
1720

1821
import flyte
@@ -279,6 +282,59 @@ async def t2(a: int, b: str) -> str:
279282
get_proto_task(t2, context)
280283

281284

285+
def test_get_k8s_pod_preserves_dra_claims_with_task_resources():
286+
"""A DRA resource claim on the pod template's primary container must survive
287+
even when the task also declares cpu/memory via Resources(...)."""
288+
pod_template = PodTemplate(
289+
pod_spec=V1PodSpec(
290+
resource_claims=[V1PodResourceClaim(name="gpu", resource_claim_template_name="flyte-task-gpu-x2")],
291+
containers=[
292+
V1Container(
293+
name="primary",
294+
resources=V1ResourceRequirements(claims=[CoreV1ResourceClaim(name="gpu")]),
295+
)
296+
],
297+
),
298+
)
299+
300+
env = flyte.TaskEnvironment(
301+
name="test_env",
302+
image="python:3.10",
303+
# Declaring cpu/memory is exactly what used to strip the .claims link.
304+
resources=flyte.Resources(cpu="24", memory="490Gi"),
305+
pod_template=pod_template,
306+
)
307+
308+
@env.task(short_name="dra_task")
309+
async def t1(a: int) -> int:
310+
return a
311+
312+
context = SerializationContext(
313+
project="test-project",
314+
domain="test-domain",
315+
version="test-version",
316+
org="test-org",
317+
input_path="/tmp/inputs",
318+
output_path="/tmp/outputs",
319+
image_cache=None,
320+
code_bundle=None,
321+
root_dir=pathlib.Path.cwd(),
322+
)
323+
324+
from google.protobuf import json_format
325+
326+
k8s_pod = _get_k8s_pod(_get_urun_container(context, t1), pod_template)
327+
pod_spec = json_format.MessageToDict(k8s_pod.pod_spec)
328+
primary = next(c for c in pod_spec["containers"] if c["name"] == "primary")
329+
resources = primary["resources"]
330+
331+
# The DRA claim link is preserved...
332+
assert resources["claims"] == [{"name": "gpu"}]
333+
# ...alongside the task-declared cpu/memory.
334+
assert resources["requests"]["cpu"] == "24"
335+
assert resources["requests"]["memory"] == "490Gi"
336+
337+
282338
@pytest.fixture(scope="module")
283339
def env_task_ctx():
284340
# Create a real task environment

tests/flyte/remote/test_task.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,45 @@ def test_gpu_override_on_pod_template_sets_container_request(self):
289289
side = next(c for c in spec["containers"] if c["name"] == "sidecar")
290290
assert not side.get("resources")
291291

292+
def test_resource_override_on_pod_template_preserves_dra_claims(self):
293+
# Regression: a resource override on a pod-template task must not drop a
294+
# DRA claim (e.g. a GPU claimed through a ResourceClaimTemplate). The
295+
# claim can only be expressed via the pod template; without it the pod
296+
# schedules with no device.
297+
from flyteidl2.task import task_definition_pb2
298+
from google.protobuf import json_format
299+
300+
from flyte._pod import _PRIMARY_CONTAINER_NAME_FIELD
301+
302+
pb2 = task_definition_pb2.TaskDetails()
303+
tmpl = pb2.spec.task_template
304+
tmpl.config[_PRIMARY_CONTAINER_NAME_FIELD] = "primary"
305+
spec = {
306+
"resourceClaims": [{"name": "gpu", "resourceClaimTemplateName": "flyte-task-gpu-x2"}],
307+
"containers": [
308+
{
309+
"name": "primary",
310+
"image": "example:latest",
311+
"resources": {
312+
"requests": {"cpu": "1", "memory": "1Gi"},
313+
"claims": [{"name": "gpu"}],
314+
},
315+
},
316+
],
317+
}
318+
json_format.ParseDict(spec, tmpl.k8s_pod.pod_spec)
319+
td = TaskDetails(pb2)
320+
321+
out = td.override(resources=flyte.Resources(cpu="24", memory="490Gi"))
322+
tmpl = out.pb2.spec.task_template
323+
324+
spec = json_format.MessageToDict(tmpl.k8s_pod.pod_spec)
325+
primary = next(c for c in spec["containers"] if c["name"] == "primary")
326+
resources = primary["resources"]
327+
assert resources["claims"] == [{"name": "gpu"}], f"DRA claim dropped: {resources}"
328+
assert resources["requests"]["cpu"] == "24"
329+
assert resources["requests"]["memory"] == "490Gi"
330+
292331
def test_env_override_on_pod_template_sets_container_env(self):
293332
# Regression: env_vars overrides on a pod-template task were silently
294333
# dropped — they were only applied to template.container, which a

0 commit comments

Comments
 (0)