Skip to content

Fix volumes on recreate#711

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

Merged
aanand merged 2 commits into from
Jan 15, 2015
Merged

Fix volumes on recreate #711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
26f45ef
Remove unused intermediate_container from return value.
dnephin Dec 6, 2014
7eb476e
Resolves #447, fix volume logic for recreate container
dnephin Dec 6, 2014
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
20 changes: 8 additions & 12 deletions fig/project.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
do_build=True):
running_containers = []
for service in self.get_services(service_names, include_links=start_links):
if recreate:
for (_, container) in service.recreate_containers(
insecure_registry=insecure_registry,
detach=detach,
do_build=do_build):
running_containers.append(container)
else:
for container in service.start_or_create_containers(
insecure_registry=insecure_registry,
detach=detach,
do_build=do_build):
running_containers.append(container)
create_func = (service.recreate_containers if recreate
else service.start_or_create_containers)

for container in create_func(
insecure_registry=insecure_registry,
detach=detach,
do_build=do_build):
running_containers.append(container)

return running_containers

Expand Down
92 changes: 66 additions & 26 deletions fig/service.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
"""
If a container for this service doesn't exist, create and start one. If there are
any, stop them, create+start new ones, and remove the old containers.
If a container for this service doesn't exist, create and start one. If
there are any, stop them, create+start new ones, and remove the old
containers.
"""
containers = self.containers(stopped=True)
if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
do_build=do_build,
**override_options)
self.start_container(container)
return [(None, container)]
return [container]
else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
self.recreate_container(
container,
insecure_registry=insecure_registry,
**override_options)
for container in containers
]

def recreate_container(self, container, **override_options):
"""Recreate a container. An intermediate container is created so that
the new container has the same name, while still supporting
`volumes-from` the original container.
"""
log.info("Recreating %s..." % container.name)
try:
container.stop()
except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
else:
raise

intermediate_options = dict(self.options, **override_options)
intermediate_container = Container.create(
self.client,
image=container.image,
entrypoint=['/bin/echo'],
command=[],
detach=True,
)
intermediate_container.start(volumes_from=container.id)
intermediate_container.start(
binds=get_container_data_volumes(
container, intermediate_options.get('volumes')))
intermediate_container.wait()
container.remove()

# TODO: volumes are being passed to both start and create, this is
# probably unnecessary
options = dict(override_options)
new_container = self.create_container(do_build=False, **options)
self.start_container(new_container, intermediate_container=intermediate_container)
self.start_container(
new_container,
intermediate_container=intermediate_container)

intermediate_container.remove()

return (intermediate_container, new_container)
return new_container

def start_container_if_stopped(self, container, **options):
if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
def start_container(self, container, intermediate_container=None, **override_options):
options = dict(self.options, **override_options)
port_bindings = build_port_bindings(options.get('ports') or [])

volume_bindings = dict(
build_volume_binding(parse_volume_spec(volume))
for volume in options.get('volumes') or []
if ':' in volume)

privileged = options.get('privileged', False)
net = options.get('net', 'bridge')
dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
cap_drop = options.get('cap_drop', None)

restart = parse_restart_spec(options.get('restart', None))
binds = get_volume_bindings(
options.get('volumes'), intermediate_container)

container.start(
links=self._get_links(link_to_self=options.get('one_off', False)),
port_bindings=port_bindings,
binds=volume_bindings,
volumes_from=self._get_volumes_from(intermediate_container),
binds=binds,
volumes_from=self._get_volumes_from(),
privileged=privileged,
network_mode=net,
dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
links.append((external_link, link_name))
return links

def _get_volumes_from(self, intermediate_container=None):
def _get_volumes_from(self):
volumes_from = []
for volume_source in self.volumes_from:
if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
elif isinstance(volume_source, Container):
volumes_from.append(volume_source.id)

if intermediate_container:
volumes_from.append(intermediate_container.id)

return volumes_from

def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
)


def get_container_data_volumes(container, volumes_option):
"""Find the container data volumes that are in `volumes_option`, and return
a mapping of volume bindings for those volumes.
"""
volumes = []
for volume in volumes_option or []:
volume = parse_volume_spec(volume)
# No need to preserve host volumes
if volume.external:
continue

volume_path = (container.get('Volumes') or {}).get(volume.internal)
# New volume, doesn't exist in the old container
if not volume_path:
continue

# Copy existing volume from old container
volume = volume._replace(external=volume_path)
volumes.append(build_volume_binding(volume))

return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
"""Return a list of volume bindings for a container. Container data volume
bindings are replaced by those in the intermediate container.
"""
volume_bindings = dict(
build_volume_binding(parse_volume_spec(volume))
for volume in volumes_option or []
if ':' in volume)

if intermediate_container:
volume_bindings.update(
get_container_data_volumes(intermediate_container, volumes_option))

return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down
35 changes: 19 additions & 16 deletions tests/integration/service_test.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
service = self.create_service('db', volumes=['/var/db'])
container = service.create_container()
service.start_container(container)
self.assertIn('/var/db', container.inspect()['Volumes'])
self.assertIn('/var/db', container.get('Volumes'))

def test_create_container_with_cpu_shares(self):
service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
self.assertEqual(old_container.name, 'figtest_db_1')
service.start_container(old_container)
volume_path = old_container.inspect()['Volumes']['/etc']
volume_path = old_container.get('Volumes')['/etc']

num_containers_before = len(self.client.containers(all=True))

service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
self.assertEqual(len(containers), 1)

intermediate_container = tuples[0][0]
new_container = tuples[0][1]
self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
new_container = containers[0]
self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
self.assertEqual(new_container.get('Config.Cmd'), ['300'])
self.assertIn('FOO=2', new_container.get('Config.Env'))
self.assertEqual(new_container.name, 'figtest_db_1')
self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
service.recreate_containers()
self.assertEqual(len(service.containers(stopped=True)), 1)

def test_recreate_containers_with_volume_changes(self):
service = self.create_service('withvolumes', volumes=['/etc'])
old_container = create_and_start_container(service)
self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

service = self.create_service('withvolumes')
container, = service.recreate_containers()
service.start_container(container)
self.assertEqual(container.get('Volumes'), {})

def test_start_container_passes_through_options(self):
db = self.create_service('db')
create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down
59 changes: 49 additions & 10 deletions tests/unit/service_test.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
APIError,
ConfigError,
split_port,
build_port_bindings,
parse_volume_spec,
build_volume_binding,
APIError,
get_container_data_volumes,
get_volume_bindings,
parse_repository_tag,
parse_volume_spec,
split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

self.assertEqual(service._get_volumes_from(), [container_id])

def test_get_volumes_from_intermediate_container(self):
container_id = 'aabbccddee'
service = Service('test')
container = mock.Mock(id=container_id, spec=Container)

self.assertEqual(service._get_volumes_from(container), [container_id])

def test_get_volumes_from_service_container_exists(self):
container_ids = ['aabbccddee', '12345']
from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
binding,
('/home/user', dict(bind='/home/user', ro=False)))

def test_get_container_data_volumes(self):
options = [
'/host/volume:/host/volume:ro',
'/new/volume',
'/existing/volume',
]

container = Container(None, {
'Volumes': {
'/host/volume': '/host/volume',
'/existing/volume': '/var/lib/docker/aaaaaaaa',
'/removed/volume': '/var/lib/docker/bbbbbbbb',
},
}, has_been_inspected=True)

expected = {
'/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
}

binds = get_container_data_volumes(container, options)
self.assertEqual(binds, expected)

def test_get_volume_bindings(self):
options = [
'/host/volume:/host/volume:ro',
'/host/rw/volume:/host/rw/volume',
'/new/volume',
'/existing/volume',
]

intermediate_container = Container(None, {
'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
}, has_been_inspected=True)

expected = {
'/host/volume': {'bind': '/host/volume', 'ro': True},
'/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
'/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
}

binds = get_volume_bindings(options, intermediate_container)
self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

def setUp(self):
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Fix volumes on recreate by dnephin · Pull Request #711 · docker/compose · GitHub
Skip to content
Fix volumes on recreate#711

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

 Merged

aanand
merged 2 commits into

from

Jan 15, 2015
 Merged


Fix volumes on recreate
#711
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
26f45ef

Remove unused intermediate_container from return value.


dnephin Dec 6, 2014
7eb476e

Resolves #447, fix volume logic for recreate container


dnephin Dec 6, 2014

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


