launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:snap-retry-build into launchpad:master.

Commit message:
Add support for retrying snap builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/394713
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:snap-retry-build into launchpad:master.

diff --git a/lib/lp/snappy/browser/configure.zcml b/lib/lp/snappy/browser/configure.zcml
index ba3dcc4..f3ce1d5 100644
--- a/lib/lp/snappy/browser/configure.zcml
+++ b/lib/lp/snappy/browser/configure.zcml
@@ -112,6 +112,12 @@
             template="../templates/snapbuild-index.pt" />
         <browser:page
             for="lp.snappy.interfaces.snapbuild.ISnapBuild"
+            class="lp.snappy.browser.snapbuild.SnapBuildRetryView"
+            permission="launchpad.Edit"
+            name="+retry"
+            template="../templates/snapbuild-retry.pt" />
+        <browser:page
+            for="lp.snappy.interfaces.snapbuild.ISnapBuild"
             class="lp.snappy.browser.snapbuild.SnapBuildCancelView"
             permission="launchpad.Edit"
             name="+cancel"
diff --git a/lib/lp/snappy/browser/snapbuild.py b/lib/lp/snappy/browser/snapbuild.py
index a485afd..49074d1 100644
--- a/lib/lp/snappy/browser/snapbuild.py
+++ b/lib/lp/snappy/browser/snapbuild.py
@@ -48,7 +48,13 @@ class SnapBuildContextMenu(ContextMenu):
 
     facet = 'overview'
 
-    links = ('cancel', 'rescore')
+    links = ('retry', 'cancel', 'rescore')
+
+    @enabled_with_permission('launchpad.Edit')
+    def retry(self):
+        return Link(
+            '+retry', 'Retry this build', icon='retry',
+            enabled=self.context.can_be_retried)
 
     @enabled_with_permission('launchpad.Edit')
     def cancel(self):
@@ -106,6 +112,32 @@ class SnapBuildView(LaunchpadFormView):
                 "possible.")
 
 
+class SnapBuildRetryView(LaunchpadFormView):
+    """View for retrying a snap package build."""
+
+    class schema(Interface):
+        """Schema for retrying a build."""
+
+    page_title = label = 'Retry build'
+
+    @property
+    def cancel_url(self):
+        return canonical_url(self.context)
+    next_url = cancel_url
+
+    @action('Retry build', name='retry')
+    def request_action(self, action, data):
+        """Retry the build."""
+        if not self.context.can_be_retried:
+            self.request.response.addErrorNotification(
+                'Build cannot be retried')
+        else:
+            self.context.retry()
+            self.request.response.addInfoNotification('Build has been queued')
+
+        self.request.response.redirect(self.next_url)
+
+
 class SnapBuildCancelView(LaunchpadFormView):
     """View for cancelling a snap package build."""
 
diff --git a/lib/lp/snappy/browser/tests/test_snapbuild.py b/lib/lp/snappy/browser/tests/test_snapbuild.py
index 3b34639..848aa0d 100644
--- a/lib/lp/snappy/browser/tests/test_snapbuild.py
+++ b/lib/lp/snappy/browser/tests/test_snapbuild.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap package build views."""
@@ -248,6 +248,38 @@ class TestSnapBuildOperations(BrowserTestCase):
         self.buildd_admin = self.factory.makePerson(
             member_of=[getUtility(ILaunchpadCelebrities).buildd_admin])
 
+    def test_retry_build(self):
+        # The requester of a build can retry it.
+        self.build.updateStatus(BuildStatus.FAILEDTOBUILD)
+        transaction.commit()
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        browser.getLink("Retry this build").click()
+        self.assertEqual(self.build_url, browser.getLink("Cancel").url)
+        browser.getControl("Retry build").click()
+        self.assertEqual(self.build_url, browser.url)
+        login(ANONYMOUS)
+        self.assertEqual(BuildStatus.NEEDSBUILD, self.build.status)
+
+    def test_retry_build_random_user(self):
+        # An unrelated non-admin user cannot retry a build.
+        self.build.updateStatus(BuildStatus.FAILEDTOBUILD)
+        transaction.commit()
+        user = self.factory.makePerson()
+        browser = self.getViewBrowser(self.build, user=user)
+        self.assertRaises(
+            LinkNotFoundError, browser.getLink, "Retry this build")
+        self.assertRaises(
+            Unauthorized, self.getUserBrowser, self.build_url + "/+retry",
+            user=user)
+
+    def test_retry_build_wrong_state(self):
+        # If the build isn't in an unsuccessful terminal state, you can't
+        # retry it.
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        self.assertRaises(
+            LinkNotFoundError, browser.getLink, "Retry this build")
+
     def test_cancel_build(self):
         # The requester of a build can cancel it.
         self.build.queueBuild()
diff --git a/lib/lp/snappy/interfaces/snapbuild.py b/lib/lp/snappy/interfaces/snapbuild.py
index c452092..e812f46 100644
--- a/lib/lp/snappy/interfaces/snapbuild.py
+++ b/lib/lp/snappy/interfaces/snapbuild.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snap package build interfaces."""
@@ -184,6 +184,11 @@ class ISnapBuildView(IPackageBuild):
         required=True, readonly=True,
         description=_("Whether this build record can be rescored manually.")))
 
