Specify collection for upload command

Currently, using the upload command in Pegleg will upload all discovered
collections to Shipyard by repo. Uploading multiple of these repos can
result in 409 errors during uplift scenarios. This change compiles all
documents into a single collection document that can then be uploaded to
Shipyard.

Requires a collection name to be specified that will be used as the
'collection_id' for uploading to Shipyard.

Buffer mode is set by default to 'replace' instead of 'auto'.

Change-Id: I546b03fd82873296fff10aba355a50e4b11352d0

doc/source/cli/cli.rst

@@ -427,13 +427,18 @@ collection does not already exist in the Shipyard buffer.
 replace: Clear the Shipyard Buffer before adding the specified
 collection.
 
-auto: Let Pegleg determine the appropriate buffer mode to use.
+**--collection** (Required, Default=<site_name>).
+
+Specifies the name of the compiled collection of documents that will be
+uploaded to Shipyard.
 
 Usage:
 
 ::
 
-    ./pegleg.sh site <options> upload <site_name> --context-marker=<uuid> --buffer=<buffer>
+    ./pegleg.sh site <options> upload <site_name> --context-marker=<uuid> \
+                                                  --buffer-mode=<buffer> \
+                                                  --collection=<collection>
 
 Site Secrets Group
 ------------------

pegleg/cli.py

@@ -346,6 +346,13 @@ def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name):
         warn_lint=warn_lint)
 
 
+def collection_default_callback(ctx, param, value):
+    LOG.debug('Evaluating %s: %s', param.name, value)
+    if not value:
+        return ctx.params['site_name']
+    return value
+
+
 @site.command('upload', help='Upload documents to Shipyard')
 # Keystone authentication parameters
 @click.option('--os-project-domain-name',
@@ -374,20 +381,26 @@ def lint_site(*, fail_on_missing_sub_src, exclude_lint, warn_lint, site_name):
     '--buffer-mode',
     'buffer_mode',
     required=False,
-    default='auto',
+    default='replace',
     show_default=True,
+    type=click.Choice(['append', 'replace']),
     help='Set the buffer mode when uploading documents. Supported buffer '
          'modes include append, replace, auto.\n'
          'append: Add the collection to the Shipyard Buffer, only if that '
          'collection does not already exist in the Shipyard buffer.\n'
          'replace: Clear the Shipyard Buffer before adding the specified '
-         'collection.\n'
+         'collection.\n')
-         'auto: Let Pegleg determine the appropriate buffer mode to use.')
+@click.option(
+    '--collection',
+    'collection',
+    help='Specifies the name to use for the uploaded collection. '
+         'Defaults to the specified `site_name`.',
+    callback=collection_default_callback)
 @SITE_REPOSITORY_ARGUMENT
 @click.pass_context
-def upload(ctx, *, os_project_domain_name,
+def upload(ctx, *, os_project_domain_name, os_user_domain_name,
-           os_user_domain_name, os_project_name, os_username,
+           os_project_name, os_username, os_password, os_auth_url,
-           os_password, os_auth_url, context_marker, site_name, buffer_mode):
+           context_marker, site_name, buffer_mode, collection):
     if not ctx.obj:
         ctx.obj = {}
 
@@ -406,6 +419,7 @@ def upload(ctx, *, os_project_domain_name,
     }
     ctx.obj['context_marker'] = str(context_marker)
     ctx.obj['site_name'] = site_name
+    ctx.obj['collection'] = collection
 
     click.echo(ShipyardHelper(ctx, buffer_mode).upload_documents())
 

pegleg/engine/util/shipyard_helper.py

@@ -52,7 +52,7 @@ class ShipyardHelper(object):
     4. Formats response from Shipyard api_client
     """
 
-    def __init__(self, context, buffer_mode='auto'):
+    def __init__(self, context, buffer_mode='replace'):
         """
         Initializes params to be used by Shipyard
 
@@ -72,80 +72,69 @@ class ShipyardHelper(object):
             self.auth_vars, self.context_marker)
         self.api_client = ShipyardClient(self.client_context)
         self.buffer_mode = buffer_mode
+        self.collection = self.ctx.obj.get('collection', self.site_name)
 
     def upload_documents(self):
