PR#1662: CGUninitBuild for cancelling CG reservations - koji

koji

#1662 CGUninitBuild for cancelling CG reservations

Merged 4 years ago by tkopecek. Opened 4 years ago by tkopecek.

tkopecek/koji issue1610 into master

CGUninitBuild for cancelling CG reservations

Tomas Kopecek • 4 years ago

4e6171a

hub/kojihub.py

file modified

+38

		`@@ -5816,6 +5816,43 @@`
		`return {'build_id': build_id, 'token': token}`


		`+ def cg_refund_build(cg, build_id, token, state=koji.BUILD_STATES['FAILED']):`
		`+ """If build is reserved and not finished yet, there is an option`
		`+ to release reservation and mark build either FAILED or CANCELED.`
		`+ For this calling CG needs to know build_id and reservation token.`
		`+`
		`+ On success it doesn't return nothing. On error it raises an exception.`
		`+ """`
		`+`
		`+ if state not in (koji.BUILD_STATES['FAILED'], koji.BUILD_STATES['CANCELED']):`
		`+ raise koji.GenericError("Only FAILED/CANCELLED build states are allowed")`
		`+`
		`+ assert_cg(cg)`
		`+ binfo = get_build(build_id, strict=True)`
		`+ if binfo['state'] != koji.BUILD_STATES['BUILDING']:`
		`+ raise koji.GenericError('Build ID %s is not in BUILDING state' % build_id)`
		`+`
		`+ build_token = get_reservation_token(build_id)`
		`+ if not build_token or build_token['token'] != token:`
		`+ raise koji.GenericError("Token doesn't match build ID %s" % build_id)`
		`+`
		`+ cg_id = lookup_name('content_generator', cg, strict=True)['id']`
		`+ if binfo['cg_id'] != cg_id:`
		`+ raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id)`
		`+`
		`+ koji.plugin.run_callbacks('preBuildStateChange', attribute='state',`
		`+ old=koji.BUILD_STATES['BUILDING'], new=state, info=binfo)`
		`+`
		`+ update = UpdateProcessor('build', values={'id': build_id}, clauses=["id = %(id)s"])`
		`+ update.set(state=state)`
		`+ update.rawset(completion_time='NOW()')`
		`+ update.execute()`
		`+`
		`+ binfo = get_build(build_id, strict=True)`
		`+ koji.plugin.run_callbacks('postBuildStateChange', attribute='state',`
		`+ old=koji.BUILD_STATES['BUILDING'], new=state, info=binfo)`
		`+`
		`+`
		`def cg_import(metadata, directory, token=None):`
		`"""Import build from a content generator`

		`@@ -9939,6 +9976,7 @@`
		`import_archive(fullpath, buildinfo, type, typeInfo)`

		`CGInitBuild = staticmethod(cg_init_build)`
		`+ CGRefundBuild = staticmethod(cg_refund_build)`
		`CGImport = staticmethod(cg_import)`

		`untaggedBuilds = staticmethod(untagged_builds)`

tests/test_hub/test_cg_importer.py

file modified

