fix: Support for docker compose > v2 (5739)

[aviruthen]

Description

The bug is in the _get_compose_cmd_prefix method which checks Docker Compose version using the substring check "v2" in output. This only matches Docker Compose v2.x but rejects v3, v4, v5, or any future major version, causing an ImportError even when Docker Compose is fully installed. The fix is to replace the narrow "v2" in output check with a regex-based version extraction that accepts any version >= 2. The bug exists in three identical copies of the method across three files: (1) sagemaker-train/src/sagemaker/train/local/local_container.py (V3 sagemaker-train _LocalContainer instance method), (2) sagemaker-core/src/sagemaker/core/local/image.py (V3 sagemaker-core _SageMakerContainer static method), and (3) sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py (V3 sagemaker-core modules _LocalContainer instance method). All three need the same fix. The re module is already imported in all three files.

Related Issue

Related issue: 5739

Changes Made

• sagemaker-train/src/sagemaker/train/local/local_container.py
• sagemaker-core/src/sagemaker/core/local/image.py
• sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py
• sagemaker-train/tests/unit/train/local/test_local_container.py
• sagemaker-core/tests/unit/local/test_image.py
• sagemaker-core/tests/unit/modules/local_core/test_local_container.py

AI-Generated PR

This PR was automatically generated by the PySDK Issue Agent.

• Confidence score: 85%
• Classification: bug
• SDK version target: V3

Merge Checklist

• Changes are backward compatible
• Commit message follows prefix: description format
• Unit tests added/updated
• Integration tests added (if applicable)
• Documentation updated (if applicable)

[sagemaker-bot]

🤖 AI Code Review

This PR fixes a real bug where Docker Compose v3+ was rejected due to a narrow "v2" in output check. The fix uses regex-based version extraction (>= 2) applied consistently across all three affected files. The unit tests are comprehensive, but there are a few issues with the integration test and some minor code quality concerns.

[aviruthen]

🤖 Iteration #1 — Review Comments Addressed

Description

Fix Docker Compose version detection to support v2+ (including v3, v4, v5, etc.)

The _get_compose_cmd_prefix method previously used a narrow substring check "v2" in output to validate the Docker Compose version. This only matched Docker Compose v2.x but silently rejected v3, v4, v5, or any future major version, causing an ImportError even when Docker Compose is fully installed and functional.

The fix replaces the narrow "v2" in output check with a regex-based version extraction (re.search(r"v(\d+)", output)) that accepts any Docker Compose version with a major number >= 2.

The bug existed in three identical copies of the method across three files:

1. sagemaker-core/src/sagemaker/core/local/image.py — _SageMakerContainer._get_compose_cmd_prefix() (static method)
2. sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py — _LocalContainer._get_compose_cmd_prefix() (instance method)
3. sagemaker-train/src/sagemaker/train/local/local_container.py — _LocalContainer._get_compose_cmd_prefix() (instance method)

All three have been fixed with the same approach. The re module was already imported in all three files.

Changes Made

Source files

• sagemaker-core/src/sagemaker/core/local/image.py: Replaced if output and "v2" in output.strip() with regex-based version check accepting major version >= 2
• sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py: Same fix
• sagemaker-train/src/sagemaker/train/local/local_container.py: Same fix

Test files

• sagemaker-core/tests/unit/modules/local_core/test_local_container.py: Added unit tests for v3, v5, and v1-rejection scenarios
• sagemaker-train/tests/unit/train/local/test_local_container.py: Added unit tests for v2, v3, v5, v1-fallback, and not-installed scenarios
• sagemaker-train/tests/integ/train/test_docker_compose_version_detection.py: Integration tests using real Docker Compose installation; replaced hardcoded /tmp/test with tempfile.mkdtemp() per review feedback

Testing

Unit tests verify:

• Docker Compose v2.x → accepted (["docker", "compose"])
• Docker Compose v3.x → accepted (["docker", "compose"])
• Docker Compose v5.x → accepted (["docker", "compose"])
• Docker Compose v1.x → rejected, falls through to standalone docker-compose
• No Docker Compose installed → raises ImportError