+    can_be_retried = exported(Bool(
+        title=_("Can be retried"),
+        required=False, readonly=True,
+        description=_("Whether this build record can be retried.")))
+
     can_be_cancelled = exported(Bool(
         title=_("Can be cancelled"),
         required=True, readonly=True,
@@ -304,6 +309,15 @@ class ISnapBuildEdit(Interface):
 
     @export_write_operation()
     @operation_for_version("devel")
+    def retry():
+        """Restore the build record to its initial state.
+
+        Build record loses its history, is moved to NEEDSBUILD and a new
+        non-scored BuildQueue entry is created for it.
+        """
+
+    @export_write_operation()
+    @operation_for_version("devel")
     def cancel():
         """Cancel the build if it is either pending or in progress.
 
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index 69cbcfe..3a9475b 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 from __future__ import absolute_import, print_function, unicode_literals
@@ -51,6 +51,7 @@ from lp.buildmaster.model.buildfarmjob import SpecificBuildFarmJobSourceMixin
 from lp.buildmaster.model.packagebuild import PackageBuildMixin
 from lp.code.interfaces.gitrepository import IGitRepository
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.person import Person
@@ -273,6 +274,27 @@ class SnapBuild(PackageBuildMixin, Storm):
             return self.buildqueue_record.lastscore
 
     @property
+    def can_be_retried(self):
+        """See `ISnapBuild`."""
+        # First check that the behaviour would accept the build if it
+        # succeeded.
+        if self.distro_series.status == SeriesStatus.OBSOLETE:
+            return False
+
+        failed_statuses = [
+            BuildStatus.FAILEDTOBUILD,
+            BuildStatus.MANUALDEPWAIT,
+            BuildStatus.CHROOTWAIT,
+            BuildStatus.FAILEDTOUPLOAD,
+            BuildStatus.CANCELLED,
+            BuildStatus.SUPERSEDED,
+            ]
+
+        # If the build is currently in any of the failed states,
+        # it may be retried.
+        return self.status in failed_statuses
+
+    @property
     def can_be_rescored(self):
         """See `ISnapBuild`."""
         return (
@@ -291,6 +313,19 @@ class SnapBuild(PackageBuildMixin, Storm):
             ]
         return self.status in cancellable_statuses
 
+    def retry(self):
+        """See `ISnapBuild`."""
+        assert self.can_be_retried, "Build %s cannot be retried" % self.id
+        self.build_farm_job.status = self.status = BuildStatus.NEEDSBUILD
+        self.build_farm_job.date_finished = self.date_finished = None
+        self.date_started = None
+        self.build_farm_job.builder = self.builder = None
+        self.log = None
+        self.upload_log = None
+        self.dependencies = None
+        self.failure_count = 0
+        self.queueBuild()
+
     def rescore(self, score):
         """See `ISnapBuild`."""
         assert self.can_be_rescored, "Build %s cannot be rescored" % self.id
diff --git a/lib/lp/snappy/templates/snapbuild-index.pt b/lib/lp/snappy/templates/snapbuild-index.pt
index dd848c0..513a81e 100644
--- a/lib/lp/snappy/templates/snapbuild-index.pt
+++ b/lib/lp/snappy/templates/snapbuild-index.pt
@@ -106,6 +106,9 @@
         on <a tal:content="context/builder/title"
               tal:attributes="href context/builder/fmt:url"/>
       </tal:built>
+      <tal:retry define="link context/menu:context/retry"
+                 condition="link/enabled"
+                 replace="structure link/fmt:link" />
       <tal:cancel define="link context/menu:context/cancel"
                   condition="link/enabled"
                   replace="structure link/fmt:link" />
diff --git a/lib/lp/snappy/templates/snapbuild-retry.pt b/lib/lp/snappy/templates/snapbuild-retry.pt
new file mode 100644
index 0000000..ba08004
--- /dev/null
+++ b/lib/lp/snappy/templates/snapbuild-retry.pt
@@ -0,0 +1,28 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml";
+  xmlns:tal="http://xml.zope.org/namespaces/tal";
+  xmlns:metal="http://xml.zope.org/namespaces/metal";
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="launchpad">
+<body>
+
+  <div metal:fill-slot="main">
+    <div metal:use-macro="context/@@launchpad_form/form">
+      <div metal:fill-slot="extra_info">
+        <p>
+          The status of <dfn tal:content="context/title" /> is
+          <span tal:replace="context/status/title" />.
+        </p>
+        <p>Retrying this build will destroy its history and logs.</p>
+        <p>
+          By default, this build will be retried only after other pending
+          builds; please contact a build daemon administrator if you need
+          special treatment.
+        </p>
+      </div>
+    </div>
+  </div>
+
+</body>
+</html>
diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
index 4a6000e..9ff69d0 100644
--- a/lib/lp/snappy/tests/test_snapbuild.py
+++ b/lib/lp/snappy/tests/test_snapbuild.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap package build features."""
@@ -36,6 +36,7 @@ from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.buildmaster.interfaces.packagebuild import IPackageBuild
 from lp.buildmaster.interfaces.processor import IProcessorSet
 from lp.registry.enums import PersonVisibility
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.authserver.xmlrpc import AuthServerAPIView
 from lp.services.config import config
 from lp.services.features.testing import FeatureFixture
@@ -181,6 +182,30 @@ class TestSnapBuild(TestCaseWithFactory):
             build = self.factory.makeSnapBuild(archive=private_archive)
             self.assertTrue(build.is_private)
 
+    def test_can_be_retried(self):
+        ok_cases = [
+            BuildStatus.FAILEDTOBUILD,
+            BuildStatus.MANUALDEPWAIT,
+            BuildStatus.CHROOTWAIT,
+            BuildStatus.FAILEDTOUPLOAD,
+            BuildStatus.CANCELLED,
+            BuildStatus.SUPERSEDED,
+            ]
+        for status in BuildStatus.items:
+            build = self.factory.makeSnapBuild(status=status)
+            if status in ok_cases:
+                self.assertTrue(build.can_be_retried)
+            else:
+                self.assertFalse(build.can_be_retried)
+
+    def test_can_be_retried_obsolete_series(self):
+        # Builds for obsolete series cannot be retried.
+        distroseries = self.factory.makeDistroSeries(
+            status=SeriesStatus.OBSOLETE)
+        das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+        build = self.factory.makeSnapBuild(distroarchseries=das)
+        self.assertFalse(build.can_be_retried)
+
     def test_can_be_cancelled(self):
         # For all states that can be cancelled, can_be_cancelled returns True.
         ok_cases = [
@@ -193,6 +218,22 @@ class TestSnapBuild(TestCaseWithFactory):
             else:
                 self.assertFalse(self.build.can_be_cancelled)
 
+    def test_retry_resets_state(self):
+        # Retrying a build resets most of the state attributes, but does
+        # not modify the first dispatch time.
+        now = datetime.now(pytz.UTC)
+        build = self.factory.makeSnapBuild()
+        build.updateStatus(BuildStatus.BUILDING, date_started=now)
+        build.updateStatus(BuildStatus.FAILEDTOBUILD)
+        build.gotFailure()
+        with person_logged_in(build.snap.owner):
+            build.retry()
+        self.assertEqual(BuildStatus.NEEDSBUILD, build.status)
+        self.assertEqual(now, build.date_first_dispatched)
+        self.assertIsNone(build.log)
+        self.assertIsNone(build.upload_log)
+        self.assertEqual(0, build.failure_count)
+
     def test_cancel_not_in_progress(self):
         # The cancel() method for a pending build leaves it in the CANCELLED
         # state.