Merge lp:~javier.collado/utah/bug1061011 into lp:utah

[Javier Collado]

As agreed in the bugs comments, this branch adds a new exception to the preseed
module, so that the traceback when a duplicated question name is found in the
preseed file is clearer. In particular, the new traceback would be something
like this:

  File "/home/javi/code/bzr/utah/bug1061011/utah/preseed.py", line 134, in section_updated
    raise DuplicatedQuestionName(new_text)
utah.preseed.DuplicatedQuestionName: partman/confirm

In addition to this a new module with a few simple test cases have been added
to the tests/ directory (they can be executed with nosetests tests/). I'm not
sure if that's the best location for server test cases, but I haven't found any
other test cases and this one looks fine to me since the files under tests/
aren't included in the binary package. Anyway, under tests/ there's already a
script that is used to run the client test cases, but I guess we can have both
things together.

In the future, we could also have a jenkins job to run the test cases for the
server as there is already one to run the test cases for the client.

[Max Brustkern (nuclearbob)]

I like that new exception. If you've already tested this, we should be good to go ahead and merge, otherwise, I'll try to test it.

review: Approve

[Javier Collado (javier.collado)]

@Max

Yes, I already tested it, so I'm merging it.

=== added file 'tests/test_preseed.py'
--- tests/test_preseed.py	1970-01-01 00:00:00 +0000
+++ tests/test_preseed.py	2012-10-08 13:46:36 +0000
@@ -0,0 +1,46 @@
+from utah.preseed import (
+        Preseed,
+        Section,
+        BlankSection,
+        CommentSection,
+        ConfigurationSection,
+        DuplicatedQuestionName,
+        )
+
+import unittest
+
+
+class TestPreseedLoadDump(unittest.TestCase):
+    """
+    Minimal load/dump test cases
+    """
+    BASIC_PRESEED = """# Comment
+
+d-i passwd/username string utah
+"""
+
+    DUPLICATED_QUESTION_NAME_PRESEED = """d-i passwd/username string utah
+d-i passwd/username string utah
+"""
+
+    def test_load(self):
+        """Load basic preseed"""
+        preseed = Preseed(self.BASIC_PRESEED.splitlines())
+
+        self.assertEqual(len(preseed.sections), 3)
+        section_types = (CommentSection, BlankSection, ConfigurationSection)
+        for index, section_type in enumerate(section_types):
+            self.assertIsInstance(preseed.sections[index], section_type)
+
+    def test_dump(self):
+        """Dump basic preseed"""
+        preseed = Preseed()
+        preseed.append(Section.new(preseed, '# Comment'))
+        preseed.append(Section.new(preseed, ''))
+        preseed.append(Section.new(preseed, 'd-i passwd/username string utah'))
+        self.assertEqual(preseed.dump(), self.BASIC_PRESEED)
+
+    def test_duplicated_question_name(self):
+        """Exception raised on duplicated question name"""
+        with self.assertRaises(DuplicatedQuestionName):
+            Preseed(self.DUPLICATED_QUESTION_NAME_PRESEED.splitlines())

=== modified file 'utah/preseed.py'
--- utah/preseed.py	2012-09-06 12:34:28 +0000
+++ utah/preseed.py	2012-10-08 13:46:36 +0000
@@ -8,12 +8,13 @@
     """
     Read/Write preseed files easily
     """
-    def __init__(self, filename):
+    def __init__(self, lines=None):
         # Used to access quickly to configuration sections by question name
         self._qnames = {}
-
-        self.filename = filename
-        self.read(filename)
+        if lines is not None:
+            self.load(lines)
+        else:
+            self.sections = []
 
     def __getitem__(self, key):
         """
@@ -37,42 +38,38 @@
 
         return key in self._qnames
 
-    def read(self, filename=None):
-        """
-        Read a whole preseed file and parse its configuration lines
-        """
-        if filename is not None:
-            self.filename = filename
-        else:
-            filename = self.filename
+    def load(self, lines):
+        """
+        Parse preseed configuration lines
 
+        :param lines: Any iterable that yields preseed file configuration lines
+        :type lines: iterable
+        """
         self.sections = []
-        with open(filename) as f:
-            # One line might be made of multiple lines
-            # that are continued with '\\'
-            output_lines = []
-            for input_line in f:
-                input_line = input_line.rstrip('\n')
-                output_lines.append(input_line)
-
-                # Line is finished only when no continuation character is found
-                if not input_line.endswith('\\'):
-                    new_section = Section.new(self, output_lines)
-                    self.append(new_section)
-                    output_lines = []
-
-    def write(self, filename=None):
+        # One line might be made of multiple lines
+        # that are continued with '\\'
+        output_lines = []
+        for input_line in lines:
+            input_line = input_line.rstrip('\n')
+            output_lines.append(input_line)
+
+            # Line is finished only when no continuation character is found
+            if not input_line.endswith('\\'):
+                new_section = Section.new(self, output_lines)
+                self.append(new_section)
+                output_lines = []
+
+        return self
+
+    def dump(self):
         """
+        Dump preseed configuration statements
         Write the modified file to the same or a new location
+
+        :returns: Formatted preseed configuration lines
+        :rtype: string
         """
-        if filename is not None:
-            self.filename = filename
-        else:
-            filename = self.filename
-
-        with open(filename, 'w') as f:
-            for section in self.sections:
-                f.write('{}'.format(section))
+        return ''.join(str(section) for section in self.sections)
 
     def append(self, new_section, ref_section=None):
         """
@@ -133,7 +130,8 @@
         """
         if property_name == 'qname':
             new_text = new_value.text
-            assert new_text not in self._qnames
+            if new_text in self._qnames:
+                raise DuplicatedQuestionName(new_text)
 
             if old_value is not None:
                 old_text = old_value.text
@@ -350,3 +348,9 @@
 
     def property_updated(self, property_name, old_value, new_value):
         self.parent.section_updated(self, property_name, old_value, new_value)
+
+
+class DuplicatedQuestionName(Exception):
+    """
+    Exception raised when a question name is found more than once in a preseed
+    """

=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2012-10-04 11:49:14 +0000
+++ utah/provisioning/provisioning.py	2012-10-08 13:46:36 +0000
@@ -787,7 +787,8 @@
         self.logger.info('Setting up preseed')
         if tmpdir is None:
             tmpdir = self.tmpdir
-        preseed = Preseed(self.preseed)
+        with open(self.preseed) as f:
+            preseed = Preseed(f)
         if 'preseed/late_command' in preseed:
             question = preseed['preseed/late_command']
             if self.installtype == 'desktop':
@@ -833,7 +834,10 @@
             question = preseed['passwd/username']
             question.value = config.user
 
-        preseed.write(os.path.join(tmpdir, 'initrd.d', 'preseed.cfg'))
+        output_preseed_filename = os.path.join(tmpdir,
+                                               'initrd.d', 'preseed.cfg')
+        with open(output_preseed_filename, 'w') as f:
+            f.write(preseed.dump())
 
         if self.installtype == 'desktop':
             self.logger.info('Inserting preseed into casper')