Added a basic locking system to avoid multiple "sync" operations happening at the same time -- not a huge risk -- but it ought to be guarded against.

7 files changed, 79 insertions, 84 deletions

diff --git a/TODO b/TODO
index 9bc8715..13c83e8 100644
--- a/TODO
+++ b/TODO
@@ -1,17 +1,30 @@
 cobbler TODO list
-  -  GTK (or Web) UI for Cobbler ?
-  -  Port over to xeninst python module once available for something other than Rawhide
-  -  Wake on lan integration for PXE
-  -  Integration with stonnith or other up/down devices?
-  -  Locking
+
+need to fix:
+  -  global locking -- one cobbler command at a time
   -  For "enchant", try to verify that syslinux and mkinitrd are installed on the remote.    
-  -  "make test" shouldn't overwrite files in /var/lib/cobbler/ 
   -  "enchant" would be better if it didn't need to know the password
   -  "enchant" has issues with ssh key changes locking it out
   -  "enchant" should be written in a more distro-agnostic way.
-  -  "add" should be distinguished from "edit" so as to not overwrite
+  -  "cobbler add" should be distinguished from "edit" so as to not overwrite
+  -  "make test" shouldn't overwrite files in /var/lib/cobbler/ 
      new configs.
+
+really nice to have:
+  -  GTK (or Web) UI for Cobbler ?
+
+making things more standardized/flexible:
+  -  Port over to xeninst python module once available for something other than Rawhide
+  -  abstract out the --pxe-arch system, possibly pseudonym it to arch,
+     and make the multiple-bootloader system more plugin oriented to allow
+     for alternative bootloaders (not neccessarily kickstart related)
+     to be added
+  -  look into what it would take to make kickstart *more optional* 
+
+bells and whistles:
+  -  Wake on lan integration for PXE
+  -  Integration with stonnith or other up/down devices?
    
-general cleanup:
+internal code cleanup:
   -  an argument parsing overhaul is probably due
   -  remove the cobbler_msg logic since i18n isn't likely
diff --git a/cobbler/cobbler.py b/cobbler/cobbler.py
index 886a24b..2bfd810 100755
--- a/cobbler/cobbler.py
+++ b/cobbler/cobbler.py
@@ -16,10 +16,14 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 
 import sys
 import api
+import os
+import os.path
 
 import cobbler_msg
 import cexceptions
 
+LOCKFILE="/var/lib/cobbler/lock"
+
 class BootCLI:
 
 
@@ -58,7 +62,6 @@ class BootCLI:
             'systems'  : self.system,
             'system'   : self.system,
             'sync'     : self.sync,
-            'enchant'  : self.enchant_system,
             '--help'   : self.usage,
             '/?'       : self.usage
         }
@@ -89,6 +92,7 @@ class BootCLI:
         commands = {
            '--name'       : lambda(a):  self.api.systems().remove(a),
            '--system'     : lambda(a):  self.api.systems().remove(a),
+           '--system'     : lambda(a):  self.api.systems().remove(a)
         }
         on_ok = lambda: True
         return self.apply_args(args,commands,on_ok)
