Modernise: current Packer, current OpenStack clients, current Python

.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

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 <command>          # 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-<uuid4hex>`, 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).

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.

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-<UUID>" 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-<UUID>" 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 <command> -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

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