Merge lp:~jsing/juju-deployer/resources-support into lp:juju-deployer

Proposed by Joel Sing
Status: Merged
Merged at revision: 209
Proposed branch: lp:~jsing/juju-deployer/resources-support
Merge into: lp:juju-deployer
Diff against target: 197 lines (+71/-11)
8 files modified
deployer/action/importer.py (+1/-0)
deployer/env/base.py (+9/-2)
deployer/service.py (+4/-0)
deployer/tests/test_data/wiki-resources.yaml (+14/-0)
deployer/tests/test_guiserver.py (+3/-3)
deployer/tests/test_importer.py (+26/-4)
deployer/tests/test_service.py (+2/-0)
doc/config.rst (+12/-2)
To merge this branch: bzr merge lp:~jsing/juju-deployer/resources-support
Reviewer Review Type Date Requested Status
Tim Van Steenburgh (community) Approve
Review via email: mp+316538@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Tim Van Steenburgh (tvansteenburgh) wrote :

This LGTM but I hit one error in the tests:

======================================================================
ERROR: test_importer_with_resources (deployer.tests.test_importer.ImporterTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tvansteenburgh/src/juju-deployer/.tox/py35/lib/python3.5/site-packages/mock/mock.py", line 1305, in patched
    return func(*args, **keywargs)
  File "/home/tvansteenburgh/src/juju-deployer/deployer/tests/test_importer.py", line 74, in test_importer_with_resources
    os.path.join(self.test_data_dir, 'wiki-resources.yaml')])
  File "/home/tvansteenburgh/src/juju-deployer/deployer/config.py", line 34, in __init__
    self.load()
  File "/home/tvansteenburgh/src/juju-deployer/deployer/config.py", line 92, in load
    for fp in self._resolve_included():
  File "/home/tvansteenburgh/src/juju-deployer/deployer/config.py", line 141, in _resolve_included
    [files.extend(self._includes(cf)) for cf in self.config_files]
  File "/home/tvansteenburgh/src/juju-deployer/deployer/config.py", line 141, in <listcomp>
    [files.extend(self._includes(cf)) for cf in self.config_files]
  File "/home/tvansteenburgh/src/juju-deployer/deployer/config.py", line 123, in _includes
    d = self._yaml_load(config_file)
  File "/home/tvansteenburgh/src/juju-deployer/deployer/config.py", line 51, in _yaml_load
    with open(config_file) as fh:
FileNotFoundError: [Errno 2] No such file or directory: '/home/tvansteenburgh/src/juju-deployer/deployer/tests/test_data/wiki-resources.yaml'

Is there a wiki-resources.yaml file that didn't get included in the commit?

review: Needs Fixing
209. By Joel Sing

[jsing] Add support for deploying resources with juju.

Revision history for this message
Joel Sing (jsing) wrote :

Yup, appears to have been a bzr add fail... fixed.

PTAL

Revision history for this message
Tim Van Steenburgh (tvansteenburgh) wrote :

Thanks, LGTM.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'deployer/action/importer.py'
2--- deployer/action/importer.py 2016-12-22 22:25:54 +0000
3+++ deployer/action/importer.py 2017-03-01 03:31:40 +0000
4@@ -201,6 +201,7 @@
5 charm.repo_path or self.deployment.repo_path,
6 svc.config,
7 svc.constraints,
8+ svc.resources,
9 svc.storage,
10 num_units,
11 placement.get(0),
12
13=== modified file 'deployer/env/base.py'
14--- deployer/env/base.py 2016-12-24 14:25:49 +0000
15+++ deployer/env/base.py 2017-03-01 03:31:40 +0000
16@@ -68,8 +68,8 @@
17 self.get_cli_status()
18 self.log.info(" Bootstrap complete")
19
20- def deploy(self, name, charm_url, repo=None, config=None,
21- constraints=None, storage=None, num_units=1, force_machine=None,
22+ def deploy(self, name, charm_url, repo=None, config=None, constraints=None,
23+ resources=None, storage=None, num_units=1, force_machine=None,
24 series=None):
25 params = self._named_env(["juju", "deploy"])
26 with temp_file() as fh:
27@@ -85,6 +85,13 @@
28 '{}={}'.format(k, v) for k, v in constraints.items()
29 ])
30 params.extend(['--constraints', constraints])
31+ if resources:
32+ if not isinstance(resources, dict):
33+ raise DeploymentError(
34+ "resources must be specified as a dictionary")
35+ for key, value in resources.items():
36+ params.append("--resource")
37+ params.append("{}={}".format(key, value))
38 if storage:
39 if not isinstance(storage, dict):
40 raise DeploymentError(
41
42=== modified file 'deployer/service.py'
43--- deployer/service.py 2016-12-22 22:25:54 +0000
44+++ deployer/service.py 2017-03-01 03:31:40 +0000
45@@ -55,6 +55,10 @@
46 return int(self.svc_data.get('num_units', 1))
47
48 @property
49+ def resources(self):
50+ return self.svc_data.get('resources', None)
51+
52+ @property
53 def storage(self):
54 return self.svc_data.get('storage', None)
55
56
57=== added file 'deployer/tests/test_data/wiki-resources.yaml'
58--- deployer/tests/test_data/wiki-resources.yaml 1970-01-01 00:00:00 +0000
59+++ deployer/tests/test_data/wiki-resources.yaml 2017-03-01 03:31:40 +0000
60@@ -0,0 +1,14 @@
61+wiki:
62+ series: precise
63+ services:
64+ wiki:
65+ charm: cs:precise/mediawiki
66+ options:
67+ name: $hi_world _are_you_there? {guess_who}
68+ resources:
69+ config: ./docs/cfg.xml
70+ tarball: /some/file.tgz
71+ db:
72+ charm: cs:precise/mysql
73+ relations:
74+ - [wiki, db]
75
76=== modified file 'deployer/tests/test_guiserver.py'
77--- deployer/tests/test_guiserver.py 2016-12-22 22:52:22 +0000
78+++ deployer/tests/test_guiserver.py 2017-03-01 03:31:40 +0000
79@@ -283,14 +283,14 @@
80 mock.call.status(),
81 mock.call.deploy(
82 'mysql', 'cs:precise/mysql-28', '', None,
83- {'arch': 'i386', 'cpu-cores': 4, 'mem': '4G'}, None, 2, None,
84- 'precise'),
85+ {'arch': 'i386', 'cpu-cores': 4, 'mem': '4G'},
86+ None, None, 2, None, 'precise'),
87 mock.call.set_annotation(
88 'mysql', {'gui-y': '164.547', 'gui-x': '494.347'}),
89 mock.call.deploy(
90 'wordpress', 'cs:precise/wordpress-20', '',
91 {'debug': 'no', 'engine': 'nginx', 'tuning': 'single'},
92- None, None, 1, None, 'precise'),
93+ None, None, None, 1, None, 'precise'),
94 mock.call.set_annotation(
95 'wordpress', {'gui-y': '414.547', 'gui-x': '425.347'}),
96 mock.call.add_units('mysql', 2),
97
98=== modified file 'deployer/tests/test_importer.py'
99--- deployer/tests/test_importer.py 2016-12-24 14:25:49 +0000
100+++ deployer/tests/test_importer.py 2017-03-01 03:31:40 +0000
101@@ -63,9 +63,31 @@
102 env.method_calls[3], mock.call.deploy(
103 'wiki', 'cs:precise/mediawiki',
104 os.environ.get("JUJU_REPOSITORY", ""),
105- config, None, None, 1, None, 'precise'))
106- env.add_relation.assert_called_once_with('wiki', 'db')
107-
108+ config, None, None, None, 1, None, 'precise'))
109+ env.add_relation.assert_called_once_with('wiki', 'db')
110+
111+ @skip_if_offline
112+ @mock.patch('deployer.action.importer.time')
113+ def test_importer_with_resources(self, mock_time):
114+ mock_time.time.return_value = 0
115+ stack = ConfigStack([
116+ os.path.join(self.test_data_dir, 'wiki-resources.yaml')])
117+ deploy = stack.get('wiki')
118+ env = mock.MagicMock()
119+ importer = Importer(env, deploy, self.options)
120+ importer.run()
121+
122+ config = {'name': '$hi_world _are_you_there? {guess_who}'}
123+ self.assertEqual(
124+ env.method_calls[3], mock.call.deploy(
125+ 'wiki', 'cs:precise/mediawiki',
126+ os.environ.get("JUJU_REPOSITORY", ""),
127+ config, None,
128+ {"config": "./docs/cfg.xml", "tarball": "/some/file.tgz"},
129+ None, 1, None, 'precise'))
130+ env.add_relation.assert_called_once_with('wiki', 'db')
131+
132+ @skip_if_offline
133 @skip_if_offline
134 @mock.patch('deployer.action.importer.time')
135 def test_importer_with_storage(self, mock_time):
136@@ -82,7 +104,7 @@
137 env.method_calls[3], mock.call.deploy(
138 'wiki', 'cs:precise/mediawiki',
139 os.environ.get("JUJU_REPOSITORY", ""),
140- config, None, {"web-data": "cinder,10G,1"},
141+ config, None, None, {"web-data": "cinder,10G,1"},
142 1, None, 'precise'))
143 env.add_relation.assert_called_once_with('wiki', 'db')
144
145
146=== modified file 'deployer/tests/test_service.py'
147--- deployer/tests/test_service.py 2016-12-22 22:25:54 +0000
148+++ deployer/tests/test_service.py 2017-03-01 03:31:40 +0000
149@@ -21,6 +21,7 @@
150 'constraints': "instance-type=m1.small",
151 'options': {"services": "include-file://stack-include.yaml"},
152 'num_units': 10,
153+ 'resources': {"config": "./docs/cfg.xml"},
154 'storage': {"data": "ebs,10G,1"}}
155 s = Service('db', data)
156 self.assertEquals(s.num_units, 10)
157@@ -28,4 +29,5 @@
158 self.assertEquals(
159 s.config,
160 {"services": "include-file://stack-include.yaml"})
161+ self.assertEquals(s.resources, {"config": "./docs/cfg.xml"})
162 self.assertEquals(s.storage, {"data": "ebs,10G,1"})
163
164=== modified file 'doc/config.rst'
165--- doc/config.rst 2016-12-23 16:54:49 +0000
166+++ doc/config.rst 2017-03-01 03:31:40 +0000
167@@ -36,6 +36,8 @@
168 tuning: optimized
169 engine: apache
170 icon: include-base64://file.ico
171+ resources:
172+ config: ./doc/cfg.xml
173 storage:
174 data-block: "ebs,1024G,3"
175
176@@ -54,12 +56,20 @@
177 include-base64://path-to-file. Relative paths are resolved
178 relative to the config file.
179
180+Service resources are specified as a dictionary with keys as the label
181+and the value being the path of the file containing the resource that
182+is to be be passed to Juju. This matches the Juju cli syntax,
183+`juju deploy <charm> --resource <label>=<file>`. NOTE: resources
184+status is not displayed in the diff command as it is not in the Juju
185+status data. For this same reason resources are only specified for new
186+services and are not added to already existing services.
187+
188 Service storage is specified as a dictionary with keys as the label
189-and the value being the pool,size,count string that should be passed
190+and the value being the pool,size,count string that is to be passed
191 to Juju. This matches the Juju cli syntax,
192 `juju deploy <charm> --storage <label>=<pool>,<size>,<count>`. Also
193 like the cli, size and count are optional parameters. NOTE: storage
194-status is not displayed in the diff command as isn't in the the Juju
195+status is not displayed in the diff command as it is not in the Juju
196 status data. For this same reason storage is only specified for new
197 services and is not added to already existing services.
198

Subscribers

People subscribed via source and target branches