launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/x86_64 into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/x86_64/+merge/97569

It's been decided that while MaaS as such will speak consistently of amd64, Cobbler will continue to see the name x86_64 instead.  The maas-import-distros script already goes through this conversion when talking to Cobbler, although the code could do with some more clarity.  And where it calls the cobbler-ubuntu-import script (part of the Ubuntu Cobbler package), that script in turn converts back to amd64 for download purposes.

This branch mainly fixes up the (Cobbler) profile names we assign to (Cobbler) systems when saving Nodes.  For uniformity, I've given the same conversion the same name in the shell script as in the python code.

One seemingly unrelated test fails intermittently in this branch, but it fails in trunk as well and the failure seems to be purely a Rabbit issue.  See bug 955674.  Also, make sure to start up a system-installed Rabbit or many other tests will fail as well (bug 955679).  I don't think it could be hiding any problems in this particular branch.
-- 
https://code.launchpad.net/~jtv/maas/x86_64/+merge/97569
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/x86_64 into lp:maas.

=== modified file 'scripts/maas-import-isos'
--- scripts/maas-import-isos	2012-03-01 15:42:15 +0000
+++ scripts/maas-import-isos	2012-03-15 08:02:19 +0000
@@ -48,40 +48,50 @@
 	fi
 fi
 
+
+name_arch_in_cobbler_style() {
+	echo "$1" | sed -e 's/amd64/x86_64/' -e 's/i686/i386/'
+}
+
+
 update_settings(){
+	local r a profile
 	# Updates the parent profiles with new settings. Only affects KOPTS
 	for r in $RELEASES; do
 		for a in $ARCHES; do
-			[ "$a" = "amd64" ] && a="x86_64"
+			a=`name_arch_in_cobbler_style "$a"`
+			profile="$r-$a"
 			# If profile exists, update it.
-			if (cobbler profile list | grep -qs " $r-$a$"); then
-				cobbler profile edit --name="$r-$a" --kopts="$KOPTS"
+			if (cobbler profile list | grep -qs " $profile$"); then
+				cobbler profile edit --name="$profile" --kopts="$KOPTS"
 			fi
 		done
 	done
 }
 
 import_isos(){
+	local r a profile
 	# Wget and import net install ISOs
 	for r in $RELEASES; do
 		for a in $ARCHES; do
-			[ "$a" = "amd64" ] && a="x86_64"
+			a=`name_arch_in_cobbler_style "$a"`
+			profile="$r-$a"
 			# Skip if cobbler already has this distro/arch combo
-			if ! (cobbler distro list | grep -qs " $r-$a$"); then
+			if ! (cobbler distro list | grep -qs " $profile$"); then
 				# Import the iso
-				cobbler-ubuntu-import $r-$a
-				cobbler profile edit --name="$r-$a" --kopts="$KOPTS" --kickstart="$KSDIR/maas.preseed"
+				cobbler-ubuntu-import $profile
+				cobbler profile edit --name="$profile" --kopts="$KOPTS" --kickstart="$KSDIR/maas.preseed"
 			else
 				# check archive for an updated ISO, update our cache if necessary
-				cobbler-ubuntu-import -c $r-$a && cobbler-ubuntu-import -u $r-$a
+				cobbler-ubuntu-import -c $profile && cobbler-ubuntu-import -u $profile
 				# Make sure the profile is using the MaaS preseed
-				cobbler profile edit --name="$r-$a" --kickstart="$KSDIR/maas.preseed"
+				cobbler profile edit --name="$profile" --kickstart="$KSDIR/maas.preseed"
 
 			fi
 			# Skip if cobbler already has this distro/arch profile
-			if ! (cobbler profile list | grep -qs " $r-$a-juju$"); then
+			if ! (cobbler profile list | grep -qs " $profile-juju$"); then
 				# Add JuJu sub-profile based on the name of the imported ISO
-				cobbler profile add --name="$r-$a-juju" --parent="$r-$a" --kickstart="$KSDIR/juju.preseed"
+				cobbler profile add --name="$profile-juju" --parent="$profile" --kickstart="$KSDIR/juju.preseed"
 			fi
 		done
 	done

=== modified file 'src/maas/demo.py'
--- src/maas/demo.py	2012-03-14 16:59:25 +0000
+++ src/maas/demo.py	2012-03-15 08:02:19 +0000
@@ -12,9 +12,9 @@
 
 import os
 
+from maas.development import *
 # SKIP, developement settings should override base settings.
 from maas.settings import *
-from maas.development import *
 
 
 MEDIA_ROOT = os.path.join(os.getcwd(), "media/demo")

=== modified file 'src/maasserver/provisioning.py'
--- src/maasserver/provisioning.py	2012-03-14 10:00:43 +0000
+++ src/maasserver/provisioning.py	2012-03-15 08:02:19 +0000
@@ -82,10 +82,31 @@
     }
 
 
