Merge lp:~vila/uci-engine/stores-put-dir into lp:uci-engine

[Vincent Ladeuil]

Upload a local directory into a swift container

/!\ Not ready for landing.

This implements DataStore.put_dir() in terms of DataStore.put_file() to
upload the content of a local hierarchy in a swift container (uci-britney
requirement).

I ran into swift eventually consistent model here and addressed the
container deletion issues in ad-hoc way that requires a separate MP.

I'll update *this* MP once the above issue is addressed.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:904
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1724/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1724/rebuild

review: Needs Fixing (continuous-integration)

[lp:~vila/uci-engine/stores-put-dir]

905. By Vincent Ladeuil on 2014-11-19

Fix pep8/pyflakes issues.

906. By Vincent Ladeuil on 2014-11-19

Raise the overall timeout for swift container deletion as 2mins was almost reached already.

$ ./run-tests tests.test_data_store.TestPutDir ^ci-utils.*test_fixtures test_style

is the proper way to test here, failures in
https://code.launchpad.net/~vila/uci-engine/stores-put-dir/+merge/242176 were caused by using ^ci_utils.*test_fixtures instead of ^ci-utils.*test_fixtures.

Ran 53 tests in 30.065s
OK

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:906
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1725/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1725/rebuild

review: Approve (continuous-integration)

[lp:~vila/uci-engine/stores-put-dir]

907. By Vincent Ladeuil on 2014-11-19

Meh, lexicographical order please.

908. By Vincent Ladeuil on 2014-11-20

Merge reliable container deletion fix

909. By Vincent Ladeuil on 2014-11-20

Merge pre-requisite again

910. By Vincent Ladeuil on 2014-11-20

Merge pre-requisite again

911. By Vincent Ladeuil on 2014-11-20

Refactor ci_utils.test_fixtures.TestPutDir based on tests.test_data_store.TestPutdir to the differences are easier to spot.

912. By Vincent Ladeuil on 2014-11-20

Clarify why ci_utils.tests.test_fixtures.TestFakeDataStorePutDir resemble tests.test_data_store.TestDataStorePutDir and where they differ.

Unmerged revisions

912. By Vincent Ladeuil on 2014-11-20

Clarify why ci_utils.tests.test_fixtures.TestFakeDataStorePutDir resemble tests.test_data_store.TestDataStorePutDir and where they differ.

911. By Vincent Ladeuil on 2014-11-20

Refactor ci_utils.test_fixtures.TestPutDir based on tests.test_data_store.TestPutdir to the differences are easier to spot.

910. By Vincent Ladeuil on 2014-11-20

Merge pre-requisite again

909. By Vincent Ladeuil on 2014-11-20

Merge pre-requisite again

908. By Vincent Ladeuil on 2014-11-20

Merge reliable container deletion fix

907. By Vincent Ladeuil on 2014-11-19

Meh, lexicographical order please.

906. By Vincent Ladeuil on 2014-11-19

Raise the overall timeout for swift container deletion as 2mins was almost reached already.

$ ./run-tests tests.test_data_store.TestPutDir ^ci-utils.*test_fixtures test_style

is the proper way to test here, failures in
https://code.launchpad.net/~vila/uci-engine/stores-put-dir/+merge/242176 were caused by using ^ci_utils.*test_fixtures instead of ^ci-utils.*test_fixtures.

Ran 53 tests in 30.065s
OK

905. By Vincent Ladeuil on 2014-11-19

Fix pep8/pyflakes issues.

904. By Vincent Ladeuil on 2014-11-19

Quick and dirty handling of container deletion: retry/sleep on 409, accept 404 when swift finally deleted the container while we were sleeping.

903. By Vincent Ladeuil on 2014-11-19

Make the tests pass by returning which files where sent to swift.

=== modified file 'britney_proxy/britney/process_results.py'
--- britney_proxy/britney/process_results.py	2014-10-30 17:30:49 +0000
+++ britney_proxy/britney/process_results.py	2014-11-19 12:34:02 +0000
@@ -51,11 +51,7 @@
 # container. This won't scale but we'll revisit the layout when we get a better
 # understanding of the britney needs (and numbers !) -- vila 2014-10-03
 class BritneyDataStore(data_store.DataStore):
-
-    def file_name(self, filename):
-        # Allow paths to be used as object names in the container by overriding
-        # the filtering in the base class.
-        return filename
+    pass
 
 
 class BritneyResultHandler(object):
 
