Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when starting containers with rogue containers already present #381

Merged
merged 2 commits into from
Mar 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 31 additions & 19 deletions remoteappmanager/docker/container_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import string
from urllib.parse import urlparse
import uuid
import random

from docker.errors import APIError, NotFound
from escapism import escape
Expand Down Expand Up @@ -295,22 +296,21 @@ def _start_container(self,
raise e

self.log.info('Got container image: {}'.format(image_name))
# build the dictionary of keyword arguments for create_container
container_name = _generate_container_name(self.realm,
user_name,
mapping_id)
container_url_id = _generate_container_url_id()

# Check if the container is present. If not, create it
container_info = yield self._get_container_info(container_name)
# Check if the container is present.
containers = yield self.containers_from_mapping_id(
user_name, mapping_id)

if container_info is not None:
# Make sure we stop and remove the container if by any change
# is already there. This will guarantee a fresh start every time.
if len(containers) != 0:
# we assume only one present. The API is made for potential
# of multiple instances.
container = containers[0]

# Make sure we stop and remove it if by any chance is already
# there. This will guarantee a fresh start every time.
self.log.info('Container for image {} '
'already present. Stopping.'.format(image_name))
container_id = container_info["Id"]
yield self.stop_and_remove_container(container_id)
yield self.stop_and_remove_container(container.docker_id)

# Data volume binding to be used with Docker Client
# volumes = {volume_source: {'bind': volume_target,
Expand All @@ -332,17 +332,19 @@ def _start_container(self,
self.log.error('Path(s) does not exist, not mounting:\n%s',
'\n'.join(volumes.keys() - filtered_volumes.keys()))

# Info for debugging
self.log.info(
'Mounting these volumes: \n%s',
'\n'.join('{0} -> {1}'.format(source, target['bind'])
for source, target in filtered_volumes.items()))

container_url_id = _generate_container_url_id()
container_urlpath = without_end_slash(
url_path_join(base_urlpath,
"containers",
container_url_id))

container_name = _generate_container_name(self.realm,
user_name,
mapping_id)
create_kwargs = dict(
image=image_name,
name=container_name,
Expand Down Expand Up @@ -651,7 +653,13 @@ def _generate_container_name(realm, user_name, mapping_id):

Return
------
A string combining the three parameters in an appropriate container name
A string combining the three parameters in an appropriate container name,
plus a random token to prevent collisions with a similarly named rogue
container.

NOTE: the container name is not meant for parsing. It's only for human
consumption in the docker list. All information and all searching should
be extracted from labels.
"""
escaped_realm = escape(realm,
safe=_CONTAINER_SAFE_CHARS,
Expand All @@ -662,10 +670,14 @@ def _generate_container_name(realm, user_name, mapping_id):
escaped_mapping_id = escape(mapping_id,
safe=_CONTAINER_SAFE_CHARS,
escape_char=_CONTAINER_ESCAPE_CHAR)

return "{}-{}-{}".format(escaped_realm,
escaped_user_name,
escaped_mapping_id)
random_token = ''.join(random.choice(string.ascii_lowercase)
for _ in range(10))

return "{}-{}-{}-{}".format(escaped_realm,
escaped_user_name,
escaped_mapping_id,
random_token
)


def _generate_container_url_id():
Expand Down
2 changes: 1 addition & 1 deletion remoteappmanager/docker/tests/test_container_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def test_start_already_present_container(self):
mock_client = self.mock_docker_client

result = yield self.manager.start_container(
"username",
"user_name",
"image_name1",
"mapping_id",
"/base/url",
Expand Down