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

[Blake Rouse]

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.

[Mike Pontillo (mpontillo)]

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

=== 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 @@
     return hosts
 
 
+def fix_sources_for_cluster(sources):
+    """Return modified sources that use the URL to the region defined in the
+    cluster configuration instead of the one the region suggested."""
+    sources = list(sources)
+    with ClusterConfiguration.open() as config:
+        maas_url = config.maas_url
+    maas_url_parsed = urlparse(maas_url)
+    maas_url_path = maas_url_parsed.path.lstrip('/').rstrip('/')
+    for source in sources:
+        url = urlparse(source['url'])
+        source_path = url.path.lstrip('/')
+        # Most likely they will both have 'MAAS/' at the start. We can't just
+        # append because then the URL would be 'MAAS/MAAS/' which is incorrect.
+        # If the initial part of the URL defined in the config matches the
+        # beginning of what the region told the cluster to use then strip it
+        # out and build the new URL.
+        if source_path.startswith(maas_url_path):
+            source_path = source_path[len(maas_url_path):]
+        url = maas_url.rstrip('/') + '/' + source_path.lstrip('/')
+        source['url'] = url
+    return sources
+
+
 @synchronous
 def _run_import(sources, http_proxy=None, https_proxy=None):
     """Run the import.
 
     This is function is synchronous so it must be called with deferToThread.
     """
+    # Fix the sources to download from the IP address defined in the cluster
+    # configuration, instead of the URL that the region asked it to use.
+    sources = fix_sources_for_cluster(sources)
     variables = {
         'GNUPGHOME': get_maas_user_gpghome(),
         }

=== 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 @@
 from provisioningserver.rpc import boot_images
 from provisioningserver.rpc.boot_images import (
     _run_import,
+    fix_sources_for_cluster,
     get_hosts_from_sources,
     import_boot_images,
     is_import_boot_images_running,
@@ -117,6 +118,72 @@
         self.assertItemsEqual(hosts, get_hosts_from_sources(sources))
 
 
+class TestFixSourcesForCluster(PservTestCase):
+
+    def set_maas_url(self, url):
+        self.useFixture(ClusterConfigurationFixture(maas_url=url))
+
+    def test__removes_matching_path_from_maas_url_with_extra_slashes(self):
+        self.set_maas_url("http://192.168.122.2/MAAS/////")
+        sources = [
+            {
+                "url": "http://localhost/MAAS/images/index.json"
+            }
+        ]
+        observered = fix_sources_for_cluster(sources)
+        self.assertEquals(
+            "http://192.168.122.2/MAAS/images/index.json",
+            observered[0]['url'])
+
+    def test__removes_matching_path_from_maas_url(self):
+        self.set_maas_url("http://192.168.122.2/MAAS/")
+        sources = [
+            {
+                "url": "http://localhost/MAAS/images/index.json"
+            }
+        ]
+        observered = fix_sources_for_cluster(sources)
+        self.assertEquals(
+            "http://192.168.122.2/MAAS/images/index.json",
+            observered[0]['url'])
+
+    def test__removes_matching_path_with_extra_slashes_from_maas_url(self):
+        self.set_maas_url("http://192.168.122.2/MAAS/")
+        sources = [
+            {
+                "url": "http://localhost///MAAS///images/index.json"
+            }
+        ]
+        observered = fix_sources_for_cluster(sources)
+        self.assertEquals(
+            "http://192.168.122.2/MAAS/images/index.json",
+            observered[0]['url'])
+
+    def test__doesnt_remove_non_matching_path_from_maas_url(self):
+        self.set_maas_url("http://192.168.122.2/not-matching/")
+        sources = [
+            {
+                "url": "http://localhost/MAAS/images/index.json"
+            }
+        ]
+        observered = fix_sources_for_cluster(sources)
+        self.assertEquals(
+            "http://192.168.122.2/not-matching/MAAS/images/index.json",
+            observered[0]['url'])
+
+    def test__doesnt_remove_non_matching_path_from_maas_url_with_slashes(self):
+        self.set_maas_url("http://192.168.122.2/not-matching////")
+        sources = [
+            {
+                "url": "http://localhost///MAAS/images/index.json"
+            }
+        ]
+        observered = fix_sources_for_cluster(sources)
+        self.assertEquals(
+            "http://192.168.122.2/not-matching/MAAS/images/index.json",
+            observered[0]['url'])
+
+
 class TestRunImport(PservTestCase):
 
     def make_archive_url(self, name=None):
@@ -169,12 +236,15 @@
         self.assertEqual(fake.env['no_proxy'], "localhost,127.0.0.1,::1")
 
     def test__run_import_sets_proxy_for_source_host(self):
-        sources, hosts = make_sources()
+        host = factory.make_name("host").lower()
+        maas_url = "http://%s/" % host
+        self.useFixture(ClusterConfigurationFixture(maas_url=maas_url))
+        sources, _ = make_sources()
         fake = self.patch_boot_resources_function()
         _run_import(sources=sources)
         self.assertItemsEqual(
             fake.env['no_proxy'].split(','),
-            ["localhost", "127.0.0.1", "::1"] + hosts)
+            ["localhost", "127.0.0.1", "::1"] + [host])
 
     def test__run_import_accepts_sources_parameter(self):
         fake = self.patch(boot_resources, 'import_images')