[PATCH] add mechanism for general purpose parameter passing to Image Factory
Mike McLean
mikem at redhat.com
Tue Nov 17 20:09:12 UTC 2015
On 10/27/2015 11:30 PM, Ian McLeod wrote:
> We originally added semi-generic parameter passing for OVA support and
> then overloaded it for Vagrant support, before breaking that back out
> as specific image types.
>
> This change adds a fairly generic mechanism for parameter passthrough
> to plugins and the base builder that does not require patching the
> builder and CLI each time a new class of parameter is added to Factory.
>
> The immediate reason for doing this is to expose the Docker plugin's
> ability to set custom labels, environments and commands, but it is
> likely to be useful for other things in the future.
The mutable default args in _do_target_image jump out at me. (One was
there before, the other has been added). Currently we never call the
function without either arg, so the code as-is won't hit that particular
python gotcha. Still, as long as we're touching this bit of code, we
should fix that to avoid future problems.
Other than that, looks sane. I'll pull in and tweak the args.
> ---
> builder/kojid | 39 ++++++++++++++++++++++++++++++++++-----
> cli/koji | 15 ++++++++++++++-
> 2 files changed, 48 insertions(+), 6 deletions(-)
>
> diff --git a/builder/kojid b/builder/kojid
> index 4ea93f2..ad427cd 100755
> --- a/builder/kojid
> +++ b/builder/kojid
> @@ -3178,6 +3178,25 @@ class BaseImageTask(OzImageTask):
> raise koji.ApplianceError('Image status is %s: %s' %
> (status, details))
>
> + def _mergeFactoryParams(self, img_opts, fixed_params):
> + """
> + Merge any KV pairs passed in via --factory-parameter CLI options
> + into an existing dictionary that will eventually be passed to
> + factory build commands. This allows a fairly generic mechanism
> + for parameter passthrough to plugins and the base builder that does
> + not require patching the builder and CLI each time.
> +
> + @args:
> + img_opts - an existing dict with any pre-existing parameters
> + fixed_params - list of parameters that must not be overriden
> + @returns:
> + nothing - dict is modified in place
> + """
> + if self.opts.get('factory_parameter'):
> + for kvp in self.opts.get('factory_parameter'):
> + if kvp[0] not in fixed_params:
> + img_opts[kvp[0]] = kvp[1]
> +
> def _buildBase(self, template, params, wait=True):
> """
> Build a base image using ImageFactory. This is a "raw" image.
> @@ -3194,7 +3213,10 @@ class BaseImageTask(OzImageTask):
> # may need to still upload ozlog and remove the log handler
> self.logger.info('dispatching a baseimg builder')
> self.logger.debug('templates: %s' % template)
> - self.logger.debug('params: %s' % params)
> + self.logger.debug('pre-merge params: %s' % params)
> + # We enforce various things related to the ks file - do not allow override
> + self._mergeFactoryParams(params, [ 'install_script' ])
> + self.logger.debug('post-merge params: %s' % params)
> base = self.bd.builder_for_base_image(template, parameters=params)
> if wait:
> base.base_thread.join()
> @@ -3245,19 +3267,22 @@ class BaseImageTask(OzImageTask):
> if format == 'vagrant-virtualbox':
> format = 'vsphere-ova'
> img_opts['vsphere_ova_format'] = 'vagrant-virtualbox'
> + fixed_params = [ 'vsphere_ova_format' ]
> if format == 'vagrant-libvirt':
> format = 'rhevm-ova'
> img_opts['rhevm_ova_format'] = 'vagrant-libvirt'
> + fixed_params = [ 'rhevm_ova_format' ]
> if format == 'vagrant-vmware-fusion':
> format = 'vsphere-ova'
> img_opts['vsphere_ova_format'] = 'vagrant-vmware-fusion'
> # The initial disk image transform for VMWare Fusion/Workstation requires a "standard" VMDK
> # not the stream oriented format used for VirtualBox or regular VMWare OVAs
> img_opts['vsphere_vmdk_format'] = 'standard'
> + fixed_params = [ 'vsphere_ova_format', 'vsphere_vmdk_format' ]
> targ = self._do_target_image(self.base_img.base_image.identifier,
> - format.replace('-ova', ''), img_opts=img_opts)
> + format.replace('-ova', ''), img_opts=img_opts, fixed_params=fixed_params)
> targ2 = self._do_target_image(targ.target_image.identifier, 'OVA',
> - img_opts=img_opts)
> + img_opts=img_opts, fixed_params=fixed_params)
> return {'image': targ2.target_image.data}
>
> def _buildDocker(self, format):
> @@ -3275,7 +3300,7 @@ class BaseImageTask(OzImageTask):
> 'docker', img_opts=img_opts)
> return {'image': targ.target_image.data}
>
> - def _do_target_image(self, base_id, image_type, img_opts={}):
> + def _do_target_image(self, base_id, image_type, img_opts={}, fixed_params=[]):
> """
> A generic method for building what ImageFactory calls "target images".
> These are images based on a raw disk that was built before using the
> @@ -3287,6 +3312,8 @@ class BaseImageTask(OzImageTask):
> uses this to figure out what plugin to run
> img_opts - a dict of additional options that specific to the target
> type we pass in via image_type
> + fixed_params - a list of parameter keys that should not be overriden
> + by the --factory-parameter CLI
> @returns:
> A Builder() object from ImageFactory that contains information
> about the image building include state and progress.
> @@ -3294,7 +3321,9 @@ class BaseImageTask(OzImageTask):
> # TODO: test the failure case where IF itself throws an exception
> # ungracefully (missing a plugin for example)
> # may need to still upload ozlog and remove the log handler
> - self.logger.debug('img_opts: %s' % img_opts)
> + self.logger.debug('img_opts_pre_merge: %s' % img_opts)
> + self._mergeFactoryParams(img_opts, fixed_params)
> + self.logger.debug('img_opts_post_merge: %s' % img_opts)
> target = self.bd.builder_for_target_image(image_type,
> image_id=base_id, template=None, parameters=img_opts)
> target.target_thread.join()
> diff --git a/cli/koji b/cli/koji
> index 64c5356..cbfa2d0 100755
> --- a/cli/koji
> +++ b/cli/koji
> @@ -5306,6 +5306,11 @@ def handle_image_build(options, session, args):
> parser.add_option("--ova-option", action="append",
> help=_("Override a value in the OVA description XML. Provide a value " +
> "in a name=value format, such as 'ovf_memory_mb=6144'"))
> + parser.add_option("--factory-parameter", nargs=2, action="append",
> + help=_("Pass a parameter to Image Factory. The results are highly specific " +
> + "to the image format being created. This is a two argument parameter " +
> + "that can be specified an arbitrary number of times. For example: "
> + "--factory-parameter docker_cmd '[ \"/bin/echo Hello World\" ]'"))
> parser.add_option("--release", help=_("Forcibly set the release field"))
> parser.add_option("--repo", action="append",
> help=_("Specify a repo that will override the repo used to install " +
> @@ -5355,6 +5360,14 @@ def handle_image_build(options, session, args):
> for k, v in config.items(section):
> task_options.ova_option.append('%s=%s' % (k, v))
>
> + # as do factory-parameters
> + section = 'factory-parameters'
> + if config.has_section(section):
> + task_options.factory_parameter = [ ]
> + for k, v in config.items(section):
> + # We do this, rather than a dict, to match what argparse spits out
> + task_options.factory_parameter.append( (k, v) )
> +
> else:
> if len(args) < 5:
> parser.error(_("At least five arguments are required: a name, " +
> @@ -5483,7 +5496,7 @@ def _build_image_oz(options, task_opts, session, args):
> hub_opts = {}
> for opt in ('ksurl', 'ksversion', 'kickstart', 'scratch', 'repo',
> 'release', 'skip_tag', 'specfile', 'distro', 'format',
> - 'disk_size', 'ova_option'):
> + 'disk_size', 'ova_option', 'factory_parameter'):
> val = getattr(task_opts, opt, None)
> if val is not None:
> hub_opts[opt] = val
>
More information about the buildsys
mailing list