Fix race condition (#21)

* feat: fix race conditions

* fix relative imports

* fix: acquire lock files on deployer init

* feat: add tests for deployment config

requirements.txt

@@ -1,6 +1,7 @@
-pyyaml
-flask
-pyjwt
-Flask-HTTPAuth
-python-dotenv
-requests
+pyyaml==6.0.1
+flask==3.0.0
+pyjwt==2.8.0
+Flask-HTTPAuth==4.8.0
+python-dotenv==1.0.0
+requests==2.31.0
+filelock==3.13.1

server/deployer.py

@@ -4,6 +4,8 @@
 import subprocess
 import typing
 
+import filelock
+
 from .utils import ComposeHelper, DeploymentConfig, NginxHelper, SecretsHelper
 
 logger = logging.getLogger(__name__)
@@ -16,26 +18,32 @@ def __init__(self, config: DeploymentConfig):
             "DEPLOYMENTS_MOUNT_DIR"
         )
         self._deployment_namespace = f"{self._config.project_name}_{self._config.branch_name}_{config.get_project_hash()}"
+        self._lock_file_path = os.path.join(
+            os.environ.get("LOCK_FILE_BASE_PATH") or "/tmp",
+            f"{self._deployment_namespace}.lock",
+        )
+        self._lock = filelock.FileLock(self._lock_file_path)
         self._project_path: typing.Final[str] = os.path.join(
             self._DEPLOYMENTS_MOUNT_DIR, self._deployment_namespace
         )
 
-        if config.rest_action != "DELETE":
-            self._setup_project()
+        with self._lock:
+            if config.rest_action != "DELETE":
+                self._setup_project()
 
-        self._compose_helper = ComposeHelper(
-            os.path.join(self._project_path, config.compose_file_location),
-            config.rest_action != "DELETE",
-        )
-        self._secrets_helper = SecretsHelper(
-            self._config.project_name, self._config.branch_name, self._project_path
-        )
-        self._outer_proxy_conf_location = (
-            os.environ.get("NGINX_PROXY_CONF_LOCATION") or "/etc/nginx/conf.d"
-        )
-        self._nginx_helper = NginxHelper(
-            config, self._outer_proxy_conf_location, self._project_path
-        )
+            self._compose_helper = ComposeHelper(
+                os.path.join(self._project_path, config.compose_file_location),
+                config.rest_action != "DELETE",
+            )
+            self._secrets_helper = SecretsHelper(
+                self._config.project_name, self._config.branch_name, self._project_path
+            )
+            self._outer_proxy_conf_location = (
+                os.environ.get("NGINX_PROXY_CONF_LOCATION") or "/etc/nginx/conf.d"
+            )
+            self._nginx_helper = NginxHelper(
+                config, self._outer_proxy_conf_location, self._project_path
+            )
 
     def _clone_project(self):
         process = subprocess.Popen(
@@ -50,18 +58,18 @@ def _clone_project(self):
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
         )
-
-        stdout, stderr = process.communicate()
-
+        try:
+            stdout, stderr = process.communicate()
+        except subprocess.TimeoutExpired as e:
+            logger.error(f"Error cloning the repo {self._config} with {e}")
+            raise
         if process.returncode == 0:
             logger.info("Git clone successful.")
         else:
             logger.error(f"Git clone failed. Return code: {process.returncode}")
-            logger.error("Standard Output:")
-            logger.error(stdout.decode())
-            logger.error("Standard Error:")
-            logger.error(stderr.decode())
-            raise Exception("Git clone failed")
+            logger.error(f"Standard Output: {stdout.decode()}")
+            logger.error(f"Standard Error: {stderr.decode()}")
+            raise Exception(f"Cloning the Git repo failed {self._config}")
 
     def _setup_project(self):
         if os.path.exists(self._project_path):
@@ -89,11 +97,6 @@ def _deploy_project(self):
         )
         return urls
 
-    def deploy_preview_environment(self):
-        urls = self._deploy_project()
-        self._configure_outer_proxy()
-        return urls
-
     def _delete_deployment_files(self):
         if not os.path.exists(self._project_path):
             print(f"{self._project_path} already deleted!")
@@ -103,8 +106,15 @@ def _delete_deployment_files(self):
         except Exception as e:
             logger.debug(f"Error removing deployment files {e}")
 
+    def deploy_preview_environment(self):
+        with self._lock:
+            urls = self._deploy_project()
+            self._configure_outer_proxy()
+        return urls
+
     def delete_preview_environment(self):
-        self._compose_helper.remove_services()
-        self._nginx_helper.remove_outer_proxy()
-        self._nginx_helper.reload_nginx()
-        self._delete_deployment_files()
+        with self._lock:
+            self._compose_helper.remove_services()
+            self._nginx_helper.remove_outer_proxy()
+            self._nginx_helper.reload_nginx()
+            self._delete_deployment_files()

server/utils.py

@@ -26,6 +26,12 @@ class DeploymentConfig:
     def get_project_hash(self):
         return get_random_stub(f"{self.project_name}:{self.branch_name}")
 
+    def __repr__(self):
+        return (
+            f"DeploymentConfig({self.project_name!r}, {self.branch_name!r}, {self.project_git_url!r}, "
+            f"{self.compose_file_location!r}, {self.rest_action!r})"
+        )
+
 
 class ComposeHelper:
     NGINX_SERVICE_TEMPLATE: typing.Final[

tests/conftest.py

@@ -77,6 +77,7 @@ def deployment_config():
         project_name="test-project-name",
         branch_name="test-branch-name",
         project_git_url="https://github.com/tushar5526/test-project-name.git",
+        rest_action="POST",
     )
 
 

tests/test_utils.py

@@ -236,3 +236,11 @@ def test_remove_outer_proxy_when_file_is_deleted_already(nginx_helper, mocker):
     nginx_helper.remove_outer_proxy()
 
     # Then No error should be raised
+
+
+def test_deployment_config_repr(deployment_config):
+    expected_repr = (
+        "DeploymentConfig('test-project-name', 'test-branch-name', "
+        "'https://github.com/tushar5526/test-project-name.git', 'docker-compose.yml', 'POST')"
+    )
+    assert repr(deployment_config) == expected_repr