#353 add pre/postSCMCheckout plugin_callbacks
Merged 6 years ago by mikem. Opened 7 years ago by julian8628.
julian8628/koji scm-decorator  into  master

file modified
+50 -12
@@ -899,7 +899,7 @@ 

      def getSRPMFromSCM(self, url, build_tag, repo_id):

          #TODO - allow different ways to get the srpm

          task_id = self.session.host.subtask(method='buildSRPMFromSCM',

-                                             arglist=[url, build_tag, {'repo_id': repo_id}],

+                                             arglist=[url, build_tag, {'repo_id': repo_id, 'scratch': self.opts.get('scratch')}],

                                              label='srpm',

                                              parent=self.id)

          # wait for subtask to finish
@@ -1264,7 +1264,7 @@ 

                  raise koji.BuildError('no repo for tag %s' % build_tag['name'])

  

          build_opts = dslice(opts, ['goals', 'profiles', 'properties', 'envs', 'patches',

-                                    'packages', 'jvm_options', 'maven_options', 'deps'],

+                                    'packages', 'jvm_options', 'maven_options', 'deps', 'scratch'],

                              strict=False)

          build_opts['repo_id'] = repo_id

  
@@ -1364,7 +1364,6 @@ 

  

          scm = SCM(url)

          scm.assert_allowed(self.options.allowed_scms)

- 

          repo_id = opts.get('repo_id')

          if not repo_id:

              raise koji.BuildError('A repo_id must be provided')
@@ -1415,8 +1414,14 @@ 

          logfile = self.workdir + '/checkout.log'

          uploadpath = self.getUploadDir()

  

+ 

+         self.run_callbacks('preSCMCheckout', scminfo=scm.get_info(), build_tag=build_tag, scratch=opts.get('scratch'))

          # Check out sources from the SCM

          sourcedir = scm.checkout(scmdir, self.session, uploadpath, logfile)

+         self.run_callbacks("postSCMCheckout",

+                            scminfo=scm.get_info(),

+                            scratch=opts.get('scratch'),

+                            srcdir=sourcedir)

  

          # zip up pristine sources for auditing purposes

          self._zip_dir(sourcedir, os.path.join(outputdir, 'scm-sources.zip'))
@@ -1426,9 +1431,14 @@ 

              patchlog = self.workdir + '/patches.log'

              patch_scm = SCM(self.opts.get('patches'))

              patch_scm.assert_allowed(self.options.allowed_scms)

+             self.run_callbacks('preSCMCheckout', scminfo=patch_scm.get_info(), build_tag=build_tag, scratch=opts.get('scratch'))

              # never try to check out a common/ dir when checking out patches

              patch_scm.use_common = False

              patchcheckoutdir = patch_scm.checkout(patchdir, self.session, uploadpath, patchlog)

+             self.run_callbacks("postSCMCheckout",

+                                scminfo=patch_scm.get_info(),

+                                scratch=opts.get('scratch'),

+                                srcdir=patchcheckoutdir)

              self._zip_dir(patchcheckoutdir, os.path.join(outputdir, 'patches.zip'))

  

          # Apply patches, if present
@@ -1709,7 +1719,12 @@ 

          logfile = os.path.join(self.workdir, 'checkout.log')

          scmdir = buildroot.rootdir() + '/tmp/scmroot'

          koji.ensuredir(scmdir)

+         self.run_callbacks('preSCMCheckout', scminfo=scm.get_info(), build_tag=build_tag, scratch=opts.get('scratch'))

          specdir = scm.checkout(scmdir, self.session, self.getUploadDir(), logfile)

+         self.run_callbacks("postSCMCheckout",

+                            scminfo=scm.get_info(),

+                            scratch=opts.get('scratch'),

+                            srcdir=specdir)

  

          spec_template = None

          for path, dir, files in os.walk(specdir):
@@ -2619,7 +2634,7 @@ 

          self.logger.debug("Image buildroot ready: " + broot.rootdir())

          return broot

  

-     def fetchKickstart(self, broot, ksfile):

+     def fetchKickstart(self, broot, ksfile, build_tag):

          """

          Retrieve the kickstart file we were given (locally or remotely) and

          upload it.
@@ -2633,6 +2648,7 @@ 

          @args:

              broot: a buildroot object

              ksfile: path to a kickstart file

+             build_tag: build tag name

          @returns: absolute path to the retrieved kickstart file

          """

          scmdir = os.path.join(broot.rootdir(), 'tmp')
