Make params a dictionary, not array

[enver]

Do you think that params should be set as dictionary instead of array? With current implementation it is impossible to override defaults params without redefining all of them. E.g. if i set defaults like this:

sysctl:
  lookup:
    pkg: procps
    config:
      location: '/etc/sysctl.conf'
    params:
      - name: net.ipv4.ip_forward
        value: 1
      - name: net.core.somaxconn
        value: 65536
      - name: fs.file-max
        value: 65536
      - name: vm.swappiness
        value: 60

and want to override some param, e.g vm.swappiness for certain set of servers I'd need to redefine all params above instead of just redefining vm.swappiness. But if params were set like dictionary:

sysctl:
  lookup:
    pkg: procps
    config:
      location: '/etc/sysctl.conf'
    params:
      net.ipv4.ip_forward:
        value: 1
      net.core.somaxconn:
        value: 65536
      fs.file-max:
        value: 65536
        config: fs.conf
      vm.swappiness:
        value: 60

overriding vm.swappiness would require following configuration:

sysctl:
  lookup:
    params:
      vm.swappiness:
        value: 20

Thoughts?

[aboe76]

@enver I can see the reason behind this, go ahead, if you want to be so kind to create a Pull Request

[enver]

@aboe76 Created pull request #10

[aboe76]

I will test it first if you don't mind

[aboe76]

@enver I have tested your code and it looks good

could you add the following, this will make the jinja rendering a little more readable

# -*- coding: utf-8 -*-
# vim: ft=sls

{## import settings from map.jinja ##}
{%- from "sysctl/map.jinja" import sysctl_settings with context -%}

{%- if sysctl_settings.params2 is defined -%}

  {%- for param_name, param in sysctl_settings.get('params2', {}).items() -%}
    {%- if param is mapping %}
sysctl-present-{{ param_name }}:
  sysctl.present:
    - name: {{ param_name }}
    - value: {{ param.value }}
      {%- if param.config is defined %}
    - config: {{ sysctl_settings.config.location }}/{{ param.config }}
      {% endif -%}
    {% endif -%}
  {% endfor %}

{% else %}

  {%- for param in  sysctl_settings.get('params', {}) -%}
    {%- if param is mapping %}
sysctl-present-{{ param.name }}:
  sysctl.present:
    - name: {{ param.name }}
    - value: {{ param.value }}
      {%- if param.config is defined %}
    - config: {{ sysctl_settings.config.location }}/{{ param.config }}
      {% endif -%}
    {% endif -%}
  {% endfor %}

{%- endif -%}

[enver]

@aboe76 Sure, first thing in the morning

[enver]

@aboe76 Done

[aboe76]

@enver merged

[aboe76]

@enver can this issue be closed with the merge?

[enver]

Yes, closed