Merge pull request #3432 from rtfd/humitos/wipe/automatically

Check versions used to create the venv and auto-wipe

Author: ericholscher

readthedocs/doc_builder/config.py

@@ -51,16 +51,7 @@ def extra_requirements(self):
 
     @property
     def python_interpreter(self):
-        ver = self.python_version
-        if ver in [2, 3]:
-            # Get the highest version of the major series version if user only
-            # gave us a version of '2', or '3'
-            ver = max(
-                list(
-                    filter(
-                        lambda x: x < ver + 1,
-                        self._yaml_config.get_valid_python_versions(),
-                    )))
+        ver = self.python_full_version
         return 'python{0}'.format(ver)
 
     @property
@@ -75,6 +66,20 @@ def python_version(self):
             version = 3
         return version
 
+    @property
+    def python_full_version(self):
+        ver = self.python_version
+        if ver in [2, 3]:
+            # Get the highest version of the major series version if user only
+            # gave us a version of '2', or '3'
+            ver = max(
+                list(
+                    filter(
+                        lambda x: x < ver + 1,
+                        self._yaml_config.get_valid_python_versions(),
+                    )))
+        return ver
+
     @property
     def use_system_site_packages(self):
         if 'use_system_site_packages' in self._yaml_config.get('python', {}):

readthedocs/doc_builder/python_environments.py

@@ -1,14 +1,19 @@
+# -*- coding: utf-8 -*-
 """An abstraction over virtualenv and Conda environments."""
 
-from __future__ import absolute_import
-from builtins import object
+from __future__ import (
+    absolute_import, division, print_function, unicode_literals)
+
+import json
 import logging
 import os
 import shutil
+from builtins import object, open
 
 from django.conf import settings
 
 from readthedocs.doc_builder.config import ConfigWrapper
+from readthedocs.doc_builder.constants import DOCKER_IMAGE
 from readthedocs.doc_builder.loader import get_builder_class
 from readthedocs.projects.constants import LOG_TEMPLATE
 from readthedocs.projects.models import Feature
@@ -38,7 +43,6 @@ def _log(self, msg):
                          msg=msg))
 
     def delete_existing_build_dir(self):
-
         # Handle deleting old build dir
         build_dir = os.path.join(
             self.venv_path(),
@@ -47,6 +51,13 @@ def delete_existing_build_dir(self):
             self._log('Removing existing build directory')
             shutil.rmtree(build_dir)
 
+    def delete_existing_venv_dir(self):
+        venv_dir = self.venv_path()
+        # Handle deleting old venv dir
+        if os.path.exists(venv_dir):
+            self._log('Removing existing venv directory')
+            shutil.rmtree(venv_dir)
+
     def install_package(self):
         if self.config.install_project:
             if self.config.pip_install or getattr(settings, 'USE_PIP_INSTALL', False):
@@ -87,6 +98,73 @@ def venv_bin(self, filename=None):
             parts.append(filename)
         return os.path.join(*parts)
 
+    def environment_json_path(self):
+        """Return the path to the ``readthedocs-environment.json`` file."""
+        return os.path.join(
+            self.venv_path(),
+            'readthedocs-environment.json',
+        )
+
+    @property
+    def is_obsolete(self):
+        """
+        Determine if the Python version of the venv obsolete.
+
+        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.
+
+        :returns: ``True`` when it's obsolete and ``False`` otherwise
+
+        :rtype: bool
+        """
+        # Always returns False if we don't have information about what Python
+        # version/Docker image was used to create the venv as backward
+        # compatibility.
+        if not os.path.exists(self.environment_json_path()):
+            return False
+
+        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
+
+        # 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,
+        ])
+
+    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
+
+        data = {
+            'python': {
+                'version': self.config.python_full_version,
+            },
+            'build': {
+                'image': build_image,
+            },
+        }
+
+        with open(self.environment_json_path(), 'w') as fpath:
+            # Compatibility for Py2 and Py3. ``io.TextIOWrapper`` expects
+            # unicode but ``json.dumps`` returns str in Py2.
+            fpath.write(unicode(json.dumps(data)))
+
 
 class Virtualenv(PythonEnvironment):
 

readthedocs/projects/tasks.py

@@ -435,8 +435,16 @@ def setup_environment(self):
                 version=self.version,
                 max_lock_age=getattr(settings, 'REPO_LOCK_SECONDS', 30)):
 
