Repository: typesafehub/conductr-cli

Issue:
bndl generates bundle.conf with bug for description
Description should not be what it is below:

```
components {
  cp-zookeeper {
    description = <pyhocon.config_tree.NoneValue object at 0x7f452549aac8>
    file-system-type = "oci-image"
    start-command = []
    endpoints {
      cp-zookeeper-tcp-2181 {
        bind-protocol = "tcp"
        bind-port = 2181
        service-name = "cp-zookeeper-tcp-2181"
      }
      cp-zookeeper-tcp-2888 {
        bind-protocol = "tcp"
        bind-port = 2888
        service-name = "cp-zookeeper-tcp-2888"
      }
      cp-zookeeper-tcp-3888 {
        bind-protocol = "tcp"
        bind-port = 3888
        service-name = "cp-zookeeper-tcp-3888"
      }
    }
    volumes {
      volume-etc-zookeeper-secrets = "/etc/zookeeper/secrets"
      volume-var-lib-zookeeper-data = "/var/lib/zookeeper/data"
      volume-var-lib-zookeeper-log = "/var/lib/zookeeper/log"
    }
  }
  bundle-status {
    description = "Status check for the bundle component"
    file-system-type = "universal"
    start-command = [
      "check"
      "--any-address"
      "$CP_ZOOKEEPER_TCP_2181_HOST?retry-delay=10&retry-count=6"
      "$CP_ZOOKEEPER_TCP_2888_HOST?retry-delay=10&retry-count=6"
      "$CP_ZOOKEEPER_TCP_3888_HOST?retry-delay=10&retry-count=6"
    ]
    endpoints {}
  }
}```

Discussion:

Patch:
diff --git a/conductr_cli/bndl_main.py b/conductr_cli/bndl_main.py
index f1b0c15..0d95348 100644
--- a/conductr_cli/bndl_main.py
+++ b/conductr_cli/bndl_main.py
@@ -53,6 +53,8 @@ class ComponentAction(argparse.Action):
             namespace.start_command_dicts[-1]['component'] = value
         elif namespace.component_action == 'volume':
             namespace.volume_dicts[-1]['component'] = value
+        elif namespace.component_action == 'description':
+            namespace.description_dicts[-1]['component'] = value
 
 
 class EndpointAction(argparse.Action):
@@ -147,6 +149,12 @@ class VolumeAction(argparse.Action):
             namespace.volume_dicts[-1][dict_key] = value
 
 
+class DescriptionAction(argparse.Action):
+    def __call__(self, parser, namespace, value, option_strings):
+        namespace.component_action = 'description'
+        namespace.description_dicts.append({'description': value})
+
+
 def process_args(args):
     log = logging.getLogger(__name__)
 
@@ -173,32 +181,44 @@ def process_args(args):
     if args.start_command_dicts:
         args.start_commands = []
 
-        start_command = type('', (), {})()
-        start_command.start_command = args.start_command_dicts[-1]['start_command']
+        for start_command_dict in args.start_command_dicts:
+            start_command = type('', (), {})()
+            start_command.start_command = start_command_dict['start_command']
 
-        if 'component' in args.start_command_dicts[-1]:
-            start_command.component = args.start_command_dicts[-1]['component']
+            if 'component' in start_command_dict:
+                start_command.component = start_command_dict['component']
 
-        args.start_commands.append(start_command)
+            args.start_commands.append(start_command)
 
     if args.volume_dicts:
         args.volumes = []
 
-        volume = type('', (), {})()
-        parts = args.volume_dicts[-1]['volume'].split('=', 1)
+        for volume_dict in args.volume_dicts:
+            volume = type('', (), {})()
+            parts = volume_dict['volume'].split('=', 1)
 
-        if len(parts) != 2:
-            log.error('bndl: volumes must be specified in the format NAME=MOUNT_POINT. '
-                      'Example: bndl --volume myvol=/data')
-            sys.exit(2)
+            if len(parts) != 2:
+                log.error('bndl: volumes must be specified in the format NAME=MOUNT_POINT. '
+                          'Example: bndl --volume myvol=/data')
+                sys.exit(2)
+
+            volume.name = parts[0]
+            volume.mount_point = parts[1]
+
+            if 'component' in volume_dict:
+                volume.component = volume_dict['component']
 
-        volume.name = parts[0]
-        volume.mount_point = parts[1]
+            args.volumes.append(volume)
 
-        if 'component' in args.volume_dicts[-1]:
-            volume.component = args.volume_dicts[-1]['component']
+    if args.description_dicts:
+        args.descriptions = []
 
