From d3d3a0ecb7041ebd38251ebdaeebd6a70045399c Mon Sep 17 00:00:00 2001 From: Anthony Lin Date: Tue, 24 Apr 2018 10:33:19 +0000 Subject: [PATCH] Add 'allow-intermediate-commits' option for Site Action Shipyard Create Action will stop user from executing a site action when the current committed revision of documents has other prior commits that have not been used as part of a site action, e.g. update_site. We can re-run the action with the 'allow-intermediate-commits' option if we are aware of these other commits and if they are intended. This patch set updates the relevant Shipyard API, API client, CLI as well as document for this new option. It also made some small updates to the 'commit configdocs' documentation. Change-Id: I94a4cad313a69dd6fbc626ab923c6eae5cf3db7a --- docs/source/API.rst | 11 +++++ docs/source/CLI.rst | 44 ++++++++++++++++++- .../control/action/actions_api.py | 42 ++++++++++++++++-- .../control/configdocs/configdocs_helper.py | 42 ++++++++++++++++++ .../tests/unit/control/test_actions_api.py | 20 +++++++-- .../api_client/shipyard_api_client.py | 12 +++-- .../shipyard_client/cli/create/actions.py | 15 ++++--- .../shipyard_client/cli/create/commands.py | 15 +++++-- .../unit/cli/create/test_create_actions.py | 24 ++++++---- .../unit/cli/create/test_create_commands.py | 2 +- 10 files changed, 198 insertions(+), 29 deletions(-) diff --git a/docs/source/API.rst b/docs/source/API.rst index c3f48c46..40955d52 100644 --- a/docs/source/API.rst +++ b/docs/source/API.rst @@ -421,6 +421,17 @@ a DAG invocation), perform any checks to validate the preconditions to run the DAG, and trigger the invocation of the DAG. The DAG will run asynchronously in airflow. +Query Parameters +'''''''''''''''' +allow-intermediate-commits=true | **false** + By default, false. User will not be able to continue with a site action, + e.g. update_site if the current committed revision of documents has other + prior commits that have not been used as part of a site action. With + allow-intermediate-commits=true, it allows user to override the default + behavior and continue with the site action. This may be the case when the + user is aware of the existence of such commits and/or when such commits are + intended. + Responses ''''''''' 201 Created diff --git a/docs/source/CLI.rst b/docs/source/CLI.rst index a2624a1c..dbd9abb2 100644 --- a/docs/source/CLI.rst +++ b/docs/source/CLI.rst @@ -107,6 +107,7 @@ downstream components. shipyard commit configdocs [--force] + [--dryrun] Example: shipyard commit configdocs @@ -122,7 +123,42 @@ Sample :: - TBD + $ shipyard commit configdocs + Configuration documents committed. + Status: Validations succeeded + Reason: Validation + - Info: DD1001 + Message: Rational Boot Storage: Validation successful. + Source: Drydock + - Info: DD2002 + Message: IP Locality Check: Validation successful. + Source: Drydock + - Info: DD2003 + Message: MTU Rationality: Validation successful. + Source: Drydock + - Info: DD2004 + Message: Network Trunking Rationalty: Validation successful. + Source: Drydock + - Info: DD2005 + Message: Duplicated IP Check: Validation successful. + Source: Drydock + - Info: DD3001 + Message: Platform Selection: Validation successful. + Source: Drydock + - Info: DD1006 + Message: Network Bond Rationality: Validation successful. + Source: Drydock + - Info: DD2002 + Message: Storage Partitioning: Validation successful. + Source: Drydock + - Info: DD2003 + Message: Storage Sizing: Validation successful. + Source: Drydock + - Info: DD1007 + Message: Allowed Network Check: Validation successful. + Source: Drydock + + #### Errors: 0, Warnings: 0, Infos: 10, Other: 0 #### Control commands ---------------- @@ -206,6 +242,7 @@ id of the action invoked so that it can be queried subsequently. shipyard create action --param= (repeatable) + [--allow-intermediate-commits] Example: shipyard create action redeploy_server --param="server-name=mcp" @@ -216,6 +253,11 @@ id of the action invoked so that it can be queried subsequently. \--param= A parameter to be provided to the action being invoked. (repeatable) +\--allow-intermediate-commits + Allows continuation of a site action, e.g. update_site even when the + current committed revision of documents has other prior commits that + have not been used as part of a site action. + Sample ^^^^^^ diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py index 84dd632d..6781427b 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/action/actions_api.py @@ -74,15 +74,24 @@ class ActionsResource(BaseResource): """ Accept an action into shipyard """ + # The 'allow-intermediate-commits' query parameter is set to False + # unless explicitly set to True + allow_intermediate_commits = ( + req.get_param_as_bool(name='allow-intermediate-commits')) + input_action = self.req_json(req, validate_json_schema=ACTION) - action = self.create_action(action=input_action, context=req.context) + action = self.create_action( + action=input_action, + context=req.context, + allow_intermediate_commits=allow_intermediate_commits) + LOG.info("Id %s generated for action %s", action['id'], action['name']) # respond with the action and location for checking status resp.status = falcon.HTTP_201 resp.body = self.to_json(action) resp.location = '/api/v1.0/actions/{}'.format(action['id']) - def create_action(self, action, context): + def create_action(self, action, context, allow_intermediate_commits=False): # use uuid assigned for this request as the id of the action. action['id'] = ulid.ulid() # the invoking user @@ -99,10 +108,15 @@ class ActionsResource(BaseResource): dag = SUPPORTED_ACTION_MAPPINGS.get(action['name'])['dag'] action['dag_id'] = dag - # Retrieve last committed design revision + # Set up configdocs_helper self.configdocs_helper = ConfigdocsHelper(context) + + # Retrieve last committed design revision action['committed_rev_id'] = self.get_committed_design_version() + # Check for intermediate commit + self.check_intermediate_commit_revision(allow_intermediate_commits) + # populate action parameters if they are not set if 'parameters' not in action: action['parameters'] = {} @@ -339,3 +353,25 @@ class ActionsResource(BaseResource): description='No committed version found in Deckhand', status=falcon.HTTP_404, retry=False) + + def check_intermediate_commit_revision(self, + allow_intermediate_commits=False): + + LOG.info("Checking for intermediate committed revision in Deckhand...") + intermediate_commits = ( + self.configdocs_helper.check_intermediate_commit()) + + if intermediate_commits and not allow_intermediate_commits: + + raise ApiError( + title='Intermediate Commit Detected', + description=( + 'The current committed revision of documents has ' + 'other prior commits that have not been used as ' + 'part of a site action, e.g. update_site. If you ' + 'are aware and these other commits are intended, ' + 'please rerun this action with the option ' + '`allow-intermediate-commits=True`'), + status=falcon.HTTP_409, + retry=False + ) diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py index 5ee6646a..8fb3ae12 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_helper.py @@ -720,3 +720,45 @@ class ConfigdocsHelper(object): # reset the revision dict so it regenerates. self.revision_dict = None return self._get_buffer_rev_id() + + def check_intermediate_commit(self): + + # Initialize variable + list_of_committed_rev = [] + + try: + # Get the list of all revisions present in Deckhand + all_revisions = self.deckhand.get_revision_list() + + except NoRevisionsExistError: + # the values of None/None/None/0 are fine + pass + + except DeckhandResponseError as drex: + raise AppError( + title='Unable to retrieve revisions', + description=( + 'Deckhand has responded unexpectedly: {}:{}'.format( + drex.status_code, drex.response_message)), + status=falcon.HTTP_500, + retry=False) + + if all_revisions: + # Get the list of 'committed' revisions + for revision in all_revisions: + if 'committed' in revision['tags']: + list_of_committed_rev.append(revision) + + # This is really applicable for scenarios where multiple + # configdocs commits and site actions were performed. Hence + # we should expect at least 2 'committed' revisions to be + # present in deckhand. + # + # We will check the second last most recent committed revision + # to see if a site-action has been executed on it + if len(list_of_committed_rev) > 1: + if (('site-action-success' and 'site-action-failure') not in + list_of_committed_rev[-2]['tags']): + return True + + return False diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py b/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py index 7c581ecc..266c48c5 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_actions_api.py @@ -39,6 +39,10 @@ CONF = cfg.CONF LOG = logging.getLogger(__name__) +def CHECK_INTERMEDIATE_COMMIT(allow_intermediate_commits): + return False + + def create_req(ctx, body): '''creates a falcon request''' env = testing.create_environ( @@ -261,7 +265,9 @@ def test_on_post(mock_info, mock_create_action, mock_authorize): mock_authorize.assert_called_once_with( 'workflow_orchestrator:create_action', context) mock_create_action.assert_called_once_with( - action=json.loads(json_body.decode('utf-8')), context=context) + action=json.loads(json_body.decode('utf-8')), + context=context, + allow_intermediate_commits=None) mock_info.assert_called_with("Id %s generated for action %s", 'test_id', 'test_name') assert resp.status == '201 Created' @@ -299,6 +305,8 @@ def test_create_action(): action_resource.insert_action = insert_action_stub action_resource.audit_control_command_db = audit_control_command_db action_resource.get_committed_design_version = lambda: DESIGN_VERSION + action_resource.check_intermediate_commit_revision = ( + CHECK_INTERMEDIATE_COMMIT) # with invalid input. fail. try: @@ -307,7 +315,8 @@ def test_create_action(): 'parameters': { 'a': 'aaa' }}, - context=context) + context=context, + allow_intermediate_commits=False) assert False, 'Should throw an ApiError' except ApiError: # expected @@ -320,7 +329,8 @@ def test_create_action(): 'parameters': { 'a': 'aaa' }}, - context=context) + context=context, + allow_intermediate_commits=False) assert action['timestamp'] assert action['id'] assert len(action['id']) == 26 @@ -333,7 +343,9 @@ def test_create_action(): # with valid input and no parameters try: action = action_resource.create_action( - action={'name': 'deploy_site'}, context=context) + action={'name': 'deploy_site'}, + context=context, + allow_intermediate_commits=False) assert action['timestamp'] assert action['id'] assert len(action['id']) == 26 diff --git a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py index 6e6f6e74..7d761ff0 100644 --- a/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py +++ b/src/bin/shipyard_client/shipyard_client/api_client/shipyard_api_client.py @@ -122,20 +122,26 @@ class ShipyardClient(BaseClient): ) return self.get_resp(url) - def post_actions(self, name=None, parameters=None): + def post_actions(self, name=None, parameters=None, + allow_intermediate_commits=False): """ Creates an action in the system. This will cause some action to start. :param str name: name of supported action to invoke :param dict parameters: parameters to use for trigger invocation + :param allow_intermediate_commits: boolean, True|False :returns: action entity created successfully :rtype: Response object """ + query_params = ( + {"allow-intermediate-commits": allow_intermediate_commits}) action_data = {"name": name, "parameters": parameters} url = ApiPaths.POST_GET_ACTIONS.value.format( self.get_endpoint() ) - return self.post_resp( - url, data=json.dumps(action_data), content_type='application/json') + return self.post_resp(url=url, + query_params=query_params, + data=json.dumps(action_data), + content_type='application/json') def get_action_detail(self, action_id=None): """ diff --git a/src/bin/shipyard_client/shipyard_client/cli/create/actions.py b/src/bin/shipyard_client/shipyard_client/cli/create/actions.py index 54f3d1d2..f37f901b 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/create/actions.py +++ b/src/bin/shipyard_client/shipyard_client/cli/create/actions.py @@ -17,19 +17,24 @@ from shipyard_client.cli import format_utils class CreateAction(CliAction): """Action to Create Action""" - def __init__(self, ctx, action_name, param): + def __init__(self, ctx, action_name, param, allow_intermediate_commits): """Sets parameters.""" super().__init__(ctx) - self.logger.debug("CreateAction action initialized with action command" - "%s and parameters %s", action_name, param) + self.logger.debug( + "CreateAction action initialized with action command " + "%s, parameters %s and allow-intermediate-commits=%s", + action_name, param, allow_intermediate_commits) self.action_name = action_name self.param = param + self.allow_intermediate_commits = allow_intermediate_commits def invoke(self): """Returns the response from API Client""" self.logger.debug("Calling API Client post_actions.") - return self.get_api_client().post_actions(name=self.action_name, - parameters=self.param) + return self.get_api_client().post_actions( + name=self.action_name, + parameters=self.param, + allow_intermediate_commits=self.allow_intermediate_commits) # Handle 400, 409 with default error handler for cli. cli_handled_err_resp_codes = [400, 409] diff --git a/src/bin/shipyard_client/shipyard_client/cli/create/commands.py b/src/bin/shipyard_client/shipyard_client/cli/create/commands.py index ee1531b4..46c64aa8 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/create/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/create/commands.py @@ -39,7 +39,7 @@ DESC_ACTION = """ DESCRIPTION: Invokes the specified workflow through Shipyard. Returns the id of the action invoked so that it can be queried subsequently. \n FORMAT: shipyard create action --param= - (repeatable) \n + (repeatable) [--allow-intermediate-commits] \n EXAMPLE: shipyard create action redeploy_server --param="server-name=mcp" """ @@ -55,8 +55,14 @@ SHORT_DESC_ACTION = ( '--param', multiple=True, help="A parameter to be provided to the action being invoked.(Repeatable)") +@click.option( + '--allow-intermediate-commits', + 'allow_intermediate_commits', + flag_value=True, + help="Allow site action to go through even though there are prior commits " + "that have not been used as part of a site action.") @click.pass_context -def create_action(ctx, action_name, param): +def create_action(ctx, action_name, param, allow_intermediate_commits=False): check_action_command(ctx, action_name) if not param and action_name is 'redeploy_server': @@ -65,7 +71,10 @@ def create_action(ctx, action_name, param): else: param = check_reformat_parameter(ctx, param) click.echo( - CreateAction(ctx, action_name, param).invoke_and_return_resp()) + CreateAction(ctx, + action_name, + param, + allow_intermediate_commits).invoke_and_return_resp()) DESC_CONFIGDOCS = """ diff --git a/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py b/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py index 666d68dc..198aea9c 100644 --- a/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py +++ b/src/bin/shipyard_client/tests/unit/cli/create/test_create_actions.py @@ -45,9 +45,11 @@ def test_create_action(*args): 'http://shiptest/actions', body=resp_body, status=201) - response = CreateAction(stubs.StubCliContext(), - action_name='deploy_site', - param=None).invoke_and_return_resp() + response = CreateAction( + stubs.StubCliContext(), + action_name='deploy_site', + param=None, + allow_intermediate_commits=False).invoke_and_return_resp() assert 'Name' in response assert 'Action' in response assert 'Lifecycle' in response @@ -64,9 +66,11 @@ def test_create_action_400(*args): body=stubs.gen_err_resp(message='Error_400', reason='bad action'), status=400) - response = CreateAction(stubs.StubCliContext(), - action_name='deploy_dogs', - param=None).invoke_and_return_resp() + response = CreateAction( + stubs.StubCliContext(), + action_name='deploy_dogs', + param=None, + allow_intermediate_commits=False).invoke_and_return_resp() assert 'Error_400' in response assert 'bad action' in response assert 'action/01BTTMFVDKZFRJM80FGD7J1AKN' not in response @@ -81,9 +85,11 @@ def test_create_action_409(*args): body=stubs.gen_err_resp(message='Error_409', reason='bad validations'), status=409) - response = CreateAction(stubs.StubCliContext(), - action_name='deploy_site', - param=None).invoke_and_return_resp() + response = CreateAction( + stubs.StubCliContext(), + action_name='deploy_site', + param=None, + allow_intermediate_commits=False).invoke_and_return_resp() assert 'Error_409' in response assert 'bad validations' in response assert 'action/01BTTMFVDKZFRJM80FGD7J1AKN' not in response diff --git a/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py b/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py index b8e50828..da6526a1 100644 --- a/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py +++ b/src/bin/shipyard_client/tests/unit/cli/create/test_create_commands.py @@ -35,7 +35,7 @@ def test_create_action(): runner.invoke(shipyard, [auth_vars, 'create', 'action', action_name, param]) mock_method.assert_called_once_with(ANY, action_name, - {'"server-name': 'mcp"'}) + {'"server-name': 'mcp"'}, False) def test_create_action_negative():