Merge lp:~bcsaller/pyjuju/subordinate-removes into lp:pyjuju

[Benjamin Saller]

Destroying a subordinate shouldn't result in tracebacks

Destroying a subordinate service after destroying its primary services results in a traceback
This patch corrects this. The associated test also previously failed to exercise all the needed
behavior to demonstrate this, which is corrected as well.

https://codereview.appspot.com/6256054/

[Benjamin Saller (bcsaller)]

Please take a look.

[Benjamin Saller (bcsaller)]

Reviewers: mp+102427_code.launchpad.net,

Message:
Please take a look.

Description:
Destroying a subordinate shouldn't result in tracebacks

Destroying a subordinate service after destroying its primary services
results in a traceback
This patch corrects this. The associated test also previously failed to
exercise all the needed
behavior to demonstrate this, which is corrected as well.

https://code.launchpad.net/~bcsaller/juju/subordinate-removes/+merge/102427

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6256054/

Affected files:
   A [revision details]
   M juju/control/destroy_service.py
   M juju/control/remove_relation.py
   M juju/control/tests/test_destroy_service.py
   M juju/control/tests/test_remove_relation.py
   M juju/hooks/tests/test_invoker.py
   M juju/lib/testing.py
   M juju/state/tests/test_firewall.py
   M juju/state/tests/test_service.py
   M juju/unit/lifecycle.py
   M juju/unit/tests/test_lifecycle.py
   M juju/unit/workflow.py

[Kapil Thangavelu (hazmat)]

Needs Fixing

There seem to be two separate implementations of unrelated issues here
that would ideally be in separate branches. One for open/close port. And
one for subordinate suicide on removal. I'm focusing this review on
subordinate suicide aspect.

https://codereview.appspot.com/6256054/diff/1/juju/control/remove_relation.py
File juju/control/remove_relation.py (right):

https://codereview.appspot.com/6256054/diff/1/juju/control/remove_relation.py#newcode72
juju/control/remove_relation.py:72: has_principal = True
this should default to false, right?

https://codereview.appspot.com/6256054/diff/1/juju/control/remove_relation.py#newcode79
juju/control/remove_relation.py:79: service_pair.insert(0, service)
pls document here why the subordinate is coming first in the ep pair.
its easy to gloss over this is just an error message formatting thing.

https://codereview.appspot.com/6256054/diff/1/juju/control/tests/test_destroy_service.py
File juju/control/tests/test_destroy_service.py (right):

https://codereview.appspot.com/6256054/diff/1/juju/control/tests/test_destroy_service.py#newcode138
juju/control/tests/test_destroy_service.py:138: # resulted in a
traceback
This doesn't actually test anything related to the branch afaics. The
actual problem is asynchronously triggered.

https://codereview.appspot.com/6256054/diff/1/juju/hooks/tests/test_invoker.py
File juju/hooks/tests/test_invoker.py (right):

https://codereview.appspot.com/6256054/diff/1/juju/hooks/tests/test_invoker.py#newcode1480
juju/hooks/tests/test_invoker.py:1480: """Verify that port hook commands
run and changes are immediate."""
does this change belong in this branch?

https://codereview.appspot.com/6256054/diff/1/juju/state/tests/test_firewall.py
File juju/state/tests/test_firewall.py (right):

https://codereview.appspot.com/6256054/diff/1/juju/state/tests/test_firewall.py#newcode189
juju/state/tests/test_firewall.py:189: """Verify that opening/closing
ports triggers the appropriate firewall
does this change belong in this branch?

https://codereview.appspot.com/6256054/diff/1/juju/unit/lifecycle.py
File juju/unit/lifecycle.py (right):

https://codereview.appspot.com/6256054/diff/1/juju/unit/lifecycle.py#newcode458
juju/unit/lifecycle.py:458: def _undeploy(self, service_relation):
method name is a bit ambigious its always a suicide op, ie.
_undeploy_self..

https://codereview.appspot.com/6256054/diff/1/juju/unit/lifecycle.py#newcode459
juju/unit/lifecycle.py:459: """Conditionally stop and remove _this_
agents deployment.
s/agents/subordinate agent

