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

@@ -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

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>]
+        [--collection=<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>]
+\--collection=<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
+     Comparing Base: committed (Deckhand revision 2)
-    coll1             present         unmodified
+             to New: buffer (Deckhand revision 3)
-    coll2             not present     created
+    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

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
 
 

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
+        :param versions: A list of 2 versions. Defaults to buffer and
-        current committed and buffer statuses
+                         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.
+        # Default ordering
-        # new revision is ok because we just checked for buffer emptiness
+        def_order = [SUCCESSFUL_SITE_ACTION,
-        old_revision_id = self.get_revision_id(COMMITTED) or 0
+                     LAST_SITE_ACTION,
-        new_revision_id = self.get_revision_id(BUFFER) or old_revision_id
+                     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,
+                old_revision_id=old_version_id,
-                new_revision_id=new_revision_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

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',
+        "base_version": 'committed',
-        "buffer_status": 'created'
+        "base_revision": 3,
+        "new_version": 'buffer',
+        "new_revision": 5,
+        "new_status": 'created',
+        "base_status": 'not present'
     }, {
         "collection_name": 'mop',
-        "committed_status": 'present',
+        "base_version": 'committed',
-        "buffer_status": 'unmodified'
+        "base_revision": 3,
+        "new_version": 'buffer',
+        "new_revision": 5,
+        "new_status": 'unmodified',
+        "base_status": 'present'
     }, {
         "collection_name": 'slop',
-        "committed_status": 'present',
+        "base_version": 'committed',
-        "buffer_status": 'deleted'
+        "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'])

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'):
         """

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('base_status'),
-                collection.get('buffer_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):

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'
+    table.align = align
-    return table.get_string()
+
+    # 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)

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]

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
+DESCRIPTION: Retrieve documents loaded into Shipyard, either committed,
-from the Shipyard Buffer.
+last site action, successful site action or from the Shipyard Buffer.
-FORMAT: shipyard get configdocs <collection>
+Allows comparison between 2 revisions using valid revision tags.
+FORMAT: shipyard get configdocs [--collection=<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,
+        _version = get_version(ctx, buffer, committed, last_site_action,
-                              successful_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,
+    _version = get_version(ctx, buffer, committed, last_site_action,
-                          successful_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 = """

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: <parameter name>=<parameter value>")
     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'")

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",
+        "collection_name": "Collection_1",
-         "committed_status": "present",
+        "base_status": "present",
-         "buffer_status": "unmodified"
+        "new_status": "unmodified",
+        "base_version": "committed",
+        "base_revision": 3,
+        "new_version": "buffer",
+        "new_revision": 5
      },
      {
-        "collection_name":"Collection_2",
+        "collection_name": "Collection_2",
-         "committed_status": "present",
+        "base_status": "present",
-         "buffer_status": "modified"
+        "new_status": "modified",
+        "base_version": "committed",
+        "base_revision": 3,
+        "new_version": "buffer",
+        "new_revision": 5
      },
      {
-        "collection_name":"Collection_3",
+        "collection_name": "Collection_3",
-         "committed_status": "not present",
+        "base_status": "not present",
-         "buffer_status": "created"
+        "new_status": "created",
+        "base_version": "committed",
+        "base_revision": 3,
+        "new_version": "buffer",
+        "new_revision": 5
      },
      {
-        "collection_name":"Collection_A",
+        "collection_name": "Collection_A",
-         "committed_status": "present",
+        "base_status": "present",
-         "buffer_status": "deleted"
+        "new_status": "deleted",
+        "base_version": "committed",
+        "base_revision": 3,
+        "new_version": "buffer",
+        "new_revision": 5
      }
 ]
 """, [{
     "collection_name": "Collection_1",
-    "committed_status": "present",
+    "base_status": "present",
-    "buffer_status": "unmodified"
+    "new_status": "unmodified",
+    "base_version": "committed",
+    "base_revision": 3,
+    "new_version": "buffer",
+    "new_revision": 5
 }, {
     "collection_name": "Collection_2",
-    "committed_status": "present",
+    "base_status": "present",
-    "buffer_status": "modified"
+    "new_status": "modified",
+    "base_version": "committed",
+    "base_revision": 3,
+    "new_version": "buffer",
+    "new_revision": 5
 }, {
     "collection_name": "Collection_3",
-    "committed_status": "not present",
+    "base_status": "not present",
-    "buffer_status": "created"
+    "new_status": "created",
+    "base_version": "committed",
+    "base_revision": 3,
+    "new_version": "buffer",
+    "new_revision": 5
 }, {
     "collection_name": "Collection_A",
-    "committed_status": "present",
+    "base_status": "present",
-    "buffer_status": "deleted"
+    "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(

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):