Merge lp:~cjwatson/launchpad/snap-build-channels-feature-flag into lp:launchpad

Proposed by Colin Watson on 2018-04-30
Status: Needs review
Proposed branch: lp:~cjwatson/launchpad/snap-build-channels-feature-flag
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/snap-build-channels-ui
Diff against target: 241 lines (+105/-15)
6 files modified
lib/lp/snappy/browser/snap.py (+0/-7)
lib/lp/snappy/browser/widgets/snapbuildchannels.py (+21/-0)
lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py (+39/-3)
lib/lp/snappy/interfaces/snap.py (+2/-0)
lib/lp/snappy/model/snapbuildbehaviour.py (+11/-4)
lib/lp/snappy/tests/test_snapbuildbehaviour.py (+32/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snap-build-channels-feature-flag
Reviewer Review Type Date Requested Status
Launchpad code reviewers 2018-04-30 Pending
Review via email: mp+344868@code.launchpad.net

Commit message

Add a feature flag to control the installation source for snapcraft.

To post a comment you must log in.

Unmerged revisions

18628. By Colin Watson on 2018-04-30

Add a feature flag to control the installation source for snapcraft.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/snappy/browser/snap.py'
2--- lib/lp/snappy/browser/snap.py 2018-04-30 22:27:39 +0000
3+++ lib/lp/snappy/browser/snap.py 2018-04-30 22:27:40 +0000
4@@ -334,13 +334,6 @@
5 'auto_build_channels',
6 'store_upload',
7 ])
8- auto_build_channels = copy_field(
9- ISnap['auto_build_channels'],
10- description=(
11- u'The channels to use for build tools when building the snap '
12- u'package. If unset, or if the channel for snapcraft is set to '
13- u'"apt", the default behaviour is to install snapcraft from the '
14- u'source archive using apt.'))
15 store_distro_series = Choice(
16 vocabulary='BuildableSnappyDistroSeries', required=True,
17 title=u'Series')
18
19=== modified file 'lib/lp/snappy/browser/widgets/snapbuildchannels.py'
20--- lib/lp/snappy/browser/widgets/snapbuildchannels.py 2018-04-30 22:27:39 +0000
21+++ lib/lp/snappy/browser/widgets/snapbuildchannels.py 2018-04-30 22:27:40 +0000
22@@ -23,10 +23,12 @@
23 from zope.security.proxy import isinstance as zope_isinstance
24
25 from lp.app.errors import UnexpectedFormData
26+from lp.services.features import getFeatureFlag
27 from lp.services.webapp.interfaces import (
28 IAlwaysSubmittedWidget,
29 ISingleLineWidgetLayout,
30 )
31+from lp.snappy.interfaces.snap import SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG
32
33
34 @implementer(ISingleLineWidgetLayout, IAlwaysSubmittedWidget, IInputWidget)
35@@ -37,6 +39,25 @@
36 snap_names = ["core", "snapcraft"]
37 _widgets_set_up = False
38
39+ def __init__(self, context, request):
40+ super(SnapBuildChannelsWidget, self).__init__(context, request)
41+ self.hint = (
42+ 'The channels to use for build tools when building the snap '
43+ 'package.\n')
44+ default_snapcraft_channel = (
45+ getFeatureFlag(SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG) or "apt")
46+ if default_snapcraft_channel == "apt":
47+ self.hint += (
48+ 'If unset, or if the channel for snapcraft is set to "apt", '
49+ 'the default is to install snapcraft from the source archive '
50+ 'using apt.')
51+ else:
52+ self.hint += (
53+ 'If unset, the default is to install snapcraft from the "%s" '
54+ 'channel. Setting the channel for snapcraft to "apt" causes '
55+ 'snapcraft to be installed from the source archive using '
56+ 'apt.' % default_snapcraft_channel)
57+
58 def setUpSubWidgets(self):
59 if self._widgets_set_up:
60 return
61
62=== modified file 'lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py'
63--- lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py 2018-04-30 22:27:39 +0000
64+++ lib/lp/snappy/browser/widgets/tests/test_snapbuildchannelswidget.py 2018-04-30 22:27:40 +0000
65@@ -14,10 +14,12 @@
66 from zope.schema import Dict
67
68 from lp.services.beautifulsoup import BeautifulSoup
69+from lp.services.features.testing import FeatureFixture
70 from lp.services.webapp.servers import LaunchpadTestRequest
71 from lp.snappy.browser.widgets.snapbuildchannels import (
72 SnapBuildChannelsWidget,
73 )
74+from lp.snappy.interfaces.snap import SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG
75 from lp.testing import (
76 TestCaseWithFactory,
77 verifyObject,
78@@ -35,9 +37,9 @@
79 __name__="auto_build_channels",
80 title="Source snap channels for automatic builds")
81 self.context = self.factory.makeSnap()
82- field = field.bind(self.context)
83- request = LaunchpadTestRequest()
84- self.widget = SnapBuildChannelsWidget(field, request)
85+ self.field = field.bind(self.context)
86+ self.request = LaunchpadTestRequest()
87+ self.widget = SnapBuildChannelsWidget(self.field, self.request)
88
89 def test_implements(self):
90 self.assertTrue(verifyObject(IBrowserWidget, self.widget))
91@@ -48,6 +50,40 @@
92 self.widget.template.filename.endswith("snapbuildchannels.pt"),
93 "Template was not set up.")
94
95+ def test_hint_no_feature_flag(self):
96+ self.assertEqual(
97+ 'The channels to use for build tools when building the snap '
98+ 'package.\n'
99+ 'If unset, or if the channel for snapcraft is set to "apt", '
100+ 'the default is to install snapcraft from the source archive '
101+ 'using apt.',
102+ self.widget.hint)
103+
104+ def test_hint_feature_flag_apt(self):
105+ self.useFixture(
106+ FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "apt"}))
107+ widget = SnapBuildChannelsWidget(self.field, self.request)
108+ self.assertEqual(
109+ 'The channels to use for build tools when building the snap '
110+ 'package.\n'
111+ 'If unset, or if the channel for snapcraft is set to "apt", '
112+ 'the default is to install snapcraft from the source archive '
113+ 'using apt.',
114+ widget.hint)
115+
116+ def test_hint_feature_flag_real_channel(self):
117+ self.useFixture(
118+ FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "stable"}))
119+ widget = SnapBuildChannelsWidget(self.field, self.request)
120+ self.assertEqual(
121+ 'The channels to use for build tools when building the snap '
122+ 'package.\n'
123+ 'If unset, the default is to install snapcraft from the "stable" '
124+ 'channel. Setting the channel for snapcraft to "apt" causes '
125+ 'snapcraft to be installed from the source archive using '
126+ 'apt.',
127+ widget.hint)
128+
129 def test_setUpSubWidgets_first_call(self):
130 # The subwidgets are set up and a flag is set.
131 self.widget.setUpSubWidgets()
132
133=== modified file 'lib/lp/snappy/interfaces/snap.py'
134--- lib/lp/snappy/interfaces/snap.py 2018-04-21 10:01:22 +0000
135+++ lib/lp/snappy/interfaces/snap.py 2018-04-30 22:27:40 +0000
136@@ -19,6 +19,7 @@
137 'NoSourceForSnap',
138 'NoSuchSnap',
139 'SNAP_PRIVATE_FEATURE_FLAG',
140+ 'SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG',
141 'SNAP_TESTING_FLAGS',
142 'SNAP_WEBHOOKS_FEATURE_FLAG',
143 'SnapAuthorizationBadMacaroon',
144@@ -102,6 +103,7 @@
145
146
147 SNAP_PRIVATE_FEATURE_FLAG = u"snap.allow_private"
148+SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG = u"snap.channels.snapcraft"
149 SNAP_WEBHOOKS_FEATURE_FLAG = u"snap.webhooks.enabled"
150
151
152
153=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
154--- lib/lp/snappy/model/snapbuildbehaviour.py 2018-04-30 22:27:39 +0000
155+++ lib/lp/snappy/model/snapbuildbehaviour.py 2018-04-30 22:27:40 +0000
156@@ -30,8 +30,12 @@
157 )
158 from lp.registry.interfaces.series import SeriesStatus
159 from lp.services.config import config
160+from lp.services.features import getFeatureFlag
161 from lp.services.webapp import canonical_url
162-from lp.snappy.interfaces.snap import SnapBuildArchiveOwnerMismatch
163+from lp.snappy.interfaces.snap import (
164+ SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
165+ SnapBuildArchiveOwnerMismatch,
166+ )
167 from lp.snappy.interfaces.snapbuild import ISnapBuild
168 from lp.soyuz.adapters.archivedependencies import (
169 get_sources_list_for_building,
170@@ -110,12 +114,15 @@
171 logger=logger))
172 args["archive_private"] = build.archive.private
173 args["build_url"] = canonical_url(build)
174- if (build.channels is not None and
175- build.channels.get("snapcraft") != "apt"):
176+ channels = build.channels or {}
177+ if "snapcraft" not in channels:
178+ channels["snapcraft"] = (
179+ getFeatureFlag(SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG) or "apt")
180+ if channels.get("snapcraft") != "apt":
181 # We have to remove the security proxy that Zope applies to this
182 # dict, since otherwise we'll be unable to serialise it to
183 # XML-RPC.
184- args["channels"] = removeSecurityProxy(build.channels)
185+ args["channels"] = removeSecurityProxy(channels)
186 if build.snap.branch is not None:
187 args["branch"] = build.snap.branch.bzr_identity
188 elif build.snap.git_ref is not None:
189
190=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
191--- lib/lp/snappy/tests/test_snapbuildbehaviour.py 2018-04-30 22:27:39 +0000
192+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py 2018-04-30 22:27:40 +0000
193@@ -55,9 +55,13 @@
194 from lp.registry.interfaces.pocket import PackagePublishingPocket
195 from lp.registry.interfaces.series import SeriesStatus
196 from lp.services.config import config
197+from lp.services.features.testing import FeatureFixture
198 from lp.services.log.logger import BufferLogger
199 from lp.services.webapp import canonical_url
200-from lp.snappy.interfaces.snap import SnapBuildArchiveOwnerMismatch
201+from lp.snappy.interfaces.snap import (
202+ SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
203+ SnapBuildArchiveOwnerMismatch,
204+ )
205 from lp.snappy.model.snapbuildbehaviour import SnapBuildBehaviour
206 from lp.soyuz.adapters.archivedependencies import (
207 get_sources_list_for_building,
208@@ -438,6 +442,33 @@
209 self.assertNotIn("channels", args)
210
211 @defer.inlineCallbacks
212+ def test_extraBuildArgs_channels_feature_flag_real_channel(self):
213+ # If the snap.channels.snapcraft feature flag is set, it identifies
214+ # the default channel to be used for snapcraft.
215+ self.useFixture(
216+ FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "stable"}))
217+ job = self.makeJob()
218+ expected_archives, expected_trusted_keys = (
219+ yield get_sources_list_for_building(
220+ job.build, job.build.distro_arch_series, None))
221+ args = yield job._extraBuildArgs()
222+ self.assertFalse(isProxy(args["channels"]))
223+ self.assertEqual({"snapcraft": "stable"}, args["channels"])
224+
225+ @defer.inlineCallbacks
226+ def test_extraBuildArgs_channels_feature_flag_overridden(self):
227+ # The snap.channels.snapcraft feature flag can be overridden by
228+ # explicit configuration.
229+ self.useFixture(
230+ FeatureFixture({SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG: "stable"}))
231+ job = self.makeJob(channels={"snapcraft": "apt"})
232+ expected_archives, expected_trusted_keys = (
233+ yield get_sources_list_for_building(
234+ job.build, job.build.distro_arch_series, None))
235+ args = yield job._extraBuildArgs()
236+ self.assertNotIn("channels", args)
237+
238+ @defer.inlineCallbacks
239 def test_extraBuildArgs_disallow_internet(self):
240 # If external network access is not allowed for the snap,
241 # _extraBuildArgs does not dispatch a proxy token.