@@ -124,63 +128,15 @@ class BootCLI:
         """
         sys = self.api.new_system()
         commands = {
-           '--name'         :  lambda(a) : sys.set_name(a),
-           '--system'       :  lambda(a) : sys.set_name(a),
-           '--profile'      :  lambda(a) : sys.set_profile(a),
-           '--kopts'        :  lambda(a) : sys.set_kernel_options(a),
-           '--ksmeta'       :  lambda(a) : sys.set_ksmeta(a),
-           '--pxe-arch'     :  lambda(a) : sys.set_pxe_arch(a),
-           '--pxe-address'  :  lambda(a) : sys.set_pxe_address(a)
-           # FIXME: surface a way to pin the IP.
+           '--name'     :  lambda(a) : sys.set_name(a),
+           '--system'   :  lambda(a) : sys.set_name(a),
+           '--profile'  :  lambda(a) : sys.set_profile(a),
+           '--kopts'    :  lambda(a) : sys.set_kernel_options(a),
+           '--ksmeta'   :  lambda(a) : sys.set_ksmeta(a)
         }
         on_ok = lambda: self.api.systems().add(sys)
         return self.apply_args(args,commands,on_ok)
 
-    def apply_cmd_args(self,args,accepted,required,err_msg):
-        """
-        This is a variation of the command parsing done in apply_args but
-        is not centric to things in the object graph.  
-        """
-        for arg in args:
-             try:
-                 (first, last) = (None, None)
-                 (first, last) = arg.split("=")
-             except:
-                 if first is None: 
-                     continue
-                 raise cexceptions.CobblerException(err_msg)
-             if first in accepted.keys():
-                 accepted[first] = last
-             else:
-                 raise cexceptions.CobblerException("weird_arg",first)
-        for arg in required:
-            if accepted[arg] is None:
-                raise cexceptions.CobblerException(err_msg)
-        return accepted
-
-    def import_distros(self,args):
-        stored = {
-            "--path" : None
-        }
-        required = [ "--path" ]
-        results = self.apply_cmd_args(args,stored,required,"import_args")
-        return self.api.import_distros(stored["--path"])
-
-    def enchant_system(self,args):
-        """
-        Replace an existing running password.
-        """
-        stored = {
-           "--name" : None,
-           "--profile" : None,
-           "--password" : None
-        }
-        required = [ "--name", "--profile", "--password" ]
-        results = self.apply_cmd_args(args,stored,required,"enchant_args")
-        return self.api.enchant(stored["--name"], 
-            stored["--profile"],
-            stored["--password"]
-        )
 
     def profile_edit(self,args):
         """
@@ -199,6 +155,7 @@ class BootCLI:
             '--ksmeta'          :  lambda(a) : profile.set_ksmeta(a)
         # the following options are most likely not useful for profiles (yet)
         # primarily due to not being implemented in koan.
+        #    '--xen-mac'         :  lambda(a) : profile.set_xen_mac(a),
         #    '--xen-paravirt'    :  lambda(a) : profile.set_xen_paravirt(a),
         }
         on_ok = lambda: self.api.profiles().add(profile)
@@ -221,6 +178,7 @@ class BootCLI:
         on_ok = lambda: self.api.distros().add(distro)
         return self.apply_args(args,commands,on_ok)
 
+
     def apply_args(self,args,input_routines,on_ok):
         """
         Custom CLI handling, instead of getopt/optparse.
@@ -269,6 +227,7 @@ class BootCLI:
             status = self.api.sync(dryrun=False)
         return status
 
+
     def check(self,args):
         """
         Check system for network boot decency/prereqs: 'cobbler check'
@@ -306,12 +265,24 @@ def main():
     """
     CLI entry point
     """
+    exitcode = 0
     try:
+        if os.path.exists(LOCKFILE):
+            raise cexceptions.CobblerException("lock")
+        try:
+            lockfile = open(LOCKFILE,"w+")
+        except:
+            raise cexceptions.CobblerException("no_create",LOCKFILE)
+        lockfile.close()
         BootCLI(sys.argv).run()
     except cexceptions.CobblerException, exc:
         print str(exc)[1:-1]  # remove framing air quotes
-        sys.exit(1)
-    sys.exit(0)
+        exitcode = 1
+    try:
+        os.remove(LOCKFILE)
+    except:
+        pass
+    return exitcode
 
 if __name__ == "__main__":
     sys.exit(main())