20 changes: 8 additions & 12 deletions
fig/project.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
 do_build=True):
 running_containers = []
 for service in self.get_services(service_names, include_links=start_links):
 if recreate:
 for (_, container) in service.recreate_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 else:
 for container in service.start_or_create_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 create_func = (service.recreate_containers if recreate
 else service.start_or_create_containers)

 for container in create_func(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)

 return running_containers

Expand Down

92 changes: 66 additions & 26 deletions
fig/service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

 def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
 """
 If a container for this service doesn't exist, create and start one. If there are
 any, stop them, create+start new ones, and remove the old containers.
 If a container for this service doesn't exist, create and start one. If
 there are any, stop them, create+start new ones, and remove the old
 containers.
 """
 containers = self.containers(stopped=True)
 if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
 do_build=do_build,
 **override_options)
 self.start_container(container)
 return [(None, container)]
 return [container]
 else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
 self.recreate_container(
 container,
 insecure_registry=insecure_registry,
**override_options)
 for container in containers
]

 def recreate_container(self, container, **override_options):
 """Recreate a container. An intermediate container is created so that
 the new container has the same name, while still supporting
 `volumes-from` the original container.
 """
 log.info("Recreating %s..." % container.name)
 try:
 container.stop()
 except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
 else:
 raise

 intermediate_options = dict(self.options, **override_options)
 intermediate_container = Container.create(
 self.client,
 image=container.image,
 entrypoint=['/bin/echo'],
 command=[],
 detach=True,
 )
 intermediate_container.start(volumes_from=container.id)
 intermediate_container.start(
 binds=get_container_data_volumes(
 container, intermediate_options.get('volumes')))
 intermediate_container.wait()
 container.remove()

 # TODO: volumes are being passed to both start and create, this is
 # probably unnecessary
 options = dict(override_options)
 new_container = self.create_container(do_build=False, **options)
 self.start_container(new_container, intermediate_container=intermediate_container)
 self.start_container(
 new_container,
 intermediate_container=intermediate_container)

 intermediate_container.remove()

 return (intermediate_container, new_container)
 return new_container

 def start_container_if_stopped(self, container, **options):
 if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
 def start_container(self, container, intermediate_container=None, **override_options):
 options = dict(self.options, **override_options)
 port_bindings = build_port_bindings(options.get('ports') or [])

 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in options.get('volumes') or []
 if ':' in volume)

 privileged = options.get('privileged', False)
 net = options.get('net', 'bridge')
 dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
 cap_drop = options.get('cap_drop', None)

 restart = parse_restart_spec(options.get('restart', None))
 binds = get_volume_bindings(
 options.get('volumes'), intermediate_container)

 container.start(
 links=self._get_links(link_to_self=options.get('one_off', False)),
 port_bindings=port_bindings,
 binds=volume_bindings,
 volumes_from=self._get_volumes_from(intermediate_container),
 binds=binds,
 volumes_from=self._get_volumes_from(),
 privileged=privileged,
 network_mode=net,
 dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
 links.append((external_link, link_name))
 return links

 def _get_volumes_from(self, intermediate_container=None):
 def _get_volumes_from(self):
 volumes_from = []
 for volume_source in self.volumes_from:
 if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
 elif isinstance(volume_source, Container):
 volumes_from.append(volume_source.id)

 if intermediate_container:
 volumes_from.append(intermediate_container.id)

 return volumes_from

 def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
 )


def get_container_data_volumes(container, volumes_option):
 """Find the container data volumes that are in `volumes_option`, and return
 a mapping of volume bindings for those volumes.
 """
 volumes = []
 for volume in volumes_option or []:
 volume = parse_volume_spec(volume)
 # No need to preserve host volumes
 if volume.external:
 continue

 volume_path = (container.get('Volumes') or {}).get(volume.internal)
 # New volume, doesn't exist in the old container
 if not volume_path:
 continue

 # Copy existing volume from old container
 volume = volume._replace(external=volume_path)
 volumes.append(build_volume_binding(volume))

 return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
 """Return a list of volume bindings for a container. Container data volume
 bindings are replaced by those in the intermediate container.
 """
 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in volumes_option or []
 if ':' in volume)

 if intermediate_container:
 volume_bindings.update(
 get_container_data_volumes(intermediate_container, volumes_option))

 return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down

35 changes: 19 additions & 16 deletions
tests/integration/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
 service = self.create_service('db', volumes=['/var/db'])
 container = service.create_container()
 service.start_container(container)
 self.assertIn('/var/db', container.inspect()['Volumes'])
 self.assertIn('/var/db', container.get('Volumes'))

 def test_create_container_with_cpu_shares(self):
 service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
 self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
 self.assertEqual(old_container.name, 'figtest_db_1')
 service.start_container(old_container)
 volume_path = old_container.inspect()['Volumes']['/etc']
 volume_path = old_container.get('Volumes')['/etc']

 num_containers_before = len(self.client.containers(all=True))

 service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
 self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
 self.assertEqual(len(containers), 1)

 intermediate_container = tuples[0][0]
 new_container = tuples[0][1]
 self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

 self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
 self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
 self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
 new_container = containers[0]
 self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
 self.assertEqual(new_container.get('Config.Cmd'), ['300'])
 self.assertIn('FOO=2', new_container.get('Config.Env'))
 self.assertEqual(new_container.name, 'figtest_db_1')
 self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
 self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

 self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
 self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
 service.recreate_containers()
 self.assertEqual(len(service.containers(stopped=True)), 1)

 def test_recreate_containers_with_volume_changes(self):
 service = self.create_service('withvolumes', volumes=['/etc'])
 old_container = create_and_start_container(service)
 self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

 service = self.create_service('withvolumes')
 container, = service.recreate_containers()
 service.start_container(container)
 self.assertEqual(container.get('Volumes'), {})

 def test_start_container_passes_through_options(self):
 db = self.create_service('db')
 create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down

