Merge lp:~rvb/maas/templ-back-bug-1439366 into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Add backward-compatibility fix to support templates created for earlier versions of MAAS when Ubuntu was the only OS supported.

This cannot "clash" since none of the architectures that MAAS support as a name that is the same as an OS name.

[Blake Rouse (blake-rouse)]

Overall looks good. Don't really like that its needed, but code wise it should work.

review: Approve

=== modified file 'docs/development/preseeds.rst'
--- docs/development/preseeds.rst	2015-02-13 09:06:14 +0000
+++ docs/development/preseeds.rst	2015-04-03 11:47:12 +0000
@@ -75,8 +75,25 @@
     {prefix}
     'generic'
 
-``prefix`` is either empty or one of ``enlist``, ``enlist_userdata``,
-``commissioning``, ``curtin``, ``curtin_userdata`` or ``preseed_master``.
+    Note: in order to be backward-compatible with earlier versions of MAAS that
+    only supported the Ubuntu OS, if the node OS is Ubuntu paths without the
+    {osystem} are also tried:
+    {prefix}_{osystem}_{node_arch}_{node_subarch}_{release}_{node_name}
+    {prefix}_{node_arch}_{node_subarch}_{release}_{node_name}
+    {prefix}_{osystem}_{node_arch}_{node_subarch}_{release}
+    {prefix}_{node_arch}_{node_subarch}_{release}
+    {prefix}_{osystem}_{node_arch}_{node_subarch}
+    {prefix}_{node_arch}_{node_subarch}
+    {prefix}_{osystem}_{node_arch}
+    {prefix}_{node_arch}
+    {prefix}_{osystem}
+    {prefix}
+    'generic'
+
+``prefix`` is either empty (in which case the following underscore is also
+ommitted: e.g. {osystem}_{node_arch}_{node_subarch}_{release}) or one of
+``enlist``, ``enlist_userdata``, ``commissioning``, ``curtin``,
+``curtin_userdata`` or ``preseed_master``.
 
 As you can see this mechanism is also used to calculate the base preseeds for
 all of installation, enlistment and commissioning.  It allows end users to

=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/preseed.py	2015-04-03 11:47:12 +0000
@@ -60,6 +60,7 @@
 from metadataserver.models import NodeKey
 from metadataserver.user_data.snippets import get_snippet_context
 from netaddr import IPAddress
+from provisioningserver.drivers.osystem.ubuntu import UbuntuOS
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
 from provisioningserver.utils import locate_config
 from provisioningserver.utils.fs import read_text_file
@@ -408,6 +409,9 @@
             osystem=node.get_osystem(), release=node.get_distro_series())
 
 
+UBUNTU_NAME = UbuntuOS().name
+
+
 def get_preseed_filenames(node, prefix='', osystem='', release='',
                           default=False):
     """List possible preseed template filenames for the given node.
@@ -435,11 +439,29 @@
     {prefix}_{osystem}
     {prefix}
     'generic'
+
+    Note: in order to be backward-compatible with earlier versions of MAAS that
+    only supported the Ubuntu OS, if the node OS is Ubuntu paths without the
+    {osystem} are also tried:
+    {prefix}_{osystem}_{node_arch}_{node_subarch}_{release}_{node_name}
+    {prefix}_{node_arch}_{node_subarch}_{release}_{node_name}
+    {prefix}_{osystem}_{node_arch}_{node_subarch}_{release}
+    {prefix}_{node_arch}_{node_subarch}_{release}
+    {prefix}_{osystem}_{node_arch}_{node_subarch}
+    {prefix}_{node_arch}_{node_subarch}
+    {prefix}_{osystem}_{node_arch}
+    {prefix}_{node_arch}
+    {prefix}_{osystem}
+    {prefix}
+    'generic'
     """
     elements = []
     # Add prefix.
     if prefix != '':
         elements.append(prefix)
+        has_prefix = True
+    else:
+        has_prefix = False
     # Add osystem
     elements.append(osystem)
     # Add architecture/sub-architecture.
@@ -453,6 +475,16 @@
         elements.append(node.hostname)
     while elements:
         yield compose_filename(elements)
+        # Backward-compatibility fix for 1439366: also generate a filename
+        # with the 'osystem' omitted when deploying with Ubuntu.
+        if osystem == UBUNTU_NAME:
+            should_emit = (
+                (not has_prefix and len(elements) > 1) or
+                (has_prefix and len(elements) > 2))
+            if should_emit:
+                cutoff = 1 if has_prefix else 0
+                yield compose_filename(
+                    elements[:cutoff] + elements[cutoff + 1:])
         elements.pop()
     if default:
         yield GENERIC_FILENAME

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/tests/test_preseed.py	2015-04-03 11:47:12 +0000
@@ -76,6 +76,7 @@
 from maastesting.matchers import MockCalledOnceWith
 from metadataserver.models import NodeKey
 from mock import ANY