https://codereview.appspot.com/6256054/diff/1/juju/unit/lifecycle.py#newcode500
juju/unit/lifecycle.py:500: yield unit_deployer.start("subordinate")
are you sure about that? i don't see anything obvious there, if so its a
bug in the deployer.. start would imply deploying again.. which is
bogus.

https://codereview.appspot.com/6256054/diff/1/juju/unit/workflow.py
File juju/unit/workflow.py (right):

https://codereview.appspot.com/6256054/diff/1/juju/unit/workflow.py#newcode229
juju/unit/workflow.py:229: if not stat:
how does this help? what if the node is already gone when the
retry_change fires. also untested.

https://codereview...

[lp:~bcsaller/pyjuju/subordinate-removes]

537. By Benjamin Saller on 2012-05-31

typo

538. By Benjamin Saller on 2012-05-31

merge trunk

539. By Benjamin Saller on 2012-05-31

move ports handling to another branch

540. By Benjamin Saller on 2012-05-31

verify unit state removal on subordinate suicide

541. By Benjamin Saller on 2012-06-04

catch another async failure location

542. By Benjamin Saller on 2012-06-04

missing import

543. By Benjamin Saller on 2012-06-04

missing import

[Kapil Thangavelu (hazmat)]

so post discussions, i believe we've said this branch/approach is dead in favor of the stop protocol spec.

Unmerged revisions

543. By Benjamin Saller on 2012-06-04

missing import

542. By Benjamin Saller on 2012-06-04

missing import

541. By Benjamin Saller on 2012-06-04

catch another async failure location

540. By Benjamin Saller on 2012-05-31

verify unit state removal on subordinate suicide

539. By Benjamin Saller on 2012-05-31

move ports handling to another branch

538. By Benjamin Saller on 2012-05-31

merge trunk

537. By Benjamin Saller on 2012-05-31

typo

536. By Benjamin Saller on 2012-05-22

remove print

535. By Benjamin Saller on 2012-05-22

add back in subordinate removal, this will have to change to honor the normal flow control model later. When stop hooks are properly added this will also need another review

534. By Benjamin Saller on 2012-05-02

change arg. order for error message

=== modified file 'juju/control/destroy_service.py'
--- juju/control/destroy_service.py	2012-04-11 17:48:26 +0000
+++ juju/control/destroy_service.py	2012-06-04 23:05:24 +0000
@@ -47,14 +47,14 @@
 
         if relations:
             principal_service = None
-            # if we have a container we can destroy the subordinate
+            # if we have a container we can't destroy the subordinate
             # (revisit in the future)
             for relation in relations:
                 if relation.relation_scope != "container":
                     continue
                 services = yield relation.get_service_states()
                 remote_service = [s for s in services if s.service_name !=
-                                     service_state.service_name][0]
+                                  service_state.service_name][0]
                 if not (yield remote_service.is_subordinate()):
                     principal_service = remote_service
                     break

=== modified file 'juju/control/remove_relation.py'
--- juju/control/remove_relation.py	2012-04-12 09:23:50 +0000
+++ juju/control/remove_relation.py	2012-06-04 23:05:24 +0000
@@ -69,17 +69,19 @@
     service_pair = []  # ordered such that sub, principal
 
     is_container = False
-    has_principal = True
+    has_principal = False
+    # Sort the endpoints so we know which is the sub and which is the
+    # principal in the event we need to raise an error below
     for ep in endpoints:
         if ep.relation_scope == "container":
             is_container = True
         service = yield service_state_manager.get_service_state(
             ep.service_name)
         if (yield service.is_subordinate()):
-            service_pair.append(service)
+            service_pair.insert(0, service)
             has_principal = True
         else:
-            service_pair.insert(0, service)
+            service_pair.append(service)
     if is_container and len(service_pair) == 2 and has_principal:
         sub, principal = service_pair
         raise UnsupportedSubordinateServiceRemoval(sub.service_name,

=== modified file 'juju/control/tests/test_destroy_service.py'
--- juju/control/tests/test_destroy_service.py	2012-04-10 10:26:54 +0000
+++ juju/control/tests/test_destroy_service.py	2012-06-04 23:05:24 +0000
@@ -94,13 +94,15 @@
     @inlineCallbacks
     def test_destroy_principal_with_subordinates(self):
         log_service = yield self.add_service_from_charm("logging")
-        lu1 = yield log_service.add_unit_state()
 
         yield self.add_relation(
             "juju-info",
+            "container",
             (self.service_state1, "juju-info", "server"),
             (log_service, "juju-info", "client"))
 
+        lu1 = yield log_service.add_unit_state(self.service1_unit)
+
         finished = self.setup_cli_reactor()
         topology = yield self.get_topology()
         service_id = topology.find_service_with_name("mysql")
@@ -111,9 +113,10 @@
 
         self.setup_exit(0)
         self.mocker.replay()
-
         main(["destroy-service", "mysql"])
+
         yield finished
+
         service_id = topology.find_service_with_name("mysql")
 
         topology = yield self.get_topology()
@@ -128,9 +131,21 @@
         # Zookeeper. see test_destroy_subordinate_without_relations.
         exists = yield self.client.exists("/services/%s" % logging_id)
         self.assertTrue(exists)
-
         self.assertIn("Service 'mysql' destroyed.", self.output.getvalue())
 
+        # At this point trigger a destroy on the subordinate
+        # this deals with issue #982353 where partial state
+        # resulted in a traceback
+        # Assert that methods related to destruction fire w/o exception
+        self.assertEquals(True, (yield log_service.is_subordinate()))
+        self.assertEquals(None, (yield lu1.get_container()))
+        rels = yield self.relation_state_manager.get_relations_for_service(
+            log_service)
+        self.assertEquals([], rels)
+        yield self.service_state_manager.remove_service_state(log_service)
+        exists = yield self.client.exists("/services/%s" % logging_id)
+        self.assertFalse(exists)
+
     @inlineCallbacks
     def test_destroy_subordinate_without_relations(self):
         """Verify we can remove a subordinate w/o relations."""
@@ -146,7 +161,6 @@
 
         main(["destroy-service", "logging"])
         yield finished
-
         topology = yield self.get_topology()
         self.assertFalse(topology.has_service(logging_id))
         exists = yield self.client.exists("/services/%s" % logging_id)

=== modified file 'juju/control/tests/test_remove_relation.py'
--- juju/control/tests/test_remove_relation.py	2012-04-10 22:26:36 +0000
+++ juju/control/tests/test_remove_relation.py	2012-06-04 23:05:24 +0000
@@ -210,6 +210,6 @@
         main(["remove-relation", "logging", "wordpress"])
         yield wait_on_reactor_stopped
         self.assertIn("Unsupported attempt to destroy "
-                      "subordinate service 'wordpress' while "
-                      "principal service 'logging' is related.",
+                      "subordinate service 'logging' while "
+                      "principal service 'wordpress' is related.",
                       self.output.getvalue())

=== modified file 'juju/hooks/tests/test_invoker.py'
--- juju/hooks/tests/test_invoker.py	2012-05-04 03:36:36 +0000
+++ juju/hooks/tests/test_invoker.py	2012-06-04 23:05:24 +0000
@@ -1467,6 +1467,8 @@
 
         mu1, mu2 = my_units
         lu1, lu2 = log_units
+        self.mysql_units = my_units
+        self.log_units = log_units
 
         mystate = pick_attr(service_states, relation_role="server")
         logstate = pick_attr(service_states, relation_role="client")

=== modified file 'juju/lib/testing.py'
--- juju/lib/testing.py	2012-04-06 14:42:06 +0000
+++ juju/lib/testing.py	2012-06-04 23:05:24 +0000
@@ -170,19 +170,19 @@
             data, stat = yield client.get(path)
             name = path.rsplit('/', 1)[1]
 
-            d['contents'] = _decode_fmt(data, yaml.load)
+            d["contents"] = _decode_fmt(data, yaml.load)
 
             children = yield client.get_children(path)
             for name in children:
                 if path == "/" and name == "zookeeper":
                     continue
 
-                cd = yield export_tree(path + '/' + name, indent)
+                cd = yield export_tree(path + "/" + name, indent)
                 d[name] = cd
 
             returnValue(d)
 
-        output[path.rsplit('/', 1)[1]] = yield export_tree(path, '')
+        output[path.rsplit("/", 1)[1]] = yield export_tree(path, "")
         returnValue(output)
 
     @inlineCallbacks

=== modified file 'juju/state/hook.py'
--- juju/state/hook.py	2012-04-09 19:40:57 +0000
+++ juju/state/hook.py	2012-06-04 23:05:24 +0000
@@ -4,8 +4,9 @@
 
 from juju.state.base import StateBase
 from juju.state.errors import (
-    UnitRelationStateNotFound, StateNotFound, RelationBrokenContextError,
-    RelationStateNotFound, InvalidRelationIdentity, StateChanged)
+    ServiceStateNotFound, UnitRelationStateNotFound, StateNotFound,
+    RelationBrokenContextError, RelationStateNotFound,
+    InvalidRelationIdentity, StateChanged)
 from juju.state.relation import (
     RelationStateManager, ServiceRelationState, UnitRelationState)
 from juju.state.service import ServiceStateManager, parse_service_name