-        """Uploads documents to Shipyard """
+        """Uploads documents to Shipyard"""
 
         collected_documents = files.collect_files_by_repo(self.site_name)
 
-        LOG.info("Uploading %d collection(s) ", len(collected_documents))
+        collection_data = []
+        LOG.info("Processing %d collection(s)", len(collected_documents))
         for idx, document in enumerate(collected_documents):
-            # Append flag is not required for the first
-            # collection being uploaded to Shipyard. It
-            # is needed for subsequent collections.
-            if self.buffer_mode == 'auto':
-                if idx == 0:
-                    buffer_mode = None
-                else:
-                    buffer_mode = 'append'
-            elif self.buffer_mode == 'append' or self.buffer_mode == 'replace':
-                buffer_mode = self.buffer_mode
-            else:
-                raise exceptions.InvalidBufferModeException()
-
             # Decrypt the documents if encrypted
             pegleg_secret_mgmt = PeglegSecretManagement(
                 docs=collected_documents[document])
             decrypted_documents = pegleg_secret_mgmt.get_decrypted_secrets()
-            data = yaml.safe_dump_all(decrypted_documents)
+            collection_data.extend(decrypted_documents)
+        collection_as_yaml = yaml.dump_all(collection_data,
+                                           Dumper=yaml.SafeDumper)
 
-            try:
+        # Append flag is not required for the first
-                self.validate_auth_vars()
+        # collection being uploaded to Shipyard. It
-                # Get current buffer status.
+        # is needed for subsequent collections.
-                response = self.api_client.get_configdocs_status()
+        if self.buffer_mode in ['append', 'replace']:
-                buff_stat = response.json()
+            buffer_mode = self.buffer_mode
-                # If buffer is empty then proceed with existing buffer value
+        else:
-                # else pass the 'replace' flag.
+            raise exceptions.InvalidBufferModeException()
-                for stat in range(len(buff_stat)):
-                    if (buff_stat[stat]['new_status'] != 'unmodified' and
-                            buffer_mode != 'append'):
-                        buffer_mode = 'replace'
-                resp_text = self.api_client.post_configdocs(
-                    collection_id=document,
-                    buffer_mode=buffer_mode,
-                    document_data=data
-                )
 
-            except AuthValuesError as ave:
+        try:
-                resp_text = "Error: {}".format(ave.diagnostic)
+            self.validate_auth_vars()
-                raise DocumentUploadError(resp_text)
+            resp_text = self.api_client.post_configdocs(
-            except Exception as ex:
+                collection_id=self.collection,
-                resp_text = (
+                buffer_mode=buffer_mode,
-                    "Error: Unable to invoke action due to: {}"
+                document_data=collection_as_yaml
-                    .format(str(ex)))
+            )
-                LOG.debug(resp_text, exc_info=True)
-                raise DocumentUploadError(resp_text)
 
-            # FIXME: Standardize status_code in Deckhand to avoid this
+        except AuthValuesError as ave:
-            # workaround.
+            resp_text = "Error: {}".format(ave.diagnostic)
-            code = 0
+            raise DocumentUploadError(resp_text)
-            if hasattr(resp_text, 'status_code'):
+        except Exception as ex:
-                code = resp_text.status_code
+            resp_text = (
-            elif hasattr(resp_text, 'code'):
+                "Error: Unable to invoke action due to: {}"
-                code = resp_text.code
+                .format(str(ex)))
-            if code >= 400:
+            LOG.debug(resp_text, exc_info=True)
-                if hasattr(resp_text, 'content'):
+            raise DocumentUploadError(resp_text)
-                    raise DocumentUploadError(resp_text.content)
+
-                else:
+        # FIXME: Standardize status_code in Deckhand to avoid this
-                    raise DocumentUploadError(resp_text)
+        # workaround.
+        code = 0
+        if hasattr(resp_text, 'status_code'):
+            code = resp_text.status_code
+        elif hasattr(resp_text, 'code'):
+            code = resp_text.code
+        if code >= 400:
+            if hasattr(resp_text, 'content'):
+                raise DocumentUploadError(resp_text.content)
             else:
