From 7199920d84908e829635d766f668c69570828bdc Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Tue, 2 Dec 2014 15:06:11 +1100 Subject: [PATCH] Check env-vars is a dictionary In change Ic6c21f715441a16a1d832163dc71921f25d654df I made the mistake of specifying env-var's as a list, rather than a dict, in the yaml. In this change we check that env-vars is a dictionary and soft-fail if it is not. Additionally a script 'fake-image-create' is added to be using during testing. This is spawned in-place of disk-image-create and can check arbitrary things, returning non-zero if it finds them. We check for some added environment variables as a first step. The documentation is also clarified slightly with an expanded example. Change-Id: I7fe1972fdcc7e8b0231e9d232485da9c5dfdb31b --- doc/source/configuration.rst | 23 ++++++-------- nodepool/nodepool.py | 17 +++++----- nodepool/tests/fake-image-create | 31 +++++++++++++++++++ nodepool/tests/fixtures/node_dib.yaml | 6 +++- .../tests/fixtures/node_dib_and_snap.yaml | 5 +++ 5 files changed, 61 insertions(+), 21 deletions(-) create mode 100755 nodepool/tests/fake-image-create diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 5331603b1..4e88c2e97 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -173,21 +173,18 @@ will be built using the provider snapshot approach:: qemu-img-options: compat=0.10 env-vars: DIB_DISTRIBUTION_MIRROR: http://archive.ubuntu.com + DIB_EXTRA_VARIABLE: foobar For diskimages, the `name` is required. The `elements` section -enumerates all the elements that will be included when building -the image, and will point to the `elements-dir` path referenced -in the same config file. `release` specifies the distro to be -used as a base image to build the image using diskimage-builder. -`qemu-img-options` allows to specify custom settings that qemu -will be using to build the final image. Settings there have to -be separated by commas, and must follow qemu syntax. - -The `env-vars` key is optional. It allows to specify a list of -environment variables that will be appended to the variables -that are send by default to diskimage-builder. Using that approach -it is possible to send the DIB_*environment variables neeeded by -diskimage-builder elements per image type. +enumerates all the elements that will be included when building the +image, and will point to the `elements-dir` path referenced in the +same config file. `release` specifies the distro to be used as a base +image to build the image using diskimage-builder. `qemu-img-options` +allows to specify custom settings that qemu will be using to build the +final image. Settings there have to be separated by commas, and must +follow qemu syntax. `env-vars` is an optional dictionary of arbitrary +environment variables that will be available in the spawned +diskimage-builder child process. providers --------- diff --git a/nodepool/nodepool.py b/nodepool/nodepool.py index fb2dc9fcc..c0da269f8 100644 --- a/nodepool/nodepool.py +++ b/nodepool/nodepool.py @@ -766,11 +766,13 @@ class DiskImageBuilder(threading.Thread): image.qemu_img_options) img_elements = image.elements - cmd = ('disk-image-create -x --no-tmpfs %s -o %s %s' % - (extra_options, out_file_path, img_elements)) - if 'fake-' in filename: - cmd = 'echo ' + cmd + dib_cmd = 'nodepool/tests/fake-image-create' + else: + dib_cmd = 'disk-image-create' + + cmd = ('%s -x --no-tmpfs %s -o %s %s' % + (dib_cmd, extra_options, out_file_path, img_elements)) log = logging.getLogger("nodepool.image.build.%s" % (image_name,)) @@ -1247,9 +1249,10 @@ class NodePool(threading.Thread): d.elements = '' d.release = diskimage.get('release', '') d.qemu_img_options = diskimage.get('qemu-img-options', '') - if 'env-vars' in diskimage: - d.env_vars = diskimage['env-vars'] - else: + d.env_vars = diskimage.get('env-vars', {}) + if not isinstance(d.env_vars, dict): + self.log.error("%s: ignoring env-vars; " + "should be a dict" % d.name) d.env_vars = {} for provider in config['providers']: diff --git a/nodepool/tests/fake-image-create b/nodepool/tests/fake-image-create new file mode 100755 index 000000000..da6936dfe --- /dev/null +++ b/nodepool/tests/fake-image-create @@ -0,0 +1,31 @@ +#!/bin/bash + +echo "*** fake-image-create: start" + +echo "arguments:" +echo "----" +echo $* +echo "----" + +# test passing of real-life env-vars +if [[ "${TMPDIR}" != "/opt/dib_tmp" ]]; then + echo "TMPDIR not set" + exit 1 +fi + +if [[ "${DIB_IMAGE_CACHE}" != "/opt/dib_cache" ]]; then + echo "DIB_IMAGE_CACHE not set" + exit 1 +fi + +if [[ "${DIB_CLOUD_IMAGES}" != "http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/" ]]; then + echo "DIB_CLOUD_IMAGES not set" + exit 1 +fi + +if [[ "${BASE_IMAGE_FILE}" != "Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2" ]]; then + echo "BASE_IMAGE_FILE not set" + exit 1 +fi + +echo "*** fake-image-create: done" diff --git a/nodepool/tests/fixtures/node_dib.yaml b/nodepool/tests/fixtures/node_dib.yaml index 3443cfcfa..68c09bb85 100644 --- a/nodepool/tests/fixtures/node_dib.yaml +++ b/nodepool/tests/fixtures/node_dib.yaml @@ -53,4 +53,8 @@ diskimages: - ubuntu - vm release: precise - + env-vars: + TMPDIR: /opt/dib_tmp + DIB_IMAGE_CACHE: /opt/dib_cache + DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/ + BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2 diff --git a/nodepool/tests/fixtures/node_dib_and_snap.yaml b/nodepool/tests/fixtures/node_dib_and_snap.yaml index feec7f7c9..e692faacc 100644 --- a/nodepool/tests/fixtures/node_dib_and_snap.yaml +++ b/nodepool/tests/fixtures/node_dib_and_snap.yaml @@ -70,4 +70,9 @@ diskimages: - ubuntu - vm release: precise + env-vars: + TMPDIR: /opt/dib_tmp + DIB_IMAGE_CACHE: /opt/dib_cache + DIB_CLOUD_IMAGES: http://download.fedoraproject.org/pub/fedora/linux/releases/test/21-Beta/Cloud/Images/x86_64/ + BASE_IMAGE_FILE: Fedora-Cloud-Base-20141029-21_Beta.x86_64.qcow2