Merge pull request #1643 from aanand/warn-about-legacy-one-off-containers

Show an error on 'run' when there are legacy one-off containers
This commit is contained in:
Aanand Prasad 2015-07-08 14:48:03 +01:00
commit 81707ef1ad
6 changed files with 221 additions and 38 deletions

View File

@ -34,7 +34,7 @@ def main():
except KeyboardInterrupt:
log.error("\nAborting.")
sys.exit(1)
except (UserError, NoSuchService, ConfigurationError, legacy.LegacyContainersError) as e:
except (UserError, NoSuchService, ConfigurationError, legacy.LegacyError) as e:
log.error(e.msg)
sys.exit(1)
except NoSuchCommand as e:
@ -336,12 +336,22 @@ class TopLevelCommand(Command):
if not options['--service-ports']:
container_options['ports'] = []
container = service.create_container(
quiet=True,
one_off=True,
insecure_registry=insecure_registry,
**container_options
)
try:
container = service.create_container(
quiet=True,
one_off=True,
insecure_registry=insecure_registry,
**container_options
)
except APIError as e:
legacy.check_for_legacy_containers(
project.client,
project.name,
[service.name],
allow_one_off=False,
)
raise e
if options['-d']:
service.start_container(container)

View File

@ -1,6 +1,7 @@
import logging
import re
from .const import LABEL_VERSION
from .container import get_container_name, Container
@ -24,41 +25,82 @@ Alternatively, remove them:
$ docker rm -f {rm_args}
"""
ONE_OFF_ADDENDUM_FORMAT = """
You should also remove your one-off containers:
$ docker rm -f {rm_args}
"""
ONE_OFF_ERROR_MESSAGE_FORMAT = """
Compose found the following containers without labels:
{names_list}
As of Compose 1.3.0, containers are identified with labels instead of naming convention.
Remove them before continuing:
$ docker rm -f {rm_args}
"""
def check_for_legacy_containers(
client,
project,
services,
stopped=False,
one_off=False):
allow_one_off=True):
"""Check if there are containers named using the old naming convention
and warn the user that those containers may need to be migrated to
using labels, so that compose can find them.
"""
containers = list(get_legacy_containers(
client,
project,
services,
stopped=stopped,
one_off=one_off))
containers = get_legacy_containers(client, project, services, one_off=False)
if containers:
raise LegacyContainersError([c.name for c in containers])
one_off_containers = get_legacy_containers(client, project, services, one_off=True)
raise LegacyContainersError(
[c.name for c in containers],
[c.name for c in one_off_containers],
)
if not allow_one_off:
one_off_containers = get_legacy_containers(client, project, services, one_off=True)
if one_off_containers:
raise LegacyOneOffContainersError(
[c.name for c in one_off_containers],
)
class LegacyContainersError(Exception):
def __init__(self, names):
class LegacyError(Exception):
def __unicode__(self):
return self.msg
__str__ = __unicode__
class LegacyContainersError(LegacyError):
def __init__(self, names, one_off_names):
self.names = names
self.one_off_names = one_off_names
self.msg = ERROR_MESSAGE_FORMAT.format(
names_list="\n".join(" {}".format(name) for name in names),
rm_args=" ".join(names),
)
def __unicode__(self):
return self.msg
if one_off_names:
self.msg += ONE_OFF_ADDENDUM_FORMAT.format(rm_args=" ".join(one_off_names))
__str__ = __unicode__
class LegacyOneOffContainersError(LegacyError):
def __init__(self, one_off_names):
self.one_off_names = one_off_names
self.msg = ONE_OFF_ERROR_MESSAGE_FORMAT.format(
names_list="\n".join(" {}".format(name) for name in one_off_names),
rm_args=" ".join(one_off_names),
)
def add_labels(project, container):
@ -76,8 +118,8 @@ def migrate_project_to_labels(project):
project.client,
project.name,
project.service_names,
stopped=True,
one_off=False)
one_off=False,
)
for container in containers:
add_labels(project, container)
@ -87,13 +129,29 @@ def get_legacy_containers(
client,
project,
services,
stopped=False,
one_off=False):
containers = client.containers(all=stopped)
return list(_get_legacy_containers_iter(
client,
project,
services,
one_off=one_off,
))
def _get_legacy_containers_iter(
client,
project,
services,
one_off=False):
containers = client.containers(all=True)
for service in services:
for container in containers:
if LABEL_VERSION in container['Labels']:
continue
name = get_container_name(container)
if has_container(project, service, name, one_off=one_off):
yield Container.from_ps(client, container)

View File

@ -308,8 +308,7 @@ class Project(object):
self.client,
self.name,
self.service_names,
stopped=stopped,
one_off=one_off)
)
return filter(matches_service_names, containers)

View File

@ -111,8 +111,7 @@ class Service(object):
self.client,
self.project,
[self.name],
stopped=stopped,
one_off=one_off)
)
return containers

View File

@ -1,3 +1,5 @@
import unittest
from docker.errors import APIError
from compose import legacy
@ -5,6 +7,64 @@ from compose.project import Project
from .testcases import DockerClientTestCase
class UtilitiesTestCase(unittest.TestCase):
def test_has_container(self):
self.assertTrue(
legacy.has_container("composetest", "web", "composetest_web_1", one_off=False),
)
self.assertFalse(
legacy.has_container("composetest", "web", "composetest_web_run_1", one_off=False),
)
def test_has_container_one_off(self):
self.assertFalse(
legacy.has_container("composetest", "web", "composetest_web_1", one_off=True),
)
self.assertTrue(
legacy.has_container("composetest", "web", "composetest_web_run_1", one_off=True),
)
def test_has_container_different_project(self):
self.assertFalse(
legacy.has_container("composetest", "web", "otherapp_web_1", one_off=False),
)
self.assertFalse(
legacy.has_container("composetest", "web", "otherapp_web_run_1", one_off=True),
)
def test_has_container_different_service(self):
self.assertFalse(
legacy.has_container("composetest", "web", "composetest_db_1", one_off=False),
)
self.assertFalse(
legacy.has_container("composetest", "web", "composetest_db_run_1", one_off=True),
)
def test_is_valid_name(self):
self.assertTrue(
legacy.is_valid_name("composetest_web_1", one_off=False),
)
self.assertFalse(
legacy.is_valid_name("composetest_web_run_1", one_off=False),
)
def test_is_valid_name_one_off(self):
self.assertFalse(
legacy.is_valid_name("composetest_web_1", one_off=True),
)
self.assertTrue(
legacy.is_valid_name("composetest_web_run_1", one_off=True),
)
def test_is_valid_name_invalid(self):
self.assertFalse(
legacy.is_valid_name("foo"),
)
self.assertFalse(
legacy.is_valid_name("composetest_web_lol_1", one_off=True),
)
class LegacyTestCase(DockerClientTestCase):
def setUp(self):
@ -30,7 +90,7 @@ class LegacyTestCase(DockerClientTestCase):
# Create a single one-off legacy container
self.containers.append(self.client.create_container(
name='{}_{}_run_1'.format(self.project.name, self.services[0].name),
name='{}_{}_run_1'.format(self.project.name, db.name),
**self.services[0].options
))
@ -47,27 +107,84 @@ class LegacyTestCase(DockerClientTestCase):
pass
def get_legacy_containers(self, **kwargs):
return list(legacy.get_legacy_containers(
return legacy.get_legacy_containers(
self.client,
self.project.name,
[s.name for s in self.services],
**kwargs
))
)
def test_get_legacy_container_names(self):
self.assertEqual(len(self.get_legacy_containers()), len(self.services))
def test_get_legacy_container_names_one_off(self):
self.assertEqual(len(self.get_legacy_containers(stopped=True, one_off=True)), 1)
self.assertEqual(len(self.get_legacy_containers(one_off=True)), 1)
def test_migration_to_labels(self):
# Trying to get the container list raises an exception
with self.assertRaises(legacy.LegacyContainersError) as cm:
self.assertEqual(self.project.containers(stopped=True), [])
self.project.containers(stopped=True)
self.assertEqual(
set(cm.exception.names),
set(['composetest_db_1', 'composetest_web_1', 'composetest_nginx_1']),
)
self.assertEqual(
set(cm.exception.one_off_names),
set(['composetest_db_run_1']),
)
# Migrate the containers
legacy.migrate_project_to_labels(self.project)
self.assertEqual(len(self.project.containers(stopped=True)), len(self.services))
# Getting the list no longer raises an exception
containers = self.project.containers(stopped=True)
self.assertEqual(len(containers), len(self.services))
def test_migration_one_off(self):
# We've already migrated
legacy.migrate_project_to_labels(self.project)
# Trying to create a one-off container results in a Docker API error
with self.assertRaises(APIError) as cm:
self.project.get_service('db').create_container(one_off=True)
# Checking for legacy one-off containers raises an exception
with self.assertRaises(legacy.LegacyOneOffContainersError) as cm:
legacy.check_for_legacy_containers(
self.client,
self.project.name,
['db'],
allow_one_off=False,
)
self.assertEqual(
set(cm.exception.one_off_names),
set(['composetest_db_run_1']),
)
# Remove the old one-off container
c = self.client.inspect_container('composetest_db_run_1')
self.client.remove_container(c)
# Checking no longer raises an exception
legacy.check_for_legacy_containers(
self.client,
self.project.name,
['db'],
allow_one_off=False,
)
# Creating a one-off container no longer results in an API error
self.project.get_service('db').create_container(one_off=True)
self.assertIsInstance(self.client.inspect_container('composetest_db_run_1'), dict)

View File

@ -97,7 +97,7 @@ class CLITestCase(unittest.TestCase):
def test_run_with_environment_merged_with_options_list(self, mock_dockerpty):
command = TopLevelCommand()
mock_client = mock.create_autospec(docker.Client)
mock_project = mock.Mock()
mock_project = mock.Mock(client=mock_client)
mock_project.get_service.return_value = Service(
'service',
client=mock_client,
@ -126,7 +126,7 @@ class CLITestCase(unittest.TestCase):
def test_run_service_with_restart_always(self):
command = TopLevelCommand()
mock_client = mock.create_autospec(docker.Client)
mock_project = mock.Mock()
mock_project = mock.Mock(client=mock_client)
mock_project.get_service.return_value = Service(
'service',
client=mock_client,
@ -150,7 +150,7 @@ class CLITestCase(unittest.TestCase):
command = TopLevelCommand()
mock_client = mock.create_autospec(docker.Client)
mock_project = mock.Mock()
mock_project = mock.Mock(client=mock_client)
mock_project.get_service.return_value = Service(
'service',
client=mock_client,