launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #26957
[Merge] ~twom/launchpad:oci-fix-partial-upload-status-being-wrong into launchpad:master
Tom Wardill has proposed merging ~twom/launchpad:oci-fix-partial-upload-status-being-wrong into launchpad:master.
Commit message:
Use a StatusSet for registry upload
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1925763 in Launchpad itself: "Registry upload status is invalid if a build has failed."
https://bugs.launchpad.net/launchpad/+bug/1925763
For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/401809
We need to keep track of partial upload status in a set of builds.
Do that using a different enum.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:oci-fix-partial-upload-status-being-wrong into launchpad:master.
diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
index eb62596..bc978e7 100644
--- a/lib/lp/oci/browser/ocirecipe.py
+++ b/lib/lp/oci/browser/ocirecipe.py
@@ -333,7 +333,7 @@ class OCIRecipeView(LaunchpadView):
status = removeSecurityProxy(
recipe_set.getStatusSummaryForBuilds([build_job]))
# Add the registry job status
- status["upload_scheduled"] = upload_status != unscheduled_upload
+ status["upload_requested"] = upload_status != unscheduled_upload
status["upload"] = upload_status
status["date"] = build_job.date
status["date_estimated"] = build_job.estimate
diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py
index b51097e..a4308ea 100644
--- a/lib/lp/oci/browser/tests/test_ocirecipe.py
+++ b/lib/lp/oci/browser/tests/test_ocirecipe.py
@@ -1525,6 +1525,7 @@ class TestOCIRecipeView(BaseTestOCIRecipeView):
When requested
When complete
There are some builds waiting to be built.
+ Waiting for builds to start.
a moment ago
in .* \(estimated\)
""", self.getMainText(recipe))
diff --git a/lib/lp/oci/interfaces/ocirecipebuild.py b/lib/lp/oci/interfaces/ocirecipebuild.py
index b9b64ea..de94bc7 100644
--- a/lib/lp/oci/interfaces/ocirecipebuild.py
+++ b/lib/lp/oci/interfaces/ocirecipebuild.py
@@ -13,6 +13,7 @@ __all__ = [
'IOCIRecipeBuild',
'IOCIRecipeBuildSet',
'OCIRecipeBuildRegistryUploadStatus',
+ 'OCIRecipeBuildSetRegistryUploadStatus',
]
from lazr.enum import (
@@ -102,6 +103,44 @@ class OCIRecipeBuildRegistryUploadStatus(EnumeratedType):
""")
+class OCIRecipeBuildSetRegistryUploadStatus(EnumeratedType):
+ """OCI build registry upload status type
+
+ OCI builds may be uploaded to a registry. This represents the state of
+ that process.
+ """
+
+ UNSCHEDULED = Item("""
+ Unscheduled
+
+ No upload of this OCI build to a registry is scheduled.
+ """)
+
+ PENDING = Item("""
+ Pending
+
+ This OCI build is queued for upload to a registry.
+ """)
+
+ FAILEDTOUPLOAD = Item("""
+ Failed to upload
+
+ The last attempt to upload this OCI build to a registry failed.
+ """)
+
+ UPLOADED = Item("""
+ Uploaded
+
+ This OCI build was successfully uploaded to a registry.
+ """)
+
+ PARTIAL = Item("""
+ Partial
+
+ Some OCI builds have uploaded.
+ """)
+
+
class IOCIRecipeBuildView(IPackageBuild):
"""`IOCIRecipeBuild` attributes that require launchpad.View permission."""
diff --git a/lib/lp/oci/model/ocirecipejob.py b/lib/lp/oci/model/ocirecipejob.py
index ac885d7..eaabfff 100644
--- a/lib/lp/oci/model/ocirecipejob.py
+++ b/lib/lp/oci/model/ocirecipejob.py
@@ -33,7 +33,10 @@ from zope.interface import (
from zope.security.proxy import removeSecurityProxy
from lp.app.errors import NotFoundError
-from lp.oci.interfaces.ocirecipebuild import OCIRecipeBuildRegistryUploadStatus
+from lp.oci.interfaces.ocirecipebuild import (
+ OCIRecipeBuildRegistryUploadStatus,
+ OCIRecipeBuildSetRegistryUploadStatus,
+ )
from lp.oci.interfaces.ocirecipejob import (
IOCIRecipeJob,
IOCIRecipeRequestBuildsJob,
@@ -288,41 +291,56 @@ class OCIRecipeRequestBuildsJob(OCIRecipeJobDerived):
def addUploadedManifest(self, build_id, manifest_info):
self.metadata["uploaded_manifests"][int(build_id)] = manifest_info
+ @property
def build_status(self):
+ builds = self.builds
# This just returns a dict, but Zope is really helpful here
status = removeSecurityProxy(
getUtility(IOCIRecipeSet).getStatusSummaryForBuilds(
- list(self.builds)))
+ list(builds)))
# This has a really long name!
- statusEnum = OCIRecipeBuildRegistryUploadStatus
+ singleStatus = OCIRecipeBuildRegistryUploadStatus
+ setStatus = OCIRecipeBuildSetRegistryUploadStatus
# Set the pending upload status if either we're not done uploading,
# or there was no upload requested in the first place (no push rules)
if status['status'] == BuildSetStatus.FULLYBUILT:
upload_status = [
- (x.registry_upload_status == statusEnum.UPLOADED or
- x.registry_upload_status == statusEnum.UNSCHEDULED)
+ (x.registry_upload_status == singleStatus.UPLOADED or
+ x.registry_upload_status == singleStatus.UNSCHEDULED)
for x in status['builds']]
if not all(upload_status):
status['status'] = BuildSetStatus.FULLYBUILT_PENDING
- # Add a flag for if we're expecting a registry upload
- status['upload_scheduled'] = any(
- x.registry_upload_status != statusEnum.UNSCHEDULED
- for x in status['builds'])
-
- # Set the equivalent of BuildSetStatus, but for registry upload
- # If any of the builds have failed to upload
- if any(x.registry_upload_status == statusEnum.FAILEDTOUPLOAD
- for x in status['builds']):
- status['upload'] = statusEnum.FAILEDTOUPLOAD
+ # expecting an upload
+ # if there's a registry upload job for any build - yes
+ # if there isn't, and any of the builds are complete/failed - no
+ upload_requested = False
+ if any(x.last_registry_upload_job for x in builds):
+ upload_requested = True
+ if any(not x.date_finished and x.recipe.can_upload_to_registry
+ for x in builds):
+ upload_requested = True
+ status['upload_requested'] = upload_requested
+
+ # Convert the set of registry statuses into a single line
+ # for display
+ if any(x.registry_upload_status == singleStatus.FAILEDTOUPLOAD
+ for x in builds):
+ status['upload'] = setStatus.FAILEDTOUPLOAD
# If any of the builds are still waiting to upload
- elif any(x.registry_upload_status == statusEnum.PENDING
- for x in status['builds']):
- status['upload'] = statusEnum.PENDING
+ elif all(x.registry_upload_status == singleStatus.UPLOADED
+ for x in builds):
+ status['upload'] = setStatus.UPLOADED
+ elif all(x.registry_upload_status == singleStatus.UNSCHEDULED
+ for x in builds):
+ status['upload'] = setStatus.UNSCHEDULED
+ elif any(x.registry_upload_status == singleStatus.UPLOADED
+ for x in builds):
+ status['upload'] = setStatus.PARTIAL
else:
- status['upload'] = statusEnum.UPLOADED
+ status['upload'] = setStatus.PENDING
# Get the longest date and whether any of them are estimated
# for the summary of the builds
diff --git a/lib/lp/oci/templates/ocirecipe-index.pt b/lib/lp/oci/templates/ocirecipe-index.pt
index 569ea43..33c38e9 100644
--- a/lib/lp/oci/templates/ocirecipe-index.pt
+++ b/lib/lp/oci/templates/ocirecipe-index.pt
@@ -145,12 +145,13 @@
</td>
<td>
- <tal:registry-upload tal:condition="build_status/upload_scheduled">
+
+ <tal:registry-upload tal:condition="build_status/upload_requested">
<span tal:content="build_status/upload/title" />
</tal:registry-upload>
- <tal:registry-upload tal:condition="not:build_status/upload_scheduled">
- <span tal:condition="python: 'FULLYBUILT' in build_status['status'].title">No registry upload requested.</span>
- <span tal:condition="python: 'FAILEDTOBUILD' in build_status['status'].title">No registry upload requested.</span>
+ <tal:registry-upload tal:condition="not:build_status/upload_requested">
+ <span tal:condition="python: 'NEEDSBUILD' in build_status['status'].title">Waiting for builds to start.</span>
+ <span tal:condition="python: 'NEEDSBUILD' not in build_status['status'].title">No registry upload requested.</span>
</tal:registry-upload>
</td>
<td>
@@ -183,7 +184,7 @@
</ul>
</td>
<td>
- <ul tal:condition="build_status/upload_scheduled" tal:repeat="build build_request/builds">
+ <ul tal:condition="build_status/upload_requested" tal:repeat="build build_request/builds">
<li style="padding-left: 22px;">
<strong><a class="sprite distribution"
tal:define="processor build/processor"
diff --git a/lib/lp/oci/tests/test_ocirecipebuildjob.py b/lib/lp/oci/tests/test_ocirecipebuildjob.py
index 0ff2938..95718bb 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildjob.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildjob.py
@@ -7,14 +7,8 @@ from __future__ import absolute_import, print_function, unicode_literals
__metaclass__ = type
-from datetime import (
- datetime,
- timedelta,
- )
import os
import signal
-import threading
-import time
from fixtures import FakeLogger
from storm.locals import Store
@@ -22,7 +16,6 @@ from testtools.matchers import (
Equals,
MatchesDict,
MatchesListwise,
- MatchesSetwise,
MatchesStructure,
)
import transaction
@@ -54,7 +47,6 @@ from lp.oci.model.ocirecipebuildjob import (
from lp.services.compat import mock
from lp.services.config import config
from lp.services.database.locking import (
- AdvisoryLockHeld,
LockType,
try_advisory_lock,
)
diff --git a/lib/lp/oci/tests/test_ocirecipejob.py b/lib/lp/oci/tests/test_ocirecipejob.py
new file mode 100644
index 0000000..d9f2acd
--- /dev/null
+++ b/lib/lp/oci/tests/test_ocirecipejob.py
@@ -0,0 +1,81 @@
+# Copyright 2021 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for classes in OCIRecipeJob."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from lp.buildmaster.enums import BuildStatus
+from lp.buildmaster.interfaces.processor import IProcessorSet
+from lp.oci.interfaces.ocirecipe import (
+ OCI_RECIPE_ALLOW_CREATE
+ )
+from lp.oci.interfaces.ocirecipebuild import (
+ OCIRecipeBuildSetRegistryUploadStatus,
+ )
+from lp.oci.interfaces.ocirecipebuildjob import IOCIRegistryUploadJobSource
+from lp.oci.interfaces.ocirecipejob import IOCIRecipeRequestBuildsJobSource
+from lp.registry.interfaces.series import SeriesStatus
+from lp.services.features.testing import FeatureFixture
+from lp.services.job.interfaces.job import JobStatus
+from lp.testing import (
+ person_logged_in,
+ TestCaseWithFactory,
+ )
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestOCIRecipeRequestBuildsJob(TestCaseWithFactory):
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestOCIRecipeRequestBuildsJob, self).setUp()
+ self.useFixture(FeatureFixture({OCI_RECIPE_ALLOW_CREATE: 'on'}))
+
+ def getDistroArchSeries(self, distroseries, proc_name="386",
+ arch_tag="i386"):
+ processor = getUtility(IProcessorSet).getByName(proc_name)
+
+ das = self.factory.makeDistroArchSeries(
+ distroseries=distroseries, architecturetag=arch_tag,
+ processor=processor)
+ fake_chroot = self.factory.makeLibraryFileAlias(
+ filename="fake_chroot.tar.gz", db_only=True)
+ das.addOrUpdateChroot(fake_chroot)
+ return das
+
+ def test_partial_on_failure(self):
+ ocirecipe = removeSecurityProxy(self.factory.makeOCIRecipe(
+ require_virtualized=False))
+ owner = ocirecipe.owner
+ distro = ocirecipe.oci_project.distribution
+ series = self.factory.makeDistroSeries(
+ distribution=distro, status=SeriesStatus.CURRENT)
+ self.getDistroArchSeries(series, "386", "386")
+ self.getDistroArchSeries(series, "hppa", "hppa")
+ job = getUtility(IOCIRecipeRequestBuildsJobSource).create(
+ ocirecipe, owner)
+
+ with person_logged_in(job.requester):
+ builds = ocirecipe.requestBuildsFromJob(
+ job.requester, build_request=job.build_request)
+ removeSecurityProxy(job).builds = builds
+
+ builds[0].updateStatus(BuildStatus.FAILEDTOBUILD)
+ builds[1].updateStatus(BuildStatus.FULLYBUILT)
+ upload_job = getUtility(IOCIRegistryUploadJobSource).create(builds[1])
+ removeSecurityProxy(upload_job).job._status = JobStatus.COMPLETED
+
+ status = job.build_status
+
+ self.assertTrue(status['upload_requested'])
+ self.assertEqual(
+ OCIRecipeBuildSetRegistryUploadStatus.PARTIAL,
+ status['upload']
+ )
