Add kolla group_vars drift checks#2569
Merged
Merged
Conversation
ideaship
changed the title
This was referenced Jul 9, 2026
The release_vs_manager remediation told operators to re-render environments/manager/images.yml or allowlist the entry. In practice the rendered manager images.yml in osism/testbed is regenerated daily by the update-manager-images GitHub Action, which opens a "chore: update versions" PR; the common cause of this drift is simply that such a PR is open but not yet merged (e.g. ara-server 1.7.5 vs the release pin 1.7.5-r1). Lead the remediation with "merge that open PR" so the usual fix is the first thing suggested, keeping re-render and allowlist as fallbacks. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Add a severity field ("actionable" | "advisory") to DriftEntry and use
it in the driver's exit logic. role_shadows now marks DORMANT findings
-- role-default tag pins that are shadowed by the rendered images.yml,
so the release pin wins at deploy and they only need syncing "when
convenient" -- as advisory.
The driver splits non-allowlisted findings by severity: an advisory-only
result collapses to exit 0, while actionable drift and stale allowlist
entries still return 1. actionable is the complement of advisory
(severity != "advisory") so an unknown or mistyped value fails closed
and still returns 1. Advisory findings stay visible in the report body,
the summary line (new advisory count), and the default JSON output,
which now emits all non-allowlisted findings so their severity key is
carried.
This clears the periodic release-tox-drift job to green once only
dormant role-default lags remain, without masking real drift. No CI
change is needed: the behaviour rides entirely on the tool's exit code,
so tox.ini and .zuul.yaml are untouched.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Roger Luethi <luethi@osism.tech>
The role-defaults parser fed by role_shadows and role_unpinned skipped
any <alias>_tag whose value contained a Jinja2 expression. That skip was
over-broad: it conflated two different patterns. A genuinely undeclared
pin, e.g. {{ lookup('vars', X, default=Undefined) }}, is correctly
skipped because there is no pin to compare. But a concrete pin behind
one hop of indirection, <alias>_tag: "{{ <alias>_version }}", where
<alias>_version holds the actual pin, was wrongly skipped too.
Roles using the second pattern were invisible to both plugins. Most
notably opentelemetry_collector: base.yml pins 0.155.0 while the role
default resolves to 0.136.0, and because the role is unwired the role
default is what deploys -- a silent stale-image drift the parser never
surfaced. substation (latest in both role and base.yml) was likewise
invisible to the rolling/unpinned check.
Resolve the single hop when <alias>_tag is a lone reference to its own
<alias>_version (the value is exactly "{{ <alias>_version }}", ignoring
whitespace): read <alias>_version from the same defaults file and use it
as the tag when concrete. Scoping to the same-file, same-alias,
single-hop reference addresses the design concern about pulling in
non-image *_version vars -- only a _version an image _tag explicitly
points at is resolved. Composed tags ("{{ foo_version }}-alpine"),
references to other vars, non-concrete _version values, and the
Undefined-lookup pattern all stay skipped.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Roger Luethi <luethi@osism.tech>
kolla_groupvars_missing diffs upstream kolla-ansible group_vars/all against everything OSISM supplies to the container's group_vars/all. osism_groupvars_keys counted two supply paths (osism/defaults all/*.yml and the rendered versions.yml) but not a third: the per-release container-image-kolla-ansible overlays/<release>/kolla-ansible.yml the Dockerfile bakes into group_vars/all at deploy time. These overlays carry the per-release group_vars upstream removed from group_vars/all but an older supported release's role still references -- the release-specific analogue of a defaults/all backward-compat entry. Any var OSISM supplies only via overlay false-positived as missing. At 2024.1/2024.2 that is the six external-Ceph keyrings (ceph_cinder_keyring and friends; the 2024.2 cinder role references ceph_cinder_keyring with no fallback) and six deploy-time swift vars (glance_backend_swift, the swift fqdn/base-endpoint/proxy-port vars) -- twelve pure false positives. Union the top-level keys of every overlays/<release>/kolla-ansible.yml into osism_groupvars_keys, enumerated from the overlays/ tree so the set tracks whatever releases carry an overlay (a repo with no overlays/ dir contributes nothing) and the overlays/release/<ver>/ image-release tree is skipped. Update the plugin docstring/INPUT_FILES to list the third path. Unioning across all overlays matches the check's existing union-over-supported-releases approximation; a stricter per-release diff is a possible follow-up. Closes the twelve false positives (verified with check-drift.py --no-allowlist: raw drift 26 -> 14). All 264 drift tests pass. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
The plugin's docstring and REMEDIATION described two resolutions for a missing upstream group_var that no longer match how OSISM handles them: they listed "an upstream typo" as a valid allowlist reason, and offered "allowlist it if the service is not shipped/supported" as an alternative to mirroring. Neither holds. An upstream typo is mirrored into all/001-kolla-defaults.yml verbatim like any other upstream key (so it is supplied and never surfaces here), while the correct name is set in 099. And OSISM supplies the full upstream union rather than judging whether a service ships: a var upstream dropped from the newest release but kept in an older supported one goes to a self-retiring all/010-<last-release>.yml, not the allowlist. Rewrite the docstring and REMEDIATION accordingly: a missing key is always supplied (001 if the newest supported release defines it, else 010-<L>.yml for the older releases that still do); the metalbox example now motivates "always supply" rather than "allowlist the not-shipped"; the allowlist is reserved for a var OSISM already supplies another way the detector cannot see. Help text only; no logic change. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
kolla_orphan_config only reports a removed service's <svc>_image / <svc>_tag when OSISM still defines its enable_<svc> flag and that flag is an enablement orphan. Services upstream removed for which OSISM never defined an enable flag (monasca, panko, vmtp, rally, storm, kafka, qdrouterd, elasticsearch, kibana, ...) leave dead image/tag blocks in the kolla image catalogue that nothing flags. Add an image-catalogue-driven check: for each *_image / *_tag var in osism/defaults all/*images-kolla*.yml, flag it when its name is absent from upstream kolla-ansible role defaults (roles/*/defaults/main.yml *_image / *_tag) across the union of supported releases. Pure variable name set-diff -- no image-name resolution, alias chains, or service-id inference -- with the same test applied independently to both suffixes. A shared enablement.upstream_image_tag_keys reads each role's defaults once. Both an empty catalogue match and an empty upstream union fail loud. OSISM-built images with no upstream role (mariabackup, tempest, the prometheus haproxy exporter, dcap) are kept via drift-allowlist.yml; stale-allowlist detection keeps that set honest. Genuine orphans are reported as actionable drift by design. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
kolla_mirror_verbatim must compare 001-kolla-defaults.yml's key->value map against upstream group_vars/all (the existing helpers are key-set-only and union across every all/*.yml, so they cannot see value drift or isolate the 001 mirror file). Add groupvars_values / upstream_groupvars_values / osism_mirror_values (001 only) / osism_supply_excluding_mirror / dropped_key_release_map next to the existing loaders, reusing their body readers. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
New kolla-group plugin enforcing that osism/defaults all/001-kolla-defaults.yml is a verbatim copy of upstream kolla-ansible group_vars/all at the newest supported release. For each deviation it prints the exact destination: mirror into 001 (missing key), restore upstream value and move the OSISM value to 099-* (value differs), or remove from 001 and route it when upstream no longer defines the key -- a self-retiring 010-<last-release>.yml if an older supported release still defines it, 099-* custom-features if OSISM-invented, or delete if another layer already supplies it. Never allowlists a group_var. Not yet registered (next commit). Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Add the plugin to the import block and to KOLLA_PLUGINS right after kolla_groupvars_missing (both guard the enabled stage), enable it in the default drift-config.yml, and extend the lifecycle-order and registered-plugin registry tests to match. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Add the plugin's section to the kolla drift-check reference, in enabled-stage order after kolla_groupvars_missing: the 001 verbatim-mirror invariant, the three deviation shapes and their guided destinations, and the reads/fix lines. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
groupvars_home(key, newest, newest_keys, dropped_map) returns
(path, note) identifying which osism/defaults file a missing
kolla-ansible group_var belongs in:
- all/001-kolla-defaults.yml when the upstream newest release
still defines the key (the current defaults mirror file)
- all/010-{L}.yml when the key was dropped by the newest release
but still defined in older supported release L (a self-retiring
backward-compat file, named for the last release carrying it)
- None as a defensive fallback when the key appears in neither
set (caller falls back to static text)
The 001/010 layering is the osism/defaults file layout: 001 mirrors
upstream verbatim, 010-* are self-retiring files for vars that older
supported releases still need but the newest upstream dropped.
Four pure unit tests cover all four cases: newest wins, dropped-only,
newest wins even when key is in both sets, and neither returns None.
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Roger Luethi <luethi@osism.tech>
In the B-dropped branch (a 001 key that upstream newest dropped but
an older supported release still carries), obtain the destination
file path from enablement.groupvars_home instead of formatting
all/010-{L}.yml inline.
The routing rule — a dropped key goes to all/010-{L}.yml where L is
the newest supported release still defining the key — now lives in
one place (groupvars_home), shared with kolla_groupvars_missing.
The summary and remediation text are byte-identical to before; L is
still computed locally for the human-readable strings.
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Roger Luethi <luethi@osism.tech>
run() now computes a per-entry summary and remediation that names
the exact osism/defaults file a missing key should go into:
- all/001-kolla-defaults.yml when the key is defined at the newest
supported release (the current defaults mirror file)
- all/010-{L}.yml when the key was dropped by the newest release
but an older supported release L still defines it (a self-retiring
backward-compat file, deleted when L EOLs)
The routing uses enablement.groupvars_home, the same helper that
kolla_mirror_verbatim uses for its B-dropped classification, so the
"which file does this var belong in" rule lives in one place.
Keys sharing a destination file carry identical (summary, remediation),
which causes the report renderer to group them into one block. If
groupvars_home returns None (defensive fallback), the entry falls back
to the static module-level SUMMARY/REMEDIATION.
The empty-range guard is preserved: missing_keys() is called first and
raises SourceError before any sorted(...)[−1] indexing can occur.
Three new tests cover: key defined at newest goes to 001, key dropped
by newest goes to 010-{L}, and two keys with different homes produce
distinct remediations so the renderer emits separate blocks.
Assisted-by: Claude:claude-sonnet-4-6
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
3 participants
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 drift-detection tooling for OSISM's kolla
group_vars: new checks that flagwhere
osism/defaultsall/has diverged from upstream kolla-ansible, plussupporting fixes. This PR is the tooling only and can land on its own; the
allowlist reconciliation follows in a follow-up PR.
kolla_mirror_verbatimcheck — enforces thatosism/defaultsall/001-kolla-defaults.ymlis a verbatim copy of upstream kolla-ansiblegroup_vars/allat the newest supported release, printing the exact destinationfor each deviation (
001, a self-retiring010-<release>.yml,099-*, or delete).kolla_image_orphancheck — flags<svc>_image/<svc>_tagcatalogueentries with no upstream kolla-ansible role.
kolla_groupvars_missingenhancements — overlay-aware supply counting,per-key destination in the remediation output, corrected help text.
group_varsloaders, advisory-severity exittier,
_tag→_versionrole-pin resolution,release_vs_managerauto-PR pointer.Notes
helpers → the coupled
kolla_groupvars_missingper-key-destination edit). Everycommit builds and passes the test suite individually (bisectable).
_get/http.py/ progress output) thatlanded in Archive backend: fetch repos as tarballs for remote drift runs #2570 (merged).
group_varsdrift until thecorresponding
osism/defaultschanges land — expected; this PR adds thedetectors, not the fixes.
Related