2:09 p.m.
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