Stopping service loses access to containers

package.json

@@ -10,7 +10,7 @@
     },
     "dependencies": {
         "bower": "*",
-        "configurable-http-proxy": "git://github.com/jupyterhub/configurable-http-proxy.git#f54c6a46a235f17cb6c36046a913d37fa45ec95b"
+        "configurable-http-proxy": "git://github.com/jupyterhub/configurable-http-proxy.git#e52764b9e742904c0982739b1479f3bf88dad3f4"
     },
     "devDependencies": {
 	    "jshint": "*",

remoteappmanager/application.py

@@ -1,7 +1,9 @@
 from tornado import web
 
 from remoteappmanager.base_application import BaseApplication
-from remoteappmanager.handlers.api import HomeHandler
+from remoteappmanager.handlers.api import (
+    HomeHandler, RegisterContainerHandler)
+from remoteappmanager.utils import url_path_join, without_end_slash
 from remoteappmanager import webapi
 
 
@@ -15,7 +17,10 @@ def _webapi_resources(self):
     def _web_handlers(self):
         base_urlpath = self.command_line_config.base_urlpath
         return [
+            (without_end_slash(
+                url_path_join(base_urlpath, "containers", "([a-z0-9_]*)")
+            )+"/?", RegisterContainerHandler),
             (base_urlpath, HomeHandler),
-            (base_urlpath.rstrip('/'),
-             web.RedirectHandler, {"url": base_urlpath}),
+            (base_urlpath.rstrip('/'), web.RedirectHandler, {
+                "url": base_urlpath}),
         ]

remoteappmanager/docker/container_manager.py

@@ -14,7 +14,8 @@
 from remoteappmanager.logging.logging_mixin import LoggingMixin
 from remoteappmanager.utils import (
     url_path_join,
-    without_end_slash)
+    without_end_slash,
+    deprecated)
 
 from tornado import gen
 from traitlets import (
@@ -38,6 +39,11 @@ class OperationInProgress(Exception):
     pass
 
 
+class MultipleResultsFound(Exception):
+    """Raised when we are asking for a specific container, but more than
+    one result is found."""
+
+
 class ContainerManager(LoggingMixin):
 
     #: The container (not host) port. We decided it's 8888 by default. It will
@@ -163,6 +169,7 @@ def stop_and_remove_container(self, container_id):
             self._stop_pending.remove(container_id)
 
     @gen.coroutine
+    @deprecated
     def containers_from_mapping_id(self, user_name, mapping_id):
         """Returns the currently running containers for a given user and
         mapping_id.
@@ -178,30 +185,22 @@ def containers_from_mapping_id(self, user_name, mapping_id):
         ------
         A list of Container objects, or an empty list if nothing is found.
         """
-        containers = yield self.containers_with_labels({
-                SIMPHONY_NS_RUNINFO.user: user_name,
-                SIMPHONY_NS_RUNINFO.mapping_id: mapping_id,
-                SIMPHONY_NS_RUNINFO.realm: self.realm
-            })
-        return containers
+        return (yield self.find_containers(user_name=user_name,
+                                           mapping_id=mapping_id))
 
     @gen.coroutine
+    @deprecated
     def container_from_url_id(self, url_id):
         """Retrieves and returns the container by its url_id, if present.
         If not present, returns None.
         """
-        containers = yield self.containers_with_labels({
-                SIMPHONY_NS_RUNINFO.url_id: url_id,
-                SIMPHONY_NS_RUNINFO.realm: self.realm
-            })
-        return containers[0] if len(containers) else None
+        return (yield self.find_container(url_id=url_id))
 
     @gen.coroutine
+    @deprecated
     def running_containers(self):
         """Returns all the running containers"""
-        containers = yield self.containers_with_labels({
-                SIMPHONY_NS_RUNINFO.realm: self.realm
-            })
+        containers = yield self.find_containers()
 
         return containers
 
@@ -253,6 +252,53 @@ def containers_from_filters(self, filters):
 
         return containers
 
+    @gen.coroutine
+    def find_containers(self,
+                        *,
+                        url_id=None,
+                        mapping_id=None,
+                        user_name=None):
+        """Finds and returns containers matching all the specified arguments.
+        """
+        labels = {
+            SIMPHONY_NS_RUNINFO.realm: self.realm
+        }
+
+        if url_id is not None:
+            labels[SIMPHONY_NS_RUNINFO.url_id] = url_id
+
+        if mapping_id is not None:
+            labels[SIMPHONY_NS_RUNINFO.mapping_id] = mapping_id
+
+        if user_name is not None:
+            labels[SIMPHONY_NS_RUNINFO.user] = user_name
+
+        containers = yield self.containers_with_labels(labels)
+
+        return containers
+
+    @gen.coroutine
+    def find_container(self,
+                       *,
+                       url_id=None,
+                       mapping_id=None,
+                       user_name=None):
+        """Find and returns a container matching the specified
+        arguments.
+
+        Returns the found container or None. If multiple containers
+        match the query, it will raise MultipleResultsFound"""
+        containers = yield self.find_containers(url_id=url_id,
+                                                mapping_id=mapping_id,
+                                                user_name=user_name)
+        if len(containers) > 1:
+            raise MultipleResultsFound()
+
+        if len(containers) == 1:
+            return containers[0]
+
+        return None
+
     @gen.coroutine
     def image(self, image_id_or_name):
         """Returns the Image object associated to a given id

remoteappmanager/docker/tests/test_container.py

@@ -92,7 +92,7 @@ def test_from_docker_dict_without_public_port(self):
             name='/container_name1',
             image_name='image_name1',
             image_id='image_id1',
-            user="user_name",
+            user="username",
             ip='0.0.0.0',
             port=80,
             url_id="url_id",
@@ -113,7 +113,7 @@ def test_from_docker_dict_inspect_container(self):
             name='/myrealm-username-mapping_5Fid',
             image_name='image_name1',
             image_id='image_id1',
-            user="user_name",
+            user="username",
             ip='0.0.0.0',
             port=666,
             url_id="url_id",

remoteappmanager/docker/tests/test_container_manager.py

@@ -58,13 +58,13 @@ def test_start_stop(self):
     @gen_test
     def test_containers_from_mapping_id(self):
         """ Test containers_for_mapping_id returns a list of Container """
-        result = yield self.manager.containers_from_mapping_id("user_name",
+        result = yield self.manager.containers_from_mapping_id("username",
                                                                "mapping_id")
         expected = Container(docker_id='container_id1',
                              mapping_id="mapping_id",
                              name='/myrealm-username-mapping_5Fid',
                              image_name='image_name1',  # noqa
-                             user="user_name",
+                             user="username",
                              image_id='image_id1',
                              ip='127.0.0.1',
                              port=666,
@@ -83,7 +83,7 @@ def test_containers_from_url_id(self):
                              mapping_id="mapping_id",
                              name='/myrealm-username-mapping_5Fid',
                              image_name='image_name1',  # noqa
-                             user="user_name",
+                             user="username",
                              image_id='image_id1',
                              ip='127.0.0.1',
                              port=666,
@@ -162,7 +162,7 @@ def test_start_already_present_container(self):
         mock_client = self.mock_docker_client
 
         result = yield self.manager.start_container(
-            "user_name",
+            "username",
             "image_name1",
             "mapping_id",
             "/base/url",

remoteappmanager/environment_config.py

@@ -20,7 +20,6 @@ def parse_config(self):
         """Parses the environment variables, and assign their
         values to our local traits.
         """
-
         for traitlet_name in self.traits().keys():
             envname = traitlet_name.upper()
             setattr(self, traitlet_name,

remoteappmanager/handlers/api.py

@@ -1,7 +1,8 @@
 from .home_handler import HomeHandler  # noqa
 from .base_handler import BaseHandler  # noqa
+from .register_container_handler import RegisterContainerHandler  # noqa
 from .admin.admin_home_handler import AdminHomeHandler  # noqa
-from .admin.containers_handler import ContainersHandler # noqa
-from .admin.users_handler import UsersHandler # noqa
+from .admin.containers_handler import ContainersHandler  # noqa
+from .admin.users_handler import UsersHandler  # noqa
 from .admin.applications_handler import ApplicationsHandler  # noqa
 from .admin.accounting_handler import AccountingHandler  # noqa

remoteappmanager/handlers/register_container_handler.py

@@ -0,0 +1,38 @@
+from tornado import gen, web
+
+from remoteappmanager.handlers.base_handler import BaseHandler
+
+
+class RegisterContainerHandler(BaseHandler):
+    """This handler intercepts calls that filter through the reverse
+    proxy and sets up the redirection if there's a receiving container
+    accepting the connection.
+
+    It will extract the url_id from the request, lookup if the appropriate
+    container is actually running, and seed the reverse proxy with the
+    appropriate data.
+    """
+
+    @web.authenticated
+    @gen.coroutine
+    def get(self, url_id):
+        container_manager = self.application.container_manager
+
+        container = yield container_manager.find_container(
+            user_name=self.current_user.name,
+            url_id=url_id)
+
+        if container is not None:
+            try:
+                yield self.application.reverse_proxy.register(
+                    container.urlpath,
+                    container.host_url)
+            except Exception:
+                self.log.exception(
+                    "Could not register reverse "
+                    "proxy for id {} in RegisterContainerHandler".format(
+                        url_id))
+            else:
+                self.redirect(self.request.uri)
+
+        raise web.HTTPError(404)

remoteappmanager/handlers/tests/test_register_container_handler.py

@@ -0,0 +1,61 @@
+from remoteappmanager.tests import utils
+from remoteappmanager.tests.mocking import dummy
+from remoteappmanager.tests.temp_mixin import TempMixin
+from remoteappmanager.tests.utils import mock_coro_factory
+
+
+class TestRegisterContainerHandler(TempMixin, utils.AsyncHTTPTestCase):
+    def get_app(self):
+        app = dummy.create_application()
+        app.hub.verify_token.return_value = {
+            'pending': None,
+            'name': app.settings['user'],
+            'admin': False,
+            'server': app.settings['base_urlpath']}
+        return app
+
+    def test_absent_url_id(self):
+        res = self.fetch("/user/username/containers/99999/",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         }
+                         )
+
+        self.assertEqual(res.code, 404)
+
+    def test_present_url_id(self):
+        res = self.fetch("/user/username/containers/url_id",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         },
+                         follow_redirects=False
+                         )
+
+        self.assertEqual(res.code, 302)
+        self.assertTrue(self._app.reverse_proxy.register.called)
+
+    def test_failed_auth(self):
+        self._app.hub.verify_token.return_value = {}
+        res = self.fetch("/user/username/containers/url_id",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         },
+                         follow_redirects=False
+                         )
+
+        # It will be sent to the login instead
+        self.assertFalse(self._app.reverse_proxy.register.called)
+        self.assertEqual(res.code, 302)
+
+    def test_failure_of_reverse_proxy(self):
+        self._app.reverse_proxy.register = mock_coro_factory(
+            side_effect=Exception("BOOM"))
+
+        res = self.fetch("/user/username/containers/url_id",
+                         headers={
+                             "Cookie": "jupyter-hub-token-username=foo"
+                         },
+                         follow_redirects=False
+                         )
+
+        self.assertEqual(res.code, 404)

remoteappmanager/tests/mocking/dummy.py

@@ -221,7 +221,7 @@ def create_container_manager(params=None):
     manager : remoteappmanager.docker.container_manager.ContainerManager
     '''
     if params is None:
-        params = {'docker_config': {}}
+        params = {'docker_config': {}, "realm": "myrealm"}
 
     manager = ContainerManager(**params)
     manager._docker_client._sync_client = create_docker_client()

remoteappmanager/tests/mocking/virtual/docker_client.py

@@ -74,12 +74,12 @@ def get_fake_container_states(num=3):
 
 
 def get_fake_container_labels(num=3):
-    samples = cycle(({SIMPHONY_NS_RUNINFO.user: 'user_name',
+    samples = cycle(({SIMPHONY_NS_RUNINFO.user: 'username',
                       SIMPHONY_NS_RUNINFO.mapping_id: 'mapping_id',
                       SIMPHONY_NS_RUNINFO.url_id: 'url_id',
                       SIMPHONY_NS_RUNINFO.realm: 'myrealm',
                       SIMPHONY_NS_RUNINFO.urlpath: '/user/username/containers/url_id'},  # noqa
-                     {SIMPHONY_NS_RUNINFO.user: 'user_name'},
+                     {SIMPHONY_NS_RUNINFO.user: 'username'},
                      {}))
     return tuple(next(samples) for _ in range(num))
 

remoteappmanager/utils.py

@@ -1,4 +1,6 @@
 import inspect
+from functools import wraps
+import warnings
 
 
 def url_path_join(*pieces):
@@ -99,3 +101,13 @@ def remove_quotes(s):
         return s[1:-1]
 
     return s
+
+
+def deprecated(func):
+    """Decorator. Marks a function/method as deprecated."""
+    @wraps(func)
+    def _deprecated(*args, **kwargs):
+        warnings.warn("Deprecation warning: {}".format(func.__name__))
+        return func(*args, **kwargs)
+
+    return _deprecated