-        args.volumes.append(volume)
+        for description_dict in args.description_dicts:
+            description = type('', (), {})()
+            description.description = description_dict['description']
+            if 'component' in description_dict:
+                description.component = description_dict['component']
+            args.descriptions.append(description)
 
 
 def add_conf_arguments(parser):
@@ -216,22 +236,27 @@ def add_conf_arguments(parser):
     parser.add_argument('--component',
                         help='Specify the component that should be modified\n'
                              'Required when the bundle has more than one component\n'
-                             'Used in conjunction with the following: --endpoint, --start-command',
+                             'Used in conjunction with the following: '
+                             '--description, --endpoint, --start-command, --volume',
                         metavar='COMPONENT',
                         action=ComponentAction)
 
-    parser.add_argument('--component-description',
-                        required=False,
-                        help='Description to use for the generated ConductR component\n'
-                             'For use with docker and oci-image formats',
-                        dest='component_description')
-
     parser.add_argument('--compatibility-version',
                         nargs='?',
                         required=False,
                         help='Sets the "compatibilityVersion" bundle.conf value',
                         dest='compatibility_version')
 
+    parser.add_argument('--description',
+                        required=False,
+                        help='Sets "description" for a component\n'
+                             'If the bundle has more than one component, you must specify --component\n'
+                             'Example: bndl --description "My service" --component service',
+                        metavar='DESCRIPTION',
+                        dest='description_dicts',
+                        default=[],
+                        action=DescriptionAction)
+
     parser.add_argument('--disk-space',
                         nargs='?',
                         required=False,
diff --git a/conductr_cli/bndl_oci.py b/conductr_cli/bndl_oci.py
index e165750..fa2d407 100644
--- a/conductr_cli/bndl_oci.py
+++ b/conductr_cli/bndl_oci.py
@@ -1,5 +1,5 @@
 from pyhocon import HOCONConverter, ConfigFactory, ConfigTree
-from conductr_cli.bndl_utils import load_bundle_args_into_conf, create_check_hocon
+from conductr_cli.bndl_utils import create_check_hocon
 from conductr_cli.constants import BNDL_DEFAULT_CHECK_RETRY_COUNT, BNDL_DEFAULT_CHECK_RETRY_DELAY
 import json
 import os
@@ -9,10 +9,7 @@ import tempfile
 
 
 def oci_image_bundle_conf(args, component_name, oci_manifest, oci_config):
-    conf = ConfigFactory.parse_string('')
-    load_bundle_args_into_conf(conf, args, args.with_defaults)
-
-    annotations_tree = conf.get('annotations')
+    annotations_tree = ConfigTree()
 
     if 'annotations' in oci_manifest and oci_manifest['annotations'] is not None:
         for key in sorted(oci_manifest['annotations']):
@@ -23,7 +20,6 @@ def oci_image_bundle_conf(args, component_name, oci_manifest, oci_config):
     endpoints_tree = ConfigTree()
 
     oci_tree = ConfigTree()
-    oci_tree.put('description', args.component_description)
     oci_tree.put('file-system-type', 'oci-image')
     oci_tree.put('start-command', [])
     oci_tree.put('endpoints', endpoints_tree)
@@ -69,6 +65,8 @@ def oci_image_bundle_conf(args, component_name, oci_manifest, oci_config):
             volumes.put(key, vol_path)
         oci_tree.put('volumes', volumes)
 
+    conf = ConfigFactory.parse_string('')
+    conf.put('annotations', annotations_tree)
     conf.put('components', components_tree)
 
     return HOCONConverter.to_hocon(conf)
diff --git a/conductr_cli/bndl_utils.py b/conductr_cli/bndl_utils.py
index d2be060..1d05942 100644
--- a/conductr_cli/bndl_utils.py
+++ b/conductr_cli/bndl_utils.py
@@ -174,13 +174,11 @@ def escape_bash_double_quotes(input):
 
 
 def load_bundle_args_into_conf(config, args, application_type):
-    # this is unrolled because it's actually pretty complicated to get the order
-    # correct given that some attributes need special handling and defaults
-
     config_defaults = application_type.config_defaults(args.format.to_file_system_type()) if application_type else None
 
     args_check_addresses = getattr(args, 'check_addresses', None)
     args_compatibility_version = getattr(args, 'compatibility_version', None)
+    args_descriptions = getattr(args, 'descriptions', None)
     args_disk_space = getattr(args, 'disk_space', None)
     args_endpoints = getattr(args, 'endpoints', None)
     args_memory = getattr(args, 'memory', None)
@@ -247,6 +245,15 @@ def load_bundle_args_into_conf(config, args, application_type):
         config.put('compatibilityVersion', config_defaults['compatibilityVersion'])
 
     # Component properties
+    if args_descriptions:
+        if 'components' not in config:
+            config.put('components', ConfigTree())
+
+        for description in args_descriptions:
+            component_name = detect_component(config, description, config_name)
+            description_key = 'components.{}.description'.format(component_name)
+            config.put(description_key, description.description)
+
     if args_endpoints is not None:
         if 'components' not in config:
             config.put('components', ConfigTree())


Test patch:
diff --git a/conductr_cli/test/test_bndl_create.py b/conductr_cli/test/test_bndl_create.py
index 7a89b0b..a5bfb9c 100644
--- a/conductr_cli/test/test_bndl_create.py
+++ b/conductr_cli/test/test_bndl_create.py
@@ -106,7 +106,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile,
-                'component_description': '',
                 'use_shazar': True,
                 'use_default_endpoints': True,
                 'annotations': [],
@@ -149,7 +148,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile,
-                'component_description': '',
                 'use_shazar': False,
                 'use_default_endpoints': True,
                 'annotations': [],
@@ -195,7 +193,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile,
-                'component_description': '',
                 'use_shazar': True,
                 'use_default_endpoints': True,
                 'annotations': [],
@@ -218,7 +215,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile2,
-                'component_description': '',
                 'use_shazar': True,
                 'use_default_endpoints': True,
                 'annotations': [],
@@ -263,7 +259,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile,
-                'component_description': '',
                 'use_shazar': True,
                 'use_default_endpoints': True,
                 'annotations': [],
@@ -308,7 +303,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile,
-                'component_description': '',
                 'use_shazar': False,
                 'use_default_endpoints': True,
                 'annotations': [],
@@ -331,7 +325,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile2,
-                'component_description': '',
                 'use_shazar': False,
                 'use_default_endpoints': True,
                 'annotations': [],
@@ -530,6 +523,17 @@ class TestBndlCreate(CliTestCase):
                         'name': 'my-vol',
                         'mount_point': '/other-data',
                         'component': 'test1'
+                    }),
+                    create_attributes_object({
+                        'name': 'my-vol2',
+                        'mount_point': '/data',
+                        'component': 'test1'
+                    })
+                ],
+                'descriptions': [
+                    create_attributes_object({
+                        'description': 'this is a test',
+                        'component': 'test2'
                     })
                 ],
                 'with_defaults': None
