From 37bb514fba474d1b9d00d99640bb110edce6d720 Mon Sep 17 00:00:00 2001 From: Charles Ruhland Date: Wed, 12 Oct 2016 10:46:41 -0700 Subject: [PATCH] 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 | 2 +- cli/dcoscli/package/main.py | 8 +- cli/tests/data/help/package.txt | 2 +- cli/tests/integrations/common.py | 23 +++-- cli/tests/integrations/test_package.py | 138 ++++++++++++++++--------- cli/tests/unit/test_package_list.py | 63 +++++++++++ dcos/package.py | 126 +++++++++++----------- 7 files changed, 236 insertions(+), 126 deletions(-) create mode 100644 cli/tests/unit/test_package_list.py diff --git a/cli/dcoscli/data/help/package.txt b/cli/dcoscli/data/help/package.txt index 76cc4c8..f10bfae 100644 --- a/cli/dcoscli/data/help/package.txt +++ b/cli/dcoscli/data/help/package.txt @@ -16,7 +16,7 @@ Usage: [--options=] [--yes] - dcos package list [--json --app-id= ] + dcos package list [--json --app-id= --cli ] dcos package search [--json ] dcos package repo add [--index=] dcos package repo remove diff --git a/cli/dcoscli/package/main.py b/cli/dcoscli/package/main.py index 17c9766..8a222d1 100644 --- a/cli/dcoscli/package/main.py +++ b/cli/dcoscli/package/main.py @@ -78,7 +78,7 @@ def _cmds(): cmds.Command( hierarchy=['package', 'list'], - arg_keys=['--json', '--app-id', ''], + arg_keys=['--json', '--app-id', '--cli', ''], 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_: diff --git a/cli/tests/data/help/package.txt b/cli/tests/data/help/package.txt index 76cc4c8..f10bfae 100644 --- a/cli/tests/data/help/package.txt +++ b/cli/tests/data/help/package.txt @@ -16,7 +16,7 @@ Usage: [--options=] [--yes] - dcos package list [--json --app-id= ] + dcos package list [--json --app-id= --cli ] dcos package search [--json ] dcos package repo add [--index=] dcos package repo remove diff --git a/cli/tests/integrations/common.py b/cli/tests/integrations/common.py index ae4f947..be1c0b8 100644 --- a/cli/tests/integrations/common.py +++ b/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 six.b( - json.dumps(json.load(f), - sort_keys=True, - indent=2, - separators=(',', ': '))) + b'\n' + return json_ast_format(file_json_ast(path)) @contextlib.contextmanager diff --git a/cli/tests/integrations/test_package.py b/cli/tests/integrations/test_package.py index 67c4781..dde82fa 100644 --- a/cli/tests/integrations/test_package.py +++ b/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, - file_json, - get_services, package_install, - package_uninstall, service_shutdown, - wait_for_service, watch_all_deployments) + delete_zk_node, delete_zk_nodes, exec_command, file_json, + get_services, package_install, package_uninstall, + service_shutdown, update_config, 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, - args=['--yes', '--package-version=0.11.1']): + uninstall_stderr=uninstall_stderr): 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() - _list(args=['xyzzy', '--json']) - _list(args=['--app-id=/xyzzy', '--json']) + empty = b'[]\n' + + _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']) - _list(args=['--json', '--app-id=/ceci-nest-pas-une-package']) + le_package = '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'], - stdout=file_json( + _list(args=['--json', '--app-id=/chronos-user-1'], stdout=file_json( 'tests/data/package/json/test_list_chronos_user_1.json')) - _list(args=['--json', '--app-id=/chronos-user-2'], - stdout=file_json( + _list(args=['--json', '--app-id=/chronos-user-2'], stdout=file_json( 'tests/data/package/json/test_list_chronos_user_2.json')) _uninstall_chronos( @@ -941,56 +971,72 @@ def _chronos_package( watch_all_deployments() -def _list(args=['--json'], - stdout=b'[]\n'): - assert_command(['dcos', 'package', 'list'] + args, - stdout=stdout) +def _list(args, 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'Installing CLI subcommand for package [helloworld] ' - b'version [0.1.0]\n' - b'New command available: dcos ' + - _executable_name(b'helloworld') + - b'\nA sample post-installation message\n') + b'[0.1.0]\n' + HELLOWORLD_CLI_STDOUT + + b'A 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'', - args=['--yes']): - """Context manager that installs a package on entrace, and uninstalls it on + uninstall_stderr=b''): + """Context manager that installs a package on entrance, 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, - stdout=stdout) + command = ['dcos', 'package', 'install', name] + args + + installed = False try: + returncode_, stdout_, stderr_ = exec_command(command) + installed = (returncode_ == 0) + + assert installed + assert stdout_ == stdout + yield finally: - assert_command( - ['dcos', 'package', 'uninstall', name], - stderr=uninstall_stderr) - watch_all_deployments() + if installed: + assert_command( + ['dcos', 'package', 'uninstall', name], + stderr=uninstall_stderr) + watch_all_deployments() def _repo_add(args=[], repo_list=[]): diff --git a/cli/tests/unit/test_package_list.py b/cli/tests/unit/test_package_list.py new file mode 100644 index 0000000..a8f60e9 --- /dev/null +++ b/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'}} diff --git a/dcos/package.py b/dcos/package.py index 32e5af4..f41e2a5 100644 --- a/dcos/package.py +++ b/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) - apps = package_manager.installed_apps(package_name, app_id) - for app in apps: - key = app['name'] - dicts[key]['apps'].append(app) - - subcommands = installed_subcommands() - for subcmd in subcommands: + 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 = [ - InstalledPackage(pkg['apps'], pkg['command']) for pkg in dicts.values() - ] - - return [pkg.dict() for pkg in installed] + return merge_installed(apps, subcommands, app_id is not None, cli_only) 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