Skip to content

Commit e0761ff

Browse files
authored
Checks to use the correct ami when on cu130 to avoid crashes at launch (#6281)
* add: checks to use the correct ami when on cu130 to avoid crashes at launch add: tests Signed-off-by: DWarez <dario.salvati@huggingface.co> * fix: reverting timeout in kwargs change Signed-off-by: DWarez <dario.salvati@huggingface.co> * fix: removing the check on vllm server in order to apply the fix for every dlc Signed-off-by: DWarez <dario.salvati@huggingface.co> * fix: addressed code bot comment Signed-off-by: DWarez <dario.salvati@huggingface.co> --------- Signed-off-by: DWarez <dario.salvati@huggingface.co>
1 parent 38d4ef9 commit e0761ff

4 files changed

Lines changed: 156 additions & 18 deletions

File tree

‎sagemaker-serve/src/sagemaker/serve/model_builder.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3988,6 +3988,12 @@ def _deploy_core_endpoint(self, **kwargs):
39883988
"Fast Model Loading. Configure by setting `num_cpus` to 0 in `resources`."
39893989
)
39903990

3991+
if serverless_inference_config is None:
3992+
inference_ami_version = self._resolve_inference_ami_version(
3993+
instance_type=instance_type,
3994+
inference_ami_version=inference_ami_version,
3995+
)
3996+
39913997
if endpoint_type == EndpointType.INFERENCE_COMPONENT_BASED:
39923998
if update_endpoint:
39933999
raise ValueError(
@@ -6053,6 +6059,9 @@ def deploy(
60536059
if not hasattr(self, "built_model") and not hasattr(self, "_deployables"):
60546060
raise ValueError("Model needs to be built before deploying")
60556061

6062+
if instance_type:
6063+
self.instance_type = instance_type
6064+
60566065
# Inference component deployments manage their own reuse by IC name
60576066
# (create vs. in-place update in _deploy_for_ic). The endpoint-return
60586067
# reuse gate must not intercept them, or an intended IC create/update
@@ -6113,11 +6122,6 @@ def deploy(
61136122
if not self.instance_type and not instance_type:
61146123
self.instance_type = self._fetch_default_instance_type_for_custom_model()
61156124

6116-
# Ensure self.instance_type reflects the caller's intent so the
6117-
# endpoint config creation in _deploy_model_customization picks it up.
6118-
if instance_type:
6119-
self.instance_type = instance_type
6120-
61216125
# Pass inference_config if it's ResourceRequirements
61226126
inference_config_param = None
61236127
if isinstance(inference_config, ResourceRequirements):

‎sagemaker-serve/src/sagemaker/serve/model_builder_servers.py‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121

2222
import os
2323
from pathlib import Path
24-
from typing import Union
24+
from typing import Optional, Union
2525

2626
# SageMaker core imports
2727
from sagemaker.core.resources import Model, Endpoint
@@ -76,6 +76,22 @@
7676
MODEL_SERVER_WORKERS_PARAM_NAME = "sagemaker_model_server_workers"
7777
SAGEMAKER_REGION_PARAM_NAME = "sagemaker_region"
7878
SAGEMAKER_OUTPUT_LOCATION = "sagemaker_s3_output"
79+
# Intentionally allowlist only instance families documented for the GPU 4.1 AMI.
80+
# Add new families only after SageMaker documents support for them.
81+
_CUDA_13_INFERENCE_AMI = "al2023-ami-sagemaker-inference-gpu-4-1"
82+
_CUDA_13_INFERENCE_AMI_COMPATIBLE_FAMILIES = frozenset(
83+
{
84+
"ml.g4dn",
85+
"ml.g5",
86+
"ml.g6",
87+
"ml.g6e",
88+
"ml.p4d",
89+
"ml.p4de",
90+
"ml.p5",
91+
"ml.p5e",
92+
"ml.p5en",
93+
}
94+
)
7995

8096

8197
class _ModelBuilderServers(object):
@@ -327,15 +343,37 @@ def _build_for_tgi(self) -> Model:
327343

328344
return model
329345

346+
def _resolve_inference_ami_version(
347+
self,
348+
instance_type: Optional[str],
349+
inference_ami_version: Optional[str],
350+
) -> Optional[str]:
351+
"""Resolve a CUDA-compatible inference AMI without overriding the caller."""
352+
if inference_ami_version is not None:
353+
return inference_ami_version
354+
if not isinstance(self.image_uri, str) or not isinstance(instance_type, str):
355+
return inference_ami_version
356+
357+
# DLC CUDA versions are hyphen-delimited tag tokens. Splitting also
358+
# recognizes a tag ending in ``-cu130`` without requiring a suffix.
359+
image_tag = self.image_uri.rpartition(":")[2]
360+
if "cu130" not in image_tag.split("-"):
361+
return inference_ami_version
362+
363+
instance_family = instance_type.rsplit(".", 1)[0]
364+
if instance_family not in _CUDA_13_INFERENCE_AMI_COMPATIBLE_FAMILIES:
365+
return inference_ami_version
366+
367+
return _CUDA_13_INFERENCE_AMI
368+
330369
def _build_for_hf_server(self, model_server: ModelServer) -> Model:
331370
"""Build a HuggingFace model for a hub-download serving container.
332371
333372
Generic build path shared by the vLLM, SGLang, and vLLM-omni servers. It
334-
configures the container to pull the model directly from the HuggingFace Hub
335-
(HF_MODEL_ID), resolves the appropriate DLC image via _auto_detect_image_uri,
336-
and prepares the model for the selected mode. Server-specific tuning (sharding,
337-
tensor parallelism, MAX_* limits, etc.) is intentionally left to the container
338-
defaults and will be added in a follow-up.
373+
configures the container to pull the model directly from the HuggingFace Hub,
374+
resolves the appropriate DLC image via _auto_detect_image_uri, and prepares the
375+
model for the selected mode. Server-specific tuning (sharding, tensor parallelism,
376+
MAX_* limits, etc.) remains configurable through environment variables.
339377
340378
Args:
341379
model_server: The HuggingFace serving model server to build for
@@ -358,8 +396,7 @@ def _build_for_hf_server(self, model_server: ModelServer) -> Model:
358396
_create_dir_structure(self.model_path)
359397

360398
if isinstance(self.model, str) and not self._is_jumpstart_model_id():
361-
# These containers download the model directly from the HuggingFace Hub
362-
# Todo: missing something?
399+
# These containers download the model directly from the HuggingFace Hub.
363400
self.env_vars.setdefault("HF_MODEL_ID", self.model)
364401

365402
if self.env_vars.get("HUGGING_FACE_HUB_TOKEN"):

‎sagemaker-serve/tests/unit/test_model_builder_servers_hf_model_id.py‎

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
"""Unit tests: HF_MODEL_ID is not overwritten when user provides it."""
1+
"""Unit tests for Hugging Face ModelBuilder server configuration."""
22
from __future__ import annotations
33

44
from typing import Dict, List, Optional
@@ -292,6 +292,102 @@ def test_sets_default_hf_model_id_when_not_provided(
292292
)
293293

294294

295+
class TestCuda13AmiResolution:
296+
"""CUDA 13 serving images select compatible host drivers at deployment."""
297+
298+
def test_selects_cuda_13_ami_for_vllm_omni(
299+
self,
300+
mock_builder: MagicMock,
301+
) -> None:
302+
"""The deploy-time G5 instance overrides the M5 instance used at build time."""
303+
builder = mock_builder
304+
builder.instance_type = "ml.m5.large"
305+
builder.image_uri = (
306+
"123456789012.dkr.ecr.us-west-2.amazonaws.com/"
307+
"huggingface-vllm-omni:0.20.0-gpu-py312-cu130-amzn2023"
308+
)
309+
310+
result = _ModelBuilderServers._resolve_inference_ami_version(
311+
builder,
312+
instance_type="ml.g5.2xlarge",
313+
inference_ami_version=None,
314+
)
315+
316+
assert result == "al2023-ami-sagemaker-inference-gpu-4-1"
317+
318+
def test_recognizes_image_tag_ending_in_cu130(
319+
self,
320+
mock_builder: MagicMock,
321+
) -> None:
322+
"""A cu130 token remains detectable without a trailing tag component."""
323+
builder = mock_builder
324+
builder.image_uri = (
325+
"123456789012.dkr.ecr.us-west-2.amazonaws.com/"
326+
"huggingface-vllm:0.21.0-gpu-py312-cu130"
327+
)
328+
329+
result = _ModelBuilderServers._resolve_inference_ami_version(
330+
builder,
331+
instance_type="ml.g5.2xlarge",
332+
inference_ami_version=None,
333+
)
334+
335+
assert result == "al2023-ami-sagemaker-inference-gpu-4-1"
336+
337+
@pytest.mark.parametrize(
338+
"image_uri, instance_type",
339+
[
340+
(
341+
"123456789012.dkr.ecr.us-west-2.amazonaws.com/"
342+
"huggingface-vllm:0.17.0-gpu-py312-cu129-ubuntu22.04",
343+
"ml.g5.2xlarge",
344+
),
345+
(
346+
"123456789012.dkr.ecr.us-west-2.amazonaws.com/"
347+
"huggingface-vllm:0.21.0-gpu-py312-cu130-ubuntu22.04",
348+
"ml.m5.large",
349+
),
350+
],
351+
ids=["non-cu130-image", "unsupported-instance-family"],
352+
)
353+
def test_leaves_ami_unset_outside_compatibility_gate(
354+
self,
355+
mock_builder: MagicMock,
356+
image_uri: str,
357+
instance_type: str,
358+
) -> None:
359+
"""Images and instances outside the compatibility gate retain the default."""
360+
builder = mock_builder
361+
builder.image_uri = image_uri
362+
363+
result = _ModelBuilderServers._resolve_inference_ami_version(
364+
builder,
365+
instance_type=instance_type,
366+
inference_ami_version=None,
367+
)
368+
369+
assert result is None
370+
371+
def test_preserves_explicit_inference_ami(
372+
self,
373+
mock_builder: MagicMock,
374+
) -> None:
375+
"""An explicit deployment AMI takes precedence over automatic selection."""
376+
builder = mock_builder
377+
builder.image_uri = (
378+
"123456789012.dkr.ecr.us-west-2.amazonaws.com/"
379+
"huggingface-vllm:0.21.0-gpu-py312-cu130-ubuntu22.04"
380+
)
381+
382+
result = _ModelBuilderServers._resolve_inference_ami_version(
383+
builder,
384+
instance_type="ml.g5.2xlarge",
385+
inference_ami_version="custom-inference-ami",
386+
)
387+
388+
assert result == "custom-inference-ami"
389+
390+
295391
# -----------------------------------------------------------
296392
# Transformers (MMS) — needs extra patches
297393
# -----------------------------------------------------------

‎sagemaker-serve/tests/unit/test_model_builder_workflows.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,13 @@ def test_deploy_generates_unique_endpoint_name(self, mock_deploy):
198198
)
199199
builder.built_model = Mock(spec=Model)
200200

201-
result = builder.deploy(instance_type="ml.m5.large")
202-
201+
result = builder.deploy(instance_type="ml.g5.xlarge")
202+
203203
self.assertIsNotNone(result)
204-
# Verify endpoint name was generated (contains uuid)
204+
# Verify endpoint name generation and the deploy-time instance override.
205205
self.assertIn("endpoint-", builder.endpoint_name)
206-
mock_deploy.assert_called_once()
206+
self.assertEqual(builder.instance_type, "ml.g5.xlarge")
207+
self.assertEqual(mock_deploy.call_args.kwargs["instance_type"], "ml.g5.xlarge")
207208

208209
@patch('sagemaker.serve.model_builder.ModelBuilder._deploy')
209210
def test_deploy_with_serverless_config(self, mock_deploy):

0 commit comments

Comments
 (0)