59 changes: 49 additions & 10 deletions
tests/unit/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
 APIError,
 ConfigError,
 split_port,
 build_port_bindings,
 parse_volume_spec,
 build_volume_binding,
 APIError,
 get_container_data_volumes,
 get_volume_bindings,
 parse_repository_tag,
 parse_volume_spec,
 split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

 self.assertEqual(service._get_volumes_from(), [container_id])

 def test_get_volumes_from_intermediate_container(self):
 container_id = 'aabbccddee'
 service = Service('test')
 container = mock.Mock(id=container_id, spec=Container)

 self.assertEqual(service._get_volumes_from(container), [container_id])

 def test_get_volumes_from_service_container_exists(self):
 container_ids = ['aabbccddee', '12345']
 from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
 binding,
 ('/home/user', dict(bind='/home/user', ro=False)))

 def test_get_container_data_volumes(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/new/volume',
 '/existing/volume',
 ]

 container = Container(None, {
 'Volumes': {
 '/host/volume': '/host/volume',
 '/existing/volume': '/var/lib/docker/aaaaaaaa',
 '/removed/volume': '/var/lib/docker/bbbbbbbb',
 },
 }, has_been_inspected=True)

 expected = {
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_container_data_volumes(container, options)
 self.assertEqual(binds, expected)

 def test_get_volume_bindings(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/host/rw/volume:/host/rw/volume',
 '/new/volume',
 '/existing/volume',
 ]

 intermediate_container = Container(None, {
 'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
 }, has_been_inspected=True)

 expected = {
 '/host/volume': {'bind': '/host/volume', 'ro': True},
 '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_volume_bindings(options, intermediate_container)
 self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

 def setUp(self):
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix volumes on recreate by dnephin · Pull Request #711 · docker/compose · GitHub
Skip to content
Fix volumes on recreate#711

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

 Merged

aanand
merged 2 commits into

from

Jan 15, 2015
 Merged


Fix volumes on recreate
#711
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
26f45ef

Remove unused intermediate_container from return value.


dnephin Dec 6, 2014
7eb476e

Resolves #447, fix volume logic for recreate container


dnephin Dec 6, 2014

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


20 changes: 8 additions & 12 deletions
fig/project.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
 do_build=True):
 running_containers = []
 for service in self.get_services(service_names, include_links=start_links):
 if recreate:
 for (_, container) in service.recreate_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 else:
 for container in service.start_or_create_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 create_func = (service.recreate_containers if recreate
 else service.start_or_create_containers)

 for container in create_func(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)

 return running_containers

Expand Down

92 changes: 66 additions & 26 deletions
fig/service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

 def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
 """
 If a container for this service doesn't exist, create and start one. If there are
 any, stop them, create+start new ones, and remove the old containers.
 If a container for this service doesn't exist, create and start one. If
 there are any, stop them, create+start new ones, and remove the old
 containers.
 """
 containers = self.containers(stopped=True)
 if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
 do_build=do_build,
 **override_options)
 self.start_container(container)
 return [(None, container)]
 return [container]
 else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
 self.recreate_container(
 container,
 insecure_registry=insecure_registry,
**override_options)
 for container in containers
]

 def recreate_container(self, container, **override_options):
 """Recreate a container. An intermediate container is created so that
 the new container has the same name, while still supporting
 `volumes-from` the original container.
 """
 log.info("Recreating %s..." % container.name)
 try:
 container.stop()
 except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
 else:
 raise

 intermediate_options = dict(self.options, **override_options)
 intermediate_container = Container.create(
 self.client,
 image=container.image,
 entrypoint=['/bin/echo'],
 command=[],
 detach=True,
 )
 intermediate_container.start(volumes_from=container.id)
 intermediate_container.start(
 binds=get_container_data_volumes(
 container, intermediate_options.get('volumes')))
 intermediate_container.wait()
 container.remove()

 # TODO: volumes are being passed to both start and create, this is
 # probably unnecessary
 options = dict(override_options)
 new_container = self.create_container(do_build=False, **options)
 self.start_container(new_container, intermediate_container=intermediate_container)
 self.start_container(
 new_container,
 intermediate_container=intermediate_container)

 intermediate_container.remove()

 return (intermediate_container, new_container)
 return new_container

 def start_container_if_stopped(self, container, **options):
 if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
 def start_container(self, container, intermediate_container=None, **override_options):
 options = dict(self.options, **override_options)
 port_bindings = build_port_bindings(options.get('ports') or [])

 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in options.get('volumes') or []
 if ':' in volume)

 privileged = options.get('privileged', False)
 net = options.get('net', 'bridge')
 dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
 cap_drop = options.get('cap_drop', None)

 restart = parse_restart_spec(options.get('restart', None))
 binds = get_volume_bindings(
 options.get('volumes'), intermediate_container)

 container.start(
 links=self._get_links(link_to_self=options.get('one_off', False)),
 port_bindings=port_bindings,
 binds=volume_bindings,
 volumes_from=self._get_volumes_from(intermediate_container),
 binds=binds,
 volumes_from=self._get_volumes_from(),
 privileged=privileged,
 network_mode=net,
 dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
 links.append((external_link, link_name))
 return links

 def _get_volumes_from(self, intermediate_container=None):
 def _get_volumes_from(self):
 volumes_from = []
 for volume_source in self.volumes_from:
 if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
 elif isinstance(volume_source, Container):
 volumes_from.append(volume_source.id)

 if intermediate_container:
 volumes_from.append(intermediate_container.id)

 return volumes_from

 def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
 )


def get_container_data_volumes(container, volumes_option):
 """Find the container data volumes that are in `volumes_option`, and return
 a mapping of volume bindings for those volumes.
 """
 volumes = []
 for volume in volumes_option or []:
 volume = parse_volume_spec(volume)
 # No need to preserve host volumes
 if volume.external:
 continue

 volume_path = (container.get('Volumes') or {}).get(volume.internal)
 # New volume, doesn't exist in the old container
 if not volume_path:
 continue

 # Copy existing volume from old container
 volume = volume._replace(external=volume_path)
 volumes.append(build_volume_binding(volume))

 return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
 """Return a list of volume bindings for a container. Container data volume
 bindings are replaced by those in the intermediate container.
 """
 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in volumes_option or []
 if ':' in volume)

 if intermediate_container:
 volume_bindings.update(
 get_container_data_volumes(intermediate_container, volumes_option))

 return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down

35 changes: 19 additions & 16 deletions
tests/integration/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
 service = self.create_service('db', volumes=['/var/db'])
 container = service.create_container()
 service.start_container(container)
 self.assertIn('/var/db', container.inspect()['Volumes'])
 self.assertIn('/var/db', container.get('Volumes'))

 def test_create_container_with_cpu_shares(self):
 service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
 self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
 self.assertEqual(old_container.name, 'figtest_db_1')
 service.start_container(old_container)
 volume_path = old_container.inspect()['Volumes']['/etc']
 volume_path = old_container.get('Volumes')['/etc']

 num_containers_before = len(self.client.containers(all=True))

 service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
 self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
 self.assertEqual(len(containers), 1)

 intermediate_container = tuples[0][0]
 new_container = tuples[0][1]
 self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

 self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
 self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
 self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
 new_container = containers[0]
 self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
 self.assertEqual(new_container.get('Config.Cmd'), ['300'])
 self.assertIn('FOO=2', new_container.get('Config.Env'))
 self.assertEqual(new_container.name, 'figtest_db_1')
 self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
 self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

 self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
 self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
 service.recreate_containers()
 self.assertEqual(len(service.containers(stopped=True)), 1)

 def test_recreate_containers_with_volume_changes(self):
 service = self.create_service('withvolumes', volumes=['/etc'])
 old_container = create_and_start_container(service)
 self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

 service = self.create_service('withvolumes')
 container, = service.recreate_containers()
 service.start_container(container)
 self.assertEqual(container.get('Volumes'), {})

 def test_start_container_passes_through_options(self):
 db = self.create_service('db')
 create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down

59 changes: 49 additions & 10 deletions
tests/unit/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
 APIError,
 ConfigError,
 split_port,
 build_port_bindings,
 parse_volume_spec,
 build_volume_binding,
 APIError,
 get_container_data_volumes,
 get_volume_bindings,
 parse_repository_tag,
 parse_volume_spec,
 split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

 self.assertEqual(service._get_volumes_from(), [container_id])

 def test_get_volumes_from_intermediate_container(self):
 container_id = 'aabbccddee'
 service = Service('test')
 container = mock.Mock(id=container_id, spec=Container)

 self.assertEqual(service._get_volumes_from(container), [container_id])

 def test_get_volumes_from_service_container_exists(self):
 container_ids = ['aabbccddee', '12345']
 from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
 binding,
 ('/home/user', dict(bind='/home/user', ro=False)))

 def test_get_container_data_volumes(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/new/volume',
 '/existing/volume',
 ]

 container = Container(None, {
 'Volumes': {
 '/host/volume': '/host/volume',
 '/existing/volume': '/var/lib/docker/aaaaaaaa',
 '/removed/volume': '/var/lib/docker/bbbbbbbb',
 },
 }, has_been_inspected=True)

 expected = {
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_container_data_volumes(container, options)
 self.assertEqual(binds, expected)

 def test_get_volume_bindings(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/host/rw/volume:/host/rw/volume',
 '/new/volume',
 '/existing/volume',
 ]

 intermediate_container = Container(None, {
 'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
 }, has_been_inspected=True)

 expected = {
 '/host/volume': {'bind': '/host/volume', 'ro': True},
 '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_volume_bindings(options, intermediate_container)
 self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

 def setUp(self):
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix volumes on recreate by dnephin · Pull Request #711 · docker/compose · GitHub
Skip to content
Fix volumes on recreate#711

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

 Merged

aanand
merged 2 commits into

from

Jan 15, 2015
 Merged


Fix volumes on recreate
#711
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
26f45ef

Remove unused intermediate_container from return value.


dnephin Dec 6, 2014
7eb476e

Resolves #447, fix volume logic for recreate container


dnephin Dec 6, 2014

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


20 changes: 8 additions & 12 deletions
fig/project.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
 do_build=True):
 running_containers = []
 for service in self.get_services(service_names, include_links=start_links):
 if recreate:
 for (_, container) in service.recreate_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 else:
 for container in service.start_or_create_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 create_func = (service.recreate_containers if recreate
 else service.start_or_create_containers)

 for container in create_func(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)

 return running_containers

Expand Down

92 changes: 66 additions & 26 deletions
fig/service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

 def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
 """
 If a container for this service doesn't exist, create and start one. If there are
 any, stop them, create+start new ones, and remove the old containers.
 If a container for this service doesn't exist, create and start one. If
 there are any, stop them, create+start new ones, and remove the old
 containers.
 """
 containers = self.containers(stopped=True)
 if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
 do_build=do_build,
 **override_options)
 self.start_container(container)
 return [(None, container)]
 return [container]
 else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
 self.recreate_container(
 container,
 insecure_registry=insecure_registry,
**override_options)
 for container in containers
]

 def recreate_container(self, container, **override_options):
 """Recreate a container. An intermediate container is created so that
 the new container has the same name, while still supporting
 `volumes-from` the original container.
 """
 log.info("Recreating %s..." % container.name)
 try:
 container.stop()
 except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
 else:
 raise

 intermediate_options = dict(self.options, **override_options)
 intermediate_container = Container.create(
 self.client,
 image=container.image,
 entrypoint=['/bin/echo'],
 command=[],
 detach=True,
 )
 intermediate_container.start(volumes_from=container.id)
 intermediate_container.start(
 binds=get_container_data_volumes(
 container, intermediate_options.get('volumes')))
 intermediate_container.wait()
 container.remove()

 # TODO: volumes are being passed to both start and create, this is
 # probably unnecessary
 options = dict(override_options)
 new_container = self.create_container(do_build=False, **options)
 self.start_container(new_container, intermediate_container=intermediate_container)
 self.start_container(
 new_container,
 intermediate_container=intermediate_container)

 intermediate_container.remove()

 return (intermediate_container, new_container)
 return new_container

 def start_container_if_stopped(self, container, **options):
 if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
 def start_container(self, container, intermediate_container=None, **override_options):
 options = dict(self.options, **override_options)
 port_bindings = build_port_bindings(options.get('ports') or [])

 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in options.get('volumes') or []
 if ':' in volume)

 privileged = options.get('privileged', False)
 net = options.get('net', 'bridge')
 dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
 cap_drop = options.get('cap_drop', None)

 restart = parse_restart_spec(options.get('restart', None))
 binds = get_volume_bindings(
 options.get('volumes'), intermediate_container)

 container.start(
 links=self._get_links(link_to_self=options.get('one_off', False)),
 port_bindings=port_bindings,
 binds=volume_bindings,
 volumes_from=self._get_volumes_from(intermediate_container),
 binds=binds,
 volumes_from=self._get_volumes_from(),
 privileged=privileged,
 network_mode=net,
 dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
 links.append((external_link, link_name))
 return links

 def _get_volumes_from(self, intermediate_container=None):
 def _get_volumes_from(self):
 volumes_from = []
 for volume_source in self.volumes_from:
 if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
 elif isinstance(volume_source, Container):
 volumes_from.append(volume_source.id)

 if intermediate_container:
 volumes_from.append(intermediate_container.id)

 return volumes_from

 def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
 )


