Merge lp:~johnsca/juju-deployer/series-fix into lp:juju-deployer

Proposed by Cory Johns
Status: Merged
Merged at revision: 197
Proposed branch: lp:~johnsca/juju-deployer/series-fix
Merge into: lp:juju-deployer
Diff against target: 77 lines (+34/-1)
2 files modified
deployer/charm.py (+22/-1)
deployer/tests/test_charm.py (+12/-0)
To merge this branch: bzr merge lp:~johnsca/juju-deployer/series-fix
Reviewer Review Type Date Requested Status
Konstantinos Tsakalozos (community) Approve
Kevin W Monroe (community) Approve
juju-deployers Pending
Review via email: mp+309143@code.launchpad.net

Description of the change

When a bundle contains charm URLs with the series embedded that don't match the bundle-level series, deployer didn't honor it and tried to use the bundle series instead.

To post a comment you must log in.
Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

LGTM, +1

review: Approve
Revision history for this message
Konstantinos Tsakalozos (kos.tsakalozos) wrote :

LGTM +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'deployer/charm.py'
--- deployer/charm.py 2016-09-14 02:25:35 +0000
+++ deployer/charm.py 2016-10-24 16:54:17 +0000
@@ -71,6 +71,22 @@
71 name = data.get('charm', name)71 name = data.get('charm', name)
72 if name.startswith('cs:'):72 if name.startswith('cs:'):
73 store_url = name73 store_url = name
74 if 'series' not in data:
75 series = deploy_series
76 if ':' in store_url and '/' in store_url:
77 # attempt to extract series from charm URL
78 url_parts = store_url.split(':', 1)[1].split('/')
79 # remove namespace
80 if url_parts[0].startswith('~'):
81 url_parts = url_parts[1:]
82 elif url_parts[0] == 'u':
83 url_parts = url_parts[2:]
84 # check for series in URL
85 if len(url_parts) > 1:
86 # URL includes series
87 series = url_parts[0]
88 # update data dict with series
89 data = dict(data, series=series)
74 elif name.startswith('local:'):90 elif name.startswith('local:'):
75 # Support vcs charms specifying their91 # Support vcs charms specifying their
76 parts = name[len('local:'):].split('/')92 parts = name[len('local:'):].split('/')
@@ -81,6 +97,7 @@
81 name = parts.pop()97 name = parts.pop()
82 else:98 else:
83 series = deploy_series99 series = deploy_series
100 data = dict(data, series=deploy_series)
84 charm_path = path_join(repo_path, series, name)101 charm_path = path_join(repo_path, series, name)
85 elif os.path.isabs(name):102 elif os.path.isabs(name):
86 # charm points to an absolute local path103 # charm points to an absolute local path
@@ -90,6 +107,7 @@
90 charm_path = path_join(repo_path, series, name)107 charm_path = path_join(repo_path, series, name)
91 else:108 else:
92 charm_path = path_join(repo_path, deploy_series, name)109 charm_path = path_join(repo_path, deploy_series, name)
110 data = dict(data, series=deploy_series)
93111
94 if not store_url:112 if not store_url:
95 store_url = data.get('charm_url', None)113 store_url = data.get('charm_url', None)
@@ -185,7 +203,10 @@
185 def series(self):203 def series(self):
186 if self.data.get('series'):204 if self.data.get('series'):
187 return self.data.get('series')205 return self.data.get('series')
188 metadata_series = self.metadata.get('series')206 try:
207 metadata_series = self.metadata.get('series')
208 except RuntimeError:
209 metadata_series = None
189 if metadata_series:210 if metadata_series:
190 return metadata_series[0]211 return metadata_series[0]
191 if self.series_path:212 if self.series_path:
192213
=== modified file 'deployer/tests/test_charm.py'
--- deployer/tests/test_charm.py 2016-05-13 14:16:48 +0000
+++ deployer/tests/test_charm.py 2016-10-24 16:54:17 +0000
@@ -21,6 +21,18 @@
21 def test_repo_path(self):21 def test_repo_path(self):
22 self.assertIsNone(self.charm.repo_path)22 self.assertIsNone(self.charm.repo_path)
2323
24 def test_series(self):
25 def _c(url, deploy_series, data_series):
26 return Charm.from_service(url, None, deploy_series, {
27 'series': data_series,
28 } if data_series else {}).series
29 self.assertEqual('trusty', _c('cs:trusty/ubuntu', None, None))
30 self.assertEqual('trusty', _c('cs:trusty/ubuntu', 'xenial', None))
31 self.assertEqual('xenial', _c('cs:trusty/ubuntu', 'trusty', 'xenial'))
32 self.assertEqual('xenial', _c('cs:ubuntu', 'trusty', 'xenial'))
33 self.assertEqual('trusty', _c('cs:ubuntu', 'trusty', None))
34 self.assertIsNone(_c('cs:ubuntu', None, None))
35
2436
25class AbsolutePathCharmTest(Base):37class AbsolutePathCharmTest(Base):
26 def setUp(self):38 def setUp(self):

Subscribers

People subscribed via source and target branches