Skip to content

Modernise: current Packer, current OpenStack clients, current Python - #27

Open
trondham wants to merge 22 commits into
norcams:masterfrom
trondham:major_refactor
Open

trondham wants to merge 22 commits into
norcams:masterfrom
trondham:major_refactor

Conversation

@trondham

Copy link
Copy Markdown
Contributor

Packer 1.6.6-era code brought up to current Packer, current OpenStack clients, and current Python. Three groups of commits: making it work again, then an audit and its fixes, then modernisation.

Why it needed doing

Packer 1.10 removed bundled plugins from the core binary, so "type": "openstack" does not resolve at all on any current Packer — builds fail before they start. Plugins now come from a required_plugins block via packer init, and that block is HCL2-only, which is what forced the template migration.

Making it build again

  • template → template.pkr.hcl, pinning github.com/hashicorp/openstack ~> 1.1
  • packer init runs before every build; it is a no-op for a legacy JSON template, which find_template() still accepts so existing template_dir setups keep working
  • script paths use ${path.root} instead of depending on Packer's working directory
  • verified against Packer 1.16.0 with openstack plugin v1.1.4

Fixes from the audit

The full audit is written up separately; the findings it produced, in severity order:

  • Two unused environment variables were mandatory. OS_IDENTITY_API_VERSION and OS_NO_CACHE were read and never used, but a missing one aborted with "Failed to read environment variables". OS_NO_CACHE is absent from many current openrc files, so valid credentials were rejected with a message pointing at the wrong problem.
  • Resources leaked on any unclean exit. Cleanup was positional, not finally, so a Ctrl-C during a twenty-minute build left the security group and keypair behind. This was the outstanding TODO.md item. cleanup() is now written for that position: it skips ids that are None and swallows API errors, since raising from a finally would mask the original failure. SIGTERM still bypasses it — verified — and TODO.md now says that rather than signals generally.
  • A failed ssh-keygen returned ('', 0) and nobody checked, so the build carried on into Packer with an empty keypair name.
  • parse_manifest() was unguarded, so a successful build with an unusable manifest ended in a traceback which, per the above, also skipped cleanup.
  • Checksum handling: hashlib.new() replaces a dispatch ladder that raised AttributeError on -t sha1 or -t SHA256; checksum files are parsed in both coreutils and BSD layouts and matched on filename, so uppercase files no longer fail; size comes from disk rather than a content-length header chunked responses do not send; both downloads take a timeout.
  • Packer's log arrived as b'openstack: Creating image...\n' for every line. Diagnostics moved to stderr, because bootstrap writes only the image id to stdout for piping.
  • Subcommands use argparse subparsers; imagebuilder __init__ used to recurse into the constructor.

Dependencies

novaclient and glanceclient are gone — keypairs moved to conn.compute, images to conn.image, and the glance image-download subprocess became an API call on the session already held rather than a second authentication from OS_*. python-neutronclient, which prints a deprecation warning on import, went the same way earlier.

Four packages instead of six, all at the newest release installable on Python 3.9. Only keystoneauth1 and openstacksdk are capped by that floor, and the 3.11+ replacements are staged as comments in requirements.txt. DEVELOPMENT.md records that EL9 ships python3.12 as a parallel AppStream package, so moving off 3.9 does not require waiting for EL10.

Modernisation

One latent bug: three text-mode opens had no encoding=, so the public key, the manifest and the config file were decoded using the locale and read differently under LC_ALL=C.

Otherwise: bare except: narrowed, an assert used for validation became a ValueError (asserts vanish under python -O), BuildFunctions' ten positional parameters made keyword-only, Helpers and ImageBuilder dissolved into module functions, f-strings, subprocess.run. main() went from 172 lines and complexity 21 to 28 lines, with run_build and run_bootstrap returning exit codes so they can be called directly.

CI

Was already failing on master before any of this: the workflow installed ruff unpinned, and 0.16.0 widened the default rule set. pyproject.toml now selects rules explicitly, so that cannot happen again, and ruff is unpinned — verified clean under both 0.15.22 and 0.16.7. It also builds every branch rather than only master, and checkout/setup-python moved to v7 for Node 24.

