Repository navigation
Remove defaults that nothing reads - #322
Merged
Merged
Conversation
ideaship
marked this pull request as ready for review
October 9, 2026 09:59
ideaship
force-pushed
the
unread-defaults-cleanup
branch
2 times, most recently
from
October 9, 2026 16:03
20915c2 to
e1b9316
Compare
ideaship
force-pushed
the
unread-defaults-cleanup
branch
from
October 9, 2026 17:30
e1b9316 to
dfeef81
Compare
certificates_dir and keystone_token_provider are set in all/099-kolla.yml, but no supported kolla-ansible release reads either of them, and no OSISM repository ever did. certificates_dir was a role default of the upstream certificates role in Stein and Train only (added 9223deeec). Ussuri renamed it to kolla_certificates_dir (b475643c1). OSISM pointed it at /share/certificates in 2020 (container-image-kolla-ansible 087bf88), together with a carried Stein/Train patch; the value was carried into this repository with its initial commit and has been inert since OSISM stopped building Train. keystone_token_provider was read by the upstream keystone role until b3b9dff3c (Zed) replaced every use with a literal fernet, the only valid value since Rocky. OSISM's "fernet" therefore never differed from upstream's behaviour. The CHANGELOG entry for #142 that calls the parameter removed refers to the copy in the former upstream mirror file; this OSISM copy was left behind. Remove both. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Roger Luethi <luethi@osism.tech>
Six keys in all/099-infrastructure.yml have had no reader since 2020. They came into this repository with its initial commit, carried over from osism/ansible-playbooks, which had taken them from the testbed and cookiecutter configuration in 2020. configure_adminer, configure_phpmyadmin, configure_rally and configure_sshconfig gated the per-service task includes of the osism/ansible-helper role. That role was split into collection roles and archived in August 2020 (ansible-helper 52a5074); the replacement adminer, phpmyadmin and sshconfig roles have no such switch, and rally was not carried over. The helper playbook has been a stub that fails with "Role has been split up" since ansible-playbooks 892ba8d. cephclient_cluster_fsid was never read in any public history; the cephclient role has no fsid setting. opentstackclient_groupname is misspelled and has been since it first appeared, so it never had an effect. The correct name, openstackclient_groupname, lost its only reader when openstackclient moved out of the helper role in May 2020 (ansible-helper 21c8901). Remove all six, and the helper section header that is left empty. cephclient_groupname stays: the ceph validate playbooks in ansible-playbooks read it. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Roger Luethi <luethi@osism.tech>
bootstrap_dirs_owner and bootstrap_dirs_group were read only by ceph-ansible's roles/ceph-docker-common/tasks/dirs_permissions.yml. ceph-ansible deleted that file in 2018 (6947f9c3e, in stable-4.0 and later) and dropped the variables from ceph-defaults in 2020 (c644ea904). OSISM set them in 2020 as part of running Ceph under UID 64045 (container-image-ceph-ansible 77282e1), when it still shipped stable-3.2. The ceph-ansible branches OSISM ships now, stable-7.0 to stable-9.0, do not mention them. ceph_uid stays. ceph_share_directory was moved here from the testbed and cookiecutter configuration in 2022 (#90), but no code has ever read it. Consumers of /share either hard-code the path or, for ceph-ansible, use fetch_directory, which is set separately in this file. osd_scenario selected the OSD layout in ceph-ansible until stable-4.0 made lvm the only scenario. stable-7.0 to stable-9.0 mention it only in docs/source/osds/scenarios.rst ("no need to configure osd_scenario anymore"), in a tests/ fixture, and in tox-update.ini, which passes it as --extra-vars to a test playbook. None of that reads this default. Remove all four. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Roger Luethi <luethi@osism.tech>
cilium_cli was added in #266 to stop k3s-ansible from installing the Cilium CLI, which the osism-kubernetes image already ships. It gated the CLI install tasks in roles/k3s_server_post/tasks/cilium.yml. osism/osism-kubernetes#241 (1063d6d, October 2025) removed those tasks, and the gate with them; the image build installs the CLI instead. Only osism-kubernetes releases between these two changes read the key, i.e. OSISM 9.2.0 to 9.4.0, and those releases pin their own defaults version. Nothing on main reads it, so remove it. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Roger Luethi <luethi@osism.tech>
Four keys in all/099-generic.yml have no reader. All came into this repository with its initial commit, carried over from osism/ansible-playbooks. docker_configure_storage_driver switched --storage-driver on the dockerd command line in the old standalone ansible-docker role. That role moved to daemon.json in 2017 and dropped the switch; the osism.services docker role never had it and always writes docker_storage_driver into daemon.json. docker_storage_driver stays. grub_hidden_timeout belonged to the old debops.grub role. debops v1.0 (fd73f3ebdd, 2018) redesigned the role: the hidden timeout is now an entry in grub__original_configuration, and the variable is gone. OSISM has shipped the debops collection since 2020, so the value "false" has not reached GRUB since then. Removing it changes nothing; restoring the intent would need a grub__configuration entry. fluentd_host was the rsyslog role's variable until osism.services #113 (98231886, January 2021) renamed it rsyslog_fluentd_host and kept fluentd_host as an alias (fluentd_host: "{{ rsyslog_fluentd_host }}"). Nothing reads fluentd_host. rsyslog_fluentd_host, which the role's template reads, stays. sysdig_configure_repository was read by the osism.commons sysdig role, which was deleted in January 2024 (ff592b2), along with the generic sysdig play (ansible-playbooks 9821528). Remove all four, and the grub comment left empty. Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Roger Luethi <luethi@osism.tech>
all/099-interfaces.yml sets bifrost_network_address_family to
"{{ network_address_family }}", in the block copied from kolla-ansible's
defaults with this repository's initial commit. It is not an OSISM
opinion: the value is identical to upstream's, which
all/001-bifrost.yml now supplies from the mirror layer.
Upstream has never read the variable either. It was added with IPv6
support for the control plane (kolla-ansible bc053c09c, 2019), but no
role or template uses it, and the bifrost configuration kolla-ansible
renders passes no address family. Removing the 099 copy therefore
changes nothing; the 001 definition stays, as that layer mirrors
upstream verbatim.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Roger Luethi <luethi@osism.tech>
all/002-images-ceph.yml sets ceph_docker_tag to
"{{ ceph_image_version }}}", with a stray closing brace. Nothing has
ever read it: not ceph-ansible on any branch or tag, not any OSISM
repository. The cephadm plays and ceph-ansible read
ceph_docker_image_tag, which is set on the line above and stays.
The key was defined in container-image-ceph-ansible from its first
public commit (604e7ae, 2019-11-05) and moved here with the rest of the
image defaults (b9194e2, 2021-05-03). The typo went unnoticed because
the value was never rendered.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Roger Luethi <luethi@osism.tech>
osism-kubernetes collects node labels with
lookup('community.general.merge_variables', '^k3s_add_labels__.+$')
(playbooks/kubernetes-kubernetes-label-nodes.yml). Every group file
names its variable k3s_add_labels__<group>, except monitoring, which
had k3s_add_labels_monitoring with one underscore. That name does not
match the pattern, so monitoring nodes never received
node-role.osism.tech/monitoring-plane=true. This has been the case
since the labels were added in #229.
Rename it to k3s_add_labels__monitoring. Monitoring nodes now get the
label. Nothing selects on it yet (osism-kubernetes only uses the
control-plane label), so no workload moves.
Found by the defaults_orphan drift check (osism/release#2811).
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Roger Luethi <luethi@osism.tech>
ideaship
force-pushed
the
unread-defaults-cleanup
branch
from
October 10, 2026 09:16
dfeef81 to
bb11b53
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.
This removes 19 defaults that nothing reads and fixes one misnamed variable.
The findings come from two passes:
defaults_orphandrift check:Each commit covers one file and traces the git history of every key it removes: which reader went away, when, and in which commit.
099-kolla.yml):certificates_dir: upstream renamed it tokolla_certificates_dirin Ussuri;keystone_token_provider: upstream hard-codesfernetsince Zed.099-infrastructure.yml):configure_*helper section, whose role was split up and archived in 2020;cephclient_cluster_fsid;opentstackclient_groupname, misspelled since it first appeared.099-ceph.yml):bootstrap_dirs_ownerandbootstrap_dirs_group, gone from ceph-ansible since 2018 and 2020;ceph_share_directory, never read;osd_scenario, gone from ceph-ansible's roles since stable-4.0. It is only passed to a test playbook bytox-update.ini.099-k3s.yml):cilium_cli. Its gate went with k3s: remove cilium installation osism-kubernetes#241.099-generic.yml):docker_configure_storage_driver;grub_hidden_timeout, dead since the debops 2018 grub redesign;fluentd_host, a reverse alias ofrsyslog_fluentd_host. The alias itself in the rsyslog role is removed separately:sysdig_configure_repository, whose role was removed in 2024.099-interfaces.yml):bifrost_network_address_family. kolla-ansible never reads it. The copy in001-bifrost.ymlstays, because that layer mirrors upstream.002-images-ceph.yml):ceph_docker_tag. Nothing ever read it, and its value had a stray}. The cephadm plays readceph_docker_image_tag.monitoring/000-defaults.yml):merge_variablesmatching^k3s_add_labels__.+$.k3s_add_labels_monitoring, with one underscore, so monitoring nodes never gotnode-role.osism.tech/monitoring-plane=true.k3s_add_labels__monitoring. Nothing selects on that label yet, so no workload moves.Kept on purpose:
grafana_server_group_nameandiscsi_gw_group_namestay, because ceph-ansible stable-7.0, which quincy pins, still reads them.keystone_admin_portandkeystone_admin_listen_portare dead upstream, but an environment overlay still templates them. They are handled in a separate change.Run against this branch, the
defaults_orphancheck (osism/release#2811) reports no findings. This PR and osism/release#2811 should land together; otherwise the nightly drift job fails on these findings.🤖 Generated with Claude Code