kolla: reconcile the drift-check allowlist#2571
Open
ideaship wants to merge 5 commits into
Open
Conversation
5a432ae to
9fd2eb3
Compare
kolla_image_orphan flags neutron_ovn_vpn_agent_image / _tag because they are defined in kolla-ansible master (neutron role) but not in any release in the supported 2024.1-2025.2 union. It is a forward-looking upstream addition OSISM carries ahead of the range, not dead cruft, so allowlist both vars. Once 2026.1 is supported the union will cover them, the check will stop flagging them, and stale-allowlist detection will report these entries as stale -- prompting their removal. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
kolla_enablement_orphan flagged enable_chrony and enable_openstack_exporter as orphaned (no upstream kolla-ansible counterpart at a supported release), and kolla_orphan_config consequently flagged their seven companion vars as dead config to remove: chrony_servers and chrony_allowed_subnets in all/099-generic.yml, and the five openstack_exporter_* vars (clouds_yml_cloud, clouds_yml_path, secure_yml_path, service_monitor_scrape_interval, service_monitor_scrape_timeout) in all/099-infrastructure.yml. Both are false positives: these are live OSISM inventions whose names collide with dead or renamed kolla services. Verified two ways: - Upstream kolla-ansible git history shows chrony_servers, chrony_allowed_subnets and every openstack_exporter_* var never existed upstream. The kolla chrony role that was removed in Xena defined chrony_image/chrony_tag/chrony_bindaddress, never chrony_servers; upstream's exporter is the separate prometheus_openstack_exporter. - They are consumed by live OSISM roles that the drift scan does not cover: ansible-collection-services/roles/chrony (chrony.conf.j2 host NTP config) and osism-kubernetes/roles/monitoring (the k8s openstack-exporter ServiceMonitor and clouds.yml wiring). Allowlist is the tool's designed resolution for OSISM inventions (cf. the existing common / kolla_operations entries), not adding the consumer repos to the scan: these are upstream-mirror checks, not consumer-usage checks, so consumer-scanning would mask genuine drift and drop the documented rationale. Allowlisting at the enable-flag level also clears the paired kolla_orphan_config companion-var findings, because that check derives its dead-service set from kolla_enablement_orphan minus this allowlist. After the change, check-drift.py --group all reports zero kolla_orphan_config drift and kolla_enablement_orphan no longer lists chrony/openstack_exporter (nine to seven flags, leaving the genuinely removed karbor/qinling/searchlight/auditd). Overall "to act on" drops from 61 to 52, allowlisted rises 53 to 55, with no stale entries, and all 262 drift tests pass. While here, correct the section comment: kolla_enablement_orphan uses explicit scope -- it flags every invented enable_* by name regardless of value -- not a "truthy-only scope"; the old wording wrongly implied a flag must be truthy to surface. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
kolla_enablement_orphan flagged enable_auditd as orphaned because upstream kolla-ansible defines no auditd service at any supported release. This is a false positive: enable_auditd is a live OSISM invention that never existed upstream (verified via kolla-ansible git history), gating OSISM's own host-level audit daemon. It is consumed by ansible-collection-services/roles/auditd and the generic/auditd.yml, bootstrap.yml and maintenance.yml playbooks (when: enable_auditd). Allowlist it alongside the other OSISM-invented enable flags rather than removing the flag or extending the scan, consistent with the chrony and openstack_exporter entries. After the change check-drift.py --group all reports zero kolla_enablement_orphan drift, no stale allowlist entries, and all 262 drift tests pass. Enrich the reason to note enable_auditd is false by default: the check still flags it because kolla_enablement_orphan uses explicit scope (by name, regardless of value), so the entry is load-bearing, not stale. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
The role_unpinned check flags role-default <alias>_tag pins that have no canonical release base.yml pin. Two of the four remaining findings are httpd role images whose tags are intentionally rolling and role-managed, not release-pinned: - httpd: the Apache static/reverse-proxy server, pinned to the rolling 'alpine' tag by design. - httpd_data: a one-shot rsync data-import sidecar (httpd_data_enable is off by default) that tracks registry.osism.tech/osism/rsync:latest by design. Neither should carry a base.yml pin, so allowlist both under role_unpinned, matching the existing zuul_* CI-stack precedent. This clears 2 of the 4 role_unpinned findings; kepler and thanos_sidecar remain and are handled separately. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
Upstream fixed the eutron_external_interface typo on stable/2025.2 (bug 2160035, merged 2026-07-08), so the kolla_groupvars_missing finding this entry suppressed is gone and the entry reports stale -- exactly the removal trigger its comment named. It was the only entry in the section, so remove the entry together with its now-empty "# --- kolla_groupvars_missing ---" header. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Roger Luethi <luethi@osism.tech>
9fd2eb3 to
5f5c065
Compare
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
Reconciles the kolla drift-check allowlist now that the checks (#2569,
merged) run against the current repos. Five commits,
src/drift-allowlist.ymlonly:neutron_ovn_vpn_agent— defined upstream at master, ahead of the supported releaserange (self-retires once 2026.1 is supported).
chrony/openstack_exporter— live OSISM inventions whose names collide with dead orrenamed kolla services (verified via upstream git history + OSISM consumers); allowlisting
at the enable-flag level also clears their companion vars in
kolla_orphan_config.auditd— live OSISM host-level auditd; never an upstream kolla service.httpdrolling image tags — role-managed by design, not release-pinned.kolla_groupvars_missingsection (eutron_external_interface):upstream fixed the typo on stable/2025.2 (bug 2160035, merged 2026-07-08), so the
finding this entry suppressed is gone and the daily check reports the entry as stale —
the removal trigger the entry's own comment named.
No merge-order constraint: the previous gate on osism/defaults#297 dissolved with the
upstream typo fix (the entry is stale on
maintoday regardless of #297).Builds on merged #2569 (drift checks) + #2570 (transport/archive backend).
Related