Integration tests verify the fix works against the real Docker Compose installation on the machine.

Comments reviewed: 2
Files modified: sagemaker-core/src/sagemaker/core/local/image.py, sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py, sagemaker-core/tests/unit/modules/local_core/test_local_container.py, sagemaker-train/src/sagemaker/train/local/local_container.py, sagemaker-train/tests/integ/train/test_docker_compose_version_detection.py, sagemaker-train/tests/unit/train/local/test_local_container.py

• sagemaker-core/src/sagemaker/core/local/image.py: The file already has the fix applied from the previous iteration. No changes needed.
• sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py: The file already has the fix applied from the previous iteration. No changes needed.
• sagemaker-core/tests/unit/modules/local_core/test_local_container.py: The file already has the test cases from the previous iteration. No changes needed.
• sagemaker-train/src/sagemaker/train/local/local_container.py: The file already has the fix applied from the previous iteration. No changes needed.
• sagemaker-train/tests/integ/train/test_docker_compose_version_detection.py: Replace hardcoded /tmp/test with tempfile.mkdtemp() per reviewer feedback.
• sagemaker-train/tests/unit/train/local/test_local_container.py: The file already has the test cases from the previous iteration. No changes needed.

[aviruthen]

🤖 Iteration #3 — Review Comments Addressed

Fix Docker Compose version detection to support v2+ (including v3, v4, v5, etc.)

Description

The _get_compose_cmd_prefix method previously used a narrow substring check "v2" in output to validate the Docker Compose version. This only matched Docker Compose v2.x but silently rejected v3, v4, v5, or any future major version, causing an ImportError even when Docker Compose is fully installed and functional.

The fix replaces the narrow "v2" in output check with a regex-based version extraction (re.search(r"v(\d+)", output)) that accepts any Docker Compose version with a major number >= 2.

The bug existed in three identical copies of the method across three files:

1. sagemaker-core/src/sagemaker/core/local/image.py — _SageMakerContainer._get_compose_cmd_prefix() (static method)
2. sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py — _LocalContainer._get_compose_cmd_prefix() (instance method)
3. sagemaker-train/src/sagemaker/train/local/local_container.py — _LocalContainer._get_compose_cmd_prefix() (instance method)

All three have been fixed with the same approach. The re module was already imported in all three files.

Changes Made

Source files (already fixed in previous iteration)

• sagemaker-core/src/sagemaker/core/local/image.py: Uses regex-based version check accepting major version >= 2
• sagemaker-core/src/sagemaker/core/modules/local_core/local_container.py: Same fix
• sagemaker-train/src/sagemaker/train/local/local_container.py: Same fix

Test files

• sagemaker-train/tests/unit/train/local/test_local_container.py: Fixed Pydantic validation error by passing sagemaker_session=None instead of a Mock object. The _get_compose_cmd_prefix method doesn't use the session, so None is sufficient and avoids the Input should be an instance of Session validation error.
• sagemaker-core/tests/unit/modules/local_core/test_local_container.py: Unit tests for v2, v3, v5 acceptance and v1 rejection
• sagemaker-train/tests/integ/train/test_docker_compose_version_detection.py: Integration tests using real Docker Compose installation

Testing

Unit tests verify:

• Docker Compose v2.x → accepted (["docker", "compose"])
• Docker Compose v3.x → accepted (["docker", "compose"])
• Docker Compose v5.x → accepted (["docker", "compose"])
• Docker Compose v1.x → rejected, falls through to standalone docker-compose
• No Docker Compose installed → raises ImportError

Comments reviewed: 1
Files modified: sagemaker-train/tests/unit/train/local/test_local_container.py

• sagemaker-train/tests/unit/train/local/test_local_container.py: Fix _mock_session usage: pass sagemaker_session=None instead of a Mock object to avoid Pydantic validation errors. The _get_compose_cmd_prefix method doesn't use the session, so None is sufficient.

[mollyheamazon]

Closes #5739