93 tests, from nothing. They use stand-in OpenStack clients and a fake HTTP layer, so they need no credentials and reach no network. test_main_flow.py drives main() itself to pin down that an interrupt or an exception still cleans up.

What has not been verified

No live build has been run against NREC. Everything here is verified by test, signature, resolution and lint. The HCL2 template validates against a real Packer with the real plugin, but has never built an actual image, and the openstacksdk calls have never reached a real cloud. Worth one real imagebuilder build before merging.

Two deliberate behaviour changes worth a look during review:

  • Checksum verification falls back to the old "hash appears anywhere in the file" check, with a warning, when the checksum file has no entry naming the download. Strict filename matching alone would have broken vendors who name things differently.
  • template_dir and download_dir are resolved only for a build, so a config without template_dir no longer fails a bootstrap that never wanted it.

CLAUDE.md is included; say the word and I will drop it in a follow-up commit if it is not wanted in-tree.

🤖 Generated with Claude Code

https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y

trondham and others added 14 commits September 10, 2026 10:02
… init

Packer stopped bundling builder plugins in 1.10, so the openstack builder no
longer resolves in a stock binary and builds fail outright on any current
Packer. Plugins are installed from a required_plugins block via packer init,
which only works with HCL2, so the legacy JSON template had to go.

- template -> template.pkr.hcl, pinning github.com/hashicorp/openstack ~> 1.1
- run packer init before every build (no-op for a legacy JSON template, which
  find_template() still accepts so existing template_dir setups keep working)
- packer build --var -> -var
- script paths use ${path.root} instead of being relative to Packer's cwd
- resolve the template and the network before creating the temporary security
  group and keypair, so those failures no longer orphan OpenStack resources

Verified against Packer 1.16.0 with openstack plugin v1.1.4: packer init
installs the plugin and packer validate passes on both templates, failing only
on the absence of live OpenStack credentials.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
The workflow installed ruff unpinned, so CI started failing the moment ruff
0.16.0 shipped a wider default rule set: it reports 73 findings on code that
predates this change (mostly LOG015 root-logger calls and UP031 percent
formatting). Pin to the newest release that lints clean so lint results are
reproducible, and adopt the new defaults deliberately instead of on whatever
day upstream releases.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
There are no tests in the repository and the workflow never invoked pytest, so
installing it only slowed the job down and made CI look like it ran a test
suite. Fix the header comment to match what the workflow actually does.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Work happens on feature branches, so waiting for a pull request to find out
that lint is broken is too late. Build every branch on push, and keep the
pull_request trigger so contributions from forks are still covered.

Add a concurrency group keyed on the source branch, since building every branch
would otherwise mean two runs for every pull request opened from this
repository.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Both actions were pinned to majors that run on Node 20, which GitHub now forces
onto Node 24 and warns about on every run. checkout v4 and setup-python v5 are
still Node 20, so the bump has to go further than one major: Node 24 arrives in
checkout v5 and setup-python v6, and v7 is current for both.

Nothing in the newer majors affects this workflow. checkout v7 only blocks fork
checkouts for pull_request_target and workflow_run, neither of which is used
here, and setup-python v7 only drops the pip-install input, which is not used
either.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Every pin was three to five years old. keystoneauth1 4.3.1, novaclient 17.4.0,
glanceclient 3.3.0 and neutronclient 7.3.0 all date from around 2021, and pbr
5.11.1 from 2023. Only cryptography was current, because dependabot bumps it.

Pins go to the newest releases that still support Python 3.9, which is what the
CI matrix tests. keystoneauth1 and python-glanceclient are the two held back by
that floor; the rest are now current. pbr goes to 7.0.3 rather than 7.1.2,
which is yanked.

Verified the pin set resolves for 3.9, 3.10 and 3.11, and that every OpenStack
API the code calls still exists with a compatible signature: v3.Password,
Session(verify=...), the nova/neutron/glance client constructors, keypairs
create and delete, the security group and rule calls, list_networks, and the
glance image create, upload and delete.

Note that importing neutronclient now prints a deprecation warning: the
bindings are slated for removal in favour of openstacksdk, which is already
installed as a transitive dependency.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
python-neutronclient prints a deprecation warning on import and its bindings
are slated for removal in favour of openstacksdk, which was already installed
as a transitive dependency. All four network calls move over: creating and
deleting the temporary security group, its two SSH rules, and the network
lookup.

