Skip to content

Commit 3666b3f

Browse files
jopemachineclaude
andauthored
fix(BA-5963): report current_revision_id correctly during rolling updates (#11494)
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 35aa1d5 commit 3666b3f

7 files changed

Lines changed: 109 additions & 9 deletions

File tree

changes/11494.fix.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Report `current_revision_id` correctly on deployment responses during rolling updates.

src/ai/backend/manager/api/adapters/deployment/adapter.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2191,7 +2191,7 @@ def _deployment_data_to_dto(data: ModelDeploymentData) -> DeploymentNode:
21912191
created_user_id=data.created_user_id,
21922192
options=deployment_options_to_info(data.options),
21932193
scaling_state=data.scaling_state,
2194-
current_revision_id=data.revision.id if data.revision is not None else None,
2194+
current_revision_id=data.current_revision_id,
21952195
deploying_revision_id=data.deploying_revision_id,
21962196
policy=policy_info,
21972197
)

src/ai/backend/manager/api/rest/service/handler.py

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
DeploymentNetworkSpec,
6969
ExecutionSpec,
7070
ImageIdentifierDraft,
71+
ModelRevisionSpec,
7172
ModelRevisionSpecDraft,
7273
MountMetadata,
7374
ReplicaSpec,
@@ -167,6 +168,14 @@ def _serve_info_from_dto(dto: ServiceInfo, runtime_variant_name: RuntimeVariant)
167168
)
168169

169170

