Merge lp:~hazmat/pyjuju/lib-files into lp:pyjuju

[Kapil Thangavelu]

Both the dummy and local dev provider would like to utilise a local disk backed provider filestorage. The implementation currently in the dummy provider should be moved into provider.common for easier reuse by both.

[William Reade (fwereade)]

[0]

get_url is missing, and is needed for download_formula.

I'm not sure whether it should return a file:// url (which download_formula does in fact undertand) because I'm not sure whether we intend local dev machines to have access to the whole filesystem; I'd suggest that they probably shouldn't, really... and so, if that's the case, we'll presumably need to run some sort of file server of our own. Thoughts?

review: Needs Fixing

[Kapil Thangavelu (hazmat)]

Excerpts from William Reade's message of Thu Sep 01 08:58:22 UTC 2011:
> Review: Needs Fixing
> [0]
>
> get_url is missing, and is needed for download_formula.
>
> I'm not sure whether it should return a file:// url (which download_formula does in fact undertand) because I'm not sure whether we intend local dev machines to have access to the whole filesystem; I'd suggest that they probably shouldn't, really... and so, if that's the case, we'll presumably need to run some sort of file server of our own. Thoughts?

Thanks for having a look at this.

[0]
Addressed the get_url missing and some other test assumptions.

The machine agent will be running in the host creating lxc containers, and needs
to do so as root. The formulas are specifically downloaded into /formulas on the
container rootfs by the machine agent. ie. using local disk is a very good fit
imo, its the simplest thing that works, and has low overhead. Additional
commonality between separate provider implementations, shouldn't come at the
expense of that.

cheers,

kapil

[Gustavo Niemeyer (niemeyer)]

[1]

I'm missing some context about how that's to be used. The LXC container doesn't have access
to a common location in the outside disk, so how does a local disk formula would work in
that case?

Depending on your thinking around this, an alternative could be the Orchestra file storage
provider, that uses webdav. We could look for a trivial webdav provider to set up
automatically in the local machine.

review: Needs Information

[Kapil Thangavelu (hazmat)]

Excerpts from Gustavo Niemeyer's message of Sat Sep 03 14:14:22 UTC 2011:
> Review: Needs Information
> [1]
>
> I'm missing some context about how that's to be used. The LXC container doesn't have access
> to a common location in the outside disk, so how does a local disk formula would work in
> that case?
>
> Depending on your thinking around this, an alternative could be the Orchestra file storage
> provider, that uses webdav. We could look for a trivial webdav provider to set up
> automatically in the local machine.

The machine agent which puts the formula into place on the unit, lives outside
the container, and has direct fs access to the provider storage directory and
container rootfs.

[Gustavo Niemeyer (niemeyer)]

Sounds good, thanks for the info.

review: Approve

[Gustavo Niemeyer (niemeyer)]

Putting it back in review for William to see if the changes look good.

[lp:~hazmat/pyjuju/lib-files]

344. By Kapil Thangavelu on 2011-09-06

Merged lib-zookeeper into lib-files.

345. By Kapil Thangavelu on 2011-09-09

Merged lib-zookeeper into lib-files.

[Gustavo Niemeyer (niemeyer)]

Please go ahead and merge this Kapil.

review: Approve

=== modified file 'ensemble/formula/tests/test_publisher.py'
--- ensemble/formula/tests/test_publisher.py	2011-08-17 14:56:42 +0000
+++ ensemble/formula/tests/test_publisher.py	2011-09-13 23:16:26 +0000
@@ -54,7 +54,8 @@
             self.formula_key, self.formula.get_sha256())
 
         self.client = ZookeeperClient(get_test_zookeeper_address())
-        self.storage = FileStorage(self.makeDir())
+        self.storage_dir = self.makeDir()
+        self.storage = FileStorage(self.storage_dir)
         self.publisher = FormulaPublisher(self.client, self.storage)
 
         yield self.client.connect()
