Skip to content

Remove defaults that nothing reads - #322

Merged
berendt merged 8 commits into
mainfrom
unread-defaults-cleanup
Oct 10, 2026
Merged

berendt merged 8 commits into
mainfrom
unread-defaults-cleanup

Conversation

@ideaship

@ideaship ideaship commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

This removes 19 defaults that nothing reads and fixes one misnamed variable.

The findings come from two passes:

Each commit covers one file and traces the git history of every key it removes: which reader went away, when, and in which commit.

  • kolla (099-kolla.yml):
    • certificates_dir: upstream renamed it to kolla_certificates_dir in Ussuri;
    • keystone_token_provider: upstream hard-codes fernet since Zed.
  • infrastructure (099-infrastructure.yml):
    • the configure_* helper section, whose role was split up and archived in 2020;
    • cephclient_cluster_fsid;
    • opentstackclient_groupname, misspelled since it first appeared.
  • ceph (099-ceph.yml):
    • bootstrap_dirs_owner and bootstrap_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 by tox-update.ini.
  • k3s (099-k3s.yml):
  • generic (099-generic.yml):
  • interfaces (099-interfaces.yml):
    • bifrost_network_address_family. kolla-ansible never reads it. The copy in 001-bifrost.yml stays, because that layer mirrors upstream.
  • ceph images (002-images-ceph.yml):
    • ceph_docker_tag. Nothing ever read it, and its value had a stray }. The cephadm plays read ceph_docker_image_tag.
  • The monitoring k3s label (monitoring/000-defaults.yml):
    • osism-kubernetes collects node labels with merge_variables matching ^k3s_add_labels__.+$.
    • The monitoring group's variable was k3s_add_labels_monitoring, with one underscore, so monitoring nodes never got node-role.osism.tech/monitoring-plane=true.
    • It is renamed to k3s_add_labels__monitoring. Nothing selects on that label yet, so no workload moves.

Kept on purpose:

  • grafana_server_group_name and iscsi_gw_group_name stay, because ceph-ansible stable-7.0, which quincy pins, still reads them.
  • keystone_admin_port and keystone_admin_listen_port are dead upstream, but an environment overlay still templates them. They are handled in a separate change.

Run against this branch, the defaults_orphan check (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

@ideaship ideaship self-assigned this Oct 9, 2026
@ideaship
ideaship marked this pull request as ready for review October 9, 2026 09:59
@ideaship
ideaship requested a review from berendt October 9, 2026 09:59
@ideaship
ideaship force-pushed the unread-defaults-cleanup branch 2 times, most recently from 20915c2 to e1b9316 Compare October 9, 2026 16:03
@ideaship
ideaship force-pushed the unread-defaults-cleanup branch from e1b9316 to dfeef81 Compare October 9, 2026 17:30
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
ideaship force-pushed the unread-defaults-cleanup branch from dfeef81 to bb11b53 Compare October 10, 2026 09:16
@berendt
berendt merged commit 82d33d2 into main Oct 10, 2026
2 checks passed
@berendt
berendt deleted the unread-defaults-cleanup branch October 10, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants