Merge ~ilasc/launchpad:add-oci-feature-flag into launchpad:master

[Ioana Lasc]

Add OCI builds feature flag

Added an OCI feature flag (oci.build_series.<distribution>) and integrated
it with the distro_arch_series property of OCIRecipeBuild.
The feature rule is a string that names the series to use
as the OCI recipe build distroseries for a given distribution.

[Colin Watson (cjwatson)]

review: Needs Fixing

9e166b6... by Ioana Lasc on 2020-03-19

Changes to OCI feature flag implementation

We want the feature rule to be a string that names
the series to use as the OCI recipe build distroseries
for a given distribution, instead of a boolean flag
that controls whether to use bionic.

[Ioana Lasc (ilasc)]

Thanks Colin! Hopefully it's closer to what we want now.

[Tom Wardill (twom)]

It's looking better to me, but I found some things while having a look through :)

review: Needs Fixing

8af9e49... by Ioana Lasc on 2020-03-20

Make changes to OCI feature flag

Addressed code review comments, changed the flag
type and added initial value.

68d2115... by Ioana Lasc on 2020-03-20

Merge branch 'master' into add-oci-feature-flag

[Ioana Lasc (ilasc)]

Thank you Tom! Made what I believe would be the necessary changes. Now ready for the next review.

[Colin Watson (cjwatson)]

Definitely getting better, but a few more tidy-ups I think, mainly in tests.

review: Needs Fixing

[Colin Watson (cjwatson)]

Also could you sync up the MP description with the current behaviour? This can be useful to future travellers looking up why a change was made.

8f5f127... by Ioana Lasc on 2020-03-26

Cleanup changes for OCI feature flag

Code cleanup and addressing code review comments for the OCI
feature flag names the series to use as the OCI recipe build
distroseries for a given distribution.

[Ioana Lasc (ilasc)]

Thank you Colin, addressed the comments and changed the Description. Ready for another review.

[Colin Watson (cjwatson)]

Just one more thing ...

8743ef0... by Ioana Lasc on 2020-03-26

Remove NoSuchDistroSeries from distro_arch_series

Removed the try-catch code in distro_arch_series from OCIRecipeBuild,
if the series named in the feature rule doesn't exist, we want to crash
instead of silently handling it.

[Ioana Lasc (ilasc)]

Agreed on your suggested approach to crash if the series doesn't exist instead of silently handling it. Thanks Colin!

[Colin Watson (cjwatson)]

review: Approve

[Tom Wardill (twom)]

review: Approve

diff --git a/lib/lp/oci/model/ocirecipebuild.py b/lib/lp/oci/model/ocirecipebuild.py
index 89b7215..c36fce4 100644
--- a/lib/lp/oci/model/ocirecipebuild.py
+++ b/lib/lp/oci/model/ocirecipebuild.py
@@ -55,6 +55,7 @@ from lp.services.database.interfaces import (
     IMasterStore,
     IStore,
     )