=== modified file 'ci-utils/ci_utils/data_store.py'
--- ci-utils/ci_utils/data_store.py	2014-10-07 10:04:01 +0000
+++ ci-utils/ci_utils/data_store.py	2014-11-19 12:34:02 +0000
@@ -93,6 +93,21 @@
 
         return self.file_path(filename)
 
+    def put_dir(self, src_dir_name, dst_dir_name):
+        # Copy files from under 'src_dir_name' to 'dst_dir_name'.
+        src_root = os.path.abspath(src_dir_name)
+        prefix_len = len(src_root) + 1
+        uploaded = []
+        for root, _, files in os.walk(src_root):
+            for name in files:
+                src = os.path.normpath(os.path.join(root, name))
+                # Replace dir_path by dst_dir_name to get the expected names
+                dst = os.path.join(dst_dir_name, src[prefix_len:])
+                with open(src) as f:
+                    self.put_file(dst, f)
+                    uploaded.append(dst)
+        return uploaded
+
     def get_file(self, filename):
         self.ensure_swift_client()
         name = self.file_name(filename)
@@ -136,6 +151,9 @@
                     self.client.delete_object(self.container_id,
                                               obj=f['name'])
                 except ClientException as e:
+                    if e.http_status == 404:
+                        # Can happen when we retry
+                        continue
                     raise DataStoreException(
                         "Failed to delete file: {}".format(e)
                     )
@@ -144,7 +162,9 @@
                 "Failed to get container: {}".format(e)
             )
 
-    def delete(self, recursive=False):
+    # Tries 300 times, sleeping 2 secs: takes 5 minutes max, measure
+    # inconsistency window with a 2s resolution.
+    def delete(self, recursive=False, retries=300):
         self.ensure_swift_client()
         try:
             if recursive:
@@ -152,9 +172,14 @@
 
             self.client.delete_container(self.container_id)
         except ClientException as e:
-            raise DataStoreException(
-                "Failed to delete container: {}".format(e)
-            )
+            if retries and e.http_status == 409:
+                import time
+                time.sleep(2)
+                self.delete(recursive, retries=retries - 1)
+            elif e.http_status != 404:
+                raise DataStoreException(
+                    "Failed to delete container (retries: {}): {}".format(
+                        retries, e))
 
     def file_name(self, filename):
         """Returns the file name that is used inside the data store.
@@ -162,7 +187,7 @@
         :param filename: The file name used to build the object name inside the
             container.
         """
-        return os.path.basename(filename)
+        return filename
 
     def file_path(self, filename):
         name = self.file_name(filename)
 
=== modified file 'ci-utils/ci_utils/testing/fixtures.py'
--- ci-utils/ci_utils/testing/fixtures.py	2014-10-29 11:42:52 +0000
+++ ci-utils/ci_utils/testing/fixtures.py	2014-11-19 12:34:02 +0000
@@ -16,8 +16,10 @@
 import errno
 import os
 
+
 from ucitests import fixtures
 
+
 from ci_utils import data_store
 from ci_utils.json_status import LAST_REVISION
 
@@ -55,7 +57,15 @@
         os.mkdir(self.container_id)
 
     def list_files(self):
-        return sorted(os.listdir(self.container_id))
+        files = []
+        prefix_len = len(self.container_id) + 1
+        for root, dirs, fnames in os.walk(self.container_id):
+            # To mimick swift, We ignore the directories
+            for fname in fnames:
+                # Keep only the path relative to the container
+                relpath = os.path.join(root, fname)[prefix_len:]
+                files.append(relpath)
+        return sorted(files)
 
     def put_file(self, filename, contents, content_type=None):
         if os.sep in filename:
@@ -70,7 +80,13 @@
                     raise
         full_path = os.path.join(self.container_id, filename)
         with open(full_path, 'w') as f:
-            f.write(contents)
+            # Like swift, we accept file and strings for 'contents'
+            if getattr(contents, 'read', None) is not None:
+                # File handle
+                f.write(contents.read())
+            else:
+                # string
+                f.write(contents)
         return self.file_path(filename)
 
     def get_file(self, filename):
 
=== modified file 'ci-utils/ci_utils/tests/test_data_store.py'
--- ci-utils/ci_utils/tests/test_data_store.py	2014-10-07 10:04:01 +0000
+++ ci-utils/ci_utils/tests/test_data_store.py	2014-11-19 12:34:02 +0000
@@ -35,23 +35,5 @@
             data_store.DataStore.validate_auth_config({})
 
 