@@ -2642,7 +2658,12 @@ 

              scm = SCM(self.opts['ksurl'])

              scm.assert_allowed(self.options.allowed_scms)

              logfile = os.path.join(self.workdir, 'checkout.log')

+             self.run_callbacks('preSCMCheckout', scminfo=scm.get_info(), build_tag=build_tag, scratch=self.opts.get('scratch'))

              scmsrcdir = scm.checkout(scmdir, self.session, self.getUploadDir(), logfile)

+             self.run_callbacks("postSCMCheckout",

+                                scminfo=scm.get_info(),

+                                scratch=self.opts.get('scratch'),

+                                srcdir=scmsrcdir)

              kspath = os.path.join(scmsrcdir, ksfile)

          else:

              kspath = self.localPath("work/%s" % ksfile)
@@ -2816,7 +2837,7 @@ 

          self.opts = opts

          broot = self.makeImgBuildRoot(build_tag, repo_info, arch,

                                        'appliance-build')

-         kspath = self.fetchKickstart(broot, ksfile)

+         kspath = self.fetchKickstart(broot, ksfile, target_info['build_tag_name'])

          self.readKickstart(kspath, opts)

          kskoji = self.prepareKickstart(repo_info, target_info, arch, broot, opts)

          # Figure out appliance-creator arguments, let it fail if something
@@ -2965,7 +2986,7 @@ 

  

          broot = self.makeImgBuildRoot(build_tag, repo_info, arch,

              'livecd-build')

-         kspath = self.fetchKickstart(broot, ksfile)

+         kspath = self.fetchKickstart(broot, ksfile, target_info['build_tag_name'])

          self.readKickstart(kspath, opts)

          kskoji = self.prepareKickstart(repo_info, target_info, arch, broot, opts)

  
@@ -3115,7 +3136,7 @@ 

  

          broot = self.makeImgBuildRoot(build_tag, repo_info, arch,

              'livemedia-build')

-         kspath = self.fetchKickstart(broot, ksfile)

+         kspath = self.fetchKickstart(broot, ksfile, target_info['build_tag_name'])

          self.readKickstart(kspath, opts)

          kskoji = self.prepareKickstart(repo_info, target_info, arch, broot, opts)

  
@@ -3233,7 +3254,7 @@ 

  class OzImageTask(BaseTaskHandler):

      Methods = []

  

-     def fetchKickstart(self):

+     def fetchKickstart(self, build_tag):

          """

          Retrieve the kickstart file we were given (locally or remotely) and

          upload it to the hub.
@@ -3244,7 +3265,8 @@ 

          relative path in a remote scm. The user should have passed in an scm

          url with --ksurl.

  

-         @args: None, use self.opts for options

+         @args: build_tag: build tag name

+                use self.opts for options

          @returns:

              absolute path to the retrieved kickstart file

          """
@@ -3254,8 +3276,13 @@ 

              scm = SCM(self.opts['ksurl'])

              scm.assert_allowed(self.options.allowed_scms)

              logfile = os.path.join(self.workdir, 'checkout-%s.log' % self.arch)

+             self.run_callbacks('preSCMCheckout', scminfo=scm.get_info(), build_tag=build_tag, scratch=self.opts.get('scratch'))

              scmsrcdir = scm.checkout(self.workdir, self.session,

                  self.getUploadDir(), logfile)

+             self.run_callbacks("postSCMCheckout",

+                                scminfo=scm.get_info(),

+                                scratch=self.opts.get('scratch'),

+                                srcdir=scmsrcdir)

              kspath = os.path.join(scmsrcdir, os.path.basename(ksfile))

          else:

              tops = dict([(k, getattr(self.options, k)) for k in 'topurl','topdir'])
@@ -3909,7 +3936,7 @@ 

          self.formats = self._format_deps(opts.get('format'))

  

          # First, prepare the kickstart to use the repos we tell it

-         kspath = self.fetchKickstart()

+         kspath = self.fetchKickstart(build_tag=target_info['build_tag_name'])

          ks = self.prepareKickstart(kspath, inst_tree)

          kskoji = self.writeKickstart(ks,

              os.path.join(self.workdir, 'koji-%s-%i-base.ks' %
@@ -4040,7 +4067,7 @@ 

  

      # END inefficient base image task method copies

  

-     def fetchHubOrSCM(self, filepath, fileurl):

+     def fetchHubOrSCM(self, filepath, fileurl, build_tag):

          """

          Retrieve a file either from the hub or a remote scm

  
@@ -4060,9 +4087,14 @@ 

          if fileurl:

              scm = SCM(fileurl)

              scm.assert_allowed(self.options.allowed_scms)

+             self.run_callbacks('preSCMCheckout', scminfo=scm.get_info(), build_tag=build_tag, scratch=self.opts.get('scratch'))

              logfile = os.path.join(self.workdir, 'checkout.log')

              scmsrcdir = scm.checkout(self.workdir, self.session,

                  self.getUploadDir(), logfile)

+             self.run_callbacks("postSCMCheckout",

+                                scminfo=scm.get_info(),

+                                scratch=self.opts.get('scratch'),

+                                srcdir=scmsrcdir)

              final_path = os.path.join(scmsrcdir, os.path.basename(filepath))

          else:

              tops = dict([(k, getattr(self.options, k)) for k in 'topurl','topdir'])
@@ -4212,7 +4244,8 @@ 

              raise koji.ApplianceError('The Release may not have a hyphen')

  

          indirection_template = self.fetchHubOrSCM(opts.get('indirection_template'),

-                                                   opts.get('indirection_template_url'))

+                                                   opts.get('indirection_template_url'),

+                                                   target_info['build_tag_name'])

  

          self.logger.debug('Got indirection template %s' % (indirection_template))

  
@@ -4403,8 +4436,13 @@ 

          logfile = self.workdir + '/checkout.log'

          uploadpath = self.getUploadDir()

  

+         self.run_callbacks('preSCMCheckout', scminfo=scm.get_info(), build_tag=build_tag, scratch=self.opts.get('scratch'))

          # Check out spec file, etc. from SCM

          sourcedir = scm.checkout(scmdir, self.session, uploadpath, logfile)

+         self.run_callbacks("postSCMCheckout",

+                            scminfo=scm.get_info(),

+                            scratch=self.opts.get('scratch'),

+                            srcdir=sourcedir)

          # chown the sourcedir and everything under it to the mockuser

          # so we can build the srpm as non-root

          uid = pwd.getpwnam(self.options.mockuser)[2]

file modified
+26 -5
@@ -23,7 +23,7 @@ 

  import koji

  import koji.tasks

  from koji.tasks import safe_rmtree

- from koji.util import md5_constructor, adler32_constructor, parseStatus

+ from koji.util import md5_constructor, adler32_constructor, parseStatus, dslice

  import os

  import signal

  import logging
@@ -229,6 +229,11 @@ 

              # should never happen

              raise koji.GenericError('Invalid SCM URL: %s' % url)

  

+     def get_info(self, keys=None):

+         if keys is None:

+             keys = ["url", "scheme", "user", "host", "repository", "module", "revision", "scmtype"]

+         return dslice(vars(self), keys)

+ 

      def _parse_url(self):

          """

          Parse the SCM url into usable components.
@@ -513,13 +518,29 @@ 

      def findHandlers(self, vars):

          """Find and index task handlers"""

          for v in vars.values():

-             if isinstance(v, type(koji.tasks.BaseTaskHandler)) and issubclass(v, koji.tasks.BaseTaskHandler):

-                 for method in v.Methods:

-                     self.handlers[method] = v

+             self.registerHandler(v)

+ 

+     def registerHandler(self, entry):

+         """register and index task handler"""

+         if isinstance(entry, type(koji.tasks.BaseTaskHandler)) and issubclass(entry, koji.tasks.BaseTaskHandler):

+             for method in entry.Methods:

+                 self.handlers[method] = entry

+ 

+     def registerCallback(self, entry):

+         """register and index callback plugins"""

+         if callable(entry) and getattr(entry, 'callbacks', None):

+             for cbtype in entry.callbacks:

+                 koji.plugin.register_callback(cbtype, entry)

+ 

+     def registerEntries(self, vars):

+         """Register task handlers and other plugins"""

+         for v in vars.values():

+             self.registerHandler(v)

+             self.registerCallback(v)

  

      def scanPlugin(self, plugin):

          """Find task handlers in a plugin"""

-         self.findHandlers(vars(plugin))

+         self.registerEntries(vars(plugin))

  

      def shutdown(self):

          """Attempt to shut down cleanly"""

file modified
+2
@@ -48,6 +48,8 @@ 

      'postRepoDone':           [],

      'preCommit':              [],

      'postCommit':             [],

+     'preSCMCheckout':         [],

+     'postSCMCheckout':        [],

      }

  

  class PluginTracker(object):

file modified
+12
@@ -383,6 +383,18 @@ 

          return repo_info

  

  

+     def run_callbacks(self, plugin, *args, **kwargs):

+         if 'taskinfo' not in kwargs:

+             try:

+                 taskinfo = self.taskinfo

+             except AttributeError:

+                 self.taskinfo = self.session.getTaskInfo(self.id, request=True)

+                 taskinfo = self.taskinfo

+             kwargs['taskinfo'] = taskinfo

+         kwargs['session'] = self.session

+         koji.plugin.run_callbacks(plugin, *args, **kwargs)

+ 

+ 

  class FakeTask(BaseTaskHandler):

      Methods = ['someMethod']

      Foreground = True

  1. remove request_keys from xibo's patch, in order to only pass taskinfo, scminfo, and srcdir to plugin
  2. add this plugin callbacks into winbuild.

1 new commit added

  • remove useless import
7 years ago

Next two lines use original info variable.

python3 doesn't know has_key, please use 'k in vars(self)' instead

Doesn't it make more sense to put these calls inside scm.checkout() ? It would be maintained only in one place and it will have access to SCM's scope. Not sure if it is not against original requirements.

fixed no1 and no2 problems from @tkopecek
About no3 problem, I think putting the calls in scm.checkout() is better too. I'm not sure why @xning write the calls around it, maybe he didn't want to modify scm.checkout() or scm.__init__()'s signature?
@mikem What's your opinion about it?

2 new commits added

  • use util.dslice instead
  • fix reference problem
7 years ago

This is digging into some of the challenge and earlier debate. The callback will also want information from the task scope (e.g. is this a for a scratch build). There's no perfect answer.

I think putting the calls in scm.checkout() is better too

In order to do anything useful with these callbacks, we will need information about the task. That is not available in the SCM context.

It currently fails for me (rebased to master) with such command:
koji build --scratch f24 git://xyz#origin/random_branch

Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/koji/daemon.py", line 1166, in runTask
    response = (handler.run(),)
  File "/usr/lib/python2.7/site-packages/koji/tasks.py", line 158, in run
    return koji.util.call_with_argcheck(self.handler, self.params, self.opts)
 File "/usr/lib/python2.7/site-packages/koji/util.py", line 156, in call_with_argcheck
    return func(*args, **kwargs)
 File "/usr/sbin/kojid", line 4428, in handler
self.run_plugin('preSCMCheckout', scminfo=scm.get_info())
  File "/usr/lib/python2.7/site-packages/koji/daemon.py", line 236, in get_info
    return dslice(vars(self), keys)
 File "/usr/lib/python2.7/site-packages/koji/util.py", line 140, in dslice
   ret[key] = dict[key]
KeyError: 'path'

vars(self) in get_info contains:

{
    'repository': '/rpms/koji',
    'url': 'git://xyz#origin/random_branch',
    'scmtype': 'GIT',
    'module': '',
    'source_cmd': ['rhpkg'],
    'host': 'xyz',
    'user': None,
    'logger': <logging.Logger object at 0x7fc6c27dcdd0>,
    'scheme': 'git://',
    'use_common': False,
    'revision': 'origin/random_branch'
}

rebased

7 years ago

@tkopecek Thanks for your test.
Updated.

One more issue. How I'm supposed to register for this call? I've tried to create primitive builder plugin:

import logging
from koji.plugin import callback

@callback('preSCMCheckout')
def mycallback(*args, **kwargs):
    logging.debug(str(kwargs))

But these callbacks are never registered in builder (via register_callback). Registering happens only in hub. Is this the intended way? In such case, koji.daemon.scanPlugin / findHandlers needs to be extended to register also these.

rebased

7 years ago

Update the code to enable callback plugin for builder and vm.
example args are:
{'scminfo': {'repository': '/azhuzhu/simple-dist', 'url': 'git://github.com/azhuzhu/simple-dist?#62ed4a12adfbe273b100f069c39965b07cf54761', 'scmtype': 'GIT', 'module': '', 'host': 'github.com', 'user': None, 'scheme': 'git://', 'revision': '62ed4a12adfbe273b100f069c39965b07cf54761'}, 'taskinfo': {'weight': 1.0, 'parent': 198, 'completion_time': None, 'request': ['git://github.com/azhuzhu/simple-dist?#62ed4a12adfbe273b100f069c39965b07cf54761', 6, {'repo_id': 25}], 'start_time': '2017-04-25 02:02:33.478973', 'start_ts': 1493085753.4789701, 'state': 1, 'awaited': True, 'label': 'srpm', 'priority': 19, 'channel_id': 1, 'waiting': None, 'create_time': '2017-04-25 02:02:33.441804', 'id': 199, 'create_ts': 1493085753.4418001, 'owner': 1, 'host_id': 1, 'completion_ts': None, 'arch': 'noarch', 'method': 'buildSRPMFromSCM'}}

Here's no taginfo, but it could be got by
1. parent task's request in plugin
2. task handler and then pass it to callback, which could be passed from parent task.
1 is easier for coding.
2 seems more direct and efficient but complex

@mikem @tkopecek Any thought?

As query is possible, I would stay with option one for simplicity.
BTW - it works for me now correctly.

One more question - how I'm supposed to get e.g. reference to hub connection? Parsing builder config shouldn't be necessary.

Guess using context works for this situation.

parent = context.handlers.call('getTaskInfo', info['parent'], request=True)

this is an example from mavensign hub plugin.

Let me test this solution at first.

You have a reference to the build_tag dict here (and everywhere else we're calling this plugin, I believe). Seems like it would make sense to pass that to the callback as well.

@mikeb Maybe whole target info?
@tkopecek Not every task has a reference to the build target. buildSRPMFromSCM only gets passed a build_tag, which could be associated with multiple targets.

For the SCM object in kojikamid.WindowsBuild, session instance cannot be got, so I'll loose the requirement of arguments of SCMCheckout plugin. build_tag, session, taskinfo, scratch will be pushed as mush as possible. I think it would work for current code.
Is there possibly any underlying problem here?

For the SCM object in kojikamid.WindowsBuild, session instance cannot be got, so I'll loose the requirement of arguments of SCMCheckout plugin. build_tag, session, taskinfo, scratch will be pushed as mush as possible. I think it would work for current code.

Find a solution here: invoking callback in kojikamid.WindowsBuild via remote call of VMExecTask

rebased

6 years ago

8 new commits added

  • push build_tag, session, scratch into callbacks
  • enable callback plugin for builder
  • fix fields in SCM
  • use util.dslice instead
  • fix reference problem
  • remove useless import
  • only path taskinfo, scminfo, [srcdir] to plugin
  • patch2 for issue 288
6 years ago

updated.
Here I don't control the type of build_tag, It might be a tagname or taginfo dict or even tagID, which should be checked in callback plugin.
session is used to get more information from hub.
Also pass scratch into postSCMCheckout callbacks.

1 new commit added

  • remove callbacks in win builder
6 years ago

I've removed the callback caller in win vm builder, since there's no requirement for winbuild.

I think this is about where we need to be. Just a couple things.

  • let's call it run_callbacks instead of run_plugin
  • callbacks should register themselves with the decorator, so I don't think we need to have that registerCallback call, but please let me know if I'm missing something
  • since we're not changing kojikamid, maybe don't touch it at all?

callbacks should register themselves with the decorator, so I don't think we need to have that registerCallback call, but please let me know if I'm missing something

@mikem current callback decorator doesn't invoke koji.plugin.register_callback(), so if I didn't misunderstand your reply, I guess it's necessary to register callbacks in kojid, like what's done in kojixmlrpc. But, refactoring plugin.py to make all available plugins to be equipped would be better than current implementation I think. Would we done this within this feature?

rebased

6 years ago

10 new commits added

  • change run_plugin to run_callbacks and remove the modification for kojikamid.py
  • remove callbacks in win builder
  • push build_tag, session, scratch into callbacks
  • enable callback plugin for builder
  • fix fields in SCM
  • use util.dslice instead
  • fix reference problem
  • remove useless import
  • only path taskinfo, scminfo, [srcdir] to plugin
  • patch2 for issue 288
6 years ago

current callback decorator doesn't invoke koji.plugin.register_callback(),

Hmm, I guess I am mistaken. I didn't realize the hub was also doing this.

rebased

6 years ago

add scratch param for 'preSCMCheckout' callbacks, too

ping @mikem - any more changes needed?

Commit 5bd1d33 fixes this pull-request

Pull-Request has been merged by mikem@redhat.com

6 years ago