EbookFoundation
/
readthedocs.org
mirror of https://github.com/EbookFoundation/readthedocs.org.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Save docker image hash to consider when auto wiping the environment (#3793) 

* Remove obsolete code

Now, we can access `self.config.build_image` directly.

* Move container_image selection to the init

At initialization time we have the project and we already know if the
project has the build image override so we can decide at that point
and save it as a instance attribute.

Then we can use this values from other places inside the same class.

* Save Docker Image hash in readthedocs-environment.json

The hash is used to know if the environment is obsolete and auto-wipe
it if necessary.

* Simplify the class naming

* Save the image hash in the json file

* Lint

* Remove invalid properties from YAML config in tests

* Add test for save_environment_json
humitos/resolver/username-regex
Manuel Kaufmann 2018-03-23 12:22:05 -05:00 committed by Anthony
parent 233febf484
commit 7ac7dfc88f
3 changed files with 119 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
readthedocs/doc_builder/environments.py
View File

@ -625,6 +625,8 @@ class DockerBuildEnvironment(BuildEnvironment):
)
if self.config and self.config.build_image:
self.container_image = self.config.build_image
if self.project.container_image:
self.container_image = self.project.container_image
if self.project.container_mem_limit:
self.container_mem_limit = self.project.container_mem_limit
if self.project.container_time_limit:
@ -786,6 +788,13 @@ class DockerBuildEnvironment(BuildEnvironment):
mem_limit=self.container_mem_limit,
)
@property
def image_hash(self):
"""Return the hash of the Docker image."""
client = self.get_client()
image_metadata = client.inspect_image(self.container_image)
return image_metadata.get('Id')
@property
def container_id(self):
"""Return id of container if it is valid."""
@ -828,13 +837,13 @@ class DockerBuildEnvironment(BuildEnvironment):
def create_container(self):
"""Create docker container."""
client = self.get_client()
image = self.container_image
if self.project.container_image:
image = self.project.container_image
try:
log.info('Creating Docker container: image=%s', image)
log.info(
'Creating Docker container: image=%s',
self.container_image,
)
self.container = client.create_container(
image=image,
image=self.container_image,
command=('/bin/sh -c "sleep {time}; exit {exit}"'
.format(time=self.container_time_limit,
exit=DOCKER_TIMEOUT_EXIT_CODE)),

11
readthedocs/doc_builder/python_environments.py
View File

@ -131,13 +131,12 @@ class PythonEnvironment(object):
environment_conf = json.load(fpath)
env_python_version = environment_conf['python']['version']
env_build_image = environment_conf['build']['image']
env_build_hash = environment_conf['build']['hash']
except (IOError, TypeError, KeyError, ValueError):
log.error('Unable to read/parse readthedocs-environment.json file')
return False
# TODO: remove getattr when https://github.com/rtfd/readthedocs.org/pull/3339 got merged
build_image = getattr(self.config, 'build_image', self.version.project.container_image) or DOCKER_IMAGE # noqa
build_image = self.config.build_image or DOCKER_IMAGE
# If the user define the Python version just as a major version
# (e.g. ``2`` or ``3``) we won't know exactly which exact version was
# used to create the venv but we can still compare it against the new
@ -145,19 +144,19 @@ class PythonEnvironment(object):
return any([
env_python_version != self.config.python_full_version,
env_build_image != build_image,
env_build_hash != self.build_env.image_hash,
])
def save_environment_json(self):
"""Save on disk Python and build image versions used to create the venv."""
# TODO: remove getattr when https://github.com/rtfd/readthedocs.org/pull/3339 got merged
build_image = getattr(self.config, 'build_image', self.version.project.container_image) or DOCKER_IMAGE # noqa
build_image = self.config.build_image or DOCKER_IMAGE
data = {
'python': {
'version': self.config.python_full_version,
},
'build': {
'image': build_image,
'hash': self.build_env.image_hash,
},
}

113
readthedocs/rtd_tests/tests/test_doc_building.py
View File

@ -9,7 +9,9 @@ from __future__ import (
absolute_import, division, print_function, unicode_literals)
import os.path
import json
import re
import tempfile
import uuid
from builtins import str
@ -837,14 +839,54 @@ class TestDockerBuildCommand(TestCase):
u'Command killed due to excessive memory consumption\n')
class TestAutoWipeEnvironment(TestCase):
class AutoWipeEnvironmentBase(object):
fixtures = ['test_data']
build_env_class = None
def setUp(self):
self.pip = Project.objects.get(slug='pip')
self.version = self.pip.versions.get(slug='0.8')
self.build_env = self.build_env_class(
project=self.pip,
version=self.version,
build={'id': DUMMY_BUILD_ID},
)
def test_save_environment_json(self):
config_data = {
'build': {
'image': '2.0',
},
'python': {
'version': 2.7,
},
}
yaml_config = create_load(config_data)()[0]
config = ConfigWrapper(version=self.version, yaml_config=yaml_config)
python_env = Virtualenv(
version=self.version,
build_env=self.build_env,
config=config,
)
with patch(
'readthedocs.doc_builder.python_environments.PythonEnvironment.environment_json_path',
return_value=tempfile.mktemp(suffix='envjson'),
):
python_env.save_environment_json()
json_data = json.load(open(python_env.environment_json_path()))
expected_data = {
'build': {
'image': 'readthedocs/build:2.0',
'hash': 'a1b2c3',
},
'python': {
'version': 2.7,
},
}
self.assertDictEqual(json_data, expected_data)
def test_is_obsolete_without_env_json_file(self):
yaml_config = create_load()()[0]
@ -854,7 +896,7 @@ class TestAutoWipeEnvironment(TestCase):
exists.return_value = False
python_env = Virtualenv(
version=self.version,
build_env=None,
build_env=self.build_env,
config=config,
)
@ -868,7 +910,7 @@ class TestAutoWipeEnvironment(TestCase):
exists.return_value = True
python_env = Virtualenv(
version=self.version,
build_env=None,
build_env=self.build_env,
config=config,
)
@ -888,10 +930,10 @@ class TestAutoWipeEnvironment(TestCase):
python_env = Virtualenv(
version=self.version,
build_env=None,
build_env=self.build_env,
config=config,
)
env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 3.5}}'
env_json_data = '{"build": {"image": "readthedocs/build:2.0", "hash": "a1b2c3"}, "python": {"version": 3.5}}' # noqa
with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa
exists.return_value = True
self.assertTrue(python_env.is_obsolete)
@ -910,10 +952,10 @@ class TestAutoWipeEnvironment(TestCase):
python_env = Virtualenv(
version=self.version,
build_env=None,
build_env=self.build_env,
config=config,
)
env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 2.7}}'
env_json_data = '{"build": {"image": "readthedocs/build:2.0", "hash": "a1b2c3"}, "python": {"version": 2.7}}' # noqa
with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa
exists.return_value = True
self.assertTrue(python_env.is_obsolete)
@ -936,10 +978,10 @@ class TestAutoWipeEnvironment(TestCase):
python_env = Virtualenv(
version=self.version,
build_env=None,
build_env=self.build_env,
config=config,
)
env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 2.7}}'
env_json_data = '{"build": {"image": "readthedocs/build:2.0", "hash": "a1b2c3"}, "python": {"version": 2.7}}' # noqa
with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa
exists.return_value = True
self.assertTrue(python_env.is_obsolete)
@ -958,10 +1000,55 @@ class TestAutoWipeEnvironment(TestCase):
python_env = Virtualenv(
version=self.version,
build_env=None,
build_env=self.build_env,
config=config,
)
env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 3.5}}'
env_json_data = '{"build": {"image": "readthedocs/build:2.0", "hash": "a1b2c3"}, "python": {"version": 3.5}}' # noqa
with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa
exists.return_value = True
self.assertFalse(python_env.is_obsolete)
def test_is_obsolete_with_json_different_build_hash(self):
config_data = {
'build': {
'image': '2.0',
},
'python': {
'version': 2.7,
},
}
yaml_config = create_load(config_data)()[0]
config = ConfigWrapper(version=self.version, yaml_config=yaml_config)
# Set container_image manually
self.pip.container_image = 'readthedocs/build:2.0'
self.pip.save()
python_env = Virtualenv(
version=self.version,
build_env=self.build_env,
config=config,
)
env_json_data = '{"build": {"image": "readthedocs/build:2.0", "hash": "foo"}, "python": {"version": 2.7}}' # noqa
with patch('os.path.exists') as exists, patch('readthedocs.doc_builder.python_environments.open', mock_open(read_data=env_json_data)) as _open: # noqa
exists.return_value = True
self.assertTrue(python_env.is_obsolete)
@patch(
'readthedocs.doc_builder.environments.DockerBuildEnvironment.image_hash',
PropertyMock(return_value='a1b2c3'),
)
class AutoWipeDockerBuildEnvironmentTest(AutoWipeEnvironmentBase, TestCase):
build_env_class = DockerBuildEnvironment
@pytest.mark.xfail(
reason='PythonEnvironment needs to be refactored to do not rely on DockerBuildEnvironment',
)
@patch(
'readthedocs.doc_builder.environments.DockerBuildEnvironment.image_hash',
PropertyMock(return_value='a1b2c3'),
)
class AutoWipeLocalBuildEnvironmentTest(AutoWipeEnvironmentBase, TestCase):
build_env_class = LocalBuildEnvironment