-                output = self.formatted_response_handler(resp_text)
+                raise DocumentUploadError(resp_text)
-                LOG.info("Uploaded document in buffer %s ", output)
+        else:
+            output = self.formatted_response_handler(resp_text)
+            LOG.info("Uploaded document in buffer %s ", output)
 
         # Commit in the last iteration of the loop when all the documents
         # have been pushed to Shipyard buffer.
-        if idx == len(collected_documents) - 1:
+        return self.commit_documents()
-            return self.commit_documents()
 
     def commit_documents(self):
         """Commit Shipyard buffer documents """

tests/unit/engine/util/test_shipyard_helper.py

@@ -14,12 +14,9 @@
 
 import os
 
-import json
 import mock
 import pytest
-
+import yaml
-from tests.unit import test_utils
-from mock import ANY
 
 from pegleg.engine import util
 from pegleg.engine.util.pegleg_secret_management import ENV_PASSPHRASE
@@ -28,14 +25,35 @@ from pegleg.engine.util.shipyard_helper import ShipyardHelper
 from pegleg.engine.util.shipyard_helper import ShipyardClient
 
 # Dummy data to be used as collected documents
-DATA = {'test-repo':
+DATA = {
+    'test-repo':
         [{'schema': 'pegleg/SiteDefinition/v1',
-            'metadata': {'schema': 'metadata/Document/v1',
+          'metadata': {'schema': 'metadata/Document/v1',
-                         'layeringDefinition': {'abstract': False,
+                       'layeringDefinition': {'abstract': False,
-                                                'layer': 'site'},
+                                              'layer': 'site'},
-                         'name': 'site-name',
+                       'name': 'site-name',
-                         'storagePolicy': 'cleartext'},
+                       'storagePolicy': 'cleartext'},
-            'data': {'site_type': 'foundry'}}]}
+          'data': {'site_type': 'foundry'}}]}
+
+MULTI_REPO_DATA = {
+    'repo1':
+        [{'schema': 'pegleg/SiteDefinition/v1',
+          'metadata': {'schema': 'metadata/Document/v1',
+                       'layeringDefinition': {'abstract': False,
+                                              'layer': 'site'},
+                       'name': 'site-name',
+                       'storagePolicy': 'cleartext'},
+          'data': {'site_type': 'foundry'}}],
+    'repo2':
+        [{'schema': 'pegleg/SiteDefinition/v1',
+          'metadata': {'schema': 'metadata/Document/v1',
+                       'layeringDefinition': {'abstract': False,
+                                              'layer': 'site'},
+                       'name': 'site-name',
+                       'storagePolicy': 'cleartext'},
+          'data': {'site_type': 'foundry'}}]
+
+}
 
 
 @pytest.fixture(autouse=True)
@@ -51,6 +69,7 @@ class context():
 class FakeResponse():
     code = 404
 
+
 def _get_context():
     ctx = context()
     ctx.obj = {}
@@ -67,8 +86,10 @@ def _get_context():
     }
     ctx.obj['context_marker'] = '88888888-4444-4444-4444-121212121212'
     ctx.obj['site_name'] = 'test-site'
+    ctx.obj['collection'] = 'test-site'
     return ctx
 
+
 def _get_bad_context():
     ctx = context()
     ctx.obj = {}
@@ -85,9 +106,18 @@ def _get_bad_context():
     }
     ctx.obj['context_marker'] = '88888888-4444-4444-4444-121212121212'
     ctx.obj['site_name'] = 'test-site'
+    ctx.obj['collection'] = None
     return ctx
 
 
+def _get_data_as_collection(data):
+    collection = []
+    for repo, documents in data.items():
+        for document in documents:
+            collection.append(document)
+    return yaml.dump_all(collection, Dumper=yaml.SafeDumper)
+
+
 def test_shipyard_helper_init_():
     """ Tests ShipyardHelper init method """
     # Scenario:
@@ -102,8 +132,9 @@ def test_shipyard_helper_init_():
     assert shipyard_helper.site_name == context.obj['site_name']
     assert isinstance(shipyard_helper.api_client, ShipyardClient)
 