def get_container_data_volumes(container, volumes_option):
 """Find the container data volumes that are in `volumes_option`, and return
 a mapping of volume bindings for those volumes.
 """
 volumes = []
 for volume in volumes_option or []:
 volume = parse_volume_spec(volume)
 # No need to preserve host volumes
 if volume.external:
 continue

 volume_path = (container.get('Volumes') or {}).get(volume.internal)
 # New volume, doesn't exist in the old container
 if not volume_path:
 continue

 # Copy existing volume from old container
 volume = volume._replace(external=volume_path)
 volumes.append(build_volume_binding(volume))

 return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
 """Return a list of volume bindings for a container. Container data volume
 bindings are replaced by those in the intermediate container.
 """
 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in volumes_option or []
 if ':' in volume)

 if intermediate_container:
 volume_bindings.update(
 get_container_data_volumes(intermediate_container, volumes_option))

 return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down

35 changes: 19 additions & 16 deletions
tests/integration/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
 service = self.create_service('db', volumes=['/var/db'])
 container = service.create_container()
 service.start_container(container)
 self.assertIn('/var/db', container.inspect()['Volumes'])
 self.assertIn('/var/db', container.get('Volumes'))

 def test_create_container_with_cpu_shares(self):
 service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
 self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
 self.assertEqual(old_container.name, 'figtest_db_1')
 service.start_container(old_container)
 volume_path = old_container.inspect()['Volumes']['/etc']
 volume_path = old_container.get('Volumes')['/etc']

 num_containers_before = len(self.client.containers(all=True))

 service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
 self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
 self.assertEqual(len(containers), 1)

 intermediate_container = tuples[0][0]
 new_container = tuples[0][1]
 self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

 self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
 self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
 self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
 new_container = containers[0]
 self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
 self.assertEqual(new_container.get('Config.Cmd'), ['300'])
 self.assertIn('FOO=2', new_container.get('Config.Env'))
 self.assertEqual(new_container.name, 'figtest_db_1')
 self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
 self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

 self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
 self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
 service.recreate_containers()
 self.assertEqual(len(service.containers(stopped=True)), 1)

 def test_recreate_containers_with_volume_changes(self):
 service = self.create_service('withvolumes', volumes=['/etc'])
 old_container = create_and_start_container(service)
 self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

 service = self.create_service('withvolumes')
 container, = service.recreate_containers()
 service.start_container(container)
 self.assertEqual(container.get('Volumes'), {})

 def test_start_container_passes_through_options(self):
 db = self.create_service('db')
 create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down

59 changes: 49 additions & 10 deletions
tests/unit/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
 APIError,
 ConfigError,
 split_port,
 build_port_bindings,
 parse_volume_spec,
 build_volume_binding,
 APIError,
 get_container_data_volumes,
 get_volume_bindings,
 parse_repository_tag,
 parse_volume_spec,
 split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

 self.assertEqual(service._get_volumes_from(), [container_id])

 def test_get_volumes_from_intermediate_container(self):
 container_id = 'aabbccddee'
 service = Service('test')
 container = mock.Mock(id=container_id, spec=Container)

 self.assertEqual(service._get_volumes_from(container), [container_id])

 def test_get_volumes_from_service_container_exists(self):
 container_ids = ['aabbccddee', '12345']
 from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
 binding,
 ('/home/user', dict(bind='/home/user', ro=False)))

 def test_get_container_data_volumes(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/new/volume',
 '/existing/volume',
 ]

 container = Container(None, {
 'Volumes': {
 '/host/volume': '/host/volume',
 '/existing/volume': '/var/lib/docker/aaaaaaaa',
 '/removed/volume': '/var/lib/docker/bbbbbbbb',
 },
 }, has_been_inspected=True)

 expected = {
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_container_data_volumes(container, options)
 self.assertEqual(binds, expected)

 def test_get_volume_bindings(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/host/rw/volume:/host/rw/volume',
 '/new/volume',
 '/existing/volume',
 ]

 intermediate_container = Container(None, {
 'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
 }, has_been_inspected=True)

 expected = {
 '/host/volume': {'bind': '/host/volume', 'ro': True},
 '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_volume_bindings(options, intermediate_container)
 self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

 def setUp(self):
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Fix volumes on recreate by dnephin · Pull Request #711 · docker/compose · GitHub
Skip to content
Fix volumes on recreate#711

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

 Merged

aanand
merged 2 commits into

from

Jan 15, 2015
 Merged


Fix volumes on recreate
#711
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
26f45ef

Remove unused intermediate_container from return value.


dnephin Dec 6, 2014
7eb476e

Resolves #447, fix volume logic for recreate container


dnephin Dec 6, 2014

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


20 changes: 8 additions & 12 deletions
fig/project.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
 do_build=True):
 running_containers = []
 for service in self.get_services(service_names, include_links=start_links):
 if recreate:
 for (_, container) in service.recreate_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 else:
 for container in service.start_or_create_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 create_func = (service.recreate_containers if recreate
 else service.start_or_create_containers)

 for container in create_func(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)

 return running_containers

Expand Down

92 changes: 66 additions & 26 deletions
fig/service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

 def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
 """
 If a container for this service doesn't exist, create and start one. If there are
 any, stop them, create+start new ones, and remove the old containers.
 If a container for this service doesn't exist, create and start one. If
 there are any, stop them, create+start new ones, and remove the old
 containers.
 """
 containers = self.containers(stopped=True)
 if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
 do_build=do_build,
 **override_options)
 self.start_container(container)
 return [(None, container)]
 return [container]
 else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
 self.recreate_container(
 container,
 insecure_registry=insecure_registry,
**override_options)
 for container in containers
]

 def recreate_container(self, container, **override_options):
 """Recreate a container. An intermediate container is created so that
 the new container has the same name, while still supporting
 `volumes-from` the original container.
 """
 log.info("Recreating %s..." % container.name)
 try:
 container.stop()
 except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
 else:
 raise

 intermediate_options = dict(self.options, **override_options)
 intermediate_container = Container.create(
 self.client,
 image=container.image,
 entrypoint=['/bin/echo'],
 command=[],
 detach=True,
 )
 intermediate_container.start(volumes_from=container.id)
 intermediate_container.start(
 binds=get_container_data_volumes(
 container, intermediate_options.get('volumes')))
 intermediate_container.wait()
 container.remove()

 # TODO: volumes are being passed to both start and create, this is
 # probably unnecessary
 options = dict(override_options)
 new_container = self.create_container(do_build=False, **options)
 self.start_container(new_container, intermediate_container=intermediate_container)
 self.start_container(
 new_container,
 intermediate_container=intermediate_container)

 intermediate_container.remove()

 return (intermediate_container, new_container)
 return new_container

 def start_container_if_stopped(self, container, **options):
 if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
 def start_container(self, container, intermediate_container=None, **override_options):
 options = dict(self.options, **override_options)
 port_bindings = build_port_bindings(options.get('ports') or [])

 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in options.get('volumes') or []
 if ':' in volume)

 privileged = options.get('privileged', False)
 net = options.get('net', 'bridge')
 dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
 cap_drop = options.get('cap_drop', None)

 restart = parse_restart_spec(options.get('restart', None))
 binds = get_volume_bindings(
 options.get('volumes'), intermediate_container)

 container.start(
 links=self._get_links(link_to_self=options.get('one_off', False)),
 port_bindings=port_bindings,
 binds=volume_bindings,
 volumes_from=self._get_volumes_from(intermediate_container),
 binds=binds,
 volumes_from=self._get_volumes_from(),
 privileged=privileged,
 network_mode=net,
 dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
 links.append((external_link, link_name))
 return links

 def _get_volumes_from(self, intermediate_container=None):
 def _get_volumes_from(self):
 volumes_from = []
 for volume_source in self.volumes_from:
 if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
 elif isinstance(volume_source, Container):
 volumes_from.append(volume_source.id)

 if intermediate_container:
 volumes_from.append(intermediate_container.id)

 return volumes_from

 def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
 )