-class TestDataStoreFileName(unittest.TestCase):
-
-    def setUp(self):
-        super(TestDataStoreFileName, self).setUp()
-        # We won't use any method that requires the auth config so we don't
-        # provide any.
-        self.ds = data_store.DataStore('foo', None)
-
-    def test_abspath(self):
-        self.assertEquals('foo', self.ds.file_name('/baz/bar/foo'))
-
-    def test_relpath(self):
-        self.assertEquals('foo', self.ds.file_name('../baz/bar/foo'))
-
-    def test_basename(self):
-        self.assertEquals('foo', self.ds.file_name('foo'))
-
-
 if __name__ == "__main__":
     unittest.main()
 
=== modified file 'ci-utils/ci_utils/tests/test_fixtures.py'
--- ci-utils/ci_utils/tests/test_fixtures.py	2014-10-29 13:55:35 +0000
+++ ci-utils/ci_utils/tests/test_fixtures.py	2014-11-19 12:34:02 +0000
@@ -98,6 +98,38 @@
         self.assertEqual(errno.ENOTEMPTY, cm.exception.args[0])
 
 
+class TestPutDir(unittest.TestCase):
+
+    def setUp(self):
+        super(TestPutDir, self).setUp()
+        uci_fixtures.set_uniq_cwd(self)
+        self.ds_path = os.path.join(self.uniq_dir, 'data_store')
+        self.ds = fixtures.FakeDataStore(self.ds_path, None)
+        self.src_dir = os.path.join(self.uniq_dir, 'src')
+        os.mkdir(self.src_dir)
+
+    def test_put_dir_empty(self):
+        self.ds.put_dir(self.src_dir, 'dst')
+        self.assertEqual([], self.ds.list_files())
+
+    def test_one_file(self):
+        uci_fixtures.build_tree('''file: src/foo
+foo content
+''')
+        self.ds.put_dir(self.src_dir, 'dst')
+        self.assertEqual(['dst/foo'], self.ds.list_files())
+
+    def test_tree(self):
+        uci_fixtures.build_tree('''file: src/foo
+foo content
+dir: src/dir
+file: src/dir/bar
+bar content
+''')
+        self.ds.put_dir(self.src_dir, 'dst')
+        self.assertEqual(['dst/dir/bar', 'dst/foo'], self.ds.list_files())
+
+
 class TestUsingFakeDataStore(unittest.TestCase):
 
     def setUp(self):
 
=== modified file 'tests/test_data_store.py'
--- tests/test_data_store.py	2014-10-08 09:54:56 +0000
+++ tests/test_data_store.py	2014-11-19 12:34:02 +0000
@@ -21,6 +21,8 @@
 import yaml
 
 
+from ucitests import fixtures
+
 HERE = os.path.abspath(os.path.dirname(__file__))
 
 sys.path.append(os.path.join(HERE, '..', 'ci-utils'))
@@ -106,6 +108,45 @@
         self.assertEqual(200, resp.getcode(), resp.read())
 
 
+class TestPutDir(unittest.TestCase):
+
+    def setUp(self):
+        super(TestPutDir, self).setUp()
+        default_config = os.path.join(HERE, '..', 'unit_config')
+        if not os.path.exists(default_config):
+            self.skipTest('{} is not available'.format(default_config))
+        with open(default_config) as fp:
+            self.auth_config = yaml.safe_load(fp)
+
+        self.ds = data_store.DataStore(self.id(), auth_config=self.auth_config)
+        self.addCleanup(self.ds.delete, recursive=True)
+        fixtures.set_uniq_cwd(self)
+        self.src_dir = os.path.join(self.uniq_dir, 'src')
+        os.mkdir(self.src_dir)
+
+    def assertUploaded(self, expected, dst):
+        uploaded = self.ds.put_dir(self.src_dir, dst)
+        self.assertEqual(expected, sorted(uploaded))
+
+    def test_put_dir_empty(self):
+        self.assertUploaded([], 'dst')
+
+    def test_one_file(self):
+        fixtures.build_tree('''file: src/foo
+foo content
+''')
+        self.assertUploaded(['dst/foo'], 'dst')
+
+    def test_tree(self):
+        fixtures.build_tree('''file: src/foo
+foo content
+dir: src/dir
+file: src/dir/bar
+bar content
+''')
+        self.assertUploaded(['dst/dir/bar', 'dst/foo'], 'dst')
+
+
 class TestDataStoreBadConfigs(unittest.TestCase):
 
     def setUp(self):