Merge lp:~jimbaker/pyjuju/new-hook-semantics-2-joined-hook into lp:pyjuju

[Jim Baker]

Adds a <name>-relation-joined hook, while keeping the existing
<name>-relation-changed hook. A similar small change to what was done
for the departed event was made for the joined event in this branch:

          elif change.change_type == "joined":
              hook_names = [
                  "%s-relation-joined" % self._relation_name,
                  "%s-relation-changed" % self._relation_name]

[Gustavo Niemeyer (niemeyer)]

This looks good. +1, but please take care of the pre-requisite branch
before this is merged.

[1]

+ ("#!/bin/bash\n" "echo joined >> %s\n" % (file_path)))

Please remove the parenthesis around file_path here too.

[2]

+ "app-relation-joined", "app-relation-changed",

Please make sure the lists are all evenly organized.

[3]

You'll have to do some changes here due to the pre-requisite branch.

review: Approve

[Jim Baker (jimbaker)]

On Wed, Apr 6, 2011 at 10:57 AM, Gustavo Niemeyer <email address hidden>wrote:

> Review: Approve
> This looks good. +1, but please take care of the pre-requisite branch
> before this is merged.
>
> [1]
>
> + ("#!/bin/bash\n" "echo joined >> %s\n" % (file_path)))
>
> Please remove the parenthesis around file_path here too.
>
>
Removed in all cases

> [2]
>
> + "app-relation-joined", "app-relation-changed",
>
> Please make sure the lists are all evenly organized.
>
>
Reorganized and commented as with the previous branch

>
> [3]
>
> You'll have to do some changes here due to the pre-requisite branch.
>

Those changes were made

=== modified file 'ensemble/unit/lifecycle.py'
--- ensemble/unit/lifecycle.py	2011-03-29 17:13:27 +0000
+++ ensemble/unit/lifecycle.py	2011-03-29 17:13:27 +0000
@@ -283,6 +283,10 @@
                 hook_names = [
                     "%s-relation-changed" % self._relation_name,
                     "%s-relation-departed" % self._relation_name]
+            elif change.change_type == "joined":
+                hook_names = [
+                    "%s-relation-joined" % self._relation_name,
+                    "%s-relation-changed" % self._relation_name]
             else:
                 hook_names = ["%s-relation-changed" % self._relation_name]
         else:

=== modified file 'ensemble/unit/tests/test_lifecycle.py'
--- ensemble/unit/tests/test_lifecycle.py	2011-03-29 17:13:27 +0000
+++ ensemble/unit/tests/test_lifecycle.py	2011-03-29 17:13:27 +0000
@@ -224,8 +224,11 @@
         self.write_hook("start", ("#!/bin/bash\n" "echo hello"))
         self.write_hook("stop", ("#!/bin/bash\n" "echo goodbye"))
         self.write_hook(
+            "%s-relation-joined" % relation_name,
+            ("#!/bin/bash\n" "echo joined >> %s\n" % (file_path)))
+        self.write_hook(
             "%s-relation-changed" % relation_name,
-            ("#!/bin/bash\n" "echo $ENSEMBLE_CHANGE >> %s\n" % (file_path)))
+            ("#!/bin/bash\n" "echo changed >> %s\n" % (file_path)))
         self.write_hook(
             "%s-relation-departed" % relation_name,
             ("#!/bin/bash\n" "echo departed >> %s\n" % (file_path)))
@@ -251,13 +254,14 @@
 
         self.assertTrue(os.path.exists(file_path))
         self.assertEqual([x.strip() for x in open(file_path).readlines()],
-                         ["joined"])
+                         ["joined", "changed"])
 
         # Queue up our wait condition, of 3 hooks firing
         hooks_complete = self.wait_on_hook(
-            sequence=["app-relation-changed",
-                      "app-relation-changed",
-                      "app-relation-changed", "app-relation-departed"])
+            sequence=[
+                "app-relation-joined", "app-relation-changed",
+                "app-relation-changed",
+                "app-relation-changed", "app-relation-departed"])
 
         # add another.
         wordpress2_states = yield self.add_opposite_service_unit(
@@ -280,7 +284,7 @@
         yield hooks_complete
         self.assertEqual(
             set([x.strip() for x in open(file_path).readlines()]),
-            set(["joined", "joined", "modified", "departed"]))
+            set(["joined", "changed", "joined", "changed", "changed", "departed"]))
 
     @inlineCallbacks
     def test_removed_relation_depart(self):