@@ -79,7 +80,7 @@
 
         self.assertEqual(
             result[0].bundle_url, "file://%s/formulas/%s" % (
-                self.storage.path, self.formula_storage_key))
+                self.storage_dir, self.formula_storage_key))
 
     @inlineCallbacks
     def test_published_formula_sans_unicode(self):
@@ -142,7 +143,7 @@
             yaml.load(content)["sha256"],
             self.formula_storage_key.split(":")[-1])
 
-        stored_files = os.listdir(os.path.join(self.storage.path, "formulas"))
+        stored_files = os.listdir(os.path.join(self.storage_dir, "formulas"))
         self.assertIn(self.formula_storage_key, stored_files)
 
         # and the binary bits where stored

=== modified file 'ensemble/lib/lxc/__init__.py'
--- ensemble/lib/lxc/__init__.py	2011-09-13 23:16:26 +0000
+++ ensemble/lib/lxc/__init__.py	2011-09-13 23:16:26 +0000
@@ -1,37 +1,38 @@
 import os
 import pipes
-import shutil
 import subprocess
 import sys
 import tempfile
 
-from twisted.internet.defer import inlineCallbacks
+from twisted.internet.defer import inlineCallbacks, returnValue
 from twisted.internet.threads import deferToThread
 
 from ensemble.errors import EnsembleError
 
-BASE_PATH = os.path.normpath(
-    os.path.abspath(
-        os.path.join(os.path.dirname(__file__), "data")))
+DATA_PATH = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), "data"))
 
 
 class LXCError(EnsembleError):
     """Indicates a low level error with an LXC container"""
-    pass
 
 
 def _cmd(args):
-    p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                         env=dict(os.environ))
-    r = p.wait()
+    p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+    stdout_data, _ = p.communicate()
+    r = p.returncode
     if  r != 0:
         # read the stdout/err streams and show the user
