Add --cli flag for dcos package list. (#743)

When the flag is provided, only the packages with subcommands are
listed. Apps installed on the DC/OS cluster are not included; no server
connections are established.

cli/dcoscli/data/help/package.txt

@@ -16,7 +16,7 @@ Usage:
                          [--options=<file>]
                          [--yes]
                          <package-name>
-    dcos package list [--json --app-id=<app-id> <package-name>]
+    dcos package list [--json --app-id=<app-id> --cli <package-name>]
     dcos package search [--json <query>]
     dcos package repo add [--index=<index>] <repo-name> <repo-url>
     dcos package repo remove <repo-name>

cli/dcoscli/package/main.py

@@ -78,7 +78,7 @@ def _cmds():
 
         cmds.Command(
             hierarchy=['package', 'list'],
-            arg_keys=['--json', '--app-id', '<package-name>'],
+            arg_keys=['--json', '--app-id', '--cli', '<package-name>'],
             function=_list),
 
         cmds.Command(
@@ -405,13 +405,15 @@ def _install(package_name, package_version, options_path, app_id, cli, app,
     return 0
 
 
-def _list(json_, app_id, package_name):
+def _list(json_, app_id, cli_only, package_name):
     """List installed apps
 
     :param json_: output json if True
     :type json_: bool
     :param app_id: App ID of app to show
     :type app_id: str
+    :param cli_only: if True, only show packages with installed subcommands
+    :type cli: bool
     :param package_name: The package to show
     :type package_name: str
     :returns: process return code
@@ -422,7 +424,7 @@ def _list(json_, app_id, package_name):
     if app_id is not None:
         app_id = util.normalize_marathon_id_path(app_id)
     results = package.installed_packages(
-        package_manager, app_id, package_name)
+        package_manager, app_id, package_name, cli_only)
 
     # only emit those packages that match the provided package_name and app_id
     if results or json_:

cli/tests/data/help/package.txt

@@ -16,7 +16,7 @@ Usage:
                          [--options=<file>]
                          [--yes]
                          <package-name>
-    dcos package list [--json --app-id=<app-id> <package-name>]
+    dcos package list [--json --app-id=<app-id> --cli <package-name>]
     dcos package search [--json <query>]
     dcos package repo add [--index=<index>] <repo-name> <repo-url>
     dcos package repo remove <repo-name>

cli/tests/integrations/common.py

@@ -469,20 +469,29 @@ def file_json_ast(path):
         return json.load(f)
 
 
+def json_ast_format(ast):
+    """Returns the given JSON AST formatted as bytes
+
+    :param ast: JSON AST
+    :returns: formatted JSON
+    :rtype: bytes
+    """
+    return six.b(
+        json.dumps(ast,
+                   sort_keys=True,
+                   indent=2,
+                   separators=(',', ': '))) + b'\n'
+
+
 def file_json(path):
     """ Returns formatted json from file
 
     :param path: path to file
     :type path: str
-    :returns: formatted json as a string
+    :returns: formatted json
     :rtype: bytes
     """
-    with open(path) as f:
+    return json_ast_format(file_json_ast(path))
-        return six.b(
-            json.dumps(json.load(f),
-                       sort_keys=True,
-                       indent=2,
-                       separators=(',', ': '))) + b'\n'
 
 
 @contextlib.contextmanager

cli/tests/integrations/test_package.py

@@ -2,6 +2,7 @@ import base64
 import contextlib
 import json
 import sys
+import time
 
 import pytest
 import six
@@ -9,11 +10,10 @@ import six
 from dcos import subcommand
 
 from .common import (assert_command, assert_lines, base64_to_dict,
-                     delete_zk_node, delete_zk_nodes, exec_command,
+                     delete_zk_node, delete_zk_nodes, exec_command, file_json,
-                     file_json,
+                     get_services, package_install, package_uninstall,
-                     get_services, package_install,
+                     service_shutdown, update_config, wait_for_service,
-                     package_uninstall, service_shutdown,
+                     watch_all_deployments)
-                     wait_for_service, watch_all_deployments)
 from ..common import file_bytes
 
 UNIVERSE_REPO = "https://universe.mesosphere.com/repo"
@@ -33,6 +33,16 @@ def setup_module(module):
         ['dcos', 'package', 'repo', 'add', 'test-universe', UNIVERSE_TEST_REPO]
     )
 
+    # Give the test universe some time to become available
+    describe_command = ['dcos', 'package', 'describe', 'helloworld']
+    for _ in range(10):
+        returncode, _, _ = exec_command(describe_command)
+        if returncode == 0:
+            break
+        time.sleep(1)
+    else:
+        assert False, 'test-universe failed to come up'
+
 
 def teardown_module(module):
     services = get_services()
@@ -378,10 +388,10 @@ def test_install_specific_version():
         b'persisted state\n'
     )
 
-    with _package('marathon',
+    with _package(name='marathon',
+                  args=['--yes', '--package-version=0.11.1'],
                   stdout=stdout,
-                  uninstall_stderr=uninstall_stderr,
+                  uninstall_stderr=uninstall_stderr):
-                  args=['--yes', '--package-version=0.11.1']):
 
         returncode, stdout, stderr = exec_command(
             ['dcos', 'package', 'list', 'marathon', '--json'])
@@ -533,7 +543,7 @@ def test_uninstall_missing():
 def test_uninstall_subcommand():
     _install_helloworld()
     _uninstall_helloworld()
-    _list()
+    _list(args=['--json'], stdout=b'[]\n')
 
 
 def test_uninstall_cli():
@@ -604,20 +614,23 @@ def test_uninstall_multiple_apps():
 
 
 def test_list(zk_znode):
-    _list()
+    empty = b'[]\n'
-    _list(args=['xyzzy', '--json'])
+
-    _list(args=['--app-id=/xyzzy', '--json'])
+    _list(args=['--json'], stdout=empty)
+    _list(args=['xyzzy', '--json'], stdout=empty)
+    _list(args=['--app-id=/xyzzy', '--json'], stdout=empty)
 
     with _chronos_package():
 
         expected_output = file_json(
             'tests/data/package/json/test_list_chronos.json')
-        _list(stdout=expected_output)
+        _list(args=['--json'], stdout=expected_output)
         _list(args=['--json', 'chronos'], stdout=expected_output)
         _list(args=['--json', '--app-id=/chronos'], stdout=expected_output)
 
-    _list(args=['--json', 'ceci-nest-pas-une-package'])
+    le_package = 'ceci-nest-pas-une-package'
-    _list(args=['--json', '--app-id=/ceci-nest-pas-une-package'])
+    _list(args=['--json', le_package], stdout=empty)
+    _list(args=['--json', '--app-id=/' + le_package], stdout=empty)
 
 
 def test_list_table():
@@ -655,7 +668,7 @@ def test_list_cli():
     _install_helloworld()
     stdout = file_json(
         'tests/data/package/json/test_list_helloworld.json')
-    _list(stdout=stdout)
+    _list(args=['--json'], stdout=stdout)
     _uninstall_helloworld()
 
     stdout = (b"Installing CLI subcommand for package [helloworld] " +
@@ -667,11 +680,30 @@ def test_list_cli():
 
     stdout = file_json(
         'tests/data/package/json/test_list_helloworld_cli.json')
-    _list(stdout=stdout)
+    _list(args=['--json'], stdout=stdout)
 
     _uninstall_cli_helloworld()
 
 
+def test_list_cli_only():
+    helloworld_path = 'tests/data/package/json/test_list_helloworld_cli.json'
+    helloworld_json = file_json(helloworld_path)
+
+    with _helloworld_cli(), update_config('core.dcos_url', 'http://nohost'):
+        assert_command(
+            cmd=['dcos', 'package', 'list', '--json', '--cli'],
+            stdout=helloworld_json)
+
+        assert_command(
+            cmd=['dcos', 'package', 'list', '--json', '--cli',
+                 '--app-id=/helloworld'],
+            stdout=b'[]\n')
+
+        assert_command(
+            cmd=['dcos', 'package', 'list', '--json', '--cli', 'helloworld'],
+            stdout=helloworld_json)
+
+
 def test_uninstall_multiple_frameworknames(zk_znode):
     _install_chronos(
         args=['--yes', '--options=tests/data/package/chronos-1.json'])
@@ -683,14 +715,12 @@ def test_uninstall_multiple_frameworknames(zk_znode):
     expected_output = file_json(
         'tests/data/package/json/test_list_chronos_two_users.json')
 
-    _list(stdout=expected_output)
+    _list(args=['--json'], stdout=expected_output)
     _list(args=['--json', 'chronos'], stdout=expected_output)
-    _list(args=['--json', '--app-id=/chronos-user-1'],
+    _list(args=['--json', '--app-id=/chronos-user-1'], stdout=file_json(
-          stdout=file_json(
         'tests/data/package/json/test_list_chronos_user_1.json'))
 
-    _list(args=['--json', '--app-id=/chronos-user-2'],
+    _list(args=['--json', '--app-id=/chronos-user-2'], stdout=file_json(
-          stdout=file_json(
         'tests/data/package/json/test_list_chronos_user_2.json'))
 
     _uninstall_chronos(
@@ -941,52 +971,68 @@ def _chronos_package(
         watch_all_deployments()
 
 
-def _list(args=['--json'],
+def _list(args, stdout):
-          stdout=b'[]\n'):
+    assert_command(['dcos', 'package', 'list'] + args, stdout=stdout)
-    assert_command(['dcos', 'package', 'list'] + args,
+
-                   stdout=stdout)
+
+HELLOWORLD_CLI_STDOUT = (b'Installing CLI subcommand for package [helloworld] '
+                         b'version [0.1.0]\n'
+                         b'New command available: dcos ' +
+                         _executable_name(b'helloworld') + b'\n')
 
 
 def _helloworld():
     stdout = (b'A sample pre-installation message\n'
               b'Installing Marathon app for package [helloworld] version '
-              b'[0.1.0]\n'
+              b'[0.1.0]\n' + HELLOWORLD_CLI_STDOUT +
-              b'Installing CLI subcommand for package [helloworld] '
+              b'A sample post-installation message\n')
-              b'version [0.1.0]\n'
-              b'New command available: dcos ' +
-              _executable_name(b'helloworld') +
-              b'\nA sample post-installation message\n')
 
     stderr = b'Uninstalled package [helloworld] version [0.1.0]\n'
-    return _package('helloworld',
+    return _package(name='helloworld',
+                    args=['--yes'],
                     stdout=stdout,
                     uninstall_stderr=stderr)
 
 
+def _helloworld_cli():
+    return _package(name='helloworld',
+                    args=['--yes', '--cli'],
+                    stdout=HELLOWORLD_CLI_STDOUT,
+                    uninstall_stderr=b'')
+
+
 @contextlib.contextmanager
 def _package(name,
+             args,
              stdout=b'',
-             uninstall_stderr=b'',
+             uninstall_stderr=b''):
-             args=['--yes']):
+    """Context manager that installs a package on entrance, and uninstalls it on
-    """Context manager that installs a package on entrace, and uninstalls it on
     exit.
 
     :param name: package name
     :type name: str
-    :param stdout: Expected stdout
-    :type stdout: str
-    :param uninstall_stderr: Expected stderr
-    :type uninstall_stderr: str
     :param args: extra CLI args
     :type args: [str]
+    :param stdout: Expected stdout
+    :type stdout: bytes
+    :param uninstall_stderr: Expected stderr
+    :type uninstall_stderr: bytes
     :rtype: None
     """
 
-    assert_command(['dcos', 'package', 'install', name] + args,
+    command = ['dcos', 'package', 'install', name] + args
-                   stdout=stdout)
+
+    installed = False
     try:
+        returncode_, stdout_, stderr_ = exec_command(command)
+        installed = (returncode_ == 0)
+
+        assert installed
+        assert stdout_ == stdout
+
         yield
     finally:
+        if installed:
             assert_command(
                 ['dcos', 'package', 'uninstall', name],
                 stderr=uninstall_stderr)

cli/tests/unit/test_package_list.py

@@ -0,0 +1,63 @@
+from dcos import package
+from ..common import assert_same_elements
+
+
+def test_merge_installed_app_req_cli_req():
+    _assert_merged_installed(
+        merged_keys=['a', 'c'], app_only=True, cli_only=True)
+
+
+def test_merge_installed_app_req_cli_opt():
+    _assert_merged_installed(
+        merged_keys=['a', 'b', 'c'], app_only=True, cli_only=False)
+
+
+def test_merged_installed_app_opt_cli_req():
+    _assert_merged_installed(
+        merged_keys=['a', 'c', 'd'], app_only=False, cli_only=True)
+
+
+def test_merged_installed_app_opt_cli_opt():
+    _assert_merged_installed(
+        merged_keys=['a', 'b', 'c', 'd'], app_only=False, cli_only=False)
+
+
+def _assert_merged_installed(merged_keys, app_only, cli_only):
+    merged = _merged()
+    expected_merged = [merged[k] for k in merged_keys]
+
+    actual_merged = package.merge_installed(
+        _apps(), _subs(), app_only, cli_only)
+
+    assert_same_elements(expected_merged, actual_merged)
+
+
+def _apps():
+    return [{'name': 'pkg_a', 'appId': '/pkg_a1', 'foo_a': 'bar_a1'},
+            {'name': 'pkg_a', 'appId': '/pkg_a2', 'foo_a': 'bar_a1'},
+            {'name': 'pkg_b', 'appId': '/pkg_b1', 'foo_b': 'bar_b1'},
+            {'name': 'pkg_b', 'appId': '/pkg_b2', 'foo_b': 'bar_b2'},
+            {'name': 'pkg_c', 'appId': '/pkg_c1', 'foo_c': 'bar_c1'}]
+
+
+def _subs():
+    return [{'name': 'pkg_a', 'command': {'name': 'pkg_a'}, 'foo_a': 'baz_a'},
+            {'name': 'pkg_c', 'command': {'name': 'pkg_c'}, 'foo_c': 'baz_c'},
+            {'name': 'pkg_d', 'command': {'name': 'pkg_d'}, 'foo_d': 'baz_d'}]
+
+
+def _merged():
+    return {'a': {'name': 'pkg_a',
+                  'command': {'name': 'pkg_a'},
+                  'apps': ['/pkg_a1', '/pkg_a2'],
+                  'foo_a': 'baz_a'},
+            'b': {'name': 'pkg_b',
+                  'apps': ['/pkg_b1', '/pkg_b2'],
+                  'foo_b': 'bar_b1'},
+            'c': {'name': 'pkg_c',
+                  'command': {'name': 'pkg_c'},
+                  'apps': ['/pkg_c1'],
+                  'foo_c': 'baz_c'},
+            'd': {'name': 'pkg_d',
+                  'command': {'name': 'pkg_d'},
+                  'foo_d': 'baz_d'}}

dcos/package.py

@@ -1,4 +1,4 @@
-import collections
+import itertools
 
 from dcos import emitting, subcommand, util
 from dcos.errors import DCOSException
@@ -28,7 +28,8 @@ def uninstall(pkg, package_name, remove_all, app_id, cli, app):
         cli = app = True
 
     uninstalled = False
-    installed = installed_packages(pkg, app_id, package_name)
+    installed = installed_packages(
+        pkg, app_id, package_name, cli_only=False)
     installed_cli = next((True for installed_pkg in installed
                           if installed_pkg.get("command")), False)
     installed_app = next((True for installed_pkg in installed
@@ -65,57 +66,6 @@ def uninstall_subcommand(distribution_name):
     return subcommand.uninstall(distribution_name)
 
 
-class InstalledPackage(object):
-    """Represents an intalled DC/OS package.  One of `app` and
-    `subcommand` must be supplied.
-
-    :param apps: A dictionary representing a marathon app. Of the
-                format returned by `installed_apps()`
-    :type apps: [dict]
-    :param subcommand: Installed subcommand
-    :type subcommand: subcommand.InstalledSubcommand
-    """
-
-    def __init__(self, apps=[], subcommand=None):
-        assert apps or subcommand
-        self.apps = apps
-        self.subcommand = subcommand
-
-    def name(self):
-        """
-        :returns: The name of the package
-        :rtype: str
-        """
-        if self.subcommand:
-            return self.subcommand.name
-        else:
-            return self.apps[0]['name']
-
-    def dict(self):
-        """ A dictionary representation of the package.  Used by `dcos package
-        list`.
-
-        :returns: A dictionary representation of the package.
-        :rtype: dict
-        """
-        ret = {}
-
-        if self.subcommand:
-            ret['command'] = {'name': self.subcommand.name}
-
-        if self.apps:
-            ret['apps'] = sorted([app['appId'] for app in self.apps])
-
-        if self.subcommand:
-            package_json = self.subcommand.package_json()
-            ret.update(package_json)
-        else:
-            ret.update(self.apps[0])
-            ret.pop('appId')
-
-        return ret
-
-
 def _matches_package_name(name, command_name):
     """
     :param name: the name of the package
@@ -130,7 +80,7 @@ def _matches_package_name(name, command_name):
     return name is None or command_name == name
 
 
-def installed_packages(package_manager, app_id, package_name):
+def installed_packages(package_manager, app_id, package_name, cli_only):
     """Returns all installed packages in the format:
 
     [{
@@ -147,27 +97,25 @@ def installed_packages(package_manager, app_id, package_name):
     :type app_id: str
     :param package_name: The package to show
     :type package_name: str
+    :param cli_only: if True, returns only packages with locally installed
+        subcommands, without retrieving the apps installed on the cluster
+    :type cli_only: bool
     :returns: A list of installed packages matching criteria
     :rtype: [dict]
     """
 
-    dicts = collections.defaultdict(lambda: {'apps': [], 'command': None})
+    apps = []
-
+    if not cli_only:
         apps = package_manager.installed_apps(package_name, app_id)
-    for app in apps:
-        key = app['name']
-        dicts[key]['apps'].append(app)
 
-    subcommands = installed_subcommands()
+    subcommands = []
-    for subcmd in subcommands:
+    for subcmd in installed_subcommands():
         if _matches_package_name(package_name, subcmd.name):
-            dicts[subcmd.name]['command'] = subcmd
+            subcmd_dict = subcmd.package_json()
+            subcmd_dict['command'] = {'name': subcmd.name}
+            subcommands.append(subcmd_dict)
 
-    installed = [
+    return merge_installed(apps, subcommands, app_id is not None, cli_only)
-        InstalledPackage(pkg['apps'], pkg['command']) for pkg in dicts.values()
-    ]
-
-    return [pkg.dict() for pkg in installed]
 
 
 def installed_subcommands():
@@ -179,3 +127,45 @@ def installed_subcommands():
 
     return [subcommand.InstalledSubcommand(name) for name in
             subcommand.distributions()]
+
+
+def merge_installed(apps, subcommands, app_only, cli_only):
+    """Combines collections of installed apps and subcommands, merging
+    elements from the same package.
+
+    :param apps: information on each running app in the cluster; must have
+        'name' and 'appId' keys
+    :type apps: [dict]
+    :param subcommands: information on each subcommand installed locally; must
+        have a 'name' key
+    :type subcommands: [dict]
+    :param app_only: if True, only returns elements that have an app
+    :type app_only: bool
+    :param cli_only: if True, only returns elements that have a subcommand
+    :type cli_only: bool
+    :returns: the resulting merged collection, with one element per package
+    :rtype: [{}]
+    """
+
+    indexed_apps = {}
+    grouped_apps = itertools.groupby(apps, key=lambda app: app['name'])
+    for name, app_group in grouped_apps:
+        app_list = list(app_group)
+        pkg = app_list[0]
+        pkg['apps'] = sorted(app['appId'] for app in app_list)
+        del pkg['appId']
+        indexed_apps[name] = pkg
+
+    indexed_subcommands = {subcmd['name']: subcmd for subcmd in subcommands}
+
+    merged = []
+    for name, app in indexed_apps.items():
+        subcmd = indexed_subcommands.pop(name, {})
+        if subcmd or not cli_only:
+            app.update(subcmd)
+            merged.append(app)
+
+    if not app_only:
+        merged.extend(indexed_subcommands.values())
+
+    return merged