@@ -299,7 +303,7 @@
 
         self.assertTrue(os.path.exists(file_path))
         self.assertEqual([x.strip() for x in open(file_path).readlines()],
-                         ["joined"])
+                         ["joined", "changed"])
 
         self.assertTrue(self.lifecycle.get_relation_workflow(
             self.states["relation"].internal_id))
@@ -320,7 +324,7 @@
 
         # Verify no notice was recieved for the modify before we where stopped.
         self.assertEqual([x.strip() for x in open(file_path).readlines()],
-                         ["joined"])
+                         ["joined", "changed"])
 
         # Verify the unit relation lifecycle has been disposed of.
         self.assertRaises(KeyError,
@@ -372,7 +376,7 @@
         # Verify no hooks are executed.
         yield self.sleep(0.1)
         self.assertEqual([x.strip() for x in open(file_path)],
-                         ["joined", "joined"])
+                         ["joined", "changed", "joined", "changed"])
 
         # XXX - With scheduler local state recovery, we should get the modify.
 
@@ -381,11 +385,12 @@
             sequence=[
                 "start", 
                 "app-relation-changed", "app-relation-departed",
-                "app-relation-changed"])
+                "app-relation-joined", "app-relation-changed"])
         yield self.lifecycle.start()
         yield hooks_executed
         self.assertEqual([x.strip() for x in open(file_path)],
-                         ["joined", "joined", "departed", "departed", "joined"])
+                         ["joined", "changed", "joined", "changed",
+                          "changed", "departed", "joined", "changed"])
 
     @inlineCallbacks
     def test_lock_start_stop_watch(self):
@@ -471,6 +476,7 @@
 
     hook_template = (
             "#!/bin/bash\n"
+            "echo %(change_type)s >> %(file_path)s\n"
             "echo ENSEMBLE_RELATION=$ENSEMBLE_RELATION >> %(file_path)s\n"
             "echo ENSEMBLE_CHANGE=$ENSEMBLE_CHANGE >> %(file_path)s\n"
             "echo ENSEMBLE_REMOTE_UNIT=$ENSEMBLE_REMOTE_UNIT >> %(file_path)s")
@@ -513,16 +519,26 @@
         yield self.add_opposite_service_unit(self.states)
 
         file_path = self.makeFile()
+        self.write_hook("%s-relation-joined" % self.relation_name,
+                        self.hook_template % dict(change_type="joined",
+                                                  file_path=file_path))
         self.write_hook("%s-relation-changed" % self.relation_name,
-                        self.hook_template % {"file_path": file_path})
+                        self.hook_template % dict(change_type="changed",
+                                                  file_path=file_path))
 
         yield self.lifecycle.start()
-        yield self.wait_on_hook("app-relation-changed")
+        yield self.wait_on_hook(
+            sequence=["app-relation-joined", "app-relation-changed"])
         self.assertTrue(os.path.exists(file_path))
 
         contents = open(file_path).read()
         self.assertEqual(contents,
-                         ("ENSEMBLE_RELATION=app\n"
+                         ("joined\n"
+                          "ENSEMBLE_RELATION=app\n"
+                          "ENSEMBLE_CHANGE=joined\n"
+                          "ENSEMBLE_REMOTE_UNIT=wordpress/0\n"
+                          "changed\n"
+                          "ENSEMBLE_RELATION=app\n"
                           "ENSEMBLE_CHANGE=joined\n"
                           "ENSEMBLE_REMOTE_UNIT=wordpress/0\n"))
 
@@ -531,8 +547,12 @@
         """If the unit relation lifecycle is stopped, hooks will no longer
         be executed."""
         file_path = self.makeFile()
+        self.write_hook("%s-relation-joined" % self.relation_name,
+                        self.hook_template % dict(change_type="joined",
+                                                  file_path=file_path))
         self.write_hook("%s-relation-changed" % self.relation_name,
-                        self.hook_template % {"file_path": file_path})
+                        self.hook_template % dict(change_type="changed",
+                                                  file_path=file_path))
 
         # starting is async
         yield self.lifecycle.start()