The connection rather than the conn.network proxy is stored on the instance.
Touching a proxy authenticates and discovers endpoints, so building one in the
constructor would have made it do network I/O, which neutronclient never did.

find_network_id() uses networks() rather than the more idiomatic
find_network(), which raises on a duplicate name where this code has always
taken the first match. Behaviour is unchanged, including returning False when
nothing matches.

Verified against openstacksdk 4.5.0 and 4.19.1 by binding a fake proxy to the
real method signatures, so a renamed or re-typed argument fails the check: both
rules are still created with the right ethertypes against the new group, the
lookup returns the id, a miss still returns False, and cleanup still deletes
the group. openstacksdk is now pinned explicitly at 4.5.0, the newest release
supporting Python 3.9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Moving off Python 3.9 does not require EL10. EL9 ships newer interpreters as
parallel-installable AppStream packages, so the venv can move to python3.12 on
the existing host whenever the three capped pins become a problem.

Record why the floor exists and when it stops being comfortable: 3.9 is the
minimum supported runtime for OpenStack 2025.1, but 2025.2 raised it to 3.10
and only keeps 3.9 fallbacks in its constraints, pinning exactly the versions
used here.

The 3.11+ set is left commented in requirements.txt so the switch is an
uncomment rather than research. Verified it resolves for 3.11, 3.12 and 3.13,
is correctly rejected on 3.9, and that keystoneauth1 5.17.0, glanceclient
4.13.0 and openstacksdk 4.20.0 pass the same API and network checks as the
current pins. Commenting the capped pins and uncommenting the block yields
exactly that set.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Four bugs from the code audit.

Two environment variables were mandatory but unused. OS_IDENTITY_API_VERSION
and OS_NO_CACHE were read into the settings dict and never looked at again, yet
a missing one aborted the run with "Failed to read environment variables".
OS_NO_CACHE is absent from many current openrc files, so valid credentials were
rejected with a message pointing at the wrong problem. Only the variables auth()
and the region lookup consume are required now, and the error names every
missing one instead of just the first.

Cleanup was positional rather than unconditional. Everything after the first
OpenStack resource is created now runs under a try/finally, so an exception or
a Ctrl-C during a long build no longer leaves the security group and keypair
behind. cleanup() is written for that position: it skips ids that are None and
swallows API errors, because raising from a finally would mask the original
failure. clean_tmp_files() likewise no longer fails on an already-removed
directory. SIGTERM still bypasses all of this, so TODO.md now says that rather
than signals generally.

A failed ssh-keygen returned ('', 0) and nobody checked, so the build carried
on into packer with an empty keypair name. It returns None now and the caller
exits through the cleanup path.

parse_manifest() indexed the JSON with no error handling, so a successful build
whose manifest was missing or malformed ended in a traceback, which by the bug
above also skipped cleanup. It reports and returns None, and the caller treats
that as a failed build.

Verified by driving main() with a fake build for six scenarios: interrupt
during packer, unexpected exception, keygen failure, unusable manifest, clean
success and a non-zero packer exit. All six now reach cleanup with the right
ids and the right exit code, and interrupts still propagate rather than being
swallowed. The same harness against the previous code shows the first two
scenarios reaching neither cleanup call. Confirmed separately that a finally
block runs on SIGINT but not on SIGTERM.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Bootstrap correctness. checksum_file() dispatched digests with a hand-written
if/if/elif ladder, so anything outside sha256/sha512/md5 stayed a string and
died with AttributeError on the first update(); -t sha1 and -t SHA256 both
crashed. It uses hashlib.new() now, which takes every supported algorithm and
raises a clear ValueError otherwise, and a bad -t is rejected before the
download rather than after it.

Checksum verification was a substring test against the repr of the raw bytes,
so it was case sensitive and never tied a hash to a filename. Checksum files
are now parsed properly in both the coreutils and BSD layouts and matched on
the basename, comparing lowercased. Where the file has no entry naming the
download, it falls back to the old contains check with a warning, so vendors
that name things differently keep working.

The size check read content-length, which is absent from chunked responses and
describes what the server claimed rather than what arrived; it uses the size on
disk now. Both urlopen() calls got a timeout, so a stalled mirror fails instead
of hanging forever.

