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

Mccalluc/remove old bits #195

Merged
merged 10 commits into from
Sep 24, 2018
Merged
Show file tree
Hide file tree
Changes from 8 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
2 changes: 0 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,4 @@ dist/
*.egg-info/
build/

django_docker_cloudformation.pem

database.sqlite
6 changes: 1 addition & 5 deletions README-USERS.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ There are essentially two ways of passing input to a container on startup:
By mounting a file from the host filesystem and by passing environment variables.
The method used depend on the parameters you use for `DockerClientSpec`.

`do_input_json_file` (boolean):
- Input is serialized to a JSON file and saved in a tmp directory.
- If Docker Engine is on a remote host, requires SSH access.

`do_input_json_envvar` (boolean):
- Instead of writing a file, JSON is stored in an environment variable.
- There is limit to the size of environment variables: Typically 2M, but it could be lower.
Expand Down Expand Up @@ -93,7 +89,7 @@ and not just have installed it via pip.)
```
>>> from django_docker_engine.docker_utils import (
... DockerClientRunWrapper, DockerClientSpec, DockerContainerSpec)
>>> client_spec = DockerClientSpec(None, do_input_json_envvar=True)
>>> client_spec = DockerClientSpec(do_input_json_envvar=True)
>>> client = DockerClientRunWrapper(client_spec)

# First, confirm no containers are already running:
Expand Down
2 changes: 1 addition & 1 deletion demo_path_routing_auth/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@


client = DockerClientRunWrapper(
DockerClientSpec(None, do_input_json_envvar=True),
DockerClientSpec(do_input_json_envvar=True),
mem_limit_mb=35 # TODO: Hard-coded limit for now
# We could also get the limit by starting a container and then:
# sdk.list()[0].stats(stream=False)['memory_stats']['limit']
Expand Down
4 changes: 2 additions & 2 deletions django_docker_engine/container_managers/base.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import abc
import sys

if sys.version_info >= (3, 4):
if sys.version_info >= (3, 4): # pragma: no cover
ABC = abc.ABC
else:
ABC = abc.ABCMeta('ABC', (), {})


class BaseManager(ABC):
class BaseManager(ABC): # pragma: no cover
"""
This provides an interface which satisfies both local and remote Docker use.
The methods are based on a subset of those provided by
Expand Down
97 changes: 3 additions & 94 deletions django_docker_engine/container_managers/docker_engine.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
import abc
import os
import re
import subprocess
import sys
from datetime import datetime
from distutils import dir_util

import docker

Expand Down Expand Up @@ -44,11 +38,8 @@ class DockerEngineManager(BaseManager):

def __init__(
self,
data_dir, # TODO: Only needed if passing input.json as file?
root_label,
client=docker.from_env(),
pem=None,
ssh_username=None
):
"""
:param string data_dir: Only needed if input is passed as file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove param comment

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Expand All @@ -57,38 +48,22 @@ def __init__(
The DOCKER_HOST environment variable specifies where the Docker Engine
is. To override this, create a DockerClient with the correct base_url
and provide it here.
:param string pem: Only needed if input is passed as file, and
the Docker Engine is remote.
:param string ssh_username: Only needed if input is passed as file, and
the Docker Engine is remote.
"""
self._base_url = client.api.base_url
self._containers_client = client.containers
self._images_client = client.images
self._volumes_client = client.volumes
self._data_dir = data_dir
self._root_label = root_label

remote_host = self._get_base_url_remote_host()
if remote_host:
self.host_files = _RemoteHostFiles(
host=remote_host,
pem=pem,
ssh_username=ssh_username)
elif self._is_base_url_local():
self.host_files = _LocalHostFiles()
else:
raise RuntimeError(
'Unexpected client base_url: %s', self._base_url)

def _get_base_url_remote_host(self):
remote_host_match = re.match(r'^http://([^:]+):\d+$', self._base_url)
if remote_host_match:
return remote_host_match.group(1)

def _is_base_url_local(self):
return self._base_url in [
'http+docker://' + host for host in ['localunixsocket', 'localhost']
'http+docker://' + host for host in
['localunixsocket', 'localhost']
]

def run(self, image_name, cmd, **kwargs):
Expand Down Expand Up @@ -139,7 +114,7 @@ def get_url(self, container_name):
"""
remote_host = self._get_base_url_remote_host()
if remote_host:
host = remote_host
host = remote_host # pragma: no cover
elif self._is_base_url_local():
host = 'localhost'
else:
Expand Down Expand Up @@ -203,72 +178,6 @@ def logs(self, container_name):
container = self._containers_client.get(container_name)
return container.logs(timestamps=True)

def _mkdtemp(self):
timestamp = re.sub(r'\W', '_', str(datetime.now()))
tmp_dir = os.path.join(self._data_dir, timestamp)
self.host_files.mkdir_p(tmp_dir)
return tmp_dir


if sys.version_info >= (3, 4):
ABC = abc.ABC
else:
ABC = abc.ABCMeta('ABC', (), {})


class _HostFiles(ABC):
@abc.abstractmethod
def write(self, path, content):
raise NotImplementedError()

@abc.abstractmethod
def mkdir_p(self, path):
raise NotImplementedError()


class _LocalHostFiles(_HostFiles):
def __init__(self):
pass

def write(self, path, content):
with open(path, 'w') as file:
file.write(content)
file.write('\n')
# TODO: For consistency with heredoc in _RemoteHostFiles, add a newline...
# I don't think this hurts with JSON, but not ideal.

def mkdir_p(self, path):
dir_util.mkpath(path)


class _RemoteHostFiles(_HostFiles):
# TODO: Try again with paramiko: https://github.com/paramiko/paramiko/issues/959
def __init__(self, host, pem, ssh_username=None,
strict_host_key_checking=False):
self.host = host
self.pem = pem
self.ssh_username = ssh_username
self.strict_host_key_checking = strict_host_key_checking

def _exec(self, command):
cmd_tokens = ['ssh']
if not self.strict_host_key_checking:
cmd_tokens.append('-oStrictHostKeyChecking=no')
if self.pem:
cmd_tokens.extend(['-i', self.pem])
if self.ssh_username:
cmd_tokens.append('{}@{}'.format(self.ssh_username, self.host))
else:
cmd_tokens.append(self.host)
cmd_tokens.append(command)
subprocess.check_call(cmd_tokens)

def write(self, path, content):
self._exec(
"cat > {} <<'END_CONTENT'\n{}\nEND_CONTENT".format(path, content))

def mkdir_p(self, path):
self._exec('mkdir -p {}'.format(path))

# TODO: At some point we need to be more abstract,
# instead of using the SDK responses directly...
Expand Down
46 changes: 7 additions & 39 deletions django_docker_engine/docker_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,19 @@ def __repr__(self):

class DockerClientSpec():

def __init__(self, data_dir,
do_input_json_file=False,
def __init__(self,
do_input_json_envvar=False,
input_json_url=None):
assert do_input_json_file or do_input_json_envvar or input_json_url,\
'Input must be provided to the container, '\
'either as a json file to mount, '\
'an environment variable containing json, '\
assert do_input_json_envvar or input_json_url,\
'Input must be provided to the container '\
'as an environment variable containing json '\
'or an environment variable containing a url pointing to json'
# Multiple can be specified: The container needs to be able
# Both can be specified: The container needs to be able
# to read from at least one specified source. Limitations:
# - do_input_json_file:
# Requires ssh access if remote
# - do_input_json_envvar:
# Creates potentially problematic huge envvar
# - input_json_url:
# World-readable URL could be an unwanted leak
self.data_dir = data_dir
self.do_input_json_file = do_input_json_file
self.do_input_json_envvar = do_input_json_envvar
self.input_json_url = input_json_url

Expand All @@ -80,9 +74,7 @@ def __init__(self,
manager_class=_DEFAULT_MANAGER,
root_label=_DEFAULT_LABEL):
self._historian = FileHistorian() if historian is None else historian
self._containers_manager = manager_class(None, root_label)
# Some methods of the manager will fail without a data_dir,
# but they shouldn't be called from the read-only client in any event.
self._containers_manager = manager_class(root_label)

def is_live(self, container_name):
try:
Expand Down Expand Up @@ -205,41 +197,24 @@ def __init__(self,
docker_client_spec,
manager_class=_DEFAULT_MANAGER,
root_label=_DEFAULT_LABEL,
pem=None,
ssh_username=None,
mem_limit_mb=float('inf')):
super(DockerClientRunWrapper, self).__init__(
manager_class=manager_class,
root_label=root_label
)
manager_kwargs = {}
if pem:
manager_kwargs['pem'] = pem
if ssh_username:
manager_kwargs['ssh_username'] = ssh_username
self._containers_manager = manager_class(
docker_client_spec.data_dir, root_label, **manager_kwargs)
self._containers_manager = manager_class(root_label, **manager_kwargs)
self.root_label = root_label
self._do_input_json_file = docker_client_spec.do_input_json_file
self._do_input_json_envvar = docker_client_spec.do_input_json_envvar
self._input_json_url = docker_client_spec.input_json_url
self._mem_limit_mb = mem_limit_mb

def _make_directory_on_host(self):
# TODO: This should only be run locally, and even then...?
return self._containers_manager._mkdtemp()

def _make_volume_on_host(self):
return self._containers_manager.create_volume().name

def _write_input_to_host(self, input): # noqa: A002
host_input_dir = self._make_directory_on_host()
# The host filename "input.json" is arbitrary.
host_input_path = os.path.join(host_input_dir, 'input.json')
content = json.dumps(input)
self._containers_manager.host_files.write(host_input_path, content)
return host_input_path

def run(self, container_spec):
"""
Run a given ContainerSpec. Returns the url for the container,
Expand Down Expand Up @@ -270,10 +245,6 @@ def run(self, container_spec):
# https://github.com/docker/docker-py/issues/1510

volume_spec = []
if self._do_input_json_file:
volume_spec.append({
'host': self._write_input_to_host(container_spec.input),
'bind': container_spec.container_input_path})

for directory in container_spec.extra_directories:
assert os.path.isabs(directory), \
Expand Down Expand Up @@ -322,6 +293,3 @@ def run(self, container_spec):
mem_reservation='{}M'.format(new_mem_reservation_mb),
)
return self.lookup_container_url(container_spec.container_name)

def _get_data_dir(self):
return self._containers_manager._data_dir
Loading