Merge lp:~nuclearbob/utah/cleanup-cleanup into lp:utah

[Max Brustkern]

This branch moves all cleanup operations to the standard cleanup methods. I think this should allow us to remove the destroy function of the machine class once the scripts are modified appropriately. I'm working on that over in lp:~nuclearbob/utah/consolidate-scripts but I'd like to get this merged first.

[Andy Doan (doanac)]

nice idea - i like it.

review: Approve

[Javier Collado (javier.collado)]

A backtrace is printed when trying to run the code:
  File "utah/provisioning/ssh.py", line 58, in initialize
    cleanup.add_function(self.ssh_client.close)
TypeError: add_function() takes at least 3 arguments (2 given)

The problem is that the `cleanup.add_function` requires a timeout parameter
that is missing in the call above.

review: Needs Fixing

[Max Brustkern (nuclearbob)]

Fixed that. Running some testing now.

[Max Brustkern (nuclearbob)]

I fixed that and ran a successful test on a VM. Bare metal is broken in the current dev branch, so I can't test there yet, but I don't think this introduces and new problems there.

review: Needs Resubmitting

[lp:~nuclearbob/utah/cleanup-cleanup]

876. By Andy Doan on 2013-04-25

uuid_check requires ssh so "self.provisioned" must be True

This call was moved into provisioning.py where its a litle
easier to manage it in conjunction with the state the machine
is in. This now means vm's will do the check as well, but that
should be harmless.

877. By Javier Collado on 2013-04-26

