From efd891bec55b8698eff7f05f28ad716df9c98bf6 Mon Sep 17 00:00:00 2001
From: Reid Mello <30907815+rjmello@users.noreply.github.com>
Date: Wed, 7 Aug 2024 21:56:38 -0400
Subject: [PATCH] Raise error for provider w/ internal container mgmt

The endpoint CLI will now raise an error if the endpoint configuration
includes both the `container_uri` field and a provider that manages
containers internally. This prevents conflicts in container management.
---
 ...lo_gcengine_with_k8s_provider_sc_33870.rst |  7 ++++++
 .../endpoint/config/model.py                  | 18 +++++++++++++++
 .../tests/unit/test_endpoint_config.py        | 23 +++++++++++++++++++
 3 files changed, 48 insertions(+)
 create mode 100644 changelog.d/20240807_183559_30907815+rjmello_gcengine_with_k8s_provider_sc_33870.rst

diff --git a/changelog.d/20240807_183559_30907815+rjmello_gcengine_with_k8s_provider_sc_33870.rst b/changelog.d/20240807_183559_30907815+rjmello_gcengine_with_k8s_provider_sc_33870.rst
new file mode 100644
index 000000000..7251a41b5
--- /dev/null
+++ b/changelog.d/20240807_183559_30907815+rjmello_gcengine_with_k8s_provider_sc_33870.rst
@@ -0,0 +1,7 @@
+Bug Fixes
+^^^^^^^^^
+
+- The endpoint CLI will now raise an error if the endpoint configuration includes
+  both the ``container_uri`` field and a provider that manages containers internally
+  (``AWSProvider``, ``GoogleCloudProvider``, or ``KubernetesProvider``). This prevents
+  conflicts in container management.
\ No newline at end of file
diff --git a/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py b/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py
index 1e7a388b7..4264808e5 100644
--- a/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py
+++ b/compute_endpoint/globus_compute_endpoint/endpoint/config/model.py
@@ -121,6 +121,24 @@ def _validate_engine_strategy(cls, values: dict):
             )
         return values
 
+    @root_validator(pre=True)
+    @classmethod
+    def _validate_provider_container_compatibility(cls, values: dict):
+        provider_type = values.get("provider", {}).get("type")
+        if provider_type in (
+            "AWSProvider",
+            "GoogleCloudProvider",
+            "KubernetesProvider",
+        ):
+            if values.get("container_uri"):
+                raise ValueError(
+                    f"The 'container_uri' field is not compatible with {provider_type}"
+                    " because this provider manages containers internally. For more"
+                    f" information on how to configure {provider_type}, please refer to"
+                    f" Parsl documentation: https://parsl.readthedocs.io/en/stable/stubs/parsl.providers.{provider_type}.html"  # noqa"
+                )
+        return values
+
 
 class ConfigModel(BaseConfigModel):
     engine: t.Optional[EngineModel]
diff --git a/compute_endpoint/tests/unit/test_endpoint_config.py b/compute_endpoint/tests/unit/test_endpoint_config.py
index 3cbb2f7b7..70b388056 100644
--- a/compute_endpoint/tests/unit/test_endpoint_config.py
+++ b/compute_endpoint/tests/unit/test_endpoint_config.py
@@ -122,3 +122,26 @@ def test_conditional_engine_strategy(
             with pytest.raises(ValidationError) as pyt_e:
                 ConfigModel(**config_dict)
             assert "string is incompatible" in str(pyt_e.value)
+
+
+@pytest.mark.parametrize(
+    "provider_type, compatible",
+    (
+        ("LocalProvider", True),
+        ("AWSProvider", False),
+        ("GoogleCloudProvider", False),
+        ("KubernetesProvider", False),
+    ),
+)
+def test_provider_container_compatibility(
+    config_dict: dict, provider_type: str, compatible: bool
+):
+    config_dict["engine"]["container_uri"] = "docker://ubuntu"
+    config_dict["engine"]["provider"] = {"type": provider_type}
+
+    if compatible:
+        ConfigModel(**config_dict)
+    else:
+        with pytest.raises(ValueError) as pyt_e:
+            ConfigModel(**config_dict)
+        assert f"not compatible with {provider_type}" in str(pyt_e.value)