Output. Packer's log was emitted through '%r' and arrived as b'...\n' for every
line; it is decoded and stripped. Diagnostics moved to stderr, which matters
because bootstrap's contract is that stdout carries only the new image id for
piping into build -s.

One client library. novaclient and glanceclient are gone: keypairs move to
conn.compute, images to conn.image. That also replaces the glance CLI
subprocess in download_image(), which re-authenticated from OS_* independently
of the session already held, and removes glanceclient from the packages capped
by Python 3.9. Bootstrap uploads through conn.image.create_image with
allow_duplicates=True, since the default returns an existing same-named image
instead of creating one, and disable_vendor_agent=False so it does not add
image properties of its own.

Subcommands use argparse subparsers rather than getattr(self, argv[1]), which
made every attribute reachable from the command line and left the command list
as unchecked prose in the usage string. --min-ram and --min-disk validate as
ints at parse time. ssh-keygen runs as a list without a shell and is found on
PATH instead of hardcoded to /usr/bin.

Verified with 60 new checks covering digest dispatch, both checksum layouts
including the uppercase and wrong-name cases, size and timeout handling,
decoded output, stdout staying empty on failure, the subparser behaviour, and
download_image writing the file, deleting from Glance on success, and removing
the partial file without deleting from Glance on failure. The earlier suites
still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Findings 09 and 10 turned out to be fixed already as a side effect of the
medium batch: delete_image() gained explicit returns and lost its duplicate log
line when it moved to conn.image, and switching the upload to create_image()
removed both the unclosed file handle and the over-wide except BaseException.

Logging. Every module now has its own logger and passes arguments lazily rather
than formatting into the message first. Levels carry meaning: progress is info,
trouble that does not stop the run is warning, and anything that ends the run is
error. Configuration happens once, in one place, instead of being repeated per
subcommand, and --debug now wins when both it and --verbose are given. The
default level is WARNING rather than nothing at all, so an ordinary run stays
quiet but a failure says why instead of just exiting non-zero.

Entry points. Both checked-in scripts use env python3, so an activated
virtualenv is actually honoured; the hardcoded /usr/bin/python3 ignored it,
which mattered increasingly now that DEVELOPMENT.md recommends a python3.12
venv. The shebang on imagebuilder.py is gone, since a module is not a script,
and __main__.py no longer runs a build merely on import.

Dead code. Config.show_config() was an empty stub. BootstrapFunctions took an
availability zone it never read; the -a flag stays on the bootstrap command
because removing a required argument is a contract change rather than a
cleanup, but it is now visibly unused.

Packaging moves to pyproject.toml, so setup.py is gone and the deprecated
python setup.py develop becomes pip install -e ., which now also pulls the two
libraries actually imported.

Tests. 87 of them, covering what the audit went through: credentials, the
keypair and security group lifecycle, cleanup from a finally block, manifest
handling, image download and its failure path, both checksum file layouts,
digests, timeouts, the subcommand parser and stream separation. test_main_flow
drives main() with a stand-in build to pin down that an interrupt or an
exception still cleans up. They use fake clients and a fake HTTP layer, so they
need no credentials and reach no network. CI runs them, with pytest pinned to
8.4.2, the last release supporting Python 3.9.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
One latent bug among the cleanup: three text-mode opens had no encoding, so the
public key, the Packer manifest and the config file were all decoded using the
locale. The same file would read differently on a build node running under
LC_ALL=C than on a UTF-8 workstation. They are explicit now.

Two bare excepts around config.get() would also have swallowed
KeyboardInterrupt; they catch the two configparser errors they meant to. The
argument check in checksum_file() was an assert, which disappears entirely
under python -O, and is now a ValueError.

BuildFunctions took ten positional parameters and main() passed ten positional
arguments, so transposing two of them would have been silent and legal. The
image-related ones are keyword-only now.

Helpers was five static methods with no state and ImageBuilder was two: both
were classes standing in for a module, which Python already provides. They are
module-level functions, and the call sites read the same because the import
became `from . import helpers`. The subcommand attributes use None rather than
False for "not this one", classes no longer inherit from object, string
building uses f-strings where it was concatenation or percent formatting, and
subprocess.run replaces subprocess.call with Popen in a with block. Lazy
percent arguments inside logging calls stay as they are, which is correct.

