From 8c15db503ded868caa5584c1d0e5a83d217726bd Mon Sep 17 00:00:00 2001 From: Yuriy Taraday Date: Thu, 15 Sep 2016 16:13:07 +0300 Subject: [PATCH] Switch to cliff Added all commands and basic app for cliff, removed all legacy/compatibility stuff around CLI. Config file CLI argument and logging will be fixed in separate commit. Change-Id: Ib52b4c139c2ac9ea5afa073febcd764edced1464 --- fuel_ccp/cli.py | 211 ++++++++++++++++++++++++++----- fuel_ccp/config/__init__.py | 53 ++------ fuel_ccp/config/cli.py | 46 ------- fuel_ccp/tests/conf_fixture.py | 2 +- fuel_ccp/tests/test_cli.py | 218 +++++++++++++++++++++++++++++++++ fuel_ccp/tests/test_config.py | 32 ----- requirements.txt | 1 + setup.cfg | 7 ++ tox.ini | 3 +- 9 files changed, 414 insertions(+), 159 deletions(-) create mode 100644 fuel_ccp/tests/test_cli.py diff --git a/fuel_ccp/cli.py b/fuel_ccp/cli.py index b7b17f09..ec4605f2 100644 --- a/fuel_ccp/cli.py +++ b/fuel_ccp/cli.py @@ -1,6 +1,14 @@ +import argparse +import logging +import os.path import signal import sys +from cliff import app +from cliff import command +from cliff import commandmanager + +import fuel_ccp from fuel_ccp import build from fuel_ccp import cleanup from fuel_ccp.common import utils @@ -12,65 +20,200 @@ from fuel_ccp import validate from fuel_ccp.validation import service as validation_service CONF = config.CONF +LOG = logging.getLogger(__name__) -def do_build(): - if CONF.builder.push and not CONF.registry.address: - raise RuntimeError('No registry specified, cannot push') - if CONF.repositories.clone: - do_fetch() - build.build_components(components=CONF.action.components) +class BaseCommand(command.Command): + def get_parser(self, *args, **kwargs): + parser = super(BaseCommand, self).get_parser(*args, **kwargs) + parser.set_defaults(**CONF.action._dict) + return parser -def do_deploy(): - if CONF.repositories.clone: - do_fetch() - components_map = utils.get_deploy_components_info() +class Build(BaseCommand): + """Build CCP docker images""" - components = CONF.action.components - if components: - components = set(components) + def get_parser(self, *args, **kwargs): + parser = super(Build, self).get_parser(*args, **kwargs) + parser.add_argument('-c', '--components', + nargs='+', + help='CCP component to build') + return parser - validation_service.validate_service_definitions(components_map, components) - deploy.deploy_components(components_map, components) + def take_action(self, parsed_args): + if CONF.builder.push and not CONF.registry.address: + raise RuntimeError('No registry specified, cannot push') + if CONF.repositories.clone: + do_fetch() + build.build_components(components=parsed_args.components) -def do_validate(): - if CONF.repositories.clone: - do_fetch() +class Deploy(BaseCommand): + """Deploy CCP""" - components = CONF.action.components - if components: - components = set(components) - validate.validate(components=components, types=CONF.action.types) + def get_parser(self, *args, **kwargs): + parser = super(Deploy, self).get_parser(*args, **kwargs) + parser.add_argument('-c', '--components', + nargs='+', + help='CCP component to build') + parser.add_argument("--dry-run", + action='store_true', + help="Print k8s objects definitions without" + "actual creation") + parser.add_argument('--export-dir', + help='Directory to export created k8s objects') + return parser + + def take_action(self, parsed_args): + if CONF.repositories.clone: + do_fetch() + # only these two are being implicitly passed + CONF.action._update( + dry_run=parsed_args.dry_run, + export_dir=parsed_args.export_dir, + ) + components_map = utils.get_deploy_components_info() + + components = parsed_args.components + if components: + components = set(components) + + validation_service.validate_service_definitions( + components_map, components) + deploy.deploy_components(components_map, components) def do_fetch(): fetch.fetch_repositories(CONF.repositories.names) -def do_cleanup(): - cleanup.cleanup(auth_url=CONF.action.auth_url, - skip_os_cleanup=CONF.action.skip_os_cleanup) +class Fetch(BaseCommand): + """Fetch all repos with components definitions""" - -def do_show_dep(): - if CONF.repositories.clone: + def take_action(self, parsed_args): do_fetch() - dependencies.show_dep(CONF.action.components) + + +class Validate(BaseCommand): + """Validate CCP components""" + + def get_parser(self, *args, **kwargs): + parser = super(Validate, self).get_parser(*args, **kwargs) + parser.add_argument('-c', '--components', + nargs='+', + help='CCP components to validate') + parser.add_argument('-t', '--types', + nargs="+", + help="List of validation types to perform. " + "If not specified - perform all " + "supported validation types") + return parser + + def take_action(self, parsed_args): + if CONF.repositories.clone: + do_fetch() + + components = parsed_args.components + if components: + components = set(components) + + validate.validate(components=components, types=parsed_args.types) + + +class Cleanup(BaseCommand): + """Remove all OpenStack resources and destroy CCP deployment""" + + def get_parser(self, *args, **kwargs): + parser = super(Cleanup, self).get_parser(*args, **kwargs) + # Making auth url configurable at least until Ingress/LB support will + # be implemented + parser.add_argument('--auth-url', + help='The URL of Keystone authentication ' + 'server') + parser.add_argument('--skip-os-cleanup', + action='store_true', + help='Skip cleanup of OpenStack environment') + return parser + + def take_action(self, parsed_args): + cleanup.cleanup(auth_url=parsed_args.auth_url, + skip_os_cleanup=parsed_args.skip_os_cleanup) + + +class ShowDep(BaseCommand): + """Show dependencies of CCP components""" + + def get_parser(self, *args, **kwargs): + parser = super(ShowDep, self).get_parser(*args, **kwargs) + parser.add_argument('components', + nargs='+', + help='CCP components to show dependencies') + return parser + + def take_action(self, parsed_args): + if CONF.repositories.clone: + do_fetch() + dependencies.show_dep(parsed_args.components) def signal_handler(signo, frame): sys.exit(-signo) -def main(): +class CCPApp(app.App): + CONSOLE_MESSAGE_FORMAT = \ + '%(asctime)s %(levelname)-8s %(name)-15s %(message)s' + + def __init__(self, **kwargs): + super(CCPApp, self).__init__( + description='Containerized Control Plane tool', + version=fuel_ccp.__version__, + command_manager=commandmanager.CommandManager('ccp.cli'), + **kwargs + ) + self.config_file = None + + @staticmethod + def add_config_file_argument(parser): + parser.add_argument( + '--config-file', + metavar='PATH', + help=('Path to a config file to use.')) + + @staticmethod + def get_config_file(argv): + parser = argparse.ArgumentParser(add_help=False) + CCPApp.add_config_file_argument(parser) + parsed_args, _ = parser.parse_known_args(argv) + if parsed_args.config_file: + return os.path.abspath(os.path.expanduser(parsed_args.config_file)) + else: + return config.find_config() + + def run(self, argv): + self.config_file = self.get_config_file(argv) + config.setup_config(self.config_file) + self.add_config_file_argument(self.parser) + defaults = {k: CONF[k] for k in ['debug', 'verbose_level', 'log_file']} + if CONF.debug: # We're used to having DEBUG logging with debug conf + defaults['verbose_level'] = 2 + self.parser.set_defaults(**defaults) + return super(CCPApp, self).run(argv) + + def configure_logging(self): + super(CCPApp, self).configure_logging() + if self.config_file: + LOG.debug('Loaded config from file %s', self.config_file) + else: + LOG.debug('No config file loaded') + + +def main(argv=None): signal.signal(signal.SIGTERM, signal_handler) signal.signal(signal.SIGINT, signal_handler) - config.setup_config() - - func = globals()['do_%s' % CONF.action.name.replace('-', '_')] - func() + if argv is None: + argv = sys.argv[1:] + CCPApp().run(argv) if __name__ == '__main__': diff --git a/fuel_ccp/config/__init__.py b/fuel_ccp/config/__init__.py index b4c1319d..0c3f6466 100644 --- a/fuel_ccp/config/__init__.py +++ b/fuel_ccp/config/__init__.py @@ -1,6 +1,4 @@ -import argparse import logging -import sys import jsonschema import os @@ -18,42 +16,16 @@ LOG = logging.getLogger(__name__) _REAL_CONF = None -def setup_config(args=None): - if args is None: - args = sys.argv[1:] - config_file, args = get_cli_config(args) - if config_file is None: - config_file = find_config() +def setup_config(config_file): yconf = get_config_defaults() if config_file: loaded_conf = _yaml.load_with_includes(config_file) yconf._merge(loaded_conf) - action_dict = parse_args(args) - yconf._merge({'action': action_dict}) - logging.basicConfig(level=logging.DEBUG) - if config_file: - LOG.debug('Loaded config from file %s', config_file) - else: - LOG.debug('No config file loaded') validate_config(yconf) global _REAL_CONF _REAL_CONF = yconf -def get_cli_config(args): - parser = argparse.ArgumentParser(add_help=False) - parser.add_argument( - '--config-file', - metavar='PATH', - help=('Path to a config file to use.')) - args, rest = parser.parse_known_args(args) - if args.config_file: - config_file = os.path.abspath(os.path.expanduser(args.config_file)) - else: - config_file = None - return config_file, rest - - def find_config(): home = os.path.expanduser('~') candidates = [ @@ -69,20 +41,6 @@ def find_config(): return None -def parse_args(args=None): - parser = argparse.ArgumentParser() - parser.add_argument('--debug', action='store_true', default=False) - parser.add_argument('--verbose', action='store_true', default=False) - parser.add_argument('--log-file', default=None) - subparsers = parser.add_subparsers(dest='action') - cli.add_parsers(subparsers) - action_dict = vars(parser.parse_args(args)) - action_dict['name'] = action_dict.pop('action') - for name in ['debug', 'verbose', 'log_file']: - del action_dict[name] - return action_dict - - class _Wrapper(object): def __getattr__(self, name): return getattr(_REAL_CONF, name) @@ -94,7 +52,11 @@ CONF = _Wrapper() def get_config_defaults(): - defaults = _yaml.AttrDict() + defaults = _yaml.AttrDict({ + 'debug': False, + 'verbose_level': 1, + 'log_file': None, + }) for module in [cli, builder, images, kubernetes, registry, repositories]: defaults._merge(module.DEFAULTS) return defaults @@ -106,7 +68,8 @@ def get_config_schema(): 'additionalProperties': False, 'properties': { 'debug': {'type': 'boolean'}, - 'verbose': {'type': 'boolean'}, + 'verbose_level': {'type': 'integer'}, + 'log_file': {'anyOf': [{'type': 'null'}, {'type': 'string'}]}, }, } for module in [cli, builder, images, kubernetes, registry, repositories]: diff --git a/fuel_ccp/config/cli.py b/fuel_ccp/config/cli.py index 95513565..5bbd8a5e 100644 --- a/fuel_ccp/config/cli.py +++ b/fuel_ccp/config/cli.py @@ -1,49 +1,3 @@ - - -def add_parsers(subparsers): - build_action = subparsers.add_parser('build') - build_action.add_argument('-c', '--components', - nargs='+', - help='CCP components to build') - - validate_action = subparsers.add_parser('validate') - validate_action.add_argument('-c', '--components', - nargs='+', - help='CCP components to validate') - validate_action.add_argument('-t', '--types', - nargs="+", - help="List of validation types to perform. " - "If not specified - perform all " - "supported validation types") - - deploy_action = subparsers.add_parser('deploy') - deploy_action.add_argument('-c', '--components', - nargs='+', - help='CCP components to deploy') - deploy_action.add_argument("--dry-run", - action='store_true', - help="Print k8s objects definitions without" - "actual creation") - deploy_action.add_argument('--export-dir', - help='Directory to export created k8s objects') - - subparsers.add_parser('fetch') - - cleanup_action = subparsers.add_parser('cleanup') - # Making auth url configurable at least until Ingress/LB support will - # be implemented - cleanup_action.add_argument('--auth-url', - help='The URL of Keystone authentication ' - 'server') - cleanup_action.add_argument('--skip-os-cleanup', - action='store_true', - help='Skip cleanup of OpenStack environment') - - show_dep_action = subparsers.add_parser('show-dep') - show_dep_action.add_argument('components', - nargs='+', - help='CCP components to show dependencies') - DEFAULTS = { 'deploy_config': None, 'action': { diff --git a/fuel_ccp/tests/conf_fixture.py b/fuel_ccp/tests/conf_fixture.py index 140131e2..574b9582 100644 --- a/fuel_ccp/tests/conf_fixture.py +++ b/fuel_ccp/tests/conf_fixture.py @@ -6,5 +6,5 @@ from fuel_ccp import config class Config(fixtures.Fixture): def _setUp(self): self.useFixture(fixtures.MockPatchObject(config, '_REAL_CONF')) - config.setup_config(['build']) + config.setup_config(None) self.conf = config._REAL_CONF diff --git a/fuel_ccp/tests/test_cli.py b/fuel_ccp/tests/test_cli.py new file mode 100644 index 00000000..0f9d8f68 --- /dev/null +++ b/fuel_ccp/tests/test_cli.py @@ -0,0 +1,218 @@ +import io + +import fixtures +import testscenarios +from testtools import content + +from fuel_ccp import cli +from fuel_ccp import config +from fuel_ccp.tests import base + + +class SafeCCPApp(cli.CCPApp): + # Cliff always outputs str + if str is bytes: + _io_cls = io.BytesIO + else: + _io_cls = io.StringIO + + def __init__(self): + super(SafeCCPApp, self).__init__( + stdin=self._io_cls(), + stdout=self._io_cls(), + stderr=self._io_cls(), + ) + + def build_option_parser(self, description, version, argparse_kwargs=None): + # Debug does magic in cliff, we need it always on + parser = super(SafeCCPApp, self).build_option_parser( + description, version, argparse_kwargs) + parser.set_defaults(debug=True) + return parser + + def get_fuzzy_matches(self, cmd): + # Turn off guessing, we need exact failures in tests + return [] + + def run(self, argv): + try: + exit_code = super(SafeCCPApp, self).run(argv) + except SystemExit as e: + exit_code = e.code + return exit_code + + +class TestCase(base.TestCase): + def setUp(self): + super(TestCase, self).setUp() + self.app = SafeCCPApp() + + +class TestCLI(TestCase): + def test_help(self): + exit_code = self.app.run(["--help"]) + self.assertEqual(exit_code, 0) + self.assertFalse(self.app.stderr.getvalue()) + self.assertNotIn('Could not', self.app.stdout.getvalue()) + + +class TestParser(testscenarios.WithScenarios, TestCase): + scenarios = [] + + cmd = None + argv = None + + def setUp(self): + super(TestParser, self).setUp() + fixture = fixtures.MockPatch('fuel_ccp.fetch.fetch_repositories') + self.fetch_mock = self.useFixture(fixture).mock + + def _run_app(self): + exit_code = self.app.run([self.cmd] + self.argv) + stdout = self.app.stdout.getvalue() + stderr = self.app.stderr.getvalue() + self.addDetail('stdout', content.text_content(stdout)) + self.addDetail('stderr', content.text_content(stderr)) + self.assertEqual(exit_code, 0) + self.assertFalse(stdout) + self.assertFalse(stderr) + + +class TestBuild(TestParser): + cmd = 'build' + scenarios = [ + ('empty', {'argv': [], 'components': None}), + ('seq', {'argv': ['-c', '1', '2'], 'components': ['1', '2']}), + ('sep', {'argv': ['-c', '1', '-c', '2'], 'components': ['2']}), + ('long', { + 'argv': ['--components', '1', '2'], + 'components': ['1', '2'], + }), + ] + + components = None + + def test_parser(self): + fixture = fixtures.MockPatch('fuel_ccp.build.build_components') + bc_mock = self.useFixture(fixture).mock + self._run_app() + bc_mock.assert_called_once_with(components=self.components) + + +class TestDeploy(TestParser): + cmd = 'deploy' + scenarios = testscenarios.multiply_scenarios(TestBuild.scenarios, [ + ('no_add', { + 'add_argv': [], + 'action_vals': {'dry_run': False, 'export_dir': None} + }), + ('dry_run', { + 'add_argv': ['--dry-run'], + 'action_vals': {'dry_run': True, 'export_dir': None} + }), + ('dry_run_export_dir', { + 'add_argv': ['--dry-run', '--export-dir', 'test'], + 'action_vals': {'dry_run': True, 'export_dir': 'test'} + }), + ]) + + add_argv = None + components = None + action_vals = None + + def test_parser(self): + fixture = fixtures.MockPatch('fuel_ccp.deploy.deploy_components') + dc_mock = self.useFixture(fixture).mock + fixture = fixtures.MockPatch( + 'fuel_ccp.validation.service.validate_service_definitions') + self.useFixture(fixture) + self.argv += self.add_argv + self._run_app() + if self.components is None: + components = None + else: + components = set(self.components) + dc_mock.assert_called_once_with({}, components) + for k, v in self.action_vals.items(): + self.assertEqual(config.CONF.action[k], v) + + +class TestFetch(TestParser): + cmd = 'fetch' + scenarios = [('empty', {'argv': []})] + + def test_parser(self): + self._run_app() + self.fetch_mock.assert_called_once_with(config.CONF.repositories.names) + + +class TestCleanup(TestParser): + cmd = 'cleanup' + scenarios = [ + ('empty', { + 'argv': [], + 'margs': {'auth_url': None, 'skip_os_cleanup': False}, + }), + ('auth_url', { + 'argv': ['--auth-url', 'testurl'], + 'margs': {'auth_url': 'testurl', 'skip_os_cleanup': False}, + }), + ('auth_url_cleanup', { + 'argv': ['--auth-url', 'testurl', '--skip-os-cleanup'], + 'margs': {'auth_url': 'testurl', 'skip_os_cleanup': True}, + }), + ] + + margs = None + + def test_parser(self): + fixture = fixtures.MockPatch('fuel_ccp.cleanup.cleanup') + c_mock = self.useFixture(fixture).mock + self._run_app() + c_mock.assert_called_once_with(**self.margs) + + +class TestShowDep(TestParser): + cmd = 'show-dep' + scenarios = [ + ('one', {'argv': ['1'], 'components': ['1']}), + ('two', {'argv': ['1', '2'], 'components': ['1', '2']}), + ] + + components = None + + def test_parser(self): + fixture = fixtures.MockPatch('fuel_ccp.dependencies.show_dep') + d_mock = self.useFixture(fixture).mock + self._run_app() + d_mock.assert_called_once_with(self.components) + + +class ArgumentParserError(Exception): + pass + + +class TestGetConfigFile(testscenarios.WithScenarios, base.TestCase): + scenarios = [ + ('base', { + 'argv': ['--config-file', '/etc/ccp.yaml'], + 'expected_result': '/etc/ccp.yaml', + }), + ('missing', { + 'argv': ['--other-arg', 'smth'], + 'expected_result': None, + }), + ('with_extra', { + 'argv': ['--config-file', '/etc/ccp.yaml', '--other-arg', 'smth'], + 'expected_result': '/etc/ccp.yaml', + }), + ] + + argv = None + expected_result = None + + def test_get_cli_config(self): + self.useFixture(fixtures.MockPatch( + 'argparse.ArgumentParser.error', side_effect=ArgumentParserError)) + result = cli.CCPApp.get_config_file(self.argv) + self.assertEqual(result, self.expected_result) diff --git a/fuel_ccp/tests/test_config.py b/fuel_ccp/tests/test_config.py index 29d07bd0..29bb4fb9 100644 --- a/fuel_ccp/tests/test_config.py +++ b/fuel_ccp/tests/test_config.py @@ -1,43 +1,11 @@ -import fixtures import jsonschema import six -import testscenarios from fuel_ccp import config from fuel_ccp.config import _yaml from fuel_ccp.tests import base -class ArgumentParserError(Exception): - pass - - -class TestGetCLIConfig(testscenarios.WithScenarios, base.TestCase): - scenarios = [ - ('base', { - 'argv': ['--config-file', '/etc/ccp.yaml'], - 'expected_result': ('/etc/ccp.yaml', []), - }), - ('missing', { - 'argv': ['--other-arg', 'smth'], - 'expected_result': (None, ['--other-arg', 'smth']), - }), - ('with_extra', { - 'argv': ['--config-file', '/etc/ccp.yaml', '--other-arg', 'smth'], - 'expected_result': ('/etc/ccp.yaml', ['--other-arg', 'smth']), - }), - ] - - argv = None - expected_result = None - - def test_get_cli_config(self): - self.useFixture(fixtures.MockPatch( - 'argparse.ArgumentParser.error', side_effect=ArgumentParserError)) - result = config.get_cli_config(self.argv) - self.assertEqual(result, self.expected_result) - - def nested_dict_to_attrdict(d): if isinstance(d, dict): return _yaml.AttrDict({k: nested_dict_to_attrdict(v) diff --git a/requirements.txt b/requirements.txt index ea938ea4..8a5db577 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,7 @@ pbr>=1.6 +cliff!=1.16.0,!=1.17.0,>=1.15.0 # Apache-2.0 futures>=3.0 # BSD docker-py>=1.6.0,<1.8.0 # Apache-2.0 GitPython>=1.0.1 # BSD License (3 clause) diff --git a/setup.cfg b/setup.cfg index 76297523..108c6d2c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,6 +26,13 @@ packages = [entry_points] console_scripts = ccp = fuel_ccp.cli:main +ccp.cli = + build = fuel_ccp.cli:Build + cleanup = fuel_ccp.cli:Cleanup + deploy = fuel_ccp.cli:Deploy + fetch = fuel_ccp.cli:Fetch + show-dep = fuel_ccp.cli:ShowDep + validate = fuel_ccp.cli:Validate [build_sphinx] source-dir = doc/source diff --git a/tox.ini b/tox.ini index 3b1e1fc7..c577068b 100644 --- a/tox.ini +++ b/tox.ini @@ -1,7 +1,8 @@ [tox] minversion = 1.6 envlist = py35,py34,py27,pypy,pep8 -skipsdist = True +# We need to run sdist to reflect changes in entrypoints during test run +skipsdist = False [testenv] usedevelop = True