Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate containers in dependency order #1444

Merged
merged 1 commit into from
May 26, 2015
Merged

Migrate containers in dependency order #1444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions compose/legacy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ def check_for_legacy_containers(
and warn the user that those containers may need to be migrated to
using labels, so that compose can find them.
"""
names = list(get_legacy_container_names(
containers = list(get_legacy_containers(
client,
project,
services,
stopped=stopped,
one_off=one_off))

if names:
raise LegacyContainersError(names)
if containers:
raise LegacyContainersError([c.name for c in containers])


class LegacyContainersError(Exception):
Expand All @@ -61,8 +61,8 @@ def __unicode__(self):
__str__ = __unicode__


def add_labels(project, container, name):
project_name, service_name, one_off, number = NAME_RE.match(name).groups()
def add_labels(project, container):
project_name, service_name, one_off, number = NAME_RE.match(container.name).groups()
if project_name != project.name or service_name not in project.service_names:
return
service = project.get_service(service_name)
Expand All @@ -72,26 +72,31 @@ def add_labels(project, container, name):
def migrate_project_to_labels(project):
log.info("Running migration to labels for project %s", project.name)

client = project.client
for container in client.containers(all=True):
name = get_container_name(container)
if not is_valid_name(name):
continue
add_labels(project, Container.from_ps(client, container), name)
containers = get_legacy_containers(
project.client,
project.name,
project.service_names,
stopped=True,
one_off=False)

for container in containers:
add_labels(project, container)

def get_legacy_container_names(

def get_legacy_containers(
client,
project,
services,
stopped=False,
one_off=False):

for container in client.containers(all=stopped):
name = get_container_name(container)
for service in services:
containers = client.containers(all=stopped)

for service in services:
for container in containers:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so if I understand this correctly, the change here is that now we're iterating over things in the order of the passed in services, instead of just the container list order (which is basically undefined).

We have to assume the services were passed in the correct order, but that's probably safe to assume, since that's how we store them generally.

Does that sound right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry, the actual fix is a bit subtle - it relies on the fact that Project keeps its services in order, and so project.service_names returns them in order. That's a widely relied-upon constraint, though, so I don't think this is brittle.

name = get_container_name(container)
if has_container(project, service, name, one_off=one_off):
yield name
yield Container.from_ps(client, container)


def has_container(project, service, name, one_off=False):
Expand Down
24 changes: 11 additions & 13 deletions tests/integration/legacy_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,51 +8,49 @@ class ProjectTest(DockerClientTestCase):
def setUp(self):
super(ProjectTest, self).setUp()

self.services = [
self.create_service('web'),
self.create_service('db'),
]
db = self.create_service('db')
web = self.create_service('web', links=[(db, 'db')])
nginx = self.create_service('nginx', links=[(web, 'web')])

self.services = [db, web, nginx]
self.project = Project('composetest', self.services, self.client)

# Create a legacy container for each service
for service in self.services:
service.ensure_image_exists()
self.client.create_container(
container = self.client.create_container(
name='{}_{}_1'.format(self.project.name, service.name),
**service.options
)
self.client.start(container)

# Create a single one-off legacy container
self.client.create_container(
name='{}_{}_run_1'.format(self.project.name, self.services[0].name),
**self.services[0].options
)

def get_names(self, **kwargs):
if 'stopped' not in kwargs:
kwargs['stopped'] = True

return list(legacy.get_legacy_container_names(
def get_legacy_containers(self, **kwargs):
return list(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_names()), len(self.services))
self.assertEqual(len(self.get_legacy_containers()), len(self.services))

def test_get_legacy_container_names_one_off(self):
self.assertEqual(len(self.get_names(one_off=True)), 1)
self.assertEqual(len(self.get_legacy_containers(stopped=True, one_off=True)), 1)

def test_migration_to_labels(self):
with self.assertRaises(legacy.LegacyContainersError) as cm:
self.assertEqual(self.project.containers(stopped=True), [])

self.assertEqual(
set(cm.exception.names),
set(['composetest_web_1', 'composetest_db_1']),
set(['composetest_db_1', 'composetest_web_1', 'composetest_nginx_1']),
)

legacy.migrate_project_to_labels(self.project)
Expand Down