From afc2ea501deda27083b951f14791c5ed6141d8ae Mon Sep 17 00:00:00 2001 From: Anthony Lin Date: Thu, 3 May 2018 09:57:35 +0000 Subject: [PATCH] Update Configdoc Status Allows comparison between Deckhand revisions with valid revision tags, i.e. buffer, committed, last-site-action and successful-site-action. This patch set updates the relevant Shipyard API, API client, CLI as well as document. Change-Id: Ia9a519d82fe8bf80f89945e678a0b02f6ec43baa --- docs/source/API.rst | 11 +- docs/source/CLI.rst | 40 +++++-- .../control/configdocs/configdocs_api.py | 3 +- .../control/configdocs/configdocs_helper.py | 105 +++++++++++++++--- .../unit/control/test_configdocs_helper.py | 24 +++- .../api_client/shipyard_api_client.py | 6 +- .../shipyard_client/cli/cli_format_common.py | 25 ++++- .../shipyard_client/cli/format_utils.py | 13 ++- .../shipyard_client/cli/get/actions.py | 9 +- .../shipyard_client/cli/get/commands.py | 40 ++++--- .../shipyard_client/cli/input_checks.py | 30 +++++ .../tests/unit/cli/get/test_get_actions.py | 72 ++++++++---- .../tests/unit/cli/get/test_get_commands.py | 6 +- 13 files changed, 305 insertions(+), 79 deletions(-) diff --git a/docs/source/API.rst b/docs/source/API.rst index dd650acd..5e697703 100644 --- a/docs/source/API.rst +++ b/docs/source/API.rst @@ -110,12 +110,21 @@ Represents the site configuration documents' current statuses GET /v1.0/configdocs ^^^^^^^^^^^^^^^^^^^^ -Returns a list of collections including their committed and buffer status. +Returns a list of collections including their base and new status. .. note:: The output type for this request is 'Content-Type: application/json' +Query Parameters +'''''''''''''''' +- version=committed,buffer (default) + Indicates which revisions tags to compare. Comparision can only be done + between 2 different revision tags and the default behavior is to compare + the revision with the 'committed' tag and the one with the 'buffer' tag. + Valid revision tags that can be used for comparison using the API include + 'buffer', 'committed', 'last_site_action' and 'successful_site_action'. + Responses ''''''''' 200 OK diff --git a/docs/source/CLI.rst b/docs/source/CLI.rst index 9f0a370c..32bb2e80 100644 --- a/docs/source/CLI.rst +++ b/docs/source/CLI.rst @@ -583,18 +583,30 @@ get configdocs Retrieve documents loaded into Shipyard. The possible options include last committed, last site action, last successful site action and retrieval from the Shipyard Buffer. Site actions include deploy_site and update_site. Note -that we can only select one of the options for each CLI call. +that we can only select one of the options when we retrieve the documents +for a particular collection. + +The command will compare the differences between the revisions specified if +the collection option is not specified. Note that we can only compare between +2 revisions. The relevant Deckhand revision id will be shown in the output as +well. + +If both collection and revisions are not specified, the output will show the +differences between the 'committed' and 'buffer' revision (default behavior). :: shipyard get configdocs - [] + [--collection=] [--committed | --last-site-action | --successful-site-action | --buffer] Example: - shipyard get configdocs design + shipyard get configdocs --collection=design + shipyard get configdocs --collection=design --last-site-action + shipyard get configdocs + shipyard get configdocs --committed --last-site-action -[] +\--collection= The collection to retrieve for viewing. If no collection is entered, the status of the collections in the buffer and those that are committed will be displayed. @@ -621,14 +633,24 @@ Samples :: $ shipyard get configdocs - Collection Committed Buffer - coll1 present unmodified - coll2 not present created - + Comparing Base: committed (Deckhand revision 2) + to New: buffer (Deckhand revision 3) + Collection Base New + coll1 present unmodified + coll2 not present created :: - $ shipyard get configdocs coll1 + $ shipyard get configdocs --committed --last-site-action + Comparing Base: last_site_action (Deckhand revision 2) + to New: committed (Deckhand revision 2) + Collection Base New + secrets present unmodified + design present unmodified + +:: + + $ shipyard get configdocs --collection=coll1 data: chart_groups: [kubernetes-proxy, container-networking, dns, kubernetes, kubernetes-rbac] release_prefix: ucp diff --git a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py index 0e7142e0..214f9b59 100644 --- a/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py +++ b/src/bin/shipyard_airflow/shipyard_airflow/control/configdocs/configdocs_api.py @@ -41,8 +41,9 @@ class ConfigDocsStatusResource(BaseResource): @policy.ApiEnforcer('workflow_orchestrator:get_configdocs_status') def on_get(self, req, resp): """Returns a list of the configdocs and their statuses""" + versions = req.params.get('versions') or None helper = ConfigdocsHelper(req.context) - resp.body = self.to_json(helper.get_configdocs_status()) + resp.body = self.to_json(helper.get_configdocs_status(versions)) resp.status = falcon.HTTP_200 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 4c2fa5df..4f720681 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 @@ -160,22 +160,93 @@ class ConfigdocsHelper(object): self.deckhand.rollback(committed_rev_id) return True - def get_configdocs_status(self): + def get_configdocs_status(self, versions=None): """ - Returns a list of the configdocs, committed or in buffer, and their - current committed and buffer statuses + :param versions: A list of 2 versions. Defaults to buffer and + commmitted if None. + + Returns a list of the configdocs based on their versions and + statuses """ configdocs_status = [] - # If there is no committed revision, then it's 0. - # new revision is ok because we just checked for buffer emptiness - old_revision_id = self.get_revision_id(COMMITTED) or 0 - new_revision_id = self.get_revision_id(BUFFER) or old_revision_id + # Default ordering + def_order = [SUCCESSFUL_SITE_ACTION, + LAST_SITE_ACTION, + COMMITTED, + BUFFER] + + # Defaults to COMMITTED and BUFFER + if versions is None: + versions = [COMMITTED, BUFFER] + + elif not len(versions) == 2: + raise AppError( + title='Incorrect number of versions for comparison', + description=( + 'User must pass in 2 valid versions for comparison'), + status=falcon.HTTP_400, + retry=False) + + elif versions[0] == versions[1]: + raise AppError( + title='Versions must be different for comparison', + description=( + 'Versions must be unique in order to perform comparison'), + status=falcon.HTTP_400, + retry=False) + + for version in versions: + if version not in def_order: + raise AppError( + title='Invalid version detected', + description=( + '{} is not a valid version, which include: ' + '{}'.format(version, ', '.join(def_order))), + status=falcon.HTTP_400, + retry=False) + + # Higher index in the def_order list will mean that it is a newer + # version. We will swap the order and sort the version if need be. + if def_order.index(versions[0]) > def_order.index(versions[1]): + sorted_versions = list(reversed(versions)) + else: + sorted_versions = versions + + # Get version id + old_version_id = self.get_revision_id(sorted_versions[0]) + new_version_id = self.get_revision_id(sorted_versions[1]) + + # Get revision name + old_version_name = sorted_versions[0] + new_version_name = sorted_versions[1] + + # Check that revision id of LAST_SITE_ACTION and SUCCESSFUL_SITE_ACTION + # is not None + for name, rev_id in [(old_version_name, old_version_id), + (new_version_name, new_version_id)]: + if (name in [LAST_SITE_ACTION, SUCCESSFUL_SITE_ACTION] and + rev_id is None): + raise AppError( + title='Version does not exist', + description='{} version does not exist'.format(name), + status=falcon.HTTP_404, + retry=False) + + # Set to 0 if there is no committed version + if old_version_name == COMMITTED and old_version_id is None: + old_version_id = 0 + new_version_id = self.get_revision_id(BUFFER) or 0 + + # Set new_version_id if None + if new_version_id is None: + new_version_id = ( + self.get_revision_id(BUFFER) or old_version_id or 0) try: diff = self.deckhand.get_diff( - old_revision_id=old_revision_id, - new_revision_id=new_revision_id) + old_revision_id=old_version_id, + new_revision_id=new_version_id) except DeckhandResponseError as drex: raise AppError( @@ -188,13 +259,20 @@ class ConfigdocsHelper(object): for collection_id in diff: collection = {"collection_name": collection_id} + if diff[collection_id] in [ "unmodified", "modified", "created", "deleted"]: - collection['buffer_status'] = diff[collection_id] + + collection['base_version'] = old_version_name + collection['base_revision'] = old_version_id + collection['new_version'] = new_version_name + collection['new_revision'] = new_version_id + collection['new_status'] = diff[collection_id] + if diff[collection_id] == "created": - collection['committed_status'] = 'not present' + collection['base_status'] = 'not present' else: - collection['committed_status'] = 'present' + collection['base_status'] = 'present' else: raise AppError( @@ -205,7 +283,8 @@ class ConfigdocsHelper(object): ' only valid collection statuses.', collection_id), status=falcon.HTTP_500, - retry=False, ) + retry=False) + configdocs_status.append(collection) return configdocs_status diff --git a/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py b/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py index 5a85f089..d590b87f 100644 --- a/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py +++ b/src/bin/shipyard_airflow/tests/unit/control/test_configdocs_helper.py @@ -303,16 +303,28 @@ def test_get_configdocs_status(): expected = [{ "collection_name": 'chum', - "committed_status": 'not present', - "buffer_status": 'created' + "base_version": 'committed', + "base_revision": 3, + "new_version": 'buffer', + "new_revision": 5, + "new_status": 'created', + "base_status": 'not present' }, { "collection_name": 'mop', - "committed_status": 'present', - "buffer_status": 'unmodified' + "base_version": 'committed', + "base_revision": 3, + "new_version": 'buffer', + "new_revision": 5, + "new_status": 'unmodified', + "base_status": 'present' }, { "collection_name": 'slop', - "committed_status": 'present', - "buffer_status": 'deleted' + "base_version": 'committed', + "base_revision": 3, + "new_version": 'buffer', + "new_revision": 5, + "new_status": 'deleted', + "base_status": 'present' }] assert expected == sorted(result, key=lambda x: x['collection_name']) 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 9e2ab411..744ad42c 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 @@ -81,13 +81,15 @@ class ShipyardClient(BaseClient): collection_id) return self.get_resp(url, query_params) - def get_configdocs_status(self): + def get_configdocs_status(self, versions=None): """ Get the status of the collection of documents from shipyard :returns: Response object + :param versions: List of versions to compare """ + query_params = {"versions": versions} url = ApiPaths.GET_CONFIGDOCS.value.format(self.get_endpoint()) - return self.get_resp(url) + return self.get_resp(url, query_params) def get_rendereddocs(self, version='buffer'): """ diff --git a/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py b/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py index f79cd19f..126f16a0 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py +++ b/src/bin/shipyard_client/shipyard_client/cli/cli_format_common.py @@ -140,22 +140,37 @@ def gen_collection_table(collection_list): """Generates a list of collections and their status Assumes collection_list is a list of dictionares with 'collection_name', - 'committed_status', and 'buffer_status' + 'base_status', 'new_status', 'base_version', 'base_revision', 'new_version' + and 'new_revision'. """ collections = format_utils.table_factory( - field_names=['Collection', 'Committed', 'Buffer']) + field_names=['Collection', + 'Base', + 'New']) if collection_list: for collection in collection_list: collections.add_row([ collection.get('collection_name'), - collection.get('committed_status'), - collection.get('buffer_status') + collection.get('base_status'), + collection.get('new_status') ]) + + collections_title = ( + 'Comparing Base: {} (Deckhand revision {}) \n\t to New: {} ' + '(Deckhand revision {})'.format( + collection.get('base_version'), + collection.get('base_revision'), + collection.get('new_version'), + collection.get('new_revision'))) + else: collections.add_row(['None', '', '']) + collections_title = '' - return format_utils.table_get_string(collections) + return format_utils.table_get_string(table=collections, + title=collections_title, + vertical_char=' ') def gen_workflow_table(workflow_list): diff --git a/src/bin/shipyard_client/shipyard_client/cli/format_utils.py b/src/bin/shipyard_client/shipyard_client/cli/format_utils.py index 47bb093f..01feadf4 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/format_utils.py +++ b/src/bin/shipyard_client/shipyard_client/cli/format_utils.py @@ -218,12 +218,19 @@ def table_factory(field_names=None, rows=None, style=None): if rows: for row in rows: p.add_row(row) + # This alignment only works if columns and rows are set up. + # Left alignment is used by default p.align = 'l' + return p -def table_get_string(table, align='l'): +def table_get_string(table, title='', vertical_char='|', align='l'): """Wrapper to return a prettytable string with the supplied alignment""" - table.align = 'l' - return table.get_string() + table.align = align + + # Title is an empty string by default + # vertical_char - Single character string used to draw vertical + # lines. Default is '|'. + return table.get_string(title=title, vertical_char=vertical_char) diff --git a/src/bin/shipyard_client/shipyard_client/cli/get/actions.py b/src/bin/shipyard_client/shipyard_client/cli/get/actions.py index 53702eb3..0c600c5a 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/get/actions.py +++ b/src/bin/shipyard_client/shipyard_client/cli/get/actions.py @@ -83,15 +83,18 @@ class GetConfigdocs(CliAction): class GetConfigdocsStatus(CliAction): """Action to get the configdocs status""" - def __init__(self, ctx): + def __init__(self, ctx, version=None): """Sets parameters.""" super().__init__(ctx) - self.logger.debug("GetConfigdocsStatus action initialized") + self.logger.debug("GetConfigdocsStatus action with version %s " + "initialized", version) + self.version = version def invoke(self): """Calls API Client and formats response from API Client""" self.logger.debug("Calling API Client get_configdocs_status") - return self.get_api_client().get_configdocs_status() + + return self.get_api_client().get_configdocs_status(self.version) # Handle 404 with default error handler for cli. cli_handled_err_resp_codes = [404] diff --git a/src/bin/shipyard_client/shipyard_client/cli/get/commands.py b/src/bin/shipyard_client/shipyard_client/cli/get/commands.py index 2f8b8973..4ec183ef 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/get/commands.py +++ b/src/bin/shipyard_client/shipyard_client/cli/get/commands.py @@ -21,6 +21,7 @@ from shipyard_client.cli.get.actions import GetConfigdocs from shipyard_client.cli.get.actions import GetConfigdocsStatus from shipyard_client.cli.get.actions import GetRenderedConfigdocs from shipyard_client.cli.get.actions import GetWorkflows +from shipyard_client.cli.input_checks import check_reformat_versions @click.group() @@ -53,20 +54,25 @@ def get_actions(ctx): DESC_CONFIGDOCS = """ COMMAND: configdocs -DESCRIPTION: Retrieve documents loaded into Shipyard, either committed or -from the Shipyard Buffer. -FORMAT: shipyard get configdocs +DESCRIPTION: Retrieve documents loaded into Shipyard, either committed, +last site action, successful site action or from the Shipyard Buffer. +Allows comparison between 2 revisions using valid revision tags. +FORMAT: shipyard get configdocs [--collection=] [--committed | --buffer | --last-site-action | --successful-site-action] -EXAMPLE: shipyard get configdocs design +EXAMPLE: shipyard get configdocs --colllection=design """ SHORT_DESC_CONFIGDOCS = ("Retrieve documents loaded into Shipyard, either " - "committed or from the Shipyard Buffer.") + "committed, last site action, successful site action " + "or from the Shipyard Buffer. Allows comparison " + "between 2 revisions using valid revision tags") @get.command( name='configdocs', help=DESC_CONFIGDOCS, short_help=SHORT_DESC_CONFIGDOCS) -@click.argument('collection', required=False) +@click.option( + '--collection', + help='A collection of document YAMLs') @click.option( '--committed', '-c', @@ -96,14 +102,22 @@ def get_configdocs(ctx, collection, buffer, committed, last_site_action, successful_site_action): if collection: # Get version - version = get_version(ctx, buffer, committed, last_site_action, - successful_site_action) + _version = get_version(ctx, buffer, committed, last_site_action, + successful_site_action) click.echo( - GetConfigdocs(ctx, collection, version).invoke_and_return_resp()) + GetConfigdocs(ctx, collection, _version).invoke_and_return_resp()) else: - click.echo(GetConfigdocsStatus(ctx).invoke_and_return_resp()) + compare_versions = check_reformat_versions(ctx, + buffer, + committed, + last_site_action, + successful_site_action) + + click.echo( + GetConfigdocsStatus(ctx, + compare_versions).invoke_and_return_resp()) DESC_RENDEREDCONFIGDOCS = """ @@ -153,10 +167,10 @@ SHORT_DESC_RENDEREDCONFIGDOCS = ( def get_renderedconfigdocs(ctx, buffer, committed, last_site_action, successful_site_action): # Get version - version = get_version(ctx, buffer, committed, last_site_action, - successful_site_action) + _version = get_version(ctx, buffer, committed, last_site_action, + successful_site_action) - click.echo(GetRenderedConfigdocs(ctx, version).invoke_and_return_resp()) + click.echo(GetRenderedConfigdocs(ctx, _version).invoke_and_return_resp()) DESC_WORKFLOWS = """ diff --git a/src/bin/shipyard_client/shipyard_client/cli/input_checks.py b/src/bin/shipyard_client/shipyard_client/cli/input_checks.py index 32f289e2..45c0d2da 100644 --- a/src/bin/shipyard_client/shipyard_client/cli/input_checks.py +++ b/src/bin/shipyard_client/shipyard_client/cli/input_checks.py @@ -72,3 +72,33 @@ def check_reformat_parameter(ctx, param): "Invalid parameter or parameter format for " + p + ". Please utilize the format: =") return param_dictionary + + +def check_reformat_versions(ctx, buffer, committed, last_site_action, + successful_site_action): + """Checks and reformat version""" + versions = [] + + if buffer: + versions.append('buffer') + if committed: + versions.append('committed') + if last_site_action: + versions.append('last_site_action') + if successful_site_action: + versions.append('successful_site_action') + + if len(versions) == 0: + return None + + elif len(versions) == 2: + return versions + + else: + ctx.fail( + "Invalid input. User must either\n" + "1. Pass in 0 versions, in which case --buffer and --committed " + "versions are assumed\n" + "2. Pass in 2 valid versions for comparison\n\n" + "Valid versions are '--buffer', '--committed', " + "'--last-site-action' and '--successful-site-action'") diff --git a/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py b/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py index d3b278e1..18af9beb 100644 --- a/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py +++ b/src/bin/shipyard_client/tests/unit/cli/get/test_get_actions.py @@ -143,42 +143,74 @@ def test_get_configdocs_not_found(*args): @pytest.mark.parametrize("test_input, expected", [(""" [ { - "collection_name":"Collection_1", - "committed_status": "present", - "buffer_status": "unmodified" + "collection_name": "Collection_1", + "base_status": "present", + "new_status": "unmodified", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 }, { - "collection_name":"Collection_2", - "committed_status": "present", - "buffer_status": "modified" + "collection_name": "Collection_2", + "base_status": "present", + "new_status": "modified", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 }, { - "collection_name":"Collection_3", - "committed_status": "not present", - "buffer_status": "created" + "collection_name": "Collection_3", + "base_status": "not present", + "new_status": "created", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 }, { - "collection_name":"Collection_A", - "committed_status": "present", - "buffer_status": "deleted" + "collection_name": "Collection_A", + "base_status": "present", + "new_status": "deleted", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 } ] """, [{ "collection_name": "Collection_1", - "committed_status": "present", - "buffer_status": "unmodified" + "base_status": "present", + "new_status": "unmodified", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 }, { "collection_name": "Collection_2", - "committed_status": "present", - "buffer_status": "modified" + "base_status": "present", + "new_status": "modified", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 }, { "collection_name": "Collection_3", - "committed_status": "not present", - "buffer_status": "created" + "base_status": "not present", + "new_status": "created", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 }, { "collection_name": "Collection_A", - "committed_status": "present", - "buffer_status": "deleted" + "base_status": "present", + "new_status": "deleted", + "base_version": "committed", + "base_revision": 3, + "new_version": "buffer", + "new_revision": 5 }])]) def test_get_configdocs_status(test_input, expected, *args): responses.add( diff --git a/src/bin/shipyard_client/tests/unit/cli/get/test_get_commands.py b/src/bin/shipyard_client/tests/unit/cli/get/test_get_commands.py index a6d97fd9..05609290 100644 --- a/src/bin/shipyard_client/tests/unit/cli/get/test_get_commands.py +++ b/src/bin/shipyard_client/tests/unit/cli/get/test_get_commands.py @@ -53,11 +53,11 @@ def test_get_actions_negative(*args): def test_get_configdocs_with_passing_collection(*args): """test get_configdocs""" - collection = 'design' runner = CliRunner() # verify GetConfigdocs is called when a collection is entered with patch.object(GetConfigdocs, '__init__') as mock_method: - runner.invoke(shipyard, [auth_vars, 'get', 'configdocs', collection]) + runner.invoke(shipyard, [auth_vars, 'get', 'configdocs', + '--collection=design']) mock_method.assert_called_once_with(ANY, 'design', 'buffer') @@ -66,7 +66,7 @@ def test_get_configdocs_without_passing_collection(*args): runner = CliRunner() with patch.object(GetConfigdocsStatus, '__init__') as mock_method: runner.invoke(shipyard, [auth_vars, 'get', 'configdocs']) - mock_method.assert_called_once_with(ANY) + mock_method.assert_called_once_with(ANY, None) def test_get_configdocs_negative(*args):