Builds: restart build commands before a new build (#7999)

This avoids ending up with duplicate commands.
This can be tested by manually triggering a retry in any part of the
task.

The user may still see duplicate commands,
as our js always adds new commands to the end,
but after a refresh the correct number of commands are shown.

Author: stsewd

readthedocs/api/v2/views/model_views.py

@@ -272,6 +272,17 @@ def retrieve(self, *args, **kwargs):
                     )
         return Response(data)
 
+    @decorators.action(
+        detail=True,
+        permission_classes=[permissions.IsAdminUser],
+        methods=['post'],
+    )
+    def reset(self, request, **kwargs):
+        """Reset the build so it can be re-used when re-trying."""
+        instance = self.get_object()
+        instance.reset()
+        return Response(status=status.HTTP_204_NO_CONTENT)
+
 
 class BuildCommandViewSet(UserSelectViewSet):
     parser_classes = [JSONParser, MultiPartParser]

readthedocs/builds/models.py

@@ -921,6 +921,24 @@ def external_version_name(self):
     def using_latest_config(self):
         return int(self.config.get('version', '1')) == LATEST_CONFIGURATION_VERSION
 
+    def reset(self):
+        """
+        Reset the build so it can be re-used when re-trying.
+
+        Dates and states are usually overriden by the build,
+        we care more about deleting the commands.
+        """
+        self.state = BUILD_STATE_TRIGGERED
+        self.status = ''
+        self.success = True
+        self.output = ''
+        self.error = ''
+        self.exit_code = None
+        self.builder = ''
+        self.cold_storage = False
+        self.commands.all().delete()
+        self.save()
+
 
 class BuildCommandResultMixin:
 

readthedocs/projects/tasks.py

@@ -72,7 +72,7 @@
 from readthedocs.projects.models import APIProject, Feature
 from readthedocs.search.utils import index_new_files, remove_indexed_files
 from readthedocs.sphinx_domains.models import SphinxDomain
-from readthedocs.storage import build_media_storage, build_environment_storage
+from readthedocs.storage import build_environment_storage, build_media_storage
 from readthedocs.vcs_support import utils as vcs_support_utils
 from readthedocs.worker import app
 
@@ -670,6 +670,10 @@ def run_setup(self, record=True):
 
         Return True if successful.
         """
+        # Reset build only if it has some commands already.
+        if self.build.get('commands'):
+            api_v2.build(self.build['id']).reset.post()
+
         if settings.DOCKER_ENABLE:
             env_cls = DockerBuildEnvironment
         else:

readthedocs/rtd_tests/tests/test_api.py

@@ -37,7 +37,13 @@
     GitLabWebhookView,
 )
 from readthedocs.api.v2.views.task_views import get_status_data
-from readthedocs.builds.constants import EXTERNAL, LATEST
+from readthedocs.builds.constants import (
+    BUILD_STATE_CLONING,
+    BUILD_STATE_TRIGGERED,
+    BUILD_STATUS_DUPLICATED,
+    EXTERNAL,
+    LATEST,
+)
 from readthedocs.builds.models import Build, BuildCommandResult, Version
 from readthedocs.integrations.models import Integration
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
@@ -55,6 +61,11 @@
 class APIBuildTests(TestCase):
     fixtures = ['eric.json', 'test_data.json']
 
+    def setUp(self):
+        self.user = User.objects.get(username='eric')
+        self.project = get(Project, users=[self.user])
+        self.version = self.project.versions.get(slug=LATEST)
+
     def test_make_build(self):
         """Test that a superuser can use the API."""
         client = APIClient()
@@ -82,6 +93,47 @@ def test_make_build(self):
         self.assertEqual(build['output'], 'Test Output')
         self.assertEqual(build['state_display'], 'Cloning')
 
+    def test_reset_build(self):
+        build = get(
+            Build,
+            project=self.project,
+            version=self.version,
+            state=BUILD_STATE_CLONING,
+            status=BUILD_STATUS_DUPLICATED,
+            success=False,
+            output='Output',
+            error='Error',
+            exit_code=9,
+            builder='Builder',
+            cold_storage=True,
+        )
+        command = get(
+            BuildCommandResult,
+            build=build,
+        )
+        build.commands.add(command)
+
+        self.assertEqual(build.commands.count(), 1)
+
+        client = APIClient()
+        client.force_login(self.user)
+        r = client.post(reverse('build-reset', args=(build.pk,)))
+
+        self.assertEqual(r.status_code, 204)
+        build.refresh_from_db()
+        self.assertEqual(build.project, self.project)
+        self.assertEqual(build.version, self.version)
+        self.assertEqual(build.state, BUILD_STATE_TRIGGERED)
+        self.assertEqual(build.status, '')
+        self.assertTrue(build.success)
+        self.assertEqual(build.output, '')
+        self.assertEqual(build.error, '')
+        self.assertIsNone(build.exit_code)
+        self.assertEqual(build.builder, '')
+        self.assertFalse(build.cold_storage)
+        self.assertEqual(build.commands.count(), 0)
+
+
     def test_api_does_not_have_private_config_key_superuser(self):
         client = APIClient()
         client.login(username='super', password='test')

readthedocs/rtd_tests/tests/test_builds.py

@@ -1,21 +1,19 @@
-# -*- coding: utf-8 -*-
 import datetime
 import os
-
 from unittest import mock
+
+from allauth.socialaccount.models import SocialAccount
 from django.contrib.auth.models import User
 from django.test import TestCase
-from django_dynamic_fixture import fixture, get
 from django.utils import timezone
-
-from allauth.socialaccount.models import SocialAccount
+from django_dynamic_fixture import fixture, get
 
 from readthedocs.builds.constants import (
     BRANCH,
     EXTERNAL,
+    GENERIC_EXTERNAL_VERSION_NAME,
     GITHUB_EXTERNAL_VERSION_NAME,
     GITLAB_EXTERNAL_VERSION_NAME,
-    GENERIC_EXTERNAL_VERSION_NAME
 )
 from readthedocs.builds.models import Build, Version
 from readthedocs.core.utils import trigger_build
@@ -325,7 +323,7 @@ def test_save_config_in_build_model(self, load_config, api_v2):
         task.run_setup()
         build_config = task.build['config']
         # For patch
-        api_v2.build.assert_called_once()
+        api_v2.build().patch.assert_called_once()
         assert build_config['version'] == '1'
         assert 'sphinx' in build_config
         assert build_config['doctype'] == 'sphinx'

readthedocs/rtd_tests/tests/test_privacy_urls.py

@@ -382,6 +382,7 @@ def setUp(self):
         }
         self.response_data = {
             'build-concurrent': {'status_code': 403},
+            'build-reset': {'status_code': 403},
             'project-sync-versions': {'status_code': 403},
             'project-token': {'status_code': 403},
             'emailhook-list': {'status_code': 403},