@@ -102,7 +103,12 @@
         relations = []
         if self._topology is None:
             self._topology = yield self._read_topology()
-        service = yield self.get_local_service()
+        try:
+            service = yield self.get_local_service()
+        except ServiceStateNotFound:
+            # The service has been removed
+            returnValue(relations)
+
         internal_service_id = service.internal_id
         for info in self._topology.get_relations_for_service(
                 internal_service_id):

=== modified file 'juju/state/tests/test_service.py'
--- juju/state/tests/test_service.py	2012-04-11 19:38:41 +0000
+++ juju/state/tests/test_service.py	2012-06-04 23:05:24 +0000
@@ -159,6 +159,43 @@
         unit_state = yield service_state.add_unit_state()
         returnValue(unit_state)
 
+    @inlineCallbacks
+    def setup_subordinate_defaults(self):
+        mysql_ep = RelationEndpoint("mysql", "juju-info", "juju-info",
+                                    "server", "global")
+        logging_ep = RelationEndpoint("logging", "juju-info", "juju-info",
+                                      "client", "container")
+
+        logging_charm_state = yield self.get_subordinate_charm()
+        self.assertTrue(logging_charm_state.is_subordinate())
+        log_state = yield self.service_state_manager.add_service_state(
+                "logging", logging_charm_state, dummy_constraints)
+        mysql_state = yield self.service_state_manager.add_service_state(
+                "mysql", self.charm_state, dummy_constraints)
+
+        m1 = yield self.machine_state_manager.add_machine_state(
+            series_constraints)
+        m2 = yield self.machine_state_manager.add_machine_state(
+            series_constraints)
+
+        relation_state, service_states = (yield
+                self.relation_state_manager.add_relation_state(
+                        mysql_ep, logging_ep))
+
+        unit_state1 = yield mysql_state.add_unit_state()
+        yield unit_state1.assign_to_machine(m1)
+        unit_state0 = yield log_state.add_unit_state(
+            container=unit_state1)
+
+        unit_state3 = yield mysql_state.add_unit_state()
+        yield unit_state3.assign_to_machine(m2)
+        unit_state2 = yield log_state.add_unit_state(
+            container=unit_state3)
+
+        returnValue([service_states, relation_state,
+                     [(unit_state0, unit_state1),
+                      (unit_state2, unit_state3)]])
+
 
 class ServiceStateManagerTest(ServiceStateManagerTestBase):
 
