PR#3538: vm: handle waitrepo tasks in kojivmd - koji

koji

#3538 vm: handle waitrepo tasks in kojivmd

Merged 2 years ago by tkopecek. Opened 2 years ago by tkopecek.

tkopecek/koji issue3536 into master

fix type check

Tomas Kopecek • 2 years ago

6f28cb1

vm: handle waitrepo tasks in kojivmd

Tomas Kopecek • 2 years ago

b2eb861

builder/kojid

file modified

-84

		`@@ -6302,90 +6302,6 @@`
		`koji.dump_json(filename, self.kojipkgs, sort_keys=False)`


		`- class WaitrepoTask(BaseTaskHandler):`
		`-`
		`- Methods = ['waitrepo']`
		`- # mostly just waiting`
		`- _taskWeight = 0.2`
		`-`
		`- PAUSE = 60`
		`- # time in minutes before we fail this task`
		`- TIMEOUT = 120`
		`-`
		`- def handler(self, tag, newer_than=None, nvrs=None):`
		`- """Wait for a repo for the tag, subject to given conditions`
		`-`
		`- newer_than: create_event timestamp should be newer than this`
		`- nvr: repo should contain this nvr (which may not exist at first)`
		`-`
		`- Only one of the options may be specified. If neither is, then`
		`- the call will wait for the first ready repo.`
		`-`
		`- Returns the repo info (from getRepo) of the chosen repo`
		`- """`
		`-`
		`- start = time.time()`
		`-`
		`- taginfo = self.session.getTag(tag, strict=True)`
		`- targets = self.session.getBuildTargets(buildTagID=taginfo['id'])`
		`- if not targets:`
		`- raise koji.GenericError("No build target for tag: %s" % taginfo['name'])`
		`-`
		`- if isinstance(newer_than, six.string_types) and newer_than.lower() == "now":`
		`- newer_than = start`
		`- if not isinstance(newer_than, tuple(to_list(six.integer_types) + [type(None), float])):`
		`- raise koji.GenericError("Invalid value for newer_than: %s" % newer_than)`
		`-`
		`- if newer_than and nvrs:`
		`- raise koji.GenericError("only one of (newer_than, nvrs) may be specified")`
		`-`
		`- if not nvrs:`
		`- nvrs = []`
		`- builds = [koji.parse_NVR(nvr) for nvr in nvrs]`
		`-`
		`- last_repo = None`
		`-`
		`- while True:`
		`- try:`
		`- taginfo = self.session.getTag(tag, strict=True)`
		`- except koji.GenericError:`
		`- self.logger.debug("Tag %s got lost while waiting for newrepo", tag)`
		`- raise koji.GenericError("Unsuccessfully waited %s for %s repo. "`
		`- "Tag was probably deleted meanwhile." %`
		`- (koji.util.duration(start), tag))`
		`- repo = self.session.getRepo(taginfo['id'])`
		`- if repo and repo != last_repo:`
		`- if builds:`
		`- if koji.util.checkForBuilds(`
		`- self.session, taginfo['id'], builds, repo['create_event']):`
		`- self.logger.debug("Successfully waited %s for %s to appear "`
		`- "in the %s repo" %`
		`- (koji.util.duration(start), koji.util.printList(nvrs),`
		`- taginfo['name']))`
		`- return repo`
		`- elif newer_than:`
		`- if repo['create_ts'] > newer_than:`
		`- self.logger.debug("Successfully waited %s for a new %s repo" %`
		`- (koji.util.duration(start), taginfo['name']))`
		`- return repo`
		`- else:`
		`- # no check requested -- return first ready repo`
		`- return repo`
		`-`
		`- if (time.time() - start) > (self.TIMEOUT * 60.0):`
		`- if builds:`
		`- raise koji.GenericError("Unsuccessfully waited %s for %s to appear "`
		`- "in the %s repo" %`
		`- (koji.util.duration(start), koji.util.printList(nvrs),`
		`- taginfo['name']))`
		`- else:`
		`- raise koji.GenericError("Unsuccessfully waited %s for a new %s repo" %`
		`- (koji.util.duration(start), taginfo['name']))`
		`-`
		`- time.sleep(self.PAUSE)`
		`- last_repo = repo`
		`-`
		`-`
		`def get_options():`
		`"""process options from command line and config file"""`
		`# parse command line args`

docs/source/winbuild.rst

file modified

-4

		`@@ -239,10 +239,6 @@`

		* They run the ``kojivmd`` daemon instead of the regular ``kojid`` daemon
		`* VM images for builds must be stored in the local image directory`
		- * You may need a ``kojid`` host in the ``vm`` channel as well. ``kojivmd``
		- cannot perform any ``waitrepo`` tasks, so you'll need something to complete
		- those tasks. kojivmd creates those tasks if a user initiates a ``winbuild``
		`- task with a build tag that does not yet have a repository.`

		`Managing VM Images`
		`------------------`

koji/tasks.py

file modified

