Save Docker image hash in RTD environment.json file

readthedocs/doc_builder/environments.py

@@ -625,6 +625,8 @@ def __init__(self, *args, **kwargs):
         )
         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 @@ def get_container_host_config(self):
             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 @@ def update_build_from_container_state(self):
     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)),

readthedocs/doc_builder/python_environments.py

@@ -109,12 +109,14 @@ def environment_json_path(self):
     @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,35 +131,43 @@ def is_obsolete(self):
         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
-
-        # 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
-
+            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
+
+        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
         # one coming from the project version config.
         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,
             },
         }
 

readthedocs/rtd_tests/tests/test_doc_building.py

@@ -9,7 +9,9 @@
     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 @@ def test_command_oom_kill(self):
             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 @@ def test_is_obsolete_without_env_json_file(self):
             exists.return_value = False
             python_env = Virtualenv(
                 version=self.version,
-                build_env=None,
+                build_env=self.build_env,
                 config=config,
             )
 
@@ -868,7 +910,7 @@ def test_is_obsolete_with_invalid_env_json_file(self):
             exists.return_value = True
             python_env = Virtualenv(
                 version=self.version,
-                build_env=None,
+                build_env=self.build_env,
                 config=config,
             )
 
@@ -888,10 +930,10 @@ def test_is_obsolete_with_json_different_python_version(self):
 
         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 @@ def test_is_obsolete_with_json_different_build_image(self):
 
         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 @@ def test_is_obsolete_with_project_different_build_image(self):
 
         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 @@ def test_is_obsolete_with_json_same_data_as_version(self):
 
         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