@@ -455,27 +492,9 @@
         """
         Validate adding units with containers specified and recovering that.
         """
-        mysql_ep = RelationEndpoint("mysql", "juju-info", "juju-info",
-                                    "server", "global")
-        logging_ep = RelationEndpoint("logging", "juju-info", "juju-info",
-                                      "client", "container")
-
-        logging_charm_state = yield self.get_subordinate_charm()
-        self.assertTrue(logging_charm_state.is_subordinate())
-        log_state = yield self.service_state_manager.add_service_state(
-                "logging", logging_charm_state, dummy_constraints)
-        mysql_state = yield self.service_state_manager.add_service_state(
-                "mysql", self.charm_state, dummy_constraints)
-
-        relation_state, service_states = (yield
-                self.relation_state_manager.add_relation_state(
-                        mysql_ep, logging_ep))
-
-        unit_state1 = yield mysql_state.add_unit_state()
-        unit_state0 = yield log_state.add_unit_state(container=unit_state1)
-
-        unit_state3 = yield mysql_state.add_unit_state()
-        unit_state2 = yield log_state.add_unit_state(container=unit_state3)
+        service_states, relation_state, unit_states = (
+            yield self.setup_subordinate_defaults())
+        (unit_state0, unit_state1), (unit_state2, unit_state3) = unit_states
 
         self.assertEquals((yield unit_state1.get_container()), None)
         self.assertEquals((yield unit_state0.get_container()), unit_state1)