+def name_arch_in_cobbler_style(architecture):
+    """Give architecture name as used in cobbler.
+
+    MaaS uses Ubuntu-style architecture names, notably including "amd64"
+    which in Cobbler terms is "x86_64."
+
+    :param architecture: An architecture name (e.g. as produced by MaaS).
+    :type architecture: basestring
+    :return: An architecture name in Cobbler style.
+    :rtype: unicode
+    """
+    conversions = {
+        'amd64': 'x86_64',
+        'i686': 'i386',
+    }
+    if isinstance(architecture, bytes):
+        architecture = architecture.decode('ascii')
+    return conversions.get(architecture, architecture)
+
+
 def select_profile_for_node(node, papi):
     """Select which profile a node should be configured for."""
     assert node.architecture, "Node's architecture is not known."
-    return "%s-%s" % ("precise", node.architecture)
+    cobbler_arch = name_arch_in_cobbler_style(node.architecture)
+    return "%s-%s" % ("precise", cobbler_arch)
 
 
 @receiver(post_save, sender=Node)

=== modified file 'src/maasserver/tests/test_provisioning.py'
--- src/maasserver/tests/test_provisioning.py	2012-03-13 11:02:14 +0000
+++ src/maasserver/tests/test_provisioning.py	2012-03-15 08:02:19 +0000
@@ -24,6 +24,7 @@
 from maasserver.provisioning import (
     compose_metadata,
     get_metadata_server_url,
+    name_arch_in_cobbler_style,
     select_profile_for_node,
     )
 from maasserver.testing.enum import map_enum
@@ -62,6 +63,21 @@
         profile_name = 'profile-%s' % shared_name
         return self.papi.add_profile(profile_name, distro)
 
+    def test_name_arch_in_cobbler_style_converts_architecture_names(self):
+        self.assertSequenceEqual(
+            ['i386', 'i386', 'x86_64', 'x86_64'],
+            map(
+                name_arch_in_cobbler_style,
+                ['i386', 'i686', 'amd64', 'x86_64']))
+
+    def test_name_arch_in_cobbler_works_for_both_bytes_and_unicode(self):
+        self.assertEqual(
+            name_arch_in_cobbler_style(u'amd64'),
+            name_arch_in_cobbler_style(b'amd64'))
+
+    def test_name_arch_in_cobbler_returns_unicode(self):
+        self.assertIsInstance(name_arch_in_cobbler_style(b'amd64'), unicode)
+
     def test_select_profile_for_node_ignores_previously_chosen_profile(self):
         node = factory.make_node(architecture='i386')
         self.papi.modify_nodes(
@@ -73,12 +89,19 @@
         nodes = {
             arch: self.make_node_without_saving(arch=arch)
             for arch in map_enum(ARCHITECTURE).values()}
-        self.assertItemsEqual(
-            ['precise-%s' % arch for arch in nodes.keys()],
+        self.assertItemsEqual([
+                'precise-%s' % name_arch_in_cobbler_style(arch)
+                for arch in nodes.keys()],
             [
                 select_profile_for_node(node, self.papi)
                 for node in nodes.values()])
 
+    def test_select_profile_for_node_converts_architecture_name(self):
+        node = factory.make_node(architecture='amd64')
+        profile = select_profile_for_node(node, self.papi)
+        self.assertNotIn('amd64', profile)
+        self.assertIn('x86_64', profile)
+
     def test_provision_post_save_Node_create(self):
         # The handler for Node's post-save signal registers the node in
         # its current state with the provisioning server.