Merged changes to fix help output to use just one runlist (LP: #1112597)

Source branch: lp:~nuclearbob/utah/bug1112597

883. By Max Brustkern on 2013-04-25

Added timeout to cleanup call

[Javier Collado (javier.collado)]

This worked fine for me with a server image with pass.run runlist.

review: Approve

=== modified file 'utah/provisioning/baremetal/bamboofeeder.py'
--- utah/provisioning/baremetal/bamboofeeder.py	2013-04-25 16:05:56 +0000
+++ utah/provisioning/baremetal/bamboofeeder.py	2013-04-26 17:14:27 +0000
@@ -40,8 +40,8 @@
 
     # TODO: raise more exceptions if ProcessRunner fails
     # maybe get some easy way to do that like failok
-    def __init__(self, inventory=None, machineinfo=None, name=None,
-                 preboot=None, *args, **kw):
+    def __init__(self, machineinfo=None, name=None, preboot=None, *args,
+                 **kw):
         # TODO: respect rewrite setting
         if name is None:
             raise UTAHBMProvisioningException(
@@ -50,7 +50,6 @@
             self.macaddress = machineinfo['mac-address']
         except AttributeError:
             raise UTAHBMProvisioningException('No MAC address specified')
-        self.inventory = inventory
         super(BambooFeederMachine, self).__init__(*args,
                                                   machineinfo=machineinfo,
                                                   name=name, **kw)
 
=== modified file 'utah/provisioning/baremetal/cobbler.py'
--- utah/provisioning/baremetal/cobbler.py	2013-04-25 16:05:56 +0000
+++ utah/provisioning/baremetal/cobbler.py	2013-04-26 17:14:27 +0000
@@ -39,8 +39,7 @@
 
     # TODO: raise more exceptions if ProcessRunner fails
     # maybe get some easy way to do that like failok
-    def __init__(self, inventory=None, machineinfo=None,
-                 name=None, *args, **kw):
+    def __init__(self, machineinfo=None,  name=None, *args, **kw):
         # TODO: support for reusing existing machines
         if name is None:
             raise UTAHBMProvisioningException(
@@ -51,7 +50,6 @@
         else:
             self.machineinfo = machineinfo
         self.power = {}
-        self.inventory = inventory
         super(CobblerMachine, self).__init__(*args, machineinfo=machineinfo,
                                              name=name, **kw)
         if self.inventory is not None:
 
=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2013-04-25 16:05:56 +0000
+++ utah/provisioning/provisioning.py	2013-04-26 17:14:27 +0000
@@ -66,10 +66,10 @@
 
     def __init__(self, arch=None, boot=None, clean=True, debug=False,
                  directory=None, image=None, dlpercentincrement=1,
-                 initrd=None, installtype=None, kernel=None, machineid=None,
-                 machineuuid=None, name=None, new=False, prefix='utah',
-                 preseed=None, rewrite=None, series=None, template=None,
-                 xml=None):
+                 initrd=None, installtype=None, inventory=None, kernel=None,
+                 machineid=None, machineuuid=None, name=None, new=False,
+                 prefix='utah', preseed=None, rewrite=None, series=None,
+                 template=None, xml=None):
         """Initialize the object representing the machine.
 
         One of these groups of arguments should be included:
@@ -90,6 +90,8 @@
                 used to install a physical machine using another method.
             dlpercentincrement: How often to log download updates to INFO
                 (All updates logged to DEBUG)
+            inventory: Inventory object managing the machine; used for cleanup
+                purposes.
             machineid: Should be passed by the request method of an Inventory
                 class.  Often used to name virtual machines.
             machineuuid: Unique identifier.  Will be randomly generated if not
@@ -117,6 +119,7 @@
         # TODO: Consider a global temp file creator, maybe as part of install.
         self.debug = debug
         self.dlpercentincrement = dlpercentincrement
+        self.inventory = inventory
         self.machineid = machineid
         self.new = new
         self.prefix = prefix
 
=== modified file 'utah/provisioning/ssh.py'
--- utah/provisioning/ssh.py	2013-04-24 11:37:21 +0000
+++ utah/provisioning/ssh.py	2013-04-26 17:14:27 +0000
@@ -26,11 +26,10 @@
 
 from stat import S_ISDIR
 
-from jinja2 import Environment, FileSystemLoader
-
 import utah.timeout
 
 from utah import config
+from utah.cleanup import cleanup
 from utah.provisioning.exceptions import UTAHProvisioningException
 from utah.provisioning.provisioning import Machine
 from utah.retry import retry
@@ -56,6 +55,7 @@
         ssh_client = paramiko.SSHClient()
         ssh_client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
         self.ssh_client = ssh_client
+        cleanup.add_function(10, self.ssh_client.close)
         self.ssh_logger = logging.getLogger('ssh')
 
     def run(self, command, quiet=False, root=False, command_timeout=None):
@@ -292,37 +292,6 @@
             else:
                 self.downloadfiles(myfile, newtarget)
 
-    def destroy(self, *args, **kw):
-        """Clean up known hosts for machine."""
-        # TODO: evaluate value of known_hosts with paramiko
-        # TODO: make this use standard cleanup if we do leave it in
-        self.ssh_logger.info('Removing machine addresses '
-                             'from ssh known_hosts file')
-        addresses = [self.name]
-        try:
-            addresses.append(socket.gethostbyname(self.name))
-        except socket.gaierror as err:
-            if err.errno in [-2, -5]:
-                self.ssh_logger.debug('{} is not resolvable, '
-                                      'so not removing from known_hosts'
-                                      .format(self.name))
-            else:
-                raise err
-
-        old_host_keys = self.ssh_client.get_host_keys()
-        new_host_keys = paramiko.HostKeys()
-        addresses = set(addresses)
-        for address, key in old_host_keys.iteritems():
-            # Skip keys so that they don't get added
-            # into the new keys (i.e. they're removed)
-            if address in addresses:
-                continue
-            new_host_keys[address] = key
-        new_host_keys.save(config.sshknownhosts)
-        self.ssh_client.close()
-
-        super(SSHMixin, self).destroy(*args, **kw)
-
     def sshcheck(self):
         """Check if the machine is available via ssh.
 
 
=== modified file 'utah/provisioning/vm/vm.py'
--- utah/provisioning/vm/vm.py	2013-04-18 17:17:45 +0000
+++ utah/provisioning/vm/vm.py	2013-04-26 17:14:27 +0000
@@ -90,6 +90,7 @@
                 self.vm.create()
         else:
             raise UTAHVMProvisioningException('Failed to provision VM')
+        self.cleanfunction(self.stop, force=True)
         self.active = True
 
     def stop(self, force=False):
@@ -163,6 +164,9 @@
             autoname = False
         super(CustomVM, self).__init__(machineid=machineid, name=name, *args,
                                        **kw)
+        if self.inventory is not None:
+            self.cleanfunction(self.inventory.destroy,
+                               machineid=self.machineid)
         # TODO: do a better job of separating installation
         # into _create rather than __init__
         if self.image is None:
@@ -202,6 +206,7 @@
             self.directory = os.path.join(config.vmpath, self.name)
         else:
             self.directory = directory
+        self.cleanfile(self.directory)
         self.logger.info('Checking if machine directory {} exists'
                          .format(self.directory))
         if not os.path.isdir(self.directory):
@@ -234,6 +239,7 @@
                     'size': disksize,
                     'type': 'qcow2'}
             self.disks.append(disk)
+            self.cleanfile(diskfile)
             self.logger.debug('Adding disk to list')
 
     def _supportsdomaintype(self, domaintype):
@@ -516,6 +522,7 @@
 
         self.logger.info('Setting up final VM')
         self.vm = self.lv.defineXML(ElementTree.tostring(xml.getroot()))
+        self.cleanfunction(self.vm.undefine)
 
     def _start(self):
         """Start the VM."""
@@ -538,19 +545,6 @@
         self.sshpoll(timeout=self.boottimeout)
         self.active = True
 
-    def destroy(self, *args, **kw):
-        """Remove the machine from libvirt and remove all the disk files."""
-        # TODO: make this use standard cleanup
-        super(CustomVM, self).destroy(*args, **kw)
-        self.stop(force=True)
-        if self.vm is not None:
-            self.vm.undefine()
-        else:
-            self.logger.info('VM not created')
-        for disk in self.disks:
-            os.unlink(disk['file'])
-        shutil.rmtree(self.directory)
-
 
 # See http://kennethreitz.com/blog/generate-a-random-mac-address-in-python/
 def random_mac_address():