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

Proposed by Blake Rouse on 2015-11-10
Status: Merged
Approved by: Blake Rouse on 2015-11-11
Approved revision: 4488
Merged at revision: 4487
Proposed branch: lp:~blake-rouse/maas/fix-1514883
Merge into: lp: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) 2015-11-10 Approve on 2015-11-11
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.
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
lp:~blake-rouse/maas/fix-1514883 updated on 2015-11-11
4487. By Blake Rouse on 2015-11-11

Add more tests.

4488. By Blake Rouse on 2015-11-11

Fix formatting.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/provisioningserver/rpc/boot_images.py'
2--- src/provisioningserver/rpc/boot_images.py 2015-08-20 14:59:45 +0000
3+++ src/provisioningserver/rpc/boot_images.py 2015-11-11 02:30:34 +0000
4@@ -67,12 +67,38 @@
5 return hosts
6
7
8+def fix_sources_for_cluster(sources):
9+ """Return modified sources that use the URL to the region defined in the
10+ cluster configuration instead of the one the region suggested."""
11+ sources = list(sources)
12+ with ClusterConfiguration.open() as config:
13+ maas_url = config.maas_url
14+ maas_url_parsed = urlparse(maas_url)
15+ maas_url_path = maas_url_parsed.path.lstrip('/').rstrip('/')
16+ for source in sources:
17+ url = urlparse(source['url'])
18+ source_path = url.path.lstrip('/')
19+ # Most likely they will both have 'MAAS/' at the start. We can't just
20+ # append because then the URL would be 'MAAS/MAAS/' which is incorrect.
21+ # If the initial part of the URL defined in the config matches the
22+ # beginning of what the region told the cluster to use then strip it
23+ # out and build the new URL.
24+ if source_path.startswith(maas_url_path):
25+ source_path = source_path[len(maas_url_path):]
26+ url = maas_url.rstrip('/') + '/' + source_path.lstrip('/')
27+ source['url'] = url
28+ return sources
29+
30+
31 @synchronous
32 def _run_import(sources, http_proxy=None, https_proxy=None):
33 """Run the import.
34
35 This is function is synchronous so it must be called with deferToThread.
36 """
37+ # Fix the sources to download from the IP address defined in the cluster
38+ # configuration, instead of the URL that the region asked it to use.
39+ sources = fix_sources_for_cluster(sources)
40 variables = {
41 'GNUPGHOME': get_maas_user_gpghome(),
42 }
43
44=== modified file 'src/provisioningserver/rpc/tests/test_boot_images.py'
45--- src/provisioningserver/rpc/tests/test_boot_images.py 2015-06-25 23:07:31 +0000
46+++ src/provisioningserver/rpc/tests/test_boot_images.py 2015-11-11 02:30:34 +0000
47@@ -33,6 +33,7 @@
48 from provisioningserver.rpc import boot_images
49 from provisioningserver.rpc.boot_images import (
50 _run_import,
51+ fix_sources_for_cluster,
52 get_hosts_from_sources,
53 import_boot_images,
54 is_import_boot_images_running,
55@@ -117,6 +118,72 @@
56 self.assertItemsEqual(hosts, get_hosts_from_sources(sources))
57
58
59+class TestFixSourcesForCluster(PservTestCase):
60+
61+ def set_maas_url(self, url):
62+ self.useFixture(ClusterConfigurationFixture(maas_url=url))
63+
64+ def test__removes_matching_path_from_maas_url_with_extra_slashes(self):
65+ self.set_maas_url("http://192.168.122.2/MAAS/////")
66+ sources = [
67+ {
68+ "url": "http://localhost/MAAS/images/index.json"
69+ }
70+ ]
71+ observered = fix_sources_for_cluster(sources)
72+ self.assertEquals(
73+ "http://192.168.122.2/MAAS/images/index.json",
74+ observered[0]['url'])
75+
76+ def test__removes_matching_path_from_maas_url(self):
77+ self.set_maas_url("http://192.168.122.2/MAAS/")
78+ sources = [
79+ {
80+ "url": "http://localhost/MAAS/images/index.json"
81+ }
82+ ]
83+ observered = fix_sources_for_cluster(sources)
84+ self.assertEquals(
85+ "http://192.168.122.2/MAAS/images/index.json",
86+ observered[0]['url'])
87+
88+ def test__removes_matching_path_with_extra_slashes_from_maas_url(self):
89+ self.set_maas_url("http://192.168.122.2/MAAS/")
90+ sources = [
91+ {
92+ "url": "http://localhost///MAAS///images/index.json"
93+ }
94+ ]
95+ observered = fix_sources_for_cluster(sources)
96+ self.assertEquals(
97+ "http://192.168.122.2/MAAS/images/index.json",
98+ observered[0]['url'])
99+
100+ def test__doesnt_remove_non_matching_path_from_maas_url(self):
101+ self.set_maas_url("http://192.168.122.2/not-matching/")
102+ sources = [
103+ {
104+ "url": "http://localhost/MAAS/images/index.json"
105+ }
106+ ]
107+ observered = fix_sources_for_cluster(sources)
108+ self.assertEquals(
109+ "http://192.168.122.2/not-matching/MAAS/images/index.json",
110+ observered[0]['url'])
111+
112+ def test__doesnt_remove_non_matching_path_from_maas_url_with_slashes(self):
113+ self.set_maas_url("http://192.168.122.2/not-matching////")
114+ sources = [
115+ {
116+ "url": "http://localhost///MAAS/images/index.json"
117+ }
118+ ]
119+ observered = fix_sources_for_cluster(sources)
120+ self.assertEquals(
121+ "http://192.168.122.2/not-matching/MAAS/images/index.json",
122+ observered[0]['url'])
123+
124+
125 class TestRunImport(PservTestCase):
126
127 def make_archive_url(self, name=None):
128@@ -169,12 +236,15 @@
129 self.assertEqual(fake.env['no_proxy'], "localhost,127.0.0.1,::1")
130
131 def test__run_import_sets_proxy_for_source_host(self):
132- sources, hosts = make_sources()
133+ host = factory.make_name("host").lower()
134+ maas_url = "http://%s/" % host
135+ self.useFixture(ClusterConfigurationFixture(maas_url=maas_url))
136+ sources, _ = make_sources()
137 fake = self.patch_boot_resources_function()
138 _run_import(sources=sources)
139 self.assertItemsEqual(
140 fake.env['no_proxy'].split(','),
141- ["localhost", "127.0.0.1", "::1"] + hosts)
142+ ["localhost", "127.0.0.1", "::1"] + [host])
143
144 def test__run_import_accepts_sources_parameter(self):
145 fake = self.patch(boot_resources, 'import_images')