def get_container_data_volumes(container, volumes_option):
 """Find the container data volumes that are in `volumes_option`, and return
 a mapping of volume bindings for those volumes.
 """
 volumes = []
 for volume in volumes_option or []:
 volume = parse_volume_spec(volume)
 # No need to preserve host volumes
 if volume.external:
 continue

 volume_path = (container.get('Volumes') or {}).get(volume.internal)
 # New volume, doesn't exist in the old container
 if not volume_path:
 continue

 # Copy existing volume from old container
 volume = volume._replace(external=volume_path)
 volumes.append(build_volume_binding(volume))

 return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
 """Return a list of volume bindings for a container. Container data volume
 bindings are replaced by those in the intermediate container.
 """
 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in volumes_option or []
 if ':' in volume)

 if intermediate_container:
 volume_bindings.update(
 get_container_data_volumes(intermediate_container, volumes_option))

 return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down

35 changes: 19 additions & 16 deletions
tests/integration/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
 service = self.create_service('db', volumes=['/var/db'])
 container = service.create_container()
 service.start_container(container)
 self.assertIn('/var/db', container.inspect()['Volumes'])
 self.assertIn('/var/db', container.get('Volumes'))

 def test_create_container_with_cpu_shares(self):
 service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
 self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
 self.assertEqual(old_container.name, 'figtest_db_1')
 service.start_container(old_container)
 volume_path = old_container.inspect()['Volumes']['/etc']
 volume_path = old_container.get('Volumes')['/etc']

 num_containers_before = len(self.client.containers(all=True))

 service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
 self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
 self.assertEqual(len(containers), 1)

 intermediate_container = tuples[0][0]
 new_container = tuples[0][1]
 self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

 self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
 self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
 self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
 new_container = containers[0]
 self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
 self.assertEqual(new_container.get('Config.Cmd'), ['300'])
 self.assertIn('FOO=2', new_container.get('Config.Env'))
 self.assertEqual(new_container.name, 'figtest_db_1')
 self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
 self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

 self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
 self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
 service.recreate_containers()
 self.assertEqual(len(service.containers(stopped=True)), 1)

 def test_recreate_containers_with_volume_changes(self):
 service = self.create_service('withvolumes', volumes=['/etc'])
 old_container = create_and_start_container(service)
 self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

 service = self.create_service('withvolumes')
 container, = service.recreate_containers()
 service.start_container(container)
 self.assertEqual(container.get('Volumes'), {})

 def test_start_container_passes_through_options(self):
 db = self.create_service('db')
 create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down

59 changes: 49 additions & 10 deletions
tests/unit/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
 APIError,
 ConfigError,
 split_port,
 build_port_bindings,
 parse_volume_spec,
 build_volume_binding,
 APIError,
 get_container_data_volumes,
 get_volume_bindings,
 parse_repository_tag,
 parse_volume_spec,
 split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

 self.assertEqual(service._get_volumes_from(), [container_id])

 def test_get_volumes_from_intermediate_container(self):
 container_id = 'aabbccddee'
 service = Service('test')
 container = mock.Mock(id=container_id, spec=Container)

 self.assertEqual(service._get_volumes_from(container), [container_id])

 def test_get_volumes_from_service_container_exists(self):
 container_ids = ['aabbccddee', '12345']
 from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
 binding,
 ('/home/user', dict(bind='/home/user', ro=False)))

 def test_get_container_data_volumes(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/new/volume',
 '/existing/volume',
 ]

 container = Container(None, {
 'Volumes': {
 '/host/volume': '/host/volume',
 '/existing/volume': '/var/lib/docker/aaaaaaaa',
 '/removed/volume': '/var/lib/docker/bbbbbbbb',
 },
 }, has_been_inspected=True)

 expected = {
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_container_data_volumes(container, options)
 self.assertEqual(binds, expected)

 def test_get_volume_bindings(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/host/rw/volume:/host/rw/volume',
 '/new/volume',
 '/existing/volume',
 ]

 intermediate_container = Container(None, {
 'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
 }, has_been_inspected=True)

 expected = {
 '/host/volume': {'bind': '/host/volume', 'ro': True},
 '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_volume_bindings(options, intermediate_container)
 self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

 def setUp(self):
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix volumes on recreate by dnephin · Pull Request #711 · docker/compose · GitHub
Skip to content
Fix volumes on recreate#711

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

 Merged

aanand
merged 2 commits into

from

Jan 15, 2015
 Merged


Fix volumes on recreate
#711
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
26f45ef

Remove unused intermediate_container from return value.


dnephin Dec 6, 2014
7eb476e

Resolves #447, fix volume logic for recreate container


dnephin Dec 6, 2014

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


20 changes: 8 additions & 12 deletions
fig/project.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
 do_build=True):
 running_containers = []
 for service in self.get_services(service_names, include_links=start_links):
 if recreate:
 for (_, container) in service.recreate_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 else:
 for container in service.start_or_create_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 create_func = (service.recreate_containers if recreate
 else service.start_or_create_containers)

 for container in create_func(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)

 return running_containers

Expand Down

92 changes: 66 additions & 26 deletions
fig/service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

 def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
 """
 If a container for this service doesn't exist, create and start one. If there are
 any, stop them, create+start new ones, and remove the old containers.
 If a container for this service doesn't exist, create and start one. If
 there are any, stop them, create+start new ones, and remove the old
 containers.
 """
 containers = self.containers(stopped=True)
 if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
 do_build=do_build,
 **override_options)
 self.start_container(container)
 return [(None, container)]
 return [container]
 else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
 self.recreate_container(
 container,
 insecure_registry=insecure_registry,
**override_options)
 for container in containers
]

 def recreate_container(self, container, **override_options):
 """Recreate a container. An intermediate container is created so that
 the new container has the same name, while still supporting
 `volumes-from` the original container.
 """
 log.info("Recreating %s..." % container.name)
 try:
 container.stop()
 except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
 else:
 raise

 intermediate_options = dict(self.options, **override_options)
 intermediate_container = Container.create(
 self.client,
 image=container.image,
 entrypoint=['/bin/echo'],
 command=[],
 detach=True,
 )
 intermediate_container.start(volumes_from=container.id)
 intermediate_container.start(
 binds=get_container_data_volumes(
 container, intermediate_options.get('volumes')))
 intermediate_container.wait()
 container.remove()

 # TODO: volumes are being passed to both start and create, this is
 # probably unnecessary
 options = dict(override_options)
 new_container = self.create_container(do_build=False, **options)
 self.start_container(new_container, intermediate_container=intermediate_container)
 self.start_container(
 new_container,
 intermediate_container=intermediate_container)

 intermediate_container.remove()

 return (intermediate_container, new_container)
 return new_container

 def start_container_if_stopped(self, container, **options):
 if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
 def start_container(self, container, intermediate_container=None, **override_options):
 options = dict(self.options, **override_options)
 port_bindings = build_port_bindings(options.get('ports') or [])

 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in options.get('volumes') or []
 if ':' in volume)

 privileged = options.get('privileged', False)
 net = options.get('net', 'bridge')
 dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
 cap_drop = options.get('cap_drop', None)

 restart = parse_restart_spec(options.get('restart', None))
 binds = get_volume_bindings(
 options.get('volumes'), intermediate_container)

 container.start(
 links=self._get_links(link_to_self=options.get('one_off', False)),
 port_bindings=port_bindings,
 binds=volume_bindings,
 volumes_from=self._get_volumes_from(intermediate_container),
 binds=binds,
 volumes_from=self._get_volumes_from(),
 privileged=privileged,
 network_mode=net,
 dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
 links.append((external_link, link_name))
 return links

 def _get_volumes_from(self, intermediate_container=None):
 def _get_volumes_from(self):
 volumes_from = []
 for volume_source in self.volumes_from:
 if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
 elif isinstance(volume_source, Container):
 volumes_from.append(volume_source.id)

 if intermediate_container:
 volumes_from.append(intermediate_container.id)

 return volumes_from

 def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
 )


def get_container_data_volumes(container, volumes_option):
 """Find the container data volumes that are in `volumes_option`, and return
 a mapping of volume bindings for those volumes.
 """
 volumes = []
 for volume in volumes_option or []:
 volume = parse_volume_spec(volume)
 # No need to preserve host volumes
 if volume.external:
 continue

 volume_path = (container.get('Volumes') or {}).get(volume.internal)
 # New volume, doesn't exist in the old container
 if not volume_path:
 continue

 # Copy existing volume from old container
 volume = volume._replace(external=volume_path)
 volumes.append(build_volume_binding(volume))

 return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
 """Return a list of volume bindings for a container. Container data volume
 bindings are replaced by those in the intermediate container.
 """
 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in volumes_option or []
 if ':' in volume)

 if intermediate_container:
 volume_bindings.update(
 get_container_data_volumes(intermediate_container, volumes_option))

 return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down

35 changes: 19 additions & 16 deletions
tests/integration/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
 service = self.create_service('db', volumes=['/var/db'])
 container = service.create_container()
 service.start_container(container)
 self.assertIn('/var/db', container.inspect()['Volumes'])
 self.assertIn('/var/db', container.get('Volumes'))

 def test_create_container_with_cpu_shares(self):
 service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
 self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
 self.assertEqual(old_container.name, 'figtest_db_1')
 service.start_container(old_container)
 volume_path = old_container.inspect()['Volumes']['/etc']
 volume_path = old_container.get('Volumes')['/etc']

 num_containers_before = len(self.client.containers(all=True))

 service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
 self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
 self.assertEqual(len(containers), 1)

 intermediate_container = tuples[0][0]
 new_container = tuples[0][1]
 self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

 self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
 self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
 self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
 new_container = containers[0]
 self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
 self.assertEqual(new_container.get('Config.Cmd'), ['300'])
 self.assertIn('FOO=2', new_container.get('Config.Env'))
 self.assertEqual(new_container.name, 'figtest_db_1')
 self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
 self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

 self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
 self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
 service.recreate_containers()
 self.assertEqual(len(service.containers(stopped=True)), 1)

 def test_recreate_containers_with_volume_changes(self):
 service = self.create_service('withvolumes', volumes=['/etc'])
 old_container = create_and_start_container(service)
 self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

 service = self.create_service('withvolumes')
 container, = service.recreate_containers()
 service.start_container(container)
 self.assertEqual(container.get('Volumes'), {})

 def test_start_container_passes_through_options(self):
 db = self.create_service('db')
 create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down

59 changes: 49 additions & 10 deletions
tests/unit/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
 APIError,
 ConfigError,
 split_port,
 build_port_bindings,
 parse_volume_spec,
 build_volume_binding,
 APIError,
 get_container_data_volumes,
 get_volume_bindings,
 parse_repository_tag,
 parse_volume_spec,
 split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

 self.assertEqual(service._get_volumes_from(), [container_id])

 def test_get_volumes_from_intermediate_container(self):
 container_id = 'aabbccddee'
 service = Service('test')
 container = mock.Mock(id=container_id, spec=Container)

 self.assertEqual(service._get_volumes_from(container), [container_id])

 def test_get_volumes_from_service_container_exists(self):
 container_ids = ['aabbccddee', '12345']
 from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
 binding,
 ('/home/user', dict(bind='/home/user', ro=False)))

 def test_get_container_data_volumes(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/new/volume',
 '/existing/volume',
 ]

 container = Container(None, {
 'Volumes': {
 '/host/volume': '/host/volume',
 '/existing/volume': '/var/lib/docker/aaaaaaaa',
 '/removed/volume': '/var/lib/docker/bbbbbbbb',
 },
 }, has_been_inspected=True)

 expected = {
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_container_data_volumes(container, options)
 self.assertEqual(binds, expected)

 def test_get_volume_bindings(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/host/rw/volume:/host/rw/volume',
 '/new/volume',
 '/existing/volume',
 ]

 intermediate_container = Container(None, {
 'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
 }, has_been_inspected=True)

 expected = {
 '/host/volume': {'bind': '/host/volume', 'ro': True},
 '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_volume_bindings(options, intermediate_container)
 self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

 def setUp(self):
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Fix volumes on recreate by dnephin · Pull Request #711 · docker/compose · GitHub
Skip to content
Fix volumes on recreate#711

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

 Merged

aanand
merged 2 commits into

from

Jan 15, 2015
 Merged


Fix volumes on recreate
#711
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
26f45ef

Remove unused intermediate_container from return value.


dnephin Dec 6, 2014
7eb476e

Resolves #447, fix volume logic for recreate container


dnephin Dec 6, 2014

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


20 changes: 8 additions & 12 deletions
fig/project.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
 do_build=True):
 running_containers = []
 for service in self.get_services(service_names, include_links=start_links):
 if recreate:
 for (_, container) in service.recreate_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 else:
 for container in service.start_or_create_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 create_func = (service.recreate_containers if recreate
 else service.start_or_create_containers)

 for container in create_func(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)

 return running_containers

Expand Down

92 changes: 66 additions & 26 deletions
fig/service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

 def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
 """
 If a container for this service doesn't exist, create and start one. If there are
 any, stop them, create+start new ones, and remove the old containers.
 If a container for this service doesn't exist, create and start one. If
 there are any, stop them, create+start new ones, and remove the old
 containers.
 """
 containers = self.containers(stopped=True)
 if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
 do_build=do_build,
 **override_options)
 self.start_container(container)
 return [(None, container)]
 return [container]
 else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
 self.recreate_container(
 container,
 insecure_registry=insecure_registry,
**override_options)
 for container in containers
]

 def recreate_container(self, container, **override_options):
 """Recreate a container. An intermediate container is created so that
 the new container has the same name, while still supporting
 `volumes-from` the original container.
 """
 log.info("Recreating %s..." % container.name)
 try:
 container.stop()
 except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
 else:
 raise

 intermediate_options = dict(self.options, **override_options)
 intermediate_container = Container.create(
 self.client,
 image=container.image,
 entrypoint=['/bin/echo'],
 command=[],
 detach=True,
 )
 intermediate_container.start(volumes_from=container.id)
 intermediate_container.start(
 binds=get_container_data_volumes(
 container, intermediate_options.get('volumes')))
 intermediate_container.wait()
 container.remove()

 # TODO: volumes are being passed to both start and create, this is
 # probably unnecessary
 options = dict(override_options)
 new_container = self.create_container(do_build=False, **options)
 self.start_container(new_container, intermediate_container=intermediate_container)
 self.start_container(
 new_container,
 intermediate_container=intermediate_container)

 intermediate_container.remove()

 return (intermediate_container, new_container)
 return new_container

 def start_container_if_stopped(self, container, **options):
 if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
 def start_container(self, container, intermediate_container=None, **override_options):
 options = dict(self.options, **override_options)
 port_bindings = build_port_bindings(options.get('ports') or [])

 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in options.get('volumes') or []
 if ':' in volume)

 privileged = options.get('privileged', False)
 net = options.get('net', 'bridge')
 dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
 cap_drop = options.get('cap_drop', None)

 restart = parse_restart_spec(options.get('restart', None))
 binds = get_volume_bindings(
 options.get('volumes'), intermediate_container)

 container.start(
 links=self._get_links(link_to_self=options.get('one_off', False)),
 port_bindings=port_bindings,
 binds=volume_bindings,
 volumes_from=self._get_volumes_from(intermediate_container),
 binds=binds,
 volumes_from=self._get_volumes_from(),
 privileged=privileged,
 network_mode=net,
 dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
 links.append((external_link, link_name))
 return links

 def _get_volumes_from(self, intermediate_container=None):
 def _get_volumes_from(self):
 volumes_from = []
 for volume_source in self.volumes_from:
 if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
 elif isinstance(volume_source, Container):
 volumes_from.append(volume_source.id)

 if intermediate_container:
 volumes_from.append(intermediate_container.id)

 return volumes_from

 def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
 )


def get_container_data_volumes(container, volumes_option):
 """Find the container data volumes that are in `volumes_option`, and return
 a mapping of volume bindings for those volumes.
 """
 volumes = []
 for volume in volumes_option or []:
 volume = parse_volume_spec(volume)
 # No need to preserve host volumes
 if volume.external:
 continue

 volume_path = (container.get('Volumes') or {}).get(volume.internal)
 # New volume, doesn't exist in the old container
 if not volume_path:
 continue

 # Copy existing volume from old container
 volume = volume._replace(external=volume_path)
 volumes.append(build_volume_binding(volume))

 return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
 """Return a list of volume bindings for a container. Container data volume
 bindings are replaced by those in the intermediate container.
 """
 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in volumes_option or []
 if ':' in volume)

 if intermediate_container:
 volume_bindings.update(
 get_container_data_volumes(intermediate_container, volumes_option))

 return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down

35 changes: 19 additions & 16 deletions
tests/integration/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
 service = self.create_service('db', volumes=['/var/db'])
 container = service.create_container()
 service.start_container(container)
 self.assertIn('/var/db', container.inspect()['Volumes'])
 self.assertIn('/var/db', container.get('Volumes'))

 def test_create_container_with_cpu_shares(self):
 service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
 self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
 self.assertEqual(old_container.name, 'figtest_db_1')
 service.start_container(old_container)
 volume_path = old_container.inspect()['Volumes']['/etc']
 volume_path = old_container.get('Volumes')['/etc']

 num_containers_before = len(self.client.containers(all=True))

 service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
 self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
 self.assertEqual(len(containers), 1)

 intermediate_container = tuples[0][0]
 new_container = tuples[0][1]
 self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

 self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
 self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
 self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
 new_container = containers[0]
 self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
 self.assertEqual(new_container.get('Config.Cmd'), ['300'])
 self.assertIn('FOO=2', new_container.get('Config.Env'))
 self.assertEqual(new_container.name, 'figtest_db_1')
 self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
 self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

 self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
 self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
 service.recreate_containers()
 self.assertEqual(len(service.containers(stopped=True)), 1)

 def test_recreate_containers_with_volume_changes(self):
 service = self.create_service('withvolumes', volumes=['/etc'])
 old_container = create_and_start_container(service)
 self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

 service = self.create_service('withvolumes')
 container, = service.recreate_containers()
 service.start_container(container)
 self.assertEqual(container.get('Volumes'), {})

 def test_start_container_passes_through_options(self):
 db = self.create_service('db')
 create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down