+133

		`@@ -11,6 +11,8 @@`
		`import kojihub`
		`from koji import GenericError`

		`+ IP = kojihub.InsertProcessor`
		`+ UP = kojihub.UpdateProcessor`

		`class TestCGImporter(unittest.TestCase):`
		`TMP_PATH = os.path.join(os.path.dirname(__file__), 'tmptest')`
		`@@ -224,3 +226,134 @@`
		`self.get_build.return_value = self.build1`
		`with self.assertRaises(koji.GenericError):`
		`self.importer.match_kojifile(comp)`
		`+`
		`+`
		`+ class TestCGReservation(unittest.TestCase):`
		`+ def getInsert(self, args, *kwargs):`
		`+ insert = IP(args, *kwargs)`
		`+ insert.execute = mock.MagicMock()`
		`+ self.inserts.append(insert)`
		`+ return insert`
		`+`
		`+ def getUpdate(self, args, *kwargs):`
		`+ update = UP(args, *kwargs)`
		`+ update.execute = mock.MagicMock()`
		`+ self.updates.append(update)`
		`+ return update`
		`+`
		`+`
		`+ def setUp(self):`
		`+ self.InsertProcessor = mock.patch('kojihub.InsertProcessor',`
		`+ side_effect=self.getInsert).start()`
		`+ self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor',`
		`+ side_effect=self.getUpdate).start()`
		`+ self.inserts = []`
		`+ self.updates = []`
		`+`
		`+ self.context = mock.patch('kojihub.context').start()`
		`+ self.context.session.user_id = 123456`
		`+ self.mock_cursor = mock.MagicMock()`
		`+ self.context.cnx.cursor.return_value = self.mock_cursor`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ @mock.patch("kojihub.new_build")`
		`+ @mock.patch("kojihub.get_user")`
		`+ @mock.patch("kojihub.lookup_name")`
		`+ @mock.patch("kojihub.assert_cg")`
		`+ def test_init_build_ok(self, assert_cg, lookup_name, get_user, new_build):`
		`+ assert_cg.return_value = True`
		`+ lookup_name.return_value = {'id': 21, 'name': 'cg_name'}`
		`+ get_user.return_value = {'id': 123456, 'name': 'username'}`
		`+ new_build.return_value = 654`
		`+ cg = 'content_generator_name'`
		`+ self.mock_cursor.fetchone.side_effect = [`
		`+ [333], # get pkg_id`
		`+ [1234], # get nextval pkg_id`
		`+ ]`
		`+ self.mock_cursor.fetchall.side_effect = [`
		`+ [[]],`
		`+ ]`
		`+`
		`+ data = {`
		`+ 'name': 'pkg_name',`
		`+ 'version': 'pkg_version',`
		`+ 'release': 'pkg_release',`
		`+ 'extra': {},`
		`+ }`
		`+`
		`+ kojihub.cg_init_build(cg, data)`
		`+`
		`+ lookup_name.assert_called_once_with('content_generator', cg, strict=True)`
		`+ assert_cg.assert_called_once_with(cg)`
		`+ self.assertEqual(1, len(self.inserts))`
		`+ insert = self.inserts[0]`
		`+ self.assertEqual(insert.table, 'build_reservations')`
		`+ self.assertEqual(insert.data['build_id'], 654)`
		`+ self.assertTrue('token' in insert.data)`
		`+ self.assertEqual(insert.rawdata, {'created': 'NOW()'})`
		`+`
		`+ @mock.patch("koji.plugin.run_callbacks")`
		`+ @mock.patch("kojihub.get_reservation_token")`
		`+ @mock.patch("kojihub.lookup_name")`
		`+ @mock.patch("kojihub.get_build")`
		`+ @mock.patch("kojihub.assert_cg")`
		`+ def test_uninit_build_ok(self, assert_cg, get_build, lookup_name, get_reservation_token,`
		`+ run_callbacks):`
		`+ assert_cg.return_value = True`
		`+ build_id = 1122`
		`+ cg_id = 888`
		`+ cg = 'content_generator_name'`
		`+ get_build.side_effect = [`
		`+ {`
		`+ 'id': build_id,`
		`+ 'state': koji.BUILD_STATES['BUILDING'],`
		`+ 'cg_id': cg_id,`
		`+ },`
		`+ {`
		`+ 'id': build_id,`
		`+ 'state': koji.BUILD_STATES['FAILED'],`
		`+ 'cg_id': cg_id,`
		`+ },`
		`+ ]`
		`+`
		`+ token = 'random_token'`
		`+ get_reservation_token.return_value = {'build_id': build_id, 'token': token}`
		`+ lookup_name.return_value = {'name': cg, 'id': cg_id}`
		`+`
		`+ kojihub.cg_refund_build(cg, build_id, token)`
		`+`
		`+ assert_cg.assert_called_once_with(cg)`
		`+ get_build.assert_has_calls([`
		`+ mock.call(build_id, strict=True),`
		`+ mock.call(build_id, strict=True),`
		`+ ])`
		`+ get_reservation_token.assert_called_once_with(build_id)`
		`+ lookup_name.assert_called_once_with('content_generator', cg, strict=True)`
		`+`
		`+ self.assertEqual(len(self.updates), 1)`
		`+ update = self.updates[0]`
		`+ self.assertEqual(update.table, 'build')`
		`+ self.assertEqual(update.values['id'], build_id)`
		`+ self.assertEqual(update.data['state'], koji.BUILD_STATES['FAILED'])`
		`+ self.assertEqual(update.rawdata, {'completion_time': 'NOW()'})`
		`+`
		`+ run_callbacks.assert_has_calls([`
		`+ mock.call('preBuildStateChange', attribute='state',`
		`+ old=koji.BUILD_STATES['BUILDING'],`
		`+ new=koji.BUILD_STATES['FAILED'],`
		`+ info={`
		`+ 'state': koji.BUILD_STATES['BUILDING'],`
		`+ 'cg_id': cg_id,`
		`+ 'id': build_id}`
		`+ ),`
		`+ mock.call('postBuildStateChange', attribute='state',`
		`+ old=koji.BUILD_STATES['BUILDING'],`
		`+ new=koji.BUILD_STATES['FAILED'],`
		`+ info={`
		`+ 'state': koji.BUILD_STATES['FAILED'],`
		`+ 'cg_id': cg_id,`
		`+ 'id': build_id}`
		`+ ),`
		`+ ])`

tkopecek commented 4 years ago

Fixes: https://pagure.io/koji/issue/1610

tkopecek commented 4 years ago

Not sure about name of the method. Any better proposal?

rebased onto fe24972c2e68ffcdf9f8210d00036caa094d0e0f

4 years ago

tkopecek commented 4 years ago

ping @mikem, @breilly, @julian8628

breilly commented 4 years ago

Code LGTM.

For the method name, how about 'cg_cancel_build'? Might be more clear than 'uninit', and this would be used in cases where the CG cancels the build on its end right?

tkopecek commented 4 years ago

@breilly This method has sense only if build fails, but reservation stays in place. For cancellation standard cancel method works (and deletes reservation). So, this does a little bit different thing.