Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions components/src/dynamo/planner/monitoring/dgd_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,15 @@ def get_component_from_type_or_name(
component_names = [name for name, _ in matching_components]
raise DuplicateSubComponentError(component_type.value, component_names)

if not matching_components and component_type == SubComponentType.DECODE:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Sign the commit with DCO

The reviewed commit has no Signed-off-by: trailer, so it violates the repository requirement to sign every commit with git commit -s and will not satisfy DCO validation. Recreate this commit with the required sign-off before merging.

AGENTS.md reference: AGENTS.md:L131-L138

Useful? React with 👍 / 👎.

generic_workers = [
(name, component)
for name, component in components.items()
if get_component_type(component) == V1BETA1_GENERIC_WORKER_COMPONENT_TYPE
]
if len(generic_workers) == 1:
matching_components = generic_workers
Comment on lines +213 to +220

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Explicitly configured decode worker can be ignored in favour of a differently-named worker

The lone generic worker is picked as the decode worker (generic_workers check at components/src/dynamo/planner/monitoring/dgd_services.py:213-220) before the explicitly configured worker name is even looked at, so the planner can scale a worker the operator never asked it to scale.
Impact: In a deployment that names its decode worker explicitly while another worker is only generically labelled, the planner resizes the wrong worker group, breaking autoscaling for the intended one.

Precedence between type-based fallback and explicit component name

In get_component_from_type_or_name, the new block runs before the component_name in components branch (components/src/dynamo/planner/monitoring/dgd_services.py:222-226). Consider a v1beta1 DGD with component A of type: worker (actually prefill) and component B with no type field, where the caller passes component_name="B" for SubComponentType.DECODE. No component has type: decode, so matching_components is empty; the fallback finds exactly one generic worker (A) and selects it, and the explicit-name branch is skipped. Previously B would have been selected via _can_use_explicit_component_name (which explicitly permits an empty type, see components/src/dynamo/planner/monitoring/dgd_services.py:88-96). All callers that pass decode_component_name (components/src/dynamo/planner/connectors/kubernetes.py:236-240, :660-664, :700-704) are affected, including replica updates.

A safer ordering is to try the explicit name first and only use the sole-generic-worker fallback when no usable explicit name is given.

Suggested change
if not matching_components and component_type == SubComponentType.DECODE:
generic_workers = [
(name, component)
for name, component in components.items()
if get_component_type(component) == V1BETA1_GENERIC_WORKER_COMPONENT_TYPE
]
if len(generic_workers) == 1:
matching_components = generic_workers
if (
not matching_components
and component_type == SubComponentType.DECODE
and component_name not in components
):
generic_workers = [
(name, component)
for name, component in components.items()
if get_component_type(component) == V1BETA1_GENERIC_WORKER_COMPONENT_TYPE
]
if len(generic_workers) == 1:
matching_components = generic_workers
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


if not matching_components and component_name in components:
component = components[component_name]
if not _can_use_explicit_component_name(component, component_type):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,55 @@ def test_get_service_name_from_v1beta_worker_type_by_name(kubernetes_connector):
assert service.number_replicas() == 2


def test_get_service_name_from_unique_v1beta_worker_type_for_decode(
kubernetes_connector,
):
deployment = _deployment(_component("arbitrary-name", "worker", replicas=2))

service = get_component_from_type_or_name(deployment, SubComponentType.DECODE)

assert service.name == "arbitrary-name"
assert service.number_replicas() == 2


def test_get_service_name_from_multiple_v1beta_workers_by_name(
kubernetes_connector,
):
deployment = _deployment(
_component("prefill-name", "worker"),
_component("decode-name", "worker"),
)

with pytest.raises(SubComponentNotFoundError):
get_component_from_type_or_name(deployment, SubComponentType.DECODE)

service = get_component_from_type_or_name(
deployment, SubComponentType.DECODE, "decode-name"
)
assert service.name == "decode-name"


@pytest.mark.asyncio
async def test_validate_deployment_agg_worker_by_type(
kubernetes_connector, mock_kube_api
):
mock_kube_api.get_graph_deployment.return_value = _deployment(
_component("Frontend", "frontend", replicas=1),
_component(
"arbitrary-name",
"worker",
replicas=2,
args=["--model", "Qwen/Qwen3-8B"],
),
)

await kubernetes_connector.validate_deployment(
decode_component_name="stale-default-name",
require_prefill=False,
require_decode=True,
)


def test_get_service_name_from_sub_component_type_not_found(kubernetes_connector):
deployment = _deployment(_component("test-component-decode", "decode", replicas=3))
with pytest.raises(SubComponentNotFoundError) as exc_info:
Expand Down
Loading