ruff now selects its rules explicitly in pyproject.toml instead of inheriting
whatever the current default happens to be, which is what broke CI when 0.16.0
widened it. That makes the pin unnecessary, so ruff is unpinned again; verified
clean under both 0.15.22 and 0.16.7. pytest stays pinned because 9 requires
Python 3.10.

No behaviour changes. The 87 tests pass unmodified except where they reached
for the things that moved: two caught the subprocess change by failing, which
is what they are for.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
main() was 172 lines with 28 branches and a cyclomatic complexity of 21. It
read arguments, configured logging, resolved directories and inlined both
subcommand workflows, which is why neither workflow could be tested without
standing up the whole thing.

It is 28 lines now: parse, configure, authenticate, dispatch. The two
workflows return an exit code rather than calling sys.exit, so they compose and
can be called directly from a test. Returning from inside the try still runs
the finally, so the cleanup guarantees are unchanged; the six main-flow tests
covering interrupt, exception, keygen failure, unusable manifest, success and
a failed packer run all pass untouched.

Two deliberate behaviour changes, both narrowing when an error can happen:

  - template_dir and download_dir are now resolved only for a build. They were
    read before either subcommand ran, so a config without template_dir failed
    a bootstrap that never wanted it.
  - run_bootstrap cleans up its temporary directory from a finally. Previously
    an exception from the download left it behind.

Six new tests for what the split exposed: the three image_properties variants,
and run_bootstrap writing only the image id to stdout, exiting 1 on a download
failure and on an upload failure, cleaning up its scratch directory in all
three cases. 93 tests, all passing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
@trondham trondham added enhancement dependencies Pull requests that update a dependency file python Pull requests that update python code do-not-merge labels Sep 14, 2026
A build against a legacy JSON template failed deep inside packer with "the
builder openstack is unknown by Packer", and by then the temporary security
group and keypair had already been created.

run_packer_init() skipped legacy templates entirely, on the grounds that only
HCL2 can declare required_plugins and so there is nothing to install from.
That is true, but returning 0 told the caller everything was ready. It now
checks whether the plugin is installed at all, via 'packer plugins installed',
and fails with the instruction to either install it by hand or migrate the
template. 'packer plugins installed' exits 0 whether or not anything is there,
so the answer comes from its output.

Reproduced both halves against Packer 1.16.0 to be sure which case this is:
with no plugin installed, a legacy JSON template gives "Failed to initialize
build openstack ... unknown by Packer", while the HCL2 template gives "Missing
plugins ... Did you run packer init for this project?". Only the first matches
the reported failure.

S607 joins the ignore list: resolving packer and ssh-keygen through PATH is
deliberate, so that a chosen packer build or an activated virtualenv is the one
that runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
trondham and others added 7 commits September 20, 2026 18:20
One cron job per image is hard to keep track of: each has its own randomised
schedule, and nothing ties a run together or notices that one of them failed.
This runs the generated build scripts one after another instead.

Categories are data rather than code. The build order, the pattern that
matches a build script name, the template directory imagebuilder should use
and the option that selects the category all live in one place, so adding a
kind of image does not mean another array, another flag and another loop.
Selection is a single map from image name to category, shared by discovery
and -i, so the two cannot disagree about what a name means.

Built for unattended use: it exits non-zero if any build failed, naming them,
and an unknown option or an unusable -t is an error rather than a quiet
success. A typo in -i is rejected before anything is built, instead of running
a path that does not exist and leaving a report file behind for an image
nobody has. Output drops its colours when stdout is not a terminal, so a cron
log does not fill with escape sequences. A flock means two runs cannot compete
for the same temporary security groups and keypairs, taken after -l and -h so
that looking at the list never blocks.

Windows images are listed and selected but not built yet, as before. The paths
are overridable so the script can be exercised away from a builder node.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Downloads died on a builder node with "terminated by signal 9" just after
Glance returned the image. It was the OOM killer.

openstacksdk's download_image defaults to stream=False, which asks requests
for the whole response body up front. The chunked writing that follows is
then only copying from memory to disk: the entire image is already resident.
A 10 GiB image needs upwards of 20 GiB of RAM that way. Measured on a 400 MiB
body, peak RSS is 837 MiB with stream=False against 34 MiB with stream=True.