@@ -558,6 +562,7 @@ class TestBndlCreate(CliTestCase):
                                |    ]
                                |    volumes {
                                |      my-vol = "/other-data"
+                               |      my-vol2 = "/data"
                                |    }
                                |  }
                                |  test2 {
@@ -565,6 +570,7 @@ class TestBndlCreate(CliTestCase):
                                |      "abc"
                                |      "test"
                                |    ]
+                               |    description = "this is a test"
                                |    volumes {
                                |      my-vol = "/data"
                                |    }
@@ -899,7 +905,6 @@ class TestBndlCreate(CliTestCase):
                 'format': BndlFormat.OCI_IMAGE,
                 'image_tag': 'latest',
                 'output': tmpfile,
-                'component_description': '',
                 'use_shazar': True,
                 'use_default_endpoints': True,
                 'use_default_volumes': True,
diff --git a/conductr_cli/test/test_bndl_main.py b/conductr_cli/test/test_bndl_main.py
index 48e968e..1ca6fb2 100644
--- a/conductr_cli/test/test_bndl_main.py
+++ b/conductr_cli/test/test_bndl_main.py
@@ -31,8 +31,6 @@ class TestBndl(CliTestCase):
             '-o',
             '/dev/null',
             '--no-shazar',
-            '--component-description',
-            'some description',
             '--version',
             '4',
             '--compatibility-version',
@@ -81,7 +79,13 @@ class TestBndl(CliTestCase):
             '--component',
             'web-component',
             '--volume',
-            'test2:/data2'
+            'test2:/data2',
+            '--description',
+            'this is a test',
+            '--description',
+            'another test',
+            '--component',
+            'web-component'
         ])
 
         self.assertEqual(args.source, 'oci-image-dir')
@@ -91,7 +95,6 @@ class TestBndl(CliTestCase):
         self.assertEqual(args.image_name, 'test')
         self.assertEqual(args.output, '/dev/null')
         self.assertFalse(args.use_shazar)
-        self.assertEqual(args.component_description, 'some description')
         self.assertEqual(args.version, '4')
         self.assertEqual(args.compatibility_version, '5')
         self.assertEqual(args.system, 'myapp')
