[et-mgmt-tools] [PATCH] Changed collection.find() to take a dictionary argument instead of name, allowing searches by ip_address, mac_address, etc.
Michael DeHaan
mdehaan at redhat.com
Wed Aug 15 20:51:42 UTC 2007
Ben Riggs wrote:
> From: root <root at xt23.math.umn.edu>
>
> diff --git a/cobbler/action_enchant.py b/cobbler/action_enchant.py
> index 3bf80f8..4a54ebe 100644
> --- a/cobbler/action_enchant.py
> +++ b/cobbler/action_enchant.py
> @@ -40,9 +40,9 @@ class Enchant:
> raise CX(_("enchant failed. no address specified"))
> if system is None and profile is None:
> raise CX(_("enchant failed. no profile specified"))
> - if system is not None and self.config.systems().find(system) is None:
> + if system is not None and self.config.systems().find(name=system) is None:
> raise CX(_("enchant failed. system not found"))
> - if profile is not None and self.config.profiles().find(profile) is None:
> + if profile is not None and self.config.profiles().find(name=profile) is None:
> raise CX(_("enchant failed. profile name not found"))
>
>
> diff --git a/cobbler/action_import.py b/cobbler/action_import.py
> index 5789ae0..c74bc81 100644
> --- a/cobbler/action_import.py
> +++ b/cobbler/action_import.py
> @@ -155,7 +155,7 @@ class Importer:
> """
>
> for profile in self.profiles:
> - distro = self.distros.find(profile.distro)
> + distro = self.distros.find(name=profile.distro)
> if distro is None or not (distro in self.distros_added):
> print _("- skipping distro %s since it wasn't imported this time") % profile.distro
> continue
> @@ -429,7 +429,7 @@ class Importer:
> pxe_arch = self.get_pxe_arch(dirname)
> name = self.get_proposed_name(dirname, pxe_arch)
>
> - existing_distro = self.distros.find(name)
> + existing_distro = self.distros.find(name=name)
>
> if existing_distro is not None:
> print _("- modifying existing distro: %s") % name
> @@ -446,7 +446,7 @@ class Importer:
> self.distros.add(distro)
> self.distros_added.append(distro)
>
> - existing_profile = self.profiles.find(name)
> + existing_profile = self.profiles.find(name=name)
>
> if existing_profile is None:
> print _("- creating new profile: %s") % name
> diff --git a/cobbler/action_litesync.py b/cobbler/action_litesync.py
> index 21e6c62..2560d8d 100644
> --- a/cobbler/action_litesync.py
> +++ b/cobbler/action_litesync.py
> @@ -53,7 +53,7 @@ class BootLiteSync:
>
> def add_single_distro(self, name):
> # get the distro record
> - distro = self.distros.find(name)
> + distro = self.distros.find(name=name)
> if distro is None:
> raise CX(_("error in distro lookup: %s") % name)
> # generate YAML file in distros/$name in webdir
> @@ -73,7 +73,7 @@ class BootLiteSync:
>
> def add_single_profile(self, name):
> # get the profile object:
> - profile = self.profiles.find(name)
> + profile = self.profiles.find(name=name)
> if profile is None:
> raise CX(_("error in profile lookup"))
> # rebuild profile_list YAML file in webdir
> @@ -93,7 +93,7 @@ class BootLiteSync:
>
> def add_single_system(self, name):
> # get the system object:
> - system = self.systems.find(name)
> + system = self.systems.find(name=name)
> if system is None:
> raise CX(_("error in system lookup for %s") % name)
> # rebuild system_list file in webdir
> @@ -106,7 +106,7 @@ class BootLiteSync:
> self.sync.validate_kickstart_for_specific_system(system)
>
> def remove_single_system(self, name):
> - system_record = self.systems.find(name)
> + system_record = self.systems.find(name=name)
> # rebuild system_list file in webdir
> self.sync.write_listings()
> # delete system YAML file in systems/$name in webdir
> @@ -118,11 +118,11 @@ class BootLiteSync:
>
> # delete PXE Linux configuration file (which might be in one of two places)
> itanic = False
> - system_record = self.systems.find(name)
> - profile = self.profiles.find(system_record.profile)
> + system_record = self.systems.find(name=name)
> + profile = self.profiles.find(name=system_record.profile)
> # allow cobbler deletes to still work in the cobbler config is discombobulated
> if profile is not None:
> - distro = self.distros.find(profile.distro)
> + distro = self.distros.find(name=profile.distro)
> if distro is not None and distro in [ "ia64", "IA64"]:
> itanic = True
> if not itanic:
> diff --git a/cobbler/action_status.py b/cobbler/action_status.py
> index 158ee30..660fb42 100644
> --- a/cobbler/action_status.py
> +++ b/cobbler/action_status.py
> @@ -17,6 +17,7 @@ import os
> import os.path
> import glob
> import time
> +import api as cobbler_api
>
> from rhpl.translate import _, N_, textdomain, utf8
>
> @@ -99,6 +100,8 @@ class BootStatusReport:
> tracking will be incomplete. This should be noted in the docs.
> """
>
> + api = cobbler_api.BootAPI()
> +
> apache_results = self.scan_apache_logfiles()
> syslog_results = self.scan_syslog_logfiles()
> ips = apache_results.keys()
> @@ -112,9 +115,9 @@ class BootStatusReport:
> last_recorded_time = 0
> time_collisions = 0
>
> - #header = ("Address", "State", "Started", "Last Request", "Seconds", "Log Entries")
> + #header = ("Name", "State", "Started", "Last Request", "Seconds", "Log Entries")
> print "%-20s | %-15s | %-25s | %-25s | %-10s | %-6s" % (
> - _("Address"),
> + _("Name"),
> _("State"),
> _("Last Request"),
> _("Started"),
> @@ -150,20 +153,19 @@ class BootStatusReport:
> else:
> entries[logtime] = "1"
>
> -
> - self.generate_report(entries,ip)
> + name = api.systems().find(ip_address=ip).name
> + self.generate_report(entries,name)
>
>
> return True
>
> #-----------------------------------------
>
> - def generate_report(self,entries,ip):
> + def generate_report(self,entries,name):
> """
> Given the information about transferred files and kickstart finish times, attempt
> to produce a report that most describes the state of the system.
> """
> -
> # sort the access times
> rtimes = entries.keys()
> rtimes.sort()
> @@ -175,7 +177,7 @@ class BootStatusReport:
> fcount = 0
>
> if len(rtimes) == 0:
> - print _("%s: ?") % ip
> + print _("%s: ?") % name
> return
>
> # for each request time the machine has made
> @@ -215,7 +217,7 @@ class BootStatusReport:
>
> # print the status line for this IP address
> print "%-20s | %-15s | %-25s | %-25s | %-10s | %-6s" % (
> - ip,
> + name,
> install_state,
> display_start,
> display_last,
> diff --git a/cobbler/action_sync.py b/cobbler/action_sync.py
> index ae65ccc..1e12aa8 100644
> --- a/cobbler/action_sync.py
> +++ b/cobbler/action_sync.py
> @@ -367,7 +367,7 @@ class BootSync:
> buf = ""
> repos = profile.repos
> for r in repos:
> - repo = self.repos.find(r)
> + repo = self.repos.find(name=r)
> if repo is None:
> continue
> http_url = "http://%s/cblr/repo_mirror/%s" % (self.settings.server, repo.name)
> @@ -389,7 +389,7 @@ class BootSync:
> repos = profile.repos
> buf = ""
> for r in repos:
> - repo = self.repos.find(r)
> + repo = self.repos.find(name=r)
> if repo is None:
> continue
> repo.local_filename = repo.local_filename.replace(".repo","")
> diff --git a/cobbler/cobbler.py b/cobbler/cobbler.py
> index 13daabd..0f34eee 100755
> --- a/cobbler/cobbler.py
> +++ b/cobbler/cobbler.py
> @@ -200,7 +200,7 @@ class BootCLI:
>
> def __list_names2(self, collection, args):
> for p in args:
> - obj = collection.find(p)
> + obj = collection.find(name=p)
> if obj is not None:
> print obj.printable()
> return True
> @@ -269,7 +269,7 @@ class BootCLI:
> control_fn(args,obj)
>
> def __generic_edit(self,args,collection_fn,control_fn,exc_msg):
> - obj = collection_fn().find(self.find_arg(args,"--name"))
> + obj = collection_fn().find(name=self.find_arg(args,"--name"))
> name2 = self.find_arg(args,"--newname")
> if name2 is not None:
> raise CX("objects cannot be renamed with the edit command, use 'rename'")
> @@ -278,7 +278,7 @@ class BootCLI:
> control_fn(args,obj)
>
> def __generic_copy(self,args,collection_fn,control_fn,exc_msg):
> - obj = collection_fn().find(self.find_arg(args,"--name"))
> + obj = collection_fn().find(name=self.find_arg(args,"--name"))
> obj2 = self.find_arg(args,"--newname")
> if obj is None:
> raise CX(exc_msg)
> diff --git a/cobbler/cobblerd.py b/cobbler/cobblerd.py
> index 54294cb..d9a5ced 100644
> --- a/cobbler/cobblerd.py
> +++ b/cobbler/cobblerd.py
> @@ -71,10 +71,11 @@ def do_syslog(bootapi, settings, port, logger):
> while 1:
> data, addr = s.recvfrom(buf)
> (ip, port) = addr
> - if not data:
> + name = bootapi.systems().find(ip_address = ip)
> + if not data and name:
> break
> else:
> - logfile = open("/var/log/cobbler/syslog/%s" % ip, "a+")
> + logfile = open("/var/log/cobbler/syslog/%s" % name, "a+")
> t = time.localtime()
> # write numeric time
> seconds = str(time.mktime(t))
> @@ -118,7 +119,7 @@ class CobblerXMLRPCInterface:
> # feature disabled!
> return False
> systems = self.api.systems()
> - obj = systems.find(name)
> + obj = systems.find(name=name)
> if obj == None:
> # system not found!
> return False
> @@ -148,7 +149,7 @@ class CobblerXMLRPCInterface:
> def __get_specific(self,collection,name):
> self.api.clear()
> self.api.deserialize()
> - item = collection.find(name)
> + item = collection.find(name=name)
> if item is None:
> return self.fix_none({})
> return self.fix_none(item.to_datastruct())
> @@ -169,7 +170,7 @@ class CobblerXMLRPCInterface:
> def get_distro_for_koan(self,name):
> self.api.clear()
> self.api.deserialize()
> - obj = cobbler_api.distros().find(name)
> + obj = cobbler_api.distros().find(name=name)
> if obj is not None:
> return self.fix_none(utils.blender(True, obj))
> return self.fix_none({})
> @@ -177,7 +178,7 @@ class CobblerXMLRPCInterface:
> def get_profile_for_koan(self,name):
> self.api.clear()
> self.api.deserialize()
> - obj = self.api.profiles().find(name)
> + obj = self.api.profiles().find(name=name)
> if obj is not None:
> return self.fix_none(utils.blender(True, obj))
> return self.fix_none({})
> @@ -185,7 +186,7 @@ class CobblerXMLRPCInterface:
> def get_system_for_koan(self,name):
> self.api.clear()
> self.api.deserialize()
> - obj = self.api.systems().find(name)
> + obj = self.api.systems().find(name=name)
> if obj is not None:
> return self.fix_none(utils.blender(True, obj))
> return self.fix_none({})
> @@ -193,7 +194,7 @@ class CobblerXMLRPCInterface:
> def get_repo_for_koan(self,name):
> self.api.clear()
> self.api.deserialize()
> - obj = self.api.repos().find(name)
> + obj = self.api.repos().find(name=name)
> if obj is not None:
> return self.fix_none(utils.blender(True, obj))
> return self.fix_none({})
> diff --git a/cobbler/collection.py b/cobbler/collection.py
> index 0c743f4..b2e3628 100644
> --- a/cobbler/collection.py
> +++ b/cobbler/collection.py
> @@ -30,8 +30,8 @@ class Collection(serializable.Serializable):
>
> def __init__(self,config):
> """
> - Constructor.
> - """
> + Constructor.
> + """
> self.config = config
> self.clear()
>
> @@ -54,17 +54,17 @@ class Collection(serializable.Serializable):
> """
> self.listing = {}
>
> - def find(self,name):
> + def find(self, **kargs):
> """
> - Return anything named 'name' in the collection, else return None if
> - no objects can be found.
> + Return first object in the collection that maches all item='value'
> + pairs passed, else return None if no objects can be found.
> """
> - n1 = name.lower()
> - listing = self.listing
> - if listing.has_key(n1):
> - return self.listing[n1]
> - else:
> - return None
> + kargs = dict((k.lower(), v.lower()) for k, v in kargs.items())
> + for entry in self.to_datastruct():
> + match = dict((k.lower(), v.lower()) for k, v in entry.items() if kargs.has_key(k))
> + if match == kargs:
> + return self.listing[entry['name'].lower()]
> + return None
>
> def to_datastruct(self):
> """
> diff --git a/cobbler/collection_distros.py b/cobbler/collection_distros.py
> index 70029eb..9ab9e3c 100644
> --- a/cobbler/collection_distros.py
> +++ b/cobbler/collection_distros.py
> @@ -51,7 +51,7 @@ class Distros(collection.Collection):
> for v in self.config.profiles():
> if v.distro == name:
> raise CX(_("removal would orphan profile: %s") % v.name)
> - if self.find(name):
> + if self.find(name=name):
> if with_delete:
> self._run_triggers(self.listing[name], "/var/lib/cobbler/triggers/delete/distro/pre/*")
> lite_sync = action_litesync.BootLiteSync(self.config)
> diff --git a/cobbler/collection_profiles.py b/cobbler/collection_profiles.py
> index ac1798e..7331b08 100644
> --- a/cobbler/collection_profiles.py
> +++ b/cobbler/collection_profiles.py
> @@ -49,7 +49,7 @@ class Profiles(collection.Collection):
> for v in self.config.systems():
> if v.profile == name:
> raise CX(_("removal would orphan system: %s") % v.name)
> - if self.find(name):
> + if self.find(name=name):
> if with_delete:
> self._run_triggers(self.listing[name], "/var/lib/cobbler/triggers/delete/profile/pre/*")
> lite_sync = action_litesync.BootLiteSync(self.config)
> diff --git a/cobbler/collection_repos.py b/cobbler/collection_repos.py
> index 8ffa1bf..ed62d88 100644
> --- a/cobbler/collection_repos.py
> +++ b/cobbler/collection_repos.py
> @@ -53,7 +53,7 @@ class Repos(collection.Collection):
> # NOTE: with_delete isn't currently meaningful for repos
> # but is left in for consistancy in the API. Unused.
> name = name.lower()
> - if self.find(name):
> + if self.find(name=name):
> if with_delete:
> self._run_triggers(self.listing[name], "/var/lib/cobbler/triggers/delete/repo/pre/*")
> self._run_triggers(self.listing[name], "/var/lib/cobbler/triggers/delete/repo/post/*")
> diff --git a/cobbler/collection_systems.py b/cobbler/collection_systems.py
> index 5ffcea5..58f9e13 100644
> --- a/cobbler/collection_systems.py
> +++ b/cobbler/collection_systems.py
> @@ -49,7 +49,7 @@ class Systems(collection.Collection):
> Remove element named 'name' from the collection
> """
> name = name.lower()
> - if self.find(name):
> + if self.find(name=name):
> if with_delete:
> self._run_triggers(self.listing[name], "/var/lib/cobbler/triggers/delete/system/pre/*")
> lite_sync = action_litesync.BootLiteSync(self.config)
> diff --git a/cobbler/item_distro.py b/cobbler/item_distro.py
> index 4bbf32f..4a97fcf 100644
> --- a/cobbler/item_distro.py
> +++ b/cobbler/item_distro.py
> @@ -55,7 +55,7 @@ class Distro(item.Item):
> if self.parent is None or self.parent == '':
> return None
> else:
> - return self.config.distros().find(self.parent)
> + return self.config.distros().find(name=self.parent)
>
> def from_datastruct(self,seed_data):
> """
> diff --git a/cobbler/item_profile.py b/cobbler/item_profile.py
> index c7c9814..4062826 100644
> --- a/cobbler/item_profile.py
> +++ b/cobbler/item_profile.py
> @@ -91,7 +91,7 @@ class Profile(item.Item):
> # check must be done in two places as set_parent could be called before/after
> # set_name...
> raise CX(_("self parentage is weird"))
> - found = self.config.profiles().find(parent_name)
> + found = self.config.profiles().find(name=parent_name)
> if found is None:
> raise CX(_("profile %s not found, inheritance not possible") % parent_name)
> self.parent = parent_name
> @@ -102,7 +102,7 @@ class Profile(item.Item):
> Sets the distro. This must be the name of an existing
> Distro object in the Distros collection.
> """
> - d = self.config.distros().find(distro_name)
> + d = self.config.distros().find(name=distro_name)
> if d is not None:
> self.distro = distro_name
> self.depth = d.depth +1 # reset depth if previously a subprofile and now top-level
> @@ -125,7 +125,7 @@ class Profile(item.Item):
> except:
> pass
> for r in repolist:
> - if not self.config.repos().find(r):
> + if not self.config.repos().find(name=r):
> ok = False
> break
> if ok:
> @@ -203,9 +203,9 @@ class Profile(item.Item):
> Return object next highest up the tree.
> """
> if self.parent is None or self.parent == '':
> - result = self.config.distros().find(self.distro)
> + result = self.config.distros().find(name=self.distro)
> else:
> - result = self.config.profiles().find(self.parent)
> + result = self.config.profiles().find(name=self.parent)
> return result
>
> def is_valid(self):
> diff --git a/cobbler/item_system.py b/cobbler/item_system.py
> index cb7d02f..3575d79 100644
> --- a/cobbler/item_system.py
> +++ b/cobbler/item_system.py
> @@ -93,9 +93,9 @@ class System(item.Item):
> Return object next highest up the tree.
> """
> if self.parent is None or self.parent == '':
> - return self.config.profiles().find(self.profile)
> + return self.config.profiles().find(name=self.profile)
> else:
> - return self.config.systems().find(self.parent)
> + return self.config.systems().find(name=self.parent)
>
> def set_name(self,name):
> """
> @@ -180,10 +180,10 @@ class System(item.Item):
>
> def set_profile(self,profile_name):
> """
> - Set the system to use a certain named profile. The profile
> - must have already been loaded into the Profiles collection.
> - """
> - p = self.config.profiles().find(profile_name)
> + Set the system to use a certain named profile. The profile
> + must have already been loaded into the Profiles collection.
> + """
> + p = self.config.profiles().find(name=profile_name)
> if p is not None:
> self.profile = profile_name
> self.depth = p.depth + 1 # subprofiles have varying depths.
> @@ -229,8 +229,8 @@ class System(item.Item):
>
> def is_valid(self):
> """
> - A system is valid when it contains a valid name and a profile.
> - """
> + A system is valid when it contains a valid name and a profile.
> + """
> # NOTE: this validation code does not support inheritable distros at this time.
> # this is by design as inheritable systems don't make sense.
> if self.name is None:
> diff --git a/tests/tests.py b/tests/tests.py
> index 2f082e7..97b685c 100644
> --- a/tests/tests.py
> +++ b/tests/tests.py
> @@ -84,20 +84,20 @@ class BootTest(unittest.TestCase):
> self.assertTrue(distro.set_kernel(self.fk_kernel))
> self.assertTrue(distro.set_initrd(self.fk_initrd))
> self.assertTrue(self.api.distros().add(distro))
> - self.assertTrue(self.api.distros().find("testdistro0"))
> + self.assertTrue(self.api.distros().find(name="testdistro0"))
>
> profile = self.api.new_profile()
> self.assertTrue(profile.set_name("testprofile0"))
> self.assertTrue(profile.set_distro("testdistro0"))
> self.assertTrue(profile.set_kickstart(FAKE_KICKSTART))
> self.assertTrue(self.api.profiles().add(profile))
> - self.assertTrue(self.api.profiles().find("testprofile0"))
> + self.assertTrue(self.api.profiles().find(name="testprofile0"))
>
> system = self.api.new_system()
> self.assertTrue(system.set_name(self.hostname))
> self.assertTrue(system.set_profile("testprofile0"))
> self.assertTrue(self.api.systems().add(system))
> - self.assertTrue(self.api.systems().find(self.hostname))
> + self.assertTrue(self.api.systems().find(name=self.hostname))
>
> class Utilities(BootTest):
>
> @@ -149,7 +149,7 @@ class Additions(BootTest):
> self.failUnlessRaises(CobblerException,distro.set_kernel,"filedoesntexist")
> self.assertTrue(distro.set_initrd(self.fk_initrd))
> self.failUnlessRaises(CobblerException, self.api.distros().add, distro)
> - self.assertFalse(self.api.distros().find("testdistro2"))
> + self.assertFalse(self.api.distros().find(name="testdistro2"))
>
> def test_invalid_distro_non_referenced_initrd(self):
> distro = self.api.new_distro()
> @@ -157,7 +157,7 @@ class Additions(BootTest):
> self.assertTrue(distro.set_kernel(self.fk_kernel))
> self.failUnlessRaises(CobblerException, distro.set_initrd, "filedoesntexist")
> self.failUnlessRaises(CobblerException, self.api.distros().add, distro)
> - self.assertFalse(self.api.distros().find("testdistro3"))
> + self.assertFalse(self.api.distros().find(name="testdistro3"))
>
> def test_invalid_profile_non_referenced_distro(self):
> profile = self.api.new_profile()
> @@ -165,7 +165,7 @@ class Additions(BootTest):
> self.failUnlessRaises(CobblerException, profile.set_distro, "distrodoesntexist")
> self.assertTrue(profile.set_kickstart(FAKE_KICKSTART))
> self.failUnlessRaises(CobblerException, self.api.profiles().add, profile)
> - self.assertFalse(self.api.profiles().find("testprofile2"))
> + self.assertFalse(self.api.profiles().find(name="testprofile2"))
>
> def test_invalid_profile_kickstart_not_url(self):
> profile = self.api.new_profile()
> @@ -174,7 +174,7 @@ class Additions(BootTest):
> self.failUnlessRaises(CobblerException, profile.set_kickstart, "kickstartdoesntexist")
> # since kickstarts are optional, you can still add it
> self.assertTrue(self.api.profiles().add(profile))
> - self.assertTrue(self.api.profiles().find("testprofile12"))
> + self.assertTrue(self.api.profiles().find(name="testprofile12"))
> # now verify the other kickstart forms would still work
> self.assertTrue(profile.set_kickstart("http://bar"))
> self.assertTrue(profile.set_kickstart("ftp://bar"))
> @@ -199,7 +199,7 @@ class Additions(BootTest):
> self.assertTrue(system.set_name(name))
> self.assertTrue(system.set_profile("testprofile0"))
> self.assertTrue(self.api.systems().add(system))
> - self.assertTrue(self.api.systems().find(name))
> + self.assertTrue(self.api.systems().find(name=name))
>
> def test_system_name_is_an_IP(self):
> system = self.api.new_system()
> @@ -207,7 +207,7 @@ class Additions(BootTest):
> self.assertTrue(system.set_name(name))
> self.assertTrue(system.set_profile("testprofile0"))
> self.assertTrue(self.api.systems().add(system))
> - self.assertTrue(self.api.systems().find(name))
> + self.assertTrue(self.api.systems().find(name=name))
>
> def test_invalid_system_non_referenced_profile(self):
> system = self.api.new_system()
> @@ -241,9 +241,9 @@ class Deletions(BootTest):
> self.api.serialize()
> self.assertTrue(self.api.profiles().remove("testprofile0"))
> self.assertTrue(self.api.distros().remove("testdistro0"))
> - self.assertFalse(self.api.systems().find(self.hostname))
> - self.assertFalse(self.api.profiles().find("testprofile0"))
> - self.assertFalse(self.api.distros().find("testdistro0"))
> + self.assertFalse(self.api.systems().find(name=self.hostname))
> + self.assertFalse(self.api.profiles().find(name="testprofile0"))
> + self.assertFalse(self.api.distros().find(name="testdistro0"))
>
> class TestCheck(BootTest):
>
>
Thanks root at xt23.math.umn.edu :)
As I've talked with Ben on IRC, this is really good, and I've applied a
minor variant of this.
We keep API compatibility so collection.find("foo") works like
collection.find(name="foo"), added exception raising when searching on
an argument
that cobbler doesn't understand, and added the ability to return a list
in the API (collection.find(distro="xyz",return_list=True) -> []). I
will add information
on this to the API page on the Wiki.
In all fairness, my implementation of find is /much/ less elegant than
the version posted above :)
The change end users will see out of this so far is that "cobbler
status" will show the system name instead of the IP if it can figure it
out -- which is great.
Nothing else is effected.
--Michael
More information about the et-mgmt-tools
mailing list