Merge lp:~timkuhlman/juju-deployer/storage-support into lp:juju-deployer

[Tim Kuhlman]

This adds support for Juju storage on initial application setup.

The addition to the integration test of the postgresql charm with storage support requires that this fix to the postgresql charm, https://code.launchpad.net/~timkuhlman/postgresql-charm/+git/storage-fix/+merge/313845

[Tim Van Steenburgh (tvansteenburgh)]

LGTM. Will proceed with merge since integration tests won't block the build.

review: Approve

=== modified file 'deployer/action/importer.py'
--- deployer/action/importer.py	2016-11-03 13:00:27 +0000
+++ deployer/action/importer.py	2016-12-23 18:56:57 +0000
@@ -201,6 +201,7 @@
                 charm.repo_path or self.deployment.repo_path,
                 svc.config,
                 svc.constraints,
+                svc.storage,
                 num_units,
                 placement.get(0),
                 charm.series or self.deployment.series,

=== modified file 'deployer/env/base.py'
--- deployer/env/base.py	2016-07-12 23:43:08 +0000
+++ deployer/env/base.py	2016-12-23 18:56:57 +0000
@@ -4,6 +4,7 @@
 from ..utils import (
     AlternateKeyDict,
     ErrorExit,
+    DeploymentError,
     yaml_load,
     yaml_dump,
     temp_file,
@@ -68,7 +69,7 @@
         self.log.info(" Bootstrap complete")
 
     def deploy(self, name, charm_url, repo=None, config=None,
-               constraints=None, num_units=1, force_machine=None,
+               constraints=None, storage=None, num_units=1, force_machine=None,
                series=None):
         params = self._named_env(["juju", "deploy"])
         with temp_file() as fh:
@@ -84,6 +85,12 @@
                         '{}={}'.format(k, v) for k, v in constraints.items()
                     ])
                 params.extend(['--constraints', constraints])
+            if storage:
+                if not isinstance(storage, dict):
+                    raise DeploymentError("storage must be specified as a dictionary")
+                params.append("--storage")
+                for key, value in storage.items():
+                    params.append("{}={}".format(key, value))
             if num_units not in (1, None):
                 params.extend(["--num-units", str(num_units)])
             if charm_url.startswith('local'):

=== modified file 'deployer/service.py'
--- deployer/service.py	2016-11-08 19:05:04 +0000
+++ deployer/service.py	2016-12-23 18:56:57 +0000
@@ -55,6 +55,10 @@
         return int(self.svc_data.get('num_units', 1))
 
     @property
+    def storage(self):
+        return self.svc_data.get('storage', None)
+
+    @property
     def unit_placement(self):
         # Separate checks to support machine 0 placement.
         value = self.svc_data.get('to')

=== modified file 'deployer/tests/test_charm.py'
--- deployer/tests/test_charm.py	2016-11-04 00:22:12 +0000
+++ deployer/tests/test_charm.py	2016-12-23 18:56:57 +0000
@@ -137,7 +137,7 @@
             "scratch", self.repo_path, "precise", params)
 
         charm.fetch()
-        self.assertEqual(charm.metadata['name'],  'couchdb')
+        self.assertEqual(charm.metadata['name'], 'couchdb')
         self.assertEqual(charm.vcs.get_cur_rev(), '3')
 
         charm.rev = '2'
@@ -286,7 +286,7 @@
         charm = Charm.from_service(
             "scratch", self.repo_path, "precise", params)
         charm.fetch()
-        self.assertEqual(charm.metadata['name'],  'couchdb')
+        self.assertEqual(charm.metadata['name'], 'couchdb')
         HEAD = charm.vcs.get_cur_rev()
 
         self.assertFalse(charm.is_modified())

=== added file 'deployer/tests/test_data/wiki-storage.yaml'
--- deployer/tests/test_data/wiki-storage.yaml	1970-01-01 00:00:00 +0000
+++ deployer/tests/test_data/wiki-storage.yaml	2016-12-23 18:56:57 +0000
@@ -0,0 +1,13 @@
+wiki:
+  series: precise
+  services:
+    wiki:
+      charm: cs:precise/mediawiki
+      options:
+        name: $hi_world _are_you_there? {guess_who}
+      storage:
+        web-data: "cinder,10G,1"
+    db:
+      charm: cs:precise/mysql
+  relations:
+    - [wiki, db]
\ No newline at end of file

=== modified file 'deployer/tests/test_goenv.py'
--- deployer/tests/test_goenv.py	2016-07-12 02:29:52 +0000
+++ deployer/tests/test_goenv.py	2016-12-23 18:56:57 +0000
@@ -59,6 +59,7 @@
         self.env.deploy("test-blog", "cs:precise/wordpress")
         self.env.deploy("test-db", "cs:precise/mysql")
         self.env.add_relation("test-db", "test-blog")
+        self.env.deploy("test-db2", "cs:postgresql", storage={"pgdata": "rootfs,10M"}, series="xenial")
         self.env.add_units('test-blog', 1)
 
         # Sleep because juju core watches are eventually consistent (5s window)
@@ -67,7 +68,7 @@
         self.env.wait_for_units(timeout=800)
         status = self.env.status()
 
-        services = ["test-blog", "test-db"]
+        services = ["test-blog", "test-db", "test-db2"]
         self.assertEqual(
             sorted(status['services'].keys()),
             services)

=== modified file 'deployer/tests/test_guiserver.py'
--- deployer/tests/test_guiserver.py	2016-10-27 18:41:34 +0000
+++ deployer/tests/test_guiserver.py	2016-12-23 18:56:57 +0000
@@ -283,14 +283,14 @@
             mock.call.status(),
             mock.call.deploy(
                 'mysql', 'cs:precise/mysql-28', '', None,
-                {'arch': 'i386', 'cpu-cores': 4, 'mem': '4G'}, 2, None,
+                {'arch': 'i386', 'cpu-cores': 4, 'mem': '4G'}, None, 2, None,
                 'precise'),
             mock.call.set_annotation(
                 'mysql', {'gui-y': '164.547', 'gui-x': '494.347'}),
             mock.call.deploy(
                 'wordpress', 'cs:precise/wordpress-20', '',
                 {'debug': 'no', 'engine': 'nginx', 'tuning': 'single'},
-                None, 1, None, 'precise'),
+                None, None, 1, None, 'precise'),
             mock.call.set_annotation(
                 'wordpress', {'gui-y': '414.547', 'gui-x': '425.347'}),
             mock.call.add_units('mysql', 2),

=== modified file 'deployer/tests/test_importer.py'
--- deployer/tests/test_importer.py	2016-10-27 18:41:34 +0000
+++ deployer/tests/test_importer.py	2016-12-23 18:56:57 +0000
@@ -63,7 +63,25 @@
             env.method_calls[3], mock.call.deploy(
                 'wiki', 'cs:precise/mediawiki',
                 os.environ.get("JUJU_REPOSITORY", ""),
-                config, None, 1, None, 'precise'))
+                config, None, None, 1, None, 'precise'))
+        env.add_relation.assert_called_once_with('wiki', 'db')
+
+    @skip_if_offline
+    @mock.patch('deployer.action.importer.time')
+    def test_importer_with_storage(self, mock_time):
+        mock_time.time.return_value = 0
+        stack = ConfigStack([os.path.join(self.test_data_dir, 'wiki-storage.yaml')])
+        deploy = stack.get('wiki')
+        env = mock.MagicMock()
+        importer = Importer(env, deploy, self.options)
+        importer.run()
+
+        config = {'name': '$hi_world _are_you_there? {guess_who}'}
+        self.assertEqual(
+            env.method_calls[3], mock.call.deploy(
+                'wiki', 'cs:precise/mediawiki',
+                os.environ.get("JUJU_REPOSITORY", ""),
+                config, None, {"web-data": "cinder,10G,1"}, 1, None, 'precise'))
         env.add_relation.assert_called_once_with('wiki', 'db')
 
     @skip_if_offline

=== modified file 'deployer/tests/test_service.py'
--- deployer/tests/test_service.py	2016-05-07 23:02:56 +0000
+++ deployer/tests/test_service.py	2016-12-23 18:56:57 +0000
@@ -14,15 +14,18 @@
         self.assertEqual(s.num_units, 1)
         self.assertEqual(s.constraints, None)
         self.assertEqual(s.config, None)
+        self.assertEqual(s.storage, None)
 
         data = {
             'branch': 'lp:precise/mysql',
             'constraints': "instance-type=m1.small",
             'options': {"services": "include-file://stack-include.yaml"},
-            'num_units': 10}
+            'num_units': 10,
+            'storage': {"data": "ebs,10G,1"}}
         s = Service('db', data)
         self.assertEquals(s.num_units, 10)
         self.assertEquals(s.constraints, "instance-type=m1.small")
         self.assertEquals(
             s.config,
             {"services": "include-file://stack-include.yaml"})
+        self.assertEquals(s.storage, {"data": "ebs,10G,1"})

=== modified file 'doc/config.rst'
--- doc/config.rst	2015-01-22 15:22:18 +0000
+++ doc/config.rst	2016-12-23 18:56:57 +0000
@@ -36,6 +36,8 @@
                 tuning: optimized
                 engine: apache
                 icon: include-base64://file.ico
+            storage:
+            	data-block: "ebs,1024G,3"
  
 
 The service's charm name will be infered from the vcs branch name if
@@ -52,6 +54,15 @@
 include-base64://path-to-file. Relative paths are resolved
 relative to the config file.
 
+Service storage is specified as a dictionary with keys as the label
+and the value being the pool,size,count string that should be passed
+to Juju. This matches the Juju cli syntax,
+`juju deploy <charm> --storage <label>=<pool>,<size>,<count>`. Also
+like the cli, size and count are optional parameters. NOTE: storage
+status is not displayed in the diff command as isn't in the the Juju
+status data. For this same reason storage is only specified for new
+services and is not added to already existing services.
+
 Relations
 =========