Repository navigation
drift: close the plugin documentation gaps - #2696
Merged
Merged
Conversation
The kolla doc presents "The checks" as the complete roster and gives every plugin a "### Plugin: <name>" section, but kolla_source_ref_phase was merged without one, so the only description of it lives in its module docstring. Add the section, covering what the check compares (each openstack_projects ref in release latest/openstack-<release>.yml against the latest lifecycle phase opendev still publishes a tarball for), why it probes artifacts rather than branches, why a skipped ref is not a finding, and why an inconclusive probe reports the ref as unverified instead of clean. Also note that the plugin declares tarballs.opendev.org in EXTERNAL_HOSTS, so a --base-dir cannot stand in for it and the driver refuses a local-only run that selects it -- a property no other kolla plugin has, and one an operator hits as an error message rather than a documented rule. The four-stage framing in "The checks" describes a kolla service, and this check is about the release manifest rather than any one service, so it is introduced as sitting ahead of the stages instead of being folded into the numbered list. Assisted-by: Claude:claude-opus-5 Signed-off-by: Roger Luethi <luethi@osism.tech>
The mirror-layer series (74ac6e5..9edc160) made the 001 mirror a set of per-service files -- all/001-aodh.yml, all/001-common.yml, ... mirroring upstream's own split group_vars/all/*.yml names -- read as their merge in lexical filename order, and taught kolla_mirror_verbatim to name the matching per-service file as the destination for a key the mirror lacks. The docs still described a single all/001-kolla-defaults.yml, so a reader following them would look for a file the repository no longer has and would not know a missing key has a specific 001 file to go into. Describe the layer, the merge order and why it matters (upstream defines the database_* keys in both common.yml and database.yml and relies on database.yml winning), and the fallback to the layer glob for the monolithic upstream layout of 2025.1 and earlier, where there is no per-service home to name. Documentation only; no behaviour change. Assisted-by: Claude:claude-opus-5 Signed-off-by: Roger Luethi <luethi@osism.tech>
The "Adding a plugin" recipe -- the canonical one, which the image doc and the catalog doc both defer to -- listed the module contract, the registry entry, the config stanza and the test pattern, but never said to document the check. The one step that was missing from the recipe is the one that got missed: kolla_source_ref_phase merged with no section in the group doc, and nothing in the run or the review flagged its absence. Add the step, in both the kolla recipe and the image doc's short form, and say what a section consists of (stage lead, command, Reads:/Fix: bullets) so it is a concrete requirement rather than a reminder to be thorough. The rationale is stated where an author will read it: a plugin's module docstring is not reachable from a red line in a nightly report, so an undocumented check has no operator-facing description at all. This is prose, not enforcement -- a plugin can still merge undocumented. Assisted-by: Claude:claude-opus-5 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.
The drift docs promise a complete roster —
## The checkssays the plugins runin registry order and "the report renders them the same way", and each one gets
a
### Plugin: <name>section — but nothing enforces it, and two gaps hadopened:
kolla_source_ref_phasemerged with no section at all, so the onlydescription of the check was its module docstring.
kolla_mirror_verbatimsection still described a singleall/001-kolla-defaults.yml, which the mirror-layer series (74ac6e5..9edc160)replaced with a per-service
all/001-*.ymllayer.The docstring is not the operator-facing surface. A maintainer meets a check as
a red line in the nightly
release-tox-driftreport and goes to the group doc;nothing in the run points at a module docstring.
Why it happened, and what stops it
## Adding a pluginin the kolla doc is the canonical recipe — the image doc andthe new catalog doc both defer to it. It listed the module contract, the registry
entry, the config stanza and the test pattern. It never said to document the
check. The one step missing from the recipe is the one that got missed, twice.
This adds the step, in the kolla recipe and in the image doc's short form, and
says what a section consists of (stage lead, command,
Reads:/Fix:bullets) soit reads as a requirement rather than a reminder to be thorough.
It is prose, not enforcement: a plugin can still merge undocumented. A tolerant
registry test — every registered plugin's name appears in some heading under
docs/, no fixed file or heading level — would close that, and would survive therestructure this file is heading for. Left out of this PR deliberately; happy to
add it.
Commits
docs: document kolla_source_ref_phase— what it compares (eachopenstack_projectsref against the latest lifecycle phase opendev stillpublishes a tarball for), why it probes artifacts rather than branches, why an
inconclusive probe reports unverified rather than clean, and why no
--base-dircan stand in for it (EXTERNAL_HOSTS, a property no other kollaplugin has and one an operator currently discovers as an error message).
docs: read the 001 mirror as a layer— the layer, its lexical merge order andwhy that matters (upstream defines the
database_*keys in bothcommon.ymland
database.ymland relies ondatabase.ymlwinning), and the fallback tothe layer glob for the monolithic upstream layout of 2025.1 and earlier.
docs: make documenting a plugin part of adding one— the recipe step.The four-stage framing in
## The checksdescribes a kolla service, andkolla_source_ref_phaseis about the release manifest rather than any oneservice, so it is introduced as sitting ahead of the stages instead of being
renumbered into the list.
Merge order for the queue behind this
Docs-only, no behaviour change, and deliberately placed first so the queued
plugin PRs land against a doc that already asks them for a section. I
test-merged this branch against each of them in both directions, then all five
sequentially:
Everything is clean except one unavoidable collision between the last two:
2643 and 2690 both append their
plugins:stanza at EOF ofsrc/drift-config.yml. It fires in either order; keeping both lines resolvesit. 2643 goes last because it is a draft and 19 commits behind, so it absorbs
that one line in the rebase it needs anyway — and picks up its own doc section
there. Verified end state of all five merged: 529 passed, 2 skipped, lint clean.
Verification
tox -e drift-teston this branch: 420 passed. flake8, black and yamllint clean.No source changes, so no drift run is affected.
Note for a reviewer
docs/check-drift-kolla.mdis 512 lines after this, with 13 plugin sections inone file, and the queued PRs add ~40 more. These commits are additive and do not
depend on the current layout, so a later split (one file per plugin, group docs
keeping run/input-resolution/allowlist/release-model) does not have to unpick
them.