Merge lp:~milo/lava-dispatcher/tar-repo into lp:lava-dispatcher

[Milo Casagrande]

Added support to the dispatcher for tar-repo: a base64-encoded string of the content of a tar file.
This is necessary in order to support ongoing work on lava-tool (LAVA-15 card).

[lp:~milo/lava-dispatcher/tar-repo]

639. By Milo Casagrande on 2013-07-17

Fixed problem with extract directory and tar file.

640. By Milo Casagrande on 2013-07-17

Fixed typo.

[Milo Casagrande (milo)]

I pushed new changes to fix a couple of problems found while running jobs.

[lp:~milo/lava-dispatcher/tar-repo]

641. By Milo Casagrande on 2013-07-18

Use 0 instead of nil for tar revision.

642. By Milo Casagrande on 2013-07-22

Merged from trunk, fixed conflicts.

643. By Milo Casagrande on 2013-07-22

Fixed PEP8 problem from previous merge.

[Antonio Terceiro (terceiro)]

Hi Milo,

This looks nice. I have a few comments to be addressed.

 review needs-fixing

> === modified file 'lava_dispatcher/actions/lava_test_shell.py'
> --- lava_dispatcher/actions/lava_test_shell.py 2013-07-17 09:16:46 +0000
> +++ lava_dispatcher/actions/lava_test_shell.py 2013-07-22 13:39:27 +0000
> @@ -107,13 +107,16 @@
>
> from datetime import datetime
> from glob import glob
> +import base64
> import logging
> import os
> import pexpect
> import pkg_resources
> import shutil
> import stat
> +import StringIO
> import subprocess
> +import tarfile
> import tempfile
> import time
> from uuid import uuid4
> @@ -187,6 +190,37 @@
> logging.error('Unable to get test definition from bzr\n' + str(e))
>
>
> +def _get_testdef_tar_repo(testdef_repo, tmpdir):
> + """Extracts the provided encoded tar archive into tmpdir."""
> + tardir = os.path.join(tmpdir, 'tartestrepo')
> + temp_tar = os.path.join(tmpdir, "tar-repo.tar")
> +
> + try:
> + if not os.path.isdir(tardir):
> + logging.info("Creating directory to extract the tar archive into.")
> + os.makedirs(tardir)
> +
> + encoded_in = StringIO.StringIO(testdef_repo)
> + decoded_out = StringIO.StringIO()
> + base64.decode(encoded_in, decoded_out)
> +
> + # The following two operations can also be done in memory
> + # using cStringIO.
> + # At the moment the tar file sent is not big, but that can change.
> + with open(temp_tar, "w") as write_tar:
> + write_tar.write(decoded_out.getvalue())
> +
> + with tarfile.open(temp_tar) as tar:
> + tar.extractall(path=tardir)
> + except Exception as ex:

can we capture a more specific type of exception here? otherwise we may be
masking random exceptions that might happen above.

> + logging.error("Error extracting the tar archive.\n" + str(ex))
> + finally:
> + # Remove the temporary created tar file after it has been extracted.
> + if os.path.isfile(temp_tar):
> + os.unlink(temp_tar)
> + return tardir
> +
> +
> def _get_testdef_info(testdef):
> metadata = {'os': '', 'devices': '', 'environment': ''}
> metadata['description'] = testdef['metadata'].get('description')
> @@ -255,8 +289,20 @@
> name = testdef_repo['bzr-repo'].replace('lp:', '').split('/')[-1]
> info = _bzr_info(testdef_repo['bzr-repo'], repo, name)
>
> + if 'tar-repo' in testdef_repo:
> + repo = _get_testdef_tar_repo(testdef_repo['tar-repo'], tmpdir)
> + # Default info structure, since we need something, but we have
> + # a tar file in this case.
> + info = {
> + "project_name": "Tar archived repository",
> + "branch_vcs": "tar",
> + "branch_revision": "0",
> + "branch_url": repo
> + }
> +
> if not repo or not info:
> logging.debug("Unable to identify specified repository. %s" % testdef_repo)
> +
> test = testdef_repo.get('testdef', 'lavatest.yaml')
> with open(os.path.join(repo, test), 'r') as f:
> logging....

review: Needs Fixing

[lp:~milo/lava-dispatcher/tar-repo]

644. By Milo Casagrande on 2013-07-23

Added more specific exceptions.

[Milo Casagrande (milo)]

Hi Antonio,

thanks for looking into it.

On Mon, Jul 22, 2013 at 11:46 PM, Antonio Terceiro
<email address hidden> wrote:
>
>> +def _get_testdef_tar_repo(testdef_repo, tmpdir):
>> + """Extracts the provided encoded tar archive into tmpdir."""
>> + tardir = os.path.join(tmpdir, 'tartestrepo')
>> + temp_tar = os.path.join(tmpdir, "tar-repo.tar")
>> +
>> + try:
>> + if not os.path.isdir(tardir):
>> + logging.info("Creating directory to extract the tar archive into.")
>> + os.makedirs(tardir)
>> +
>> + encoded_in = StringIO.StringIO(testdef_repo)
>> + decoded_out = StringIO.StringIO()
>> + base64.decode(encoded_in, decoded_out)
>> +
>> + # The following two operations can also be done in memory
>> + # using cStringIO.
>> + # At the moment the tar file sent is not big, but that can change.
>> + with open(temp_tar, "w") as write_tar:
>> + write_tar.write(decoded_out.getvalue())
>> +
>> + with tarfile.open(temp_tar) as tar:
>> + tar.extractall(path=tardir)
>> + except Exception as ex:
>
> can we capture a more specific type of exception here? otherwise we may be
> masking random exceptions that might happen above.