diff --git a/cobbler/cobbler_msg.py b/cobbler/cobbler_msg.py
index 48e357b..dbcdfff 100644
--- a/cobbler/cobbler_msg.py
+++ b/cobbler/cobbler_msg.py
@@ -86,6 +86,7 @@ _msg_table = {
   "exc_no_template" : "can't read /etc/cobbler/dhcp.template",
   "exc_dhcp_nomac"  : "when cobbler is managing dhcpd.conf, all system names must be MAC addresses.  Aborting.", 
   "exc_ia64_noip"   : "due to an IA64 bootloader limitation, system (%s) must define a pxe-address value (which is an IP address)",
+  "lock"            : "Locked.  If cobbler is currently running, wait for termination, otherwise remove /var/lib/cobbler/lock",
   "check_ok"        : """
 No setup problems found.
 
diff --git a/cobbler/item.py b/cobbler/item.py
index 6c55225..0bec45f 100644
--- a/cobbler/item.py
+++ b/cobbler/item.py
@@ -48,6 +48,16 @@ class Item(serializable.Serializable):
                 return False
         return True
 
+    def load_item(self,datastruct,key):
+        """
+        Used in subclass from_datastruct functions to load items from
+        a hash.  Intented to ease backwards compatibility of config
+        files during upgrades.
+        """
+        if datastruct.has_key(key):
+            return datastruct[key]
+        return '' 
+
     def to_datastruct(self):
         """
 	Returns an easily-marshalable representation of the collection.
diff --git a/cobbler/item_distro.py b/cobbler/item_distro.py
index 838dc1a..0d0f26d 100644
--- a/cobbler/item_distro.py
+++ b/cobbler/item_distro.py
@@ -43,11 +43,11 @@ class Distro(item.Item):
         """
         Modify this object to take on values in seed_data
         """
-        self.name = seed_data['name']
-        self.kernel = seed_data['kernel']
-        self.initrd = seed_data['initrd']
-        self.kernel_options = seed_data['kernel_options']
-        self.ks_meta = seed_data['ks_meta']
+        self.name           = self.load_item(seed_data,'name')
+        self.kernel         = self.load_item(seed_data,'kernel')
+        self.initrd         = self.load_item(seed_data,'initrd')
+        self.kernel_options = self.load_item(seed_data,'kernel_options')
+        self.ks_meta        = self.load_item(seed_data,'ks_meta')
         return self
 
     def set_kernel(self,kernel):
diff --git a/cobbler/item_profile.py b/cobbler/item_profile.py
index 6a626d3..ad1cf45 100644
--- a/cobbler/item_profile.py
+++ b/cobbler/item_profile.py
@@ -43,17 +43,17 @@ class Profile(item.Item):
         """
         Load this object's properties based on seed_data
         """
-        self.name            = seed_data['name']
-        self.distro          = seed_data['distro']
-        self.kickstart       = seed_data['kickstart']
-        self.kernel_options  = seed_data['kernel_options']
-        self.ks_meta         = seed_data['ks_meta']
-        self.xen_name        = seed_data['xen_name']
+        self.name            = self.load_item(seed_data,'name')
+        self.distro          = self.load_item(seed_data,'distro')
+        self.kickstart       = self.load_item(seed_data,'kickstart')
+        self.kernel_options  = self.load_item(seed_data,'kernel_options')
+        self.ks_meta         = self.load_item(seed_data,'ks_meta')
+        self.xen_name        = self.load_item(seed_data,'xen_name')
         if not self.xen_name or self.xen_name == '':
             self.xen_name    = self.name
-        self.xen_ram         = seed_data['xen_ram']
-        self.xen_file_size   = seed_data['xen_file_size']
-        self.xen_paravirt    = seed_data['xen_paravirt']
+        self.xen_ram         = self.load_item(seed_data,'xen_ram')
+        self.xen_file_size   = self.load_item(seed_data,'xen_file_size')
+        self.xen_paravirt    = self.load_item(seed_data,'xen_paravirt')
         return self
 
     def set_distro(self,distro_name):
diff --git a/cobbler/item_system.py b/cobbler/item_system.py
index 217e511..2568959 100644
--- a/cobbler/item_system.py
+++ b/cobbler/item_system.py
@@ -31,12 +31,12 @@ class System(item.Item):
         self.pxe_address = ""
 
     def from_datastruct(self,seed_data):
-        self.name = seed_data['name']
-        self.profile = seed_data['profile']
-        self.kernel_options = seed_data['kernel_options']
-        self.ks_meta = seed_data['ks_meta']
-        self.pxe_arch = seed_data['pxe_arch']
-        self.pxe_address = seed_data['pxe_address']
+        self.name = self.load_item(seed_data,'name')
+        self.profile = self.load_item(seed_data,'profile')
+        self.kernel_options = self.load_item(seed_data,'kernel_options')
+        self.ks_meta = self.load_item(seed_data,'ks_meta')
+        self.pxe_arch = self.load_item(seed_data,'pxe_arch')
+        self.pxe_address = self.load_item(seed_data,'pxe_address')
         return self
 
     def set_name(self,name):