Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Docker Compose V2 compatibility for local mode #3659

Closed
dbpprt opened this issue Feb 15, 2023 · 9 comments · Fixed by #3855 or #4111
Closed

Docker Compose V2 compatibility for local mode #3659

dbpprt opened this issue Feb 15, 2023 · 9 comments · Fixed by #3855 or #4111

Comments

@dbpprt
Copy link

dbpprt commented Feb 15, 2023

Describe the feature you'd like
Currently local mode relies on docker-compose (v1), v2 is mostly compatible with the commandline and makes installing more straight forward.

How would this feature be used? Please describe.
Allow local mode with Docker Compose V2

Describe alternatives you've considered
Install V1 which will be EoL in June 2023.

@dancsalo
Copy link

dancsalo commented Mar 1, 2023

+1

@pdifranc
Copy link

+1

@ozancaglayan
Copy link

Hello,

Nowadays all installations of docker are defaulting to compose v2. The fact that this package does not support this and installs the old docker-compose from pip is very confusing. In short, if your system is happily using docker compose v2 already, there is no way to use local mode with sagemaker on that machine.

@ozancaglayan
Copy link

This is more or less the change required in backward-compatible way. I tested it and it works. It does not work without setting the count/driver by the way.

diff --git a/src/sagemaker/local/image.py b/src/sagemaker/local/image.py
index adba12d3..1d9a07ce 100644
--- a/src/sagemaker/local/image.py
+++ b/src/sagemaker/local/image.py
@@ -31,6 +31,7 @@ import tarfile
 import tempfile
 
 from distutils.spawn import find_executable
+from functools import cache
 from threading import Thread
 from six.moves.urllib.parse import urlparse
 
@@ -52,6 +53,33 @@ S3_ENDPOINT_URL_ENV_NAME = "S3_ENDPOINT_URL"
 logger = logging.getLogger(__name__)
 
 
+@cache
+def get_docker_compose_cmd(*args):
+    cmd = []
+    # check docker compose v2 first
+    try:
+        output = subprocess.check_output(
+            ['docker', 'compose', 'version'],
+            universal_newlines=True,
+        )
+        if 'v2' in output.strip():
+            cmd.extend(['docker', 'compose'])
+
+    except subprocess.CalledProcessError:
+        # check if docker-compose is installed
+        if find_executable("docker-compose") is None:
+            raise ImportError(
+                "'docker-compose' is not installed. "
+                "Local Mode features will not work without docker-compose. "
+                "For more information on how to install 'docker-compose', please, see "
+                "https://docs.docker.com/compose/install/"
+            )
+        cmd.append('docker-compose')
+
+    cmd.extend(args)
+    return cmd
+
+
 class _SageMakerContainer(object):
     """Handle the lifecycle and configuration of a local container execution.
 
@@ -86,15 +114,6 @@ class _SageMakerContainer(object):
         """
         from sagemaker.local.local_session import LocalSession
 
-        # check if docker-compose is installed
-        if find_executable("docker-compose") is None:
-            raise ImportError(
-                "'docker-compose' is not installed. "
-                "Local Mode features will not work without docker-compose. "
-                "For more information on how to install 'docker-compose', please, see "
-                "https://docs.docker.com/compose/install/"
-            )
-
         self.sagemaker_session = sagemaker_session or LocalSession()
         self.instance_type = instance_type
         self.instance_count = instance_count
@@ -707,17 +726,14 @@ class _SageMakerContainer(object):
         Args:
             detached:
         """
-        compose_cmd = "docker-compose"
-
-        command = [
-            compose_cmd,
+        compose_args = [
             "-f",
             os.path.join(self.container_root, DOCKER_COMPOSE_FILENAME),
             "up",
             "--build",
             "--abort-on-container-exit" if not detached else "--detach",  # mutually exclusive
         ]
-
+        command = get_docker_compose_cmd(*compose_args)
         logger.info("docker command: %s", " ".join(command))
         return command
 
@@ -760,7 +776,7 @@ class _SageMakerContainer(object):
         # to setting --runtime=nvidia in the docker commandline.
         if self.instance_type == "local_gpu":
             host_config["deploy"] = {
-                "resources": {"reservations": {"devices": [{"capabilities": ["gpu"]}]}}
+                "resources": {"reservations": {"devices": [{"driver": "nvidia", "count": "all", "capabilities": ["gpu"]}]}}
             }
 
         if command == "serve":

@knikure knikure linked a pull request May 15, 2023 that will close this issue
9 tasks
@pdifranc
Copy link

@knikure How is #3855 fixing this issue? As a matter of fact, after this merge I cant pip install sagemaker[local] anymore due to a conflict in PyYAML versions

@ozancaglayan
Copy link

Yeah no idea, 3855 is totally irrelevant to this issue.

cfculhane pushed a commit to cfculhane/sagemaker-python-sdk that referenced this issue May 22, 2023
cfculhane pushed a commit to cfculhane/sagemaker-python-sdk that referenced this issue May 22, 2023
cfculhane pushed a commit to cfculhane/sagemaker-python-sdk that referenced this issue May 22, 2023
cfculhane pushed a commit to cfculhane/sagemaker-python-sdk that referenced this issue May 22, 2023
@Thesane
Copy link

Thesane commented Jul 20, 2023

you can just do that with compose v2
sudo ln -s /usr/libexec/docker/cli-plugins/docker-compose /usr/bin/docker-compose and run in local mode fine

@ozancaglayan
Copy link

This should be properly fixed in the SDK. Can you please re-open this issue @knikure ?

@Ce11an
Copy link

Ce11an commented Aug 17, 2023

Any movement on this? I have tried several different methods in my CI pipeline to no avail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment