Merge lp:~blake-rouse/maas/fix-1514883 into lp:~maas-committers/maas/trunk

Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: no longer in the source branch.
Merged at revision: 4487
Proposed branch: lp:~blake-rouse/maas/fix-1514883
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 145 lines (+98/-2)
2 files modified
src/provisioningserver/rpc/boot_images.py (+26/-0)
src/provisioningserver/rpc/tests/test_boot_images.py (+72/-2)
To merge this branch: bzr merge lp:~blake-rouse/maas/fix-1514883
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
Review via email: mp+277201@code.launchpad.net

Commit message

Replace the URL provided from the region with the maas_url from the cluster configuration file. This will make sure that the cluster uses the same URL to connect for RPC for downloading boot images.

To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Just a few minor issues/suggestions:

 - Format imports.
 - Refactor fix_sources_for_cluster to pull out URL replacement logic.
 - Add tests for more edge cases in URL replacement logic.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/provisioningserver/rpc/boot_images.py'
--- src/provisioningserver/rpc/boot_images.py 2015-08-20 14:59:45 +0000
+++ src/provisioningserver/rpc/boot_images.py 2015-11-11 02:30:34 +0000
@@ -67,12 +67,38 @@
67 return hosts67 return hosts
6868
6969
70def fix_sources_for_cluster(sources):
71 """Return modified sources that use the URL to the region defined in the
72 cluster configuration instead of the one the region suggested."""
73 sources = list(sources)
74 with ClusterConfiguration.open() as config:
75 maas_url = config.maas_url
76 maas_url_parsed = urlparse(maas_url)
77 maas_url_path = maas_url_parsed.path.lstrip('/').rstrip('/')
78 for source in sources:
79 url = urlparse(source['url'])
80 source_path = url.path.lstrip('/')
81 # Most likely they will both have 'MAAS/' at the start. We can't just
82 # append because then the URL would be 'MAAS/MAAS/' which is incorrect.
83 # If the initial part of the URL defined in the config matches the
84 # beginning of what the region told the cluster to use then strip it
85 # out and build the new URL.
86 if source_path.startswith(maas_url_path):
87 source_path = source_path[len(maas_url_path):]
88 url = maas_url.rstrip('/') + '/' + source_path.lstrip('/')
89 source['url'] = url
90 return sources
91
92
70@synchronous93@synchronous
71def _run_import(sources, http_proxy=None, https_proxy=None):94def _run_import(sources, http_proxy=None, https_proxy=None):
72 """Run the import.95 """Run the import.
7396
74 This is function is synchronous so it must be called with deferToThread.97 This is function is synchronous so it must be called with deferToThread.
75 """98 """
99 # Fix the sources to download from the IP address defined in the cluster
100 # configuration, instead of the URL that the region asked it to use.
101 sources = fix_sources_for_cluster(sources)
76 variables = {102 variables = {
77 'GNUPGHOME': get_maas_user_gpghome(),103 'GNUPGHOME': get_maas_user_gpghome(),
78 }104 }
79105
=== modified file 'src/provisioningserver/rpc/tests/test_boot_images.py'
--- src/provisioningserver/rpc/tests/test_boot_images.py 2015-06-25 23:07:31 +0000
+++ src/provisioningserver/rpc/tests/test_boot_images.py 2015-11-11 02:30:34 +0000
@@ -33,6 +33,7 @@
33from provisioningserver.rpc import boot_images33from provisioningserver.rpc import boot_images
34from provisioningserver.rpc.boot_images import (34from provisioningserver.rpc.boot_images import (
35 _run_import,35 _run_import,
36 fix_sources_for_cluster,
36 get_hosts_from_sources,37 get_hosts_from_sources,
37 import_boot_images,38 import_boot_images,
38 is_import_boot_images_running,39 is_import_boot_images_running,
@@ -117,6 +118,72 @@
117 self.assertItemsEqual(hosts, get_hosts_from_sources(sources))118 self.assertItemsEqual(hosts, get_hosts_from_sources(sources))
118119
119120
121class TestFixSourcesForCluster(PservTestCase):
122
123 def set_maas_url(self, url):
124 self.useFixture(ClusterConfigurationFixture(maas_url=url))
125
126 def test__removes_matching_path_from_maas_url_with_extra_slashes(self):
127 self.set_maas_url("http://192.168.122.2/MAAS/////")
128 sources = [
129 {
130 "url": "http://localhost/MAAS/images/index.json"
131 }
132 ]
133 observered = fix_sources_for_cluster(sources)
134 self.assertEquals(
135 "http://192.168.122.2/MAAS/images/index.json",
136 observered[0]['url'])
137
138 def test__removes_matching_path_from_maas_url(self):
139 self.set_maas_url("http://192.168.122.2/MAAS/")
140 sources = [
141 {
142 "url": "http://localhost/MAAS/images/index.json"
143 }
144 ]
145 observered = fix_sources_for_cluster(sources)
146 self.assertEquals(
147 "http://192.168.122.2/MAAS/images/index.json",
148 observered[0]['url'])
149
150 def test__removes_matching_path_with_extra_slashes_from_maas_url(self):
151 self.set_maas_url("http://192.168.122.2/MAAS/")
152 sources = [
153 {
154 "url": "http://localhost///MAAS///images/index.json"
155 }
156 ]
157 observered = fix_sources_for_cluster(sources)
158 self.assertEquals(
159 "http://192.168.122.2/MAAS/images/index.json",
160 observered[0]['url'])
161
162 def test__doesnt_remove_non_matching_path_from_maas_url(self):
163 self.set_maas_url("http://192.168.122.2/not-matching/")
164 sources = [
165 {
166 "url": "http://localhost/MAAS/images/index.json"
167 }
168 ]
169 observered = fix_sources_for_cluster(sources)
170 self.assertEquals(
171 "http://192.168.122.2/not-matching/MAAS/images/index.json",
172 observered[0]['url'])
173
174 def test__doesnt_remove_non_matching_path_from_maas_url_with_slashes(self):
175 self.set_maas_url("http://192.168.122.2/not-matching////")
176 sources = [
177 {
178 "url": "http://localhost///MAAS/images/index.json"
179 }
180 ]
181 observered = fix_sources_for_cluster(sources)
182 self.assertEquals(
183 "http://192.168.122.2/not-matching/MAAS/images/index.json",
184 observered[0]['url'])
185
186
120class TestRunImport(PservTestCase):187class TestRunImport(PservTestCase):
121188
122 def make_archive_url(self, name=None):189 def make_archive_url(self, name=None):
@@ -169,12 +236,15 @@
169 self.assertEqual(fake.env['no_proxy'], "localhost,127.0.0.1,::1")236 self.assertEqual(fake.env['no_proxy'], "localhost,127.0.0.1,::1")
170237
171 def test__run_import_sets_proxy_for_source_host(self):238 def test__run_import_sets_proxy_for_source_host(self):
172 sources, hosts = make_sources()239 host = factory.make_name("host").lower()
240 maas_url = "http://%s/" % host
241 self.useFixture(ClusterConfigurationFixture(maas_url=maas_url))
242 sources, _ = make_sources()
173 fake = self.patch_boot_resources_function()243 fake = self.patch_boot_resources_function()
174 _run_import(sources=sources)244 _run_import(sources=sources)
175 self.assertItemsEqual(245 self.assertItemsEqual(
176 fake.env['no_proxy'].split(','),246 fake.env['no_proxy'].split(','),
177 ["localhost", "127.0.0.1", "::1"] + hosts)247 ["localhost", "127.0.0.1", "::1"] + [host])
178248
179 def test__run_import_accepts_sources_parameter(self):249 def test__run_import_accepts_sources_parameter(self):
180 fake = self.patch(boot_resources, 'import_images')250 fake = self.patch(boot_resources, 'import_images')