+
 @mock.patch('pegleg.engine.util.files.collect_files_by_repo', autospec=True,
-            return_value=DATA)
+            return_value=MULTI_REPO_DATA)
 @mock.patch.object(ShipyardHelper, 'formatted_response_handler',
                    autospec=True, return_value=None)
 @mock.patch.dict(os.environ, {
@@ -119,19 +150,22 @@ def test_upload_documents(*args):
     # 3) Check documents uploaded to Shipyard with correct parameters
 
     context = _get_context()
-    shipyard_helper = ShipyardHelper(context)
     with mock.patch('pegleg.engine.util.shipyard_helper.ShipyardClient',
                     autospec=True) as mock_shipyard:
         mock_api_client = mock_shipyard.return_value
         mock_api_client.post_configdocs.return_value = 'Success'
-        result = ShipyardHelper(context).upload_documents()
+        result = ShipyardHelper(context, 'replace').upload_documents()
 
         # Validate Shipyard call to post configdocs was invoked with correct
         # collection name and buffer mode.
-        mock_api_client.post_configdocs.assert_called_with('test-repo',
+        expected_data = _get_data_as_collection(MULTI_REPO_DATA)
-                                            None, ANY)
+        mock_api_client.post_configdocs.assert_called_with(
+            collection_id='test-site',
+            buffer_mode='replace',
+            document_data=expected_data)
         mock_api_client.post_configdocs.assert_called_once()
 
+
 @mock.patch('pegleg.engine.util.files.collect_files_by_repo', autospec=True,
             return_value=DATA)
 @mock.patch.object(ShipyardHelper, 'formatted_response_handler',
@@ -158,6 +192,7 @@ def test_upload_documents_fail(*args):
         with pytest.raises(util.shipyard_helper.DocumentUploadError):
             ShipyardHelper(context).upload_documents()
 
+
 @mock.patch('pegleg.engine.util.files.collect_files_by_repo', autospec=True,
             return_value=DATA)
 @mock.patch.object(ShipyardHelper, 'formatted_response_handler',
@@ -175,6 +210,7 @@ def test_fail_auth(*args):
     with pytest.raises(util.shipyard_helper.AuthValuesError):
         ShipyardHelper(context).validate_auth_vars()
 
+
 @mock.patch.object(ShipyardHelper, 'formatted_response_handler',
                    autospec=True, return_value=None)
 def test_commit_documents(*args):

tests/unit/test_cli.py

@@ -13,10 +13,8 @@
 # limitations under the License.
 
 import os
-import shutil
 
 from click.testing import CliRunner
-from mock import ANY
 import mock
 import pytest
 import yaml
@@ -375,6 +373,8 @@ class TestSiteCliActions(BaseCLIActionTest):
         repo_path = self.treasuremap_path
         self._validate_render_site_action(repo_path)
 
+    ### Upload tests ###
+
     def test_upload_documents_shipyard_using_local_repo_path(self):
         """Validates ShipyardHelper is called with correct arguments."""
         # Scenario:
@@ -387,11 +387,28 @@ class TestSiteCliActions(BaseCLIActionTest):
         with mock.patch('pegleg.cli.ShipyardHelper') as mock_obj:
             result = self.runner.invoke(cli.site,
                                         ['-r', repo_path, 'upload',
-                                         self.site_name])
+                                         self.site_name, '--collection',
+                                         'collection'])
 
         assert result.exit_code == 0
         mock_obj.assert_called_once()
 
+    def test_upload_collection_callback_default_to_site_name(self):
+        """Validates that collection will default to the given site_name"""
+        # Scenario:
+        #
+        # 1) Mock out ShipyardHelper
+        # 2) Check that ShipyardHelper was called with collection set to
+        #    site_name
+        repo_path = self.treasuremap_path
+
+        with mock.patch('pegleg.cli.ShipyardHelper') as mock_obj:
+            result = self.runner.invoke(cli.site,
+                                        ['-r', repo_path, 'upload',
+                                         self.site_name])
+        assert result.exit_code == 0
+        mock_obj.assert_called_once()
+
 
 class TestGenerateActions(BaseCLIActionTest):
     def test_generate_passphrase(self):