Merge lp:~jjo/juju-deployer/diff-usability-fixes into lp:juju-deployer

[JuanJo Ciarlante]

* action/diff.py:
  - create an output-less do_diff() method, to be used to implement
    unit-tests
  - add more detail to diff output re: values coming from cfg or env.
    Rationale: after seeing a config diff, one usually then goes for a
    juju get <svc_name> to find the env config value, which is a non-sense
    workflow given that we do know its current value.
    Ditto constraints.
* tests/test_data/:
  - fix blog.yaml: proper indentation for memcache,haproxy (so that they're
    actually parsed as services).
  - add blog-haproxy-services.yaml as a proper example
    (instead of nonexistent blog-include.yaml).
* tests/test_deployment.py: call resolve() instead of resolve_config(),
  for wider coverage.

[JuanJo Ciarlante (jjo)]

To ease/minimize this review, lp:~jjo/juju-deployer/diff-usability-fixes_and_diff-tests is stacked over this.

[Kapil Thangavelu (hazmat)]

thanks

On Fri, Nov 29, 2013 at 5:27 PM, JuanJo Ciarlante <
<email address hidden>> wrote:

> To ease/minimize this review,
> lp:~jjo/juju-deployer/diff-usability-fixes_and_diff-tests is stacked over
> this.
> --
>
> https://code.launchpad.net/~jjo/juju-deployer/diff-usability-fixes/+merge/197270
> Your team juju-deployers is requested to review the proposed merge of
> lp:~jjo/juju-deployer/diff-usability-fixes into lp:juju-deployer.
>

=== modified file 'deployer/action/diff.py'
--- deployer/action/diff.py	2013-11-21 03:19:56 +0000
+++ deployer/action/diff.py	2013-11-29 22:24:08 +0000
@@ -122,10 +122,10 @@
 
     def _diff_service(self, e_s, d_s, charm):
         mod = {}
-        if 'constraints' in d_s:
-            d_sc = parse_constraints(d_s['constraints'])
-            if d_sc != e_s['constraints']:
-                mod['constraints'] = e_s['constraints']
+        d_sc = parse_constraints(d_s.get('constraints',''))
+        if d_sc != e_s['constraints']:
+            mod['env-constraints'] = e_s['constraints']
+            mod['cfg-constraints'] = d_sc
         for k, v in d_s.get('options', {}).items():
             # Deploy options not known to the env may originate
             # from charm version delta or be an invalid config.
@@ -133,17 +133,22 @@
                 continue
             e_v = e_s['options'].get(k, {}).get('value')
             if e_v != v:
-                mod['config'] = {k: e_v}
+                mod.setdefault('env-config', {}).update({k: e_v})
+                mod.setdefault('cfg-config', {}).update({k: v})
         if not charm or not charm.is_subordinate():
             if e_s['unit_count'] != d_s.get('num_units', 1):
                 mod['num_units'] = e_s['unit_count'] - d_s.get('num_units', 1)
         return mod
 
-    def run(self):
+    def do_diff(self):
         self.start_time = time.time()
+        self.deployment.resolve()
         self.env.connect()
         self.env_status = self.env.status()
         self.load_env()
-        delta = self.get_delta()
-        if delta:
-            print yaml_dump(delta)
+        return self.get_delta()
+
+    def run(self):
+        diff = self.do_diff()
+        if diff:
+            print yaml_dump(diff)

=== modified file 'deployer/env/go.py'
--- deployer/env/go.py	2013-11-15 15:13:01 +0000
+++ deployer/env/go.py	2013-11-29 22:24:08 +0000
@@ -62,7 +62,7 @@
             return self.client.get_constraints(svc_name)
         except EnvError, e:
             if 'constraints do not apply to subordinate services' in str(e):
-                return None
+                return {}
             else:
                 raise e
 

=== added file 'deployer/tests/test_data/blog-haproxy-services.yaml'
--- deployer/tests/test_data/blog-haproxy-services.yaml	1970-01-01 00:00:00 +0000
+++ deployer/tests/test_data/blog-haproxy-services.yaml	2013-11-29 22:24:08 +0000
@@ -0,0 +1,14 @@
+- service_name: blog
+  service_host: 0.0.0.0
+  service_port: 80
+  service_options:
+    - mode http
+    - option httplog
+    - balance uri
+    - hash-type consistent
+    - timeout client 60000
+    - timeout server 60000
+  server_options:
+    - check inter 2000
+    - rise 2
+    - fall 5

=== modified file 'deployer/tests/test_data/blog.yaml'
--- deployer/tests/test_data/blog.yaml	2013-07-22 15:29:31 +0000
+++ deployer/tests/test_data/blog.yaml	2013-11-29 22:24:08 +0000
@@ -28,14 +28,14 @@
         tuning: optimized
         engine: apache
         wp-content: include-base64://blog.snippet
-      memcached:
-        branch: lp:charms/precise/memcached
-        options:
-          size: 100
-      haproxy:
-        charm_url: cs:precise/haproxy
-        options:
-          services: include-file://blog-include.yaml
+    cache:
+      branch: lp:charms/precise/memcached
+      options:
+        size: 100
+    haproxy:
+      charm_url: cs:precise/haproxy
+      options:
+        services: include-file://blog-haproxy-services.yaml
   relations:
         - [blog, db]
         - - blog
@@ -56,4 +56,4 @@
         db:
             constraints: instance-type=m1.large
             options:
-                tuning-level: safest
\ No newline at end of file
+                tuning-level: safest

=== modified file 'deployer/tests/test_deployment.py'
--- deployer/tests/test_deployment.py	2013-10-31 19:19:36 +0000
+++ deployer/tests/test_deployment.py	2013-11-29 22:24:08 +0000
@@ -41,7 +41,7 @@
 
         # Load up overrides and resolves
         d.load_overrides(["key=abc"])
-        d.resolve_config()
+        d.resolve()
 
         # Verify include-base64
         self.assertEqual(d.get_service('newrelic').config, {'key': 'abc'})