-            self.python_env.delete_existing_build_dir()
+            # Check if the python version/build image in the current venv is the
+            # same to be used in this build and if it differs, wipe the venv to
+            # avoid conflicts.
+            if self.python_env.is_obsolete:
+                self.python_env.delete_existing_venv_dir()
+            else:
+                self.python_env.delete_existing_build_dir()
+
             self.python_env.setup_base()
+            self.python_env.save_environment_json()
             self.python_env.install_core_requirements()
             self.python_env.install_user_requirements()
             self.python_env.install_package()

readthedocs/rtd_tests/tests/test_doc_building.py

@@ -14,18 +14,22 @@
 from builtins import str
 
 import mock
+import pytest
 from django.test import TestCase
 from docker.errors import APIError as DockerAPIError
 from docker.errors import DockerException
-from mock import Mock, PropertyMock, patch
+from mock import Mock, PropertyMock, mock_open, patch
 
 from readthedocs.builds.constants import BUILD_STATE_CLONING
 from readthedocs.builds.models import Version
+from readthedocs.doc_builder.config import ConfigWrapper
 from readthedocs.doc_builder.environments import (
     BuildCommand, DockerBuildCommand, DockerEnvironment, LocalEnvironment)
 from readthedocs.doc_builder.exceptions import BuildEnvironmentError
+from readthedocs.doc_builder.python_environments import Virtualenv
 from readthedocs.projects.models import Project
 from readthedocs.rtd_tests.mocks.environment import EnvironmentMockGroup
+from readthedocs.rtd_tests.tests.test_config_wrapper import create_load
 
 DUMMY_BUILD_ID = 123
 SAMPLE_UNICODE = u'HérÉ îß sömê ünïçó∂é'
@@ -831,3 +835,134 @@ def test_command_oom_kill(self):
         self.assertEqual(
             str(cmd.output),
             u'Command killed due to excessive memory consumption\n')
+
+
+
+
+class TestAutoWipeEnvironment(TestCase):
+    fixtures = ['test_data']
+
+    def setUp(self):
+        self.pip = Project.objects.get(slug='pip')
+        self.version = self.pip.versions.get(slug='0.8')
+
+    def test_is_obsolete_without_env_json_file(self):
+        yaml_config = create_load()()[0]
+        config = ConfigWrapper(version=self.version, yaml_config=yaml_config)
+
+        with patch('os.path.exists') as exists:
+            exists.return_value = False
+            python_env = Virtualenv(
+                version=self.version,
+                build_env=None,
+                config=config,
+            )
+
+        self.assertFalse(python_env.is_obsolete)
+
+    def test_is_obsolete_with_invalid_env_json_file(self):
+        yaml_config = create_load()()[0]
+        config = ConfigWrapper(version=self.version, yaml_config=yaml_config)
+
+        with patch('os.path.exists') as exists:
+            exists.return_value = True
+            python_env = Virtualenv(
+                version=self.version,
+                build_env=None,
+                config=config,
+            )
+
+        self.assertFalse(python_env.is_obsolete)
+
+    def test_is_obsolete_with_json_different_python_version(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=None,
+            config=config,
+        )
+        env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 3.5}}'
+        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)
+
+    @pytest.mark.xfail(reason='build.image is not being considered yet')
+    def test_is_obsolete_with_json_different_build_image(self):
+        config_data = {
+            'build': {
+                'image': 'latest',
+            },
+            '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=None,
+            config=config,
+        )
+        env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 2.7}}'
+        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_project_different_build_image(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:latest'
+        self.pip.save()
+
+        python_env = Virtualenv(
+            version=self.version,
+            build_env=None,
+            config=config,
+        )
+        env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 2.7}}'
+        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_same_data_as_version(self):
+        config_data = {
+            'build': {
+                'image': '2.0',
+            },
+            'python': {
+                'version': 3.5,
+            },
+        }
+        yaml_config = create_load(config_data)()[0]
+        config = ConfigWrapper(version=self.version, yaml_config=yaml_config)
+
+        python_env = Virtualenv(
+            version=self.version,
+            build_env=None,
+            config=config,
+        )
+        env_json_data = '{"build": {"image": "readthedocs/build:2.0"}, "python": {"version": 3.5}}'
+        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)