@@ -126,6 +129,10 @@ class TestBndl(CliTestCase):
             {'component': 'web-component', 'volume': 'test:/data'},
             {'volume': 'test2:/data2'}
         ])
+        self.assertEqual(args.description_dicts, [
+            {'description': 'this is a test'},
+            {'description': 'another test', 'component': 'web-component'}
+        ])
 
     def test_parser_acl_params(self):
         parser = bndl_main.build_parser()
diff --git a/conductr_cli/test/test_bndl_oci.py b/conductr_cli/test/test_bndl_oci.py
index 05b6699..aa9ba6e 100644
--- a/conductr_cli/test/test_bndl_oci.py
+++ b/conductr_cli/test/test_bndl_oci.py
@@ -108,7 +108,6 @@ class TestBndlOci(CliTestCase):
             'format': BndlFormat.OCI_IMAGE,
             'name': 'world',
             'tags': [],
-            'component_description': 'testing desc 1',
             'image_tag': 'testing',
             'use_default_endpoints': True,
             'use_default_volumes': True,
@@ -121,7 +120,6 @@ class TestBndlOci(CliTestCase):
             'name': 'world',
             'tags': [],
             'annotations': [],
-            'component_description': 'testing desc 2',
             'version': '4',
             'compatibility_version': '5',
             'system': 'myapp',
@@ -149,23 +147,8 @@ class TestBndlOci(CliTestCase):
                             |    }
                             |  }
                             |}
-                            |compatibilityVersion = "0"
-                            |diskSpace = 1073741824
-                            |memory = 402653184
-                            |name = "world"
-                            |nrOfCpus = 0.1
-                            |roles = [
-                            |  "web"
-                            |]
-                            |system = "world"
-                            |systemVersion = "0"
-                            |tags = [
-                            |  "testing"
-                            |]
-                            |version = "1"
                             |components {
                             |  my-component {
-                            |    description = "testing desc 1"
                             |    file-system-type = "oci-image"
                             |    start-command = []
                             |    endpoints {}
@@ -188,24 +171,8 @@ class TestBndlOci(CliTestCase):
                             |    }
                             |  }
                             |}
