Merging with mdehaan's changes

5 files changed, 218 insertions, 213 deletions

diff --git a/cobbler/IPy.py b/cobbler/IPy.py
index 6f6af41..f0d2bcb 100644
--- a/cobbler/IPy.py
+++ b/cobbler/IPy.py
@@ -777,7 +777,7 @@ class IPint:
         thehash = int(-1)
         ip = self.ip
         while ip > 0:
-            thehash = thehash ^ (ip & 0x7fffffff)
+            thehash = (thehash & 0xffffffff) ^ (ip & 0x7fffffff)
             ip = ip >> 32
         thehash = thehash ^ self._prefixlen
         return int(thehash)
diff --git a/cobbler/api.py b/cobbler/api.py
index f2a18f8..77dd835 100644
--- a/cobbler/api.py
+++ b/cobbler/api.py
@@ -125,176 +125,6 @@ class BootAPI:
        """
        self.config.deserialize()
 
-#--------------------------------------
-
-"""
-Base class for any serializable lists of things...
-"""
-class Collection:
-
-
-    def find(self,name):
-        """
-        Return anything named 'name' in the collection, else return None if
-        no objects can be found.
-        """
-        if name in self.listing.keys():
-            return self.listing[name]
-        return None
-
-
-    def to_datastruct(self):
-        """
-        Return datastructure representation of this collection suitable
-        for feeding to a serializer (such as YAML)
-        """
-        return [x.to_datastruct() for x in self.listing.values()]
-
-
-    def add(self,ref):
-        """
-        Add an object to the collection, if it's valid.  Returns True
-        if the object was added to the collection.  Returns False if the
-        object specified by ref deems itself invalid (and therefore
-        won't be added to the collection).
-        """
-        if ref is None or not ref.is_valid():
-            if self.api.last_error is None or self.api.last_error == "":
-                self.api.last_error = m("bad_param")
-            return False
-        self.listing[ref.name] = ref
-        return True
-
-
-    def printable(self):
-        """
-        Creates a printable representation of the collection suitable
-        for reading by humans or parsing from scripts.  Actually scripts
-        would be better off reading the YAML in the config files directly.
-        """
-        buf = ""
-        values = map(lambda(a): a.printable(), sorted(self.listing.values()))
-        if len(values) > 0:
-           return "\n\n".join(values)
-        else:
-           return m("empty_list")
-
-    #def contents(self):
-    #    """
-    #	Access the raw contents of the collection.  Classes shouldn't
-    #	be doing this (preferably) and should use the __iter__ interface.
-    #    Deprecrated.
-    #	 """
-    #    return self.listing.values()
-
-    def __iter__(self):
-        """
-	Iterator for the collection.  Allows list comprehensions, etc
-	"""
-        for a in self.listing.values():
-	    yield a
-
-    def __len__(self):
-        """
-	Returns size of the collection
-	"""
-        return len(self.listing.values())
-
-
-#--------------------------------------------
-
-"""
-A distro represents a network bootable matched set of kernels
-and initrd files
-"""
-class Distros(Collection):
-
-    def __init__(self,api,seed_data):
-        """
-	Constructor.  Requires an API reference.  seed_data
-	is a hash of data to feed into the collection, that would
-	come from the config file in /var.
-	"""
-        self.api = api
-        self.listing = {}
-        if seed_data is not None:
-           for x in seed_data:
-               self.add(Distro(self.api,x))
-
-    def remove(self,name):
-        """
-        Remove element named 'name' from the collection
-        """
-        # first see if any Groups use this distro
-        for k,v in self.api.get_profiles().listing.items():
-            if v.distro == name:
-               self.api.last_error = m("orphan_profiles")
-               return False
-        if self.find(name):
-            del self.listing[name]
-            return True
-        self.api.last_error = m("delete_nothing")
-        return False
-
-
-#--------------------------------------------
-
-"""
-A profile represents a distro paired with a kickstart file.
-For instance, FC5 with a kickstart file specifying OpenOffice
-might represent a 'desktop' profile.  For Xen, there are many
-additional options, with client-side defaults (not kept here).
-"""
-class Profiles(Collection):
-
-    def __init__(self,api,seed_data):
-        self.api = api
-        self.listing = {}
-        if seed_data is not None:
-           for x in seed_data:
-               self.add(Profile(self.api,x))
-
-    def remove(self,name):
-        """
-        Remove element named 'name' from the collection
-        """
-        for k,v in self.api.get_systems().listing.items():
-           if v.profile == name:
-               self.api.last_error = m("orphan_system")
-               return False
-        if self.find(name):
-            del self.listing[name]
-            return True
-        self.api.last_error = m("delete_nothing")
-        return False
-
-
-#--------------------------------------------
-
-"""
-Systems are hostnames/MACs/IP names and the associated profile
-they belong to.
-"""
-class Systems(Collection):
-
-    def __init__(self,api,seed_data):
-        self.api = api
-        self.listing = {}
-        if seed_data is not None:
-           for x in seed_data:
-               self.add(System(self.api,x))
-
-    def remove(self,name):
-        """
-        Remove element named 'name' from the collection
-        """
-        if self.find(name):
-            del self.listing[name]
-            return True
-        self.api.last_error = m("delete_nothing")
-        return False
-
-
 #-----------------------------------------
 
 """
@@ -638,3 +468,161 @@ class System(Item):
         buf = buf + "kernel opts  : %s" % self.kernel_options
         return buf
 
+#--------------------------------------
+
+"""
+Base class for any serializable lists of things...
+"""
+class Collection:
+    _item_factory = None
+
+    def __init__(self, api, seed_data):
+        """
+	Constructor.  Requires an API reference.  seed_data
+	is a hash of data to feed into the collection, that would
+	come from the config file in /var.
+	"""
+        self.api = api
+        self.listing = {}
+        if seed_data is not None:
+           for x in seed_data:
+               self.add(self._item_factory(self.api, x))
+
+    def find(self,name):
+        """
+        Return anything named 'name' in the collection, else return None if
+        no objects can be found.
+        """
+        if name in self.listing.keys():
+            return self.listing[name]
+        return None
+
+
+    def to_datastruct(self):
+        """
+        Return datastructure representation of this collection suitable
+        for feeding to a serializer (such as YAML)
+        """
+        return [x.to_datastruct() for x in self.listing.values()]
+
+
+    def add(self,ref):
+        """
+        Add an object to the collection, if it's valid.  Returns True
+        if the object was added to the collection.  Returns False if the
+        object specified by ref deems itself invalid (and therefore
+        won't be added to the collection).
+        """
+        if ref is None or not ref.is_valid():
+            if self.api.last_error is None or self.api.last_error == "":
+                self.api.last_error = m("bad_param")
+            return False
+        self.listing[ref.name] = ref
+        return True
+
+
+    def printable(self):
+        """
+        Creates a printable representation of the collection suitable
+        for reading by humans or parsing from scripts.  Actually scripts
+        would be better off reading the YAML in the config files directly.
+        """
+        values = map(lambda(a): a.printable(), sorted(self.listing.values()))
+        if len(values) > 0:
+           return "\n\n".join(values)
+        else:
+           return m("empty_list")
+
+    #def contents(self):
+    #    """
+    #	Access the raw contents of the collection.  Classes shouldn't
+    #	be doing this (preferably) and should use the __iter__ interface.
+    #    Deprecrated.
+    #	 """
+    #    return self.listing.values()
+
+    def __iter__(self):
+        """
+	Iterator for the collection.  Allows list comprehensions, etc
+	"""
+        for a in self.listing.values():
+	    yield a
+
+    def __len__(self):
+        """
+	Returns size of the collection
+	"""
+        return len(self.listing.values())
+
+
+#--------------------------------------------
+
+"""
+A distro represents a network bootable matched set of kernels
+and initrd files
+"""
+class Distros(Collection):
+    _item_factory = Distro
+
+    def remove(self,name):
+        """
+        Remove element named 'name' from the collection
+        """
+        # first see if any Groups use this distro
+        for k,v in self.api.get_profiles().listing.items():
+            if v.distro == name:
+               self.api.last_error = m("orphan_profiles")
+               return False
+        if self.find(name):
+            del self.listing[name]
+            return True
+        self.api.last_error = m("delete_nothing")
+        return False
+
+
+#--------------------------------------------
+
+"""
+A profile represents a distro paired with a kickstart file.
+For instance, FC5 with a kickstart file specifying OpenOffice
+might represent a 'desktop' profile.  For Xen, there are many
+additional options, with client-side defaults (not kept here).
+"""
+class Profiles(Collection):
+    _item_factory = Profile
+
+    def remove(self,name):
+        """
+        Remove element named 'name' from the collection
+        """
+        for k,v in self.api.get_systems().listing.items():
+           if v.profile == name:
+               self.api.last_error = m("orphan_system")
+               return False
+        if self.find(name):
+            del self.listing[name]
+            return True
+        self.api.last_error = m("delete_nothing")
+        return False
+
+
+#--------------------------------------------
+
+"""
+Systems are hostnames/MACs/IP names and the associated profile
+they belong to.
+"""
+class Systems(Collection):
+    _item_factory = System
+
+    def remove(self,name):
+        """
+        Remove element named 'name' from the collection
+        """
+        if self.find(name):
+            del self.listing[name]
+            return True
+        self.api.last_error = m("delete_nothing")
+        return False
+
+
diff --git a/cobbler/msg.py b/cobbler/msg.py
index d770fd2..f50443c 100644
--- a/cobbler/msg.py
+++ b/cobbler/msg.py
@@ -1,4 +1,4 @@
-# Messages used by cobber.
+# Messages used by cobbler.
 # Michael DeHaan <mdehaan@redhat.com>
 
 """
@@ -8,19 +8,19 @@ be reused and potentially translated.
 
 msg_table = {
   "bad_server"      : "server field in /etc/cobbler.conf must be set to something other than localhost, or kickstarts will fail",
-  "parse_error"     : "could not parse /etc/cobber.conf",
+  "parse_error"     : "could not parse /etc/cobbler.conf",
   "parse_error2"    : "could not parse /var/cobbler/cobbler.conf",
   "no_create"       : "cannot create: %s",
   "no_args"         : "this command requires arguments.",
   "missing_options" : "cannot add, all parameters have not been set",
-  "unknown_cmd"     : "cobber doesn't understand '%s'",
+  "unknown_cmd"     : "cobbler doesn't understand '%s'",
   "bad_arg"         : "expecting an equal sign in argument '%s'",
   "reject_arg"      : "the value of parameter '%s' is not valid",
   "weird_arg"       : "this command doesn't take a parameter named '%s'",
   "bad_sys_name"    : "system name must be a MAC, IP, or resolveable host",
   "usage"           : "for help, see 'man cobbler'",
   "need_to_fix"     : "the following potential problems were detected:",
-  "need_root"       : "cobber must be run as root",
+  "need_root"       : "cobbler must be run as root",
   "no_dhcpd"        : "can't find dhcpd, try 'yum install dhcpd'",
   "no_pxelinux"     : "can't find pxelinux, try 'yum install pxelinux'",
   "no_tftpd"        : "can't find tftpd, try 'yum install tftpd'",
@@ -28,8 +28,8 @@ msg_table = {
   "chg_attrib"      : "need to change '%s' to '%s' in '%s'",
   "no_exist"        : "%s does not exist",
   "no_line"         : "file '%s' should have a line '%s' somewhere",
-  "no_dir2"         : "can't find %s for %s in cobber.conf",
-  "no_cfg"          : "could not find cobber.conf, recreating",
+  "no_dir2"         : "can't find %s for %s in cobbler.conf",
+  "no_cfg"          : "could not find cobbler.conf, recreating",
   "bad_param"       : "at least one parameter is missing for this function",
   "empty_list"      : "(Empty)",
   "err_resolv"      : "system (%s) did not resolve",
@@ -48,11 +48,11 @@ msg_table = {
   "check_ok"        : """
 No setup problems found.
 
-Manual editing of /etc/dhcpd.conf and /etc/cobber.conf is suggested to tailor them to your specific configuration.  Your dhcpd.conf has some PXE related information in it, but it's imposible to tell automatically that it's totally correct in a general sense.  We'll leave this up to you.
+Manual editing of /etc/dhcpd.conf and /etc/cobbler.conf is suggested to tailor them to your specific configuration.  Your dhcpd.conf has some PXE related information in it, but it's imposible to tell automatically that it's totally correct in a general sense.  We'll leave this up to you.
 
 Good luck.
 """,
-  "help"           : "see 'man cobber'"
+  "help"           : "see 'man cobbler'"
 }
 
 def m(key):
diff --git a/cobbler/util.py b/cobbler/util.py
index cdf7bbe..ae7dde7 100644
--- a/cobbler/util.py
+++ b/cobbler/util.py
@@ -83,25 +83,25 @@ class BootUtil:
        """
        files = self.find_matching_files(directory, regex)
        get_numbers = re.compile(r'(\d+).(\d+).(\d+)')
-       def sort(a,b):
+       def max2(a, b):
+           """Returns the larger of the two values"""
            av  = get_numbers.search(os.path.basename(a)).groups()
            bv  = get_numbers.search(os.path.basename(b)).groups()
-           if av[0]<bv[0]: return -1
-           elif av[0]>bv[0]: return 1
-           elif av[1]<bv[1]: return -1
-           elif av[1]>bv[1]: return 1
-           elif av[2]<bv[2]: return -1
-           elif av[2]>bv[2]: return 1
-           return 0
+
+           ret = cmp(av[0], bv[0]) or cmp(av[1], bv[1]) or cmp(av[2], bv[2])
+           if ret < 0:
+                return b
+           return a
+
        if len(files) > 0:
-           return sorted(files, sort)[-1]
-       else:
-           # couldn't find a highest numbered file, but maybe there
-           # is just a 'vmlinuz' or an 'initrd.img' in this directory?
-           last_chance = os.path.join(directory,unversioned)
-           if os.path.exists(last_chance):
-               return last_chance
-           return None
+           return reduce(max2, files)
+
+       # couldn't find a highest numbered file, but maybe there
+       # is just a 'vmlinuz' or an 'initrd.img' in this directory?
+       last_chance = os.path.join(directory,unversioned)
+       if os.path.exists(last_chance):
+           return last_chance
+       return None
 
 
    def find_kernel(self,path):
diff --git a/tests/tests.py b/tests/tests.py
index 440c588..2673cdd 100644
--- a/tests/tests.py
+++ b/tests/tests.py
@@ -7,6 +7,8 @@ import sys
 import unittest
 import os
 import subprocess
+import tempfile
+import shutil
 
 sys.path.append('../cobbler')
 sys.path.append('./cobbler')
@@ -14,16 +16,27 @@ sys.path.append('./cobbler')
 import api
 import config
 
-FAKE_INITRD="/tmp/initrd-2.6.15-1.2054_FAKE.img"
-FAKE_INITRD2="/tmp/initrd-2.5.16-2.2055_FAKE.img"
-FAKE_INITRD3="/tmp/initrd-1.8.18-3.9999_FAKE.img"
-FAKE_KERNEL="/tmp/vmlinuz-2.6.15-1.2054_FAKE"
-FAKE_KERNEL2="/tmp/vmlinuz-2.5.16-2.2055_FAKE"
-FAKE_KERNEL3="/tmp/vmlinuz-1.8.18-3.9999_FAKE"
+FAKE_INITRD="initrd-2.6.15-1.2054_FAKE.img"
+FAKE_INITRD2="initrd-2.5.16-2.2055_FAKE.img"
+FAKE_INITRD3="initrd-1.8.18-3.9999_FAKE.img"
+FAKE_KERNEL="vmlinuz-2.6.15-1.2054_FAKE"
+FAKE_KERNEL2="vmlinuz-2.5.16-2.2055_FAKE"
+FAKE_KERNEL3="vmlinuz-1.8.18-3.9999_FAKE"
 FAKE_KICKSTART="http://127.0.0.1/fake.ks"
 
 class BootTest(unittest.TestCase):
+    
     def setUp(self):
+        # Create temp dir
+        self.topdir = tempfile.mkdtemp(prefix="_cobbler-")
+        self.fk_initrd = os.path.join(self.topdir, FAKE_INITRD)
+        self.fk_initrd2 = os.path.join(self.topdir, FAKE_INITRD2)
+        self.fk_initrd3 = os.path.join(self.topdir, FAKE_INITRD3)
+
+        self.fk_kernel = os.path.join(self.topdir, FAKE_KERNEL)
+        self.fk_kernel2 = os.path.join(self.topdir, FAKE_KERNEL2)
+        self.fk_kernel3 = os.path.join(self.topdir, FAKE_KERNEL3)
+        
         try:
            # it will interfere with results...
            os.remove("/etc/cobbler.conf")
@@ -31,20 +44,21 @@ class BootTest(unittest.TestCase):
            pass
         self.api = api.BootAPI()
         self.hostname = os.uname()[1]
-        create =  [FAKE_INITRD,FAKE_INITRD2,FAKE_INITRD3,
-                   FAKE_KERNEL,FAKE_KERNEL2,FAKE_KERNEL3]
+        create = [ self.fk_initrd, self.fk_initrd2, self.fk_initrd3,
+                self.fk_kernel, self.fk_kernel2, self.fk_kernel3 ]
         for fn in create:
             f = open(fn,"w+")
         self.make_basic_config()
 
     def tearDown(self):
+        shutil.rmtree(self.topdir, ignore_errors=1)
         self.api = None
 
     def make_basic_config(self):
         distro = self.api.new_distro()
         self.assertTrue(distro.set_name("testdistro0"))
-        self.assertTrue(distro.set_kernel(FAKE_KERNEL))
-        self.assertTrue(distro.set_initrd(FAKE_INITRD))
+        self.assertTrue(distro.set_kernel(self.fk_kernel))
+        self.assertTrue(distro.set_initrd(self.fk_initrd))
         self.assertTrue(self.api.get_distros().add(distro))
         self.assertTrue(self.api.get_distros().find("testdistro0"))
 
@@ -63,25 +77,28 @@ class BootTest(unittest.TestCase):
 
 class Utilities(BootTest):
 
+    def _expeq(self, expected, actual):
+        self.failUnlessEqual(expected, actual, 
+            "Expected: %s; actual: %s" % (expected, actual))
 
     def test_kernel_scan(self):
-        self.assertTrue(self.api.utils.find_kernel(FAKE_KERNEL))
+        self.assertTrue(self.api.utils.find_kernel(self.fk_kernel))
         self.assertFalse(self.api.utils.find_kernel("/etc/fstab"))
         self.assertFalse(self.api.utils.find_kernel("filedoesnotexist"))
-        self.assertTrue(self.api.utils.find_kernel("/tmp") == FAKE_KERNEL)
+        self._expeq(self.fk_kernel, self.api.utils.find_kernel(self.topdir))
 
     def test_initrd_scan(self):
-        self.assertTrue(self.api.utils.find_initrd(FAKE_INITRD))
+        self.assertTrue(self.api.utils.find_initrd(self.fk_initrd))
         self.assertFalse(self.api.utils.find_kernel("/etc/fstab"))
         self.assertFalse(self.api.utils.find_initrd("filedoesnotexist"))
-        self.assertTrue(self.api.utils.find_initrd("/tmp") == FAKE_INITRD)
+        self._expeq(self.fk_initrd, self.api.utils.find_initrd(self.topdir))
 
     def test_kickstart_scan(self):
         # we don't check to see if kickstart files look like anything
         # so this will pass
-        self.assertTrue(self.api.utils.find_kickstart(FAKE_INITRD) is None)
+        self.assertTrue(self.api.utils.find_kickstart(self.fk_initrd) is None)
         self.assertTrue(self.api.utils.find_kickstart("filedoesnotexist") is None)
-        self.assertTrue(self.api.utils.find_kickstart("/tmp") == None)
+        self.assertTrue(self.api.utils.find_kickstart(self.topdir) == None)
         self.assertTrue(self.api.utils.find_kickstart("http://bar"))
         self.assertTrue(self.api.utils.find_kickstart("ftp://bar"))
         self.assertTrue(self.api.utils.find_kickstart("nfs://bar"))
@@ -106,14 +123,14 @@ class Additions(BootTest):
         distro = self.api.new_distro()
         self.assertTrue(distro.set_name("testdistro2"))
         self.assertFalse(distro.set_kernel("filedoesntexist"))
-        self.assertTrue(distro.set_initrd(FAKE_INITRD))
+        self.assertTrue(distro.set_initrd(self.fk_initrd))
         self.assertFalse(self.api.get_distros().add(distro))
         self.assertFalse(self.api.get_distros().find("testdistro2"))
 
     def test_invalid_distro_non_referenced_initrd(self):
         distro = self.api.new_distro()
         self.assertTrue(distro.set_name("testdistro3"))
-        self.assertTrue(distro.set_kernel(FAKE_KERNEL))
+        self.assertTrue(distro.set_kernel(self.fk_kernel))
         self.assertFalse(distro.set_initrd("filedoesntexist"))
         self.assertFalse(self.api.get_distros().add(distro))
         self.assertFalse(self.api.get_distros().find("testdistro3"))