Skip to content

Commit 30ee2f9

Browse files
seedspiritclaude
andcommitted
fix(BA-6015): carry mount metadata in RevisionDraft merge chain
Mount identity (model vfolder, definition path, destination, extra mounts, subpath) now travels inside RevisionDraft and participates in the merge chain instead of being threaded as a sidecar argument through add_revision and the draft readers. The vfolder scan records the matched candidate path back into the merged draft, so the resolved model-definition path is persisted on the revision instead of the request's possibly-empty value. ModelRevisionCreator.to_draft is renamed to to_draft_with_extra_mount and now derives the rest of the mount metadata itself, taking only the permission-resolved extra mounts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 761cc8f commit 30ee2f9

9 files changed

Lines changed: 205 additions & 77 deletions

File tree

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from ai.backend.common.identifier.deployment_preset import DeploymentPresetID
99
from ai.backend.common.identifier.image import ImageID
1010
from ai.backend.common.identifier.vfolder import VFolderUUID
11+
from ai.backend.common.types import MountInfoEntry
1112
from ai.backend.manager.data.deployment.types import (
1213
DeploymentMetadata,
1314
DeploymentNetworkSpec,
@@ -17,6 +18,7 @@
1718
ModelRevisionSpec,
1819
ModelRevisionSpecDraft,
1920
MountInfo,
21+
MountMetadata,
2022
ReplicaSpec,
2123
ResourceSpec,
2224
RevisionDraft,
@@ -65,7 +67,7 @@ class ModelRevisionCreator:
6567
revision_preset_id: DeploymentPresetID | None = None
6668
preset_values: list[PresetValueData] = field(default_factory=list)
6769

68-
def to_draft(self) -> RevisionDraft:
70+
def to_draft_with_extra_mount(self, extra_mounts: list[MountInfoEntry]) -> RevisionDraft:
6971
"""Project this v2 creator onto a ``RevisionDraft`` layer.
7072
7173
``image_id`` is already resolved upstream. Optional ``resource_spec`` /
@@ -77,6 +79,13 @@ def to_draft(self) -> RevisionDraft:
7779
ex = self.execution
7880
return RevisionDraft(
7981
image_id=self.image_id,
82+
mounts=MountMetadata(
83+
model_vfolder_id=self.mounts.model_vfolder_id,
84+
model_definition_path=self.mounts.model_definition_path,
85+
model_mount_destination=self.mounts.model_mount_destination,
86+
extra_mounts=extra_mounts,
87+
vfolder_subpath=self.mounts.vfolder_subpath,
88+
),
8089
resource_slots=rs.resource_slots if rs is not None else None,
8190
resource_opts=rs.resource_opts if rs is not None else None,
8291
cluster_mode=rs.cluster_mode if rs is not None else None,

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

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,17 @@ class DeploymentConfig(BackendAISchema):
6666
environ: dict[str, str] | None = None
6767

6868

69+
@dataclass(frozen=True)
70+
class FetchedModelDefinition:
71+
"""Model definition draft with the vfolder path it was read from.
72+
73+
``path`` is the matched candidate path inside the model vfolder.
74+
"""
75+
76+
path: str
77+
model_definition: ModelDefinitionDraft
78+
79+
6980
class RouteStatus(enum.Enum):
7081
"""Lifecycle status of a route (independent of health)."""
7182

@@ -511,12 +522,11 @@ def to_draft(self, image_id: ImageID | None) -> RevisionDraft:
511522
"""Project this legacy spec draft onto a ``RevisionDraft`` layer.
512523
513524
``image_id`` is resolved upstream from the spec's ``image_identifier``
514-
(canonical + architecture) via the repository; mount-identifying
515-
fields live on ``MountMetadata`` and are passed alongside into
516-
``add_revision`` rather than through the draft chain.
525+
(canonical + architecture) via the repository.
517526
"""
518527
return RevisionDraft(
519528
image_id=image_id,
529+
mounts=self.mounts,
520530
resource_slots=self.resource_spec.resource_slots,
521531
resource_opts=self.resource_spec.resource_opts,
522532
cluster_mode=self.resource_spec.cluster_mode,
@@ -548,6 +558,8 @@ class RevisionDraft:
548558
# (see ``deployment_revisions.image`` SET NULL FK) — downstream
549559
# resolvers must treat the latter as non-deployable.
550560
image_id: ImageID | None = None
561+
# Mount
562+
mounts: MountMetadata | None = None
551563
# Resource
552564
resource_slots: Mapping[str, Any] | None = None
553565
resource_opts: Mapping[str, Any] | None = None
@@ -570,6 +582,7 @@ def merge(self, other: RevisionDraft) -> RevisionDraft:
570582
"""Return a new draft with ``other`` layered on top of ``self``."""
571583
return RevisionDraft(
572584
image_id=other.image_id if other.image_id is not None else self.image_id,
585+
mounts=_merge_mounts(self.mounts, other.mounts),
573586
resource_slots=_merge_mappings(self.resource_slots, other.resource_slots),
574587
resource_opts=_merge_mappings(self.resource_opts, other.resource_opts),
575588
cluster_mode=other.cluster_mode
@@ -600,7 +613,7 @@ def merge(self, other: RevisionDraft) -> RevisionDraft:
600613
else (list(self.preset_values) if self.preset_values is not None else None),
601614
)
602615

603-
def to_model_revision_spec(self, mounts: MountMetadata) -> ModelRevisionSpec:
616+
def to_model_revision_spec(self) -> ModelRevisionSpec:
604617
"""Project the merged draft into a final ``ModelRevisionSpec``.
605618
606619
Validates that the merge chain produced an ``image_id`` and a
@@ -611,6 +624,8 @@ def to_model_revision_spec(self, mounts: MountMetadata) -> ModelRevisionSpec:
611624
"""
612625
if self.image_id is None:
613626
raise InvalidAPIParameters("image_id is required to build a revision")
627+
if self.mounts is None:
628+
raise InvalidAPIParameters("mounts are required to build a revision")
614629
if self.runtime_variant_id is None:
615630
raise InvalidAPIParameters("runtime_variant_id is required to build a revision")
616631
return ModelRevisionSpec(
@@ -621,7 +636,7 @@ def to_model_revision_spec(self, mounts: MountMetadata) -> ModelRevisionSpec:
621636
resource_slots=self.resource_slots or {},
622637
resource_opts=self.resource_opts,
623638
),
624-
mounts=mounts,
639+
mounts=self.mounts,
625640
execution=ExecutionSpec(
626641
startup_command=self.startup_command,
627642
bootstrap_script=self.bootstrap_script,
@@ -636,6 +651,33 @@ def to_model_revision_spec(self, mounts: MountMetadata) -> ModelRevisionSpec:
636651
)
637652

638653

654+
def _merge_mounts(
655+
lower: MountMetadata | None,
656+
upper: MountMetadata | None,
657+
) -> MountMetadata | None:
658+
if upper is None:
659+
return lower
660+
if lower is None:
661+
return MountMetadata(
662+
model_vfolder_id=upper.model_vfolder_id,
663+
model_definition_path=upper.model_definition_path,
664+
model_mount_destination=upper.model_mount_destination,
665+
extra_mounts=list(upper.extra_mounts),
666+
vfolder_subpath=upper.vfolder_subpath,
667+
)
668+
return MountMetadata(
669+
model_vfolder_id=upper.model_vfolder_id,
670+
model_definition_path=upper.model_definition_path
671+
if upper.model_definition_path is not None
672+
else lower.model_definition_path,
673+
model_mount_destination=upper.model_mount_destination,
674+
extra_mounts=list(upper.extra_mounts),
675+
vfolder_subpath=upper.vfolder_subpath
676+
if upper.vfolder_subpath is not None
677+
else lower.vfolder_subpath,
678+
)
679+
680+
639681
def _merge_mappings(
640682
lower: Mapping[str, Any] | None,
641683
upper: Mapping[str, Any] | None,
@@ -993,6 +1035,17 @@ def to_draft(self) -> RevisionDraft:
9931035
)
9941036
return RevisionDraft(
9951037
image_id=self.image_id,
1038+
mounts=(
1039+
MountMetadata(
1040+
model_vfolder_id=self.model_mount_config.vfolder_id,
1041+
model_definition_path=self.model_mount_config.definition_path or None,
1042+
model_mount_destination=self.model_mount_config.mount_destination or "/models",
1043+
extra_mounts=list(self.model_mount_config.extra_mounts),
1044+
vfolder_subpath=self.model_mount_config.subpath,
1045+
)
1046+
if self.model_mount_config.vfolder_id is not None
1047+
else None
1048+
),
9961049
resource_slots=resource_slots,
9971050
resource_opts=resource_opts,
9981051
cluster_mode=self.cluster_config.mode,

src/ai/backend/manager/repositories/deployment/repository.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from ai.backend.common.clients.valkey_client.valkey_live.client import ValkeyLiveClient
1515
from ai.backend.common.clients.valkey_client.valkey_schedule.client import ValkeyScheduleClient
1616
from ai.backend.common.clients.valkey_client.valkey_stat.client import ValkeyStatClient
17-
from ai.backend.common.config import ModelDefinitionDraft, ModelHealthCheck
17+
from ai.backend.common.config import ModelHealthCheck
1818
from ai.backend.common.data.endpoint.types import EndpointLifecycle
1919
from ai.backend.common.exception import BackendAIError, InvalidAPIParameters
2020
from ai.backend.common.identifier.deployment import DeploymentID
@@ -64,6 +64,7 @@
6464
DeploymentRevisionReadBundle,
6565
DeploymentSummarySearchResult,
6666
DeploymentWithHistory,
67+
FetchedModelDefinition,
6768
LegacyRevisionCreateReadBundle,
6869
ModelDeploymentAccessTokenData,
6970
ModelDeploymentAutoScalingRuleData,
@@ -485,12 +486,13 @@ async def fetch_model_definition(
485486
self,
486487
vfolder_id: VFolderUUID,
487488
model_definition_path: str | None,
488-
) -> ModelDefinitionDraft | None:
489+
) -> FetchedModelDefinition | None:
489490
"""Fetch and validate the model-definition file from the model vfolder.
490491
491-
Returns a ``ModelDefinitionDraft`` because the file is user-authored
492+
Returns a ``FetchedModelDefinition`` because the file is user-authored
492493
and may omit optional fields; strict-field validation is deferred to
493-
the persistence boundary where the merged result is resolved.
494+
the persistence boundary where the merged result is resolved. The
495+
result also carries the exact candidate path that matched.
494496
Returns ``None`` when no candidate file exists.
495497
"""
496498
vfolder_location = await self._db_source.get_vfolder_by_id(vfolder_id)

src/ai/backend/manager/repositories/deployment/storage_source/storage_source.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Storage source implementation for deployment repository."""
22

33
from collections.abc import Mapping
4+
from dataclasses import dataclass
45
from typing import Any, override
56

67
import tomli
@@ -9,10 +10,17 @@
910
from ai.backend.common.config import ModelDefinitionDraft
1011
from ai.backend.common.exception import BackendAIError, InvalidAPIParameters
1112
from ai.backend.common.types import BackendAISchema, SchemaValidationFailureInfo, VFolderID
13+
from ai.backend.manager.data.deployment.types import FetchedModelDefinition
1214
from ai.backend.manager.data.vfolder.types import VFolderLocation
1315
from ai.backend.manager.models.storage import StorageSessionManager
1416

1517

18+
@dataclass(frozen=True)
19+
class FetchedConfigFile:
20+
filename: str
21+
payload: Mapping[str, Any]
22+
23+
1624
class DeploymentConfigInput(BackendAISchema):
1725
"""Validated ``deployment-config.yaml`` / ``service-definition.toml`` payload.
1826
@@ -60,13 +68,13 @@ async def fetch_deployment_config(
6068
raw = await self._fetch_config_file_in_candidates(vfolder_location, candidates)
6169
if raw is None:
6270
return None
63-
return DeploymentConfigInput.model_validate(dict(raw))
71+
return DeploymentConfigInput.model_validate(dict(raw.payload))
6472

6573
async def fetch_model_definition(
6674
self,
6775
vfolder_location: VFolderLocation,
6876
candidates: list[str],
69-
) -> ModelDefinitionDraft | None:
77+
) -> FetchedModelDefinition | None:
7078
"""Fetch the first existing model-definition file among ``candidates``.
7179
7280
Uses the draft type because user-authored files may supply only a
@@ -77,13 +85,16 @@ async def fetch_model_definition(
7785
raw = await self._fetch_config_file_in_candidates(vfolder_location, candidates)
7886
if raw is None:
7987
return None
80-
return ModelDefinitionDraft.model_validate(dict(raw))
88+
return FetchedModelDefinition(
89+
path=raw.filename,
90+
model_definition=ModelDefinitionDraft.model_validate(dict(raw.payload)),
91+
)
8192

8293
async def _fetch_config_file_in_candidates(
8394
self,
8495
vfolder_location: VFolderLocation,
8596
candidates: list[str],
86-
) -> Mapping[str, Any] | None:
97+
) -> FetchedConfigFile | None:
8798
"""Return the first parsed ``Mapping`` among the candidates.
8899
89100
Candidates are tried in priority order (new name first, legacy
@@ -122,5 +133,5 @@ async def _fetch_config_file_in_candidates(
122133
raise InvalidAPIParameters(
123134
f"Invalid config file '{filename}': top-level value must be a mapping."
124135
)
125-
return loaded
136+
return FetchedConfigFile(filename=filename, payload=loaded)
126137
return None

src/ai/backend/manager/services/model_serving/services/model_serving.py

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -829,38 +829,30 @@ async def modify_endpoint(self, action: ModifyEndpointAction) -> ModifyEndpointA
829829
if spec.has_revision_changes():
830830
latest_rev = await self._deployment_repository.get_latest_revision(action.deployment_id)
831831
latest_draft = latest_rev.to_draft()
832+
if latest_draft.mounts is None:
833+
raise InvalidAPIParameters("model vfolder id is missing on the latest revision")
832834
override_draft = await self._build_revision_overrides_from_spec(spec)
835+
definition_path_override = spec.model_definition_path.optional_value()
836+
draft_with_definition_path_override = RevisionDraft(
837+
mounts=MountMetadata(
838+
model_vfolder_id=latest_draft.mounts.model_vfolder_id,
839+
model_definition_path=definition_path_override,
840+
model_mount_destination=latest_draft.mounts.model_mount_destination,
841+
extra_mounts=list(latest_draft.mounts.extra_mounts),
842+
vfolder_subpath=latest_draft.mounts.vfolder_subpath,
843+
)
844+
)
845+
override_draft = override_draft.merge(draft_with_definition_path_override)
833846
# Legacy ``ModifyEndpoint`` semantics preserve untouched fields by
834847
# layering overrides on top of the existing revision. The
835848
# controller's ``add_revision`` no longer accepts a caller-provided
836849
# ``base`` layer, so we pre-merge here and pass the result as the
837850
# request-level override draft.
838851
overrides = latest_draft.merge(override_draft)
839852

840-
vfolder_id = latest_rev.model_mount_config.vfolder_id
841-
if vfolder_id is None:
842-
raise InvalidAPIParameters("model vfolder id is missing on the latest revision")
843-
definition_path_override = spec.model_definition_path.optional_value()
844-
mounts = MountMetadata(
845-
model_vfolder_id=vfolder_id,
846-
model_definition_path=(
847-
definition_path_override
848-
if definition_path_override is not None
849-
else latest_rev.model_mount_config.definition_path
850-
),
851-
model_mount_destination=latest_rev.model_mount_config.mount_destination
852-
or "/models",
853-
# Carry over the latest revision's extra mounts so that an
854-
# otherwise unrelated modify (e.g. replica-count) does not
855-
# strip them; ``mount_perm`` survives because the
856-
# revision-side projection uses the same ``MountInfoEntry``
857-
# shape.
858-
extra_mounts=list(latest_rev.model_mount_config.extra_mounts),
859-
)
860853
revision = await self._deployment_controller.add_revision(
861854
endpoint_id=action.deployment_id,
862855
overrides=overrides,
863-
mounts=mounts,
864856
)
865857
await self._deployment_controller.activate_revision(action.deployment_id, revision.id)
866858
elif spec.replica_count_modified():

0 commit comments

Comments
 (0)