Fixed now, I used: (OSError, tarfile.TarError) to catch possible
errors with both open() and tarfile.open().

>> {'git-repo': {'type': 'string',
>> - 'optional': True},
>> - 'bzr-repo': {'type': 'string',
>> - 'optional': True},
>> - 'revision': {'type': 'string',
>> - 'optional': True},
>> - 'testdef': {'type': 'string',
>> - 'optional': True}
>> + 'optional': True},
>> + 'bzr-repo': {'type': 'string',
>> + 'optional': True},
>> + 'tar-repo': {'type': 'string',
>> + 'optional': True},
>> + 'revision': {'type': 'string',
>> + 'optional': True},
>> + 'testdef': {'type': 'string',
>> + 'optional': True}
>
> I think this indentation here got changed with a recent PEP8-related merge so
> that this will not merge cleanly, please check.

I merged from trunk and cleaned up, it should be OK.
Ciao.

--
Milo Casagrande | Automation Engineer
Linaro.org <www.linaro.org> │ Open source software for ARM SoCs

[Antonio Terceiro (terceiro)]

On Tue, Jul 23, 2013 at 08:31:32AM -0000, Milo Casagrande wrote:
> >> + with tarfile.open(temp_tar) as tar:
> >> + tar.extractall(path=tardir)
> >> + except Exception as ex:
> >
> > can we capture a more specific type of exception here? otherwise we may be
> > masking random exceptions that might happen above.
>
> Fixed now, I used: (OSError, tarfile.TarError) to catch possible
> errors with both open() and tarfile.open().

cool

 review approve

review: Approve

=== modified file 'lava_dispatcher/actions/lava_test_shell.py'
--- lava_dispatcher/actions/lava_test_shell.py	2013-07-17 09:16:46 +0000
+++ lava_dispatcher/actions/lava_test_shell.py	2013-07-23 08:14:26 +0000
@@ -107,13 +107,16 @@
 
 from datetime import datetime
 from glob import glob
+import base64
 import logging
 import os
 import pexpect
 import pkg_resources
 import shutil
 import stat
+import StringIO
 import subprocess
+import tarfile
 import tempfile
 import time
 from uuid import uuid4
@@ -187,6 +190,37 @@
         logging.error('Unable to get test definition from bzr\n' + str(e))
 
 
+def _get_testdef_tar_repo(testdef_repo, tmpdir):
+    """Extracts the provided encoded tar archive into tmpdir."""
+    tardir = os.path.join(tmpdir, 'tartestrepo')
+    temp_tar = os.path.join(tmpdir, "tar-repo.tar")
+
+    try:
+        if not os.path.isdir(tardir):
+            logging.info("Creating directory to extract the tar archive into.")
+            os.makedirs(tardir)
+
+        encoded_in = StringIO.StringIO(testdef_repo)
+        decoded_out = StringIO.StringIO()
+        base64.decode(encoded_in, decoded_out)
+
+        # The following two operations can also be done in memory
+        # using cStringIO.
+        # At the moment the tar file sent is not big, but that can change.
+        with open(temp_tar, "w") as write_tar:
+            write_tar.write(decoded_out.getvalue())
+
+        with tarfile.open(temp_tar) as tar:
+            tar.extractall(path=tardir)
+    except (OSError, tarfile.TarError) as ex:
+        logging.error("Error extracting the tar archive.\n" + str(ex))
+    finally:
+        # Remove the temporary created tar file after it has been extracted.
+        if os.path.isfile(temp_tar):
+            os.unlink(temp_tar)
+    return tardir
+
+
 def _get_testdef_info(testdef):
     metadata = {'os': '', 'devices': '', 'environment': ''}
     metadata['description'] = testdef['metadata'].get('description')
@@ -255,8 +289,20 @@
             name = testdef_repo['bzr-repo'].replace('lp:', '').split('/')[-1]
             info = _bzr_info(testdef_repo['bzr-repo'], repo, name)
 
+        if 'tar-repo' in testdef_repo:
+            repo = _get_testdef_tar_repo(testdef_repo['tar-repo'], tmpdir)
+            # Default info structure, since we need something, but we have
+            # a tar file in this case.
+            info = {
+                "project_name": "Tar archived repository",
+                "branch_vcs": "tar",
+                "branch_revision": "0",
+                "branch_url": repo
+            }
+
         if not repo or not info:
             logging.debug("Unable to identify specified repository. %s" % testdef_repo)
+
         test = testdef_repo.get('testdef', 'lavatest.yaml')
         with open(os.path.join(repo, test), 'r') as f:
             logging.info('loading test definition ...')
@@ -462,13 +508,15 @@
                               'items': {'type': 'object',
                                         'properties':
                                         {'git-repo': {'type': 'string',
-                                                'optional': True},
-                                        'bzr-repo': {'type': 'string',
-                                                'optional': True},
-                                        'revision': {'type': 'string',
-                                                'optional': True},
-                                        'testdef': {'type': 'string',
-                                                'optional': True}
+                                                      'optional': True},
+                                         'bzr-repo': {'type': 'string',
+                                                      'optional': True},
+                                         'tar-repo': {'type': 'string',
+                                                      'optional': True},
+                                         'revision': {'type': 'string',
+                                                      'optional': True},
+                                         'testdef': {'type': 'string',
+                                                     'optional': True}
                                          },
                                         'additionalProperties': False},
                               'optional': True