171+
def _resolve_target_revision_spec(info: DeploymentInfo) -> ModelRevisionSpec | None:
172+
"""Resolve the target revision spec by id (current first, then deploying)."""
173+
target_id = info.current_revision_id or info.deploying_revision_id
174+
if target_id is None:
175+
return None
176+
return next((r for r in info.model_revisions if r.revision_id == target_id), None)
177+
178+
170179
def _serve_info_from_deployment_info(
171180
deployment_info: DeploymentInfo,
172181
runtime_variant_name: RuntimeVariant,
@@ -177,7 +186,7 @@ def _serve_info_from_deployment_info(
177186
active revision's ``runtime_variant_id`` (internal data types are
178187
id-only; the legacy REST response still exposes the name string).
179188
"""
180-
model_revision = deployment_info.model_revisions[0] if deployment_info.model_revisions else None
189+
model_revision = _resolve_target_revision_spec(deployment_info)
181190

182191
return ServeInfoModel(
183192
endpoint_id=deployment_info.id,
@@ -378,9 +387,7 @@ async def create(self, body: BodyParam[NewServiceRequestModel], req: RequestCtx)
378387
await self._deployment.create_legacy_deployment.wait_for_complete(deployment_action)
379388
)
380389
deployment_info = deployment_result.data
381-
model_revision = (
382-
deployment_info.model_revisions[0] if deployment_info.model_revisions else None
383-
)
390+
model_revision = _resolve_target_revision_spec(deployment_info)
384391
if model_revision is None:
385392
raise RuntimeVariantNotFound()
386393
runtime_variant_name = await self._resolve_runtime_variant_name(

src/ai/backend/manager/data/deployment/types.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,7 @@ class ModelDeploymentData:
959959
metadata: ModelDeploymentMetadataInfo
960960
network_access: DeploymentNetworkSpec
961961
revision: ModelRevisionData | None
962+
current_revision_id: DeploymentRevisionID | None
962963
deploying_revision_id: DeploymentRevisionID | None
963964
revision_history_ids: list[DeploymentRevisionID]
964965
scaling_rule_ids: list[UUID]

src/ai/backend/manager/models/endpoint/row.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ class EndpointRow(Base): # type: ignore[misc]
291291
"DeploymentRevisionRow",
292292
back_populates="endpoint_row",
293293
primaryjoin=_get_endpoint_revisions_join_condition,
294+
order_by="DeploymentRevisionRow.revision_number.desc()",
294295
)
295296

296297
auto_scaling_policy: Mapped[DeploymentAutoScalingPolicyRow | None] = relationship(

src/ai/backend/manager/services/deployment/service.py

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,10 +233,23 @@ def _convert_deployment_info_to_data(info: DeploymentInfo) -> ModelDeploymentDat
233233
234234
Note: Some fields are set to defaults as DeploymentInfo doesn't have all the data.
235235
"""
236-
# Map revision if available
237236
revision: ModelRevisionData | None = None
238-
if info.model_revisions:
239-
rev = info.model_revisions[0]
237+
rev: ModelRevisionSpec | None = None
238+
if info.current_revision_id is not None:
239+
rev = next(
240+
(r for r in info.model_revisions if r.revision_id == info.current_revision_id),
241+
None,
242+
)
243+
if rev is None:
244+
log.error(
245+
"Deployment {} has current_revision_id {} but no matching "
246+
"ModelRevisionSpec was found in DeploymentInfo.model_revisions; "
247+
"current_revision will be reported as null. This usually means "
248+
"EndpointRow.revisions was not eagerly loaded by the caller.",
249+
info.id,
250+
info.current_revision_id,
251+
)
252+
if rev is not None:
240253
if rev.revision_id is None:
241254
raise ValueError(f"ModelRevisionSpec has no revision_id for deployment {info.id}")
242255
revision = ModelRevisionData(
@@ -283,6 +296,7 @@ def _convert_deployment_info_to_data(info: DeploymentInfo) -> ModelDeploymentDat
283296
network_access=info.network,
284297
revision_history_ids=[info.current_revision_id] if info.current_revision_id else [],
285298
revision=revision,
299+
current_revision_id=info.current_revision_id,
286300
deploying_revision_id=info.deploying_revision_id,
287301
scaling_rule_ids=[], # Not available in DeploymentInfo
288302
replica_state=ReplicaStateData(

tests/unit/manager/services/deployment/test_deployment_service.py

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from __future__ import annotations
88

99
import uuid
10+
from collections.abc import Callable
1011
from datetime import UTC, datetime
1112
from typing import cast
1213
from unittest.mock import AsyncMock, MagicMock
@@ -21,6 +22,7 @@
2122
)
2223
from ai.backend.common.dto.manager.v2.deployment.types import IntOrPercent
2324
from ai.backend.common.identifier.deployment import DeploymentID
25+
from ai.backend.common.identifier.deployment_revision import DeploymentRevisionID
2426
from ai.backend.common.identifier.image import ImageID
2527
from ai.backend.common.identifier.runtime_variant import RuntimeVariantID
2628
from ai.backend.common.identifier.vfolder import VFolderUUID
@@ -50,7 +52,9 @@
5052
ExecutionSpec,
5153
ModelMountConfigData,
5254
ModelRevisionData,
55+
ModelRevisionSpec,
5356
ModelRuntimeConfigData,
57+
MountMetadata,
5458
ReplicaSpec,
5559
ResourceConfigData,
5660
ResourceSpec,
@@ -76,7 +80,10 @@
7680
AddModelRevisionAction,
7781
)
7882
from ai.backend.manager.services.deployment.processors import DeploymentProcessors
79-
from ai.backend.manager.services.deployment.service import DeploymentService
83+
from ai.backend.manager.services.deployment.service import (
84+
DeploymentService,
85+
_convert_deployment_info_to_data,
86+
)
8087
from ai.backend.manager.sokovan.deployment import DeploymentController
8188

8289

@@ -634,3 +641,72 @@ async def test_persists_coordinator_jwt_instead_of_random(
634641
creator = cast(RBACEntityCreator[object], repo_call.args[0])
635642
spec = cast(EndpointTokenCreatorSpec, creator.spec)
636643
assert spec.token == sample_coordinator_jwt
644+
645+
646+
class TestConvertDeploymentInfoToData:
647+
"""Regression test for ``_convert_deployment_info_to_data`` (BA-5963)."""
648+
649+
@pytest.fixture
650+
def make_revision_spec(self) -> Callable[[], ModelRevisionSpec]:
651+
def make() -> ModelRevisionSpec:
652+
return ModelRevisionSpec(
653+
revision_id=DeploymentRevisionID(uuid.uuid4()),
654+
image_id=ImageID(uuid.uuid4()),
655+
resource_spec=ResourceSpec(
656+
cluster_mode=ClusterMode.SINGLE_NODE,
657+
cluster_size=1,
658+
resource_slots={"cpu": "1"},
659+
),
660+
mounts=MountMetadata(
661+
model_vfolder_id=VFolderUUID(uuid.uuid4()),
662+
model_definition_path="model-definition.yaml",
663+
model_mount_destination="/models",
664+
extra_mounts=[],
665+
),
666+
execution=ExecutionSpec(
667+
runtime_variant_id=RuntimeVariantID(uuid.uuid4()),
668+
),
669+
)
670+
671+
return make
672+
673+
def test_current_revision_resolved_by_id_match_not_list_order(
674+
self,
675+
make_revision_spec: Callable[[], ModelRevisionSpec],
676+
) -> None:
677+
"""Pin: revision lookup must use explicit ``current_revision_id``, not list[0]."""
678+
deploying_spec = make_revision_spec()
679+
current_spec = make_revision_spec()
680+
681+
deployment_info = DeploymentInfo(
682+
id=DeploymentID(uuid.uuid4()),
683+
metadata=DeploymentMetadata(
684+
name="ba5963-test",
685+
domain="default",
686+
project=uuid.uuid4(),
687+
resource_group="default",
688+
created_user=uuid.uuid4(),
689+
session_owner=uuid.uuid4(),
690+
created_at=datetime(2024, 1, 1, tzinfo=UTC),
691+
revision_history_limit=10,
692+
),
693+
state=DeploymentState(
694+
lifecycle=EndpointLifecycle.DEPLOYING,
695+
scaling_state=ScalingState.STABLE,
696+
retry_count=0,
697+
),
698+
replica_spec=ReplicaSpec(replica_count=1),
699+
network=DeploymentNetworkSpec(open_to_public=False),
700+
model_revisions=[deploying_spec, current_spec],
701+
options=DeploymentOptions(),
702+
current_revision_id=current_spec.revision_id,
703+
deploying_revision_id=deploying_spec.revision_id,
704+
)
705+
706+
deployment_data = _convert_deployment_info_to_data(deployment_info)
707+
708+
assert deployment_data.current_revision_id == current_spec.revision_id
709+
assert deployment_data.deploying_revision_id == deploying_spec.revision_id
710+
assert deployment_data.current_revision_id != deployment_data.deploying_revision_id
711+
assert deployment_data.revision is not None
712+
assert deployment_data.revision.id == current_spec.revision_id

0 commit comments

Comments
 (0)