From c9ac1e44791f8f670672e0970df1e5c9dae1c334 Mon Sep 17 00:00:00 2001 From: Mihai Ibanescu Date: Thu, 4 May 2006 15:44:34 -0400 Subject: Fixed so it doesn't fail on 64-bit platforms --- cobbler/IPy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) -- cgit From 13f1e9b0f62cedc8aed257ec2faaafac3544e428 Mon Sep 17 00:00:00 2001 From: Mihai Ibanescu Date: Thu, 4 May 2006 15:57:46 -0400 Subject: Reducing duplication in the constructors We define a top-level constructor in the base class and define an _item_factory that is the class to be instantiated for each item. The code as it is checked in now is broken because we reference the class before we define it in the global scope; but wanted the checkins to be separate, the next one will move all the Collection-related stuff after the Item-related stuff (but the diff would have been difficult to read) --- cobbler/api.py | 42 +++++++++++++++--------------------------- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/cobbler/api.py b/cobbler/api.py index f2a18f8..e928643 100644 --- a/cobbler/api.py +++ b/cobbler/api.py @@ -131,7 +131,19 @@ class BootAPI: 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): """ @@ -172,7 +184,6 @@ class Collection: 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) @@ -208,18 +219,7 @@ 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)) + _item_factory = Distro def remove(self,name): """ @@ -246,13 +246,7 @@ 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)) + _item_factory = Profile def remove(self,name): """ @@ -276,13 +270,7 @@ 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)) + _item_factory = System def remove(self,name): """ -- cgit From 1b28e7fcdb8b39acf6a358c163fe018561fb35f8 Mon Sep 17 00:00:00 2001 From: Mihai Ibanescu Date: Thu, 4 May 2006 16:01:15 -0400 Subject: Moving code around Just to define Collections after Items. --- cobbler/api.py | 316 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 158 insertions(+), 158 deletions(-) diff --git a/cobbler/api.py b/cobbler/api.py index e928643..cef4e6b 100644 --- a/cobbler/api.py +++ b/cobbler/api.py @@ -125,164 +125,6 @@ class BootAPI: """ self.config.deserialize() -#-------------------------------------- - -""" -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 - - #----------------------------------------- """ @@ -626,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 + + -- cgit From 06def04ab9f88ac89f85a4f1401ff85de8a606b1 Mon Sep 17 00:00:00 2001 From: Mihai Ibanescu Date: Thu, 4 May 2006 16:05:36 -0400 Subject: Fixing regression in common constructor When transcribing the constructors, moved one bracket too far to the right. --- cobbler/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cobbler/api.py b/cobbler/api.py index cef4e6b..77dd835 100644 --- a/cobbler/api.py +++ b/cobbler/api.py @@ -486,7 +486,7 @@ class Collection: self.listing = {} if seed_data is not None: for x in seed_data: - self.add(self._item_factory(self.api), x) + self.add(self._item_factory(self.api, x)) def find(self,name): """ -- cgit From bd4e362ebf0f5759263ce86531fbb01dd2fec1e4 Mon Sep 17 00:00:00 2001 From: Mihai Ibanescu Date: Thu, 4 May 2006 17:12:08 -0400 Subject: s/cobber/cobbler/ --- cobbler/msg.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) 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 """ @@ -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): -- cgit From 48cf03916eb198e1b5a11c3e110c4d32787789fc Mon Sep 17 00:00:00 2001 From: Mihai Ibanescu Date: Thu, 4 May 2006 18:55:31 -0400 Subject: Comparison function is simpler, and no need to sort when we need max reduce is a nice way to extract the max of a sequence. --- cobbler/util.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) 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[1]bv[1]: 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): -- cgit From 17dc6975f607468c4a0c6b428a5cfaeeaf58fb9e Mon Sep 17 00:00:00 2001 From: Mihai Ibanescu Date: Thu, 4 May 2006 19:20:12 -0400 Subject: Test case runs from a clean dir now We create temp dirs and we remove them when we're done. --- tests/tests.py | 53 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 18 deletions(-) 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")) -- cgit