@@ -560,6 +580,7 @@
     @inlineCallbacks
     def test_hook_error_handler(self):
         # use an error handler that completes async.
+        self.write_hook("app-relation-joined", "#!/bin/bash\nexit 0\n")
         self.write_hook("app-relation-changed", "#!/bin/bash\nexit 1\n")
 
         results = []
@@ -589,6 +610,7 @@
         output = (
             "start relation lifecycle",
             "app: executing change hook",
+            "app: executing change hook",
             "app: change hook error: %s '%s/%s': exit code 1." % (
                 "Error processing",
                 self.unit_directory, hook_relative_path),
@@ -601,14 +623,18 @@
         """If a relation is departed, the depart hook is executed.
         """
         file_path = self.makeFile()
+        self.write_hook("%s-relation-joined" % self.relation_name,
+                        "#!/bin/bash\n echo joined")
         self.write_hook("%s-relation-changed" % self.relation_name,
                         "#!/bin/bash\n echo hello")
         self.write_hook("%s-relation-broken" % self.relation_name,
-                        self.hook_template % {"file_path": file_path})
+                        self.hook_template % dict(change_type="broken",
+                                                  file_path=file_path))
 
         yield self.lifecycle.start()
         wordpress_states = yield self.add_opposite_service_unit(self.states)
-        yield self.wait_on_hook("app-relation-changed")
+        yield self.wait_on_hook(
+            sequence=["app-relation-joined", "app-relation-changed"])
         yield self.lifecycle.stop()
         yield self.relation_manager.remove_relation_state(
             wordpress_states["relation"])

=== modified file 'ensemble/unit/tests/test_workflow.py'
--- ensemble/unit/tests/test_workflow.py	2010-12-23 18:40:40 +0000
+++ ensemble/unit/tests/test_workflow.py	2011-03-29 17:13:27 +0000
@@ -210,6 +210,8 @@
     def test_up_down_cycle(self):
         """The workflow can be transition from up to down, and back.
         """
+        self.write_hook("%s-relation-joined" % self.relation_name,
+                        "#!/bin/bash\nexit 0\n")
         self.write_hook("%s-relation-changed" % self.relation_name,
                         "#!/bin/bash\nexit 0\n")
 
@@ -253,6 +255,8 @@
         in the workflow transitioning to the down state.
         """
         self.capture_logging("unit.relation.lifecycle", logging.DEBUG)
+        self.write_hook("%s-relation-joined" % self.relation_name,
+                        "#!/bin/bash\nexit 0\n")
         self.write_hook("%s-relation-changed" % self.relation_name,
                         "#!/bin/bash\nexit 1\n")
 
@@ -287,6 +291,8 @@
         broken hook is executed, and the unit stops responding to relation
         changes.
         """
+        self.write_hook("%s-relation-joined" % self.relation_name,
+                        "#!/bin/bash\necho hello\n")
         self.write_hook("%s-relation-changed" % self.relation_name,
                         "#!/bin/bash\necho hello\n")
         self.write_hook("%s-relation-broken" % self.relation_name,
@@ -326,6 +332,8 @@
         """A depart hook error, still results in a transition to the
         departed state with a state variable noting the error."""
 
+        self.write_hook("%s-relation-joined" % self.relation_name,
+                        "#!/bin/bash\necho hello\n")
         self.write_hook("%s-relation-changed" % self.relation_name,
                         "#!/bin/bash\necho hello\n")
         self.write_hook("%s-relation-broken" % self.relation_name,