This came in with the move off the glance CLI. That CLI streamed to disk, and
the API call that replaced it took the default and did not.

The upload path in bootstrap is unaffected: openstacksdk hands create_image an
open file object, which requests streams.

The test for this asserts the argument rather than the behaviour, since a fake
cannot run out of memory, and it was checked to fail without the fix. The
earlier fake accepted any keyword arguments and so could never have caught
this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
A build killed outright cannot run its own cleanup. The OOM kills during
image download left an imagebuilder-<uuid> security group and keypair in the
project every time, and nothing was ever going to remove them: SIGKILL cannot
be caught, and SIGTERM does not run a finally block either.

Every build now starts by clearing out what earlier runs could not. Two things
keep this from deleting something in use. Only names shaped exactly like
imagebuilder- followed by 32 hex characters are considered, so a security
group called default or somebody's keypair is never a candidate. And nothing
younger than a day is touched, because the builder nodes share one project and
a sweep here would otherwise delete resources out from under a build running
on another node.

Images are deliberately left alone. 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, since something else already tidies that.

Failures during the sweep are logged and the build carries on: not being able
to list keypairs is no reason to refuse to build. Removals are logged at
warning level so they show up without -v.

--no-sweep turns it off, for when something needs to be preserved for
inspection.

Nova does not always report created_at when listing keypairs, so the age is
fetched for the individual keypair if the listing did not carry it. Anything
whose age still cannot be read is left alone rather than guessed at.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Three claims had drifted out of date over the course of this branch. The
environment section still listed OS_IDENTITY_API_VERSION and OS_NO_CACHE as
required, which is the bug that was fixed, and still said the glance CLI must
be on PATH after it stopped being used. The architecture section described
main() as a straight-line driver dispatching through getattr, which the split
into run_build and run_bootstrap replaced. And bootstrap was described as
matching a checksum by substring, which the filename-aware parsing replaced.
-S built the windows images along with the standard ones.

A windows image installs two build scripts: winsrv_<version>_wrapper, which
cron calls, and winsrv_<version>, which that wrapper calls. Only the first
matched the pattern for the windows category, so the second fell through to
the default and was listed and built as a standard image.

That is worse than an unwanted build. The inner script sshes to the windows
build host and starts packer there, so -S was kicking off windows builds
outside their schedule, twice over, while the wrapper that does the rest of
the work around them never ran.

Discovery now skips a winsrv_ script that is not a _wrapper. It is not an
image in its own right and nothing here should pick it up. -i is checked
against what discovery found rather than against the filesystem, so the inner
script cannot be reached that way either.

With the real set of build scripts, -S now builds 17 images rather than 19,
-B builds 2, -G builds 5, and -W still finds both wrappers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
build-images.sh had no automated checking at all. The bug where -S built the
windows images was caught by hand, and while shellcheck would not have found
that one either, it is the obvious floor to put under a script that is about
to become the only entry point for every build.

Its own job rather than a step in the matrix, since the shell has nothing to
do with the Python version and there is no point running it three times.

Only build-images.sh is linted. The provisioning scripts under scripts/ run on
the guest rather than here, and produce around a hundred findings between
them, so including them needs a cleanup first rather than a blanket ignore.

The runner has shellcheck 0.9.0, which reports the deliberately unused
palette helpers as SC2317 where 0.11 calls the same thing SC2329. The file
level directive now lists both codes, so it is quiet on either version;
checked against both rather than assumed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
--no-sweep becomes --sweep. Removing resources that the current run did not
create is not something to do by default on every build: it should happen
because something asked for it.

build-images.sh gets -s for the same purpose. It cannot pass the flag along,
because it runs the generated build scripts and those take no arguments of
their own, so it asks through the environment instead: IB_SWEEP, alongside the
IB_TEMPLATE_DIR and IB_DOWNLOAD_DIR that are already read from there.
IB_SWEEP is exported either way, so a stray value in whoever's environment
cannot turn a sweep on from outside. IB_SWEEP=false does not enable it, which
is why the value is parsed rather than merely tested for being set.

Note the case: -S selects the standard images and -s asks for the sweep. The
help says so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A4Y4JQ39uQMSZcBYoTE84y
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file do-not-merge enhancement python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant