Skip to content

Commit

Permalink
address some review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dwgebler committed Jul 7, 2015
1 parent 5caa920 commit d60b09a
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
5 changes: 3 additions & 2 deletions flocker/node/_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,14 @@ def exists(unit_name):
otherwise ``False``.
"""

def remove(unit_name):
def remove(unit_name, force):

This comment has been minimized.

Copy link
@wallrj

wallrj Jul 10, 2015

Contributor

I'd have thought zope.verify would have caught this, but the signature in the interface should match the signature in the implementation, so the interface ought to have force=False.

"""
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.

This comment has been minimized.

Copy link
@wallrj

wallrj Jul 10, 2015

Contributor

And maybe add a note about the default behaviour to the docstring.

:return: ``Deferred`` that fires once the unit has been stopped
and removed.
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions flocker/node/functional/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def test_force_remove(self):
expected_states = (u'active',)
d = client.add(
unit_name=unit_name,
image_name=u'clusterhq/flask'
image_name=u"openshift/busybox-http-app"
)
d.addCallback(
lambda _: wait_for_unit_state(
Expand All @@ -162,7 +162,7 @@ def check_is_listed(_):
listed = client.list()

def check_unit(results):
self.assertTrue(unit_name in [unit.name for unit in results])
self.assertIn(unit_name, [unit.name for unit in results])
listed.addCallback(check_unit)
return listed

Expand All @@ -173,7 +173,7 @@ def check_is_removed(_):
listed = client.list()

def check_unit(results):
self.assertFalse(unit_name in [unit.name for unit in results])
self.assertNotIn(unit_name, [unit.name for unit in results])
listed.addCallback(check_unit)
return listed

Expand Down
25 changes: 25 additions & 0 deletions flocker/node/test/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()``.
Expand Down

0 comments on commit d60b09a

Please sign in to comment.