launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23754
[Merge] lp:~twom/launchpad/snap-release-intents into lp:launchpad
Tom Wardill has proposed merging lp:~twom/launchpad/snap-release-intents into lp:launchpad.
Commit message:
Use release intents to release snapbuilds to channels after building
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~twom/launchpad/snap-release-intents/+merge/369028
Use the SCA support for 'release_intents' and pass the required parameters to produce a release to a channel after review stage.
Pass `channels` and `only_if_newer` to ensure that the build is only released if it is newer that the current existing build in the specified channels.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/snap-release-intents into lp:launchpad.
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2018-12-18 18:10:39 +0000
+++ lib/lp/snappy/model/snapbuildjob.py 2019-06-19 10:53:29 +0000
@@ -350,7 +350,6 @@
raise ManualReview(
"Package held for manual review on the store; "
"cannot release it automatically.")
- client.release(self.snapbuild, self.store_revision)
self.error_message = None
except self.retry_error_types:
raise
=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py 2019-06-14 14:46:15 +0000
+++ lib/lp/snappy/model/snapstoreclient.py 2019-06-19 10:53:29 +0000
@@ -266,6 +266,14 @@
"series": snap.store_series.name,
"built_at": snapbuild.date_started.isoformat(),
}
+ # The security proxy is useless and breaks JSON serialisation.
+ channels = removeSecurityProxy(snap.store_channels)
+ if channels:
+ # This will cause a release
+ data.update({
+ "channels": channels,
+ "only_if_newer": True,
+ })
# XXX cjwatson 2016-05-09: This should add timeline information, but
# that's currently difficult in jobs.
try:
@@ -394,41 +402,3 @@
if channels is None:
channels = _default_store_channels
return channels
-
- @classmethod
- def _release(cls, snap, revision):
- """Release a snap revision to specified channels."""
- release_url = urlappend(
- config.snappy.store_url, "dev/api/snap-release/")
- data = {
- "name": snap.store_name,
- "revision": revision,
- # The security proxy is useless and breaks JSON serialisation.
- "channels": removeSecurityProxy(snap.store_channels),
- "series": snap.store_series.name,
- }
- # XXX cjwatson 2016-06-28: This should add timeline information, but
- # that's currently difficult in jobs.
- try:
- assert snap.store_secrets is not None
- urlfetch(
- release_url, method="POST", json=data,
- auth=MacaroonAuth(
- snap.store_secrets["root"],
- snap.store_secrets.get("discharge")))
- except requests.HTTPError as e:
- if e.response.status_code == 401:
- if (e.response.headers.get("WWW-Authenticate") ==
- "Macaroon needs_refresh=1"):
- raise NeedsRefreshResponse()
- raise cls._makeSnapStoreError(ReleaseFailedResponse, e)
-
- @classmethod
- def release(cls, snapbuild, revision):
- """See `ISnapStoreClient`."""
- assert config.snappy.store_url is not None
- snap = snapbuild.snap
- assert snap.store_name is not None
- assert snap.store_series is not None
- assert snap.store_channels
- cls.refreshIfNecessary(snap, cls._release, snap, revision)
=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py 2019-05-22 14:57:45 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py 2019-06-19 10:53:29 +0000
@@ -65,7 +65,6 @@
self.upload = FakeMethod()
self.checkStatus = FakeMethod()
self.listChannels = FakeMethod(result=[])
- self.release = FakeMethod()
class TestSnapBuildJob(TestCaseWithFactory):
@@ -160,7 +159,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
@@ -180,7 +178,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
@@ -206,7 +203,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
@@ -253,7 +249,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
@@ -272,7 +267,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
@@ -298,7 +292,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
@@ -349,7 +342,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
@@ -399,7 +391,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
@@ -418,7 +409,6 @@
JobRunner([job]).runAll()
self.assertEqual([], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
@@ -448,7 +438,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.store_url)
self.assertIsNone(job.store_revision)
@@ -497,7 +486,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([((snapbuild, 1), {})], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertEqual(1, job.store_revision)
@@ -525,7 +513,6 @@
JobRunner([job]).runAll()
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([], client.release.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual(self.store_url, job.store_url)
self.assertIsNone(job.store_revision)
@@ -558,57 +545,6 @@
self.assertWebhookDeliveries(
snapbuild, ["Pending", "Failed to release to channels"])
- def test_run_release_failure_notifies(self):
- # A run configured to automatically release the package to certain
- # channels but that fails to do so sends mail.
- requester = self.factory.makePerson(name="requester")
- requester_team = self.factory.makeTeam(
- owner=requester, name="requester-team", members=[requester])
- snapbuild = self.makeSnapBuild(
- requester=requester_team, name="test-snap", owner=requester_team,
- store_channels=["stable", "edge"])
- self.assertContentEqual([], snapbuild.store_upload_jobs)
- job = SnapStoreUploadJob.create(snapbuild)
- client = FakeSnapStoreClient()
- client.upload.result = self.status_url
- client.checkStatus.result = (self.store_url, 1)
- client.release.failure = ReleaseFailedResponse("Failed to publish")
- self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
- with dbuser(config.ISnapStoreUploadJobSource.dbuser):
- JobRunner([job]).runAll()
- self.assertEqual([((snapbuild,), {})], client.upload.calls)
- self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
- self.assertEqual([((snapbuild, 1), {})], client.release.calls)
- self.assertContentEqual([job], snapbuild.store_upload_jobs)
- self.assertEqual(self.store_url, job.store_url)
- self.assertEqual(1, job.store_revision)
- self.assertEqual("Failed to publish", job.error_message)
- [notification] = pop_notifications()
- self.assertEqual(
- config.canonical.noreply_from_address, notification["From"])
- self.assertEqual(
- "Requester <%s>" % requester.preferredemail.email,
- notification["To"])
- subject = notification["Subject"].replace("\n ", " ")
- self.assertEqual("Store release failed for test-snap", subject)
- self.assertEqual(
- "Requester @requester-team",
- notification["X-Launchpad-Message-Rationale"])
- self.assertEqual(
- requester_team.name, notification["X-Launchpad-Message-For"])
- self.assertEqual(
- "snap-build-release-failed",
- notification["X-Launchpad-Notification-Type"])
- body, footer = notification.get_payload(decode=True).split("\n-- \n")
- self.assertIn("Failed to publish", body)
- self.assertIn(self.store_url, body)
- self.assertEqual(
- "http://launchpad.test/~requester-team/+snap/test-snap/+build/%d\n";
- "Your team Requester Team is the requester of the build.\n" %
- snapbuild.id, footer)
- self.assertWebhookDeliveries(
- snapbuild, ["Pending", "Failed to release to channels"])
-
def test_retry_delay(self):
# The job is retried every minute, unless it just made one of its
# first four attempts to poll the status endpoint, in which case the
@@ -655,15 +591,12 @@
client = FakeSnapStoreClient()
client.upload.result = self.status_url
client.checkStatus.result = (self.store_url, 1)
- client.release.failure = UploadFailedResponse(
- "Proxy error", can_retry=True)
self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
with dbuser(config.ISnapStoreUploadJobSource.dbuser):
JobRunner([job]).runAll()
previous_upload = client.upload.calls
previous_checkStatus = client.checkStatus.calls
- len_previous_release = len(client.release.calls)
# Check we uploaded as expected
self.assertEqual(self.store_url, job.store_url)
@@ -676,10 +609,10 @@
with dbuser(config.ISnapStoreUploadJobSource.dbuser):
JobRunner([job]).runAll()
- # We should not have called `upload`, but moved straight to `release`
+ # Release is not called due to release intent in upload
+ # but ensure that we have not called upload twice
self.assertEqual(previous_upload, client.upload.calls)
self.assertEqual(previous_checkStatus, client.checkStatus.calls)
- self.assertEqual(len_previous_release + 1, len(client.release.calls))
self.assertIsNone(job.error_message)
def test_with_snapbuild_metadata_as_none(self):
=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py 2019-06-14 14:49:25 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py 2019-06-19 10:53:29 +0000
@@ -402,6 +402,41 @@
]))
@responses.activate
+ def test_upload_with_release_intent(self):
+ snapbuild = self.makeUploadableSnapBuild()
+ snapbuild.snap.store_channels = ['beta', 'edge']
+ transaction.commit()
+ self._addUnscannedUploadResponse()
+ self._addSnapPushResponse()
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ self.assertEqual(
+ "http://sca.example/dev/api/snaps/1/builds/1/status";,
+ self.client.upload(snapbuild))
+ requests = [call.request for call in responses.calls]
+ self.assertThat(requests, MatchesListwise([
+ RequestMatches(
+ url=Equals("http://updown.example/unscanned-upload/";),
+ method=Equals("POST"),
+ form_data={
+ "binary": MatchesStructure.byEquality(
+ name="binary", filename="test-snap.snap",
+ value="dummy snap content",
+ type="application/octet-stream",
+ )}),
+ RequestMatches(
+ url=Equals("http://sca.example/dev/api/snap-push/";),
+ method=Equals("POST"),
+ headers=ContainsDict(
+ {"Content-Type": Equals("application/json")}),
+ auth=("Macaroon", MacaroonsVerify(self.root_key)),
+ json_data={
+ "name": "test-snap", "updown_id": 1, "series": "rolling",
+ "built_at": snapbuild.date_started.isoformat(),
+ "only_if_newer": True, "channels": ['beta', 'edge'],
+ }),
+ ]))
+
+ @responses.activate
def test_upload_no_discharge(self):
root_key = hashlib.sha256(self.factory.getUniqueString()).hexdigest()
root_macaroon = Macaroon(key=root_key)
@@ -638,100 +673,3 @@
self.assertContentEqual([], responses.calls)
self.assertIsNone(
getUtility(IMemcacheClient).get(self.channels_memcache_key))
-
- @responses.activate
- def test_release(self):
- snap = self.factory.makeSnap(
- store_upload=True,
- store_series=self.factory.makeSnappySeries(name="rolling"),
- store_name="test-snap",
- store_secrets=self._make_store_secrets(),
- store_channels=["stable", "edge"])
- snapbuild = self.factory.makeSnapBuild(snap=snap)
- self._addSnapReleaseResponse()
- self.client.release(snapbuild, 1)
- self.assertThat(responses.calls[-1].request, RequestMatches(
- url=Equals("http://sca.example/dev/api/snap-release/";),
- method=Equals("POST"),
- headers=ContainsDict({"Content-Type": Equals("application/json")}),
- auth=("Macaroon", MacaroonsVerify(self.root_key)),
- json_data={
- "name": "test-snap", "revision": 1,
- "channels": ["stable", "edge"], "series": "rolling",
- }))
-
- @responses.activate
- def test_release_no_discharge(self):
- root_key = hashlib.sha256(self.factory.getUniqueString()).hexdigest()
- root_macaroon = Macaroon(key=root_key)
- snap = self.factory.makeSnap(
- store_upload=True,
- store_series=self.factory.makeSnappySeries(name="rolling"),
- store_name="test-snap",
- store_secrets={"root": root_macaroon.serialize()},
- store_channels=["stable", "edge"])
- snapbuild = self.factory.makeSnapBuild(snap=snap)
- self._addSnapReleaseResponse()
- self.client.release(snapbuild, 1)
- self.assertThat(responses.calls[-1].request, RequestMatches(
- url=Equals("http://sca.example/dev/api/snap-release/";),
- method=Equals("POST"),
- headers=ContainsDict({"Content-Type": Equals("application/json")}),
- auth=("Macaroon", MacaroonsVerify(root_key)),
- json_data={
- "name": "test-snap", "revision": 1,
- "channels": ["stable", "edge"], "series": "rolling",
- }))
-
- @responses.activate
- def test_release_needs_discharge_macaroon_refresh(self):
- store_secrets = self._make_store_secrets()
- snap = self.factory.makeSnap(
- store_upload=True,
- store_series=self.factory.makeSnappySeries(name="rolling"),
- store_name="test-snap", store_secrets=store_secrets,
- store_channels=["stable", "edge"])
- snapbuild = self.factory.makeSnapBuild(snap=snap)
- responses.add(
- "POST", "http://sca.example/dev/api/snap-release/";, status=401,
- headers={"WWW-Authenticate": "Macaroon needs_refresh=1"})
- self._addMacaroonRefreshResponse()
- self._addSnapReleaseResponse()
- self.client.release(snapbuild, 1)
- requests = [call.request for call in responses.calls]
- self.assertThat(requests, MatchesListwise([
- MatchesStructure.byEquality(path_url="/dev/api/snap-release/"),
- MatchesStructure.byEquality(path_url="/api/v2/tokens/refresh"),
- MatchesStructure.byEquality(path_url="/dev/api/snap-release/"),
- ]))
- self.assertNotEqual(
- store_secrets["discharge"], snap.store_secrets["discharge"])
-
- @responses.activate
- def test_release_error(self):
- snap = self.factory.makeSnap(
- store_upload=True,
- store_series=self.factory.makeSnappySeries(name="rolling"),
- store_name="test-snap", store_secrets=self._make_store_secrets(),
- store_channels=["stable", "edge"])
- snapbuild = self.factory.makeSnapBuild(snap=snap)
- responses.add(
- "POST", "http://sca.example/dev/api/snap-release/";, status=503,
- json={"error_list": [{"message": "Failed to publish"}]})
- self.assertRaisesWithContent(
- ReleaseFailedResponse, "Failed to publish",
- self.client.release, snapbuild, 1)
-
- @responses.activate
- def test_release_404(self):
- snap = self.factory.makeSnap(
- store_upload=True,
- store_series=self.factory.makeSnappySeries(name="rolling"),
- store_name="test-snap", store_secrets=self._make_store_secrets(),
- store_channels=["stable", "edge"])
- snapbuild = self.factory.makeSnapBuild(snap=snap)
- responses.add(
- "POST", "http://sca.example/dev/api/snap-release/";, status=404)
- self.assertRaisesWithContent(
- ReleaseFailedResponse, b"404 Client Error: Not Found",
- self.client.release, snapbuild, 1)
Follow ups
