Skip to content

all: document the shared-value role trap - #310

Merged
berendt merged 1 commit into
mainfrom
docs-groupvars-guidance
Sep 14, 2026
Merged

berendt merged 1 commit into
mainfrom
docs-groupvars-guidance

Conversation

@ideaship

@ideaship ideaship commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

What

Documentation only — all/README.md. No behaviour change, no variable touched.

Why

The 2026.1 mirror re-sync shipped two regressions of the same shape, and this
file did not describe that shape:

Both are fixed. This records what they have in common, while it is concrete.

The gap being filled

all/ is one tree serving every supported release, but each release's roles
come from its own kolla-ansible image
. So when upstream changes a value and
the code that reads it in the same commit, mirroring the value alone splits the
pair — the new value meets the old consumer.

The README already said the detector cannot catch "a value that must differ by
release". It did not say why, and it did not describe the variant that is
easy to miss:

what changed why the usual check misses it
openstack_auth the value's meaning — 2026.1 reads the rest from a templated clouds.yaml older releases do not have nothing; a cross-release value comparison would have caught it
designate_backend_external the value's type, truth value preserved ("no"false) both are false under | bool, so every boolean-equivalence check calls it inert — but the older role compares against the string, and false == 'no' is False. (The bare string "no" is truthy; it is the filter that makes them agree.)

So the new text states the rule: a notation change is inert only if every
consumer coerces.
Search the older releases' roles for a literal comparison
against the key, and evaluate it against both values rather than stopping at
the match. Of the three keys that matched in the 2026.1 sweep, only one
inverted.

Also documented

Two things that cost real time during the investigation:

  • Resolving a variable by hand needs more than the command this file
    recommends.
    An ad-hoc ansible … -m debug -a "var=…" has no filter
    plugins, no vault and no site inputs. Each missing piece fails differently,
    and a missing secret prints VARIABLE IS NOT DEFINED! for a key that is
    defined — the opposite of the truth. The working invocation and the three
    failure modes are now written down.
  • A resolved value proves delivery, not consumption.
    om_rabbitmq_qos_prefetch_count resolves on every release and is rendered
    into no config file at all, because the template that would emit it sits
    behind a guard OSISM turns off.

Plus a worked example for gating a non-scalar value (the openstack_auth
gate from #307), including the caveat that sync-mirror's --retain cannot
verify a mapping-valued gate today — it only parses quoted scalar literals, so
such a gate takes --retain-unverified.

Scope

Guidance, not an incident write-up. The two regressions appear as one clause
each with a PR reference; the forensics stay in the PRs. Deliberately not
covered here: where to read an effective deployed value on a live cluster —
that is diagnosis workflow rather than group_vars layering, and stating it
correctly needs more caveats than it earns in this file.

Drive-by

Where a variable goes named the supported range in a parenthetical, and it
was stale — it still said 2024.1–2025.2. Rather than update it to a value that
moves every release, the parenthetical is gone: the file already states that
the range is derived, not declared (check-drift.py globs
latest/openstack-*.yml in osism/release), so this now points there, the way
the openstack_release note two sections up points at 001-common.yml.

🤖 Generated with Claude Code

@ideaship
ideaship force-pushed the docs-groupvars-guidance branch 3 times, most recently from e4fa040 to faa00a0 Compare September 12, 2026 11:03
@ideaship ideaship self-assigned this Sep 12, 2026
The 2026.1 mirror re-sync shipped two regressions of the same shape, and
this file did not describe that shape. Both are fixed -- #307 and #309 --
so record what they have in common while it is concrete.

all/ is one tree serving every supported release, but each release's
roles come from its own kolla-ansible image. When upstream changes a
value and the code that reads it in one commit, mirroring the value alone
splits the pair: the new value meets the old consumer. The README
described the case where a value must differ by release; it did not say
why, and it did not cover the variant that survives the usual check.

openstack_auth is the first shape -- the meaning moved, since 2026.1
reads five of the six keys from a clouds.yaml older releases do not have.
designate_backend_external is the second: "no" became false, the truth
value was preserved, and every boolean-equivalence check therefore called
it inert, while the older role compares against the string and
false == 'no' is False. Note "no" is truthy on its own; it is | bool that
makes the two agree. So state the rule: a notation change is inert only
if every consumer's behaviour is preserved. Search the older releases'
roles for a comparison against the key and evaluate it against both
values.

Document two further things that cost time in that investigation:

- Resolving a variable by hand needs the filter-plugin path, the vault
  secrets and the site inputs. Each missing piece fails differently, and
  a missing secret prints VARIABLE IS NOT DEFINED! for a key that is
  defined. The result is also synthetic -- every -e replaces a real
  input -- and a NOT DEFINED cannot distinguish an undefined key from an
  undefined dependency; the raw definition settles that, which is what
  ansible-inventory --host is good for.
- A resolved value proves delivery, not consumption.
  om_rabbitmq_qos_prefetch_count resolves on every release and, under
  OSISM's default profile, reaches no config file at all.

Add a worked example for gating a non-scalar value, using the
openstack_auth gate, with the note that sync-mirror's --retain cannot
verify one: it parses only quoted scalar literals, so such a key takes
--retain-unverified.

Drop the parenthetical naming the supported range in Where a variable
goes. It was stale before this branch touched it, and naming a range that
moves every release would only go stale again. The file already says the
range is derived rather than declared -- check-drift.py globs
latest/openstack-*.yml in osism/release -- so point there instead, the
way the openstack_release note two sections up points at 001-common.yml.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Roger Luethi <luethi@osism.tech>
@ideaship
ideaship force-pushed the docs-groupvars-guidance branch from faa00a0 to 5f37173 Compare September 14, 2026 08:08
@ideaship
ideaship marked this pull request as ready for review September 14, 2026 08:23
@berendt
berendt merged commit d563300 into main Sep 14, 2026
2 checks passed
@berendt
berendt deleted the docs-groupvars-guidance branch September 14, 2026 08:23
@github-project-automation github-project-automation Bot moved this from New to Done in Human Board Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants