diff --git a/flocker/node/_docker.py b/flocker/node/_docker.py index 594148a87f..9ad2c01a4c 100644 --- a/flocker/node/_docker.py +++ b/flocker/node/_docker.py @@ -208,13 +208,14 @@ def exists(unit_name): otherwise ``False``. """ - def remove(unit_name): + def remove(unit_name, force): """ Stop and delete the given unit. This can be done multiple times in a row for the same unit. :param unicode unit_name: The name of the unit to stop. + :param bool force: Force removal of the unit. :return: ``Deferred`` that fires once the unit has been stopped and removed. @@ -271,7 +272,7 @@ def add(self, unit_name, image_name, ports=frozenset(), environment=None, def exists(self, unit_name): return succeed(unit_name in self._units) - def remove(self, unit_name): + def remove(self, unit_name, force=False): if unit_name in self._units: del self._units[unit_name] return succeed(None) @@ -546,7 +547,7 @@ def _blocking_container_runs(self, container_name): ).write() return result['State']['Running'] - def remove(self, unit_name): + def remove(self, unit_name, force=False): container_name = self._to_container_name(unit_name) def _remove(): @@ -608,7 +609,7 @@ def _remove(): message_type="flocker:docker:container_remove", container=container_name ).write() - self._client.remove_container(container_name) + self._client.remove_container(container_name, force=force) Message.new( message_type="flocker:docker:container_removed", container=container_name diff --git a/flocker/node/functional/test_docker.py b/flocker/node/functional/test_docker.py index b0b2121790..036d8e1025 100644 --- a/flocker/node/functional/test_docker.py +++ b/flocker/node/functional/test_docker.py @@ -99,7 +99,7 @@ def start_container(self, unit_name, environment=None, volumes=(), mem_limit=None, cpu_shares=None, restart_policy=RestartNever(), - command_line=None): + command_line=None, force=False): """ Start a unit and wait until it reaches the `active` state or the supplied `expected_state`. @@ -114,6 +114,8 @@ def start_container(self, unit_name, :param cpu_shares: See ``IDockerClient.add``. :param restart_policy: See ``IDockerClient.add``. :param command_line: See ``IDockerClient.add``. + :param bool force: Force container removal via SIGKILL + during cleanup. :return: ``Deferred`` that fires with the ``DockerClient`` when the unit reaches the expected state. @@ -130,7 +132,7 @@ def start_container(self, unit_name, restart_policy=restart_policy, command_line=command_line, ) - self.addCleanup(client.remove, unit_name) + self.addCleanup(client.remove, unit_name, force) d.addCallback(lambda _: wait_for_unit_state(client, unit_name, expected_states)) @@ -138,6 +140,59 @@ def start_container(self, unit_name, return d + def test_force_remove(self): + """ + ``DockerClient.remove`` can forcefully remove a running container + when the ``force`` parameter is set to True. + + This is intended to help avoid an intermittent error in other + tests caused by a Docker behaviour where an "device busy" error + occurs when attempting to remove a container. + + In many cases, it is reported that forced removal of the container + via the Docker API or "-f" CLI switch is a work-around that prevents + this error, though is not a solution to the underlying problem. + + See https://github.com/docker/docker/issues/9665 + See http://blog.hashbangbash.com/2014/11/ + docker-devicemapper-fix-for-device-or-resource-busy-ebusy/ + See FLOC-1194. + """ + client = DockerClient() + unit_name = u'test-force-remove' + expected_states = (u'active',) + d = client.add( + unit_name=unit_name, + image_name=u"openshift/busybox-http-app" + ) + d.addCallback( + lambda _: wait_for_unit_state( + client, unit_name, expected_states + ) + ) + + def check_is_listed(_): + listed = client.list() + + def check_unit(results): + self.assertIn(unit_name, [unit.name for unit in results]) + listed.addCallback(check_unit) + return listed + + d.addCallback(check_is_listed) + d.addCallback(lambda _: client.remove(unit_name, force=True)) + + def check_is_removed(_): + listed = client.list() + + def check_unit(results): + self.assertNotIn(unit_name, [unit.name for unit in results]) + listed.addCallback(check_unit) + return listed + + d.addCallback(check_is_removed) + return d + def test_default_base_url(self): """ ``DockerClient`` instantiated with a default base URL for a socket @@ -356,6 +411,7 @@ def image_built(image_name): unit_name=unit_name, image_name=image_name, environment=Environment(variables=expected_variables), + force=True, ) d.addCallback(image_built) diff --git a/flocker/node/test/test_docker.py b/flocker/node/test/test_docker.py index c3a8b9b232..cf3227ab16 100644 --- a/flocker/node/test/test_docker.py +++ b/flocker/node/test/test_docker.py @@ -117,6 +117,31 @@ def test_removed_does_not_exist(self): d.addCallback(self.assertFalse) return d + def test_forced_removed_does_not_exist(self): + """ + A container removed with the ``force`` switch does not exist. + """ + client = fixture(self) + name = random_name(self) + image = u"openshift/busybox-http-app" + d = client.add(name, image) + d.addCallback(lambda _: client.list()) + expected = Unit( + name=name, container_name=name, activation_state=u"active", + container_image=image, + ) + + def got_units(units): + result = units.pop() + result = result.set("container_name", name) + self.assertEqual(expected, result) + + d.addCallback(got_units) + d.addCallback(lambda _: client.remove(name, force=True)) + d.addCallback(lambda _: client.exists(name)) + d.addCallback(self.assertFalse) + return d + def test_added_is_listed(self): """ An added container is included in the output of ``list()``.