From 3a0ece856bb6cef9e2ef886d407846241a072f55 Mon Sep 17 00:00:00 2001 From: Justintime50 <39606064+Justintime50@users.noreply.github.com> Date: Mon, 28 Nov 2022 19:55:05 -0700 Subject: [PATCH] feat: locks can be distinguished between user and system --- CHANGELOG.md | 1 + harvey/api.py | 10 +++++++--- harvey/deployments.py | 8 ++++++-- harvey/locks.py | 31 +++++++++++++++++++------------ harvey/utils.py | 9 ++++----- test/unit/test_deployments.py | 4 ++-- 6 files changed, 39 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e24f7e..c6c7b21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Healthchecks of containers now check when the container was started to ensure that not only are they running, but that they restarted within the last 60 seconds as a part of the deploy - Fixed a bug where containers may not properly recreate when their configs or images don't differ from the last deploy - If a project's lock status cannot be determined, we now kill the process instead of continuing and logging only +- Harvey can now distinguish between a system lock and a user lock allowing for user-locked deployments to stay that way even if a deployment fails - Wraps `store_deployment_details` in a try/except to log out errors with saving details to the DB ## v0.21.0 (2022-11-21) diff --git a/harvey/api.py b/harvey/api.py index 7b2e137..ffe5e6d 100644 --- a/harvey/api.py +++ b/harvey/api.py @@ -222,12 +222,16 @@ def retrieve_lock(project_name: str) -> Dict[str, bool]: """Retrieve the `lock` status of a project via its fully-qualified repo name.""" lock_status = Lock.lookup_project_lock(project_name) - return {'locked': lock_status} + return {'locked': lock_status['locked']} @staticmethod def lock_project(project_name: str): - """Locks the deployments of a project.""" - lock_status = Lock.update_project_lock(project_name=project_name, locked=True) + """Locks the deployments of a project via user request.""" + lock_status = Lock.update_project_lock( + project_name=project_name, + locked=True, + system_lock=False, + ) return {'locked': lock_status} diff --git a/harvey/deployments.py b/harvey/deployments.py index 2ed49a5..804ea3f 100644 --- a/harvey/deployments.py +++ b/harvey/deployments.py @@ -30,7 +30,7 @@ def initialize_deployment(webhook: Dict[str, Any]) -> Tuple[Dict[str, Any], str, try: # Kill the deployment if the project is locked - if Lock.lookup_project_lock(Webhook.repo_full_name(webhook)) is True: + if Lock.lookup_project_lock(Webhook.repo_full_name(webhook))['locked'] is True: Utils.kill_deployment( f'{Webhook.repo_full_name(webhook)} deployments are locked. Please try again later or unlock' ' deployments.', @@ -45,7 +45,11 @@ def initialize_deployment(webhook: Dict[str, Any]) -> Tuple[Dict[str, Any], str, start_time = datetime.datetime.utcnow() - _ = Lock.update_project_lock(project_name=Webhook.repo_full_name(webhook), locked=True) + _ = Lock.update_project_lock( + project_name=Webhook.repo_full_name(webhook), + locked=True, + system_lock=True, + ) Utils.store_deployment_details(webhook) # Run git operation first to ensure the config is present and up-to-date git = Git.update_git_repo(webhook) diff --git a/harvey/locks.py b/harvey/locks.py index 82f8224..6a299b2 100644 --- a/harvey/locks.py +++ b/harvey/locks.py @@ -1,3 +1,9 @@ +from typing import ( + Any, + Dict, + Optional, +) + import woodchips from sqlitedict import SqliteDict # type: ignore @@ -10,22 +16,25 @@ class Lock: @staticmethod - def update_project_lock(project_name: str, locked: bool = False) -> bool: + def update_project_lock(project_name: str, locked: bool = False, system_lock: Optional[bool] = True) -> bool: """Locks or unlocks the project's deployments to ensure we don't crash Docker with two inflight deployments. This function will also create locks for new projects. - Locking should only happen once a deployment is begun. A locked deployment should then always be - unlocked once it's finished regardless of status so another deployment can follow. + Locking should only happen once a deployment has begun. A locked deployment should then always be + unlocked once it's finished regardless of status (except for user requested locks) so another deployment can + follow. """ logger = woodchips.get(Config.logger_name) locked_string = 'Locking' if locked is True else 'Unlocking' logger.info(f'{locked_string} deployments for {project_name}...') - corrected_project_name = project_name.replace("/", "-") + formatted_project_name = project_name.replace("/", "-") + system_lock_value = None if locked is False else system_lock # Don't allow this to be set if unlocking with SqliteDict(filename=Config.database_file, tablename=DATABASE_TABLE_NAME) as database_table: - database_table[corrected_project_name.replace("/", "-")] = { + database_table[formatted_project_name] = { 'locked': locked, + 'system_lock': system_lock_value, } database_table.commit() @@ -33,14 +42,12 @@ def update_project_lock(project_name: str, locked: bool = False) -> bool: return locked @staticmethod - def lookup_project_lock(project_name: str) -> bool: - """Checks if a project is locked or not by its full name.""" - locked_value = False - corrected_project_name = project_name.replace("/", "-") + def lookup_project_lock(project_name: str) -> Dict[str, Any]: + """Looks up a project's lock object by its full name.""" + formatted_project_name = project_name.replace("/", "-") with SqliteDict(filename=Config.database_file, tablename=DATABASE_TABLE_NAME) as database_table: for key, value in database_table.iteritems(): - if key == corrected_project_name: - locked_value = value['locked'] - return locked_value + if key == formatted_project_name: + return value raise HarveyError('Lock does not exist!') diff --git a/harvey/utils.py b/harvey/utils.py index ea6047c..1ad92ff 100644 --- a/harvey/utils.py +++ b/harvey/utils.py @@ -33,11 +33,10 @@ def kill_deployment(message: str, webhook: Dict[str, Any], raise_error: Optional if Config.use_slack: Message.send_slack_message(error_message) - # TODO: We need to distinguish between a user defined lock and a system defined lock. Users may want to ensure - # something can't get deployed and should always stay locked whereas the system may lock this during a deploy - # but on a failure, we should unlock to allow a new deployment to go through. Currently, all deployments get - # unlocked which may not be ideal if a deployment fails. - _ = Lock.update_project_lock(project_name=Webhook.repo_full_name(webhook), locked=False) + # Only unlock deployments that were locked by the system and not a user to preserve their preferences + deployment_lock = Lock.lookup_project_lock(project_name=Webhook.repo_full_name(webhook)) + if deployment_lock['system_lock']: + _ = Lock.update_project_lock(project_name=Webhook.repo_full_name(webhook), locked=False) if raise_error: raise HarveyError(error_message) diff --git a/test/unit/test_deployments.py b/test/unit/test_deployments.py index 0adb6d5..21379b7 100644 --- a/test/unit/test_deployments.py +++ b/test/unit/test_deployments.py @@ -24,7 +24,7 @@ def mock_config(deployment_type='deploy', prod_compose=False): return mock_config -@patch('harvey.locks.Lock.lookup_project_lock', return_value=False) +@patch('harvey.locks.Lock.lookup_project_lock', return_value={'locked': False}) @patch('harvey.config.Config.use_slack', True) @patch('harvey.git.Git.update_git_repo') @patch('harvey.deployments.Deployment.open_project_config', return_value=mock_config()) @@ -37,7 +37,7 @@ def test_initialize_deployment_slack( mock_slack_message.assert_called_once() -@patch('harvey.locks.Lock.lookup_project_lock', return_value=False) +@patch('harvey.locks.Lock.lookup_project_lock', return_value={'locked': False}) @patch('harvey.git.Git.update_git_repo') @patch('harvey.deployments.Deployment.open_project_config', return_value=mock_config()) def test_initialize_deployment(mock_open_project_config, mock_update_git_repo, mock_project_lock, mock_webhook):