-        print >>sys.stderr, p.stderr.read()
-        raise LXCError(p.stdout.read())
-    return r
-
-
-def _lxc_create(container_name, template="ubuntu", config_file=None, release="oneiric"):
+        print >>sys.stderr, stdout_data
+        raise LXCError(stdout_data)
+    return (r, stdout_data)
+
+
+# Wrapped lxc cli primitives
+def _lxc_create(
+    container_name, template="ubuntu", config_file=None, release="oneiric"):
+    # the -- argument indicates the last parameters are passed
+    # to the template and not handled by lxc-create
     args = ["sudo", "lxc-create",
             "-n", container_name,
             "-t", template,
@@ -50,9 +51,7 @@
 
 
 def _lxc_stop(container_name):
-    subprocess.Popen(["sudo", "lxc-stop", "-n",
-                      container_name],
-                     stderr=subprocess.PIPE).wait()
+    _cmd(["sudo", "lxc-stop", "-n", container_name])
 
 
 def _lxc_destroy(container_name):
@@ -60,8 +59,7 @@
 
 
 def _lxc_ls():
-    output = subprocess.Popen(["lxc-ls"],
-                              stdout=subprocess.PIPE).communicate()[0]
+    _, output = _cmd(["lxc-ls"])
     output = output.replace("\n", " ")
     return set([c for c in output.split(" ") if c])
 
@@ -70,7 +68,7 @@
     """Wait for container to be in a given state RUNNING|STOPPED."""
 
     def wait(container_name):
-        rc = _cmd(["sudo", "lxc-wait",
+        rc, _ = _cmd(["sudo", "lxc-wait",
                    "-n", container_name,
                    "-s", state])
         return rc == 0
@@ -83,27 +81,52 @@
         raise LXCError("Expect container root directory: %s" %
                        container_root)
     # write the container scripts into the container
-    base_script = os.path.basename(customize_script)
-    in_path = os.path.join(container_root, "tmp", base_script)
-
-    shutil.copy(customize_script, in_path)
-    os.chmod(in_path, 0770)
-
-    args = ["sudo", "chroot",
-            container_root,
-            os.path.join("/tmp", base_script)]
+    fd, in_path = tempfile.mkstemp(prefix=os.path.basename(customize_script),
+                                   dir=os.path.join(container_root, "tmp"))
+
+    os.write(fd, open(customize_script, "r").read())
+    os.close(fd)
+    os.chmod(in_path, 0755)
+
+    args = ["sudo", "chroot", container_root,
+            os.path.join("/tmp", os.path.basename(in_path))]
     return _cmd(args)
 
 
 def validate_path(pathname):
     if not os.access(pathname, os.R_OK):
-        raise LXCError("Invalid file %s" % pathname)
+        raise LXCError("Invalid or unreadable file: %s" % pathname)
+
+
+@inlineCallbacks
+def get_containers(prefix):
+    """Return a dictionary of containers key names to runtime boolean value.
+
+    :param prefix: Optionally specify a prefix that the container should
+    match any returned containers.
+    """
+    _, output = yield deferToThread(_cmd, ["lxc-ls"])
+
+    containers = {}
+    for i in filter(None, output.split("\n")):
+        if i in containers:
+            containers[i] = True
+        else:
+            containers[i] = False
+
+    if prefix:
+        remove = [k for k in containers.keys() if not
+                  k.startswith(prefix)]
+        map(containers.pop, remove)
+
+    returnValue(containers)
 
 
 class LXCContainer(object):
-    def __init__(self, container_name, configuration=None, network_name="virbr0",
+    def __init__(self,
+                 container_name, configuration=None, network_name="virbr0",
                  customize_script=None):
-        """Create an LXC container
+        """Create an LXCContainer
 
         :param container_name: should be unique within the system
 
@@ -118,17 +141,15 @@
         """
 
         self.container_name = container_name
-        # open the template file and create a new temp processed
-        # version
-        self.lxc_config = self._make_lxc_config(network_name)
 
         if customize_script is None:
-            customize_script = os.path.join(BASE_PATH,
+            customize_script = os.path.join(DATA_PATH,
                                             "ensemble-create")
         self.customize_script = customize_script
         validate_path(self.customize_script)
 
         self.configuration = configuration
+        self.network_name = network_name
         self.running = False
 
     @property
@@ -141,18 +162,19 @@
         return os.path.join(self.rootfs, path)
 
     def _make_lxc_config(self, network_name):
-        lxc_config = os.path.join(BASE_PATH, "lxc.conf")
-        template = open(lxc_config, "r").read()
-
-        fd, output_fn = tempfile.mkstemp(suffix=".conf")
-        output_config = open(output_fn, "w")
-        output_config.write(template % {"network_name": network_name})
-        output_config.close()
-
-        validate_path(output_fn)
-        return output_fn
-
-    def _injectConfiguration(self):
+        lxc_config = os.path.join(DATA_PATH, "lxc.conf")
+        with open(lxc_config, "r") as fh:
+            template = fh.read()
+            fd, output_fn = tempfile.mkstemp(suffix=".conf")
+            output_config = open(output_fn, "w")
+            output_config.write(template % {"network_name": network_name})
+            output_config.close()
+
+            validate_path(output_fn)
+            return output_fn
+
+    @inlineCallbacks
+    def _customize_container(self):
         config_dir = self._p("etc/ensemble")
         if not os.path.exists(config_dir):
             _cmd(["sudo", "mkdir", config_dir])
@@ -168,16 +190,26 @@
         _cmd(["sudo", "mv", fn,
               self._p("etc/ensemble/ensemble.conf")])
 
+        yield deferToThread(
+            _customize_container, self.customize_script, self.rootfs)
+
     @inlineCallbacks
-    def _customize_container(self):
-        self._injectConfiguration()
-        yield deferToThread(_customize_container, self.customize_script, self.rootfs)
+    def execute(self, args):
+        if not isinstance(args, (list, tuple)):
+            args = [args, ]
+
+        args = ["sudo", "chroot", self.rootfs] + args
+        yield deferToThread(_cmd, args)
 
     @inlineCallbacks
     def create(self):
+        # open the template file and create a new temp processed
+        # version
+        lxc_config = self._make_lxc_config(self.network_name)
         yield deferToThread(_lxc_create, self.container_name,
-                            config_file=self.lxc_config)
+                            config_file=lxc_config)
         yield self._customize_container()
+        os.unlink(lxc_config)
 
     @inlineCallbacks
     def run(self):
@@ -197,4 +229,3 @@
     def destroy(self):
         yield self.stop()
         yield deferToThread(_lxc_destroy, self.container_name)
-        os.unlink(self.lxc_config)

=== modified file 'ensemble/lib/lxc/data/ensemble-create'
--- ensemble/lib/lxc/data/ensemble-create	2011-09-13 23:16:26 +0000
+++ ensemble/lib/lxc/data/ensemble-create	2011-09-13 23:16:26 +0000
@@ -13,21 +13,25 @@
 
     if [ $ENSEMBLE_ORIGIN = "ppa" ]; then
         echo "Using Ensemble PPA for container"
-        ENSEMBLE_SOURCE="ppa:ensemble/ppa"
-    elif [ $ENSEMBLE_ORIGIN = "lp" ]; then
-        echo "Using Ensemble Branch $ensemble_source"
+    elif [ $ENSEMBLE_ORIGIN = "branch" ]; then
+        echo "Using Ensemble Branch $ENSEMBLE_SOURCE"
     elif [ $ENSEMBLE_ORIGIN = "distro" ]; then
         echo "Using Ensemble distribution packages"
+    else
+        echo "Unknown Ensemble origin policy $ENSEMBLE_ORIGIN: expected [ppa|branch|disto]"
+        exit 1
     fi
 
     echo "Setting up ensemble in container"
     apt-get install --force-yes -y bzr tmux
 
     if [ $ENSEMBLE_ORIGIN = "ppa" ]; then
-        apt-add-repository $ENSEMBLE_SOURCE
+        # the echo forces an enter to get around the interactive
+        # prompt in apt-add-repository
+        echo y | apt-add-repository ppa:ensemble/ppa
         apt-get update
         apt-get install --force-yes -y ensemble python-txzookeeper
-    elif [ $ENSEMBLE_ORIGIN = "lp" ]; then
+    elif [ $ENSEMBLE_ORIGIN = "branch" ]; then
         apt-get install --force-yes -y python-txzookeeper
         mkdir /usr/lib/ensemble
         bzr branch $ENSEMBLE_SOURCE /usr/lib/ensemble/ensemble
@@ -45,8 +49,8 @@
     ENSEMBLE_ORIGIN=distro
 fi
 
-if [ -z "$ZOOKEEPER_ADDRESS" ]; then
-    echo "'ZOOKEEPER_ADDRESS' is required"
+if [ -z "$ENSEMBLE_ZOOKEEPER" ]; then
+    echo "'ENSEMBLE_ZOOKEEPER' is required"
     exit 1
 fi
 

=== modified file 'ensemble/lib/lxc/tests/test_lxc.py'
--- ensemble/lib/lxc/tests/test_lxc.py	2011-09-13 23:16:26 +0000
+++ ensemble/lib/lxc/tests/test_lxc.py	2011-09-13 23:16:26 +0000
@@ -1,11 +1,12 @@
 import os
+import tempfile
 
 from twisted.internet.defer import inlineCallbacks
 from twisted.internet.threads import deferToThread
 
 from ensemble.lib.lxc import (_lxc_start, _lxc_stop, _lxc_create,
                               _lxc_wait, _lxc_ls, _lxc_destroy,
-                              LXCContainer)
+                              LXCContainer, get_containers)
 from ensemble.lib.testing import TestCase, get_test_zookeeper_address
 
 
@@ -15,9 +16,8 @@
     return "TEST_LXC=1 to include lxc tests"
 
 
-BASE_PATH = os.path.normpath(
-    os.path.abspath(
-        os.path.join(os.path.dirname(__file__), "..", "data")))
+DATA_PATH = os.path.abspath(
+    os.path.join(os.path.dirname(__file__), "..", "data"))
 
 
 DEFAULT_CONTAINER = "lxc_test"
@@ -27,6 +27,25 @@
     timeout = 240
     skip = run_lxc_tests()
 
+    def setUp(self):
+        self.config = self.make_config()
+
+        @self.addCleanup
+        def remove_config():
+            if os.path.exists(self.config):
+                os.unlink(self.config)
+
+    def make_config(self, network_name="virbr0"):
+        lxc_config = os.path.join(DATA_PATH, "lxc.conf")
+        template = open(lxc_config, "r").read()
+
+        fd, output_fn = tempfile.mkstemp(suffix=".conf")
+        output_config = open(output_fn, "w")
+        output_config.write(template % {"network_name": network_name})
+        output_config.close()
+
+        return output_fn
+
     def cleanContainer(self, container_name):
         if os.path.exists("/var/lib/lxc/%s" % container_name):
             _lxc_stop(container_name)
@@ -35,8 +54,7 @@
     def test_lxc_create(self):
         self.addCleanup(self.cleanContainer, DEFAULT_CONTAINER)
 
-        _lxc_create(DEFAULT_CONTAINER,
-                   config_file=os.path.join(BASE_PATH, "lxc.conf"))
+        _lxc_create(DEFAULT_CONTAINER, config_file=self.config)
 
         # verify we can find the container
         output = _lxc_ls()
@@ -50,8 +68,7 @@
     def test_lxc_start(self):
         self.addCleanup(self.cleanContainer, DEFAULT_CONTAINER)
 
-        _lxc_create(DEFAULT_CONTAINER,
-                   config_file=os.path.join(BASE_PATH, "lxc.conf"))
+        _lxc_create(DEFAULT_CONTAINER, config_file=self.config)
 
         _lxc_start(DEFAULT_CONTAINER)
         _lxc_stop(DEFAULT_CONTAINER)
@@ -59,8 +76,7 @@
     @inlineCallbacks
     def test_lxc_deferred(self):
         self.addCleanup(self.cleanContainer, DEFAULT_CONTAINER)
-        yield deferToThread(_lxc_create, DEFAULT_CONTAINER,
-                            config_file=os.path.join(BASE_PATH, "lxc.conf"))
+        yield deferToThread(_lxc_create, DEFAULT_CONTAINER, config_file=self.config)
         yield deferToThread(_lxc_start, DEFAULT_CONTAINER)
 
     @inlineCallbacks
@@ -68,7 +84,7 @@
         self.addCleanup(self.cleanContainer, DEFAULT_CONTAINER)
 
         c = LXCContainer(DEFAULT_CONTAINER,
-                         dict(zookeeper_address=get_test_zookeeper_address(),
+                         dict(ensemble_zookeeper=get_test_zookeeper_address(),
                               ensemble_origin="distro"))
         self.assertFalse(c.running)
         yield c.run()
@@ -85,7 +101,7 @@
         #network = open(os.path.join(c.rootfs, "..", "config"), "r").read()
         #self.assertIn("lxc.network.link=virbr0", network)
 
-        # check that some of the customise modifications were made
+        # check that some of the customize modifications were made
         self.assertTrue(os.path.exists(os.path.join(
                     c.rootfs, "etc", "ensemble")))
         self.assertTrue(os.path.exists(os.path.join(
@@ -93,12 +109,19 @@
 
         # verify that we have shell variables in rootfs/etc/ensemble.conf
         config = open(os.path.join(c.rootfs, "etc", "ensemble", "ensemble.conf"), "r").read()
-        self.assertIn("ZOOKEEPER_ADDRESS=%s" % get_test_zookeeper_address(), config)
+        self.assertIn("ENSEMBLE_ZOOKEEPER=%s" % get_test_zookeeper_address(), config)
+
+        # verify that we are in containers
+        containers = yield get_containers(None)
+        self.assertEqual(containers[DEFAULT_CONTAINER], True)
 
         # tear it down
         yield c.destroy()
         self.assertFalse(c.running)
 
+        containers = yield get_containers(None)
+        self.assertNotIn(DEFAULT_CONTAINER, containers)
+
         # and its gone
         output = _lxc_ls()
         self.assertNotIn(DEFAULT_CONTAINER, output)
@@ -107,8 +130,7 @@
     def test_lxc_wait(self):
         self.addCleanup(self.cleanContainer, DEFAULT_CONTAINER)
 
-        _lxc_create(DEFAULT_CONTAINER,
-                   config_file=os.path.join(BASE_PATH, "lxc.conf"))
+        _lxc_create(DEFAULT_CONTAINER, config_file=self.config)
 
         _lxc_start(DEFAULT_CONTAINER)
 

=== added file 'ensemble/providers/common/files.py'
--- ensemble/providers/common/files.py	1970-01-01 00:00:00 +0000
+++ ensemble/providers/common/files.py	2011-09-13 23:16:26 +0000
@@ -0,0 +1,40 @@
+"""
+Directory based file storage (for lxc and dummy).
+"""
+
+import os
+
+from twisted.internet.defer import fail, succeed
+from ensemble.errors import FileNotFound
+
+
+class FileStorage(object):
+
+    def __init__(self, path):
+        self._path = path
+
+    def get(self, name):
+        file_path = os.path.join(
+            self._path, *filter(None, name.split("/")))
+        if os.path.exists(file_path):
+            return succeed(open(file_path))
+        return fail(FileNotFound(file_path))
+
+    def put(self, remote_path, file_object):
+        store_path = os.path.join(
+            self._path, *filter(None, remote_path.split("/")))
+        store_path = os.path.abspath(store_path)
+        if not store_path.startswith(self._path):
+            return fail(AssertionError("Invalid Remote Path %s" % remote_path))
+
+        parent_store_path = os.path.dirname(store_path)
+        if not os.path.exists(parent_store_path):
+            os.makedirs(parent_store_path)
+        with open(store_path, "wb") as f:
+            f.write(file_object.read())
+        return succeed(True)
+
+    def get_url(self, name):
+        file_path = os.path.abspath(os.path.join(
+            self._path, *filter(None, name.split("/"))))
+        return "file://%s" % file_path

=== added file 'ensemble/providers/common/tests/test_files.py'
--- ensemble/providers/common/tests/test_files.py	1970-01-01 00:00:00 +0000
+++ ensemble/providers/common/tests/test_files.py	2011-09-13 23:16:26 +0000
@@ -0,0 +1,62 @@
+import os
+from StringIO import StringIO
+
+from twisted.internet.defer import inlineCallbacks
+
+from ensemble.lib.testing import TestCase
+from ensemble.providers.common.files import FileStorage
+
+from ensemble.errors import FileNotFound
+
+
+class FileStorageTest(TestCase):
+
+    def setUp(self):
+        super(FileStorageTest, self).setUp()
+        self.storage_dir = self.makeDir()
+        self.storage = FileStorage(self.storage_dir)
+
+    def test_get_file_non_existent(self):
+        return self.failUnlessFailure(self.storage.get("/abc"), FileNotFound)
+
+    def test_get_url(self):
+        url = self.storage.get_url("/abc.txt")
+        self.assertEqual(url, "file://%s/abc.txt" % self.storage_dir)
+
+    @inlineCallbacks
+    def test_get_file(self):
+        path = os.path.join(self.storage_dir, "abc.txt")
+        self.makeFile("content", path=path)
+        fh = yield self.storage.get("/abc.txt")
+        self.assertEqual(fh.read(), "content")
+
+    @inlineCallbacks
+    def test_put_and_get_file(self):
+        file_obj = StringIO("rabbits")
+        yield self.storage.put("/magic/beans.txt", file_obj)
+        fh = yield self.storage.get("/magic/beans.txt")
+        self.assertEqual(fh.read(), "rabbits")
+
+    @inlineCallbacks
+    def test_put_same_path_multiple(self):
+        file_obj = StringIO("rabbits")
+        yield self.storage.put("/magic/beans.txt", file_obj)
+        file_obj = StringIO("elephant")
+        yield self.storage.put("/magic/beans.txt", file_obj)
+        fh = yield self.storage.get("/magic/beans.txt")
+        self.assertEqual(fh.read(), "elephant")
+
+    @inlineCallbacks
+    def test_put_file_relative_path(self):
+        file_obj = StringIO("moon")
+        yield self.storage.put("zebra/../zoo/reptiles/snakes.txt", file_obj)
+        fh = yield self.storage.get("/zoo/reptiles/snakes.txt")
+        self.assertEqual(fh.read(), "moon")
+
+    def test_put_file_invalid_relative_path(self):
+        """Relative paths work as long as their contained in the storage path.
+        """
+        file_obj = StringIO("moon")
+        return self.failUnlessFailure(
+            self.storage.put("../../etc/profile.txt", file_obj),
+            AssertionError)

=== modified file 'ensemble/providers/dummy.py'
--- ensemble/providers/dummy.py	2011-08-23 12:32:25 +0000
+++ ensemble/providers/dummy.py	2011-09-13 23:16:26 +0000
@@ -7,8 +7,10 @@
 from txzookeeper import ZookeeperClient
 
 from ensemble.errors import (
-    EnvironmentNotFound, FileNotFound, MachinesNotFound, ProviderError)
+    EnvironmentNotFound, MachinesNotFound, ProviderError)
+
 from ensemble.machine import ProviderMachine
+from ensemble.providers.common.files import FileStorage
 
 log = logging.getLogger("ensemble.providers")
 
@@ -179,40 +181,3 @@
         if self._machines:
             return succeed(self._machines[:1])
         return fail(EnvironmentNotFound("not bootstrapped"))
-
-
-class FileStorage(object):
-
-    def __init__(self, path):
-        self._path = path
-
-    @property
-    def path(self):
-        # not part of the api, just for test cleanup
-        return self._path
-
-    def get_url(self, name):
-        file_path = os.path.abspath(os.path.join(
-            self._path, *filter(None, name.split("/"))))
-        return "file://%s" % file_path
-
-    def get(self, name):
-        file_path = os.path.join(
-            self._path, *filter(None, name.split("/")))
-        if os.path.exists(file_path):
-            return succeed(open(file_path))
-        return fail(FileNotFound(file_path))
-
-    def put(self, remote_path, file_object):
-        store_path = os.path.join(
-            self._path, *filter(None, remote_path.split("/")))
-        store_path = os.path.abspath(store_path)
-        if not store_path.startswith(self._path):
-            return fail(AssertionError("Invalid Remote Path %s" % remote_path))
-
-        parent_store_path = os.path.dirname(store_path)
-        if not os.path.exists(parent_store_path):
-            os.makedirs(parent_store_path)
-        with open(store_path, "wb") as f:
-            f.write(file_object.read())
-        return succeed(True)

=== modified file 'ensemble/providers/tests/test_dummy.py'
--- ensemble/providers/tests/test_dummy.py	2011-08-22 23:03:03 +0000
+++ ensemble/providers/tests/test_dummy.py	2011-09-13 23:16:26 +0000
@@ -1,10 +1,10 @@
 from cStringIO import StringIO
-import os
+
 import zookeeper
 
 from twisted.internet.defer import inlineCallbacks
 
-from ensemble.errors import FileNotFound, ProviderError
+from ensemble.errors import ProviderError
 from ensemble.machine import ProviderMachine
 from ensemble.providers.dummy import MachineProvider, DummyMachine
 
@@ -152,79 +152,14 @@
         self.assertEqual(exposed_ports,
                          set([(53, 'udp'), (80, 'tcp'), (443, 'tcp')]))
 
-
-class DummyProviderFileStorageTest(TestCase):
-
-    def setUp(self):
-        super(DummyProviderFileStorageTest, self).setUp()
-        directory = self.makeDir()
-        self.provider = MachineProvider(
-            "foo", {"storage-directory": directory})
-        self.storage = self.provider.get_file_storage()
-
-    def test_get_file_non_existent(self):
-        return self.failUnlessFailure(self.storage.get("/abc"), FileNotFound)
-
+    @inlineCallbacks
     def test_file_storage_returns_same_storage(self):
         """Multiple invocations of MachineProvider.get_file_storage use the
         same path.
         """
-        provider = MachineProvider("foo", {})
-        storage1 = provider.get_file_storage()
-        storage2 = provider.get_file_storage()
-        self.assertEqual(storage1.path, storage2.path)
-
-    @inlineCallbacks
-    def test_file_storage_uses_configured_path(self):
         file_obj = StringIO("rabbits")
-        yield self.storage.put("/magic/beans.txt", file_obj)
+        storage = self.provider.get_file_storage()
+        yield storage.put("/magic/beans.txt", file_obj)
         storage2 = self.provider.get_file_storage()
         fh = yield storage2.get("/magic/beans.txt")
         self.assertEqual(fh.read(), "rabbits")
-
-    def test_get_url(self):
-        url = self.storage.get_url("/abc.txt")
-        self.assertEqual(url, "file://%s/abc.txt" % self.storage.path)
-
-    @inlineCallbacks
-    def test_get_file(self):
-        path = os.path.join(self.storage.path, "abc.txt")
-        self.makeFile("content", path=path)
-        fh = yield self.storage.get("/abc.txt")
-        self.assertEqual(fh.read(), "content")
-
-    @inlineCallbacks
-    def test_put_and_get_file(self):
-        file_obj = StringIO("rabbits")
-        yield self.storage.put("/magic/beans.txt", file_obj)
-        fh = yield self.storage.get("/magic/beans.txt")
-        self.assertEqual(fh.read(), "rabbits")
-
-    @inlineCallbacks
-    def test_put_same_path_multiple(self):
-        file_obj = StringIO("rabbits")
-        yield self.storage.put("/magic/beans.txt", file_obj)
-        file_obj = StringIO("elephant")
-        yield self.storage.put("/magic/beans.txt", file_obj)
-        fh = yield self.storage.get("/magic/beans.txt")
-        self.assertEqual(fh.read(), "elephant")
-
-    @inlineCallbacks
-    def test_put_file_relative_path(self):
-        file_obj = StringIO("moon")
-        yield self.storage.put("zebra/../zoo/reptiles/snakes.txt", file_obj)
-        fh = yield self.storage.get("/zoo/reptiles/snakes.txt")
-        self.assertEqual(fh.read(), "moon")
-
-    def test_put_file_invalid_relative_path(self):
-        """Relative paths work as long as their contained in the storage path.
-        """
-        file_obj = StringIO("moon")
-        return self.failUnlessFailure(
-            self.storage.put("../../etc/profile.txt", file_obj),
-            AssertionError)
-
-    def test_provider_random_storage_dir(self):
-        provider = MachineProvider("foo", {})
-        storage = provider.get_file_storage()
-        self.assertTrue(os.path.exists(storage.path))