+from provisioningserver.drivers.osystem.ubuntu import UbuntuOS
 from provisioningserver.rpc.exceptions import NoConnectionsAvailable
 from provisioningserver.utils import locate_config
 from provisioningserver.utils.enum import map_enum
@@ -145,7 +146,7 @@
 class TestGetPreseedFilenames(MAASServerTestCase):
     """Tests for `get_preseed_filenames`."""
 
-    def test_get_preseed_filenames_returns_filenames(self):
+    def test__returns_filenames(self):
         hostname = factory.make_string()
         prefix = factory.make_string()
         osystem = factory.make_string()
@@ -166,7 +167,7 @@
             list(get_preseed_filenames(
                 node, prefix, osystem, release, default=True)))
 
-    def test_get_preseed_filenames_if_node_is_None(self):
+    def test__returns_limited_filenames_if_node_is_None(self):
         osystem = factory.make_string()
         release = factory.make_string()
         prefix = factory.make_string()
@@ -178,7 +179,7 @@
             ],
             list(get_preseed_filenames(None, prefix, osystem, release)))
 
-    def test_get_preseed_filenames_supports_empty_prefix(self):
+    def test__supports_empty_prefix(self):
         hostname = factory.make_string()
         osystem = factory.make_string()
         release = factory.make_string()
@@ -194,7 +195,7 @@
             ],
             list(get_preseed_filenames(node, '', osystem, release)))
 
-    def test_get_preseed_filenames_returns_list_without_default(self):
+    def test__returns_list_without_default(self):
         # If default=False is passed to get_preseed_filenames, the
         # returned list won't include the default template name as a
         # last resort template.
@@ -207,7 +208,7 @@
             list(get_preseed_filenames(
                 node, prefix, release, default=True))[-1])
 
-    def test_get_preseed_filenames_returns_list_with_default(self):
+    def test__returns_list_with_default(self):
         # If default=True is passed to get_preseed_filenames, the
         # returned list will include the default template name as a
         # last resort template.
@@ -220,6 +221,53 @@
             list(get_preseed_filenames(
                 node, prefix, release, default=False))[-1])
 
+    def test__returns_backward_compatible_name_for_ubuntu_without_prefix(self):
+        # If the OS is Ubuntu, also include backward-compatible filenames.
+        # See bug 1439366 for details.
+        hostname = factory.make_string()
+        osystem = UbuntuOS().name
+        release = factory.make_string()
+        node = factory.make_Node(hostname=hostname)
+        arch, subarch = node.architecture.split('/')
+        self.assertSequenceEqual(
+            [
+                '%s_%s_%s_%s_%s' % (osystem, arch, subarch, release, hostname),
+                '%s_%s_%s_%s' % (arch, subarch, release, hostname),
+                '%s_%s_%s_%s' % (osystem, arch, subarch, release),
+                '%s_%s_%s' % (arch, subarch, release),
+                '%s_%s_%s' % (osystem, arch, subarch),
+                '%s_%s' % (arch, subarch),
+                '%s_%s' % (osystem, arch),
+                '%s' % (arch),
+                '%s' % osystem,
+            ],
+            list(get_preseed_filenames(node, '', osystem, release)))
+
+    def test__returns_backward_compatible_name_for_ubuntu_with_prefix(self):
+        # If the OS is Ubuntu, also include backward-compatible filenames.
+        # See bug 1439366 for details.
+        hostname = factory.make_string()
+        osystem = UbuntuOS().name
+        release = factory.make_string()
+        node = factory.make_Node(hostname=hostname)
+        arch, subarch = node.architecture.split('/')
+        prefix = factory.make_string()
+        self.assertSequenceEqual(
+            [
+                '%s_%s_%s_%s_%s_%s' % (
+                    prefix, osystem, arch, subarch, release, hostname),
+                '%s_%s_%s_%s_%s' % (prefix, arch, subarch, release, hostname),
+                '%s_%s_%s_%s_%s' % (prefix, osystem, arch, subarch, release),
+                '%s_%s_%s_%s' % (prefix, arch, subarch, release),
+                '%s_%s_%s_%s' % (prefix, osystem, arch, subarch),
+                '%s_%s_%s' % (prefix, arch, subarch),
+                '%s_%s_%s' % (prefix, osystem, arch),
+                '%s_%s' % (prefix, arch),
+                '%s_%s' % (prefix, osystem),
+                '%s' % prefix,
+            ],
+            list(get_preseed_filenames(node, prefix, osystem, release)))
+
 
 class TestConfiguration(MAASServerTestCase):
     """Test for correct configuration of the preseed component."""