map.jinja and sls

[Alex Leonhardt]

hi guys,

am trying myself on slightly rewriting the redis formula and looked at http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html so trying to create a map.jinja and use the variables from there inside the sls - however, I seemingly am hitting a wall - not sure why it cant find the value ? 

here's the map.jinja : 

{% set redis = salt['grains.filter_by']({

    'Debian': {                          

        'pkg_name': 'redis-server',      

        'svc_name': 'redis-server',

        'cfg_name': '/etc/redis/redis.conf',      

        'cfg_version': '2.6',            

    },

    'RedHat': {

        'pkg_name': 'redis',             

        'svc_name': 'redis',             

        'cfg_name': '/etc/redis.conf',   

        'cfg_version': '2.4',            

    },

}, merge=salt['pillar.get']('redis:lookup')) %}                                                                                                                                                  

here's the sls : 

{% from "redis/map.jinja" import redis with context %}

{% set redis = pillar.get('redis', {}) -%}

{% set install_from = redis.get('install_from', 'package') -%}

{% if install_from == 'source' %}

{% set version = redis.get('version', '2.8.8') -%}

{% set checksum = redis.get('checksum', 'sha1=aa811f399db58c92c8ec5e48271d307e9ab8eb81') -%}

{% set root = redis.get('root', '/usr/local') -%}

  

[snip as we dont install from src]

{% elif install_from == 'package' %}

install-redis:

  pkg.installed:

    - name: {{ redis.pkg_name }}

    {% if version %}

    - version: {{ redis.version }}

    {% endif %}

{% endif %}                                                                                                                                                                                      

but when I run salt on the minion : 

[root@vm1 ~]# salt-call state.highstate

[INFO    ] Loading fresh modules for state activity