@@ -498,7 +517,7 @@
 
         @inlineCallbacks
         def verify_container(relation_state, service_relation_state,
-                                                 unit_state, container):
+                             unit_state, container):
             presence_path = self.get_presence_path(
                     relation_state,
                     service_relation_state.relation_role,
@@ -533,6 +552,11 @@
             # we verify the content elsewhere
             self.assertTrue(settings_info["private-address"])
 
+            mid = yield unit_state.get_assigned_machine_id()
+            cmid = yield container.get_assigned_machine_id()
+            if container:
+                self.assertEqual(mid, cmid)
+
         # verify all the units are constructed as expected
         # first the client roles with another container
         yield verify_container(relation_state, logstate,
@@ -2540,6 +2564,38 @@
         self.assertEqual(exposed_flag, False)
 
     @inlineCallbacks
+    def test_set_and_clear_exposed_flag_subordinate(self):
+        """An exposed flag can be set on a subordinate service."""
+        service_states, relation_state, unit_states = (
+            yield self.setup_subordinate_defaults())
+        (u0, u1), (u2, u3) = unit_states
+        service_state = yield u1.get_service_state()
+
+        # Defaults to false
+        exposed_flag = yield service_state.get_exposed_flag()
+        self.assertEqual(exposed_flag, False)
+
+        # Can be set
+        yield service_state.set_exposed_flag()
+        exposed_flag = yield service_state.get_exposed_flag()
+        self.assertEqual(exposed_flag, True)
+
+        # Can be set multiple times
+        yield service_state.set_exposed_flag()
+        exposed_flag = yield service_state.get_exposed_flag()
+        self.assertEqual(exposed_flag, True)
+
+        # Can be cleared
+        yield service_state.clear_exposed_flag()
+        exposed_flag = yield service_state.get_exposed_flag()
+        self.assertEqual(exposed_flag, False)
+
+        # Can be cleared multiple times
+        yield service_state.clear_exposed_flag()
+        exposed_flag = yield service_state.get_exposed_flag()
+        self.assertEqual(exposed_flag, False)
+
+    @inlineCallbacks
     def test_watch_exposed_flag(self):
         """An exposed watch is setup on a permanent basis."""
         service_state = yield self.add_service("wordpress")

=== modified file 'juju/unit/lifecycle.py'
--- juju/unit/lifecycle.py	2012-05-01 00:26:25 +0000
+++ juju/unit/lifecycle.py	2012-06-04 23:05:24 +0000
@@ -106,6 +106,7 @@
         self._state_dir = state_dir
         self._relations = None
         self._running = False
+        self._remote_service = {}
         self._watching_relation_memberships = False
         self._watching_relation_resolved = False
         self._run_lock = DeferredLock()
@@ -404,6 +405,14 @@
                 yield workflow.transition_state("departed")
             self._store_relations()
 
+            if not is_principal:
+                service_relation = old_relations[relation_id]
+                removed = yield self._undeploy_subordinate(service_relation)
+                if removed:
+                    # we don't even need to process adds at this point
+                    # (that would be a sick, sad world)
+                    returnValue(None)
+
         # Process new relations.
         for relation_id in added:
             service_relation = new_relations[relation_id]
@@ -434,10 +443,65 @@
 
         self._relations[service_relation.internal_relation_id] = workflow
 
+        # Cache the remote unit's service (to ease subordinate undeploy)
+        service_states = yield service_relation.get_service_states()
+        remote_service = [s for s in service_states if
+                               s.service_name != self._unit.service_name][0]
+
+        self._remote_service[ \
+            service_relation.internal_relation_id] = remote_service
+
         with (yield workflow.lock()):
             yield workflow.synchronize()
 
     @inlineCallbacks
+    def _undeploy_subordinate(self, service_relation):
+        """Conditionally stop and remove _this_ subordinate unit.
+
+        When a relationship is removed we check if its with this subordinate's
+        principal service. If it is we need to stop the agent and remove the
+        on disk deployment.
+
+        returns boolean indicating if we have self-terminated or not.
+        """
+        # determine if the remote end of this service relation is the
+        # container of this unit (it could be a scope:container relation under
+        # the same principal).
+
+        container = yield self._unit.get_container()
+        container_service = yield container.get_service_state()
+        # We cannot use service_relation.get_service_states() as we're
+        # reacting to relation removal and its no longer in the topology.
+        # (forcing us to manage this cache)
+        remote_service = self._remote_service[
+            service_relation.internal_relation_id]
+
+        if container is None or (
+            remote_service.internal_id == container_service.internal_id):
+            # The container has already been removed we can safely proceed
+            # OR
+            # The remote side of the relationship being removed is the
+            # principal.
+            # At this point we will being shutting down this unit,
+            # remove our unit state
+            self._log.debug("Removing unit state for %s", self._unit)
+            yield self._service.remove_unit_state(self._unit)
+            # the final problem
+            yield self._do_unit_kill(self._unit.unit_name, "",
+                                     self._unit_dir)
+            returnValue(True)
+
+        returnValue(False)
+
+    @inlineCallbacks
+    def _do_unit_kill(self, unit_name, machine_id, charm_dir):
+        unit_deployer = UnitDeployer(self._client, machine_id, charm_dir)
+        # start the unit deployer (this is needed to bootstrap the factory)
+        # kill ourself
+        yield unit_deployer.start("subordinate")
+        yield unit_deployer.kill_service_unit(unit_name)
+
+    @inlineCallbacks
     def _do_unit_deploy(self, unit_name, machine_id, charm_dir):
         # this method exists to aid testing rather than being an
         # inline

=== modified file 'juju/unit/tests/test_lifecycle.py'
--- juju/unit/tests/test_lifecycle.py	2012-05-04 03:39:27 +0000
+++ juju/unit/tests/test_lifecycle.py	2012-06-04 23:05:24 +0000
@@ -23,7 +23,8 @@
 from juju.machine.tests.test_constraints import series_constraints
 
 from juju.state.endpoint import RelationEndpoint
-from juju.state.errors import UnitRelationStateNotFound
+from juju.state.errors import (UnitRelationStateNotFound,
+                               ServiceUnitStateNotFound)
 from juju.state.machine import MachineStateManager
 from juju.state.relation import ClientServerUnitWatcher
 from juju.state.service import NO_HOOKS
@@ -34,7 +35,7 @@
 from juju.unit.tests.test_charm import CharmPublisherTestBase
 
 from juju.lib.testing import TestCase
-from juju.lib.mocker import MATCH
+from juju.lib.mocker import MATCH, ANY, Mocker
 
 
 class UnwriteablePath(object):
@@ -1441,3 +1442,76 @@
         yield lifecycle.start()
         yield test_deferred
         # mocker has already verified the call signature, we're happy
+
+    @inlineCallbacks
+    def test_subordinate_lifecycle_stop_principal(self):
+        state = yield self.setup_default_subordinate_relation()
+        mu1, my_srv = state["principal"]
+        lu1, log_srv = state["subordinate"]
+
+        # Principal setup
+        principal_lifecycle = UnitLifecycle(
+            self.client, mu1, my_srv,
+            self.unit_directory, self.state_directory, self.executor)
+
+        test_deferred = Deferred()
+        principal_results = []
+
+        def test_deploy(unit_name, machine_id, charm_dir):
+            principal_results.append([unit_name, machine_id, charm_dir])
+            test_deferred.callback(True)
+
+        mock_unit_deploy = self.mocker.patch(principal_lifecycle)
+        mock_unit_deploy._do_unit_deploy("logging/0", 0, ANY)
+        self.mocker.call(test_deploy)
+
+        # Subordinate setup
+        test_sub_deferred = Deferred()
+        sub_results = []
+
+        def test_undeploy(unit_name, machine_id, charm_dir):
+            sub_results.append([unit_name, machine_id, charm_dir])
+            test_sub_deferred.callback(True)
+
+        # Actual Test
+        self.mocker.replay()
+        yield principal_lifecycle.start()
+        yield test_deferred
+
+        # The principal is running and did a deploy (mocked out)
+        # at this point we should be able to collect the relation
+        # and the unit and inject them into the subordinate (which
+        # would have them in its own process normally)
+        log = state["subordinate"][1]
+        log_units = yield log.get_all_unit_states()
+        lu1 = log_units[0]
+
+        # A second instance of mocker around the subordinate
+        # avoids issues with patch/replace confusing the single
+        # instance
+        mocker2 = Mocker()
+        subordinate_lifecycle = UnitLifecycle(
+            self.client, lu1, log_srv,
+            self.unit_directory, self.state_directory, self.executor)
+
+        mock_subunit_deploy = mocker2.patch(subordinate_lifecycle)
+        mock_subunit_deploy._do_unit_kill(
+              "logging/0", "", ANY)
+        mocker2.call(test_undeploy)
+
+        with mocker2:
+            yield subordinate_lifecycle.start()
+
+            # break relation to principal
+            yield self.relation_manager.remove_relation_state(
+                state["relation_state"])
+
+            # verify that we attempt to we see the departed event
+            # and the attempt of the subordinate to self terminate
+            yield test_sub_deferred
+            # again, mocker verified the call on do_unit_kill
+            # this verifies that the unit attempted to self destruct
+
+            # verify the subordinate unit state was removed
+            failure = self.service_manager.get_unit_state("logging/0")
+            yield self.assertFailure(failure, ServiceUnitStateNotFound)

=== modified file 'juju/unit/workflow.py'
--- juju/unit/workflow.py	2012-03-29 07:52:41 +0000
+++ juju/unit/workflow.py	2012-06-04 23:05:24 +0000
@@ -1,7 +1,7 @@
-import yaml
 import csv
+import logging
 import os
-import logging
+import yaml
 
 from zookeeper import NoNodeException
 from twisted.internet.defer import inlineCallbacks, returnValue
@@ -226,6 +226,9 @@
         state_serialized = yaml.safe_dump(state_dict)
 
         def update_state(content, stat):
+            if not stat:
+                # State has gone away
+                return
             unit_data = yaml.load(content)
             if not unit_data:
                 unit_data = {}