+from lp.services.features import getFeatureFlag
 from lp.services.librarian.model import (
     LibraryFileAlias,
     LibraryFileContent,
@@ -271,8 +272,17 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
 
     @property
     def distro_arch_series(self):
-        return self.distribution.currentseries.getDistroArchSeriesByProcessor(
-            self.processor)
+        # For OCI builds we default to the series set by the feature flag.
+        # If the feature flag is not set we default to current series under
+        # the OCIRecipeBuild distribution.
+
+        oci_series = getFeatureFlag('oci.build_series.%s'
+                                    % self.distribution.name)
+        if oci_series:
+            oci_series = self.distribution.getSeries(oci_series)
+        else:
+            oci_series = self.distribution.currentseries
+        return oci_series.getDistroArchSeriesByProcessor(self.processor)
 
     def updateStatus(self, status, builder=None, slave_status=None,
                      date_started=None, date_finished=None,
diff --git a/lib/lp/oci/tests/test_ocirecipebuild.py b/lib/lp/oci/tests/test_ocirecipebuild.py
index fcce007..eec8850 100644
--- a/lib/lp/oci/tests/test_ocirecipebuild.py
+++ b/lib/lp/oci/tests/test_ocirecipebuild.py
@@ -226,13 +226,32 @@ class TestOCIRecipeBuildSet(TestCaseWithFactory):
         distro_arch_series = self.factory.makeDistroArchSeries(
             distroseries=distroseries, architecturetag="i386",
             processor=processor)
-        distro_arch_series = self.factory.makeDistroArchSeries()
         oci_project = self.factory.makeOCIProject(pillar=distribution)
         recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
         target = getUtility(IOCIRecipeBuildSet).new(
             requester, recipe, distro_arch_series)
         with admin_logged_in():
             self.assertProvides(target, IOCIRecipeBuild)
+            self.assertEqual(distro_arch_series, target.distro_arch_series)
+
+    def test_new_oci_feature_flag_enabled(self):
+        requester = self.factory.makePerson()
+        distribution = self.factory.makeDistribution()
+        distroseries = self.factory.makeDistroSeries(
+            distribution=distribution, status=SeriesStatus.CURRENT)
+        processor = getUtility(IProcessorSet).getByName("386")
+        self.useFixture(FeatureFixture({
+            "oci.build_series.%s" % distribution.name: distroseries.name}))
+        distro_arch_series = self.factory.makeDistroArchSeries(
+            distroseries=distroseries, architecturetag="i386",
+            processor=processor)
+        oci_project = self.factory.makeOCIProject(pillar=distribution)
+        recipe = self.factory.makeOCIRecipe(oci_project=oci_project)
+        target = getUtility(IOCIRecipeBuildSet).new(
+            requester, recipe, distro_arch_series)
+        with admin_logged_in():
+            self.assertProvides(target, IOCIRecipeBuild)
+            self.assertEqual(distro_arch_series, target.distro_arch_series)
 
     def test_getByID(self):
         builds = [self.factory.makeOCIRecipeBuild() for x in range(3)]
diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
index eeefc88..58b6adc 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
@@ -33,7 +33,6 @@ from testtools.twistedsupport import (
     AsynchronousDeferredRunTestForBrokenTwisted,
     )
 from twisted.internet import defer
-from twisted.trial.unittest import TestCase as TrialTestCase
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -67,6 +66,7 @@ from lp.buildmaster.tests.test_buildfarmjobbehaviour import (
 from lp.oci.model.ocirecipebuildbehaviour import OCIRecipeBuildBehaviour
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.config import config
+from lp.services.features.testing import FeatureFixture
 from lp.services.log.logger import DevNullLogger
 from lp.services.webapp import canonical_url
 from lp.soyuz.adapters.archivedependencies import (
@@ -95,12 +95,13 @@ class MakeOCIBuildMixin:
         build.queueBuild()
         return build
 
-    def makeJob(self, git_ref, recipe=None):
+    def makeJob(self, git_ref, recipe=None, build=None):
         """Create a sample `IOCIRecipeBuildBehaviour`."""
-        if recipe is None:
-            build = self.factory.makeOCIRecipeBuild()
-        else:
-            build = self.factory.makeOCIRecipeBuild(recipe=recipe)
+        if build is None:
+            if recipe is None:
+                build = self.factory.makeOCIRecipeBuild()
+            else:
+                build = self.factory.makeOCIRecipeBuild(recipe=recipe)
         build.recipe.git_ref = git_ref
 
         job = IBuildFarmJobBehaviour(build)
@@ -339,6 +340,45 @@ class TestAsyncOCIRecipeBuildBehaviour(MakeOCIBuildMixin, TestCaseWithFactory):
         self.assertEqual(
             ('ensurepresent', chroot_lfa.http_url, '', ''), slave.call_log[0])
 
+    @defer.inlineCallbacks
+    def test_dispatchBuildToSlave_oci_feature_flag_enabled(self):
+        self.pushConfig("snappy", builder_proxy_host=None)
+        [ref] = self.factory.makeGitRefs()
+
+        distribution = self.factory.makeDistribution()
+        distroseries = self.factory.makeDistroSeries(
+            distribution=distribution, status=SeriesStatus.CURRENT)
+        processor = getUtility(IProcessorSet).getByName("386")
+        self.useFixture(FeatureFixture({
+            "oci.build_series.%s" % distribution.name: distroseries.name}))
+        distro_arch_series = self.factory.makeDistroArchSeries(
+            distroseries=distroseries, architecturetag="i386",
+            processor=processor)
+
+        build = self.factory.makeOCIRecipeBuild(
+            distro_arch_series=distro_arch_series)
+        job = self.makeJob(git_ref=ref, build=build)
+        builder = MockBuilder()
+        builder.processor = job.build.processor
+        slave = OkSlave()
+        job.setBuilder(builder, slave)
+        chroot_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+
+        job.build.distro_arch_series.addOrUpdateChroot(
+            chroot_lfa, image_type=BuildBaseImageType.CHROOT)
+        lxd_lfa = self.factory.makeLibraryFileAlias(db_only=True)
+        job.build.distro_arch_series.addOrUpdateChroot(
+            lxd_lfa, image_type=BuildBaseImageType.LXD)
+        yield job.dispatchBuildToSlave(DevNullLogger())
+        self.assertEqual(distroseries.name,
+            job.build.distro_arch_series.distroseries.name)
+        self.assertEqual(
+            ('ensurepresent', lxd_lfa.http_url, '', ''), slave.call_log[0])
+        # grab the build method log from the OKMockSlave
+        # and check inside the arguments dict that we build
+        # for Distro series
+        self.assertEqual(distroseries.name, slave.call_log[1][5]['series'])
+
 
 class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
                                         TestCaseWithFactory):