From 3f8c56e00b3f66926d8a88e8d8d454e26a960651 Mon Sep 17 00:00:00 2001 From: Chi Lo Date: Fri, 30 Aug 2019 05:18:27 -0700 Subject: [PATCH] Git push retries when updating SoT Git push may fail when concurrent git pushes to code cloud occurs. This patch provides retries capability when git push failed. Change-Id: I6cab95cb4caf3bf914c47046a20007570ff634d0 --- orm/base_config.py | 8 ++ orm/services/resource_distributor/config.py | 4 +- .../rds/sot/git_sot/git_native.py | 14 ++- .../rds/sot/git_sot/git_sot.py | 94 +++++++++++------- .../unit/rds/sot/git_sot/test_git_sot.py | 95 ++++++++++++++----- 5 files changed, 152 insertions(+), 63 deletions(-) diff --git a/orm/base_config.py b/orm/base_config.py index 0b9a1db0..861f6da5 100644 --- a/orm/base_config.py +++ b/orm/base_config.py @@ -295,6 +295,12 @@ OrmRdsGroup = [ cfg.StrOpt('customer_domain', default='Default', help='Keystone project domain'), + cfg.IntOpt('git_retries', + default=5, + help='Maximum number of retries for git operation.'), + cfg.IntOpt('git_retries_interval', + default=10, + help='Wait time in seconds for git retries.'), cfg.StrOpt('log', default='rds.log', help='Rds log name.') @@ -386,6 +392,8 @@ rds = {'port': CONF.rds.port, 'repo_email': CONF.rds.repo_email, 'customer_domain': CONF.rds.customer_domain, 'base_url': '{}://{}:{}/'.format(protocol, orm_host, CONF.rds.port), + 'git_retries': CONF.rds.git_retries, + 'git_retries_interval': CONF.rds.git_retries_interval, 'log': '{}/{}'.format(CONF.log_location, CONF.rds.log)} cli = {'base_region': CONF.cli.base_region} diff --git a/orm/services/resource_distributor/config.py b/orm/services/resource_distributor/config.py index 39a42a9d..b9982f5a 100755 --- a/orm/services/resource_distributor/config.py +++ b/orm/services/resource_distributor/config.py @@ -31,7 +31,9 @@ git = { 'commit_user': config.rds['repo_user'], 'commit_email': config.rds['repo_email'], 'git_server_url': config.rds['repo_remote_location'], - 'git_cmd_timeout': 45 + 'git_cmd_timeout': 45, + 'git_retries': config.rds['git_retries'], + 'git_retries_interval': config.rds['git_retries_interval'] } audit = { diff --git a/orm/services/resource_distributor/rds/sot/git_sot/git_native.py b/orm/services/resource_distributor/rds/sot/git_sot/git_native.py index 4fb207c7..cb5b9e3c 100644 --- a/orm/services/resource_distributor/rds/sot/git_sot/git_native.py +++ b/orm/services/resource_distributor/rds/sot/git_sot/git_native.py @@ -20,10 +20,14 @@ class GitNative(BaseGit): try: logger.info("Local repository path:{}, " "Git server url: {}, " - "Git command timeout: " + "Git command timeout: {} seconds, " + "Git retries: {}, " + "Git retries interval: " "{} seconds".format(conf.git.local_repository_path, conf.git.git_server_url, - conf.git.git_cmd_timeout)) + conf.git.git_cmd_timeout, + conf.git.git_retries, + conf.git.git_retries_interval)) out, error = self._git_pull(conf.git.local_repository_path) if self._is_conflict(out) or self._is_error(error): @@ -137,8 +141,10 @@ class GitNative(BaseGit): def _git_config(self, repo_dir): logger.info("Set git configuration params started.") - cmds = ['git config --global user.name {}'.format(conf.git.commit_user), - 'git config --global user.email {}'.format(conf.git.commit_email)] + cmds = ['git config --global user.name {}'.format( + conf.git.commit_user), + 'git config --global user.email {}'.format( + conf.git.commit_email)] for cmd in cmds: self._execute_git_cmd(cmd, repo_dir) diff --git a/orm/services/resource_distributor/rds/sot/git_sot/git_sot.py b/orm/services/resource_distributor/rds/sot/git_sot/git_sot.py index 64781861..cbdc78a5 100755 --- a/orm/services/resource_distributor/rds/sot/git_sot/git_sot.py +++ b/orm/services/resource_distributor/rds/sot/git_sot/git_sot.py @@ -1,14 +1,18 @@ import logging import os import threading +import time import git_factory from git_base import (GitInitError, GitResetError, GitUploadError, GitValidateError) -from orm.services.resource_distributor.rds.ordupdate.ord_notifier import notify_ord +from orm.services.resource_distributor.rds.ordupdate.ord_notifier \ + import notify_ord from orm.services.resource_distributor.rds.sot import base_sot from orm.services.resource_distributor.rds.sot.base_sot import SoTError +from pecan import conf + logger = logging.getLogger(__name__) lock = threading.Lock() @@ -49,45 +53,67 @@ class GitSoT(base_sot.BaseSoT): thread.start() +def update_sot_with_retries( + git_impl, my_lock, resource_list, retries_interval=1, retries=1): + commit_id = "" + # Only retry when GitUploadError occurs + while(retries > 0): + try: + logger.info(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>") + logger.info("Acquire Git lock...") + # Lock the entire git operations, so that no other threads + # change local # files. + my_lock.acquire() + logger.info("Git lock acquired !!!!") + logger.info("Update sot retries count: {}".format(retries)) + + init_git(git_impl) + handle_file_operations(resource_list) + commit_id = update_git(git_impl) + break + except SoTError as exc: + logger.error("Save resource to SoT Git repository failed. " + "Reason: {}.". + format(exc.message)) + break + except GitInitError as init_exc: + logger.error("Initializing Git repository Failed. Reason: {}.". + format(init_exc.message)) + break + except GitUploadError as upload_exc: + logger.error("Uploading to Git repository Failed. Reason: {}.". + format(upload_exc.message)) + cleanup(git_impl) + retries -= 1 + if (retries > 0): + time.sleep(retries_interval) + except Exception as e: + logger.error("Unknown Exception when updating sot: {}".format( + e.message)) + break + finally: + logger.info("Release Git lock...") + my_lock.release() + logger.info("Git lock released !!!!") + logger.info("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<") + + return commit_id + + def update_sot(git_impl, my_lock, tracking_id, transaction_id, resource_list, application_id, user_id, headers={}): logger.info("Save resource to SoT. start ...") - commit_id = "" - result = False - logger.info(">>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>") - logger.info("Acquire Git lock...") - # Lock the entire git operations, so that no other threads change local - # files. - my_lock.acquire() - logger.info("Git lock acquired !!!!") - try: - init_git(git_impl) - handle_file_operations(resource_list) - - commit_id = update_git(git_impl) + commit_id = update_sot_with_retries( + git_impl, + my_lock, + resource_list, + retries_interval=conf.git.git_retries_interval, + retries=conf.git.git_retries) + if commit_id: logger.info("All files were successfully updated in Git server :-)\n") - result = True - - except SoTError as exc: - logger.error("Save resource to SoT Git repository failed. " - "Reason: {}.". - format(exc.message)) - except GitInitError as init_exc: - logger.error("Initializing Git repository Failed. Reason: {}.". - format(init_exc.message)) - except GitUploadError as upload_exc: - logger.error("Uploading to Git repository Failed. Reason: {}.". - format(upload_exc.message)) - cleanup(git_impl) - finally: - logger.info("Release Git lock...") - my_lock.release() - logger.info("Git lock released !!!!") - logger.info("<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<") - # This method is called also in case exception raised. # Notification to ords will not be sent but status db and audit # will be updated. @@ -105,7 +131,7 @@ def update_sot(git_impl, my_lock, tracking_id, transaction_id, resource_list, application_id, # application_id is not available user_id, # user_id is not available "NA", # external_id is not available - not result, + not commit_id, headers) except Exception as e: logger.error("Error in updating ORD! Error: {}".format( diff --git a/orm/tests/unit/rds/sot/git_sot/test_git_sot.py b/orm/tests/unit/rds/sot/git_sot/test_git_sot.py index cb0a4218..215b8d1d 100755 --- a/orm/tests/unit/rds/sot/git_sot/test_git_sot.py +++ b/orm/tests/unit/rds/sot/git_sot/test_git_sot.py @@ -5,8 +5,8 @@ import unittest import mock from mock import patch from orm.services.resource_distributor.rds.sot.base_sot import SoTError -from orm.services.resource_distributor.rds.sot.git_sot.git_base import (GitInitError, GitResetError, - GitUploadError, GitValidateError) +from orm.services.resource_distributor.rds.sot.git_sot.git_base import ( + GitInitError, GitResetError, GitUploadError, GitValidateError) from orm.services.resource_distributor.rds.sot.git_sot import git_sot as sot from orm.services.resource_distributor.rds.sot import sot_factory from orm.tests.unit.rds import config as conf @@ -43,39 +43,75 @@ class GitSoTTest(unittest.TestCase): git_factory.get_git_impl = mock.MagicMock() # update_sot - @patch.object(sot, 'init_git', side_effect=GitInitError("Failed to initialize Git")) - def test_git_sot_update_sot_init_git_fail(self, result): + @patch.object(sot, 'init_git', + side_effect=GitInitError("Failed to initialize Git")) + def test_git_sot_update_sot_retries_init_git_fail(self, mock_init): """" init_git fails and raise exception""" try: - sot.update_sot("", lock, '1', '2', ['3', '5'], '4', '6') - except SoTError: + sot.update_sot_with_retries("", lock, ['c', 'd']) + except GitInitError: self.fail("Exception should have been handled inside method") - @patch.object(sot, 'handle_file_operations', side_effect=SoTError("Failed to create file path")) + @patch.object(sot, 'handle_file_operations', + side_effect=SoTError("Failed to create file path")) @patch.object(sot, 'init_git') - def test_git_sot_update_sot_create_files_fail(self, git_repo, result): + def test_git_sot_update_sot_retries_create_files_fail( + self, mock_init, mock_handle): """" create_files fails and raise exception""" try: - sot.update_sot("", lock, "a", "b", ['c', 'd'], 'e', 'f') + sot.update_sot_with_retries("", lock, ['c', 'd']) except SoTError: self.fail("Exception should have been handled inside method") - @patch.object(sot, 'update_git', side_effect=GitUploadError("Failed to upload file to Git")) + @patch.object(sot, 'update_git', + side_effect=GitUploadError("Failed to upload file to Git")) @patch.object(sot, 'init_git') @patch.object(sot, 'handle_file_operations') @patch.object(sot, 'cleanup') - def test_git_sot_update_sot_update_git_fail(self, git_repo, files_created, result, clean_result): + def test_git_sot_update_sot_retries_update_git_fail( + self, mock_cleanup, mock_handle, mock_init, mock_update): """" update git fails and raise exception""" try: - sot.update_sot("", lock, 'a', 'b', ['c', 'd'], 'e', 'f') + sot.update_sot_with_retries("", lock, ['c', 'd']) except GitUploadError: self.fail("Exception should have been handled inside method") + @patch.object(sot, 'init_git', + side_effect=GitResetError("Unknown error in test method")) + def test_git_sot_update_sot_retries_unknown_error_thrown(self, mock_init): + """" init_git fails and raise exception""" + try: + sot.update_sot_with_retries("", lock, ['c', 'd']) + except Exception: + self.fail("Exception should have been handled inside method") + @patch.object(sot, 'update_git') @patch.object(sot, 'init_git') @patch.object(sot, 'handle_file_operations') @patch.object(sot, 'cleanup') - def test_git_sot_update_sot_success(self, git_repo, files_created, result, cleanup_result): + def test_git_sot_update_sot_retries_success( + self, mock_cleanup, mock_handle, mock_init, mock_update): + """"no exceptions raised""" + try: + sot.update_sot_with_retries("", lock, ['c', 'd']) + except SoTError: + self.fail("Exception should have been handled inside method") + + @patch.object(sot, 'conf') + @patch.object(sot, 'update_sot_with_retries') + @patch.object(sot, 'notify_ord', + side_effect=Exception("Error in updating ORD")) + def test_git_sot_update_sot_notify_ord_fail( + self, mock_notify, mock_update, mock_conf): + """"no exceptions raised""" + try: + sot.update_sot("", lock, "a", "b", ['c', 'd'], 'e', 'f') + except Exception: + self.self("Exception should have been handled inside method") + + @patch.object(sot, 'conf') + @patch.object(sot, 'update_sot_with_retries', return_value="") + def test_git_sot_update_sot_success(self, mock_update, mock_conf): """"no exceptions raised""" try: sot.update_sot("", lock, "a", "b", ['c', 'd'], 'e', 'f') @@ -89,7 +125,8 @@ class GitSoTTest(unittest.TestCase): @patch.object(os.path, 'dirname', return_value="File path") @patch.object(os.path, 'exists', return_value=False) @patch.object(os, 'makedirs') - def test_git_sot_create_dir_path_not_exist_success(self, dir_name, exists, makedirs): + def test_git_sot_create_dir_path_not_exist_success( + self, dir_name, exists, makedirs): """create directory path when path not exist and makedir success""" sot.create_dir("my_file") @@ -102,8 +139,10 @@ class GitSoTTest(unittest.TestCase): @patch.object(os.path, 'dirname', return_value="File path") @patch.object(os.path, 'exists', return_value=False) @patch.object(os, 'makedirs', side_effect=OSError("Could not make dir")) - def test_git_sot_create_dir_makedir_fails(self, dir_name, exists, makedirs): - """create direcory path makedir throws exception and the methos throws SoTError""" + def test_git_sot_create_dir_makedir_fails( + self, dir_name, exists, makedirs): + """create direcory path makedir throws exception and """ + """the methos throws SoTError""" with self.assertRaises(SoTError): sot.create_dir("my_file") @@ -111,7 +150,8 @@ class GitSoTTest(unittest.TestCase): # save_resource_to_sot # ############################ - @patch.object(threading, 'Thread', return_value=threading.Thread(target=dummy_thread_method)) + @patch.object(threading, 'Thread', + return_value=threading.Thread(target=dummy_thread_method)) def test_git_sot_save_resource_to_sot(self, thread): """Check the save runs in a new thread""" sot_factory.sot_type = "git" @@ -134,16 +174,19 @@ class GitSoTTest(unittest.TestCase): except SoTError: self.fail("No exceptions should be thrown in this case") - @patch.object(sot, 'create_dir', side_effect=SoTError("Failed to create file path")) + @patch.object(sot, 'create_dir', + side_effect=SoTError("Failed to create file path")) def test_git_sot_create_file_in_path_create_dir_failed(self, aDir): """create file in path fail, create file raise exception """ with self.assertRaises(SoTError): sot.create_file_in_path("path", "data") @patch.object(sot, 'create_dir', return_value="File path") - @patch.object(sot, 'write_data_to_file', side_effect=SoTError("Could not write to file")) + @patch.object(sot, 'write_data_to_file', + side_effect=SoTError("Could not write to file")) def test_git_sot_create_file_in_path_create_file_failed(self, aDir, aFile): - """create file in path fail,create dir success, writing data to file failed """ + """create file in path fail,create dir success, """ + """writing data to file failed """ with self.assertRaises(SoTError): sot.create_file_in_path("path", "data") @@ -198,14 +241,16 @@ class GitSoTTest(unittest.TestCase): @patch('__builtin__.open') @patch.object(os, 'close') @patch.object(os, 'write', return_value=True) - def test_git_sot_write_data_to_file_success(self, result1, result2, result3): + def test_git_sot_write_data_to_file_success( + self, result1, result2, result3): """Check create_file success """ try: sot.write_data_to_file("file_path", "data11") except SoTError: self.fail("No exceptions should be thrown in this case") - @patch('__builtin__.open', side_effect=IOError("Failed writing data to file")) + @patch('__builtin__.open', + side_effect=IOError("Failed writing data to file")) def test_git_sot_write_data_to_file_failed(self, result1): """Check create file failed , could not write to file """ with self.assertRaises(SoTError): @@ -227,7 +272,8 @@ class GitSoTTest(unittest.TestCase): def test_git_sot_cleanup_files_remove_failed(self): """Check cleanup fail because reset git failed """ git_impl = mock.MagicMock() - git_impl.git_reset_changes = mock.MagicMock(side_effect=GitResetError("failed to reset")) + git_impl.git_reset_changes = mock.MagicMock( + side_effect=GitResetError("failed to reset")) with self.assertRaises(SoTError): sot.cleanup(git_impl) @@ -247,7 +293,8 @@ class GitSoTTest(unittest.TestCase): def test_git_sot_init_git_gittle_failed(self): """Check init_git failed """ git_impl = mock.MagicMock() - git_impl.git_init = mock.MagicMock(side_effect=GitInitError("failed to init")) + git_impl.git_init = mock.MagicMock(side_effect=GitInitError( + "failed to init")) with self.assertRaises(GitInitError): sot.init_git(git_impl)