Save Docker image hash in RTD environment.json file (#3880)
* 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 * Improve docstring * Handle obsolete cases better * when the file is corrupted or we don't have access, we return that it's OBSOLETE * when there is a new setting that we need to compare and it's not in the JSON file, we return OBSOLETE * Test case for build image in the config but not in the JSONhumitos/resolver/username-regex
parent
7aa6f4d41a
commit
393e31ad3a
|
@ -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)),
|
||||
|
|
|
@ -109,12 +109,14 @@ class PythonEnvironment(object):
|
|||
@property
|
||||
def is_obsolete(self):
|
||||
"""
|
||||
Determine if the Python version of the venv obsolete.
|
||||
Determine if the environment is obsolete for different reasons.
|
||||
|
||||
It checks the the data stored at ``readthedocs-environment.json`` and
|
||||
compares it against the Python version in the project version to be
|
||||
built and the Docker image used to create the venv against the one in
|
||||
the project version config.
|
||||
compares it with the one to be used. In particular:
|
||||
|
||||
* the Python version (e.g. 2.7, 3, 3.6, etc)
|
||||
* the Docker image name
|
||||
* the Docker image hash
|
||||
|
||||
:returns: ``True`` when it's obsolete and ``False`` otherwise
|
||||
|
||||
|
@ -129,15 +131,23 @@ class PythonEnvironment(object):
|
|||
try:
|
||||
with open(self.environment_json_path(), 'r') as fpath:
|
||||
environment_conf = json.load(fpath)
|
||||
env_python_version = environment_conf['python']['version']
|
||||
env_build_image = environment_conf['build']['image']
|
||||
except (IOError, TypeError, KeyError, ValueError):
|
||||
log.error('Unable to read/parse readthedocs-environment.json file')
|
||||
return False
|
||||
log.warning('Unable to read/parse readthedocs-environment.json file')
|
||||
# We remove the JSON file here to avoid cycling over time with a
|
||||
# corrupted file.
|
||||
os.remove(self.environment_json_path())
|
||||
return True
|
||||
|
||||
# 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
|
||||
env_python = environment_conf.get('python', {})
|
||||
env_build = environment_conf.get('build', {})
|
||||
|
||||
# By defaulting non-existent options to ``None`` we force a wipe since
|
||||
# we don't know how the environment was created
|
||||
env_python_version = env_python.get('version', None)
|
||||
env_build_image = env_build.get('image', None)
|
||||
env_build_hash = env_build.get('hash', None)
|
||||
|
||||
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 +155,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,
|
||||
},
|
||||
}
|
||||
|
||||
|
|
|
@ -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,82 @@ 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)
|
||||
|
||||
def test_is_obsolete_with_json_missing_build_hash(self):
|
||||
config_data = {
|
||||
'build': {
|
||||
'image': '2.0',
|
||||
'hash': 'a1b2c3',
|
||||
},
|
||||
'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"}, "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
|
||||
|
|
Loading…
Reference in New Issue