Merge lp:~javier.collado/utah/bug1037056 into lp:utah

[Javier Collado]

- Updated Machine.run and SSHMixin.run to return (returncode, stdout, stderr) instead of just returncode.
- Added parsing of stderr output from gdebi to be able to detect a postinstall script failure
- Code will exit if that happens

After testing the code the output when a postinstall script error happens is:
INFO: Uploading /usr/share/utah/utah-client_0.4ubuntu46-rev644~precise1_all.deb from the host to /tmp on the machine
INFO: Running command through SSH: DEBIAN_FRONTEND=noninteractive gdebi -n -q /tmp/utah-client_0.4ubuntu46-rev644~precise1_all.deb
Exception: Failed to install client

The only thing that I don't really like yet is that the exception text is not written to the log file. I think that aside from the logger object in the machine we should have another logger object for the cases in which the machine might not exist so that every error message is written to the logs. Let me know if you agree and I'll open a bug for that.

[Max Brustkern (nuclearbob)]

I think that's a good idea. More logging at this point is almost always going to be helpful.

[Max Brustkern (nuclearbob)]

Also, this looks to me like it covers what we need. I'm a little surprised we have that few things that call the run() method of a machine, but I guess that's the idea. I think I'll need to update my bootspeed stuff as well, but that's in a separate branch.

review: Approve

[Javier Collado (javier.collado)]

Thanks Max.

I'll create a bug to review the print statements and see where using a logger outside from the machine object makes sense.

=== modified file 'utah/provisioning/baremetal/cobbler.py'
--- utah/provisioning/baremetal/cobbler.py	2012-08-03 21:44:39 +0000
+++ utah/provisioning/baremetal/cobbler.py	2012-08-16 08:40:40 +0000
@@ -144,9 +144,9 @@
         retry(self.sshcheck, checktimeout, logmethod=self.logger.info)
         self.provisioned = True
         self.active = True
-        if (self.run([('[ "{uuid}" == '
-                      + '"$(cat /etc/utah/uuid)" ]').format(uuid=self.uuid)])
-            != 0):
+        uuid_check_command = ('[ "{uuid}" == "$(cat /etc/utah/uuid)" ]'
+                              .format(uuid=self.uuid))
+        if self.run(uuid_check_command)[0] != 0:
             self.provisioned = False
             raise UTAHProvisioningException(
                     'Installed UUID differs from CobblerMachine UUID; '

=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2012-08-15 09:35:14 +0000
+++ utah/provisioning/provisioning.py	2012-08-16 08:40:40 +0000
@@ -16,6 +16,7 @@
 import urllib
 import uuid
 import paramiko
+import re
 
 
 import apt.cache
@@ -272,9 +273,11 @@
 
         remote_clientdeb = os.path.join(tmppath, os.path.basename(clientdeb))
         install_command = ('DEBIAN_FRONTEND=noninteractive '
-                           'gdebi -n {}'
+                           'gdebi -n -q {}'
                            .format(remote_clientdeb))
-        if self.run(install_command, root=True) != 0:
+        returncode, stdout, stderr = self.run(install_command, root=True)
+        if (returncode != 0
+            or re.search(r'script returned error exit status \d+', stderr)):
             raise UTAHProvisioningException('Failed to install client\n')
 
     def _provision(self):
@@ -364,8 +367,8 @@
 
     def run(self, command, quiet=None, root=False, timeout=None):
         """
-        Run a command on the machine and return the output status of that
-        command.
+        Run a command on the machine and return a three element tuple:
+        (returncode, stdout, stder)
         If quiet is True, all output other than the output of the command
         should be suppressed (i.e., no output from helper programs such as
         ssh.)
@@ -463,8 +466,8 @@
 
         self.logger.debug('Opening SSH session')
         channel = self.ssh_client.get_transport().open_session()
+
         self.logger.info('Running command through SSH: ' + commandstring)
-        channel.makefile('wb')
         stdout = channel.makefile('rb')
         stderr = channel.makefile_stderr('rb')
         if timeout is None:
@@ -472,19 +475,27 @@
         else:
             utah.timeout.timeout(timeout, channel.exec_command, commandstring)
         retval = channel.recv_exit_status()
+
         self.logger.debug('Closing SSH connection')
         self.ssh_client.close()
+
+        log_message = 'Return code: {}'.format(retval)
         if retval == 0:
-            self.logger.debug('Return code: {}'.format(retval))
+            self.logger.debug(log_message)
         else:
-            self.logger.warning('Return code: {}'.format(retval))
+            self.logger.warning(log_message)
+
         self.logger.debug('Standard output follows:')
-        for line in stdout:
+        stdout_lines = stdout.readlines()
+        for line in stdout_lines:
             self.logger.debug(line.strip())
+
         self.logger.debug('Standard error follows:')
-        for line in stderr:
+        stderr_lines = stderr.readlines()
+        for line in stderr_lines:
             self.logger.debug(line.strip())
-        return retval
+
+        return retval, ''.join(stdout_lines), ''.join(stderr_lines)
 
     def uploadfiles(self, files, target=os.path.normpath('/tmp/')):
         """

=== modified file 'utah/run.py'
--- utah/run.py	2012-08-14 21:10:29 +0000
+++ utah/run.py	2012-08-16 08:40:40 +0000
@@ -42,9 +42,9 @@
             machine.uploadfiles([locallist], os.path.normpath('/tmp'))
             options = (' -r /tmp/' + os.path.basename(locallist)
                        + ' -o ' + remotelog)
+            utah_command = 'utah' + extraopts + options
             try:
-                returncode = machine.run('utah' + extraopts + options,
-                                          root=True)
+                returncode = machine.run(utah_command, root=True)[0]
                 if returncode != 0:
                     machine.logger.error('utah failed with return code: {}\n'
                                          .format(returncode))