launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #27196
[Merge] ~cjwatson/launchpad:charm-recipe-delete-builds into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:charm-recipe-delete-builds into launchpad:master with ~cjwatson/launchpad:charm-recipe-listing-views as a prerequisite.
Commit message:
Delete charm recipe builds and jobs when deleting recipes
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/403959
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-recipe-delete-builds into launchpad:master.
diff --git a/lib/lp/charms/interfaces/charmrecipe.py b/lib/lp/charms/interfaces/charmrecipe.py
index c972097..9c76cf4 100644
--- a/lib/lp/charms/interfaces/charmrecipe.py
+++ b/lib/lp/charms/interfaces/charmrecipe.py
@@ -513,6 +513,9 @@ class ICharmRecipeSet(Interface):
def getByName(owner, project, name):
"""Returns the appropriate `ICharmRecipe` for the given objects."""
+ def exists(owner, project, name):
+ """Check to see if a matching charm recipe exists."""
+
def findByPerson(person, visible_by_user=None):
"""Return all charm recipes relevant to `person`.
diff --git a/lib/lp/charms/model/charmrecipe.py b/lib/lp/charms/model/charmrecipe.py
index d9c9b5c..f2066d4 100644
--- a/lib/lp/charms/model/charmrecipe.py
+++ b/lib/lp/charms/model/charmrecipe.py
@@ -20,6 +20,7 @@ from lazr.lifecycle.event import ObjectCreatedEvent
import pytz
from storm.databases.postgres import JSON
from storm.locals import (
+ And,
Bool,
DateTime,
Desc,
@@ -28,6 +29,7 @@ from storm.locals import (
Not,
Or,
Reference,
+ Select,
Store,
Unicode,
)
@@ -46,6 +48,8 @@ from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
from lp.buildmaster.model.builder import Builder
+from lp.buildmaster.model.buildfarmjob import BuildFarmJob
+from lp.buildmaster.model.buildqueue import BuildQueue
from lp.charms.adapters.buildarch import determine_instances_to_build
from lp.charms.interfaces.charmrecipe import (
BadCharmRecipeSearchContext,
@@ -67,12 +71,14 @@ from lp.charms.interfaces.charmrecipe import (
ICharmRecipeSet,
MissingCharmcraftYaml,
NoSourceForCharmRecipe,
+ NoSuchCharmRecipe,
)
from lp.charms.interfaces.charmrecipebuild import ICharmRecipeBuildSet
from lp.charms.interfaces.charmrecipejob import (
ICharmRecipeRequestBuildsJobSource,
)
from lp.charms.model.charmrecipebuild import CharmRecipeBuild
+from lp.charms.model.charmrecipejob import CharmRecipeJob
from lp.code.errors import (
GitRepositoryBlobNotFound,
GitRepositoryScanFault,
@@ -116,6 +122,7 @@ from lp.services.database.stormexpr import (
)
from lp.services.features import getFeatureFlag
from lp.services.job.interfaces.job import JobStatus
+from lp.services.job.model.job import Job
from lp.services.librarian.model import LibraryFileAlias
from lp.services.propertycache import (
cachedproperty,
@@ -639,7 +646,35 @@ class CharmRecipe(StormBase):
def destroySelf(self):
"""See `ICharmRecipe`."""
- IStore(CharmRecipe).remove(self)
+ store = IStore(self)
+ # Remove build jobs. There won't be many queued builds, so we can
+ # afford to do this the safe but slow way via BuildQueue.destroySelf
+ # rather than in bulk.
+ buildqueue_records = store.find(
+ BuildQueue,
+ BuildQueue._build_farm_job_id ==
+ CharmRecipeBuild.build_farm_job_id,
+ CharmRecipeBuild.recipe == self)
+ for buildqueue_record in buildqueue_records:
+ buildqueue_record.destroySelf()
+ build_farm_job_ids = list(store.find(
+ CharmRecipeBuild.build_farm_job_id,
+ CharmRecipeBuild.recipe == self))
+ store.execute("""
+ DELETE FROM CharmFile
+ USING CharmRecipeBuild
+ WHERE
+ CharmFile.build = CharmRecipeBuild.id AND
+ CharmRecipeBuild.recipe = ?
+ """, (self.id,))
+ store.find(CharmRecipeBuild, CharmRecipeBuild.recipe == self).remove()
+ affected_jobs = Select(
+ [CharmRecipeJob.job_id],
+ And(CharmRecipeJob.job == Job.id, CharmRecipeJob.recipe == self))
+ store.find(Job, Job.id.is_in(affected_jobs)).remove()
+ store.remove(self)
+ store.find(
+ BuildFarmJob, BuildFarmJob.id.is_in(build_farm_job_ids)).remove()
@implementer(ICharmRecipeSet)
@@ -664,7 +699,7 @@ class CharmRecipeSet:
if git_ref is None:
raise NoSourceForCharmRecipe
- if self.getByName(owner, project, name) is not None:
+ if self.exists(owner, project, name):
raise DuplicateCharmRecipeName
# The relevant validators will do their own checks as well, but we
@@ -690,11 +725,21 @@ class CharmRecipeSet:
return recipe
- def getByName(self, owner, project, name):
- """See `ICharmRecipeSet`."""
+ def _getByName(self, owner, project, name):
return IStore(CharmRecipe).find(
CharmRecipe, owner=owner, project=project, name=name).one()
+ def exists(self, owner, project, name):
+ """See `ICharmRecipeSet`."""
+ return self._getByName(owner, project, name) is not None
+
+ def getByName(self, owner, project, name):
+ """See `ICharmRecipeSet`."""
+ recipe = self._getByName(owner, project, name)
+ if recipe is None:
+ raise NoSuchCharmRecipe(name)
+ return recipe
+
def _getRecipesFromCollection(self, collection, owner=None,
visible_by_user=None):
id_column = CharmRecipe.git_repository_id
diff --git a/lib/lp/charms/tests/test_charmrecipe.py b/lib/lp/charms/tests/test_charmrecipe.py
index af7f1e3..edd34d2 100644
--- a/lib/lp/charms/tests/test_charmrecipe.py
+++ b/lib/lp/charms/tests/test_charmrecipe.py
@@ -32,6 +32,7 @@ from lp.buildmaster.interfaces.processor import (
IProcessorSet,
ProcessorNotFound,
)
+from lp.buildmaster.model.buildfarmjob import BuildFarmJob
from lp.buildmaster.model.buildqueue import BuildQueue
from lp.charms.interfaces.charmrecipe import (
BadCharmRecipeSearchContext,
@@ -46,28 +47,40 @@ from lp.charms.interfaces.charmrecipe import (
ICharmRecipeSet,
NoSourceForCharmRecipe,
)
-from lp.charms.interfaces.charmrecipebuild import ICharmRecipeBuild
+from lp.charms.interfaces.charmrecipebuild import (
+ ICharmRecipeBuild,
+ ICharmRecipeBuildSet,
+ )
from lp.charms.interfaces.charmrecipejob import (
ICharmRecipeRequestBuildsJobSource,
)
+from lp.charms.model.charmrecipebuild import CharmFile
+from lp.charms.model.charmrecipejob import CharmRecipeJob
from lp.code.tests.helpers import GitHostingFixture
from lp.registry.interfaces.series import SeriesStatus
from lp.services.database.constants import (
ONE_DAY_AGO,
UTC_NOW,
)
+from lp.services.config import config
from lp.services.database.interfaces import IStore
-from lp.services.database.sqlbase import get_transaction_timestamp
+from lp.services.database.sqlbase import (
+ flush_database_caches,
+ get_transaction_timestamp,
+ )
from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
+from lp.services.job.runner import JobRunner
from lp.services.webapp.snapshot import notify_modified
from lp.testing import (
admin_logged_in,
person_logged_in,
TestCaseWithFactory,
)
+from lp.testing.dbuser import dbuser
from lp.testing.layers import (
DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
)
@@ -500,12 +513,95 @@ class TestCharmRecipe(TestCaseWithFactory):
project = self.factory.makeProduct()
recipe = self.factory.makeCharmRecipe(
registrant=owner, owner=owner, project=project, name="condemned")
- self.assertIsNotNone(
- getUtility(ICharmRecipeSet).getByName(owner, project, "condemned"))
+ self.assertTrue(
+ getUtility(ICharmRecipeSet).exists(owner, project, "condemned"))
with person_logged_in(recipe.owner):
recipe.destroySelf()
- self.assertIsNone(
- getUtility(ICharmRecipeSet).getByName(owner, project, "condemned"))
+ self.assertFalse(
+ getUtility(ICharmRecipeSet).exists(owner, project, "condemned"))
+
+
+class TestCharmRecipeDeleteWithBuilds(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(TestCharmRecipeDeleteWithBuilds, self).setUp()
+ self.useFixture(FeatureFixture({CHARM_RECIPE_ALLOW_CREATE: "on"}))
+
+ def test_delete_with_builds(self):
+ # A charm recipe with build requests and builds can be deleted.
+ # Doing so deletes all its build requests, their builds, and their
+ # files.
+ owner = self.factory.makePerson()
+ project = self.factory.makeProduct()
+ distroseries = self.factory.makeDistroSeries()
+ processor = self.factory.makeProcessor(supports_virtualized=True)
+ das = self.factory.makeDistroArchSeries(
+ distroseries=distroseries, architecturetag=processor.name,
+ processor=processor)
+ das.addOrUpdateChroot(
+ self.factory.makeLibraryFileAlias(
+ filename="fake_chroot.tar.gz", db_only=True))
+ self.useFixture(GitHostingFixture(blob=dedent("""\
+ bases:
+ - build-on:
+ - name: "%s"
+ channel: "%s"
+ architectures: [%s]
+ """ % (
+ distroseries.distribution.name, distroseries.name,
+ processor.name))))
+ [git_ref] = self.factory.makeGitRefs()
+ condemned_recipe = self.factory.makeCharmRecipe(
+ registrant=owner, owner=owner, project=project, name="condemned",
+ git_ref=git_ref)
+ other_recipe = self.factory.makeCharmRecipe(
+ registrant=owner, owner=owner, project=project, git_ref=git_ref)
+ self.assertTrue(
+ getUtility(ICharmRecipeSet).exists(owner, project, "condemned"))
+ with person_logged_in(owner):
+ requests = []
+ jobs = []
+ for recipe in (condemned_recipe, other_recipe):
+ requests.append(recipe.requestBuilds(owner))
+ jobs.append(removeSecurityProxy(requests[-1])._job)
+ with dbuser(config.ICharmRecipeRequestBuildsJobSource.dbuser):
+ JobRunner(jobs).runAll()
+ for job in jobs:
+ self.assertEqual(JobStatus.COMPLETED, job.job.status)
+ [build] = requests[0].builds
+ [other_build] = requests[1].builds
+ charm_file = self.factory.makeCharmFile(build=build)
+ other_charm_file = self.factory.makeCharmFile(build=other_build)
+ store = Store.of(condemned_recipe)
+ store.flush()
+ job_ids = [job.job_id for job in jobs]
+ build_id = build.id
+ build_queue_id = build.buildqueue_record.id
+ build_farm_job_id = removeSecurityProxy(build).build_farm_job_id
+ charm_file_id = removeSecurityProxy(charm_file).id
+ with person_logged_in(condemned_recipe.owner):
+ condemned_recipe.destroySelf()
+ flush_database_caches()
+ # The deleted recipe, its build requests, and its builds are gone.
+ self.assertFalse(
+ getUtility(ICharmRecipeSet).exists(owner, project, "condemned"))
+ self.assertIsNone(store.get(CharmRecipeJob, job_ids[0]))
+ self.assertIsNone(getUtility(ICharmRecipeBuildSet).getByID(build_id))
+ self.assertIsNone(store.get(BuildQueue, build_queue_id))
+ self.assertIsNone(store.get(BuildFarmJob, build_farm_job_id))
+ self.assertIsNone(store.get(CharmFile, charm_file_id))
+ # Unrelated build requests and builds are still present.
+ self.assertEqual(
+ removeSecurityProxy(jobs[1]).context,
+ store.get(CharmRecipeJob, job_ids[1]))
+ self.assertEqual(
+ other_build,
+ getUtility(ICharmRecipeBuildSet).getByID(other_build.id))
+ self.assertIsNotNone(other_build.buildqueue_record)
+ self.assertIsNotNone(
+ store.get(CharmFile, removeSecurityProxy(other_charm_file).id))
class TestCharmRecipeSet(TestCaseWithFactory):