+84

		`@@ -837,3 +837,87 @@`
		`results['task_id'] = rpm_task_id`

		`return results`
		`+`
		`+`
		`+ class WaitrepoTask(BaseTaskHandler):`
		`+`
		`+ Methods = ['waitrepo']`
		`+ # mostly just waiting`
		`+ _taskWeight = 0.2`
		`+`
		`+ PAUSE = 60`
		`+ # time in minutes before we fail this task`
		`+ TIMEOUT = 120`
		`+`
		`+ def handler(self, tag, newer_than=None, nvrs=None):`
		`+ """Wait for a repo for the tag, subject to given conditions`
		`+`
		`+ newer_than: create_event timestamp should be newer than this`
		`+ nvr: repo should contain this nvr (which may not exist at first)`
		`+`
		`+ Only one of the options may be specified. If neither is, then`
		`+ the call will wait for the first ready repo.`
		`+`
		`+ Returns the repo info (from getRepo) of the chosen repo`
		`+ """`
		`+`
		`+ start = time.time()`
		`+`
		`+ taginfo = self.session.getTag(tag, strict=True)`
		`+ targets = self.session.getBuildTargets(buildTagID=taginfo['id'])`
		`+ if not targets:`
		`+ raise koji.GenericError("No build target for tag: %s" % taginfo['name'])`
		`+`
		`+ if isinstance(newer_than, six.string_types) and newer_than.lower() == "now":`
		`+ newer_than = start`
		`+ if not isinstance(newer_than, six.integer_types + (type(None), float)):`
		`+ raise koji.GenericError("Invalid value for newer_than: %s" % newer_than)`
		`+`
		`+ if newer_than and nvrs:`
		`+ raise koji.GenericError("only one of (newer_than, nvrs) may be specified")`
		`+`
		`+ if not nvrs:`
		`+ nvrs = []`
		`+ builds = [koji.parse_NVR(nvr) for nvr in nvrs]`
		`+`
		`+ last_repo = None`
		`+`
		`+ while True:`
		`+ try:`
		`+ taginfo = self.session.getTag(tag, strict=True)`
		`+ except koji.GenericError:`
		`+ self.logger.debug("Tag %s got lost while waiting for newrepo", tag)`
		`+ raise koji.GenericError("Unsuccessfully waited %s for %s repo. "`
		`+ "Tag was probably deleted meanwhile." %`
		`+ (koji.util.duration(start), tag))`
		`+ repo = self.session.getRepo(taginfo['id'])`
		`+ if repo and repo != last_repo:`
		`+ if builds:`
		`+ if koji.util.checkForBuilds(`
		`+ self.session, taginfo['id'], builds, repo['create_event']):`
		`+ self.logger.debug("Successfully waited %s for %s to appear "`
		`+ "in the %s repo" %`
		`+ (koji.util.duration(start), koji.util.printList(nvrs),`
		`+ taginfo['name']))`
		`+ return repo`
		`+ elif newer_than:`
		`+ if repo['create_ts'] > newer_than:`
		`+ self.logger.debug("Successfully waited %s for a new %s repo" %`
		`+ (koji.util.duration(start), taginfo['name']))`
		`+ return repo`
		`+ else:`
		`+ # no check requested -- return first ready repo`
		`+ return repo`
		`+`
		`+ if (time.time() - start) > (self.TIMEOUT * 60.0):`
		`+ if builds:`
		`+ raise koji.GenericError("Unsuccessfully waited %s for %s to appear "`
		`+ "in the %s repo" %`
		`+ (koji.util.duration(start), koji.util.printList(nvrs),`
		`+ taginfo['name']))`
		`+ else:`
		`+ raise koji.GenericError("Unsuccessfully waited %s for a new %s repo" %`
		`+ (koji.util.duration(start), taginfo['name']))`
		`+`
		`+ time.sleep(self.PAUSE)`
		`+ last_repo = repo`

vm/kojivmd

file modified

+4 -3

		`@@ -51,10 +51,11 @@`
		`from koji.tasks import ( # noqa: F401`
		`BaseTaskHandler,`
		`MultiPlatformTask,`
		`- RestartTask,`
		`- RestartVerifyTask,`
		`+ RestartTask, # handle restarted vm tasks`
		`+ RestartVerifyTask, # handle restarted vm tasks`
		`ServerExit,`
		`- ServerRestart`
		`+ ServerRestart,`
		`+ WaitrepoTask, # wait for repo regens spawned by kojivmd`
		`)`

tkopecek commented 2 years ago

When moved there, kojivmd can import it and wait for
regen repos it started.

rebased onto c3107c529ec9532aaf958457d3f73fa097f85d27

2 years ago

rebased onto b2eb861

2 years ago

mikem commented 2 years ago

The relocated code has the following change

-        if not isinstance(newer_than, tuple(to_list(six.integer_types) + [type(None), float])):
+        if not isinstance(newer_than, list(six.integer_types) + [type(None), float]):

While I suspect this check could benefit from some cleanup, I'm pretty sure this check is going to fail because isinstance expects a type or a tuple of types, not a list.

I think the right check should be:

if not isinstance(newer_than, six.integer_types + (type(None), float)):

1 new commit added

fix type check

2 years ago

tkopecek commented 2 years ago

ouch, you're right - fixed

mikem commented 2 years ago

:thumbsup:

mikem commented 2 years ago

For future reference, the change in the relocated code now looks like:

-        if not isinstance(newer_than, tuple(to_list(six.integer_types) + [type(None), float])):
+        if not isinstance(newer_than, six.integer_types + (type(None), float)):

six.integer_types is meant to be used with isinstance, so it should always be a tuple

Metadata Update from @tkopecek:
- Pull-request tagged with: no_qe

2 years ago

tkopecek commented 2 years ago

Commit fb53395 fixes this pull-request

Pull-Request has been merged by tkopecek

2 years ago