[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://top.sls'

[INFO    ] Creating module dir '/var/cache/salt/minion/extmods/modules'

[INFO    ] Syncing modules for environment 'base'

[INFO    ] Loading cache from salt://_modules, for base)

[INFO    ] Caching directory '_modules/' for environment 'base'

[INFO    ] Creating module dir '/var/cache/salt/minion/extmods/states'

[INFO    ] Syncing states for environment 'base'

[INFO    ] Loading cache from salt://_states, for base)

[INFO    ] Caching directory '_states/' for environment 'base'

[INFO    ] Creating module dir '/var/cache/salt/minion/extmods/grains'

[INFO    ] Syncing grains for environment 'base'

[INFO    ] Loading cache from salt://_grains, for base)

[INFO    ] Caching directory '_grains/' for environment 'base'

[INFO    ] Creating module dir '/var/cache/salt/minion/extmods/renderers'

[INFO    ] Syncing renderers for environment 'base'

[INFO    ] Loading cache from salt://_renderers, for base)

[INFO    ] Caching directory '_renderers/' for environment 'base'

[INFO    ] Creating module dir '/var/cache/salt/minion/extmods/returners'

[INFO    ] Syncing returners for environment 'base'

[INFO    ] Loading cache from salt://_returners, for base)

[INFO    ] Caching directory '_returners/' for environment 'base'

[INFO    ] Creating module dir '/var/cache/salt/minion/extmods/outputters'

[INFO    ] Syncing outputters for environment 'base'

[INFO    ] Loading cache from salt://_outputters, for base)

[INFO    ] Caching directory '_outputters/' for environment 'base'

[INFO    ] Loading fresh modules for state activity

[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://editor/init.sls'

[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://epel/init.sls'

[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://redis/init.sls'

[INFO    ] Fetching file from saltenv 'base', ** done ** 'redis/common.sls'

[INFO    ] Fetching file from saltenv 'base', ** skipped ** latest already in cache 'salt://redis/map.jinja'

[CRITICAL] Rendering SLS "base:redis.common" failed: Jinja variable 'dict' object has no attribute 'pkg_name'; line 57

---

[...]

{% elif install_from == 'package' %}

install-redis:

  pkg.installed:

    - name: {{ redis.pkg_name }}    <======================

    {% if version %}

    - version: {{ redis.version }}

    {% endif %}

[...]

---

local:

    Data failed to compile:

----------

    Rendering SLS "base:redis.common" failed: Jinja variable 'dict' object has no attribute 'pkg_name'; line 57

---

[...]

{% elif install_from == 'package' %}

install-redis:

  pkg.installed:

    - name: {{ redis.pkg_name }}    <======================

    {% if version %}

    - version: {{ redis.version }}

    {% endif %}

[...]

---

Is the above happening because of this ?? :

{% set redis = pillar.get('redis', {}) -%}

If so, and I'd remove it, how do I get these : 

  {% set install_from = redis.get('install_from', 'package') -%}

Thanks!
Alex

[Arnold Bechtoldt]

> here's the sls :
>
> {% from "redis/map.jinja" import redis with context %}
>
> {% set redis = pillar.get('redis', {}) -%}

I think that doesn't make sense at all. You are assigning a new value to
redis.

At
https://github.com/bechtoldt/httpd-formula/blob/master/httpd/init.sls#L1
you can see how I am merging OS specific with pillar defined data.


--
Arnold Bechtoldt

Karlsruhe, Germany

> --
> You received this message because you are subscribed to the Google
> Groups "Salt-users" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to salt-users+...@googlegroups.com
> <mailto:salt-users+...@googlegroups.com>.
> For more options, visit https://groups.google.com/d/optout.

[Alex Leonhardt]

Thanks Arnold,

I took it from the original formula, which I suppose with the map.jinja won't work unless I define the values from the map to be accessed differently like you did in the httpd formula.

Cheers,

Alex

[Wolodja Wentland]

Hi,

On Sun, Jun 22, 2014 at 04:22 -0700, Alex Leonhardt wrote:
> On Sunday, 22 June 2014 11:35:20 UTC+1, Arnold Bechtoldt wrote:

> I took it from the original formula, which I suppose with the map.jinja won't
> work unless I define the values from the map to be accessed differently like
> you did in the httpd formula.

> > here's the sls :
> >
> > {% from "redis/map.jinja" import redis with context %}
> >
> > {% set redis = pillar.get('redis', {}) -%}

> I think that doesn't make sense at all. You are assigning a new value to
> redis.

I think that the original author made a mistake there and wanted to guard
against the case in which values in the map are not defined, which is good
style if you read things from *directly* from the pillar, but doesn't make
much sense if you are simply important a pre-defined map. I would simply
remove the {% set redis ... %} as it doesn't add much.

Another thing I don't like about this formula is that it mixes things the user
genuinely wants to override and values that are, more or less, static such as
package and service names. The current implementation conflates these by
defining all of them in map.jinja and allowing the user to override them in
redis:lookup.

It would be, IMHO, advisable to keep these two things separate in the formula
and would therefore recommend to do the following:

* map.jinja redis should still define:

- home: /var/lib/redis
- user: redis
- port: 6379
- cfg_name: /etc/redis.conf
- pkg_name: redis
- svc_name: redis

These are values that are unlikely to change from one distribution to each
other and lend themselves to being, almost statically, defined in the lookup
map.

* Move the following elsewhere:

- svc_state: stopped
- version: 1.2.3
- bind: 0.0.0.0
- databases: 8
- snapshots:

These are all values users of the formula are likely to override or define
themselves and I would make that explicit by defining that in the "redis"
pillar (i.e. not "redis:lokup") directly.

Some people might say that one would *also* like to define default values for
these settings, but these should IMHO not be mixed in with the aforementioned
static data. There are two approaches to settings these default values that I
see frequently:

* Inline: salt['pillar.get']('redis:databases', 8)
* By using a macro to look up values:

{% from "redis/map.jinja" import defaults with context -%}
{%- macro get_config_item(item_name) -%}
{%- set default = defaults['config'].get(item_name, None) -%}
{%- salt['pillar.get']('memcached:%s' % (item_name), default) -%}
{%- endmacro -%}

This allows you to have *two* dictionaries in your map.jinja, one "redis" and
one "defaults". You will set distribution specific static data in the former
and allow the user to override those in redis:lookup and reserve the redis
pillar for things the user wants to define.

It happens way too frequently that the whole foo:lookup idiom is blindly
copied from other formulas without even making lookup explicit in
pillar.example or understanding why it is done that way.
--
Wolodja <bab...@gmail.com>

4096R/CAF14EFC
081C B7CD FF04 2BA9 94EA 36B2 8B7F 7D30 CAF1 4EFC

[Alex Leonhardt]

hi wolodja,

thanks - it's already merged - have a look: https://github.com/saltstack-formulas/redis-formula/pull/16 

cheers!

alex

[Wolodja Wentland]

Hi Alex,

On Mon, Jun 30, 2014 at 20:02 +0100, Alex Leonhardt wrote:
> thanks - it's already merged - have a look: https://github.com/
> saltstack-formulas/redis-formula/pull/16 

I've seen the PR a bit later, but think that my original point still stands:
The role of the lookup table in map.jinja should be to set more or less static
values that might differ between distributions, while everything the user
might want to change should be defined in the normal, non-lookup, pillar.

It simply shouldn't be necessary to *always* nest FOO:pillar:BAR and I believe
that this is essentially unidiomatic. I see that you chose the "set defaults
inline" approach which is fine, but I would still recommend to move a number
of settings out of the lookup table.

[Wolodja Wentland]

On Mon, Jun 30, 2014 at 22:40 +0200, Wolodja Wentland wrote:
> Hi Alex,
>
> On Mon, Jun 30, 2014 at 20:02 +0100, Alex Leonhardt wrote:
> > thanks - it's already merged - have a look: https://github.com/
> > saltstack-formulas/redis-formula/pull/16 

> It simply shouldn't be necessary to *always* nest > FOO:pillar:BAR and I believe

^^^^^^^^^^^^^^
FOO:lookup:BAR naturally

[Alex Leonhardt]

Hi,

thanks - when I was thinking about it, i didnt want to have to try and find/get data 2 different ways, so just re-using foo:lookup:bar made sense to me

Alex