Switch to standard formula layout

[wwentland]

This PR introduces a formula layout that is more in line with that exemplified in the template formula, while retaining backwards compatibility.

[aboe76]

@babilen why is de default locale not in the defaults.yml ?

[wwentland]

@aboe76 That would cause the formula to always set a default locale, which is suboptimal for SSH and other areas.

The formula had:

{%- set default = salt['pillar.get']('locale:default', 'en_US.UTF-8') %}
...
{% if default is mapping %}
  locale_default:
    locale.system:
...

before, which would also only set the default locale if the user provided a suitably formatted pillar (i.e. a dictionary/mapping). The 'default' of 'en_US.UTF-8' is not a mapping and would have had no effect. I am not entirely sure why it was implemented that way.

Unless I'm missing something :)

[aboe76]

Then let's merge this.

[aboe76]

When I test your additions I get the following error:

Rendering SLS 'base:locale' failed: Jinja variable 'default' is undefined

with the following pillar data:

local:
    ----------
    locale:
        ----------
        conf:
            - LANG=en_US.UTF-8
            - LANGUAGE=en
            - LC_NUMERIC=nl_NL.UTF-8
            - LC_TIME=nl_NL.UTF-8
            - LC_MONETARY=nl_NL@euro
            - LC_COLLATE=nl_NL.UTF-8
            - LC_PAPER=nl_NL.UTF-8
            - LC_NAME=nl_NL.UTF-8
            - LC_ADDRESS=nl_NL.UTF-8
            - LC_TELEPHONE=nl_NL.UTF-8
            - LC_MEASUREMENT=nl_NL.UTF-8
            - LC_IDENTIFICATION=nl_NL.UTF-8
        default:
            ----------
            name:
                en_US.UTF-8
            requires:
                en_US.UTF-8 UTF-8
        present:
            - en_US.UTF-8 UTF-8
            - nl_NL.UTF-8 UTF-8
            - nl_NL@euro ISO-8859-15

[aboe76]

@babilen

With these changes they work again:

d3
< {%- set default = locale.get('default', {}) %}
21c20
< {% if default is defined %}
---
> {% if locale.default is defined %}
24c23
<     - name: {{ default.name }}
---
>     - name: {{ locale.default.name }}

[wwentland]

Cheers! I'll incorporate those tomorrow.

[wwentland]

@aboe76 Thanks for catching this. I did not incorporate your exact changes as that default in there would always be defined, but corrected the missing locale. in:

      - locale: locale_present_{{ locale.default.requires|replace('.', '_')|replace(' ', '_') }}

Could you please test again?

I am also not entirely happy with the fact that, both here and in the original formula, the requisite has to be defined in the pillar while it is not optional. It would be great if we could define that requisite programmatically, but am not sure about the best approach for that.

[aboe76]

@babilen the requisite was probably introduced around 2015.3/4 when the locale state would try to make a locale present but didn't check locale.conf if it existed, that's why it was created:

https://github.com/saltstack/salt/issues/22981

If you want to break backward compatibility on pillar data for this and clean it up more I don't mind.

[wwentland]

@aboe76: No, I'm not keen on breaking backwards compatibility at all and that would be an issue for another PR anyway.

Did the changes I pushed enable you to use the refactored formula now?

[aboe76]

@babilen I will test tonight, at the moment i'm @work

[wwentland]

Sure, enjoy your workday, @aboe76 :)

[aboe76]

@babilen Tested your changes and they work great, I'm merging this because I see no issues any more.

[wwentland]

Thanks again for catching this and have a great day, @aboe76 !