59 changes: 49 additions & 10 deletions
tests/unit/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
 APIError,
 ConfigError,
 split_port,
 build_port_bindings,
 parse_volume_spec,
 build_volume_binding,
 APIError,
 get_container_data_volumes,
 get_volume_bindings,
 parse_repository_tag,
 parse_volume_spec,
 split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

 self.assertEqual(service._get_volumes_from(), [container_id])

 def test_get_volumes_from_intermediate_container(self):
 container_id = 'aabbccddee'
 service = Service('test')
 container = mock.Mock(id=container_id, spec=Container)

 self.assertEqual(service._get_volumes_from(container), [container_id])

 def test_get_volumes_from_service_container_exists(self):
 container_ids = ['aabbccddee', '12345']
 from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
 binding,
 ('/home/user', dict(bind='/home/user', ro=False)))

 def test_get_container_data_volumes(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/new/volume',
 '/existing/volume',
 ]

 container = Container(None, {
 'Volumes': {
 '/host/volume': '/host/volume',
 '/existing/volume': '/var/lib/docker/aaaaaaaa',
 '/removed/volume': '/var/lib/docker/bbbbbbbb',
 },
 }, has_been_inspected=True)

 expected = {
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_container_data_volumes(container, options)
 self.assertEqual(binds, expected)

 def test_get_volume_bindings(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/host/rw/volume:/host/rw/volume',
 '/new/volume',
 '/existing/volume',
 ]

 intermediate_container = Container(None, {
 'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
 }, has_been_inspected=True)

 expected = {
 '/host/volume': {'bind': '/host/volume', 'ro': True},
 '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_volume_bindings(options, intermediate_container)
 self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

 def setUp(self):
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Fix volumes on recreate by dnephin · Pull Request #711 · docker/compose · GitHub
Skip to content
Fix volumes on recreate#711

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

 Merged

aanand
merged 2 commits into

from

Jan 15, 2015
 Merged


Fix volumes on recreate
#711
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
26f45ef

Remove unused intermediate_container from return value.


dnephin Dec 6, 2014
7eb476e

Resolves #447, fix volume logic for recreate container


dnephin Dec 6, 2014

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


20 changes: 8 additions & 12 deletions
fig/project.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -176,18 +176,14 @@ def up(self,
 do_build=True):
 running_containers = []
 for service in self.get_services(service_names, include_links=start_links):
 if recreate:
 for (_, container) in service.recreate_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 else:
 for container in service.start_or_create_containers(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)
 create_func = (service.recreate_containers if recreate
 else service.start_or_create_containers)

 for container in create_func(
 insecure_registry=insecure_registry,
 detach=detach,
 do_build=do_build):
 running_containers.append(container)

 return running_containers

Expand Down

92 changes: 66 additions & 26 deletions
fig/service.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -242,8 +242,9 @@ def create_container(self,

 def recreate_containers(self, insecure_registry=False, do_build=True, **override_options):
 """
 If a container for this service doesn't exist, create and start one. If there are
 any, stop them, create+start new ones, and remove the old containers.
 If a container for this service doesn't exist, create and start one. If
 there are any, stop them, create+start new ones, and remove the old
 containers.
 """
 containers = self.containers(stopped=True)
 if not containers:
Expand All@@ -253,21 +254,22 @@ def recreate_containers(self, insecure_registry=False, do_build=True, **override
 do_build=do_build,
 **override_options)
 self.start_container(container)
 return [(None, container)]
 return [container]
 else:
tuples = []

for c in containers:
log.info("Recreating %s..." % c.name)
tuples.append(self.recreate_container(c, insecure_registry=insecure_registry, **override_options))

return tuples
return [
 self.recreate_container(
 container,
 insecure_registry=insecure_registry,
**override_options)
 for container in containers
]

 def recreate_container(self, container, **override_options):
 """Recreate a container. An intermediate container is created so that
 the new container has the same name, while still supporting
 `volumes-from` the original container.
 """
 log.info("Recreating %s..." % container.name)
 try:
 container.stop()
 except APIError as e:
Expand All@@ -278,24 +280,30 @@ def recreate_container(self, container, **override_options):
 else:
 raise

 intermediate_options = dict(self.options, **override_options)
 intermediate_container = Container.create(
 self.client,
 image=container.image,
 entrypoint=['/bin/echo'],
 command=[],
 detach=True,
 )
 intermediate_container.start(volumes_from=container.id)
 intermediate_container.start(
 binds=get_container_data_volumes(
 container, intermediate_options.get('volumes')))
 intermediate_container.wait()
 container.remove()

 # TODO: volumes are being passed to both start and create, this is
 # probably unnecessary
 options = dict(override_options)
 new_container = self.create_container(do_build=False, **options)
 self.start_container(new_container, intermediate_container=intermediate_container)
 self.start_container(
 new_container,
 intermediate_container=intermediate_container)

 intermediate_container.remove()

 return (intermediate_container, new_container)
 return new_container

 def start_container_if_stopped(self, container, **options):
 if container.is_running:
Expand All@@ -307,12 +315,6 @@ def start_container_if_stopped(self, container, **options):
 def start_container(self, container, intermediate_container=None, **override_options):
 options = dict(self.options, **override_options)
 port_bindings = build_port_bindings(options.get('ports') or [])

 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in options.get('volumes') or []
 if ':' in volume)

 privileged = options.get('privileged', False)
 net = options.get('net', 'bridge')
 dns = options.get('dns', None)
Expand All@@ -321,12 +323,14 @@ def start_container(self, container, intermediate_container=None, **override_opt
 cap_drop = options.get('cap_drop', None)

 restart = parse_restart_spec(options.get('restart', None))
 binds = get_volume_bindings(
 options.get('volumes'), intermediate_container)

 container.start(
 links=self._get_links(link_to_self=options.get('one_off', False)),
 port_bindings=port_bindings,
 binds=volume_bindings,
 volumes_from=self._get_volumes_from(intermediate_container),
 binds=binds,
 volumes_from=self._get_volumes_from(),
 privileged=privileged,
 network_mode=net,
 dns=dns,
Expand DownExpand Up@@ -388,7 +392,7 @@ def _get_links(self, link_to_self):
 links.append((external_link, link_name))
 return links

 def _get_volumes_from(self, intermediate_container=None):
 def _get_volumes_from(self):
 volumes_from = []
 for volume_source in self.volumes_from:
 if isinstance(volume_source, Service):
Expand All@@ -402,9 +406,6 @@ def _get_volumes_from(self, intermediate_container=None):
 elif isinstance(volume_source, Container):
 volumes_from.append(volume_source.id)

 if intermediate_container:
 volumes_from.append(intermediate_container.id)

 return volumes_from

 def _get_container_create_options(self, override_options, one_off=False):
Expand DownExpand Up@@ -519,6 +520,45 @@ def pull(self, insecure_registry=False):
 )


def get_container_data_volumes(container, volumes_option):
 """Find the container data volumes that are in `volumes_option`, and return
 a mapping of volume bindings for those volumes.
 """
 volumes = []
 for volume in volumes_option or []:
 volume = parse_volume_spec(volume)
 # No need to preserve host volumes
 if volume.external:
 continue

 volume_path = (container.get('Volumes') or {}).get(volume.internal)
 # New volume, doesn't exist in the old container
 if not volume_path:
 continue

 # Copy existing volume from old container
 volume = volume._replace(external=volume_path)
 volumes.append(build_volume_binding(volume))

 return dict(volumes)


def get_volume_bindings(volumes_option, intermediate_container):
 """Return a list of volume bindings for a container. Container data volume
 bindings are replaced by those in the intermediate container.
 """
 volume_bindings = dict(
 build_volume_binding(parse_volume_spec(volume))
 for volume in volumes_option or []
 if ':' in volume)

 if intermediate_container:
 volume_bindings.update(
 get_container_data_volumes(intermediate_container, volumes_option))

 return volume_bindings


NAME_RE = re.compile(r'^([^_]+)_([^_]+)_(run_)?(\d+)$')


Expand Down

35 changes: 19 additions & 16 deletions
tests/integration/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -98,7 +98,7 @@ def test_create_container_with_unspecified_volume(self):
 service = self.create_service('db', volumes=['/var/db'])
 container = service.create_container()
 service.start_container(container)
 self.assertIn('/var/db', container.inspect()['Volumes'])
 self.assertIn('/var/db', container.get('Volumes'))

 def test_create_container_with_cpu_shares(self):
 service = self.create_service('db', cpu_shares=73)
Expand DownExpand Up@@ -148,30 +148,23 @@ def test_recreate_containers(self):
 self.assertIn('FOO=1', old_container.dictionary['Config']['Env'])
 self.assertEqual(old_container.name, 'figtest_db_1')
 service.start_container(old_container)
 volume_path = old_container.inspect()['Volumes']['/etc']
 volume_path = old_container.get('Volumes')['/etc']

 num_containers_before = len(self.client.containers(all=True))

 service.options['environment']['FOO'] = '2'
tuples = service.recreate_containers()
 self.assertEqual(len(tuples), 1)
containers = service.recreate_containers()
 self.assertEqual(len(containers), 1)

 intermediate_container = tuples[0][0]
 new_container = tuples[0][1]
 self.assertEqual(intermediate_container.dictionary['Config']['Entrypoint'], ['/bin/echo'])

 self.assertEqual(new_container.dictionary['Config']['Entrypoint'], ['sleep'])
 self.assertEqual(new_container.dictionary['Config']['Cmd'], ['300'])
 self.assertIn('FOO=2', new_container.dictionary['Config']['Env'])
 new_container = containers[0]
 self.assertEqual(new_container.get('Config.Entrypoint'), ['sleep'])
 self.assertEqual(new_container.get('Config.Cmd'), ['300'])
 self.assertIn('FOO=2', new_container.get('Config.Env'))
 self.assertEqual(new_container.name, 'figtest_db_1')
 self.assertEqual(new_container.inspect()['Volumes']['/etc'], volume_path)
 self.assertIn(intermediate_container.id, new_container.dictionary['HostConfig']['VolumesFrom'])
 self.assertEqual(new_container.get('Volumes')['/etc'], volume_path)

 self.assertEqual(len(self.client.containers(all=True)), num_containers_before)
 self.assertNotEqual(old_container.id, new_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(
Expand All@@ -186,6 +179,16 @@ def test_recreate_containers_when_containers_are_stopped(self):
 service.recreate_containers()
 self.assertEqual(len(service.containers(stopped=True)), 1)

 def test_recreate_containers_with_volume_changes(self):
 service = self.create_service('withvolumes', volumes=['/etc'])
 old_container = create_and_start_container(service)
 self.assertEqual(old_container.get('Volumes').keys(), ['/etc'])

 service = self.create_service('withvolumes')
 container, = service.recreate_containers()
 service.start_container(container)
 self.assertEqual(container.get('Volumes'), {})

 def test_start_container_passes_through_options(self):
 db = self.create_service('db')
 create_and_start_container(db, environment={'FOO': 'BAR'})
Expand Down

59 changes: 49 additions & 10 deletions
tests/unit/service_test.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,13 +11,15 @@
from fig import Service
from fig.container import Container
from fig.service import (
 APIError,
 ConfigError,
 split_port,
 build_port_bindings,
 parse_volume_spec,
 build_volume_binding,
 APIError,
 get_container_data_volumes,
 get_volume_bindings,
 parse_repository_tag,
 parse_volume_spec,
 split_port,
)


Expand DownExpand Up@@ -57,13 +59,6 @@ def test_get_volumes_from_container(self):

 self.assertEqual(service._get_volumes_from(), [container_id])

 def test_get_volumes_from_intermediate_container(self):
 container_id = 'aabbccddee'
 service = Service('test')
 container = mock.Mock(id=container_id, spec=Container)

 self.assertEqual(service._get_volumes_from(container), [container_id])

 def test_get_volumes_from_service_container_exists(self):
 container_ids = ['aabbccddee', '12345']
 from_service = mock.create_autospec(Service)
Expand DownExpand Up@@ -288,6 +283,50 @@ def test_building_volume_binding_with_home(self):
 binding,
 ('/home/user', dict(bind='/home/user', ro=False)))

 def test_get_container_data_volumes(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/new/volume',
 '/existing/volume',
 ]

 container = Container(None, {
 'Volumes': {
 '/host/volume': '/host/volume',
 '/existing/volume': '/var/lib/docker/aaaaaaaa',
 '/removed/volume': '/var/lib/docker/bbbbbbbb',
 },
 }, has_been_inspected=True)

 expected = {
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_container_data_volumes(container, options)
 self.assertEqual(binds, expected)

 def test_get_volume_bindings(self):
 options = [
 '/host/volume:/host/volume:ro',
 '/host/rw/volume:/host/rw/volume',
 '/new/volume',
 '/existing/volume',
 ]

 intermediate_container = Container(None, {
 'Volumes': {'/existing/volume': '/var/lib/docker/aaaaaaaa'},
 }, has_been_inspected=True)

 expected = {
 '/host/volume': {'bind': '/host/volume', 'ro': True},
 '/host/rw/volume': {'bind': '/host/rw/volume', 'ro': False},
 '/var/lib/docker/aaaaaaaa': {'bind': '/existing/volume', 'ro': False},
 }

 binds = get_volume_bindings(options, intermediate_container)
 self.assertEqual(binds, expected)


class ServiceEnvironmentTest(unittest.TestCase):

 def setUp(self):
Expand Down