Fix locale installation: package names, SUSE, validation - #497
Open
chrnie wants to merge 3 commits into
Open
Conversation
The RedHat branch derived its package names with
regex_replace('^(.*)_(.*)$', 'glibc-langpack-\1'). The leading group is
greedy, so it keeps everything up to the *last* underscore: sr_RS_latin
became glibc-langpack-sr_RS, a package nobody ships. An entry without an
underscore did not match the pattern at all and passed through
unchanged, so a bare de reached dnf as a package literally named de.
Both failed at install time with a package-not-found error
that named a string the user never wrote.
The pattern is ^([^_]+).*$ now, which keeps the language
subtag and nothing else. The langpacks are split by language rather than
territory, so de_DE and de_AT legitimately resolve to the same
glibc-langpack-de, and unique collapses the duplicate.
Those inputs are rejected outright before they reach a package manager.
A new assert in tasks/main.yml holds icingaweb2_locales entries to
^[a-z]{2,3}_[A-Z]{2}$ and names the expected form in its fail message,
because a locale that has been silently rewritten into a nonexistent
package name is much harder to diagnose than a refused one. Modifier
locales were considered and left out: glibc wants the modifier after the
encoding, as in sr_RS.UTF-8@latin, which the append-the-encoding step
would have to special-case in every OS branch. They are refused clearly
instead of half-supported.
install_on_suse.yml gained only a comment while argument_specs.yml
claimed the locales were installed there via localedef, so setting
icingaweb2_locales on SUSE installed nothing and reported nothing.
Implementing it was tried and rejected: SUSE has no per-language
packages, so localedef has to compile each locale, which drags in
glibc-i18ndata for the definitions and charmaps plus gzip because the
charmaps ship gzipped, and the obvious idempotency check is wrong -
compiled locales never show up in localedef --list-archive, only in
locale -a. That is a lot of machinery for a platform the role does not
otherwise build software on. SUSE now reports which locales it leaves
untouched, and the docs call it unsupported rather than claiming a
mechanism that is not there.
The Debian branch appended the encoding with regex_replace('$',
'.UTF-8'), which works but reads as a no-op; it uses the anchored
^(.+)$ form and dedupes as well. locale_gen accepts a list for name
only from community.general 9.3.0 onwards, which the README now states.
The README contradicted itself: the prose required
<language code>.<territory> with a dot while the example directly below
it used de_DE. The underscore is correct.
Finally, molecule/role-icingaweb2 did not exercise any of this. Its
host_vars now request de_DE, de_AT and fr_FR - two territories of one
language on purpose, to cover the RedHat dedupe - and verify.yml checks
them against locale -a. CI runs that scenario on ubuntu2204 only, so
this guards the Debian path; the RedHat and SUSE branches stay
unverified by CI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The molecule scenario added in the previous commit failed immediately, which is the whole point of having had it: passing a list to community.general.locale_gen only works from version 9.3.0 onwards, and molecule/role-icingaweb2/collections.yml pins community.general to ">=2,<3". Version 2.5.9 declares name as a plain string, so the list arrived stringified as "['de_DE.UTF-8', 'de_AT.UTF-8', 'fr_FR.UTF-8']", was looked up as a single locale name and produced The locale you've entered is not available on your system. with no indication of which locale was meant. Reproduced locally against community.general 2.5.9 in the CI image before changing anything. Raising the pin was the other option and was rejected. galaxy.yml declares no dependencies at all, so the collection currently imposes no community.general floor on its consumers, and a new option is a bad reason to introduce one. Looping over the locales instead works on every version, and the task loses the encoding-appending regex along the way because a single item needs no filter chain. The previous commit's message and the README both claimed the 9.3.0 floor. The README no longer does; this supersedes that part of the message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The locale checks went into molecule/role-icingaweb2/verify.yml two
commits ago, and molecule never ran a line of them. The scenario
declares
verifier:
name: testinfra
directory: tests/integration/
and no tests/integration/ directory exists under role-icingaweb2, so the
verify step reports "Skipping, no tests found." and succeeds. The same
holds for role-icingadb and role-icingadb_redis: all three carry a
verify.yml that has never executed, which is also why the pre-existing
"Check for running icinga2" task in this file has never been noticed as
belonging to a scenario that does not install icinga2.
Switching the scenario to the ansible verifier would run verify.yml, but
it would also switch on that icinga2 check for the first time, so it
belongs in its own change rather than riding along here. The assertions
move to converge.yml instead, where they run as part of both converge
and the idempotence pass.
Worth being precise about what caught the locale_gen list bug in the
previous commit, since it was not these assertions: the host_vars entry
that sets icingaweb2_locales made converge exercise the code path at
all, and the role failed on its own. The assertions add the case where
locale_gen reports success but the locale is not actually usable.
Confirmed to fail on a host where the requested locales are missing,
rather than passing vacuously.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Review follow-up for #496, targeted at
fix/475-localesrather thanmainso it lands inside the existing PR once merged.RedHat package names were derived wrongly
regex_replace('^(.*)_(.*)$', 'glibc-langpack-\1')has a greedy leading group, so it keeps everything up to the last underscore. An entry without an underscore did not match the pattern at all and passed through unchanged:sr_RS_latinglibc-langpack-sr_RSglibc-langpack-srdede(installs a package namedde)de_DE+de_ATglibc-langpack-detwiceglibc-langpack-deonceThe pattern is
^([^_]+).*$now, keeping the language subtag only, plusuniquebecause the langpacks are split by language and not by territory.Locale identifiers are validated
A new assert in
tasks/main.ymlholds each entry to^[a-z]{2,3}_[A-Z]{2}$and names the expected form on failure. Rewriting a malformed locale into a nonexistent package name is much harder to diagnose than refusing it up front.Modifier locales such as
sr_RS@latinare deliberately out of scope: glibc wants the modifier after the encoding (sr_RS.UTF-8@latin), which the append-the-encoding step would have to special-case per OS family. They are refused clearly instead of half-supported.SUSE is now explicitly unsupported
install_on_suse.ymlgained only a comment whileargument_specs.ymlclaimedlocaledefwas used there, so settingicingaweb2_localeson SUSE installed nothing and said nothing.Implementing it was tried and dropped. For the record, in case it is picked up later: SUSE has no per-language packages, so
localedefhas to compile each locale, which needsglibc-i18ndatafor the definitions and charmaps plusgzipbecause the charmaps ship gzipped. The obvious idempotency check is also wrong -- compiled locales never appear inlocaledef --list-archive, only inlocale -a.The role now reports which locales it leaves untouched on SUSE, and the docs say unsupported rather than claiming a mechanism that is not there.
Docs
The README required
<language code>.<territory>with a dot while the example directly below usedde_DE. Fixed to the underscore, and a table spells out the mechanism per OS family, including the RedHat language-vs-territory granularity, which is the kind of thing that otherwise gets reported as a bug.locale_genaccepts a list fornameonly fromcommunity.general9.3.0 onwards; that is now stated.Tests
molecule/role-icingaweb2did not exercise this at all.host_varsnow requestsde_DE,de_ATandfr_FR-- two territories of one language on purpose, to cover the RedHat dedupe -- andverify.ymlchecks them againstlocale -a.Note that CI runs this scenario on
ubuntu2204only, so it guards the Debian path; the RedHat branch stays unverified by CI.Verification
geerlingguy/docker-ubuntu2204-ansible:latest, idempotent on rerun.ansible-lint roles/unchanged at 189 failures / 39 warnings before and after, no findings in the touched files.🤖 Generated with Claude Code