launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/mirror-settings-re-3 into lp:maas with lp:~rvb/maas/mirror-settings-re-2 as a prerequisite.

Commit message:
Use the archive settings in the generated preseed.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/mirror-settings-re-3/+merge/133464

This is a followup on https://code.launchpad.net/~rvb/maas/mirror-settings-re-2/+merge/133461.

Use the new settings in the generated preseed.

= Notes =

There is one quirk: even if the setting 'main_archive' uses https (as opposed to http), the format of the preseed forces us to use 'http' as the transport mechanism.  Not really sure what to do here, I tried to see if d-i would support something like "d-i     mirror/https/hostname string my.host.com" but I could not find anything.  Another solution is to add a tiny validator to prevent reject urls with 'https' for the settings 'main_archive' and 'ports_archive'.  Anyway, this is open for discussion, I think this can be tackled in a follow-up branch.
-- 
https://code.launchpad.net/~rvb/maas/mirror-settings-re-3/+merge/133464
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/mirror-settings-re-3 into lp:maas.

=== modified file 'contrib/preseeds_v2/generic'
--- contrib/preseeds_v2/generic	2012-10-05 02:18:39 +0000
+++ contrib/preseeds_v2/generic	2012-11-08 14:28:24 +0000
@@ -12,11 +12,11 @@
 {{def proxy}}
 d-i     mirror/country string manual
 {{if node.architecture in {'i386/generic', 'amd64/generic'} }}
-d-i     mirror/http/hostname string archive.ubuntu.com
-d-i     mirror/http/directory string /ubuntu
+d-i     mirror/http/hostname string {{main_archive_hostname}}
+d-i     mirror/http/directory string {{main_archive_directory}}
 {{else}}
-d-i     mirror/http/hostname string ports.ubuntu.com
-d-i     mirror/http/directory string /ubuntu-ports
+d-i     mirror/http/hostname string {{ports_archive_hostname}}
+d-i     mirror/http/directory string {{ports_archive_directory}}
 {{endif}}
 d-i     mirror/http/proxy string http://{{server_host}}:8000/
 {{enddef}}

=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py	2012-11-08 06:34:48 +0000
+++ src/maasserver/preseed.py	2012-11-08 14:28:24 +0000
@@ -21,6 +21,7 @@
 from os.path import join
 from pipes import quote
 from urllib import urlencode
+from urlparse import urlparse
 
 from django.conf import settings
 from maasserver.compose_preseed import compose_preseed
@@ -203,6 +204,12 @@
     return get_template(prefix, None, default=True)
 
 
+def get_hostname_and_path(url):
+    """Return a tuple of the hostname and the hierarchical path from a url."""
+    parsed_url = urlparse(url)
+    return parsed_url.hostname, parsed_url.path
+
+
 def get_preseed_context(node, release=''):
     """Return the context dictionary to be used to render preseed templates
     for this node.
@@ -214,7 +221,15 @@
     :rtype: dict.
     """
     server_host = get_maas_facing_server_host()
+    main_archive_hostname, main_archive_directory = get_hostname_and_path(
+        Config.objects.get_config('main_archive'))
+    ports_archive_hostname, ports_archive_directory = get_hostname_and_path(
+        Config.objects.get_config('ports_archive'))
     context = {
+        'main_archive_hostname': main_archive_hostname,
+        'main_archive_directory': main_archive_directory,
+        'ports_archive_hostname': ports_archive_hostname,
+        'ports_archive_directory': ports_archive_directory,
         'release': release,
         'server_host': server_host,
         'server_url': absolute_reverse('nodes_handler'),

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2012-11-08 14:28:24 +0000
+++ src/maasserver/tests/test_preseed.py	2012-11-08 14:28:24 +0000
@@ -22,12 +22,16 @@
     NODE_STATUS,
     PRESEED_TYPE,
     )
-from maasserver.models import BootImage
+from maasserver.models import (
+    BootImage,
+    Config,
+    )
 from maasserver.preseed import (
     compose_enlistment_preseed_url,
     compose_preseed_url,
     GENERIC_FILENAME,
     get_enlist_preseed,
+    get_hostname_and_path,
     get_preseed,
     get_preseed_context,
     get_preseed_filenames,
@@ -60,6 +64,20 @@
         self.assertEqual(['amd64', 'test'], split_subarch('amd64/test'))
 
 
+class TestGetHostnameAndPath(TestCase):
+    """Tests for `get_hostname_and_path`."""
+
+    def test_get_hostname_and_path(self):
+        input_and_results = [
+            ('http://name.domain/my/path',  ('name.domain', '/my/path')),
+            ('https://domain/path',  ('domain', '/path')),
+            ('http://domain/',  ('domain', '/')),
+            ]
+        inputs = [input for input, _ in input_and_results]
+        results = [result for _, result in input_and_results]
+        self.assertEqual(results, map(get_hostname_and_path, inputs))
+
+
 class TestGetPreseedFilenames(TestCase):
     """Tests for `get_preseed_filenames`."""
 
@@ -300,7 +318,9 @@
             ['node', 'release', 'metadata_enlist_url',
              'server_host', 'server_url', 'preseed_data',
              'node_disable_pxe_url', 'node_disable_pxe_data',
-             'use_squashfs'],
+             'use_squashfs', 'main_archive_hostname',
+             'main_archive_directory', 'ports_archive_hostname',
+             'ports_archive_directory'],
             context)
 
     def test_get_preseed_context_if_node_None(self):
@@ -310,9 +330,40 @@
         release = factory.getRandomString()
         context = get_preseed_context(None, release)
         self.assertItemsEqual(
-            ['release', 'metadata_enlist_url', 'server_host', 'server_url'],
+            ['release', 'metadata_enlist_url', 'server_host', 'server_url',
+            'main_archive_hostname', 'main_archive_directory',
+            'ports_archive_hostname', 'ports_archive_directory'],
             context)
 
+    def test_get_preseed_context_archive_refs(self):
+        # urlparse lowercases the hostnames. That should not have any
+        # impact but for testing, create lower-case hostnames.
+        main_archive_hostname = factory.make_name('hostname').lower()
+        main_archive_directory = factory.make_name('directory')
+        ports_archive_hostname = factory.make_name('hostname').lower()
+        ports_archive_directory = factory.make_name('directory')
+        main_archive = 'http://%s/%s' % (
+            main_archive_hostname, main_archive_directory)
+        ports_archive = 'http://%s/%s' % (
+            ports_archive_hostname, ports_archive_directory)
+        Config.objects.set_config('main_archive', main_archive)
+        Config.objects.set_config('ports_archive', ports_archive)
+        context = get_preseed_context(
+            factory.make_node(), factory.getRandomString())
+        self.assertEqual(
+            (
+                context['main_archive_hostname'],
+                context['main_archive_directory'],
+                context['ports_archive_hostname'],
+                context['ports_archive_directory'],
+            ),
+            (
+                main_archive_hostname,
+                '/%s' % main_archive_directory,
+                ports_archive_hostname,
+                '/%s' % ports_archive_directory,
+            ))
+
 
 class TestSquashFSAvailable(TestCase):
     """Tests for `is_squashfs_image_present`."""