Add drift-detection tooling (check-drift) to the release repo#2561
Merged
Conversation
8154aeb to
12c2956
Compare
Introduce the osism_drift framework that later plugins build on. It exists to catch OSISM version-chain drift automatically instead of at deploy/run time. - source: read a file or list a directory from any OSISM repo, transparently from a local checkout (--base-dir) or, with none, from GitHub raw + API. Pinned upstream repos are read at explicit historical refs; consumer repos are read from the working tree / remote head. - model.DriftEntry: one disagreement between an authoritative source and a consumer, with optional per-entry summary/remediation overrides. - driver: selects enabled plugins (or --plugin), applies the allowlist, renders a grouped lifecycle-ordered report (text or JSON), flags stale allowlist entries, and exits 0 clean / 1 drift-or-stale / 2 input error so it maps onto a CI gate. It is plugin-agnostic: plugin groups and report headers are passed in. - config + allowlist loaders: per-plugin enable flags; every allowlist entry needs a reason; unmatched entries are stale. - demo: illustrative report built from each plugin's live metadata, short-circuiting before any config/source/allowlist load. Plugins, their config entries and allowlist entries follow in later commits. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Image-version-pin checks. An image's version flows release base.yml → rendered manager images.yml → role defaults; these plugins find where the chain disagrees. - release_vs_manager: release pin vs the rendered manager images.yml. - role_shadows: role-default <alias>_tag that disagrees with the release base.yml pin. A LIVE finding (alias not overridden by images.yml.j2) is fixed by wiring the alias into the manager render template images.yml.j2 so the release pin governs -- NOT by editing the role default, which must never govern the deployed version. - role_unpinned: role-default tag with no canonical release pin. - image_orphan: a manager-template image alias no role or playbook consumes. Scans ansible-collection-services/roles and ansible-playbooks-manager/playbooks; only .yml/.yaml/.j2 files. role_shadows and role_unpinned skip stream-resolved aliases (ceph_ansible, kolla_ansible, the osism family) that resolve via <name>_version at deploy time. Allowlist: rolling-release osism images (release_vs_manager) and the OSISM CI/zuul stack whose versions live in the zuul role, not the release (osism/issues#1397). Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Checks for the kolla container-image version chain: openstack/kolla defines images (docker/ dirs) -> container-images-kolla pins concrete versions (SBOM map) -> container-image-kolla-ansible consumes them via versions.yml.j2. Plugins are registered in lifecycle order (enabled -> built -> version-pinned -> deployed): - kolla_enablement_orphan: OSISM enable_* flags whose service is gone from upstream across the supported release range. - kolla_orphan_config: leftover config vars of those dead services. - kolla_secrets_orphan: OSISM kolla secrets with no upstream counterpart. - kolla_enablement_build: OSISM-enabled, upstream-buildable services missing from the release build set. - kolla_version_chain_upstream: kolla docker/ services with no versions['K'] line in the kolla-ansible template. - kolla_version_chain_inner: versions['K'] keys the SBOM map never emits (inert pins); classified add-vs-remove by whether OSISM builds the image. - kolla_inventory: upstream multinode groups absent from the OSISM 50/51-kolla inventory (configparser, strict=False for dup hosts). Enable-defaults layout changes at 2025.2 (monolithic group_vars/all.yml -> split group_vars/all/*.yml). zun ships a kolla image but OSISM does not build it; valkey is intentionally left to flag ahead of enablement. Allowlists use exact matches for the underscore-namespaced plugins and prefix (- / :) only for inventory group names. This commit also lands the shared driver-group/demo test and the report test that renders a kolla plugin, so the full suite is green here. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Reference docs for the two check groups, presenting each plugin in lifecycle order under a uniform template, plus an "adding a plugin" guide (allowlist.apply idiom) and the exit-code contract. - docs/check-drift-image.md - docs/check-drift-kolla.md Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Add the tox envs and Zuul jobs to run the tooling. - tox -e drift-test: the unit suite (pytest tests/). - tox -e drift: the detector, defaulting to --group all so the periodic job needs no posargs; --group stays required on the direct CLI. - .zuul.yaml: release-tox-drift-test runs in check and periodic-daily; release-tox-drift runs in periodic-daily only, because it inspects live upstream state and is unfit as a PR gate. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
The three kolla plugins that inspect enable_* flags (kolla_enablement_orphan, kolla_enablement_build, and kolla_version_chain_inner) each read them from a single hardcoded file, osism/defaults all/099-kolla.yml. That couples the detector to one file layout: if a future osism/defaults reorganization moves an enable_<id> into another file (for example a per-service split mirroring upstream kolla-ansible's 2025.2 group_vars/all/*.yml restructuring), the flag would silently drop out of the comparison and produce false negatives or false positives. Add a shared helper, enablement.osism_enable_flags(config), that reads the union of enable_* flags across every osism/defaults all/*.yml file (skipping non-.yml entries), mirroring how kolla_orphan_config already globs the directory. Switch all three plugins to it and drop their per-module _ENABLE constants. The comparison now depends on variable names, not on which file a flag lives in, so the kolla detector is immune to any defaults file reorganization. Behaviour is unchanged for the current layout, since 099-kolla.yml is today the only defaults file carrying enable_* flags. Update INPUT_FILES and docs/check-drift-kolla.md to reflect all/*.yml. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
The range-aware kolla plugins read osism/defaults at its single tip and compare against the union of upstream refs across the supported release range, but the docs never said why. Without that rationale a plugin author cannot tell whether union, intersection, or newest-release is correct for a new check. Add a "Release model and why the range-aware checks use unions" subsection documenting the model: osism/defaults is a single-tip repo (dated tags, no per-release branches); a defaults tag is baked into each osism/release manifest at release-cut time; and one defaults_version is shared across every supported OpenStack release in a manager version, so that one snapshot must be correct for all of them at once. Hence both directions compare against the union — removal-direction to avoid false orphans, add-direction because the tip must satisfy every release. Also cross-reference it from the range-aware note in kolla_enablement_build. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
The three defaults<->upstream kolla plugins all run the removal direction: they flag things OSISM ships that upstream no longer defines. Nothing checked the opposite (add) direction, so an upstream global var that osism/defaults never mirrored went unnoticed until it broke a deploy. This is the class behind the 2025.2 keystone_listen_port failure: added upstream in the uWSGI migration, never picked up by osism/defaults, so the keystone upgrade aborted on an undefined variable. Add kolla_groupvars_missing, the add-direction mirror of kolla_enablement_orphan. It diffs key spaces: the union of upstream kolla-ansible group_vars/all top-level keys across the supported release range MINUS everything OSISM supplies to the container's group_vars/all. OSISM supplies from two paths, both counted: osism/defaults all/*.yml (via the generics gilt overlay) and the rendered container-image-kolla-ansible versions.yml (openstack_release, the kolla_*_version pins, ...) -- a var supplied only by the latter must not false-positive as missing. Union across releases because one osism/defaults snapshot is baked per manager version and must satisfy every supported OpenStack release at once (see the release-model note in docs/check-drift-kolla.md); top-level group_vars only, keys compared verbatim (an Ansible var name is an exact identifier). Deliberate divergences (an upstream typo, a var OSISM supplies another way) are carried in the allowlist with a reason. Extract the shared readers behind the enablement helpers - top_level_keys, plus _osism_defaults_bodies and _upstream_groupvars_bodies feeding both the enable-flag and the group_vars key sets - so the new check reuses the monolithic/split group_vars probe and the all/*.yml union. Register the plugin in the config stage of the kolla lifecycle, right after kolla_enablement_orphan, enable it in the default config, and document it in docs/check-drift-kolla.md. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
kolla_groupvars_missing flags eutron_external_interface, an upstream kolla-ansible group_var that OSISM does not mirror. It is an upstream typo for neutron_external_interface (which OSISM defines correctly), so mirroring it would be wrong. Allowlist it with a reason pointing at the upstream fix under review; once that merges the key disappears upstream and the check reports this entry as stale, prompting its removal. Waiting for: https://review.opendev.org/c/openstack/kolla-ansible/+/985031 Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
release_to_ref re-probed upstream refs on every call, so each kolla plugin that resolves a release independently repeated the same ref_exists commits-API requests -- the docstring's "resolved once per run" was aspirational. Cache resolved refs on a new Config.ref_cache dict keyed by (repo, release). Config is frozen, but mutating the dict it holds is allowed; a fresh Config per run (and per test) isolates the cache. The release_refs override path is unchanged. Measured effect on an unauthenticated remote --group kolla run: 42 -> 32 api.github.com requests (10 redundant ref_exists probes removed), which matters against GitHub's 60/hr per-IP unauthenticated ceiling. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Remote reads were always unauthenticated, capping the drift detector at GitHub's 60/hr per-IP limit. When GITHUB_TOKEN (or GH_TOKEN) is present in the environment, send it as a bearer token on every raw and API request, lifting the ceiling to 5000/hr; when absent, behaviour is unchanged. The Zuul periodic-daily drift job carries no token today, so this is a no-op there; it helps local/developer runs and any future authenticated CI. remote.github_raw / remote.github_api are configurable (a mirror, proxy, or test server), so the token is attached only when the request host is github.com / api.github.com / raw.githubusercontent.com. This keeps an ambient token from leaking to a non-GitHub endpoint someone pointed the base URLs at; the default config targets public GitHub, so authenticated runs there are unchanged. _auth_headers takes the request url; all seven call sites pass it. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
A remote drift run makes many GitHub requests, and an unauthenticated run shares a low per-IP hourly budget it can exhaust. When that happened the source layer raised a bare "HTTP 403 fetching <url>", giving the user no clue it was throttling rather than a real failure, nor that setting a token would help. Add _rate_limit_hint(), which recognizes GitHub's throttling signals -- 403/429 with X-RateLimit-Remaining: 0 (primary limit) or a Retry-After header (secondary limit) -- and builds an actionable hint: when to retry (from X-RateLimit-Reset or Retry-After) and, for unauthenticated runs, advice to set GITHUB_TOKEN/GH_TOKEN. A plain 403 with no such marker (e.g. a permission refusal) is left as an unadorned HTTP error so the hint never misleads. Route all remote request sites through a shared _http_error() helper so every one of them gains the hint instead of only one. The quota figures are not hardcoded: the message echoes the X-RateLimit-Limit value the server reported for that request, so it stays accurate whenever the script runs even if GitHub changes its limits. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds
check-drift.py, a plugin-based drift detector for OSISM's image andversion wiring, and wires it into CI. It catches divergence — when it is
introduced — before it breaks a deploy or a gate, across two check groups
(
--group image | kolla | all).Image version pins — follows an image's version down the chain
release
base.yml→ rendered managerimages.yml→ role defaults:release_vs_manager— the rendered managerimages.ymldisagrees with the release pinrole_shadows— a role-default tag shadows the release pin (LIVE = the role default actually deploys; DORMANT = the release pin wins)role_unpinned— a role-default tag with no release pinimage_orphan— a manager image alias is emitted but consumed by no role or playbookKolla container-image toolchain — divergence between the OSISM consumer repos
(
defaults,release,generics, the container-image repos) and upstreamopenstack/kolla/openstack/kolla-ansible:kolla_enablement_orphan— OSISM enables a service upstream no longer defineskolla_groupvars_missing— an upstreamgroup_vars/allvar OSISM's defaults never mirrored, so it is undefined at deploy/upgrade (the add-direction mirror ofkolla_enablement_orphan; the class behind the 2025.2keystone_listen_portbreak)kolla_orphan_config— dead companion vars / image blocks for an orphaned servicekolla_secrets_orphan— an OSISM kolla secret with no upstream counterpartkolla_enablement_build— an enabled, upstream-buildable service missing from the build setkolla_version_chain_upstream— an upstreamdocker/service with no version-pin linekolla_version_chain_inner— a referenced version pin that resolves to nothing (inert pin)kolla_inventory— an upstream inventory group missing from the OSISM50/51-kollafilesThe framework lands first (empty registry, flags nothing on its own), then the
plugins in lifecycle order, each reviewable in isolation. Sources resolve
remotely (GitHub) by default, so the detector runs anywhere including CI;
--base-dirreads local checkouts. An allowlist marks intentional exceptions(each requiring a reason); entries that match nothing are reported stale, so
suppressions can't silently rot. Exit codes: 0 clean / 1 drift or stale / 2 input
error. Full reference:
docs/check-drift-image.md,docs/check-drift-kolla.md(incl. the
osism/defaultsrelease/bake model and why the range-aware checkscompare against the union of supported releases).
Commits (each independently green — unit tests pass at every commit)
drift: add plugin framework and driverdrift: add image-drift pluginsdrift: add kolla-drift pluginsdrift: add drift-detection docsdrift: wire tox and Zuul CIkolla: read enable flags from all defaults files— make the enable-flag readers layout-agnostic (readenable_*from the union ofall/*.yml, not just099-kolla.yml), so a defaults reorganization can't silently drop a flagdocs: explain defaults bake model and unionskolla: add kolla_groupvars_missing drift check— the add-direction group_vars check (both OSISM delivery paths counted:osism/defaults+ the renderedcontainer-image-kolla-ansibleversions.yml)kolla: allowlist eutron_external_interface typoCI
release-tox-drift-test— the unit suite (tox -e drift-test, 248 tests).Runs in check (gates this PR) and periodic-daily.
release-tox-drift— the detector (tox -e drift,check-drift.py --group all). Runs periodic-daily only: it reads live upstream state, soit's unfit as a PR gate. It exits non-zero when drift exists, so the daily job
goes red with the report in its log.
What this finds in practice
Run against the live OSISM repos today, the detector reports 73 actionable
items, 42 allowlisted, 0 stale (115 total):
ara_server), 3 LIVErole-default shadows that actually deploy (
dnsmasq,scaphandre,stepca),9 DORMANT lags, 4 unpinned tags (
httpd,httpd_data,kepler,thanos_sidecar), and 3 orphaned manager aliases (nginx,osism_netbox,registry).this branch). The layout-agnostic enable-flag reader scans
enable_*acrossall
osism/defaultsall/*.ymlfiles (not just099-kolla.yml), surfacing9 enablement orphans (
auditd,chrony,openstack_exporter,karbor,qinling,searchlight, threehorizon_*) and their 9 dead companion vars— some are removed-upstream projects to drop, some are OSISM inventions to
allowlist. The new add-direction
kolla_groupvars_missingsurfaces 35upstream
group_vars/allkeys OSISM never mirrored — theosism/defaultsre-sync backlog behind the 2025.2
keystone_listen_portupgrade break, incl.enable_ironic_pxe_filterand thenova_database_*/nova_cell0_database_*batch.
eutron_external_interfaceis allowlisted (upstream typo forneutron_external_interface, awaitinghttps://review.opendev.org/c/openstack/kolla-ansible/+/985031).
Earlier, the removal-direction kolla checks already drove a wave of cleanups
across the consumer repos (kolla actionable-drift 81 → 0 at the time, all merged);
this branch's layout-agnostic reader and new add-direction check now surface a
further backlog — chiefly the
osism/defaultsre-sync thekeystone_listen_portbreak called for:
It self-audits its allowlist too: an entry that no longer suppresses real drift is
a hard error (exit 1), so suppressions can't silently rot.
Because the detector is daily-only, this output does not appear in this PR's
checks. Full
release-tox-driftreport against current upstream(
check-drift.py --group all):How to read it
role_shadowslag but the release pin governs atdeploy (low priority); LIVE
role_shadows(dnsmasq,scaphandre,stepca)actually deploy the role default (wire into
images.yml.j2);role_unpinned(
httpd*,kepler,thanos_sidecar) have no release pin;image_orphan(
nginx,osism_netbox,registry) are emitted but consumed nowhere.kolla_enablement_orphan+kolla_orphan_config:stale
enable_*flags and their companion vars for services upstream dropped;drop the removed projects, allowlist the OSISM inventions.
kolla_groupvars_missing: theosism/defaultsre-sync backlog. The right fix for most is to mirror the upstream var into
osism/defaults(harmless when the service is off, correct when an environmentenables it — e.g. metalbox turns on ironic, so
enable_ironic_pxe_filterbitesthere); allowlist only genuine divergences (the
eutrontypo).