diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 263716b..95f0880 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -1,14 +1,22 @@ -# This workflow will install Python dependencies, run tests and lint with a variety of Python versions +# This workflow will install Python dependencies, lint and run tests with a +# variety of Python versions, and lint the shell scripts # For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python name: Python package on: push: - branches: [ "master" ] + branches: [ "**" ] pull_request: branches: [ "master" ] +# Without this, a pull request from a branch in this repository is built twice, +# once for the push and once for the pull_request. Keying on the source branch +# puts both in the same group, and a new push supersedes a running build. +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.ref }} + cancel-in-progress: true + jobs: build: @@ -19,19 +27,41 @@ jobs: python-version: ["3.9", "3.10", "3.11"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v7 - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 + uses: actions/setup-python@v7 with: python-version: ${{ matrix.python-version }} - name: Install dependencies run: | python -m pip install --upgrade pip - pip install ruff pytest + # ruff is unpinned now that pyproject.toml selects rules explicitly, + # so a new release cannot widen the set under us. pytest stays pinned + # because 9 requires Python 3.10. + pip install ruff 'pytest==8.4.2' if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + pip install -e . - name: Lint with ruff run: | # stop the build if there are Python syntax errors or undefined names - ruff check --output-format=github --select=E9,F63,F7,F82 --target-version=py39 . - # default set of ruff rules with GitHub Annotations - ruff check --output-format=github --target-version=py39 . + ruff check --output-format=github --select=E9,F63,F7,F82 . + # the rule set from pyproject.toml, with GitHub Annotations + ruff check --output-format=github . + - name: Run tests + run: pytest -q + + shellcheck: + # Its own job rather than a step in the matrix: the shell scripts have + # nothing to do with the Python version, so there is no point linting them + # three times + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v7 + - name: Lint shell scripts + run: | + shellcheck --version + # Only build-images.sh for now. The provisioning scripts under + # scripts/ run on the guest rather than here and currently produce + # around a hundred findings, so adding them needs a cleanup first + shellcheck build-images.sh diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..e638c6f --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,144 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What this is + +A thin Python CLI wrapper around [Packer](https://packer.io) for building OpenStack (Glance) images in +[NREC](https://www.nrec.no). It exists so users don't have to write Packer templates: it creates the +throwaway OpenStack resources Packer needs, invokes `packer build` against a bundled template, and +cleans up afterwards. + +This fork's `origin` is `trondham/imagebuilder`; `upstream` is `norcams/imagebuilder`. + +## Commands + +```bash +# Lint — this is what CI runs (.github/workflows/python-package.yml), matrix py3.9/3.10/3.11. +# The rule set and target-version live in pyproject.toml, so ruff needs no flags +ruff check --select=E9,F63,F7,F82 . +ruff check . +shellcheck build-images.sh # its own CI job; scripts/ is not linted yet + +# Tests — CI runs these too; pytest is pinned to 8.4.2 because 9 requires 3.10 +pytest -q +pytest tests/test_main_flow.py -q # a single file +pytest -k "cleanup" -q # a single case + +# Dev install (see DEVELOPMENT.md) +python3 -m venv . && source bin/activate +pip install -r requirements.txt +pip install -e . + +# Run without installing +./imagebuilder # wrapper around image_builder.imagebuilder:main +``` + +Tests live in `tests/` and use stand-in OpenStack clients (`tests/conftest.py`) plus a fake HTTP +layer, so they need no credentials and touch no network. `test_main_flow.py` drives `main()` itself +with a fake `BuildFunctions` — that is where the cleanup-on-failure guarantees are pinned down. + +Packaging metadata and the console script live in `pyproject.toml`; there is no `setup.py`. + +Running any command requires OpenStack credentials in the environment (`source keystone_rc.sh`) — +`REQUIRED_ENV` in `imagebuilder.py` lists exactly what is read, and a missing one is reported by +name. `OS_CACERT` is optional. Deliberately *not* required: `OS_IDENTITY_API_VERSION` and +`OS_NO_CACHE` were once mandatory but never used, which rejected valid openrc files. `packer` and +`ssh-keygen` must be on `$PATH` — on RHEL/CentOS beware the unrelated `/usr/sbin/packer`; +`/usr/bin/packer` is the right one. + +## Architecture + +`image_builder/imagebuilder.py:main()` parses, configures logging, authenticates and dispatches to +`run_build()` or `run_bootstrap()`, which return an exit code rather than calling `sys.exit` so they +can be driven from a test. `parsecommands.Commands` uses argparse subparsers and exposes +`build_args` / `bootstrap_args`, one of which is `None`. + +**`build`** (`build.py:BuildFunctions`) — the core flow, in order: +1. `helpers.make_tmp_dir()` — a `tempfile.mkdtemp(prefix='imagebuilder-')` holding the SSH keypair and + Packer's manifest; everything in the flow is scoped to it. +2. `find_template()` — locates `template.pkr.hcl` in `template_dir`. +3. `find_network_id()` — resolves the network name (default `Dualstack`) to an id via openstacksdk. +4. `run_packer_init()` — `packer init`, which installs the openstack plugin. + + Steps 2–4 deliberately run *before* anything is created in OpenStack: each can fail on bad user + input, and failing here costs nothing to clean up. +5. `create_security_group()` / `create_keypairs()` — temporary OpenStack resources named + `imagebuilder-`, allowing SSH ingress on v4 and v6. The keypair is generated by shelling + out to `ssh-keygen`. +6. `run_packer()` — builds a `packer build -var k=v ...` argv and streams stdout through + `helpers.log_subprocess_output` into `logging`. +7. `parse_manifest()` — Packer's `manifest` post-processor writes `packer-manifest.json` into the temp + dir; `builds[0].artifact_id` is the resulting Glance image id. This is the only channel by which the + build result comes back from Packer. +8. Optional `download_image()` (via `conn.image.download_image`, then **deletes the image from + Glance**), optional `--purge-source`, then `cleanup()` + `clean_tmp_files()`. + +One openstacksdk `Connection` covers everything: `conn.compute` (keypairs), `conn.network` (security +groups, networks), `conn.image` (image create, download, delete). novaclient and glanceclient were +dropped once the last calls moved over. Note `self.conn` holds the connection rather than a proxy +such as `conn.network`, because touching +a proxy authenticates and discovers endpoints — the constructor is deliberately free of network +traffic. + +Everything after the first OpenStack resource is created runs under a `try/finally`, so `cleanup()` is +reached on exceptions and on Ctrl-C (SIGINT becomes `KeyboardInterrupt`, which unwinds). `cleanup()` is +written for that position: it skips ids that are `None` and swallows API errors, because raising there +would mask the original failure. SIGTERM and SIGKILL still bypass it — verified — so those remain the +one case that orphans `imagebuilder-*` resources. `sweep_orphans()` clears those up, but only when asked +— `--sweep`, or `IB_SWEEP=true` in the environment, which is how `build-images.sh -s` reaches it, +since the generated build scripts take no arguments of their own. It is opt-in because it deletes +resources the current run did not create. Only names matching `imagebuilder-<32 hex>`, only once +older than `SWEEP_MAX_AGE_HOURS` (builder nodes share a project, so the age is what stops it deleting a +resource a build elsewhere is using), and never images, whose names are indistinguishable from +legitimate ones. + +**`bootstrap`** (`bootstrap.py:BootstrapFunctions`) — downloads a cloud image from a URL, optionally +verifies it against a checksum file (`find_expected_checksum()` parses both the coreutils and BSD +layouts and matches on the filename, falling back to "the hash appears anywhere" with a warning when +no line names the download), and uploads it to Glance with hardware properties set from the flags (`hw_disk_bus=scsi` + +`hw_scsi_model=virtio-scsi` unless `-s`, `hw_firmware_type=uefi` + `hw_machine_type=q35` with `-e`, +always `hw_rng_model=virtio`). On success it writes the bare image id to **stdout** and nothing else — +all other output goes to `logging` — so `bootstrap ... | xargs -I % imagebuilder build -s % ...` works. + +**Config** (`config.py`) — `configparser` over `./config`, `~/.imagebuilder/config`, +`/etc/imagebuilder/config` (later files win). Only `[main] template_dir` and `download_dir` are read, +and `IB_TEMPLATE_DIR` / `IB_DOWNLOAD_DIR` override them. + +### Packer template and provision scripts + +`template.pkr.hcl` (HCL2) is a *file*, not a directory — `find_template()` looks for it in +`template_dir`, falling back to a legacy JSON template named `template` if that's all it finds (in +which case `packer init` is skipped and the openstack plugin must be installed by hand). Its +`provisioner "shell"` `scripts` list is a fixed pipeline of `scripts/*.sh` with `var.provision_script` +(the user's `-p` script, default `/bin/true`) slotted in near the end, before the cleanup scripts. +Adding a stage to every build means editing this list. Paths there use `${path.root}` — the directory +holding the template — so builds don't depend on Packer's working directory. + +Packer stopped bundling builder plugins in 1.10, so `packer init` must run before `packer build`; the +`required_plugins` block pins `github.com/hashicorp/openstack`. `required_plugins` is HCL2-only, which +is why the JSON template had to go. + +Scripts under `scripts/` run on the builder VM as the cloud-init user via `sudo`, across a wide range of +guests (RHEL 8/9/10, Alma, CentOS Stream, Fedora, Debian, Ubuntu). They share a copy-pasted +Omnitruck-style platform-detection preamble that sets `platform` (`el`/`fedora`/`debian`/`ubuntu`) and +`major_version`; keep that convention when adding one. A script that doesn't apply to the detected +platform prints a message and `exit 0` — it must not fail the build. Note `grub.sh`, `nvidia-grid-driver.sh`, +and `uib-puppet.sh` are not in the default template's list; they are opt-in, used as the `-p` script or +by site-specific templates. + +`nvidia-grid-driver.sh` pulls the vGPU guest driver and per-region licensing tokens from +`download.iaas.uio.no` and selects the BGO/OSL `gridd.conf` by grepping `/run/cloud-init/` for the +availability zone. Recent history is mostly churn here (pinning/unpinning driver versions, refreshing +client tokens) — check `git log -- scripts/nvidia-grid-driver.sh` before changing it. + +`example/` is a self-contained smoke-test setup (its own `config`, a trimmed `template`, a trivial +`provision.sh`, and Terraform to boot the resulting image); use it to sanity-check changes end to end. + +## Conventions + +- Commit messages follow Conventional Commits (`feat:`, `fix:`) for functional changes; script tweaks + are often plain imperative one-liners. +- `pyproject.toml` disables ruff `E501` (line length) and `E722` (bare `except`) — bare excepts are used + deliberately throughout and should not be "fixed" as drive-by cleanup. +- New Python code targets 3.9 (CI's floor). diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index c48718e..d325be1 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -9,7 +9,48 @@ cd imagebuilder virtualenv . -p /usr/bin/python3 source bin/activate pip install --upgrade pip -pip install --upgrade setuptools -python setup.py develop pip install -r requirements.txt +pip install -e . ``` + +`pip install -e .` replaces the old `python setup.py develop`, which setuptools +has deprecated. Package metadata and the console script live in +`pyproject.toml`; there is no `setup.py` any more. + +## Tests + +``` bash +pip install pytest +pytest +``` + +They use stand-in OpenStack clients and a fake HTTP layer, so nothing reaches a +cloud or the network and no credentials are needed. CI runs them on every push. + +## Python version + +`requirements.txt` pins for Python 3.9, the stock interpreter on EL9. Red Hat +maintains 3.9 for the full RHEL 9 life cycle, so it stays patched well past its +upstream end of life, but two pins are capped by it: keystoneauth1 and +openstacksdk. + +That floor is fine for OpenStack 2025.1, where 3.9 is the minimum supported +runtime. 2025.2 raised the minimum to 3.10, and while its constraints still +carry 3.9 fallbacks pinning exactly the versions used here, those fallbacks +will not last forever. + +Moving off 3.9 does not require EL10. Newer interpreters ship on EL9 as +parallel-installable AppStream packages, alongside the stock 3.9 and with their +own pip and site-packages: + +``` bash +dnf install -y python3.12 +python3.12 -m venv . +source bin/activate +pip install --upgrade pip +pip install -r requirements.txt +``` + +When switching, uncomment the 3.11+ block at the bottom of `requirements.txt` +and comment out the three capped pins above it. Nothing in the code needs +changing; it has been checked on interpreters up to 3.14. diff --git a/README.md b/README.md index 37f248a..2c17b8a 100644 --- a/README.md +++ b/README.md @@ -19,11 +19,16 @@ Pull requests are welcome! ### Requirements * Python 3 * [keystoneauth](https://github.com/openstack/keystoneauth) -* [python-novaclient](https://github.com/openstack/python-novaclient) -* [python-glanceclient](https://github.com/openstack/python-glanceclient) -* [Packer](https://packer.io) +* [openstacksdk](https://github.com/openstack/openstacksdk) +* [Packer](https://packer.io) 1.7 or newer * OpenSSL command line tool +Packer no longer ships with the OpenStack builder built in, so imagebuilder runs +`packer init` before every build to install the +[openstack plugin](https://github.com/hashicorp/packer-plugin-openstack). This +needs network access the first time; afterwards the plugin is cached under +`$HOME/.config/packer/plugins`. + ### Installation - Clone this repository `git clone https://github.com/norcams/imagebuilder` @@ -50,8 +55,11 @@ image of choice (i.e. centos, ubuntu), -p is the path to your provision script and -d downloads the image after it's been built. imagebuilder creates a temporary security group and keypair named -"imagebuilder-" which will be deleted after completion. Note that you for -now will have to delete these manually if the command is not allowed to finish. +"imagebuilder-" which will be deleted after completion. These are also +removed if the build fails or you interrupt it with Ctrl-C. A process killed +outright (SIGTERM, SIGKILL) cannot run its cleanup, so in that case you will +still have to delete the leftover "imagebuilder-*" security group and keypair +yourself. Run `imagebuilder -h` for a complete list of options. @@ -91,6 +99,12 @@ template_dir = /home/user/.imagebuilder download_dir = /tmp/images ``` +`template_dir` must contain `template.pkr.hcl` along with the `scripts/` +directory it refers to. A legacy JSON template named `template` is still picked +up if no `template.pkr.hcl` is present, but Packer cannot install plugins for it +— run `packer plugins install github.com/hashicorp/openstack` yourself, or +migrate the template with `packer hcl2_upgrade template`. + ### Provision scripts A provision script is (when using imagebuilder) simply a shell script that will be executed on the virtual machine Packer builds an image from. In it's simplest diff --git a/TODO.md b/TODO.md index dbc2810..a06d0e6 100644 --- a/TODO.md +++ b/TODO.md @@ -1,6 +1,10 @@ * Exception handling * Isn't very good at the moment - * Handle signals (clean up before dying) + * Handle SIGTERM (clean up before dying). Exceptions and Ctrl-C already + unwind through the try/finally in main(), but a SIGTERM kills the process + without running it, so those builds still leak a security group and + keypair. sweep_orphans() clears them up on a later run, but only once they + are a day old * Post-process images * Convert image from raw to qcow2 after download diff --git a/build-images.sh b/build-images.sh new file mode 100755 index 0000000..3ba4f1d --- /dev/null +++ b/build-images.sh @@ -0,0 +1,423 @@ +#!/usr/bin/bash +# +# Build NREC images sequentially, one after another, in place of a cron job +# per image. +# +# The colour palette and the XY/line/cnt helpers below are kept whole for +# future use, so unused variables and uncalled functions are expected here. +# SC2317 and SC2329 are the same complaint: shellcheck renamed it, and both +# codes are listed so this stays quiet on either version +# shellcheck disable=SC2034,SC2317,SC2329 + +# Continue past a failed build, but treat unset variables and broken pipes as +# errors. Not -e: one image failing must not stop the rest of the run +set -uo pipefail + +#--------------------------------------------------------------------+ +#Color picker, usage: printf ${BLD}${CUR}${RED}${BBLU}"Hello!)"${DEF}| +#-------------------------+--------------------------------+---------+ +# Text color | Background color | | +#-----------+-------------+--------------+-----------------+ | +# Base color|Lighter shade| Base color | Lighter shade | | +#-----------+-------------+--------------+-----------------+ | +BLK='\e[30m'; blk='\e[90m'; BBLK='\e[40m'; bblk='\e[100m' #| Black | +RED='\e[31m'; red='\e[91m'; BRED='\e[41m'; bred='\e[101m' #| Red | +GRN='\e[32m'; grn='\e[92m'; BGRN='\e[42m'; bgrn='\e[102m' #| Green | +YLW='\e[33m'; ylw='\e[93m'; BYLW='\e[43m'; bylw='\e[103m' #| Yellow | +BLU='\e[34m'; blu='\e[94m'; BBLU='\e[44m'; bblu='\e[104m' #| Blue | +MGN='\e[35m'; mgn='\e[95m'; BMGN='\e[45m'; bmgn='\e[105m' #| Magenta | +CYN='\e[36m'; cyn='\e[96m'; BCYN='\e[46m'; bcyn='\e[106m' #| Cyan | +WHT='\e[37m'; wht='\e[97m'; BWHT='\e[47m'; bwht='\e[107m' #| White | +#----------------------------------------------------------+---------+ +# Effects | +#--------------------------------------------------------------------+ +DEF='\e[0m' #Default color and effects | +BLD='\e[1m' #Bold\brighter | +DIM='\e[2m' #Dim\darker | +CUR='\e[3m' #Italic font | +UND='\e[4m' #Underline | +INV='\e[7m' #Inverted | +COF='\e[?25l' #Cursor Off | +CON='\e[?25h' #Cursor On | +#--------------------------------------------------------------------+ +# Text positioning, usage: XY 10 10 "Hello World!" | +# shellcheck disable=SC2059 +XY () { printf "\e[${2};${1}H${3}"; } # | +#--------------------------------------------------------------------+ +# Print line, usage: line - 10 | line -= 20 | line "Hello World!" 20 | +# shellcheck disable=SC2059 +line () { printf -v LINE "%$2s"; printf -- "${LINE// /$1}"; } # | +# Create sequence like {0..X} | +# shellcheck disable=SC2059,SC2086,SC2206 +cnt () { printf -v _N %$1s; _N=(${_N// / 1}); printf "${!_N[*]}"; } #| +#--------------------------------------------------------------------+ + +# Under cron stdout is a pipe, and the escape sequences above would end up +# verbatim in the log and in the mail. Blank the palette unless we are on a +# terminal +if [ ! -t 1 ]; then + for _colour in BLK blk BBLK bblk RED red BRED bred GRN grn BGRN bgrn \ + YLW ylw BYLW bylw BLU blu BBLU bblu MGN mgn BMGN bmgn \ + CYN cyn BCYN bcyn WHT wht BWHT bwht \ + DEF BLD DIM CUR UND INV COF CON; do + printf -v "$_colour" '%s' '' + done + unset _colour +fi + +# Set path +PATH=/usr/bin:/usr/sbin +export PATH + +# Variables. Overridable so the script can be exercised outside a builder node +BUILD_PATH=${BUILD_PATH:-/home/imagebuilder/build_scripts} +LOG_PATH=${LOG_PATH:-/var/log/imagebuilder} +TEMPLATE_ROOT=${TEMPLATE_ROOT:-/etc/imagebuilder} +LOCK_FILE=${LOCK_FILE:-${LOG_PATH}/build-images.lock} + +#--------------------------------------------------------------------- +# Image categories +# +# Everything that differs between kinds of image lives here. Adding a +# category means adding a line to each map below and a letter to the getopts +# string, rather than another array, another flag and another build loop. +#--------------------------------------------------------------------- + +# Build order +CATEGORIES=(std uib gpu win) + +# How a build script name maps to a category. std has no pattern: it is the +# fallback for anything the others do not claim +declare -A CATEGORY_PATTERN=( + [uib]='_uib_puppet$' + [gpu]='_nv_vgpu$' + [win]='^winsrv_.*_wrapper$' +) + +# The template directory imagebuilder should use +declare -A CATEGORY_TEMPLATE=( + [std]="${TEMPLATE_ROOT}/default" + [uib]="${TEMPLATE_ROOT}/uib_puppet" + [gpu]="${TEMPLATE_ROOT}/nv_vgpu" + [win]="${TEMPLATE_ROOT}/default" +) + +# What -l and the summary call them +declare -A CATEGORY_LABEL=( + [std]='Standard images' + [uib]='UiB images' + [gpu]='vGPU images' + [win]='Windows images' +) + +# The option that selects a whole category +declare -A OPTION_CATEGORY=( + [S]=std + [B]=uib + [G]=gpu + [W]=win +) + +# Windows builds are not wired up yet. Listing and selecting them works, so +# that turning this off is the only change needed later +WIN_PLACEHOLDER=1 + +# Every build script found, and which category it belongs to. One map rather +# than an array per category: bash has no arrays of arrays, and the nameref +# tricks that fake them are not worth the reading +declare -A IMAGE_CATEGORY=() +declare -a ALL_IMAGES=() + +# Images to build, and the categories selected wholesale with -S/-B/-G/-W +declare -a SELECTED=() +declare -A SELECT_ALL=() + +# Option variables +LIST=0 +DRYRUN=0 +TIMEOUT=0 +SWEEP=0 + +# Every -i given, kept so they can be checked before anything is built +requested_images=() + +# Outcomes, for the summary and the exit code +built_images=() +failed_images=() +skipped_images=() + +#--------------------------------------------------------------------- +# Functions +#--------------------------------------------------------------------- + +# Which category a build script name belongs to. Used for both discovery and +# -i, so the two cannot disagree about what a name means +categorise() { + local name=$1 cat + for cat in "${CATEGORIES[@]}"; do + [ -n "${CATEGORY_PATTERN[$cat]:-}" ] || continue + if [[ $name =~ ${CATEGORY_PATTERN[$cat]} ]]; then + printf '%s' "$cat" + return + fi + done + printf 'std' +} + +# Seconds as 1h02m03s, dropping the units that are zero +duration() { + local total=$1 + if [ "$total" -ge 3600 ]; then + printf '%dh%02dm%02ds' $((total / 3600)) $((total % 3600 / 60)) $((total % 60)) + elif [ "$total" -ge 60 ]; then + printf '%dm%02ds' $((total / 60)) $((total % 60)) + else + printf '%ds' "$total" + fi +} + +usage() { + echo "Usage: $0 [-h] [-S] [-B] [-G] [-W] [-t ] [-l] [-i ]..." + echo + echo 'Options:' + echo ' -S Build all standard images. Negates -i options' + echo ' -B Build all UiB images. Negates -i options' + echo ' -G Build all vgpu images. Negates -i options' + echo ' -W Build all Windows images. Negates -i options' + echo ' -i Specify image to build. Can be used multiple times' + echo ' -s Sweep: before each build, remove temporary security groups and' + echo ' keypairs left behind by runs that were killed. Note the lower case:' + echo ' -S selects the standard images, -s asks for the sweep' + echo ' -t Terminate build job if still running after this duration. See timeout(1)' + echo ' -l List all images and exit' + echo ' -n Dry-run: Only show what would be done' + echo ' -h Display help' + echo +} + +function build_image() { + local image=$1 + local exitval start elapsed + printf '%b>>>>>>>>>>>>>>>> BUILDING IMAGE >>%b %b%s %b[timeout: %s]%b\n' \ + "${BMGN}${BLD}${wht}" "$DEF" "${cyn}${BLD}" "$image" "${DEF}${DIM}" "$TIMEOUT" "$DEF" + if [ $DRYRUN -eq 0 ]; then + start=$SECONDS + timeout "$TIMEOUT" "${BUILD_PATH}/${image}" + exitval=$? + elapsed=$(duration $((SECONDS - start))) + if [ $exitval -eq 0 ]; then + printf '[%bSUCCESS%b] Build completed in %s: %b%s%b\n' \ + "${grn}${BLD}" "$DEF" "$elapsed" "$BLD" "$image" "$DEF" + built_images+=("$image") + elif [ $exitval -eq 124 ]; then + printf '[%bERROR%b] TIMEOUT: Build timed out after %s: %b%s%b\n' \ + "${RED}${BLD}" "$DEF" "$TIMEOUT" "$BLD" "$image" "$DEF" >&2 + jq -nc '{"result": "failed"}' >> "${LOG_PATH}/${image}-report.jsonl" + failed_images+=("$image") + else + printf '[%bERROR%b] Build failed after %s (exit %s): %b%s%b\n' \ + "${RED}${BLD}" "$DEF" "$elapsed" "$exitval" "$BLD" "$image" "$DEF" >&2 + jq -nc '{"result": "failed"}' >> "${LOG_PATH}/${image}-report.jsonl" + failed_images+=("$image") + fi + else + printf '%b(DRY-RUN)%b %btimeout %s %s/%s%b\n' \ + "$GRN" "$DEF" "$DIM" "$TIMEOUT" "$BUILD_PATH" "$image" "$DEF" + built_images+=("$image") + fi + echo +} + +#--------------------------------------------------------------------- +# Find images +#--------------------------------------------------------------------- + +if [ ! -d "$BUILD_PATH" ]; then + printf 'No build scripts directory: %s\n' "$BUILD_PATH" >&2 + exit 1 +fi + +# A glob rather than $(ls): a build script whose name contains a space used to +# be split into two images that do not exist +for script_path in "$BUILD_PATH"/*; do + [ -f "$script_path" ] || continue + script=${script_path##*/} + # A windows image installs two scripts: winsrv__wrapper, which is + # the entry point, and winsrv_, which the wrapper calls. The + # second is not an image of its own, and running it directly starts a + # build on the windows build host outside its schedule, so it is not + # something anything here should ever pick up + if [[ $script =~ ^winsrv_ ]] && [[ ! $script =~ _wrapper$ ]]; then + continue + fi + IMAGE_CATEGORY[$script]=$(categorise "$script") + ALL_IMAGES+=("$script") +done + +#--------------------------------------------------------------------- +# Options +#--------------------------------------------------------------------- + +# Display usage and exit with error if no options given +if [ $# -eq 0 ]; then + usage >&2 + exit 1 +fi + +while getopts "SBGWlnshi:t:" arg; do + case $arg in + S|B|G|W) + SELECT_ALL[${OPTION_CATEGORY[$arg]}]=1 + ;; + i) + requested_images+=("$OPTARG") + SELECTED+=("$OPTARG") + ;; + t) + # timeout(1) takes a number with an optional s/m/h/d suffix. Catch + # a bad one now rather than once per build + if ! [[ $OPTARG =~ ^[0-9]+([.][0-9]+)?[smhd]?$ ]]; then + printf 'Invalid timeout: %s. See timeout(1)\n' "$OPTARG" >&2 + exit 1 + fi + TIMEOUT=$OPTARG + ;; + s) + SWEEP=1 + ;; + l) + LIST=1 + ;; + n) + DRYRUN=1 + ;; + h) + usage + exit 0 + ;; + *) + # An unknown option is an error, so that a typo in a cron entry or + # a unit file does not look like a successful run + usage >&2 + exit 1 + ;; + esac +done + +# Display all images and exit +if [ $LIST -eq 1 ]; then + for cat in "${CATEGORIES[@]}"; do + printf '%s:\n' "${CATEGORY_LABEL[$cat]}" + for image in "${ALL_IMAGES[@]}"; do + if [ "${IMAGE_CATEGORY[$image]}" = "$cat" ]; then + printf ' %s\n' "$image" + fi + done + done + exit 0 +fi + +# Check every -i before building anything. A typo used to run a path that does +# not exist, report it as a failed build, and leave a report file behind for an +# image nobody has +unknown_images=() +for image in "${requested_images[@]}"; do + if [ -z "${IMAGE_CATEGORY[$image]:-}" ]; then + unknown_images+=("$image") + fi +done +if [ ${#unknown_images[@]} -gt 0 ]; then + printf 'No such image: %s\n' "${unknown_images[@]}" >&2 + printf "Run '%s -l' for the images that exist\n" "$0" >&2 + exit 1 +fi + +# -S, -B, -G and -W replace any -i for that category +for cat in "${CATEGORIES[@]}"; do + [ -n "${SELECT_ALL[$cat]:-}" ] || continue + keep=() + for image in "${SELECTED[@]}"; do + if [ "${IMAGE_CATEGORY[$image]}" != "$cat" ]; then + keep+=("$image") + fi + done + for image in "${ALL_IMAGES[@]}"; do + if [ "${IMAGE_CATEGORY[$image]}" = "$cat" ]; then + keep+=("$image") + fi + done + SELECTED=("${keep[@]}") +done + +#--------------------------------------------------------------------- +# MAIN +#--------------------------------------------------------------------- + +# One run at a time. These builds share a project, and two runs would compete +# for the temporary security groups and keypairs. Taken after -l and -h so +# that looking at the list never blocks +if ! exec 9>"$LOCK_FILE"; then + printf 'Cannot open lock file: %s\n' "$LOCK_FILE" >&2 + exit 1 +fi +if ! flock -n 9; then + printf 'Another run is already in progress (%s)\n' "$LOCK_FILE" >&2 + exit 1 +fi + +export IMAGEBUILDER_REPORT=true + +# The generated build scripts take no arguments of their own, so a sweep is +# asked for through the environment. Exported either way, so that a stray +# IB_SWEEP in whoever's environment cannot turn it on behind our back +if [ $SWEEP -eq 1 ]; then + export IB_SWEEP=true +else + export IB_SWEEP=false +fi + +run_start=$SECONDS + +for cat in "${CATEGORIES[@]}"; do + batch=() + for image in "${SELECTED[@]}"; do + if [ "${IMAGE_CATEGORY[$image]}" = "$cat" ]; then + batch+=("$image") + fi + done + [ ${#batch[@]} -gt 0 ] || continue + + export IB_TEMPLATE_DIR="${CATEGORY_TEMPLATE[$cat]}" + for img in "${batch[@]}"; do + if [ "$cat" = win ] && [ $WIN_PLACEHOLDER -eq 1 ]; then + # Do NOT build these if cron failed. + export PLACEHOLDER=true + echo "Do NOT build $img! Poke Tor if cron job failed" + skipped_images+=("$img") + else + build_image "$img" + fi + done +done + +#--------------------------------------------------------------------- +# Summary +#--------------------------------------------------------------------- + +printf '%b%s%b %d built, %d failed, %d skipped in %s\n' \ + "$BLD" 'Summary:' "$DEF" \ + "${#built_images[@]}" "${#failed_images[@]}" "${#skipped_images[@]}" \ + "$(duration $((SECONDS - run_start)))" + +if [ ${#skipped_images[@]} -gt 0 ]; then + printf ' %bskipped:%b %s\n' "$YLW" "$DEF" "${skipped_images[*]}" +fi + +# Report what failed and exit non-zero, so that cron, a systemd unit or a +# monitoring check can tell a bad run from a good one +if [ ${#failed_images[@]} -gt 0 ]; then + printf ' %bfailed:%b %s\n' "${RED}${BLD}" "$DEF" "${failed_images[*]}" >&2 + exit 1 +fi + +exit 0 diff --git a/example/README.md b/example/README.md index 2d36a14..377b5d9 100644 --- a/example/README.md +++ b/example/README.md @@ -2,6 +2,8 @@ This is tested on Alma Linux 8.8. Beware of a duplicate packer executable in `/usr/sbin`, the correct one is in `/usr/bin`. Modify the order of $PATH to ensure the correct one is used. +Requires Packer 1.7 or newer. The OpenStack builder is installed automatically by `packer init`, which imagebuilder runs for you. + ### Install ``` bash diff --git a/example/imagebuilder b/example/imagebuilder index 60aa80a..0766a13 100755 --- a/example/imagebuilder +++ b/example/imagebuilder @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 from image_builder.imagebuilder import main diff --git a/example/requirements.txt b/example/requirements.txt index d7a68bf..e6bcb0f 100644 --- a/example/requirements.txt +++ b/example/requirements.txt @@ -1,6 +1,4 @@ -e git+https://github.com/norcams/imagebuilder.git#egg=imagebuilder keystoneauth1 -python-novaclient -python-glanceclient -python-neutronclient +openstacksdk python-openstackclient diff --git a/example/template b/example/template deleted file mode 100644 index cb3e03e..0000000 --- a/example/template +++ /dev/null @@ -1,28 +0,0 @@ -{ - "builders": [{ - "type": "openstack", - "image_name": "{{user `image_name`}}", - "source_image": "{{user `source_image`}}", - "flavor": "{{user `flavor`}}", - "networks": "{{user `network`}}", - "security_groups": "{{user `security_group`}}", - "ssh_username": "{{user `ssh_username`}}", - "ssh_keypair_name": "{{user `ssh_keypair_name`}}", - "ssh_private_key_file": "{{user `ssh_key_path`}}", - "ssh_ip_version": "4", - "ssh_pty": "true", - "availability_zone": "{{user `availability_zone`}}" - }], - "post-processors": [{ - "type": "manifest", - "output": "{{user `manifest_path`}}" - }], - "provisioners": [{ - "scripts": [ - "../scripts/upgrade.sh", - "{{user `provision_script`}}", - "../scripts/cleanup.sh" - ], - "type": "shell" - }] -} diff --git a/example/template.pkr.hcl b/example/template.pkr.hcl new file mode 100644 index 0000000..7812d01 --- /dev/null +++ b/example/template.pkr.hcl @@ -0,0 +1,83 @@ +packer { + required_plugins { + openstack = { + version = "~> 1.1" + source = "github.com/hashicorp/openstack" + } + } +} + +variable "image_name" { + type = string +} + +variable "source_image" { + type = string +} + +variable "flavor" { + type = string +} + +variable "network" { + type = string +} + +variable "security_group" { + type = string +} + +variable "ssh_username" { + type = string +} + +variable "ssh_keypair_name" { + type = string +} + +variable "ssh_key_path" { + type = string +} + +variable "availability_zone" { + type = string +} + +variable "manifest_path" { + type = string +} + +variable "provision_script" { + type = string + default = "/bin/true" +} + +source "openstack" "image" { + image_name = var.image_name + source_image = var.source_image + flavor = var.flavor + networks = [var.network] + security_groups = [var.security_group] + ssh_username = var.ssh_username + ssh_keypair_name = var.ssh_keypair_name + ssh_private_key_file = var.ssh_key_path + ssh_ip_version = "4" + ssh_pty = true + availability_zone = var.availability_zone +} + +build { + sources = ["source.openstack.image"] + + provisioner "shell" { + scripts = [ + "${path.root}/../scripts/upgrade.sh", + var.provision_script, + "${path.root}/../scripts/cleanup.sh", + ] + } + + post-processor "manifest" { + output = var.manifest_path + } +} diff --git a/image_builder/__main__.py b/image_builder/__main__.py index a85f7d4..9c5f01d 100644 --- a/image_builder/__main__.py +++ b/image_builder/__main__.py @@ -1,2 +1,4 @@ from .imagebuilder import main -main() + +if __name__ == '__main__': + main() diff --git a/image_builder/bootstrap.py b/image_builder/bootstrap.py index 9a2b26f..fb2e02d 100644 --- a/image_builder/bootstrap.py +++ b/image_builder/bootstrap.py @@ -1,24 +1,66 @@ import logging import os import urllib.request -from glanceclient import Client -from .helpers import Helpers as helpers -class BootstrapFunctions(object): +from openstack.connection import Connection + +from . import helpers + +log = logging.getLogger(__name__) + +# Blocking socket operations give up after this long. This is a per-read +# timeout, not a limit on the whole transfer, so a large image is fine as long +# as bytes keep arriving +SOCKET_TIMEOUT = 60 + +# Anything smaller than this is an error page, not a cloud image +MIN_IMAGE_BYTES = 1000 + +class BootstrapFunctions: def __init__(self, session, - region, - avail_zone): + region): self.session = session - self.avail_zone = avail_zone self.tmp_dir = helpers.make_tmp_dir() - self.glance = Client("2", session=session, region_name=region) + self.conn = Connection(session=session, region_name=region) + + @staticmethod + def find_expected_checksum(checksum_text, file_name): + """Finds the checksum recorded for file_name in a checksum file + + Handles the two layouts in common use: the coreutils one, ' + ', where the name may carry a '*' prefix, and the BSD one, + 'SHA256 () = '. Returns the hash lowercased, or None when + the file has no entry for this name. + """ + for line in checksum_text.splitlines(): + line = line.strip() + if not line or line.startswith('#'): + continue + if '(' in line and ')' in line and '=' in line: + name = line[line.index('(') + 1:line.rindex(')')] + value = line.rsplit('=', 1)[1] + else: + fields = line.split() + if len(fields) < 2: + continue + value, name = fields[0], fields[-1].lstrip('*') + if os.path.basename(name) == file_name: + return value.strip().lower() + return None def download_and_check(self, url, checksum_url=None, checksum_dig='sha256'): user_agent = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/35.0.1916.47 Safari/537.36' CHUNK = 16 * 1024 file_name = url.split("/")[-1] file_path = os.path.join(self.tmp_dir, file_name) + + # Checked up front so a typo in -t fails now rather than after the + # download has finished + if checksum_url and not helpers.valid_digest(checksum_dig): + log.error("Unknown checksum digest: %s", checksum_dig) + return None + req = urllib.request.Request( url, data=None, @@ -26,7 +68,7 @@ def download_and_check(self, url, checksum_url=None, checksum_dig='sha256'): 'User-Agent': user_agent } ) - response = urllib.request.urlopen(req) + response = urllib.request.urlopen(req, timeout=SOCKET_TIMEOUT) with open(file_path, "wb") as f: while True: chunk = response.read(CHUNK) @@ -34,49 +76,75 @@ def download_and_check(self, url, checksum_url=None, checksum_dig='sha256'): break f.write(chunk) response.close() - if int(response.headers["content-length"]) < 1000: - logging.info("File is too small: %s" % url) + + # The size of what actually landed, not what the server claimed in a + # header it is not obliged to send + size = os.path.getsize(file_path) + if size < MIN_IMAGE_BYTES: + log.error("File is too small (%d bytes): %s", size, url) os.remove(file_path) return None - if checksum_url: - req = urllib.request.Request( - checksum_url, - data=None, - headers = { - 'User-Agent': user_agent - } - ) - logging.info("Verifying checksum of %s..." % file_path) - response = urllib.request.urlopen(req) - checksum_all = str(response.read()) - response.close() - logging.debug(checksum_all) - logging.debug("Checksum type is %s" % checksum_dig) - checksum_file = helpers.checksum_file(file_path, checksum_dig) - if checksum_file in checksum_all: - logging.info("Checksum ok: %s" % checksum_file) + + if not checksum_url: + return file_path + + req = urllib.request.Request( + checksum_url, + data=None, + headers = { + 'User-Agent': user_agent + } + ) + log.info("Verifying checksum of %s...", file_path) + response = urllib.request.urlopen(req, timeout=SOCKET_TIMEOUT) + checksum_text = response.read().decode('utf-8', errors='replace') + response.close() + log.debug(checksum_text) + log.debug("Checksum type is %s", checksum_dig) + + actual = helpers.checksum_file(file_path, checksum_dig) + expected = self.find_expected_checksum(checksum_text, file_name) + + if expected is None: + # No entry naming this file. Fall back to the old behaviour of + # looking for the hash anywhere in the file, which is weaker but + # keeps working with checksum files that name things differently + log.warning("No entry for %s in %s, falling back to matching the " + "hash anywhere in the file", file_name, checksum_url) + if actual in checksum_text.lower(): + log.info("Checksum ok: %s", actual) return file_path - else: - logging.info("Checksum not ok: %s" % checksum_file) - return None - else: + log.error("Checksum not found: %s", actual) + return None + + if actual == expected: + log.info("Checksum ok: %s", actual) return file_path + log.error("Checksum mismatch for %s: expected %s, got %s", + file_name, expected, actual) + return None + def create_glance_image(self, image_file, name, disk_format, min_disk, min_ram, properties): - image = self.glance.images.create(name=name, - visibility="private", - disk_format=disk_format, - min_disk=min_disk, - min_ram=min_ram, - container_format="bare", - **properties) - logging.info("Created image %s" % name) - logging.debug(image) try: - self.glance.images.upload(image.id, open(image_file, "rb")) - logging.info("Successfully uploaded %s" % image_file) - return image.id - except BaseException as error: - logging.debug(error) + # allow_duplicates keeps this a create: left at its default, + # create_image returns an existing same-named image instead, and + # this command's whole contract is to report a new image id. + # disable_vendor_agent would otherwise add properties of its own + image = self.conn.image.create_image(name=name, + filename=image_file, + disk_format=disk_format, + container_format="bare", + visibility="private", + min_disk=min_disk, + min_ram=min_ram, + allow_duplicates=True, + disable_vendor_agent=False, + **properties) + except Exception as error: + log.error("Failed to upload %s: %s", image_file, error) return None + log.info("Successfully uploaded %s as image %s", image_file, name) + log.debug(image) + return image.id diff --git a/image_builder/build.py b/image_builder/build.py index fedb642..f55f2c6 100644 --- a/image_builder/build.py +++ b/image_builder/build.py @@ -1,18 +1,32 @@ +import datetime import json import logging import os +import re import subprocess import time import uuid -from novaclient import client as novaclient -from neutronclient.v2_0 import client as neutronclient -from glanceclient import Client as glanceclient -from .helpers import Helpers as helpers -class BuildFunctions(object): +from openstack.connection import Connection + +from . import helpers + +log = logging.getLogger(__name__) + +# The temporary security group and keypair are named imagebuilder-. +# Only names of exactly this shape are ever removed by the sweep +TEMP_NAME = re.compile(r'^imagebuilder-[0-9a-f]{32}$') + +# Nothing younger than this is swept. Builder nodes share one project, so the +# age is what stops a sweep here from deleting resources out from under a +# build running somewhere else +SWEEP_MAX_AGE_HOURS = 24 + +class BuildFunctions: def __init__(self, session, region, + *, image_name, avail_zone, flavor, @@ -31,145 +45,332 @@ def __init__(self, self.template_dir = template_dir self.download_dir = download_dir self.tmp_dir = helpers.make_tmp_dir() - self.nova = novaclient.Client("2", session=session, region_name=region) - self.neutron = neutronclient.Client(session=session, region_name=region) - self.glance = glanceclient('2', session=session, region_name=region) + # Only the connection is built here. Reaching for a proxy such as + # conn.network authenticates and discovers endpoints, so that is left + # to the call sites to keep this constructor free of network traffic + self.conn = Connection(session=session, region_name=region) def cleanup(self, secgroup_id, keypair_id): - """Cleans up the mess we've made""" - logging.info('Removing temporary security group...') - self.neutron.delete_security_group(secgroup_id) - logging.info('Removing temporary keypair...') - self.nova.keypairs.delete(key=keypair_id) + """Cleans up the mess we've made + + Runs from a finally block, so it has to cope with a build that only got + half way and must not raise on its way out: an exception here would + mask whatever actually went wrong. + """ + if secgroup_id: + log.info('Removing temporary security group...') + try: + self.conn.network.delete_security_group(secgroup_id) + except Exception as error: + log.warning("Failed to remove security group %s: %s", secgroup_id, error) + if keypair_id: + log.info('Removing temporary keypair...') + try: + self.conn.compute.delete_keypair(keypair_id) + except Exception as error: + log.warning("Failed to remove keypair %s: %s", keypair_id, error) + + @staticmethod + def resource_age_hours(created_at): + """Hours since an OpenStack timestamp, or None if it cannot be read + + Neutron and nova do not agree on the format, and python 3.9 cannot + parse a trailing Z, so this is more forgiving than fromisoformat. + """ + if not created_at: + return None + text = str(created_at).strip() + if text.endswith('Z'): + text = text[:-1] + '+00:00' + try: + stamp = datetime.datetime.fromisoformat(text) + except (TypeError, ValueError): + return None + if stamp.tzinfo is None: + stamp = stamp.replace(tzinfo=datetime.timezone.utc) + now = datetime.datetime.now(datetime.timezone.utc) + return (now - stamp).total_seconds() / 3600 + + def sweep_orphans(self, max_age_hours=SWEEP_MAX_AGE_HOURS): + """Removes temporary resources left behind by runs that were killed + + A build that dies outright, from the OOM killer or a SIGKILL, cannot + run its own cleanup, so its security group and keypair stay in the + project forever. Only names shaped exactly like imagebuilder- + are considered, and only once they are older than max_age_hours. + + Images are deliberately not swept. They are named after the image + being built, so a leftover cannot be told apart from a good one, and + deleting the wrong one would destroy a published image. The download + directory is left alone too: something else tidies that. + """ + removed = self._sweep_security_groups(max_age_hours) + removed += self._sweep_keypairs(max_age_hours) + if removed: + log.warning("Removed %d orphaned resource(s) from earlier runs", removed) + return removed + + def _sweep_security_groups(self, max_age_hours): + try: + groups = list(self.conn.network.security_groups()) + except Exception as error: + log.warning("Could not list security groups to sweep: %s", error) + return 0 + removed = 0 + for group in groups: + if not TEMP_NAME.match(group.name or ''): + continue + age = self.resource_age_hours(group.created_at) + if age is None: + log.info("Leaving security group %s alone: no usable timestamp", + group.name) + continue + if age < max_age_hours: + continue + log.warning("Removing orphaned security group %s, %.0f hours old", + group.name, age) + try: + self.conn.network.delete_security_group(group.id) + removed += 1 + except Exception as error: + log.warning("Could not remove security group %s: %s", + group.name, error) + return removed + + def _sweep_keypairs(self, max_age_hours): + try: + keypairs = list(self.conn.compute.keypairs()) + except Exception as error: + log.warning("Could not list keypairs to sweep: %s", error) + return 0 + removed = 0 + for keypair in keypairs: + if not TEMP_NAME.match(keypair.name or ''): + continue + created = keypair.created_at + if not created: + # Nova does not always report created_at when listing, so ask + # for the one keypair rather than guess at its age + try: + created = self.conn.compute.get_keypair(keypair.name).created_at + except Exception as error: + log.info("Could not read the age of keypair %s: %s", + keypair.name, error) + age = self.resource_age_hours(created) + if age is None: + log.info("Leaving keypair %s alone: no usable timestamp", + keypair.name) + continue + if age < max_age_hours: + continue + log.warning("Removing orphaned keypair %s, %.0f hours old", + keypair.name, age) + try: + self.conn.compute.delete_keypair(keypair.name) + removed += 1 + except Exception as error: + log.warning("Could not remove keypair %s: %s", keypair.name, error) + return removed def create_keypairs(self): """Creates a temporary keypair""" - keyname = "imagebuilder-" + str(uuid.uuid4().hex) - cmd = "/usr/bin/ssh-keygen -b 521 -t ecdsa -N '' -f " + os.path.join(self.tmp_dir, keyname) - out = subprocess.call(cmd, shell=True) # generate temporary ssh key + keyname = f"imagebuilder-{uuid.uuid4().hex}" + keypath = os.path.join(self.tmp_dir, keyname) + # No shell: there is nothing here a shell is needed for, and letting + # PATH find ssh-keygen beats hardcoding where it lives + cmd = ['ssh-keygen', '-b', '521', '-t', 'ecdsa', '-N', '', '-f', keypath] + log.debug(cmd) + out = subprocess.run(cmd, check=False).returncode # generate temporary ssh key if out: # something went wrong with the key generation - return '', 0 + log.error("Failed to generate SSH key, ssh-keygen exited %s", out) + return None, None # read public key string and store into Openstack - with open(os.path.join(self.tmp_dir, keyname) + ".pub", "r") as pubfile: - pubkeystring = pubfile.read().replace('\n', '') - keypair = self.nova.keypairs.create(name=keyname, public_key=pubkeystring) + with open(keypath + ".pub", encoding="utf-8") as pubfile: + pubkeystring = pubfile.read().replace('\n', '') + keypair = self.conn.compute.create_keypair(name=keyname, + public_key=pubkeystring) return keyname, keypair.id def create_security_group(self): """Creates a temporary security group""" - secgroup_name = "imagebuilder-" + str(uuid.uuid4().hex) - # pylint: disable=line-too-long - secgroup = self.neutron.create_security_group(body={ - 'security_group': - { - 'name': secgroup_name, - 'description': 'Temporary security group for image building' - } - }) - secgroup_id = secgroup['security_group']['id'] - logging.info('Creating rule allowing SSH traffic...') - self.neutron.create_security_group_rule(body={ - 'security_group_rule': - { - 'security_group_id': secgroup_id, - 'direction': 'ingress', - 'protocol': 'tcp', - 'port_range_min': 22, - 'port_range_max': 22, - 'ethertype': 'IPv4' - } - }) - self.neutron.create_security_group_rule(body={ - 'security_group_rule': - { - 'security_group_id': secgroup_id, - 'direction': 'ingress', - 'protocol': 'tcp', - 'port_range_min': 22, - 'port_range_max': 22, - 'ethertype': 'IPv6' - } - }) - return secgroup_name, secgroup_id + secgroup_name = f"imagebuilder-{uuid.uuid4().hex}" + secgroup = self.conn.network.create_security_group( + name=secgroup_name, + description='Temporary security group for image building') + log.info('Creating rule allowing SSH traffic...') + for ethertype in ('IPv4', 'IPv6'): + self.conn.network.create_security_group_rule( + security_group_id=secgroup.id, + direction='ingress', + protocol='tcp', + port_range_min=22, + port_range_max=22, + ethertype=ethertype) + return secgroup_name, secgroup.id def delete_image(self, image_id): - logging.info('Removing image %s' % image_id) - if(image_id is not None): - try: - logging.info('Removing image %s' % image_id) - self.glance.images.delete(image_id) - return True - except: - logging.info('Removing image failed.') - return False + if image_id is None: + log.warning('No image to remove') + return False + log.info("Removing image %s", image_id) + try: + self.conn.image.delete_image(image_id) + except Exception as error: + log.error("Removing image %s failed: %s", image_id, error) + return False + return True def download_image(self, artifact_id): - """Downloads image from Glance""" - logging.info('Downloading image...') + """Downloads image from Glance + + Straight through the API on the session we already hold. Shelling out + to the glance CLI meant a second, independent authentication from the + OS_* environment, and that CLI is deprecated upstream. + + stream=True is load bearing. Without it openstacksdk asks requests for + the whole response body up front, so the entire image is held in + memory before a byte of it is written, and a large image takes the + process out via the OOM killer. + """ timestr = time.strftime("%Y%m%d") - filename = self.image_name + '-' + timestr + '.qcow2' - cmd = ['glance', - 'image-download', - '--file', os.path.join(self.download_dir, filename), - artifact_id] - process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - with process.stdout: - helpers.log_subprocess_output(process.stdout) - exitcode = process.wait() - # Not so pretty but will do for now - if exitcode == 0: - logging.info('Download successful, deleting from Glance...') - self.delete_image(artifact_id) - return exitcode + filename = f'{self.image_name}-{timestr}.qcow2' + target = os.path.join(self.download_dir, filename) + log.info("Downloading image to %s...", target) + try: + with open(target, 'wb') as image_file: + self.conn.image.download_image(artifact_id, + output=image_file, + stream=True) + except Exception as error: + log.error("Failed to download image %s: %s", artifact_id, error) + if os.path.exists(target): + os.remove(target) # don't leave a half-written image behind + return 1 + log.info('Download successful, deleting from Glance...') + self.delete_image(artifact_id) + return 0 def find_network_id(self, name): - networks = self.neutron.list_networks(name=name) - if networks['networks']: - network_id = networks['networks'][0]['id'] - logging.info("Found network %s with id %s" % (name, network_id)) + # networks() rather than find_network(), which raises on a duplicate + # name where this has always just taken the first match + network = next(self.conn.network.networks(name=name), None) + if network: + network_id = network.id + log.info("Found network %s with id %s", name, network_id) else: network_id = False - logging.info("Cannot find network %s" % name) + log.error("Cannot find network %s", name) return network_id + def find_template(self): + """Finds the Packer template in template_dir + + The HCL2 template is preferred. A legacy JSON template (a file simply + named 'template') is still accepted so existing template_dir setups + keep working, but it cannot declare required_plugins. + """ + for name in ('template.pkr.hcl', 'template'): + template_path = os.path.join(self.template_dir, name) + if os.path.isfile(template_path): + if name == 'template': + log.warning("Using legacy JSON template %s", template_path) + log.warning("Packer no longer bundles the openstack builder. " + "Install it manually with 'packer plugins install " + "github.com/hashicorp/openstack', or migrate to " + "template.pkr.hcl") + return template_path + log.error("No Packer template found in %s", self.template_dir) + return None + def parse_manifest(self): - """Parses the manifest file generated by packer""" - with open(os.path.join(self.tmp_dir, 'packer-manifest.json')) as data_file: - manifest = json.load(data_file) - artifact_id = manifest['builds'][0]['artifact_id'] - return artifact_id + """Parses the manifest file generated by packer + + Packer exiting successfully does not guarantee the manifest + post-processor left a usable file behind, so report that rather than + raising and taking the build down with a traceback. + """ + manifest_path = os.path.join(self.tmp_dir, 'packer-manifest.json') + try: + with open(manifest_path, encoding="utf-8") as data_file: + manifest = json.load(data_file) + return manifest['builds'][0]['artifact_id'] + except (OSError, ValueError, KeyError, IndexError) as error: + log.error("Failed to read the image id from %s: %s", manifest_path, error) + return None - def run_packer(self, secgroup_name, key_name, network_id): + def run_packer(self, template_path, secgroup_name, key_name, network_id): """Executes Packer command""" - image_name_var = 'image_name=' + self.image_name - avail_zone_var = 'availability_zone=' + self.avail_zone - secgroup_var = 'security_group=' + secgroup_name - sshuser_var = 'ssh_username=' + self.ssh_user - keyname_var = 'ssh_keypair_name=' + key_name # key as it is named in Openstack - keypath_var = 'ssh_key_path=' + os.path.join(self.tmp_dir, key_name) # the "real" private key - flavor_var = 'flavor=' + self.flavor - network_var = 'network=' + network_id - source_image_var = 'source_image=' + self.source_image - provision_script_var = 'provision_script=' + self.provision_script - manifest_path_var = 'manifest_path=' + os.path.join(self.tmp_dir, 'packer-manifest.json') + image_name_var = f'image_name={self.image_name}' + avail_zone_var = f'availability_zone={self.avail_zone}' + secgroup_var = f'security_group={secgroup_name}' + sshuser_var = f'ssh_username={self.ssh_user}' + keyname_var = f'ssh_keypair_name={key_name}' # key as it is named in Openstack + keypath_var = f'ssh_key_path={os.path.join(self.tmp_dir, key_name)}' # the "real" private key + flavor_var = f'flavor={self.flavor}' + network_var = f'network={network_id}' + source_image_var = f'source_image={self.source_image}' + provision_script_var = f'provision_script={self.provision_script}' + manifest_path_var = f"manifest_path={os.path.join(self.tmp_dir, 'packer-manifest.json')}" cmd = ['packer', 'build', '-color=false', - '--var', image_name_var, - '--var', sshuser_var, - '--var', secgroup_var, - '--var', flavor_var, - '--var', network_var, - '--var', source_image_var, - '--var', keyname_var, - '--var', keypath_var, - '--var', avail_zone_var, - '--var', provision_script_var, - '--var', manifest_path_var, - os.path.join(self.template_dir, 'template')] - logging.debug(cmd) - process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) - with process.stdout: + '-var', image_name_var, + '-var', sshuser_var, + '-var', secgroup_var, + '-var', flavor_var, + '-var', network_var, + '-var', source_image_var, + '-var', keyname_var, + '-var', keypath_var, + '-var', avail_zone_var, + '-var', provision_script_var, + '-var', manifest_path_var, + template_path] + log.debug(cmd) + with subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) as process: + helpers.log_subprocess_output(process.stdout) + return process.wait() + + def openstack_plugin_installed(self): + """Whether Packer can already see the openstack builder + + 'packer plugins installed' exits 0 whether or not anything is there, so + the answer is in the output rather than the status. + """ + try: + installed = subprocess.run(['packer', 'plugins', 'installed'], + capture_output=True, text=True, + check=False) + except OSError as error: + log.error("Could not run packer: %s", error) + return False + log.debug(installed.stdout) + return 'openstack' in installed.stdout + + def run_packer_init(self, template_path): + """Installs the Packer plugins required by the template + + Packer does not bundle builder plugins anymore, so the openstack + builder has to be installed before the build. Only HCL2 templates can + declare required_plugins, so for a legacy JSON template there is + nothing to install from: all this can do is check that someone already + installed it by hand, and say so now rather than let packer fail with + 'the builder openstack is unknown' once the temporary security group + and keypair already exist. + """ + if not template_path.endswith('.pkr.hcl'): + if self.openstack_plugin_installed(): + return 0 + log.error("The openstack builder is not installed, and %s is a " + "legacy JSON template so it cannot declare it. Either " + "run 'packer plugins install " + "github.com/hashicorp/openstack', or migrate to " + "template.pkr.hcl", template_path) + return 1 + cmd = ['packer', 'init', template_path] + log.debug(cmd) + with subprocess.Popen(cmd, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) as process: helpers.log_subprocess_output(process.stdout) - exitcode = process.wait() - return exitcode + return process.wait() diff --git a/image_builder/config.py b/image_builder/config.py index 052127a..54523f2 100644 --- a/image_builder/config.py +++ b/image_builder/config.py @@ -1,7 +1,8 @@ -import os import configparser +import os -class Config(object): + +class Config: def __init__(self, configfile_path=None): if configfile_path is None: self.configfile_path = [os.curdir, @@ -14,11 +15,7 @@ def __init__(self, configfile_path=None): self.config_file_list.append(os.path.join(path, "config")) self.config = self.__read_config() - def show_config(self): - # Not implemented yet - pass - def __read_config(self): config = configparser.ConfigParser() - config.read(self.config_file_list) + config.read(self.config_file_list, encoding='utf-8') return config diff --git a/image_builder/helpers.py b/image_builder/helpers.py index db06506..8e22d34 100644 --- a/image_builder/helpers.py +++ b/image_builder/helpers.py @@ -1,42 +1,62 @@ +"""Small shared helpers. + +Module-level functions rather than static methods on a class: a module is +already the namespace a class was being used for here. +""" import hashlib import logging import shutil import tempfile -class Helpers(object): - - @staticmethod - def clean_tmp_files(tmp_dir): - logging.info('Removing temporary directory with content...') - shutil.rmtree(tmp_dir) - - @staticmethod - def make_tmp_dir(): - logging.info('Creating a directory for temporary files...') - tmp_dir = tempfile.mkdtemp(prefix='imagebuilder-') - return tmp_dir - - @staticmethod - def log_subprocess_output(pipe): - for line in iter(pipe.readline, b''): - logging.info('%r', line) - - @staticmethod - def checksum_file(file_path, digest='sha256', chunk_size=65536): - """Read the file in small pieces, so as to prevent failures to read - particularly large files. Also ensures memory usage is kept to a - minimum. Testing shows default is a pretty good size.""" - assert isinstance(chunk_size, int) and chunk_size > 0 - if digest == 'sha512': - digest = hashlib.sha512() - if digest == 'sha256': - digest = hashlib.sha256() - elif digest == 'md5': - digest = hashlib.md5() - #pylint: disable=invalid-name - with open(file_path, 'rb') as f: - for block in iter(lambda: f.read(chunk_size), b''): - digest.update(block) - checksum = digest.hexdigest() - logging.debug("hexdigest of %s is %s" % (file_path, checksum)) - return checksum +log = logging.getLogger(__name__) + + +def clean_tmp_files(tmp_dir): + # Also called from a finally block, so failing to remove a scratch + # directory must not be what the user ends up seeing + log.info('Removing temporary directory with content...') + shutil.rmtree(tmp_dir, ignore_errors=True) + + +def make_tmp_dir(): + log.info('Creating a directory for temporary files...') + return tempfile.mkdtemp(prefix='imagebuilder-') + + +def log_subprocess_output(pipe): + # Decoded, not repr'd: this is the build log the user watches, and every + # line of it used to arrive looking like b'...\n' + for line in iter(pipe.readline, b''): + log.info(line.decode('utf-8', errors='replace').rstrip()) + + +def valid_digest(digest): + """Whether hashlib knows this digest name + + Used to reject a bad -t before spending a download on it. + """ + try: + hashlib.new(digest.lower()) + except (ValueError, TypeError): + return False + return True + + +def checksum_file(file_path, digest='sha256', chunk_size=65536): + """Read the file in small pieces, so as to prevent failures to read + particularly large files. Also ensures memory usage is kept to a + minimum. Testing shows default is a pretty good size.""" + if not isinstance(chunk_size, int) or chunk_size <= 0: + # Raised rather than asserted: assertions disappear under python -O + raise ValueError(f"chunk_size must be a positive integer, got {chunk_size!r}") + # hashlib.new() takes any algorithm it supports and raises a clear + # ValueError naming a bad one. The hand-written if/elif ladder this + # replaces silently left unknown names as a string and then died with + # AttributeError on the first update() + hasher = hashlib.new(digest.lower()) + with open(file_path, 'rb') as handle: + for block in iter(lambda: handle.read(chunk_size), b''): + hasher.update(block) + checksum = hasher.hexdigest() + log.debug("hexdigest of %s is %s", file_path, checksum) + return checksum diff --git a/image_builder/imagebuilder.py b/image_builder/imagebuilder.py index 12df5f2..ff67ff5 100644 --- a/image_builder/imagebuilder.py +++ b/image_builder/imagebuilder.py @@ -1,201 +1,247 @@ -#!/usr/bin/python3 - +import configparser import logging import os import sys -from keystoneauth1.identity import v3 + from keystoneauth1 import session -from .parsecommands import Commands -from .build import BuildFunctions +from keystoneauth1.identity import v3 + +from . import helpers from .bootstrap import BootstrapFunctions +from .build import BuildFunctions from .config import Config -from .helpers import Helpers as helpers +from .parsecommands import Commands + +log = logging.getLogger(__name__) + +# Only what auth() and the region lookup actually consume. Asking for more than +# that turns a perfectly good openrc file into a failed login +REQUIRED_ENV = { + 'username': 'OS_USERNAME', + 'project_name': 'OS_PROJECT_NAME', + 'password': 'OS_PASSWORD', + 'auth_url': 'OS_AUTH_URL', + 'user_domain_name': 'OS_USER_DOMAIN_NAME', + 'project_domain_name': 'OS_PROJECT_DOMAIN_NAME', + 'region_name': 'OS_REGION_NAME', +} -class ImageBuilder(object): - @staticmethod - def auth(rc): - auth = v3.Password(auth_url=rc['auth_url'], + +def auth(rc): + """Builds an authenticated keystone session from the settings dict""" + password = v3.Password(auth_url=rc['auth_url'], project_name=rc['project_name'], username=rc['username'], password=rc['password'], user_domain_name=rc['user_domain_name'], project_domain_name=rc['project_domain_name']) - if rc['cacert'] is not None: - sess = session.Session(auth, - verify=rc['cacert']) - else: - sess = session.Session(auth) - return sess - - @staticmethod - def get_openstack_rc(): - env_var = {} - env_var['username'] = os.environ['OS_USERNAME'] - env_var['project_name'] = os.environ['OS_PROJECT_NAME'] - env_var['cacert'] = os.environ['OS_CACERT'] if "OS_CACERT" in os.environ else None - env_var['password'] = os.environ['OS_PASSWORD'] - env_var['auth_url'] = os.environ['OS_AUTH_URL'] - env_var['api_version'] = os.environ['OS_IDENTITY_API_VERSION'] - env_var['user_domain_name'] = os.environ['OS_USER_DOMAIN_NAME'] - env_var['project_domain_name'] = os.environ['OS_PROJECT_DOMAIN_NAME'] - env_var['region_name'] = os.environ['OS_REGION_NAME'] - env_var['no_cache'] = os.environ['OS_NO_CACHE'] - return env_var + if rc['cacert'] is not None: + return session.Session(password, verify=rc['cacert']) + return session.Session(password) + +def get_openstack_rc(): + """Reads the credentials from the environment + + OS_CACERT is optional. Raises KeyError naming every variable that is + missing rather than just the first one found. + """ + missing = [name for name in REQUIRED_ENV.values() if name not in os.environ] + if missing: + raise KeyError(', '.join(missing)) + env_var = {key: os.environ[name] for key, name in REQUIRED_ENV.items()} + env_var['cacert'] = os.environ.get('OS_CACERT') + return env_var + +def configure_logging(args): + """Sets up logging once for whichever subcommand was chosen + + Default is WARNING rather than silence: progress stays hidden without -v, + but a failure now says so instead of the command simply exiting non-zero. + --debug wins when both flags are given, which is the way round people + expect. + """ + if args.debug: + logging.basicConfig(format="%(asctime)s %(levelname)s %(name)s %(message)s", + level=logging.DEBUG) + elif args.verbose: + logging.basicConfig(format="%(message)s", level=logging.INFO) + else: + logging.basicConfig(format="%(message)s", level=logging.WARNING) -def main(): - commands = Commands() - imagebuilder = ImageBuilder() +def sweep_requested_by_environment(): + """Whether IB_SWEEP asks for a sweep + + build-images.sh runs the generated build scripts, which take no arguments + of their own, so the environment is how a sweep is asked for from there. + """ + return os.environ.get('IB_SWEEP', '').strip().lower() in ('1', 'true', 'yes', 'on') + + +def resolve_directory(config, env_var, option): + """Finds a configured directory, with the environment variable winning""" + if env_var in os.environ: + return os.environ[env_var] try: - rc = imagebuilder.get_openstack_rc() - except KeyError: - print("""Failed to read environment variables. -Please run: - source -and try again.""") + return config.get('main', option) + except (configparser.NoSectionError, configparser.NoOptionError): + print(f"Failed to read {option} from config", file=sys.stderr) sys.exit(1) - ib_session = imagebuilder.auth(rc) - region = rc['region_name'] - config = Config().config +def image_properties(args): + """Hardware properties to set on the uploaded image""" + properties = {'hw_rng_model': 'virtio'} + if not args.no_scsi_mode: + properties['hw_disk_bus'] = 'scsi' + properties['hw_scsi_model'] = 'virtio-scsi' + if args.efi: + properties['hw_firmware_type'] = 'uefi' + properties['hw_machine_type'] = 'q35' + return properties + + +def run_build(args, ib_session, region, template_dir, download_dir): + """Builds an image with Packer, returning the exit code""" + build = BuildFunctions(ib_session, + region, + image_name=args.name, + avail_zone=args.availability_zone, + flavor=args.flavor, + source_image=args.source_image, + ssh_user=args.ssh_username, + provision_script=args.provision_script, + template_dir=template_dir, + download_dir=download_dir) + + # Everything that can fail without creating anything in OpenStack is done + # first, so a bad template or network name doesn't leave a security group + # and a keypair behind + template_path = build.find_template() + if not template_path: + helpers.clean_tmp_files(build.tmp_dir) + return 1 - if "IB_TEMPLATE_DIR" in os.environ: - template_dir = os.environ['IB_TEMPLATE_DIR'] - else: - try: - template_dir = config.get('main', 'template_dir') - except: - print("Failed to read template_dir from config") - sys.exit(1) - - if "IB_DOWNLOAD_DIR" in os.environ: - download_dir = os.environ['IB_DOWNLOAD_DIR'] - else: - try: - download_dir = config.get('main', 'download_dir') - except: - print("Failed to read download_dir from config") - sys.exit(1) + network_id = build.find_network_id(args.network_name) + if not network_id: + helpers.clean_tmp_files(build.tmp_dir) + return 1 - if commands.build_args: - image_name = commands.build_args.name - avail_zone = commands.build_args.availability_zone - flavor = commands.build_args.flavor - source_image = commands.build_args.source_image - sshuser = commands.build_args.ssh_username - provision_script = commands.build_args.provision_script - network_name = commands.build_args.network_name - - if commands.build_args.verbose: - logging.basicConfig(format="%(message)s", level=logging.INFO) - elif commands.build_args.debug: - logging.basicConfig(level=logging.DEBUG) - - build = BuildFunctions(ib_session, - region, - image_name, - avail_zone, - flavor, - source_image, - sshuser, - provision_script, - template_dir, - download_dir) - - logging.info('Creating Packer security group...') - secgroup_name, secgroup_id = build.create_security_group() - - logging.info('Creating Packer keypair...') - key_name, keypair_id = build.create_keypairs() + log.info('Installing Packer plugins...') + if build.run_packer_init(template_path) != 0: + log.error('Failed to install Packer plugins') + helpers.clean_tmp_files(build.tmp_dir) + return 1 - network_id = build.find_network_id(network_name) + # Asked for rather than assumed: a sweep deletes resources this run did not + # create, so it happens when something wants it to, not on every build + if args.sweep or sweep_requested_by_environment(): + build.sweep_orphans() - if not network_id: - sys.exit(1) + log.info('Creating Packer security group...') + secgroup_name, secgroup_id = build.create_security_group() - logging.info('Running Packer...') - exitcode = build.run_packer(secgroup_name, key_name, network_id) + keypair_id = None + exitcode = 1 + + # From here on something exists in OpenStack, so the rest runs under a + # finally. An interrupt during the build, or any exception on the way, has + # to still remove the security group and the keypair rather than leave them + # behind in the project + try: + log.info('Creating Packer keypair...') + key_name, keypair_id = build.create_keypairs() + if key_name is None: + return 1 + + log.info('Running Packer...') + exitcode = build.run_packer(template_path, secgroup_name, key_name, + network_id) if exitcode == 0: artifact_id = build.parse_manifest() - logging.info("Successfully created image id %s" % artifact_id) - if commands.build_args.download: - exitcode = build.download_image(artifact_id) + if artifact_id is None: + exitcode = 1 else: - exitcode = 0 + log.info("Successfully created image id %s", artifact_id) + if args.download: + exitcode = build.download_image(artifact_id) else: - logging.info('Build failed') + log.error('Build failed') exitcode = 1 - logging.info('Cleaning up...') - build.cleanup(secgroup_id, keypair_id) - if commands.build_args.purge_source: - if build.delete_image(source_image): - logging.info('Successfully deleted source image') + if args.purge_source: + if build.delete_image(args.source_image): + log.info('Successfully deleted source image') else: - logging.info('Failed to delete source image') + log.error('Failed to delete source image') exitcode = 1 + finally: + log.info('Cleaning up...') + build.cleanup(secgroup_id, keypair_id) helpers.clean_tmp_files(build.tmp_dir) - sys.exit(exitcode) - - if commands.bootstrap_args: - image_name = commands.bootstrap_args.name - avail_zone = commands.bootstrap_args.availability_zone - url = commands.bootstrap_args.url - checksum_url = commands.bootstrap_args.checksum_url - checksum_digest = commands.bootstrap_args.checksum_digest - disk_format = commands.bootstrap_args.disk_format - min_disk = int(commands.bootstrap_args.min_disk) - min_ram = int(commands.bootstrap_args.min_ram) - - if commands.bootstrap_args.verbose: - logging.basicConfig(format="%(message)s", level=logging.INFO) - elif commands.bootstrap_args.debug: - logging.basicConfig(level=logging.DEBUG) - - properties = {} - - if not commands.bootstrap_args.no_scsi_mode: - properties['hw_disk_bus'] = 'scsi' - properties['hw_scsi_model'] = 'virtio-scsi' - - if commands.bootstrap_args.efi: - properties['hw_firmware_type'] = 'uefi' - properties['hw_machine_type'] = 'q35' - - properties['hw_rng_model'] = 'virtio' - - bootstrap = BootstrapFunctions(ib_session, - region, - avail_zone) - logging.info('Downloading image...') - image_file = bootstrap.download_and_check(url, checksum_url, checksum_digest) - - if image_file: - logging.info('Uploading image to Glance...') - image_id = bootstrap.create_glance_image(image_file, - image_name, - disk_format, - min_disk, - min_ram, - properties) - else: - logging.info('Downloading failed.') - logging.info('Cleaning up...') - helpers.clean_tmp_files(bootstrap.tmp_dir) - sys.exit(1) + return exitcode - if image_id: - sys.stdout.write(image_id) - else: - logging.info('Uploading failed.') - logging.info('Cleaning up...') - helpers.clean_tmp_files(bootstrap.tmp_dir) - sys.exit(1) - logging.info('Cleaning up...') +def run_bootstrap(args, ib_session, region): + """Uploads an upstream cloud image to Glance, returning the exit code + + On success the new image id is the only thing on stdout, so the result can + be piped straight into a build. + """ + bootstrap = BootstrapFunctions(ib_session, region) + try: + log.info('Downloading image...') + image_file = bootstrap.download_and_check(args.url, + args.checksum_url, + args.checksum_digest) + if not image_file: + log.error('Downloading failed.') + return 1 + + log.info('Uploading image to Glance...') + image_id = bootstrap.create_glance_image(image_file, + args.name, + args.disk_format, + args.min_disk, + args.min_ram, + image_properties(args)) + if not image_id: + log.error('Uploading failed.') + return 1 + + sys.stdout.write(image_id) + return 0 + finally: + log.info('Cleaning up...') helpers.clean_tmp_files(bootstrap.tmp_dir) - sys.exit(0) + + +def main(): + commands = Commands() + args = commands.build_args or commands.bootstrap_args + configure_logging(args) + + try: + rc = get_openstack_rc() + except KeyError as missing: + print(f"""Missing environment variable(s): {missing.args[0]} +Please run: + source +and try again.""", file=sys.stderr) + sys.exit(1) + + ib_session = auth(rc) + region = rc['region_name'] + config = Config().config + + if commands.build_args: + # Only the build needs these, so only the build has to have them + # configured + template_dir = resolve_directory(config, 'IB_TEMPLATE_DIR', 'template_dir') + download_dir = resolve_directory(config, 'IB_DOWNLOAD_DIR', 'download_dir') + sys.exit(run_build(args, ib_session, region, template_dir, download_dir)) + + sys.exit(run_bootstrap(args, ib_session, region)) # vim: set ft=python3 diff --git a/image_builder/parsecommands.py b/image_builder/parsecommands.py index 0677992..8e4731c 100644 --- a/image_builder/parsecommands.py +++ b/image_builder/parsecommands.py @@ -1,124 +1,116 @@ import argparse -import sys -class Commands(object): - def __init__(self): - parser = argparse.ArgumentParser( - description='Usage', - usage='''imagebuilder [] +class Commands: - build Builds an image - bootstrap Downloads a cloud-ready image from a URL and uploads to Glance -''') - parser.add_argument('command', help='Subcommand to run') - args = parser.parse_args(sys.argv[1:2]) - if not hasattr(self, args.command): - print('Unrecognized command') - parser.print_help() - exit(1) - self.build_args = False - self.bootstrap_args = False - getattr(self, args.command)() + def __init__(self, argv=None): + parser = argparse.ArgumentParser( + prog='imagebuilder', + description='Build fully provisioned images in NREC') + # Subparsers rather than dispatching on getattr(self, argv[1]): that + # made every attribute on this object reachable from the command line, + # and left the list of commands as prose in the usage string with + # nothing keeping it honest + subcommands = parser.add_subparsers(dest='command', required=True, + metavar='') - def build(self): - parser = argparse.ArgumentParser(description='Build an image') - parser.add_argument('-a', '--availability-zone', - help='Availability zone, i.e. bgo-default-1, osl-default-1', - default=False, - required=True) - parser.add_argument('-d', '--download', - help='Download image after build', - action='store_true', - default=False) - parser.add_argument('-f', '--flavor', - help='Flavor to use when building (default: m1.small)', - default='m1.small', - required=False) - parser.add_argument('-n', '--name', - help='Name of the image', - default=False, - required=True) - parser.add_argument('-N', '--network-name', - help='Name of the network we want to use', - default='Dualstack', - required=False) - parser.add_argument('-p', '--provision-script', - help='Path to your provision script', - default='/bin/true') - parser.add_argument('-s', '--source-image', - help='Name or id of the source image we build from', - default=False, - required=True) - parser.add_argument('-u', '--ssh-username', - help='SSH username as set up by cloud-init (usually named after distro or OS, i.e. centos, ubuntu)', - default=False, - required=True), - parser.add_argument('-v', '--verbose', - help='Be verbose (default is no output)', - action='store_true', - default=False) - parser.add_argument('-x', '--purge-source', - help='Purge source image you are building from', - action='store_true', - default=False) - parser.add_argument('--debug', - help='Debug mode', - action='store_true', - default=False) - self.build_args = parser.parse_args(sys.argv[2:]) - return self.build_args + build = subcommands.add_parser( + 'build', + help='Builds an image', + description='Build an image') + build.add_argument('-a', '--availability-zone', + help='Availability zone, i.e. bgo-default-1, osl-default-1', + required=True) + build.add_argument('-d', '--download', + help='Download image after build', + action='store_true', + default=False) + build.add_argument('-f', '--flavor', + help='Flavor to use when building (default: m1.small)', + default='m1.small') + build.add_argument('-n', '--name', + help='Name of the image', + required=True) + build.add_argument('-N', '--network-name', + help='Name of the network we want to use', + default='Dualstack') + build.add_argument('-p', '--provision-script', + help='Path to your provision script', + default='/bin/true') + build.add_argument('-s', '--source-image', + help='Name or id of the source image we build from', + required=True) + build.add_argument('-u', '--ssh-username', + help='SSH username as set up by cloud-init (usually named after distro or OS, i.e. centos, ubuntu)', + required=True) + build.add_argument('-v', '--verbose', + help='Be verbose (default is no output)', + action='store_true', + default=False) + build.add_argument('--sweep', + help='Before building, remove temporary security ' + 'groups and keypairs left behind by runs that ' + 'were killed. Can also be asked for with ' + 'IB_SWEEP=true in the environment', + action='store_true', + default=False) + build.add_argument('-x', '--purge-source', + help='Purge source image you are building from', + action='store_true', + default=False) + build.add_argument('--debug', + help='Debug mode', + action='store_true', + default=False) - def bootstrap(self): - parser = argparse.ArgumentParser( + bootstrap = subcommands.add_parser( + 'bootstrap', + help='Downloads a cloud-ready image from a URL and uploads to Glance', description='Downloads a cloud-ready image from a URL and uploads to glance') - parser.add_argument('-a', '--availability-zone', - help='Availability zone, i.e. bgo-default-1, osl-default-1', - default=False, - required=True) - parser.add_argument('-u', '--url', - help='URL to upstream image', - default=False, - required=True) - parser.add_argument('-c', '--checksum-url', - help='URL to checksum file', - default=None, - required=False) - parser.add_argument('-t', '--checksum-digest', - help='Checksum digest (defaults to sha256)', - default='sha256', - required=False), - parser.add_argument('-n', '--name', - help='Name of the image', - default=False, - required=True), - parser.add_argument('-r', '--min-ram', - help='Minimum amount of ram in MB', - default=False, - required=True) - parser.add_argument('-d', '--min-disk', - help='Minimum amount of disk in GB', - default=False, - required=True) - parser.add_argument('-f', '--disk-format', - help='Format of the disk', - default=False, - required=True) - parser.add_argument('-s', '--no-scsi-mode', - help='Do not create image with scsi disk properties', - action='store_true', - default=False) - parser.add_argument('-e', '--efi', - help='Boot image with uefi boot firmware', - action='store_true', - default=False) - parser.add_argument('-v', '--verbose', - help='Be verbose', - action='store_true', - default=False) - parser.add_argument('--debug', - help='Debug mode', - action='store_true', - default=False) - self.bootstrap_args = parser.parse_args(sys.argv[2:]) - return self.bootstrap_args + bootstrap.add_argument('-a', '--availability-zone', + help='Availability zone, i.e. bgo-default-1, osl-default-1', + required=True) + bootstrap.add_argument('-u', '--url', + help='URL to upstream image', + required=True) + bootstrap.add_argument('-c', '--checksum-url', + help='URL to checksum file', + default=None) + bootstrap.add_argument('-t', '--checksum-digest', + help='Checksum digest (defaults to sha256)', + default='sha256') + bootstrap.add_argument('-n', '--name', + help='Name of the image', + required=True) + bootstrap.add_argument('-r', '--min-ram', + help='Minimum amount of ram in MB', + type=int, + required=True) + bootstrap.add_argument('-d', '--min-disk', + help='Minimum amount of disk in GB', + type=int, + required=True) + bootstrap.add_argument('-f', '--disk-format', + help='Format of the disk', + required=True) + bootstrap.add_argument('-s', '--no-scsi-mode', + help='Do not create image with scsi disk properties', + action='store_true', + default=False) + bootstrap.add_argument('-e', '--efi', + help='Boot image with uefi boot firmware', + action='store_true', + default=False) + bootstrap.add_argument('-v', '--verbose', + help='Be verbose', + action='store_true', + default=False) + bootstrap.add_argument('--debug', + help='Debug mode', + action='store_true', + default=False) + + args = parser.parse_args(argv) + self.command = args.command + self.build_args = args if args.command == 'build' else None + self.bootstrap_args = args if args.command == 'bootstrap' else None diff --git a/imagebuilder b/imagebuilder index b7fe288..0766a13 100755 --- a/imagebuilder +++ b/imagebuilder @@ -1,4 +1,4 @@ -#!/usr/bin/python3 +#!/usr/bin/env python3 from image_builder.imagebuilder import main diff --git a/pyproject.toml b/pyproject.toml index 3732536..277f63d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,4 +1,63 @@ +[build-system] +requires = ["setuptools>=64"] +build-backend = "setuptools.build_meta" + +[project] +name = "imagebuilder" +description = "CLI for Packer-driven image building in UH IaaS" +readme = "README.md" +requires-python = ">=3.9" +license = { file = "LICENSE" } +authors = [{ name = "Anders Vaage", email = "anders@uib.no" }] +dynamic = ["version"] +# The two libraries actually imported. Exact pins live in requirements.txt, +# which is where the Python 3.9 ceiling is worked out +dependencies = [ + "keystoneauth1", + "openstacksdk", +] + +[project.urls] +Homepage = "https://github.com/norcams/imagebuilder" + +[project.scripts] +imagebuilder = "image_builder.imagebuilder:main" + +[tool.setuptools] +packages = ["image_builder"] + +[tool.setuptools.dynamic] +version = { attr = "image_builder.__version__" } + [tool.ruff] +target-version = "py39" + +# Selected explicitly so the rule set is this repository's decision rather than +# whatever ruff currently defaults to. Relying on the default is what broke CI +# when 0.16.0 widened it. +lint.select = [ + "E", # pycodestyle errors + "F", # pyflakes + "W", # pycodestyle warnings + "I", # import sorting + "UP", # pyupgrade + "B", # bugbear + "SIM", # simplify + "RET", # return consistency + "C4", # comprehensions + "PIE", # misc lints + "LOG", # logging misuse + "S", # bandit +] + # Never enforce `E501` (line length violations). -lint.ignore = ["E501", "E722"] +lint.ignore = [ + "E501", + "S310", # urlopen: the URLs come from the operator running the command + "S603", # subprocess without shell: the argv is built here, not user input + "S607", # packer and ssh-keygen are deliberately resolved through PATH, + # so a virtualenv or a chosen packer build can be picked up +] +[tool.ruff.lint.per-file-ignores] +"tests/*" = ["S101"] # asserts are the point of a test diff --git a/requirements.txt b/requirements.txt index 87939c9..3bc1888 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,19 @@ -keystoneauth1==4.3.1 -python-novaclient==17.4.0 -python-glanceclient==3.3.0 -python-neutronclient==7.3.0 -cryptography==46.0.7 -pbr==5.11.1 +# Pinned for Python 3.9, the stock interpreter on EL9. Two packages are capped +# by that floor: keystoneauth1 5.12 requires 3.10 and 5.15 requires 3.11, +# openstacksdk 4.6 requires 3.10 and 4.14 requires 3.11. +# +# This is the combination OpenStack itself sanctions for 3.9: the 2025.2 +# upper-constraints pin openstacksdk 4.5.0 for python_version=='3.9', and +# everything here meets or exceeds the 2025.1 constraints. +keystoneauth1==5.11.1 +openstacksdk==4.5.0 +cryptography==50.0.1 +pbr==7.0.3 + +# On Python 3.11 or newer, comment out the two capped pins above and uncomment +# these. cryptography and pbr are already current and stay as they are. See +# DEVELOPMENT.md for installing a newer interpreter on EL9 without waiting for +# EL10. +# +# keystoneauth1==5.17.0 +# openstacksdk==4.20.0 diff --git a/setup.py b/setup.py deleted file mode 100644 index c9ea5ac..0000000 --- a/setup.py +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/python3 - -from setuptools import setup -import image_builder - - -setup( - name=image_builder.__productname__, - description=image_builder.__description__, - version=image_builder.__version__, - author=image_builder.__author__, - author_email=image_builder.__author_email__, - url=image_builder.__url__, - packages=['image_builder'], - entry_points={ - 'console_scripts': [ - 'imagebuilder = image_builder.imagebuilder:main' - ] - } - ) diff --git a/template b/template deleted file mode 100644 index 3e5c510..0000000 --- a/template +++ /dev/null @@ -1,35 +0,0 @@ -{ - "builders": [{ - "type": "openstack", - "image_name": "{{user `image_name`}}", - "source_image": "{{user `source_image`}}", - "flavor": "{{user `flavor`}}", - "networks": "{{user `network`}}", - "security_groups": "{{user `security_group`}}", - "ssh_username": "{{user `ssh_username`}}", - "ssh_keypair_name": "{{user `ssh_keypair_name`}}", - "ssh_private_key_file": "{{user `ssh_key_path`}}", - "ssh_pty": "true", - "availability_zone": "{{user `availability_zone`}}" - }], - "post-processors": [{ - "type": "manifest", - "output": "{{user `manifest_path`}}" - }], - "provisioners": [{ - "scripts": [ - "scripts/uio-rhel.sh", - "scripts/fstrim.sh", - "scripts/qemu_guest_agent.sh", - "scripts/upgrade.sh", - "scripts/enable_ipv6.sh", - "scripts/autopatch.sh", - "scripts/report.sh", - "scripts/sshd_hardening.sh", - "{{user `provision_script`}}", - "scripts/cleanup.sh", - "scripts/uio-cleanup.sh" - ], - "type": "shell" - }] -} diff --git a/template.pkr.hcl b/template.pkr.hcl new file mode 100644 index 0000000..4b9fd9d --- /dev/null +++ b/template.pkr.hcl @@ -0,0 +1,101 @@ +packer { + required_plugins { + openstack = { + version = "~> 1.1" + source = "github.com/hashicorp/openstack" + } + } +} + +variable "image_name" { + type = string + description = "Name of the resulting image in Glance" +} + +variable "source_image" { + type = string + description = "ID of the image to build from" +} + +variable "flavor" { + type = string + description = "Flavor of the build instance" +} + +variable "network" { + type = string + description = "ID of the network to attach the build instance to" +} + +variable "security_group" { + type = string + description = "Name of the temporary security group" +} + +variable "ssh_username" { + type = string + description = "User created by cloud-init in the source image" +} + +variable "ssh_keypair_name" { + type = string + description = "Name of the temporary keypair as stored in OpenStack" +} + +variable "ssh_key_path" { + type = string + description = "Path to the private key matching ssh_keypair_name" +} + +variable "availability_zone" { + type = string + description = "Availability zone to build in" +} + +variable "manifest_path" { + type = string + description = "Where the manifest post-processor writes the build result" +} + +variable "provision_script" { + type = string + description = "User supplied provision script" + default = "/bin/true" +} + +source "openstack" "image" { + image_name = var.image_name + source_image = var.source_image + flavor = var.flavor + networks = [var.network] + security_groups = [var.security_group] + ssh_username = var.ssh_username + ssh_keypair_name = var.ssh_keypair_name + ssh_private_key_file = var.ssh_key_path + ssh_pty = true + availability_zone = var.availability_zone +} + +build { + sources = ["source.openstack.image"] + + provisioner "shell" { + scripts = [ + "${path.root}/scripts/uio-rhel.sh", + "${path.root}/scripts/fstrim.sh", + "${path.root}/scripts/qemu_guest_agent.sh", + "${path.root}/scripts/upgrade.sh", + "${path.root}/scripts/enable_ipv6.sh", + "${path.root}/scripts/autopatch.sh", + "${path.root}/scripts/report.sh", + "${path.root}/scripts/sshd_hardening.sh", + var.provision_script, + "${path.root}/scripts/cleanup.sh", + "${path.root}/scripts/uio-cleanup.sh", + ] + } + + post-processor "manifest" { + output = var.manifest_path + } +} diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..f107f0f --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,158 @@ +"""Shared fakes. + +Nothing here touches a network or a cloud: the OpenStack calls are replaced by +recorders that assert on what the code asked for. +""" +import os +import pathlib +import sys + +import pytest + +REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent +sys.path.insert(0, str(REPO_ROOT)) + + +@pytest.fixture +def repo_root(): + return REPO_ROOT + + +class FakeNetwork: + def __init__(self, log): + self.log = log + self.networks_result = [] + self.groups = [] + + def create_security_group(self, **kw): + self.log.append(('sg_created', kw)) + return type('SG', (), {'id': 'sg-uuid'})() + + def create_security_group_rule(self, **kw): + self.log.append(('rule_created', kw)) + + def delete_security_group(self, sgid): + self.log.append(('sg_deleted', sgid)) + + def networks(self, **kw): + self.log.append(('networks_listed', kw)) + return iter(self.networks_result) + + def security_groups(self, **kw): + return iter(self.groups) + + +class FakeCompute: + def __init__(self, log): + self.log = log + self.pairs = [] + + def create_keypair(self, **kw): + self.log.append(('kp_created', kw)) + return type('KP', (), {'id': kw.get('name')})() + + def delete_keypair(self, keypair): + self.log.append(('kp_deleted', keypair)) + + def keypairs(self, **kw): + return iter(self.pairs) + + def get_keypair(self, name): + for pair in self.pairs: + if pair.name == name: + return pair + raise RuntimeError(f'no such keypair: {name}') + + +class FakeImage: + def __init__(self, log, fail=False): + self.log = log + self.fail = fail + self.download_kwargs = None + + def create_image(self, **kw): + self.log.append(('image_created', kw)) + if self.fail: + raise RuntimeError('glance is unhappy') + return type('IMG', (), {'id': 'new-image-uuid'})() + + def download_image(self, image, output=None, **kw): + self.log.append(('image_downloaded', image)) + self.download_kwargs = kw + if self.fail: + raise RuntimeError('glance is unhappy') + output.write(b'qcow2 data') + + def delete_image(self, image, **kw): + self.log.append(('image_deleted', image)) + + +class FakeConn: + """Stands in for an openstacksdk Connection.""" + + def __init__(self, fail_image=False): + self.log = [] + self.network = FakeNetwork(self.log) + self.compute = FakeCompute(self.log) + self.image = FakeImage(self.log, fail=fail_image) + + def kinds(self): + return [entry[0] for entry in self.log] + + +@pytest.fixture +def conn(): + return FakeConn() + + +@pytest.fixture +def build(conn, tmp_path): + """A BuildFunctions with the constructor bypassed, so no cloud is needed.""" + from image_builder.build import BuildFunctions + + b = BuildFunctions.__new__(BuildFunctions) + b.conn = conn + b.tmp_dir = str(tmp_path / 'tmp') + os.makedirs(b.tmp_dir, exist_ok=True) + b.download_dir = str(tmp_path / 'downloads') + os.makedirs(b.download_dir, exist_ok=True) + b.template_dir = str(tmp_path / 'templates') + os.makedirs(b.template_dir, exist_ok=True) + b.image_name = 'test-image' + b.avail_zone = 'bgo-default-1' + b.flavor = 'm1.small' + b.source_image = 'src-uuid' + b.ssh_user = 'almalinux' + b.provision_script = '/bin/true' + return b + + +@pytest.fixture +def bootstrap(tmp_path): + from image_builder.bootstrap import BootstrapFunctions + + b = BootstrapFunctions.__new__(BootstrapFunctions) + b.conn = FakeConn() + b.tmp_dir = str(tmp_path / 'boot') + os.makedirs(b.tmp_dir, exist_ok=True) + return b + + +@pytest.fixture +def clean_env(monkeypatch): + """An environment with every OS_* variable removed.""" + for key in list(os.environ): + if key.startswith(('OS_', 'IB_')): + monkeypatch.delenv(key, raising=False) + return monkeypatch + + +FULL_OPENRC = { + 'OS_USERNAME': 'user', + 'OS_PROJECT_NAME': 'project', + 'OS_PASSWORD': 'secret', + 'OS_AUTH_URL': 'https://api.example.com:5000/v3', + 'OS_USER_DOMAIN_NAME': 'dom', + 'OS_PROJECT_DOMAIN_NAME': 'dom', + 'OS_REGION_NAME': 'bgo', +} diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py new file mode 100644 index 0000000..d537ad2 --- /dev/null +++ b/tests/test_bootstrap.py @@ -0,0 +1,187 @@ +"""Downloading, verifying and uploading an upstream cloud image.""" +import hashlib +import io +import os +import urllib.request + +import pytest + +from image_builder import bootstrap as bootstrap_module +from image_builder import helpers +from image_builder.bootstrap import BootstrapFunctions + +HASH = 'a' * 64 +IMAGE = b'x' * 5000 + + +# --------------------------------------------------------------- digests + +@pytest.mark.parametrize('name', ['sha256', 'sha512', 'md5', 'sha1', 'SHA256', + 'sha384', 'blake2b']) +def test_every_hashlib_digest_works(tmp_path, name): + """The old if/elif ladder died with AttributeError on anything else.""" + sample = tmp_path / 'sample' + sample.write_bytes(b'hello') + assert len(helpers.checksum_file(str(sample), name)) > 16 + + +def test_unknown_digest_raises_a_clear_error(tmp_path): + sample = tmp_path / 'sample' + sample.write_bytes(b'hello') + with pytest.raises(ValueError): + helpers.checksum_file(str(sample), 'notahash') + + +def test_valid_digest(): + assert helpers.valid_digest('sha1') + assert not helpers.valid_digest('notahash') + + +# ------------------------------------------------- checksum file parsing + +find = BootstrapFunctions.find_expected_checksum + + +@pytest.mark.parametrize('text,name,expected', [ + pytest.param(f'{HASH} img.qcow2\n', 'img.qcow2', HASH, id='coreutils'), + pytest.param(f'{HASH} *img.img\n', 'img.img', HASH, id='binary-star'), + pytest.param(f'SHA256 (img.qcow2) = {HASH}\n', 'img.qcow2', HASH, id='bsd'), + pytest.param(f'{HASH.upper()} img.qcow2\n', 'img.qcow2', HASH, id='uppercase'), + pytest.param(f'# c\n\n{HASH} img.qcow2\n', 'img.qcow2', HASH, id='comments'), + pytest.param(f'{HASH} ./images/img.qcow2\n', 'img.qcow2', HASH, id='path'), + pytest.param(f'{HASH} other.qcow2\n', 'img.qcow2', None, id='name-absent'), +]) +def test_checksum_layouts(text, name, expected): + assert find(text, name) == expected + + +def test_the_right_line_is_picked_among_several(): + text = f"{'a' * 64} a.qcow2\n{'b' * 64} b.qcow2\n" + assert find(text, 'b.qcow2') == 'b' * 64 + + +# --------------------------------------------------- download and verify + +class FakeResponse: + def __init__(self, payload): + self.stream = io.BytesIO(payload) + self.headers = {} # deliberately no content-length + + def read(self, n=None): + return self.stream.read(n) if n else self.stream.read() + + def close(self): + pass + + +@pytest.fixture +def fake_http(monkeypatch): + """Serves image bytes for the image URL and text for the checksum URL.""" + state = {'image': IMAGE, 'checksum': b'', 'timeouts': []} + + def fake_urlopen(req, timeout=None): + state['timeouts'].append(timeout) + url = req.full_url if hasattr(req, 'full_url') else str(req) + payload = state['checksum'] if 'sum' in url.lower() else state['image'] + return FakeResponse(payload) + + monkeypatch.setattr(urllib.request, 'urlopen', fake_urlopen) + return state + + +IMAGE_URL = 'https://example.com/img.qcow2' +SUM_URL = 'https://example.com/sha256sum.txt' + + +def test_a_real_image_is_accepted_without_a_content_length_header(bootstrap, fake_http): + assert bootstrap.download_and_check(IMAGE_URL) is not None + + +def test_an_error_page_is_rejected_on_its_actual_size(bootstrap, fake_http): + fake_http['image'] = b'404 not found' + assert bootstrap.download_and_check(IMAGE_URL) is None + + +def test_requests_carry_a_timeout(bootstrap, fake_http): + bootstrap.download_and_check(IMAGE_URL) + assert fake_http['timeouts'] + assert all(t == bootstrap_module.SOCKET_TIMEOUT for t in fake_http['timeouts']) + + +def test_matching_checksum_is_accepted(bootstrap, fake_http): + digest = hashlib.sha256(IMAGE).hexdigest() + fake_http['checksum'] = f'{digest} img.qcow2\n'.encode() + assert bootstrap.download_and_check(IMAGE_URL, SUM_URL) is not None + + +def test_uppercase_checksum_file_is_accepted(bootstrap, fake_http): + digest = hashlib.sha256(IMAGE).hexdigest() + fake_http['checksum'] = f'{digest.upper()} img.qcow2\n'.encode() + assert bootstrap.download_and_check(IMAGE_URL, SUM_URL) is not None + + +def test_mismatched_checksum_is_rejected(bootstrap, fake_http): + fake_http['checksum'] = f"{'f' * 64} img.qcow2\n".encode() + assert bootstrap.download_and_check(IMAGE_URL, SUM_URL) is None + + +def test_hash_listed_under_another_name_still_passes_via_fallback(bootstrap, fake_http): + """Kept so vendors whose checksum files name things differently still work.""" + digest = hashlib.sha256(IMAGE).hexdigest() + fake_http['checksum'] = f'{digest} somethingelse.qcow2\n'.encode() + assert bootstrap.download_and_check(IMAGE_URL, SUM_URL) is not None + + +def test_checksum_file_with_nothing_usable_is_rejected(bootstrap, fake_http): + fake_http['checksum'] = b'garbage\n' + assert bootstrap.download_and_check(IMAGE_URL, SUM_URL) is None + + +def test_bad_digest_is_rejected_before_downloading(bootstrap, fake_http): + assert bootstrap.download_and_check(IMAGE_URL, SUM_URL, 'notahash') is None + assert fake_http['timeouts'] == [] # nothing was fetched + + +# ------------------------------------------------------------- uploading + +def test_upload_creates_a_new_image_and_returns_its_id(bootstrap, tmp_path): + image_file = tmp_path / 'img.qcow2' + image_file.write_bytes(IMAGE) + image_id = bootstrap.create_glance_image( + str(image_file), 'GOLD Test', 'qcow2', 8, 768, + {'hw_disk_bus': 'scsi', 'hw_rng_model': 'virtio'}) + assert image_id == 'new-image-uuid' + + created = [e[1] for e in bootstrap.conn.log if e[0] == 'image_created'][0] + assert created['name'] == 'GOLD Test' + assert created['disk_format'] == 'qcow2' + assert created['container_format'] == 'bare' + assert created['visibility'] == 'private' + assert created['min_disk'] == 8 and created['min_ram'] == 768 + assert created['hw_disk_bus'] == 'scsi' + + +def test_upload_forces_a_create_and_adds_no_metadata_of_its_own(bootstrap, tmp_path): + """Defaults would return an existing image and inject vendor properties.""" + image_file = tmp_path / 'img.qcow2' + image_file.write_bytes(IMAGE) + bootstrap.create_glance_image(str(image_file), 'n', 'qcow2', 8, 768, {}) + created = [e[1] for e in bootstrap.conn.log if e[0] == 'image_created'][0] + assert created['allow_duplicates'] is True + assert created['disable_vendor_agent'] is False + + +def test_failed_upload_returns_none(bootstrap, tmp_path): + bootstrap.conn.image.fail = True + image_file = tmp_path / 'img.qcow2' + image_file.write_bytes(IMAGE) + assert bootstrap.create_glance_image( + str(image_file), 'n', 'qcow2', 8, 768, {}) is None + + +def test_bootstrap_no_longer_carries_an_unused_availability_zone(): + assert not hasattr(BootstrapFunctions, 'avail_zone') + import inspect + params = inspect.signature(BootstrapFunctions.__init__).parameters + assert 'avail_zone' not in params + assert os.environ is not None # keep flake quiet about the import diff --git a/tests/test_build.py b/tests/test_build.py new file mode 100644 index 0000000..f276410 --- /dev/null +++ b/tests/test_build.py @@ -0,0 +1,380 @@ +"""The build path: resource lifecycle, manifest handling, image download.""" +import datetime +import io +import json +import os +import subprocess +import types + +import pytest + +from image_builder import helpers + +# --------------------------------------------------------------- keypairs + +def test_keygen_failure_reports_rather_than_returning_a_usable_key(build, monkeypatch): + monkeypatch.setattr(subprocess, 'run', + lambda *a, **k: types.SimpleNamespace(returncode=1)) + name, keypair_id = build.create_keypairs() + assert name is None and keypair_id is None + + +def test_keygen_runs_without_a_shell_and_off_PATH(build, monkeypatch): + seen = {} + + def fake_run(cmd, **kwargs): + seen['cmd'] = cmd + seen['kwargs'] = kwargs + return types.SimpleNamespace(returncode=1) # stop before the key is read + + monkeypatch.setattr(subprocess, 'run', fake_run) + build.create_keypairs() + assert isinstance(seen['cmd'], list) + assert seen['cmd'][0] == 'ssh-keygen' + assert 'shell' not in seen['kwargs'] + + +def test_successful_keygen_registers_the_public_key(build): + name, keypair_id = build.create_keypairs() + assert name.startswith('imagebuilder-') + assert keypair_id == name + created = [e for e in build.conn.log if e[0] == 'kp_created'] + assert len(created) == 1 + assert created[0][1]['public_key'].startswith('ecdsa-') + + +# -------------------------------------------------------- security groups + +def test_security_group_opens_ssh_on_both_ethertypes(build): + name, sgid = build.create_security_group() + assert name.startswith('imagebuilder-') + rules = [e[1] for e in build.conn.log if e[0] == 'rule_created'] + assert sorted(r['ethertype'] for r in rules) == ['IPv4', 'IPv6'] + for rule in rules: + assert rule['security_group_id'] == sgid + assert rule['direction'] == 'ingress' + assert rule['protocol'] == 'tcp' + assert rule['port_range_min'] == rule['port_range_max'] == 22 + + +# ---------------------------------------------------------------- cleanup + +def test_cleanup_removes_both_resources(build): + build.cleanup('sg-1', 'kp-1') + assert ('sg_deleted', 'sg-1') in build.conn.log + assert ('kp_deleted', 'kp-1') in build.conn.log + + +def test_cleanup_skips_resources_that_were_never_created(build): + """Runs from a finally, so it sees half-built states.""" + build.cleanup('sg-1', None) + assert ('sg_deleted', 'sg-1') in build.conn.log + assert 'kp_deleted' not in build.conn.kinds() + + +def test_cleanup_with_nothing_to_do_is_a_noop(build): + build.cleanup(None, None) + assert build.conn.log == [] + + +def test_cleanup_never_raises_so_it_cannot_mask_the_real_error(build): + def explode(_): + raise RuntimeError('neutron is down') + + build.conn.network.delete_security_group = explode + build.cleanup('sg-1', 'kp-1') # must not raise + assert ('kp_deleted', 'kp-1') in build.conn.log # and carries on + + +def test_clean_tmp_files_tolerates_a_missing_directory(): + helpers.clean_tmp_files('/nonexistent/path/at/all') + + +# --------------------------------------------------------------- manifest + +def write_manifest(build, payload): + path = os.path.join(build.tmp_dir, 'packer-manifest.json') + with open(path, 'w') as fh: + if isinstance(payload, str): + fh.write(payload) + else: + json.dump(payload, fh) + + +def test_good_manifest_yields_the_artifact_id(build): + write_manifest(build, {'builds': [{'artifact_id': 'bgo:image-uuid'}]}) + assert build.parse_manifest() == 'bgo:image-uuid' + + +@pytest.mark.parametrize('payload', [ + pytest.param('{ not json at all', id='malformed'), + pytest.param({}, id='no-builds-key'), + pytest.param({'builds': []}, id='empty-builds'), + pytest.param({'builds': [{}]}, id='no-artifact-id'), +]) +def test_unusable_manifest_returns_none_instead_of_raising(build, payload): + write_manifest(build, payload) + assert build.parse_manifest() is None + + +def test_missing_manifest_returns_none(build): + assert build.parse_manifest() is None + + +# --------------------------------------------------------------- networks + +def test_find_network_returns_the_first_match(build): + build.conn.network.networks_result = [ + type('Net', (), {'id': 'net-1'})(), + type('Net', (), {'id': 'net-2'})(), + ] + assert build.find_network_id('Dualstack') == 'net-1' + + +def test_find_network_returns_false_when_absent(build): + build.conn.network.networks_result = [] + assert build.find_network_id('Nope') is False + + +# ----------------------------------------------------------------- images + +def test_download_writes_the_file_and_then_deletes_from_glance(build): + assert build.download_image('img-uuid') == 0 + written = os.listdir(build.download_dir) + assert len(written) == 1 + assert written[0].startswith('test-image-') and written[0].endswith('.qcow2') + assert ('image_deleted', 'img-uuid') in build.conn.log + + +def test_failed_download_keeps_the_image_and_removes_the_partial_file(build): + build.conn.image.fail = True + assert build.download_image('img-uuid') == 1 + assert os.listdir(build.download_dir) == [] + assert 'image_deleted' not in build.conn.kinds() + + +def test_delete_image_of_nothing_does_not_call_the_api(build): + assert build.delete_image(None) is False + assert build.conn.log == [] + + +def test_delete_image_reports_failure(build): + def explode(image, **kw): + raise RuntimeError('nope') + + build.conn.image.delete_image = explode + assert build.delete_image('img-uuid') is False + + +# --------------------------------------------------------------- template + +def test_hcl_template_is_preferred_over_the_legacy_one(build): + open(os.path.join(build.template_dir, 'template.pkr.hcl'), 'w').close() + open(os.path.join(build.template_dir, 'template'), 'w').close() + assert build.find_template().endswith('template.pkr.hcl') + + +def test_legacy_template_is_still_accepted(build): + open(os.path.join(build.template_dir, 'template'), 'w').close() + assert build.find_template().endswith('template') + + +def test_missing_template_returns_none(build): + assert build.find_template() is None + + +def test_legacy_template_passes_when_the_plugin_is_already_installed(build, monkeypatch): + """required_plugins is HCL2 only, so init has nothing to install from.""" + monkeypatch.setattr(type(build), 'openstack_plugin_installed', + lambda self: True) + assert build.run_packer_init('/some/path/template') == 0 + + +def test_legacy_template_fails_early_when_the_plugin_is_missing(build, monkeypatch): + """Better here than after the security group and keypair exist.""" + monkeypatch.setattr(type(build), 'openstack_plugin_installed', + lambda self: False) + assert build.run_packer_init('/some/path/template') == 1 + + +@pytest.mark.parametrize('stdout,expected', [ + pytest.param('', False, id='nothing-installed'), + pytest.param('/home/u/.config/packer/plugins/github.com/hashicorp/openstack/' + 'packer-plugin-openstack_v1.1.4_x5.0_linux_amd64\n', True, + id='openstack-present'), + pytest.param('/home/u/.config/packer/plugins/github.com/hashicorp/qemu/' + 'packer-plugin-qemu_v1.1.0_x5.0_linux_amd64\n', False, + id='other-plugin-only'), +]) +def test_plugin_detection_reads_the_output_not_the_exit_code(build, monkeypatch, + stdout, expected): + monkeypatch.setattr(subprocess, 'run', + lambda *a, **k: types.SimpleNamespace(stdout=stdout, + returncode=0)) + assert build.openstack_plugin_installed() is expected + + +def test_missing_packer_binary_is_reported_not_raised(build, monkeypatch): + def no_packer(*a, **k): + raise FileNotFoundError('packer') + + monkeypatch.setattr(subprocess, 'run', no_packer) + assert build.openstack_plugin_installed() is False + + +# ------------------------------------------------------------ packer argv + +def test_packer_argv_supplies_every_declared_variable(build, monkeypatch, repo_root): + captured = {} + + class FakePopen: + def __init__(self, cmd, **kwargs): + captured['cmd'] = cmd + self.stdout = io.BytesIO(b'') + + def __enter__(self): + return self + + def __exit__(self, *exc): + self.stdout.close() + return False + + def wait(self): + return 0 + + monkeypatch.setattr(subprocess, 'Popen', FakePopen) + build.run_packer(os.path.join(build.template_dir, 'template.pkr.hcl'), + 'sg-name', 'kp-name', 'net-uuid') + + cmd = captured['cmd'] + assert cmd[:2] == ['packer', 'build'] + assert '--var' not in cmd # the old long form + supplied = {cmd[i + 1].split('=')[0] for i, a in enumerate(cmd) if a == '-var'} + + declared = set() + with open(repo_root / 'template.pkr.hcl') as fh: + for line in fh: + if line.startswith('variable "'): + declared.add(line.split('"')[1]) + assert supplied == declared + + +def test_download_streams_rather_than_buffering_the_whole_image(build): + """Without stream=True openstacksdk reads the entire image into memory. + + A 10 GiB image then needs more than 20 GiB of RAM and the process is + killed, which is exactly what happened on a builder node. + """ + build.download_image('img-uuid') + assert build.conn.image.download_kwargs.get('stream') is True + + +# --------------------------------------------------------------- sweeping +# +# Resources left behind by a run that was killed outright. The name shape and +# the age are the only things standing between this and deleting something a +# build on another node is using. + +def stamp(hours_ago): + when = datetime.datetime.now(datetime.timezone.utc) - datetime.timedelta(hours=hours_ago) + return when.strftime('%Y-%m-%dT%H:%M:%SZ') + + +def fake_group(name, hours_ago): + return type('SG', (), {'id': 'id-' + name, 'name': name, + 'created_at': stamp(hours_ago)})() + + +def fake_keypair(name, hours_ago): + created = stamp(hours_ago) if hours_ago is not None else None + return type('KP', (), {'name': name, 'created_at': created})() + + +@pytest.mark.parametrize('name,swept', [ + pytest.param('imagebuilder-' + 'a' * 32, True, id='ours'), + pytest.param('imagebuilder-' + 'A' * 32, False, id='uppercase-not-uuid'), + pytest.param('imagebuilder-short', False, id='wrong-shape'), + pytest.param('imagebuilder-' + 'a' * 33, False, id='too-long'), + pytest.param('default', False, id='someone-elses'), + pytest.param('my-imagebuilder-' + 'a' * 32, False, id='suffix-match'), + pytest.param('web-servers', False, id='unrelated'), +]) +def test_only_our_own_resource_names_are_swept(build, name, swept): + build.conn.network.groups = [fake_group(name, hours_ago=48)] + build.sweep_orphans() + deleted = [e for e in build.conn.log if e[0] == 'sg_deleted'] + assert bool(deleted) is swept + + +@pytest.mark.parametrize('hours,swept', [ + pytest.param(48, True, id='two-days-old'), + pytest.param(25, True, id='just-over'), + pytest.param(23, False, id='just-under'), + pytest.param(0, False, id='brand-new'), +]) +def test_only_resources_older_than_the_threshold_are_swept(build, hours, swept): + """A build on another node shares this project and may be using it.""" + build.conn.network.groups = [fake_group('imagebuilder-' + 'b' * 32, hours)] + build.sweep_orphans() + deleted = [e for e in build.conn.log if e[0] == 'sg_deleted'] + assert bool(deleted) is swept + + +def test_a_resource_with_no_usable_timestamp_is_left_alone(build): + group = fake_group('imagebuilder-' + 'c' * 32, 48) + group.created_at = None + build.conn.network.groups = [group] + assert build.sweep_orphans() == 0 + + +def test_keypairs_are_swept_too(build): + build.conn.compute.pairs = [fake_keypair('imagebuilder-' + 'd' * 32, 48)] + assert build.sweep_orphans() == 1 + assert ('kp_deleted', 'imagebuilder-' + 'd' * 32) in build.conn.log + + +def test_a_keypair_whose_age_is_unknown_is_left_alone(build): + """Nova does not always report created_at when listing keypairs.""" + build.conn.compute.pairs = [fake_keypair('imagebuilder-' + 'e' * 32, None)] + assert build.sweep_orphans() == 0 + + +def test_images_are_never_swept(build): + """A leftover image is named like a good one, so it cannot be told apart.""" + build.conn.network.groups = [fake_group('imagebuilder-' + 'f' * 32, 48)] + build.sweep_orphans() + assert 'image_deleted' not in build.conn.kinds() + + +def test_a_listing_failure_does_not_stop_the_build(build): + def explode(**kw): + raise RuntimeError('neutron is down') + + build.conn.network.security_groups = explode + assert build.sweep_orphans() == 0 # must not raise + + +def test_a_delete_failure_does_not_stop_the_build(build): + def explode(sgid): + raise RuntimeError('in use') + + build.conn.network.groups = [fake_group('imagebuilder-' + 'a' * 32, 48)] + build.conn.network.delete_security_group = explode + assert build.sweep_orphans() == 0 # must not raise + + +@pytest.mark.parametrize('value', [ + '2026-09-20T16:43:06Z', + '2026-09-20T16:43:06.123456Z', + '2026-09-20T16:43:06+00:00', + '2026-09-20T16:43:06.000000', +]) +def test_timestamp_formats_openstack_actually_returns(build, value): + from image_builder.build import BuildFunctions + assert BuildFunctions.resource_age_hours(value) is not None + + +@pytest.mark.parametrize('value', [None, '', 'not a date', 12345]) +def test_unreadable_timestamps_give_no_age(build, value): + from image_builder.build import BuildFunctions + assert BuildFunctions.resource_age_hours(value) is None diff --git a/tests/test_cli.py b/tests/test_cli.py new file mode 100644 index 0000000..3b2585c --- /dev/null +++ b/tests/test_cli.py @@ -0,0 +1,106 @@ +"""The command line itself, driven as a subprocess.""" +import os +import subprocess +import sys + +import pytest + +from image_builder.parsecommands import Commands + + +def run(repo_root, *args): + """Runs the checked-in entry script with no OpenStack credentials.""" + env = {k: v for k, v in os.environ.items() + if not k.startswith('OS_') and not k.startswith('IB_')} + env['PATH'] = os.environ['PATH'] + return subprocess.run([sys.executable, 'imagebuilder', *args], + cwd=str(repo_root), env=env, + capture_output=True, text=True) + + +# ------------------------------------------------------------ dispatching + +def test_help_lists_both_subcommands(repo_root): + result = run(repo_root, '--help') + assert 'build' in result.stdout + assert 'bootstrap' in result.stdout + + +def test_no_subcommand_is_an_error(repo_root): + assert run(repo_root).returncode != 0 + + +@pytest.mark.parametrize('name', ['__init__', 'cleanup', 'nonsense']) +def test_arbitrary_attributes_are_not_dispatchable(repo_root, name): + """Dispatch used to be getattr(self, argv[1]).""" + result = run(repo_root, name) + assert result.returncode != 0 + assert 'invalid choice' in (result.stderr + result.stdout) + + +# ------------------------------------------------------------- parsing + +def test_build_args_parse(): + commands = Commands(['build', '-n', 'img', '-a', 'bgo-default-1', + '-s', 'src', '-u', 'almalinux']) + assert commands.build_args.name == 'img' + assert commands.build_args.flavor == 'm1.small' + assert commands.build_args.network_name == 'Dualstack' + assert commands.bootstrap_args is None + + +def test_bootstrap_args_parse_numbers_as_ints(): + commands = Commands(['bootstrap', '-a', 'bgo-default-1', + '-u', 'https://example.com/i.qcow2', '-n', 'n', + '-r', '768', '-d', '8', '-f', 'qcow2']) + assert commands.bootstrap_args.min_ram == 768 + assert commands.bootstrap_args.min_disk == 8 + assert commands.build_args is None + + +def test_non_numeric_min_ram_is_rejected_at_parse_time(repo_root): + result = run(repo_root, 'bootstrap', '-a', 'z', '-u', 'https://x/i.qcow2', + '-n', 'n', '-r', 'notanumber', '-d', '8', '-f', 'qcow2') + assert result.returncode != 0 + assert 'invalid int' in result.stderr + + +# ------------------------------------------------------------- streams + +def test_diagnostics_go_to_stderr_leaving_stdout_clean(repo_root): + """bootstrap pipes its image id out of stdout, so errors must not go there.""" + result = run(repo_root, 'build', '-n', 'x', '-a', 'z', '-s', 'i', '-u', 'u') + assert result.returncode != 0 + assert result.stdout == '' + assert 'Missing environment variable' in result.stderr + + +def test_the_missing_variables_are_named(repo_root): + result = run(repo_root, 'build', '-n', 'x', '-a', 'z', '-s', 'i', '-u', 'u') + assert 'OS_USERNAME' in result.stderr + + +# ------------------------------------------------------------- logging + +@pytest.mark.parametrize('flags,expected', [ + pytest.param([], 'WARNING', id='default'), + pytest.param(['-v'], 'INFO', id='verbose'), + pytest.param(['--debug'], 'DEBUG', id='debug'), + pytest.param(['-v', '--debug'], 'DEBUG', id='debug-wins-over-verbose'), +]) +def test_log_level_selection(flags, expected): + import logging + + from image_builder.imagebuilder import configure_logging + + commands = Commands(['build', '-n', 'i', '-a', 'z', '-s', 's', '-u', 'u', + *flags]) + root = logging.getLogger() + saved_level, saved_handlers = root.level, root.handlers[:] + root.handlers = [] # basicConfig is a no-op otherwise + try: + configure_logging(commands.build_args) + assert logging.getLevelName(root.level) == expected + finally: + root.handlers = saved_handlers + root.setLevel(saved_level) diff --git a/tests/test_credentials.py b/tests/test_credentials.py new file mode 100644 index 0000000..519b6f7 --- /dev/null +++ b/tests/test_credentials.py @@ -0,0 +1,51 @@ +"""Reading credentials from the environment.""" +import pytest +from conftest import FULL_OPENRC + +from image_builder import imagebuilder + + +def test_openrc_without_the_legacy_variables_is_accepted(clean_env): + """OS_IDENTITY_API_VERSION and OS_NO_CACHE were required but never used.""" + for key, value in FULL_OPENRC.items(): + clean_env.setenv(key, value) + rc = imagebuilder.get_openstack_rc() + assert rc['username'] == 'user' + assert rc['region_name'] == 'bgo' + + +def test_unused_keys_are_not_carried_around(clean_env): + for key, value in FULL_OPENRC.items(): + clean_env.setenv(key, value) + rc = imagebuilder.get_openstack_rc() + assert 'api_version' not in rc + assert 'no_cache' not in rc + + +def test_cacert_is_optional(clean_env): + for key, value in FULL_OPENRC.items(): + clean_env.setenv(key, value) + assert imagebuilder.get_openstack_rc()['cacert'] is None + clean_env.setenv('OS_CACERT', '/etc/ssl/ca.pem') + assert imagebuilder.get_openstack_rc()['cacert'] == '/etc/ssl/ca.pem' + + +@pytest.mark.parametrize('missing', sorted(FULL_OPENRC)) +def test_each_required_variable_is_reported_by_name(clean_env, missing): + for key, value in FULL_OPENRC.items(): + if key != missing: + clean_env.setenv(key, value) + with pytest.raises(KeyError) as excinfo: + imagebuilder.get_openstack_rc() + assert missing in excinfo.value.args[0] + + +def test_all_missing_variables_are_reported_not_just_the_first(clean_env): + for key, value in FULL_OPENRC.items(): + if key not in ('OS_PASSWORD', 'OS_REGION_NAME'): + clean_env.setenv(key, value) + with pytest.raises(KeyError) as excinfo: + imagebuilder.get_openstack_rc() + message = excinfo.value.args[0] + assert 'OS_PASSWORD' in message + assert 'OS_REGION_NAME' in message diff --git a/tests/test_main_flow.py b/tests/test_main_flow.py new file mode 100644 index 0000000..f91b9a3 --- /dev/null +++ b/tests/test_main_flow.py @@ -0,0 +1,256 @@ +"""main() end to end: every exit path must still clean up. + +These drive main() with a stand-in BuildFunctions, so they cover the wiring +between the pieces rather than the pieces themselves. +""" +import os +import tempfile + +import pytest +from conftest import FULL_OPENRC + +import image_builder.imagebuilder as ib +from image_builder.parsecommands import Commands + + +@pytest.fixture +def run_build(clean_env, monkeypatch): + """Runs main() for a build, returning (exit code, propagated exception, calls).""" + + def run(packer=lambda: 0, keygen_ok=True, manifest='img-uuid', argv=None): + scratch = tempfile.mkdtemp(prefix='mainflow-dirs-') + calls = [] + + class FakeBuild: + def __init__(self, *a, **k): + self.tmp_dir = tempfile.mkdtemp(prefix='mainflow-') + self.image_name = 'img' + + def find_template(self): + return os.path.join(scratch, 'template.pkr.hcl') + + def find_network_id(self, name): + return 'net-1' + + def run_packer_init(self, template): + return 0 + + def sweep_orphans(self, *a, **k): + calls.append('swept') + return 0 + + def create_security_group(self): + calls.append('sg_created') + return 'sg-name', 'sg-1' + + def create_keypairs(self): + if not keygen_ok: + return None, None + calls.append('kp_created') + return 'kp-name', 'kp-1' + + def run_packer(self, *a): + return packer() + + def parse_manifest(self): + return manifest + + def download_image(self, artifact_id): + return 0 + + def delete_image(self, image_id): + return True + + def cleanup(self, secgroup_id, keypair_id): + calls.append(f'cleanup(sg={secgroup_id} kp={keypair_id})') + + for key, value in FULL_OPENRC.items(): + clean_env.setenv(key, value) + clean_env.setenv('IB_TEMPLATE_DIR', scratch) + clean_env.setenv('IB_DOWNLOAD_DIR', scratch) + monkeypatch.setattr(ib, 'BuildFunctions', FakeBuild) + monkeypatch.setattr(ib, 'auth', lambda rc: object()) + monkeypatch.setattr('sys.argv', argv or + ['imagebuilder', 'build', '-n', 'img', + '-a', 'bgo-default-1', '-s', 'src', '-u', 'almalinux']) + + code, raised = None, None + try: + ib.main() + except SystemExit as exc: + code = exc.code + except BaseException as exc: # noqa: BLE001 + raised = exc + return code, raised, calls + + return run + + +def cleaned(calls): + return [c for c in calls if c.startswith('cleanup')] + + +def test_successful_build_exits_zero_and_cleans_up(run_build): + code, raised, calls = run_build() + assert code == 0 and raised is None + assert cleaned(calls) == ['cleanup(sg=sg-1 kp=kp-1)'] + + +def test_failed_packer_run_exits_one_and_cleans_up(run_build): + code, raised, calls = run_build(packer=lambda: 1) + assert code == 1 + assert cleaned(calls) + + +def test_interrupt_during_build_still_cleans_up(run_build): + """The case that used to orphan a security group and a keypair.""" + def interrupt(): + raise KeyboardInterrupt() + + code, raised, calls = run_build(packer=interrupt) + assert cleaned(calls) == ['cleanup(sg=sg-1 kp=kp-1)'] + assert isinstance(raised, KeyboardInterrupt) # and is not swallowed + + +def test_unexpected_exception_still_cleans_up(run_build): + def boom(): + raise RuntimeError('packer blew up') + + code, raised, calls = run_build(packer=boom) + assert cleaned(calls) + assert isinstance(raised, RuntimeError) + + +def test_keygen_failure_exits_before_packer_and_cleans_up(run_build): + code, raised, calls = run_build(keygen_ok=False) + assert code == 1 + assert 'kp_created' not in calls + assert cleaned(calls) == ['cleanup(sg=sg-1 kp=None)'] + + +def test_unusable_manifest_fails_the_build(run_build): + code, raised, calls = run_build(manifest=None) + assert code == 1 + assert cleaned(calls) + + +# The split made these reachable without driving the whole of main() + +def test_image_properties_defaults_to_scsi_and_rng(): + args = Commands(['bootstrap', '-a', 'z', '-u', 'https://x/i.qcow2', '-n', 'n', + '-r', '768', '-d', '8', '-f', 'qcow2']).bootstrap_args + props = ib.image_properties(args) + assert props == {'hw_rng_model': 'virtio', 'hw_disk_bus': 'scsi', + 'hw_scsi_model': 'virtio-scsi'} + + +def test_image_properties_without_scsi(): + args = Commands(['bootstrap', '-a', 'z', '-u', 'https://x/i.qcow2', '-n', 'n', + '-r', '768', '-d', '8', '-f', 'qcow2', '-s']).bootstrap_args + props = ib.image_properties(args) + assert 'hw_disk_bus' not in props + assert props['hw_rng_model'] == 'virtio' + + +def test_image_properties_with_efi(): + args = Commands(['bootstrap', '-a', 'z', '-u', 'https://x/i.qcow2', '-n', 'n', + '-r', '768', '-d', '8', '-f', 'qcow2', '-e']).bootstrap_args + props = ib.image_properties(args) + assert props['hw_firmware_type'] == 'uefi' + assert props['hw_machine_type'] == 'q35' + + +class FakeBootstrap: + """Records what run_bootstrap asked of it.""" + + instances = [] + + def __init__(self, *a, **k): + self.tmp_dir = tempfile.mkdtemp(prefix='bootflow-') + self.downloaded = None + self.uploaded = None + FakeBootstrap.instances.append(self) + + def download_and_check(self, url, checksum_url, digest): + self.downloaded = (url, checksum_url, digest) + return None if url.endswith('fail') else os.path.join(self.tmp_dir, 'img.qcow2') + + def create_glance_image(self, *a): + self.uploaded = a + return None if a[1] == 'upload-fail' else 'new-image-uuid' + + +@pytest.fixture +def run_bootstrap(clean_env, monkeypatch, capsys): + def run(name='img', url='https://example.com/i.qcow2'): + FakeBootstrap.instances = [] + monkeypatch.setattr(ib, 'BootstrapFunctions', FakeBootstrap) + args = Commands(['bootstrap', '-a', 'z', '-u', url, '-n', name, + '-r', '768', '-d', '8', '-f', 'qcow2']).bootstrap_args + code = ib.run_bootstrap(args, object(), 'bgo') + return code, capsys.readouterr(), FakeBootstrap.instances[0] + + return run + + +def test_bootstrap_writes_only_the_image_id_to_stdout(run_bootstrap): + code, captured, fake = run_bootstrap() + assert code == 0 + assert captured.out == 'new-image-uuid' + assert not os.path.exists(fake.tmp_dir) # cleaned up + + +def test_bootstrap_download_failure_exits_one_and_cleans_up(run_bootstrap): + code, captured, fake = run_bootstrap(url='https://example.com/fail') + assert code == 1 + assert captured.out == '' + assert not os.path.exists(fake.tmp_dir) + + +def test_bootstrap_upload_failure_exits_one_and_cleans_up(run_bootstrap): + code, captured, fake = run_bootstrap(name='upload-fail') + assert code == 1 + assert captured.out == '' + assert not os.path.exists(fake.tmp_dir) + + +BUILD_ARGV = ['imagebuilder', 'build', '-n', 'img', '-a', 'bgo-default-1', + '-s', 'src', '-u', 'almalinux'] + + +def test_no_sweep_unless_it_is_asked_for(run_build): + """It deletes resources this run did not create, so it is opt-in.""" + code, raised, calls = run_build() + assert 'swept' not in calls + assert code == 0 + + +def test_sweep_happens_before_anything_new_is_created(run_build): + code, raised, calls = run_build(argv=BUILD_ARGV + ['--sweep']) + assert 'swept' in calls + assert calls.index('swept') < calls.index('sg_created') + + +def test_ib_sweep_in_the_environment_asks_for_one_too(run_build, monkeypatch): + """build-images.sh runs build scripts that take no arguments of their own.""" + monkeypatch.setenv('IB_SWEEP', 'true') + code, raised, calls = run_build() + assert 'swept' in calls + + +@pytest.mark.parametrize('value,swept', [ + pytest.param('true', True, id='true'), + pytest.param('TRUE', True, id='uppercase'), + pytest.param('1', True, id='one'), + pytest.param('yes', True, id='yes'), + pytest.param('on', True, id='on'), + pytest.param('false', False, id='false'), + pytest.param('0', False, id='zero'), + pytest.param('no', False, id='no'), + pytest.param('', False, id='empty'), +]) +def test_ib_sweep_values(run_build, monkeypatch, value, swept): + """IB_SWEEP=false must not turn it on.""" + monkeypatch.setenv('IB_SWEEP', value) + code, raised, calls = run_build() + assert ('swept' in calls) is swept