From d833400206dc33312feaa0af185f60cc555d40e9 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 1/17] API for reserving NVRs for content generators Fixes: https://pagure.io/koji/issue/1463 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index f947116..fee6470 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5246,11 +5246,10 @@ def recycle_build(old, data): st_desc = koji.BUILD_STATES[old['state']] if st_desc == 'BUILDING': # check to see if this is the controlling task - if data['state'] == old['state'] and data.get('task_id', '') == old['task_id']: + if data['state'] == old['state'] and data.get('task_id', '') == old.get('task_id', ''): #the controlling task must have restarted (and called initBuild again) return - raise koji.GenericError("Build already in progress (task %(task_id)d)" - % old) + raise koji.GenericError("Build already in progress (task %(task_id)d)" % old) # TODO? - reclaim 'stale' builds (state=BUILDING and task_id inactive) if st_desc not in ('FAILED', 'CANCELED'): @@ -5534,6 +5533,10 @@ def import_rpm(fn, buildinfo=None, brootid=None, wrapper=False, fileinfo=None): return rpminfo +def cg_init_build(cg, data): + importer = CG_Importer() + return importer.init_build(cg, data) + def cg_import(metadata, directory): """Import build from a content generator @@ -5553,8 +5556,16 @@ class CG_Importer(object): self.buildinfo = None self.metadata_only = False - def do_import(self, metadata, directory): + def init_build(self, cg, data): + assert_cg(cg) + data['owner'] = context.session.user_id + data['state'] = koji.BUILD_STATES['BUILDING'] + data['completion_time'] = None + data['extra'] = {'reserved_by_cg': True} + build_id = new_build(data) + return build_id + def do_import(self, metadata, directory): metadata = self.get_metadata(metadata, directory) self.directory = directory @@ -5679,24 +5690,37 @@ class CG_Importer(object): def prep_build(self): metadata = self.metadata - buildinfo = get_build(metadata['build'], strict=False) - if buildinfo: - # TODO : allow in some cases - raise koji.GenericError("Build already exists: %r" % buildinfo) + if metadata['build'].get('build_id'): + build_id = metadata['build']['build_id'] + buildinfo = get_build(build_id, strict=True) + if not buildinfo['extra'] or not buildinfo['extra'].get('reserved_by_cg') or \ + buildinfo['owner_id'] != context.session.user_id: + raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id) + if buildinfo['name'] != metadata['build']['name'] or \ + buildinfo['version'] != metadata['build']['version'] or \ + buildinfo['release'] != metadata['build']['release'] or \ + buildinfo['epoch'] != metadata['build']['epoch']: + raise koji.GenericError("Build (%i) NVR is different" % build_id) else: - # gather needed data - buildinfo = dslice(metadata['build'], ['name', 'version', 'release', 'extra', 'source']) - # epoch is not in the metadata spec, but we allow it to be specified - buildinfo['epoch'] = metadata['build'].get('epoch', None) - buildinfo['start_time'] = \ - datetime.datetime.fromtimestamp(float(metadata['build']['start_time'])).isoformat(' ') - buildinfo['completion_time'] = \ - datetime.datetime.fromtimestamp(float(metadata['build']['end_time'])).isoformat(' ') - owner = metadata['build'].get('owner', None) - if owner: - if not isinstance(owner, six.string_types): - raise koji.GenericError("Invalid owner format (expected username): %s" % owner) - buildinfo['owner'] = get_user(owner, strict=True)['id'] + buildinfo = get_build(metadata['build'], strict=False) + if buildinfo and not metadata['build'].get('build_id'): + # TODO : allow in some cases + raise koji.GenericError("Build already exists: %r" % buildinfo) + # gather needed data + buildinfo = dslice(metadata['build'], ['name', 'version', 'release', 'extra', 'source']) + if 'build_id' in metadata['build']: + buildinfo['build_id'] = metadata['build']['build_id'] + # epoch is not in the metadata spec, but we allow it to be specified + buildinfo['epoch'] = metadata['build'].get('epoch', None) + buildinfo['start_time'] = \ + datetime.datetime.fromtimestamp(float(metadata['build']['start_time'])).isoformat(' ') + buildinfo['completion_time'] = \ + datetime.datetime.fromtimestamp(float(metadata['build']['end_time'])).isoformat(' ') + owner = metadata['build'].get('owner', None) + if owner: + if not isinstance(owner, six.string_types): + raise koji.GenericError("Invalid owner format (expected username): %s" % owner) + buildinfo['owner'] = get_user(owner, strict=True)['id'] self.buildinfo = buildinfo koji.check_NVR(buildinfo, strict=True) @@ -5723,9 +5747,22 @@ class CG_Importer(object): def get_build(self): - build_id = new_build(self.buildinfo) - buildinfo = get_build(build_id, strict=True) - + try: + binfo = dslice(self.buildinfo, ('name', 'version', 'release')) + buildinfo = get_build(binfo, strict=True) + if buildinfo.get('task_id') or \ + buildinfo['state'] != koji.BUILD_STATES['BUILDING'] or \ + buildinfo['owner_id'] != context.session.user_id or \ + not buildinfo['extra'] or not buildinfo['extra'].get('reserved_by_cg'): + raise koji.GenericError("Build is not reserved") + del buildinfo['extra']['reserved_by_cg'] + build_id = buildinfo['build_id'] + except Exception as ex: + build_id = new_build(self.buildinfo) + buildinfo = get_build(build_id, strict=True) + #if not self.buildinfo.get('build_id'): + #else: + # buildinfo = get_build(self.buildinfo['build_id'], strict=True) # handle special build types for btype in self.typeinfo: tinfo = self.typeinfo[btype] @@ -5745,6 +5782,26 @@ class CG_Importer(object): if [o for o in self.prepped_outputs if o['type'] == 'rpm']: new_typed_build(buildinfo, 'rpm') + # update build state, delete 'reserved_by_cg' placeholder + print(buildinfo) + print(self.buildinfo) + if buildinfo.get('extra'): + extra = json.dumps(buildinfo['extra']) + else: + extra = None + owner = get_user(self.buildinfo['owner'], strict=True)['id'] + source = self.buildinfo.get('source') + st_complete = koji.BUILD_STATES['COMPLETE'] + st_old = buildinfo['state'] + koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=st_old, new=st_complete, info=buildinfo) + update = UpdateProcessor('build', clauses=['id=%(id)s'], values=buildinfo) + update.set(state=st_complete, extra=extra, owner=owner, source=source) + update.rawset(completion_time='NOW()') + print(update) + update.execute() + buildinfo = get_build(build_id, strict=True) + koji.plugin.run_callbacks('postBuildStateChange', attribute='state', old=st_old, new=st_complete, info=buildinfo) + self.buildinfo = buildinfo return buildinfo @@ -9481,6 +9538,7 @@ class RootExports(object): fullpath = '%s/%s' % (koji.pathinfo.work(), filepath) import_archive(fullpath, buildinfo, type, typeInfo) + CGInitBuild = staticmethod(cg_init_build) CGImport = staticmethod(cg_import) untaggedBuilds = staticmethod(untagged_builds) From e90be908dff6cd3c2f4957d2b7d35e53537360a8 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 2/17] raise an error on repeated call cgInitBuild for same nvr --- diff --git a/hub/kojihub.py b/hub/kojihub.py index fee6470..6294743 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5182,8 +5182,11 @@ def apply_volume_policy(build, strict=False): _set_build_volume(build, volume, strict=True) -def new_build(data): - """insert a new build entry""" +def new_build(data, strict=False): + """insert a new build entry + + If strict is specified, raise an exception, if build already exists. + """ data = data.copy() @@ -5220,7 +5223,7 @@ def new_build(data): data.setdefault('volume_id', 0) #check for existing build - old_binfo = get_build(data) + old_binfo = get_build(data, strict=strict) if old_binfo: recycle_build(old_binfo, data) # Raises exception if there is a problem @@ -5557,12 +5560,16 @@ class CG_Importer(object): self.metadata_only = False def init_build(self, cg, data): + """Create (reserve) a build_id for given data. + + If build already exists, init_build will raise GenericError + """ assert_cg(cg) data['owner'] = context.session.user_id data['state'] = koji.BUILD_STATES['BUILDING'] data['completion_time'] = None data['extra'] = {'reserved_by_cg': True} - build_id = new_build(data) + build_id = new_build(data, strict=True) return build_id def do_import(self, metadata, directory): From f9e9ac3ae25ea8f7bc26f09a83eef0fb8a51a9e5 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 3/17] reverse wrong strict logic --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 6294743..3507f67 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5223,12 +5223,13 @@ def new_build(data, strict=False): data.setdefault('volume_id', 0) #check for existing build - old_binfo = get_build(data, strict=strict) + old_binfo = get_build(data) if old_binfo: + if strict: + raise koji.GenericError('No matching build found: %s' % data) recycle_build(old_binfo, data) # Raises exception if there is a problem return old_binfo['id'] - #else koji.plugin.run_callbacks('preBuildStateChange', attribute='state', old=None, new=data['state'], info=data) #insert the new data @@ -5764,7 +5765,7 @@ class CG_Importer(object): raise koji.GenericError("Build is not reserved") del buildinfo['extra']['reserved_by_cg'] build_id = buildinfo['build_id'] - except Exception as ex: + except Exception: build_id = new_build(self.buildinfo) buildinfo = get_build(build_id, strict=True) #if not self.buildinfo.get('build_id'): From fcf8402e107bda72ae5fdae667762593e1a44696 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 4/17] move init_build out of CG_Importer --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 3507f67..20ca4ca 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5538,8 +5538,17 @@ def import_rpm(fn, buildinfo=None, brootid=None, wrapper=False, fileinfo=None): def cg_init_build(cg, data): - importer = CG_Importer() - return importer.init_build(cg, data) + """Create (reserve) a build_id for given data. + + If build already exists, init_build will raise GenericError + """ + assert_cg(cg) + data['owner'] = context.session.user_id + data['state'] = koji.BUILD_STATES['BUILDING'] + data['completion_time'] = None + data['extra'] = {'reserved_by_cg': True} + build_id = new_build(data, strict=True) + return build_id def cg_import(metadata, directory): """Import build from a content generator @@ -5560,19 +5569,6 @@ class CG_Importer(object): self.buildinfo = None self.metadata_only = False - def init_build(self, cg, data): - """Create (reserve) a build_id for given data. - - If build already exists, init_build will raise GenericError - """ - assert_cg(cg) - data['owner'] = context.session.user_id - data['state'] = koji.BUILD_STATES['BUILDING'] - data['completion_time'] = None - data['extra'] = {'reserved_by_cg': True} - build_id = new_build(data, strict=True) - return build_id - def do_import(self, metadata, directory): metadata = self.get_metadata(metadata, directory) self.directory = directory From 9a686ae9279c3916ed83d7252c8d3fe7b93fdc4f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 5/17] leave recycle_build untouched --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 20ca4ca..8d0a53b 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5226,7 +5226,7 @@ def new_build(data, strict=False): old_binfo = get_build(data) if old_binfo: if strict: - raise koji.GenericError('No matching build found: %s' % data) + raise koji.GenericError('Existing build found: %s' % data) recycle_build(old_binfo, data) # Raises exception if there is a problem return old_binfo['id'] @@ -5250,10 +5250,11 @@ def recycle_build(old, data): st_desc = koji.BUILD_STATES[old['state']] if st_desc == 'BUILDING': # check to see if this is the controlling task - if data['state'] == old['state'] and data.get('task_id', '') == old.get('task_id', ''): + if data['state'] == old['state'] and data.get('task_id', '') == old['task_id']: #the controlling task must have restarted (and called initBuild again) return - raise koji.GenericError("Build already in progress (task %(task_id)d)" % old) + raise koji.GenericError("Build already in progress (task %(task_id)d)" + % old) # TODO? - reclaim 'stale' builds (state=BUILDING and task_id inactive) if st_desc not in ('FAILED', 'CANCELED'): @@ -5698,7 +5699,8 @@ class CG_Importer(object): build_id = metadata['build']['build_id'] buildinfo = get_build(build_id, strict=True) if not buildinfo['extra'] or not buildinfo['extra'].get('reserved_by_cg') or \ - buildinfo['owner_id'] != context.session.user_id: + buildinfo['owner_id'] != context.session.user_id or \ + buildinfo['state'] != koji.BUILD_STATES['BUILDING']: raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id) if buildinfo['name'] != metadata['build']['name'] or \ buildinfo['version'] != metadata['build']['version'] or \ @@ -5759,14 +5761,11 @@ class CG_Importer(object): buildinfo['owner_id'] != context.session.user_id or \ not buildinfo['extra'] or not buildinfo['extra'].get('reserved_by_cg'): raise koji.GenericError("Build is not reserved") - del buildinfo['extra']['reserved_by_cg'] + buildinfo['extra'] = self.buildinfo['extra'] build_id = buildinfo['build_id'] except Exception: build_id = new_build(self.buildinfo) buildinfo = get_build(build_id, strict=True) - #if not self.buildinfo.get('build_id'): - #else: - # buildinfo = get_build(self.buildinfo['build_id'], strict=True) # handle special build types for btype in self.typeinfo: tinfo = self.typeinfo[btype] @@ -5787,8 +5786,6 @@ class CG_Importer(object): new_typed_build(buildinfo, 'rpm') # update build state, delete 'reserved_by_cg' placeholder - print(buildinfo) - print(self.buildinfo) if buildinfo.get('extra'): extra = json.dumps(buildinfo['extra']) else: From 4622bff464ec7eca37f2ca949ab04b68d917cc3a Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 6/17] remove debug print --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 8d0a53b..7415c71 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5798,7 +5798,6 @@ class CG_Importer(object): update = UpdateProcessor('build', clauses=['id=%(id)s'], values=buildinfo) update.set(state=st_complete, extra=extra, owner=owner, source=source) update.rawset(completion_time='NOW()') - print(update) update.execute() buildinfo = get_build(build_id, strict=True) koji.plugin.run_callbacks('postBuildStateChange', attribute='state', old=st_old, new=st_complete, info=buildinfo) From 6038c39e79e18231905d46e75e44f86529a60899 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 7/17] use token for reservation --- diff --git a/docs/schema.sql b/docs/schema.sql index 647e647..f0155d2 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -299,6 +299,13 @@ CREATE TABLE build_types ( ) WITHOUT OIDS; +CREATE TABLE build_reservations ( + build_id INTEGER NOT NULL REFERENCES build(id), + user_id INTEGER NOT NULL REFERENCES users(id), + token VARCHAR(64), + PRIMARY KEY (build_id) +) WITHOUT OIDS; + -- Note: some of these CREATEs may seem a little out of order. This is done to keep -- the references sane. diff --git a/hub/kojihub.py b/hub/kojihub.py index 7415c71..949741e 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -44,6 +44,12 @@ import time import traceback import six.moves.xmlrpc_client import zipfile +try: + # py 3.6+ + import secrets +except ImportError: + import binascii + import random import rpm import six @@ -5538,6 +5544,17 @@ def import_rpm(fn, buildinfo=None, brootid=None, wrapper=False, fileinfo=None): return rpminfo +def generate_token(nbytes=32): + """ + Generate random hex-string token of length 2 * nbytes + """ + if secrets: + return secrets.token_hex(nbytes=nbytes) + else: + values = ['%02x' % random.randint(0, 256) for x in range(nbytes)] + return ''.join(values) + + def cg_init_build(cg, data): """Create (reserve) a build_id for given data. @@ -5547,9 +5564,17 @@ def cg_init_build(cg, data): data['owner'] = context.session.user_id data['state'] = koji.BUILD_STATES['BUILDING'] data['completion_time'] = None - data['extra'] = {'reserved_by_cg': True} build_id = new_build(data, strict=True) - return build_id + # store token + token = generate_token() + insert = InsertProcessor(table='build_reservations') + insert.set(build_id=build_id) + insert.set(user_id = context.session.user_id) + insert.set(token = token) + insert.execute() + + return {'build_id': build_id, 'token': token} + def cg_import(metadata, directory): """Import build from a content generator @@ -5693,13 +5718,24 @@ class CG_Importer(object): raise koji.GenericError("Destination directory already exists: %s" % path) + def get_reserve_token(self, build_id): + query = QueryProcessor( + tables=['build_reservations'], + columns=['build_id', 'user_id', 'token'], + clauses=['build_id = %(build_id)d'], + values=locals(), + ) + return query.executeOne() + + def prep_build(self): metadata = self.metadata if metadata['build'].get('build_id'): build_id = metadata['build']['build_id'] buildinfo = get_build(build_id, strict=True) - if not buildinfo['extra'] or not buildinfo['extra'].get('reserved_by_cg') or \ - buildinfo['owner_id'] != context.session.user_id or \ + token = self.get_reserve_token(build_id) + if not token or token['token'] != metadata['build']['token'] or \ + token['user_id'] != context.session.user_id or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING']: raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id) if buildinfo['name'] != metadata['build']['name'] or \ @@ -5756,10 +5792,12 @@ class CG_Importer(object): try: binfo = dslice(self.buildinfo, ('name', 'version', 'release')) buildinfo = get_build(binfo, strict=True) + token = self.get_reserve_token(buildinfo['build_id']) if buildinfo.get('task_id') or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING'] or \ - buildinfo['owner_id'] != context.session.user_id or \ - not buildinfo['extra'] or not buildinfo['extra'].get('reserved_by_cg'): + not token or \ + token['user_id'] != context.session.user_id or \ + token['token'] != self.metadata['build']['token']: raise koji.GenericError("Build is not reserved") buildinfo['extra'] = self.buildinfo['extra'] build_id = buildinfo['build_id'] @@ -5785,7 +5823,7 @@ class CG_Importer(object): if [o for o in self.prepped_outputs if o['type'] == 'rpm']: new_typed_build(buildinfo, 'rpm') - # update build state, delete 'reserved_by_cg' placeholder + # update build state if buildinfo.get('extra'): extra = json.dumps(buildinfo['extra']) else: From e82c5eeda4c5560fe9a5f9128e662fb4a920164a Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 8/17] migration for build_reservations --- diff --git a/docs/schema-upgrade-1.17-1.18.sql b/docs/schema-upgrade-1.17-1.18.sql index be531dc..4faddde 100644 --- a/docs/schema-upgrade-1.17-1.18.sql +++ b/docs/schema-upgrade-1.17-1.18.sql @@ -15,4 +15,13 @@ insert into archivetypes (name, description, extensions) values ('qcow2-compress -- add better index for sessions CREATE INDEX sessions_expired ON sessions(expired); +-- table for content generator build reservations +CREATE TABLE build_reservations ( + build_id INTEGER NOT NULL REFERENCES build(id), + user_id INTEGER NOT NULL REFERENCES users(id), + token VARCHAR(64), + PRIMARY KEY (build_id) +) WITHOUT OIDS; + + COMMIT; From bfa7a429c9c35603fb2c87537ef34b47ad5ea7f5 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 9/17] move token from metadata to api option --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 7a67f80..034523f 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -1290,6 +1290,7 @@ def handle_import_cg(goptions, session, args): help=_("Do not display progress of the upload")) parser.add_option("--link", action="store_true", help=_("Attempt to hardlink instead of uploading")) parser.add_option("--test", action="store_true", help=_("Don't actually import")) + parser.add_option("--token", action="store", default=None, help=_("Build reservarion token")) (options, args) = parser.parse_args(args) if len(args) < 2: parser.error(_("Please specify metadata files directory")) @@ -1334,7 +1335,7 @@ def handle_import_cg(goptions, session, args): if callback: print('') - session.CGImport(metadata, serverdir) + session.CGImport(metadata, serverdir, options.token) def handle_import_comps(goptions, session, args): diff --git a/hub/kojihub.py b/hub/kojihub.py index 949741e..8aa0ac6 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -48,7 +48,6 @@ try: # py 3.6+ import secrets except ImportError: - import binascii import random import rpm @@ -5569,14 +5568,14 @@ def cg_init_build(cg, data): token = generate_token() insert = InsertProcessor(table='build_reservations') insert.set(build_id=build_id) - insert.set(user_id = context.session.user_id) - insert.set(token = token) + insert.set(user_id=context.session.user_id) + insert.set(token=token) insert.execute() return {'build_id': build_id, 'token': token} -def cg_import(metadata, directory): +def cg_import(metadata, directory, token=None): """Import build from a content generator metadata can be one of the following @@ -5586,7 +5585,7 @@ def cg_import(metadata, directory): """ importer = CG_Importer() - return importer.do_import(metadata, directory) + return importer.do_import(metadata, directory, token) class CG_Importer(object): @@ -5595,7 +5594,7 @@ class CG_Importer(object): self.buildinfo = None self.metadata_only = False - def do_import(self, metadata, directory): + def do_import(self, metadata, directory, token=None): metadata = self.get_metadata(metadata, directory) self.directory = directory @@ -5608,7 +5607,7 @@ class CG_Importer(object): self.assert_cg_access() # prepare data for import - self.prep_build() + self.prep_build(token) self.prep_brs() self.prep_outputs() @@ -5620,7 +5619,7 @@ class CG_Importer(object): directory=directory) # finalize import - self.get_build() + self.get_build(token) self.import_brs() try: self.import_outputs() @@ -5728,15 +5727,17 @@ class CG_Importer(object): return query.executeOne() - def prep_build(self): + def prep_build(self, token=None): metadata = self.metadata if metadata['build'].get('build_id'): build_id = metadata['build']['build_id'] buildinfo = get_build(build_id, strict=True) - token = self.get_reserve_token(build_id) - if not token or token['token'] != metadata['build']['token'] or \ - token['user_id'] != context.session.user_id or \ + build_token = self.get_reserve_token(build_id) + if not build_token or build_token['token'] != token or \ + build_token['user_id'] != context.session.user_id or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING']: + print(build_token) + print(token) raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id) if buildinfo['name'] != metadata['build']['name'] or \ buildinfo['version'] != metadata['build']['version'] or \ @@ -5788,16 +5789,16 @@ class CG_Importer(object): return buildinfo - def get_build(self): + def get_build(self, token=None): try: binfo = dslice(self.buildinfo, ('name', 'version', 'release')) buildinfo = get_build(binfo, strict=True) - token = self.get_reserve_token(buildinfo['build_id']) + build_token = self.get_reserve_token(buildinfo['build_id']) if buildinfo.get('task_id') or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING'] or \ - not token or \ - token['user_id'] != context.session.user_id or \ - token['token'] != self.metadata['build']['token']: + not build_token or \ + build_token['user_id'] != context.session.user_id or \ + build_token['token'] != token: raise koji.GenericError("Build is not reserved") buildinfo['extra'] = self.buildinfo['extra'] build_id = buildinfo['build_id'] From b48d498d3fcb2cf0c82891b21e1521486f0b1690 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 10/17] remove debug --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 8aa0ac6..330d3fb 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5736,8 +5736,6 @@ class CG_Importer(object): if not build_token or build_token['token'] != token or \ build_token['user_id'] != context.session.user_id or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING']: - print(build_token) - print(token) raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id) if buildinfo['name'] != metadata['build']['name'] or \ buildinfo['version'] != metadata['build']['version'] or \ From c9182e9cbde3d14700bd9b93a22bb8458bd36e34 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 11/17] restrict to cg_id --- diff --git a/docs/schema-upgrade-1.17-1.18.sql b/docs/schema-upgrade-1.17-1.18.sql index 4faddde..13ab448 100644 --- a/docs/schema-upgrade-1.17-1.18.sql +++ b/docs/schema-upgrade-1.17-1.18.sql @@ -18,10 +18,11 @@ CREATE INDEX sessions_expired ON sessions(expired); -- table for content generator build reservations CREATE TABLE build_reservations ( build_id INTEGER NOT NULL REFERENCES build(id), - user_id INTEGER NOT NULL REFERENCES users(id), + cg_id INTEGER NOT NULL REFERENCES content_generator(id), token VARCHAR(64), + created TIMESTAMP NOT NULL, PRIMARY KEY (build_id) ) WITHOUT OIDS; - +CREATE INDEX build_reservations_created ON build_reservations(created); COMMIT; diff --git a/docs/schema.sql b/docs/schema.sql index f0155d2..b5ebaea 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -299,13 +299,6 @@ CREATE TABLE build_types ( ) WITHOUT OIDS; -CREATE TABLE build_reservations ( - build_id INTEGER NOT NULL REFERENCES build(id), - user_id INTEGER NOT NULL REFERENCES users(id), - token VARCHAR(64), - PRIMARY KEY (build_id) -) WITHOUT OIDS; - -- Note: some of these CREATEs may seem a little out of order. This is done to keep -- the references sane. @@ -514,6 +507,14 @@ CREATE TABLE cg_users ( UNIQUE (cg_id, user_id, active) ) WITHOUT OIDS; +CREATE TABLE build_reservations ( + build_id INTEGER NOT NULL REFERENCES build(id), + cg_id INTEGER NOT NULL REFERENCES content_generator(id), + token VARCHAR(64), + created TIMESTAMP NOT NULL, + PRIMARY KEY (build_id) +) WITHOUT OIDS; +CREATE INDEX build_reservations_created ON build_reservations(created); -- here we track the buildroots on the machines CREATE TABLE buildroot ( diff --git a/hub/kojihub.py b/hub/kojihub.py index 330d3fb..e4c1606 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5566,10 +5566,12 @@ def cg_init_build(cg, data): build_id = new_build(data, strict=True) # store token token = generate_token() + cg_id = lookup_name('content_generator', cg, strict=True)['id'] insert = InsertProcessor(table='build_reservations') - insert.set(build_id=build_id) - insert.set(user_id=context.session.user_id) - insert.set(token=token) + insert.set(build_id=build_id, + cg_id=cg_id, + token=token) + insert.rawset(created='NOW()') insert.execute() return {'build_id': build_id, 'token': token} @@ -5720,7 +5722,7 @@ class CG_Importer(object): def get_reserve_token(self, build_id): query = QueryProcessor( tables=['build_reservations'], - columns=['build_id', 'user_id', 'token'], + columns=['build_id', 'cg_id', 'token'], clauses=['build_id = %(build_id)d'], values=locals(), ) @@ -5730,11 +5732,14 @@ class CG_Importer(object): def prep_build(self, token=None): metadata = self.metadata if metadata['build'].get('build_id'): + if len(self.cgs) != 1: + raise koji.GenericError("Reserved builds can handle only single content generator.") + cg_id = list(self.cgs)[0] build_id = metadata['build']['build_id'] buildinfo = get_build(build_id, strict=True) build_token = self.get_reserve_token(build_id) if not build_token or build_token['token'] != token or \ - build_token['user_id'] != context.session.user_id or \ + build_token['cg_id'] != cg_id or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING']: raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id) if buildinfo['name'] != metadata['build']['name'] or \ @@ -5792,10 +5797,13 @@ class CG_Importer(object): binfo = dslice(self.buildinfo, ('name', 'version', 'release')) buildinfo = get_build(binfo, strict=True) build_token = self.get_reserve_token(buildinfo['build_id']) + if len(self.cgs) != 1: + raise koji.GenericError("Reserved builds can handle only single content generator.") + cg_id = list(self.cgs)[0] if buildinfo.get('task_id') or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING'] or \ not build_token or \ - build_token['user_id'] != context.session.user_id or \ + build_token['cg_id'] != cg_id or \ build_token['token'] != token: raise koji.GenericError("Build is not reserved") buildinfo['extra'] = self.buildinfo['extra'] From c300867aa44a4587dd521097011bc8cb3c096ec2 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 12/17] delete tokens on cancelBuild --- diff --git a/hub/kojihub.py b/hub/kojihub.py index e4c1606..9a5537b 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5554,6 +5554,16 @@ def generate_token(nbytes=32): return ''.join(values) +def get_reservation_token(build_id): + query = QueryProcessor( + tables=['build_reservations'], + columns=['build_id', 'cg_id', 'token'], + clauses=['build_id = %(build_id)d'], + values=locals(), + ) + return query.executeOne() + + def cg_init_build(cg, data): """Create (reserve) a build_id for given data. @@ -5719,15 +5729,6 @@ class CG_Importer(object): raise koji.GenericError("Destination directory already exists: %s" % path) - def get_reserve_token(self, build_id): - query = QueryProcessor( - tables=['build_reservations'], - columns=['build_id', 'cg_id', 'token'], - clauses=['build_id = %(build_id)d'], - values=locals(), - ) - return query.executeOne() - def prep_build(self, token=None): metadata = self.metadata @@ -5737,7 +5738,7 @@ class CG_Importer(object): cg_id = list(self.cgs)[0] build_id = metadata['build']['build_id'] buildinfo = get_build(build_id, strict=True) - build_token = self.get_reserve_token(build_id) + build_token = get_reservation_token(build_id) if not build_token or build_token['token'] != token or \ build_token['cg_id'] != cg_id or \ buildinfo['state'] != koji.BUILD_STATES['BUILDING']: @@ -5796,7 +5797,7 @@ class CG_Importer(object): try: binfo = dslice(self.buildinfo, ('name', 'version', 'release')) buildinfo = get_build(binfo, strict=True) - build_token = self.get_reserve_token(buildinfo['build_id']) + build_token = get_reservation_token(buildinfo['build_id']) if len(self.cgs) != 1: raise koji.GenericError("Reserved builds can handle only single content generator.") cg_id = list(self.cgs)[0] @@ -7606,6 +7607,11 @@ def cancel_build(build_id, cancel_task=True): build_notification(task_id, build_id) if cancel_task: Task(task_id).cancelFull(strict=False) + + # remove possible CG reservations + delete = "DELETE FROM build_reservations WHERE build_id = %(build_id)i" + _dml(delete, {'build_id': build_id}) + build = get_build(build_id, strict=True) koji.plugin.run_callbacks('postBuildStateChange', attribute='state', old=st_old, new=st_canceled, info=build) return True From 29b669a96e9c09daa9f5770dc27df87a3de891df Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 13/17] extended getBuild to return reservations --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 9a5537b..f9968ec 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -3708,6 +3708,8 @@ def get_build(buildInfo, strict=False): completion_ts: time the build was completed (epoch, may be null) source: the SCM URL of the sources used in the build extra: dictionary with extra data about the build + reserved_id: ID of CG which reserved this build (only in BUILDING state) + reserved_name: name of CG which reserved this build (only in BUILDING state) If there is no build matching the buildInfo given, and strict is specified, raise an error. Otherwise return None. @@ -3751,6 +3753,13 @@ def get_build(buildInfo, strict=False): else: return None else: + result['reserved_by'] = None + if result['state'] == koji.BUILD_STATES['BUILDING']: + token = get_reservation_token(result['id']) + if token: + cg = lookup_name('content_generator', token['cg_id'], strict=True) + result['reserved_by_id'] = cg['id'] + result['reserved_by_name'] = cg['name'] return result diff --git a/www/kojiweb/buildinfo.chtml b/www/kojiweb/buildinfo.chtml index cd9844d..1868b05 100644 --- a/www/kojiweb/buildinfo.chtml +++ b/www/kojiweb/buildinfo.chtml @@ -82,6 +82,11 @@ Completed$util.formatTimeLong($build.completion_time) #end if + #if $build.reserved_by_name + + Reserved by$build.reserved_by_name + + #end if #if $task Task$koji.taskLabel($task) From 6a13a93130cfe781f49e2ed1262bc9ae627df5d8 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 14/17] fix tests for CLI --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index 034523f..d6588d9 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -1290,7 +1290,7 @@ def handle_import_cg(goptions, session, args): help=_("Do not display progress of the upload")) parser.add_option("--link", action="store_true", help=_("Attempt to hardlink instead of uploading")) parser.add_option("--test", action="store_true", help=_("Don't actually import")) - parser.add_option("--token", action="store", default=None, help=_("Build reservarion token")) + parser.add_option("--token", action="store", default=None, help=_("Build reservation token")) (options, args) = parser.parse_args(args) if len(args) < 2: parser.error(_("Please specify metadata files directory")) diff --git a/tests/test_cli/test_import_cg.py b/tests/test_cli/test_import_cg.py index 3af9248..72eae49 100644 --- a/tests/test_cli/test_import_cg.py +++ b/tests/test_cli/test_import_cg.py @@ -95,7 +95,7 @@ class TestImportCG(utils.CliTestCase): self.assert_console_message(stdout, expected) linked_upload_mock.assert_not_called() session.uploadWrapper.assert_has_calls(calls) - session.CGImport.assert_called_with(metadata, fake_srv_path) + session.CGImport.assert_called_with(metadata, fake_srv_path, None) # Case 2, running in fg, progress off with mock.patch(utils.get_builtin_open()): @@ -105,7 +105,7 @@ class TestImportCG(utils.CliTestCase): self.assert_console_message(stdout, expected) linked_upload_mock.assert_not_called() session.uploadWrapper.assert_has_calls(calls) - session.CGImport.assert_called_with(metadata, fake_srv_path) + session.CGImport.assert_called_with(metadata, fake_srv_path, None) # reset mocks linked_upload_mock.reset_mock() @@ -129,7 +129,7 @@ class TestImportCG(utils.CliTestCase): linked_upload_mock.assert_has_calls(calls) session.uploadWrapper.assert_not_called() - session.CGImport.assert_called_with(metadata, fake_srv_path) + session.CGImport.assert_called_with(metadata, fake_srv_path, None) # make sure there is no message on output self.assert_console_message(stdout, '') @@ -213,10 +213,11 @@ class TestImportCG(utils.CliTestCase): (Specify the --help global option for a list of other help options) Options: - -h, --help show this help message and exit - --noprogress Do not display progress of the upload - --link Attempt to hardlink instead of uploading - --test Don't actually import + -h, --help show this help message and exit + --noprogress Do not display progress of the upload + --link Attempt to hardlink instead of uploading + --test Don't actually import + --token=TOKEN Build reservation token """ % self.progname) From 99425548fe5fd960b19bf272a0562936cd67013b Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 15/17] CLI shows reservation --- diff --git a/cli/koji_cli/commands.py b/cli/koji_cli/commands.py index d6588d9..fe57aa3 100644 --- a/cli/koji_cli/commands.py +++ b/cli/koji_cli/commands.py @@ -3153,6 +3153,8 @@ def anon_handle_buildinfo(goptions, session, args): info['state'] = koji.BUILD_STATES[info['state']] print("BUILD: %(name)s-%(version)s-%(release)s [%(id)d]" % info) print("State: %(state)s" % info) + if info['state'] == 'BUILDING': + print("Reserved by: %(reserved_by_name)s" % info) print("Built by: %(owner_name)s" % info) source = info.get('source') if source is not None: From 6159b0e0067743ee27871c89c29c140b3320e9a0 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 16/17] docs for CG reservation API --- diff --git a/docs/source/content_generator_metadata.rst b/docs/source/content_generator_metadata.rst index 47c8165..ee7708f 100644 --- a/docs/source/content_generator_metadata.rst +++ b/docs/source/content_generator_metadata.rst @@ -44,6 +44,7 @@ The build map contains the following entries: epoch. - owner: The owner of the build task in username format. This field is optional. +- build_id: Reserved build ID. This field is optional. - extra: A map of extra metadata associated with the build, which must include one of: diff --git a/docs/source/content_generators.rst b/docs/source/content_generators.rst index 738557a..fcbe31d 100644 --- a/docs/source/content_generators.rst +++ b/docs/source/content_generators.rst @@ -114,3 +114,30 @@ Metadata Metadata will be provided by the Content Generator as a JSON file. There is a proposal of the :doc:`Content Generator Metadata ` format available for review. + +API +=== + +Relevant API calls for Content Generator are: + +- ``CGImport(metadata, directory, token=None)``: This is basic integration point + of Content Generator with koji. It is supplied with metadata as json encoded + string or dict or filename of metadata described in previous chapter and + directory with all uploaded content referenced from metadata. These files + needs to be uploaded before ``CGImport`` is called. + + Optionally, ``token`` can be specified in case, that build ID reservation was + done before. + +- ``CGInitBuild(cg, data)``: It can be helpful in many cases to reserve NVR for + future build before Content Generator evend starts building. Especially, if + there is some CI or other workflow competing for same NVRs. This call creates + special ``token`` which can be used to claim specific build (ID + NVR). Such + claimed build will be displayed as BUILDING and can be used by ``CGImport`` + call later. + + As an input are here Content Generator name and `data` which is basically + dictionary with name/version/release/epoch keys. Call will return a dict + containing ``token`` and ``build_id``. ``token`` would be used in subsequent + call of ``CGImport`` while ``build_id`` needs to be part of metadata (as item + in ``build`` key). From f1e6c6c519e3ed11292a6838e53e2e0b4ec5fb55 Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jul 11 2019 13:49:27 +0000 Subject: [PATCH 17/17] better error messages --- diff --git a/hub/kojihub.py b/hub/kojihub.py index f9968ec..c0f6063 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5748,10 +5748,12 @@ class CG_Importer(object): build_id = metadata['build']['build_id'] buildinfo = get_build(build_id, strict=True) build_token = get_reservation_token(build_id) - if not build_token or build_token['token'] != token or \ - build_token['cg_id'] != cg_id or \ - buildinfo['state'] != koji.BUILD_STATES['BUILDING']: + if not build_token or build_token['token'] != token: + raise koji.GenericError("Token doesn't match build ID %s" % build_id) + if build_token['cg_id'] != cg_id: raise koji.GenericError('Build ID %s is not reserved by this CG' % build_id) + if buildinfo['state'] != koji.BUILD_STATES['BUILDING']: + raise koji.GenericError('Build ID %s is not in BUILDING state' % build_id) if buildinfo['name'] != metadata['build']['name'] or \ buildinfo['version'] != metadata['build']['version'] or \ buildinfo['release'] != metadata['build']['release'] or \