-                            |compatibilityVersion = "5"
-                            |diskSpace = "16384"
-                            |memory = "65536"
-                            |name = "world"
-                            |nrOfCpus = "8"
-                            |roles = [
-                            |  "web"
-                            |  "backend"
-                            |]
-                            |system = "myapp"
-                            |systemVersion = "3"
-                            |tags = [
-                            |  "0.0.1"
-                            |]
-                            |version = "4"
                             |components {
                             |  my-other-component {
-                            |    description = "testing desc 2"
                             |    file-system-type = "oci-image"
                             |    start-command = []
                             |    endpoints {}
@@ -217,7 +184,6 @@ class TestBndlOci(CliTestCase):
         base_args = create_attributes_object({
             'format': BndlFormat.OCI_IMAGE,
             'name': 'world',
-            'component_description': 'testing desc 1',
             'image_tag': 'testing',
             'use_default_endpoints': True,
             'use_default_check': True,
@@ -245,23 +211,8 @@ class TestBndlOci(CliTestCase):
                             |    }
                             |  }
                             |}
-                            |compatibilityVersion = "0"
-                            |diskSpace = 1073741824
-                            |memory = 402653184
-                            |name = "world"
-                            |nrOfCpus = 0.1
-                            |roles = [
-                            |  "web"
-                            |]
-                            |system = "world"
-                            |systemVersion = "0"
-                            |tags = [
-                            |  "testing"
-                            |]
-                            |version = "1"
                             |components {
                             |  my-component {
-                            |    description = "testing desc 1"
                             |    file-system-type = "oci-image"
                             |    start-command = []
                             |    endpoints {
@@ -289,7 +240,6 @@ class TestBndlOci(CliTestCase):
         base_args = create_attributes_object({
             'format': BndlFormat.OCI_IMAGE,
             'name': 'world',
-            'component_description': 'testing desc 1',
             'image_tag': 'testing',
             'use_default_endpoints': False,
             'use_default_volumes': True,
@@ -316,23 +266,8 @@ class TestBndlOci(CliTestCase):
                             |    }
                             |  }
                             |}
-                            |compatibilityVersion = "0"
-                            |diskSpace = 1073741824
-                            |memory = 402653184
-                            |name = "world"
-                            |nrOfCpus = 0.1
-                            |roles = [
-                            |  "web"
-                            |]
-                            |system = "world"
-                            |systemVersion = "0"
-                            |tags = [
-                            |  "testing"
-                            |]
-                            |version = "1"
                             |components {
                             |  my-component {
-                            |    description = "testing desc 1"
                             |    file-system-type = "oci-image"
                             |    start-command = []
                             |    endpoints {}
@@ -344,7 +279,6 @@ class TestBndlOci(CliTestCase):
         base_args = create_attributes_object({
             'format': BndlFormat.OCI_IMAGE,
             'name': 'world',
-            'component_description': 'testing desc 1',
             'image_tag': 'testing',
             'use_default_endpoints': True,
             'use_default_check': True,
@@ -372,23 +306,8 @@ class TestBndlOci(CliTestCase):
                             |    }
                             |  }
                             |}
-                            |compatibilityVersion = "0"
-                            |diskSpace = 1073741824
-                            |memory = 402653184
-                            |name = "world"
-                            |nrOfCpus = 0.1
-                            |roles = [
-                            |  "web"
-                            |]
-                            |system = "world"
-                            |systemVersion = "0"
-                            |tags = [
-                            |  "testing"
-                            |]
-                            |version = "1"
                             |components {
                             |  my-component {
-                            |    description = "testing desc 1"
                             |    file-system-type = "oci-image"
                             |    start-command = []
                             |    endpoints {
@@ -419,10 +338,10 @@ class TestBndlOci(CliTestCase):
         )
 
     def test_oci_image_with_default_endpoints_no_check(self):
+        self.maxDiff = None
         base_args = create_attributes_object({
             'format': BndlFormat.OCI_IMAGE,
             'name': 'world',
-            'component_description': 'testing desc 1',
             'image_tag': 'testing',
             'use_default_endpoints': True,
             'use_default_check': False,
@@ -450,23 +369,8 @@ class TestBndlOci(CliTestCase):
                             |    }
                             |  }
                             |}
-                            |compatibilityVersion = "0"
-                            |diskSpace = 1073741824
-                            |memory = 402653184
-                            |name = "world"
-                            |nrOfCpus = 0.1
-                            |roles = [
-                            |  "web"
-                            |]
-                            |system = "world"
-                            |systemVersion = "0"
-                            |tags = [
-                            |  "testing"
-                            |]
-                            |version = "1"
                             |components {
                             |  my-component {
-                            |    description = "testing desc 1"
                             |    file-system-type = "oci-image"
                             |    start-command = []
                             |    endpoints {
@@ -491,7 +395,6 @@ class TestBndlOci(CliTestCase):
         base_args = create_attributes_object({
             'format': BndlFormat.OCI_IMAGE,
             'name': 'world',
-            'component_description': 'testing desc 1',
             'image_tag': 'testing',
             'use_default_endpoints': True,
             'use_default_check': True,
@@ -529,23 +432,8 @@ class TestBndlOci(CliTestCase):
                             |  }
                             |  description = "hello world"
                             |}
-                            |compatibilityVersion = "0"
-                            |diskSpace = 1073741824
-                            |memory = 402653184
-                            |name = "world"
-                            |nrOfCpus = 0.1
-                            |roles = [
-                            |  "web"
-                            |]
-                            |system = "world"
-                            |systemVersion = "0"
-                            |tags = [
-                            |  "testing"
-                            |]
-                            |version = "1"
                             |components {
                             |  my-component {
-                            |    description = "testing desc 1"
                             |    file-system-type = "oci-image"
                             |    start-command = []
                             |    endpoints {
diff --git a/conductr_cli/test/test_bndl_utils.py b/conductr_cli/test/test_bndl_utils.py
index 180194b..865fd14 100644
--- a/conductr_cli/test/test_bndl_utils.py
+++ b/conductr_cli/test/test_bndl_utils.py
@@ -168,7 +168,6 @@ class TestBndlUtils(CliTestCase):
         base_args = create_attributes_object({
             'name': 'world',
             'format': BndlFormat.BUNDLE,
-            'component_description': 'testing desc 1',
             'tags': ['testing'],
             'annotations': {}
         })
@@ -176,7 +175,6 @@ class TestBndlUtils(CliTestCase):
         base_args_dup_tags = create_attributes_object({
             'name': 'world',
             'format': BndlFormat.BUNDLE,
-            'component_description': 'testing desc 1',
             'tags': ['testing', 'testing', 'testing'],
             'annotations': {}
         })
@@ -184,7 +182,6 @@ class TestBndlUtils(CliTestCase):
         extended_args = create_attributes_object({
             'name': 'world',
             'format': BndlFormat.BUNDLE,
-            'component_description': 'testing desc 2',
             'version': '4',
             'compatibility_version': '5',
             'system': 'myapp',