From 3388cf2c71deb56994670e64b22d0dc2d311461e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 30 Jul 2014 01:42:58 -0400 Subject: [PATCH 1/7] Resolves #369, add verbose output on --verbose flag Signed-off-by: Daniel Nephin --- fig/cli/command.py | 107 ++++++++++++++------------- fig/cli/docopt_command.py | 4 +- fig/cli/main.py | 66 +++++++++-------- fig/cli/utils.py | 19 ----- fig/cli/verbose_proxy.py | 58 +++++++++++++++ tests/integration/cli_test.py | 63 ++++++++-------- tests/unit/cli/__init__.py | 0 tests/unit/cli/verbose_proxy_test.py | 30 ++++++++ tests/unit/cli_test.py | 25 +++++-- 9 files changed, 235 insertions(+), 137 deletions(-) create mode 100644 fig/cli/verbose_proxy.py create mode 100644 tests/unit/cli/__init__.py create mode 100644 tests/unit/cli/verbose_proxy_test.py diff --git a/fig/cli/command.py b/fig/cli/command.py index 6efe83b6269..fd266d03bd4 100644 --- a/fig/cli/command.py +++ b/fig/cli/command.py @@ -12,9 +12,10 @@ from ..project import Project from ..service import ConfigError from .docopt_command import DocoptCommand -from .formatter import Formatter -from .utils import cached_property, docker_url, call_silently, is_mac, is_ubuntu +from .utils import docker_url, call_silently, is_mac, is_ubuntu +from . import verbose_proxy from . import errors +from .. import __version__ log = logging.getLogger(__name__) @@ -22,10 +23,6 @@ class Command(DocoptCommand): base_dir = '.' - def __init__(self): - self._yaml_path = os.environ.get('FIG_FILE', None) - self.explicit_project_name = None - def dispatch(self, *args, **kwargs): try: super(Command, self).dispatch(*args, **kwargs) @@ -40,60 +37,70 @@ def dispatch(self, *args, **kwargs): elif call_silently(['which', 'docker-osx']) == 0: raise errors.ConnectionErrorDockerOSX() else: - raise errors.ConnectionErrorGeneric(self.client.base_url) - - def perform_command(self, options, *args, **kwargs): - if options['--file'] is not None: - self.yaml_path = os.path.join(self.base_dir, options['--file']) - if options['--project-name'] is not None: - self.explicit_project_name = options['--project-name'] - return super(Command, self).perform_command(options, *args, **kwargs) - - @cached_property - def client(self): - return Client(docker_url()) - - @cached_property - def project(self): + raise errors.ConnectionErrorGeneric(self.get_client().base_url) + + def perform_command(self, options, handler, command_options): + explicit_config_path = options.get('--file') or os.environ.get('FIG_FILE') + project = self.get_project( + self.get_config_path(explicit_config_path), + project_name=options.get('--project-name'), + verbose=options.get('--verbose')) + + handler(project, command_options) + + def get_client(self, verbose=False): + client = Client(docker_url()) + if verbose: + version_info = six.iteritems(client.version()) + log.info("Fig version %s", __version__) + log.info("Docker base_url: %s", client.base_url) + log.info("Docker version: %s", + ", ".join("%s=%s" % item for item in version_info)) + return verbose_proxy.VerboseProxy('docker', client) + return client + + def get_config(self, config_path): try: - config = yaml.safe_load(open(self.yaml_path)) + with open(config_path, 'r') as fh: + return yaml.safe_load(fh) except IOError as e: if e.errno == errno.ENOENT: raise errors.FigFileNotFound(os.path.basename(e.filename)) raise errors.UserError(six.text_type(e)) + def get_project(self, config_path, project_name=None, verbose=False): try: - return Project.from_config(self.project_name, config, self.client) + return Project.from_config( + self.get_project_name(config_path, project_name), + self.get_config(config_path), + self.get_client(verbose=verbose)) except ConfigError as e: raise errors.UserError(six.text_type(e)) - @cached_property - def project_name(self): - project = os.path.basename(os.path.dirname(os.path.abspath(self.yaml_path))) - if self.explicit_project_name is not None: - project = self.explicit_project_name - project = re.sub(r'[^a-zA-Z0-9]', '', project) - if not project: - project = 'default' - return project - - @cached_property - def formatter(self): - return Formatter() - - @cached_property - def yaml_path(self): - if self._yaml_path is not None: - return self._yaml_path - elif os.path.exists(os.path.join(self.base_dir, 'fig.yaml')): - - log.warning("Fig just read the file 'fig.yaml' on startup, rather than 'fig.yml'") - log.warning("Please be aware that fig.yml the expected extension in most cases, and using .yaml can cause compatibility issues in future") + def get_project_name(self, config_path, project_name=None): + def normalize_name(name): + return re.sub(r'[^a-zA-Z0-9]', '', name) + + if project_name is not None: + return normalize_name(project_name) + + project = os.path.basename(os.path.dirname(os.path.abspath(config_path))) + if project: + return normalize_name(project) + + return 'default' + + def get_config_path(self, file_path=None): + if file_path: + return os.path.join(self.base_dir, file_path) + + if os.path.exists(os.path.join(self.base_dir, 'fig.yaml')): + log.warning("Fig just read the file 'fig.yaml' on startup, rather " + "than 'fig.yml'") + log.warning("Please be aware that fig.yml the expected extension " + "in most cases, and using .yaml can cause compatibility " + "issues in future") return os.path.join(self.base_dir, 'fig.yaml') - else: - return os.path.join(self.base_dir, 'fig.yml') - @yaml_path.setter - def yaml_path(self, value): - self._yaml_path = value + return os.path.join(self.base_dir, 'fig.yml') diff --git a/fig/cli/docopt_command.py b/fig/cli/docopt_command.py index cbb8e5303c8..8105d3b3feb 100644 --- a/fig/cli/docopt_command.py +++ b/fig/cli/docopt_command.py @@ -23,7 +23,7 @@ def sys_dispatch(self): def dispatch(self, argv, global_options): self.perform_command(*self.parse(argv, global_options)) - def perform_command(self, options, command, handler, command_options): + def perform_command(self, options, handler, command_options): handler(command_options) def parse(self, argv, global_options): @@ -43,7 +43,7 @@ def parse(self, argv, global_options): raise NoSuchCommand(command, self) command_options = docopt_full_help(docstring, options['ARGS'], options_first=True) - return (options, command, handler, command_options) + return options, handler, command_options class NoSuchCommand(Exception): diff --git a/fig/cli/main.py b/fig/cli/main.py index 1c9ff416a37..e85342dbf4d 100644 --- a/fig/cli/main.py +++ b/fig/cli/main.py @@ -98,7 +98,7 @@ def docopt_options(self): options['version'] = "fig %s" % __version__ return options - def build(self, options): + def build(self, project, options): """ Build or rebuild services. @@ -112,9 +112,9 @@ def build(self, options): --no-cache Do not use cache when building the image. """ no_cache = bool(options.get('--no-cache', False)) - self.project.build(service_names=options['SERVICE'], no_cache=no_cache) + project.build(service_names=options['SERVICE'], no_cache=no_cache) - def help(self, options): + def help(self, project, options): """ Get help on a command. @@ -125,25 +125,25 @@ def help(self, options): raise NoSuchCommand(command, self) raise SystemExit(getdoc(getattr(self, command))) - def kill(self, options): + def kill(self, project, options): """ Force stop service containers. Usage: kill [SERVICE...] """ - self.project.kill(service_names=options['SERVICE']) + project.kill(service_names=options['SERVICE']) - def logs(self, options): + def logs(self, project, options): """ View output from containers. Usage: logs [SERVICE...] """ - containers = self.project.containers(service_names=options['SERVICE'], stopped=True) + containers = project.containers(service_names=options['SERVICE'], stopped=True) print("Attaching to", list_containers(containers)) LogPrinter(containers, attach_params={'logs': True}).run() - def ps(self, options): + def ps(self, project, options): """ List containers. @@ -152,7 +152,7 @@ def ps(self, options): Options: -q Only display IDs """ - containers = self.project.containers(service_names=options['SERVICE'], stopped=True) + self.project.containers(service_names=options['SERVICE'], one_off=True) + containers = project.containers(service_names=options['SERVICE'], stopped=True) + project.containers(service_names=options['SERVICE'], one_off=True) if options['-q']: for container in containers: @@ -177,7 +177,7 @@ def ps(self, options): ]) print(Formatter().table(headers, rows)) - def rm(self, options): + def rm(self, project, options): """ Remove stopped service containers. @@ -187,21 +187,21 @@ def rm(self, options): --force Don't ask to confirm removal -v Remove volumes associated with containers """ - all_containers = self.project.containers(service_names=options['SERVICE'], stopped=True) + all_containers = project.containers(service_names=options['SERVICE'], stopped=True) stopped_containers = [c for c in all_containers if not c.is_running] if len(stopped_containers) > 0: print("Going to remove", list_containers(stopped_containers)) if options.get('--force') \ or yesno("Are you sure? [yN] ", default=False): - self.project.remove_stopped( + project.remove_stopped( service_names=options['SERVICE'], v=options.get('-v', False) ) else: print("No stopped containers") - def run(self, options): + def run(self, project, options): """ Run a one-off command on a service. @@ -223,14 +223,13 @@ def run(self, options): --rm Remove container after run. Ignored in detached mode. --no-deps Don't start linked services. """ - - service = self.project.get_service(options['SERVICE']) + service = project.get_service(options['SERVICE']) if not options['--no-deps']: deps = service.get_linked_names() if len(deps) > 0: - self.project.up( + project.up( service_names=deps, start_links=True, recreate=False, @@ -256,14 +255,14 @@ def run(self, options): print(container.name) else: service.start_container(container, ports=None, one_off=True) - dockerpty.start(self.client, container.id) + dockerpty.start(project.client, container.id) exit_code = container.wait() if options['--rm']: log.info("Removing %s..." % container.name) - self.client.remove_container(container.id) + project.client.remove_container(container.id) sys.exit(exit_code) - def scale(self, options): + def scale(self, project, options): """ Set number of containers to run for a service. @@ -284,19 +283,24 @@ def scale(self, options): raise UserError('Number of containers for service "%s" is not a ' 'number' % service_name) try: - self.project.get_service(service_name).scale(num) + project.get_service(service_name).scale(num) except CannotBeScaledError: - raise UserError('Service "%s" cannot be scaled because it specifies a port on the host. If multiple containers for this service were created, the port would clash.\n\nRemove the ":" from the port definition in fig.yml so Docker can choose a random port for each container.' % service_name) - - def start(self, options): + raise UserError( + 'Service "%s" cannot be scaled because it specifies a port ' + 'on the host. If multiple containers for this service were ' + 'created, the port would clash.\n\nRemove the ":" from the ' + 'port definition in fig.yml so Docker can choose a random ' + 'port for each container.' % service_name) + + def start(self, project, options): """ Start existing containers. Usage: start [SERVICE...] """ - self.project.start(service_names=options['SERVICE']) + project.start(service_names=options['SERVICE']) - def stop(self, options): + def stop(self, project, options): """ Stop running containers without removing them. @@ -304,9 +308,9 @@ def stop(self, options): Usage: stop [SERVICE...] """ - self.project.stop(service_names=options['SERVICE']) + project.stop(service_names=options['SERVICE']) - def up(self, options): + def up(self, project, options): """ Build, (re)create, start and attach to containers for a service. @@ -334,13 +338,13 @@ def up(self, options): recreate = not options['--no-recreate'] service_names = options['SERVICE'] - self.project.up( + project.up( service_names=service_names, start_links=start_links, recreate=recreate ) - to_attach = [c for s in self.project.get_services(service_names) for c in s.containers()] + to_attach = [c for s in project.get_services(service_names) for c in s.containers()] if not detached: print("Attaching to", list_containers(to_attach)) @@ -350,12 +354,12 @@ def up(self, options): log_printer.run() finally: def handler(signal, frame): - self.project.kill(service_names=service_names) + project.kill(service_names=service_names) sys.exit(0) signal.signal(signal.SIGINT, handler) print("Gracefully stopping... (press Ctrl+C again to force)") - self.project.stop(service_names=service_names) + project.stop(service_names=service_names) def list_containers(containers): diff --git a/fig/cli/utils.py b/fig/cli/utils.py index cc9435bcfac..af16e244916 100644 --- a/fig/cli/utils.py +++ b/fig/cli/utils.py @@ -7,25 +7,6 @@ import platform -def cached_property(f): - """ - returns a cached property that is calculated by function f - http://code.activestate.com/recipes/576563-cached-property/ - """ - def get(self): - try: - return self._property_cache[f] - except AttributeError: - self._property_cache = {} - x = self._property_cache[f] = f(self) - return x - except KeyError: - x = self._property_cache[f] = f(self) - return x - - return property(get) - - def yesno(prompt, default=None): """ Prompt the user for a yes or no. diff --git a/fig/cli/verbose_proxy.py b/fig/cli/verbose_proxy.py new file mode 100644 index 00000000000..939cc532099 --- /dev/null +++ b/fig/cli/verbose_proxy.py @@ -0,0 +1,58 @@ + +import functools +from itertools import chain +import logging +import pprint + +from fig.packages import six + + +def format_call(args, kwargs): + args = (repr(a) for a in args) + kwargs = ("{0!s}={1!r}".format(*item) for item in six.iteritems(kwargs)) + return "({0})".format(", ".join(chain(args, kwargs))) + + +def format_return(result, max_lines): + if isinstance(result, (list, tuple, set)): + return "({0} with {1} items)".format(type(result).__name__, len(result)) + + if result: + lines = pprint.pformat(result).split('\n') + extra = '\n...' if len(lines) > max_lines else '' + return '\n'.join(lines[:max_lines]) + extra + + return result + + +class VerboseProxy(object): + """Proxy all function calls to another class and log method name, arguments + and return values for each call. + """ + + def __init__(self, obj_name, obj, log_name=None, max_lines=10): + self.obj_name = obj_name + self.obj = obj + self.max_lines = max_lines + self.log = logging.getLogger(log_name or __name__) + + def __getattr__(self, name): + attr = getattr(self.obj, name) + + if not six.callable(attr): + return attr + + return functools.partial(self.proxy_callable, name) + + def proxy_callable(self, call_name, *args, **kwargs): + self.log.info("%s %s <- %s", + self.obj_name, + call_name, + format_call(args, kwargs)) + + result = getattr(self.obj, call_name)(*args, **kwargs) + self.log.info("%s %s -> %s", + self.obj_name, + call_name, + format_return(result, self.max_lines)) + return result diff --git a/tests/integration/cli_test.py b/tests/integration/cli_test.py index 931b867a0c4..d0c8585ea36 100644 --- a/tests/integration/cli_test.py +++ b/tests/integration/cli_test.py @@ -5,6 +5,7 @@ from fig.packages.six import StringIO import sys + class CLITestCase(DockerClientTestCase): def setUp(self): super(CLITestCase, self).setUp() @@ -15,12 +16,16 @@ def setUp(self): def tearDown(self): sys.exit = self.old_sys_exit - self.command.project.kill() - self.command.project.remove_stopped() + self.project.kill() + self.project.remove_stopped() + + @property + def project(self): + return self.command.get_project(self.command.get_config_path()) @patch('sys.stdout', new_callable=StringIO) def test_ps(self, mock_stdout): - self.command.project.get_service('simple').create_container() + self.project.get_service('simple').create_container() self.command.dispatch(['ps'], None) self.assertIn('simplefigfile_simple_1', mock_stdout.getvalue()) @@ -64,17 +69,17 @@ def test_build_no_cache(self, mock_stdout): def test_up(self): self.command.dispatch(['up', '-d'], None) - service = self.command.project.get_service('simple') - another = self.command.project.get_service('another') + service = self.project.get_service('simple') + another = self.project.get_service('another') self.assertEqual(len(service.containers()), 1) self.assertEqual(len(another.containers()), 1) def test_up_with_links(self): self.command.base_dir = 'tests/fixtures/links-figfile' self.command.dispatch(['up', '-d', 'web'], None) - web = self.command.project.get_service('web') - db = self.command.project.get_service('db') - console = self.command.project.get_service('console') + web = self.project.get_service('web') + db = self.project.get_service('db') + console = self.project.get_service('console') self.assertEqual(len(web.containers()), 1) self.assertEqual(len(db.containers()), 1) self.assertEqual(len(console.containers()), 0) @@ -82,16 +87,16 @@ def test_up_with_links(self): def test_up_with_no_deps(self): self.command.base_dir = 'tests/fixtures/links-figfile' self.command.dispatch(['up', '-d', '--no-deps', 'web'], None) - web = self.command.project.get_service('web') - db = self.command.project.get_service('db') - console = self.command.project.get_service('console') + web = self.project.get_service('web') + db = self.project.get_service('db') + console = self.project.get_service('console') self.assertEqual(len(web.containers()), 1) self.assertEqual(len(db.containers()), 0) self.assertEqual(len(console.containers()), 0) def test_up_with_recreate(self): self.command.dispatch(['up', '-d'], None) - service = self.command.project.get_service('simple') + service = self.project.get_service('simple') self.assertEqual(len(service.containers()), 1) old_ids = [c.id for c in service.containers()] @@ -105,7 +110,7 @@ def test_up_with_recreate(self): def test_up_with_keep_old(self): self.command.dispatch(['up', '-d'], None) - service = self.command.project.get_service('simple') + service = self.project.get_service('simple') self.assertEqual(len(service.containers()), 1) old_ids = [c.id for c in service.containers()] @@ -117,19 +122,18 @@ def test_up_with_keep_old(self): self.assertEqual(old_ids, new_ids) - @patch('dockerpty.start') def test_run_service_without_links(self, mock_stdout): self.command.base_dir = 'tests/fixtures/links-figfile' self.command.dispatch(['run', 'console', '/bin/true'], None) - self.assertEqual(len(self.command.project.containers()), 0) + self.assertEqual(len(self.project.containers()), 0) @patch('dockerpty.start') def test_run_service_with_links(self, __): self.command.base_dir = 'tests/fixtures/links-figfile' self.command.dispatch(['run', 'web', '/bin/true'], None) - db = self.command.project.get_service('db') - console = self.command.project.get_service('console') + db = self.project.get_service('db') + console = self.project.get_service('console') self.assertEqual(len(db.containers()), 1) self.assertEqual(len(console.containers()), 0) @@ -137,14 +141,14 @@ def test_run_service_with_links(self, __): def test_run_with_no_deps(self, __): self.command.base_dir = 'tests/fixtures/links-figfile' self.command.dispatch(['run', '--no-deps', 'web', '/bin/true'], None) - db = self.command.project.get_service('db') + db = self.project.get_service('db') self.assertEqual(len(db.containers()), 0) @patch('dockerpty.start') def test_run_does_not_recreate_linked_containers(self, __): self.command.base_dir = 'tests/fixtures/links-figfile' self.command.dispatch(['up', '-d', 'db'], None) - db = self.command.project.get_service('db') + db = self.project.get_service('db') self.assertEqual(len(db.containers()), 1) old_ids = [c.id for c in db.containers()] @@ -161,11 +165,11 @@ def test_run_without_command(self, __): self.command.base_dir = 'tests/fixtures/commands-figfile' self.client.build('tests/fixtures/simple-dockerfile', tag='figtest_test') - for c in self.command.project.containers(stopped=True, one_off=True): + for c in self.project.containers(stopped=True, one_off=True): c.remove() self.command.dispatch(['run', 'implicit'], None) - service = self.command.project.get_service('implicit') + service = self.project.get_service('implicit') containers = service.containers(stopped=True, one_off=True) self.assertEqual( [c.human_readable_command for c in containers], @@ -173,7 +177,7 @@ def test_run_without_command(self, __): ) self.command.dispatch(['run', 'explicit'], None) - service = self.command.project.get_service('explicit') + service = self.project.get_service('explicit') containers = service.containers(stopped=True, one_off=True) self.assertEqual( [c.human_readable_command for c in containers], @@ -181,7 +185,7 @@ def test_run_without_command(self, __): ) def test_rm(self): - service = self.command.project.get_service('simple') + service = self.project.get_service('simple') service.create_container() service.kill() self.assertEqual(len(service.containers(stopped=True)), 1) @@ -189,24 +193,23 @@ def test_rm(self): self.assertEqual(len(service.containers(stopped=True)), 0) def test_scale(self): - project = self.command.project + project = self.project - self.command.scale({'SERVICE=NUM': ['simple=1']}) + self.command.scale(project, {'SERVICE=NUM': ['simple=1']}) self.assertEqual(len(project.get_service('simple').containers()), 1) - self.command.scale({'SERVICE=NUM': ['simple=3', 'another=2']}) + self.command.scale(project, {'SERVICE=NUM': ['simple=3', 'another=2']}) self.assertEqual(len(project.get_service('simple').containers()), 3) self.assertEqual(len(project.get_service('another').containers()), 2) - self.command.scale({'SERVICE=NUM': ['simple=1', 'another=1']}) + self.command.scale(project, {'SERVICE=NUM': ['simple=1', 'another=1']}) self.assertEqual(len(project.get_service('simple').containers()), 1) self.assertEqual(len(project.get_service('another').containers()), 1) - self.command.scale({'SERVICE=NUM': ['simple=1', 'another=1']}) + self.command.scale(project, {'SERVICE=NUM': ['simple=1', 'another=1']}) self.assertEqual(len(project.get_service('simple').containers()), 1) self.assertEqual(len(project.get_service('another').containers()), 1) - self.command.scale({'SERVICE=NUM': ['simple=0', 'another=0']}) + self.command.scale(project, {'SERVICE=NUM': ['simple=0', 'another=0']}) self.assertEqual(len(project.get_service('simple').containers()), 0) self.assertEqual(len(project.get_service('another').containers()), 0) - diff --git a/tests/unit/cli/__init__.py b/tests/unit/cli/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/unit/cli/verbose_proxy_test.py b/tests/unit/cli/verbose_proxy_test.py new file mode 100644 index 00000000000..b17c610db83 --- /dev/null +++ b/tests/unit/cli/verbose_proxy_test.py @@ -0,0 +1,30 @@ +from __future__ import unicode_literals +from __future__ import absolute_import +from tests import unittest + +from fig.cli import verbose_proxy + + +class VerboseProxy(unittest.TestCase): + + def test_format_call(self): + expected = "(u'arg1', True, key=u'value')" + actual = verbose_proxy.format_call( + ("arg1", True), + {'key': 'value'}) + + self.assertEqual(expected, actual) + + def test_format_return_sequence(self): + expected = "(list with 10 items)" + actual = verbose_proxy.format_return(list(range(10)), 2) + self.assertEqual(expected, actual) + + def test_format_return(self): + expected = "{u'Id': u'ok'}" + actual = verbose_proxy.format_return({'Id': 'ok'}, 2) + self.assertEqual(expected, actual) + + def test_format_return_no_result(self): + actual = verbose_proxy.format_return(None, 2) + self.assertEqual(None, actual) diff --git a/tests/unit/cli_test.py b/tests/unit/cli_test.py index c69235793d3..488e78926e0 100644 --- a/tests/unit/cli_test.py +++ b/tests/unit/cli_test.py @@ -4,6 +4,8 @@ import os from .. import unittest +import mock + from fig.cli import main from fig.cli.main import TopLevelCommand from fig.packages.six import StringIO @@ -16,24 +18,37 @@ def test_default_project_name(self): try: os.chdir('tests/fixtures/simple-figfile') command = TopLevelCommand() - self.assertEquals('simplefigfile', command.project_name) + project_name = command.get_project_name(command.get_config_path()) + self.assertEquals('simplefigfile', project_name) finally: os.chdir(cwd) def test_project_name_with_explicit_base_dir(self): command = TopLevelCommand() command.base_dir = 'tests/fixtures/simple-figfile' - self.assertEquals('simplefigfile', command.project_name) + project_name = command.get_project_name(command.get_config_path()) + self.assertEquals('simplefigfile', project_name) def test_project_name_with_explicit_project_name(self): command = TopLevelCommand() - command.explicit_project_name = 'explicit-project-name' - self.assertEquals('explicitprojectname', command.project_name) + name = 'explicit-project-name' + project_name = command.get_project_name(None, project_name=name) + self.assertEquals('explicitprojectname', project_name) def test_yaml_filename_check(self): command = TopLevelCommand() command.base_dir = 'tests/fixtures/longer-filename-figfile' - self.assertTrue(command.project.get_service('definedinyamlnotyml')) + with mock.patch('fig.cli.command.log', autospec=True) as mock_log: + self.assertTrue(command.get_config_path()) + self.assertEqual(mock_log.warning.call_count, 2) + + def test_get_project(self): + command = TopLevelCommand() + command.base_dir = 'tests/fixtures/longer-filename-figfile' + project = command.get_project(command.get_config_path()) + self.assertEqual(project.name, 'longerfilenamefigfile') + self.assertTrue(project.client) + self.assertTrue(project.services) def test_help(self): command = TopLevelCommand() From f53e698060b5a4337ea7b228421e98d54f014258 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sat, 2 Aug 2014 16:31:08 -0400 Subject: [PATCH 2/7] Remove extra calls to docker server. --- fig/container.py | 8 +++----- fig/project.py | 12 ++++++------ fig/service.py | 20 +++++++++++--------- tests/integration/service_test.py | 4 +++- tests/unit/container_test.py | 18 ++++++++++++++++++ 5 files changed, 41 insertions(+), 21 deletions(-) diff --git a/fig/container.py b/fig/container.py index e9bbd59d359..dc3366a6ea2 100644 --- a/fig/container.py +++ b/fig/container.py @@ -97,11 +97,8 @@ def human_readable_command(self): @property def environment(self): self.inspect_if_not_inspected() - out = {} - for var in self.dictionary.get('Config', {}).get('Env', []): - k, v = var.split('=', 1) - out[k] = v - return out + return dict(var.split("=", 1) + for var in self.dictionary.get('Config', {}).get('Env', [])) @property def is_running(self): @@ -132,6 +129,7 @@ def logs(self, *args, **kwargs): def inspect(self): self.dictionary = self.client.inspect_container(self.id) + self.has_been_inspected = True return self.dictionary def links(self): diff --git a/fig/project.py b/fig/project.py index 614b04899d6..d0c556c487c 100644 --- a/fig/project.py +++ b/fig/project.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals from __future__ import absolute_import import logging + from .service import Service from .container import Container from .packages.docker.errors import APIError @@ -179,12 +180,11 @@ def remove_stopped(self, service_names=None, **options): for service in self.get_services(service_names): service.remove_stopped(**options) - def containers(self, service_names=None, *args, **kwargs): - l = [] - for service in self.get_services(service_names): - for container in service.containers(*args, **kwargs): - l.append(container) - return l + def containers(self, service_names=None, stopped=False, one_off=False): + return [Container.from_ps(self.client, container) + for container in self.client.containers(all=stopped) + for service in self.get_services(service_names) + if service.has_container(container, one_off=one_off)] def _inject_links(self, acc, service): linked_names = service.get_linked_names() diff --git a/fig/service.py b/fig/service.py index 65bcf51972b..48f63e6a5b2 100644 --- a/fig/service.py +++ b/fig/service.py @@ -65,15 +65,17 @@ def __init__(self, name, client=None, project='default', links=None, volumes_fro self.options = options def containers(self, stopped=False, one_off=False): - l = [] - for container in self.client.containers(all=stopped): - name = get_container_name(container) - if not name or not is_valid_name(name, one_off): - continue - project, name, number = parse_name(name) - if project == self.project and name == self.name: - l.append(Container.from_ps(self.client, container)) - return l + return [Container.from_ps(self.client, container) + for container in self.client.containers(all=stopped) + if self.has_container(container, one_off=one_off)] + + def has_container(self, container, one_off=False): + """Return True if `container` was created to fulfill this service.""" + name = get_container_name(container) + if not name or not is_valid_name(name, one_off): + return False + project, name, number = parse_name(name) + return project == self.project and name == self.name def start(self, **options): for c in self.containers(stopped=True): diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index 75275d8a70b..eb8b91b6130 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -143,7 +143,9 @@ def test_recreate_containers(self): self.assertEqual(len(self.client.containers(all=True)), num_containers_before) self.assertNotEqual(old_container.id, new_container.id) - self.assertRaises(APIError, lambda: self.client.inspect_container(intermediate_container.id)) + self.assertRaises(APIError, + self.client.inspect_container, + intermediate_container.id) def test_recreate_containers_when_containers_are_stopped(self): service = self.create_service( diff --git a/tests/unit/container_test.py b/tests/unit/container_test.py index 6e5b9884e27..9f48618913e 100644 --- a/tests/unit/container_test.py +++ b/tests/unit/container_test.py @@ -1,7 +1,12 @@ from __future__ import unicode_literals from .. import unittest + +import mock +from fig.packages import docker + from fig.container import Container + class ContainerTest(unittest.TestCase): def test_from_ps(self): container = Container.from_ps(None, { @@ -67,3 +72,16 @@ def test_name_without_project(self): "Names":["/figtest_db_1"] }, has_been_inspected=True) self.assertEqual(container.name_without_project, "db_1") + + def test_inspect_if_not_inspected(self): + mock_client = mock.create_autospec(docker.Client) + container = Container(mock_client, dict(Id="the_id")) + + container.inspect_if_not_inspected() + mock_client.inspect_container.assert_called_once_with("the_id") + self.assertEqual(container.dictionary, + mock_client.inspect_container.return_value) + self.assertTrue(container.has_been_inspected) + + container.inspect_if_not_inspected() + self.assertEqual(mock_client.inspect_container.call_count, 1) From 9ba38b275acb38e0a6552d46379df40e13a3d293 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Sat, 2 Aug 2014 17:51:01 -0400 Subject: [PATCH 3/7] Fix broken integration tests Signed-off-by: Daniel Nephin --- tests/integration/project_test.py | 15 +++++++++------ tests/integration/service_test.py | 4 +++- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/integration/project_test.py b/tests/integration/project_test.py index 83a988a14a5..a9123af85a2 100644 --- a/tests/integration/project_test.py +++ b/tests/integration/project_test.py @@ -108,8 +108,9 @@ def test_project_up_recreates_containers(self): self.assertEqual(len(project.containers()), 2) db_container = [c for c in project.containers() if 'db' in c.name][0] - self.assertNotEqual(c.id, old_db_id) - self.assertEqual(c.inspect()['Volumes']['/var/db'], db_volume_path) + self.assertNotEqual(db_container.id, old_db_id) + self.assertEqual(db_container.inspect()['Volumes']['/var/db'], + db_volume_path) project.kill() project.remove_stopped() @@ -130,8 +131,9 @@ def test_project_up_with_no_recreate_running(self): self.assertEqual(len(project.containers()), 2) db_container = [c for c in project.containers() if 'db' in c.name][0] - self.assertEqual(c.id, old_db_id) - self.assertEqual(c.inspect()['Volumes']['/var/db'], db_volume_path) + self.assertEqual(db_container.id, old_db_id) + self.assertEqual(db_container.inspect()['Volumes']['/var/db'], + db_volume_path) project.kill() project.remove_stopped() @@ -158,8 +160,9 @@ def test_project_up_with_no_recreate_stopped(self): self.assertEqual(len(new_containers), 2) db_container = [c for c in new_containers if 'db' in c.name][0] - self.assertEqual(c.id, old_db_id) - self.assertEqual(c.inspect()['Volumes']['/var/db'], db_volume_path) + self.assertEqual(db_container.id, old_db_id) + self.assertEqual(db_container.inspect()['Volumes']['/var/db'], + db_volume_path) project.kill() project.remove_stopped() diff --git a/tests/integration/service_test.py b/tests/integration/service_test.py index eb8b91b6130..6dc551a8dc4 100644 --- a/tests/integration/service_test.py +++ b/tests/integration/service_test.py @@ -1,11 +1,13 @@ from __future__ import unicode_literals from __future__ import absolute_import +import os + from fig import Service from fig.service import CannotBeScaledError from fig.container import Container from fig.packages.docker.errors import APIError from .testcases import DockerClientTestCase -import os + class ServiceTest(DockerClientTestCase): def test_containers(self): From bc48e65d7ca33bb5f5d92b9ab65672ef98044495 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 8 Aug 2014 09:41:52 -0700 Subject: [PATCH 4/7] Add Service.get_container --- fig/service.py | 13 +++++++++++++ tests/unit/service_test.py | 28 ++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/fig/service.py b/fig/service.py index 48f63e6a5b2..2676ce4e0af 100644 --- a/fig/service.py +++ b/fig/service.py @@ -77,6 +77,19 @@ def has_container(self, container, one_off=False): project, name, number = parse_name(name) return project == self.project and name == self.name + def get_container(self, number=0): + """Return a :class:`fig.container.Container` for this service. The + container must be active, and match `number`. + """ + for container in self.client.containers(): + if not self.has_container(container): + continue + _, _, container_number = parse_name(get_container_name(container)) + if container_number == number: + return Container.from_ps(self.client, container) + + raise ValueError("No container found for %s_%s", self.name, number) + def start(self, **options): for c in self.containers(stopped=True): self.start_container_if_stopped(c, **options) diff --git a/tests/unit/service_test.py b/tests/unit/service_test.py index 9cdc5081bde..a3650fa41ee 100644 --- a/tests/unit/service_test.py +++ b/tests/unit/service_test.py @@ -1,9 +1,14 @@ from __future__ import unicode_literals from __future__ import absolute_import from .. import unittest + +import mock +from fig.packages import docker + from fig import Service from fig.service import ConfigError, split_port + class ServiceTest(unittest.TestCase): def test_name_validations(self): self.assertRaises(ConfigError, lambda: Service(name='')) @@ -75,10 +80,29 @@ def test_split_domainname_both(self): def test_split_domainname_weird(self): service = Service('foo', - hostname = 'name.sub', - domainname = 'domain.tld', + hostname='name.sub', + domainname='domain.tld', ) service.next_container_name = lambda x: 'foo' opts = service._get_container_create_options({}) self.assertEqual(opts['hostname'], 'name.sub', 'hostname') self.assertEqual(opts['domainname'], 'domain.tld', 'domainname') + + def test_get_container_not_found(self): + mock_client = mock.create_autospec(docker.Client) + mock_client.containers.return_value = [] + service = Service('foo', client=mock_client) + + self.assertRaises(ValueError, service.get_container) + + @mock.patch('fig.service.Container', autospec=True) + def test_get_container(self, mock_container_class): + mock_client = mock.create_autospec(docker.Client) + container_dict = dict(Name='default_foo_2') + mock_client.containers.return_value = [container_dict] + service = Service('foo', client=mock_client) + + container = service.get_container(number=2) + self.assertEqual(container, mock_container_class.from_ps.return_value) + mock_container_class.from_ps.assert_called_once_with( + mock_client, container_dict) From fb84eccaa024b13a4fe8a6bfcf5ea56333893287 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 8 Aug 2014 13:50:17 -0700 Subject: [PATCH 5/7] Add a new fig command for retrieving the locally bound port of a service. --- fig/cli/main.py | 20 +++++++++ fig/container.py | 31 ++++++++----- fig/service.py | 2 +- tests/unit/container_test.py | 86 ++++++++++++++++++++++-------------- 4 files changed, 94 insertions(+), 45 deletions(-) diff --git a/fig/cli/main.py b/fig/cli/main.py index e85342dbf4d..a8734500648 100644 --- a/fig/cli/main.py +++ b/fig/cli/main.py @@ -143,6 +143,26 @@ def logs(self, project, options): print("Attaching to", list_containers(containers)) LogPrinter(containers, attach_params={'logs': True}).run() + def local_port(self, project, options): + """ + Print the local port for a port binding. + + Usage: local_port [options] SERVICE PORT + + Options: + --protocol=proto tcp or udp (defaults to tcp) + --index=index index of the container if there are multiple + instances of a service (defaults to 1) + """ + service = project.get_service(options['SERVICE']) + try: + container = service.get_container(number=options.get('--index') or 1) + except ValueError as e: + raise UserError(str(e)) + print(container.get_local_port( + options['PORT'], + protocol=options.get('--protocol') or 'tcp') or '') + def ps(self, project, options): """ List containers. diff --git a/fig/container.py b/fig/container.py index dc3366a6ea2..5b78215a3f0 100644 --- a/fig/container.py +++ b/fig/container.py @@ -1,6 +1,8 @@ from __future__ import unicode_literals from __future__ import absolute_import +from fig.packages import six + class Container(object): """ @@ -63,17 +65,19 @@ def number(self): return None @property - def human_readable_ports(self): + def ports(self): self.inspect_if_not_inspected() - if not self.dictionary['NetworkSettings']['Ports']: - return '' - ports = [] - for private, public in list(self.dictionary['NetworkSettings']['Ports'].items()): - if public: - ports.append('%s->%s' % (public[0]['HostPort'], private)) - else: - ports.append(private) - return ', '.join(ports) + return self.dictionary['NetworkSettings']['Ports'] or {} + + @property + def human_readable_ports(self): + def format_port(private, public): + if not public: + return private + return '%s->%s' % (public[0]['HostPort'], private) + + return ', '.join(format_port(*item) + for item in sorted(six.iteritems(self.ports))) @property def human_readable_state(self): @@ -105,6 +109,13 @@ def is_running(self): self.inspect_if_not_inspected() return self.dictionary['State']['Running'] + def get_local_port(self, port, protocol='tcp'): + self.inspect_if_not_inspected() + port = self.ports.get("%s/%s" % (port, protocol)) + if not port: + return None + return int(port[0]['HostPort']) + def start(self, **options): return self.client.start(self.id, **options) diff --git a/fig/service.py b/fig/service.py index 2676ce4e0af..c1745a4f3b9 100644 --- a/fig/service.py +++ b/fig/service.py @@ -88,7 +88,7 @@ def get_container(self, number=0): if container_number == number: return Container.from_ps(self.client, container) - raise ValueError("No container found for %s_%s", self.name, number) + raise ValueError("No container found for %s_%s" % (self.name, number)) def start(self, **options): for c in self.containers(stopped=True): diff --git a/tests/unit/container_test.py b/tests/unit/container_test.py index 9f48618913e..61aa2a99169 100644 --- a/tests/unit/container_test.py +++ b/tests/unit/container_test.py @@ -8,18 +8,28 @@ class ContainerTest(unittest.TestCase): + + + def setUp(self): + self.container_dict = { + "Id": "abc", + "Image": "busybox:latest", + "Command": "sleep 300", + "Created": 1387384730, + "Status": "Up 8 seconds", + "Ports": None, + "SizeRw": 0, + "SizeRootFs": 0, + "Names": ["/figtest_db_1"], + "NetworkSettings": { + "Ports": {}, + }, + } + def test_from_ps(self): - container = Container.from_ps(None, { - "Id":"abc", - "Image":"busybox:latest", - "Command":"sleep 300", - "Created":1387384730, - "Status":"Up 8 seconds", - "Ports":None, - "SizeRw":0, - "SizeRootFs":0, - "Names":["/figtest_db_1"] - }, has_been_inspected=True) + container = Container.from_ps(None, + self.container_dict, + has_been_inspected=True) self.assertEqual(container.dictionary, { "Id": "abc", "Image":"busybox:latest", @@ -42,35 +52,21 @@ def test_environment(self): }) def test_number(self): - container = Container.from_ps(None, { - "Id":"abc", - "Image":"busybox:latest", - "Command":"sleep 300", - "Created":1387384730, - "Status":"Up 8 seconds", - "Ports":None, - "SizeRw":0, - "SizeRootFs":0, - "Names":["/figtest_db_1"] - }, has_been_inspected=True) + container = Container.from_ps(None, + self.container_dict, + has_been_inspected=True) self.assertEqual(container.number, 1) def test_name(self): - container = Container.from_ps(None, { - "Id":"abc", - "Image":"busybox:latest", - "Command":"sleep 300", - "Names":["/figtest_db_1"] - }, has_been_inspected=True) + container = Container.from_ps(None, + self.container_dict, + has_been_inspected=True) self.assertEqual(container.name, "figtest_db_1") def test_name_without_project(self): - container = Container.from_ps(None, { - "Id":"abc", - "Image":"busybox:latest", - "Command":"sleep 300", - "Names":["/figtest_db_1"] - }, has_been_inspected=True) + container = Container.from_ps(None, + self.container_dict, + has_been_inspected=True) self.assertEqual(container.name_without_project, "db_1") def test_inspect_if_not_inspected(self): @@ -85,3 +81,25 @@ def test_inspect_if_not_inspected(self): container.inspect_if_not_inspected() self.assertEqual(mock_client.inspect_container.call_count, 1) + + def test_human_readable_ports_none(self): + container = Container(None, self.container_dict, has_been_inspected=True) + self.assertEqual(container.human_readable_ports, '') + + def test_human_readable_ports_public_and_private(self): + self.container_dict['NetworkSettings']['Ports'].update({ + "45454/tcp": [ { "HostIp": "0.0.0.0", "HostPort": "49197" } ], + "45453/tcp": [], + }) + container = Container(None, self.container_dict, has_been_inspected=True) + + expected = "45453/tcp, 49197->45454/tcp" + self.assertEqual(container.human_readable_ports, expected) + + def test_get_local_port(self): + self.container_dict['NetworkSettings']['Ports'].update({ + "45454/tcp": [ { "HostIp": "0.0.0.0", "HostPort": "49197" } ], + }) + container = Container(None, self.container_dict, has_been_inspected=True) + + self.assertEqual(container.get_local_port(45454, protocol='tcp'), 49197) From df5da8d8a00cec07fdb03b27906f6266b8415b2b Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 11 Aug 2014 10:42:17 -0700 Subject: [PATCH 6/7] Fix default container number. --- fig/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fig/service.py b/fig/service.py index c1745a4f3b9..8e3b88dad85 100644 --- a/fig/service.py +++ b/fig/service.py @@ -77,7 +77,7 @@ def has_container(self, container, one_off=False): project, name, number = parse_name(name) return project == self.project and name == self.name - def get_container(self, number=0): + def get_container(self, number=1): """Return a :class:`fig.container.Container` for this service. The container must be active, and match `number`. """ From 2bd26b30ae95a838c5f56e70c9c71926732694fd Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Mon, 11 Aug 2014 11:27:22 -0700 Subject: [PATCH 7/7] Prefix number with underscore so it's clear it is not used. --- fig/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fig/service.py b/fig/service.py index 8e3b88dad85..78ca481bff9 100644 --- a/fig/service.py +++ b/fig/service.py @@ -74,7 +74,7 @@ def has_container(self, container, one_off=False): name = get_container_name(container) if not name or not is_valid_name(name, one_off): return False - project, name, number = parse_name(name) + project, name, _number = parse_name(name) return project == self.project and name == self.name def get_container(self, number=1):