Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(map.jinja): load configuration values from configurables sources #70

Merged

Conversation

baby-gnu
Copy link
Contributor

@baby-gnu baby-gnu commented Mar 24, 2020

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

saltstack-formulas/template-formula#186

Describe the changes you're proposing

Load the formula parameters values from configurable map.jinja sources.

The configuration for map.jinja comes from:

  1. builtin default

    1. osarch grain
    2. os_family grain
    3. os grain
    4. osfinger grain
    5. lookup table retrived by config.get
    6. configuration retrived by config.get
    7. id grain
  2. defaults.yaml: optionally define a formula specific map_jinja:sources

  3. global configuration lookup map_jinja:sources

  4. formula specific <tplroot>:map_jinja:sources

The hard coded default is backward compatible and add the minion id at the end of the list.

If an entry does not match a salt['config.get'] parameter, it's used as a literal file path (with .yaml added if it's missing).

Each YAML file is formatted with the following top level keys:

  • values: contains the the parameter values

  • strategy (optional): define the merge strategy for the function salt.slsutils.merge (aggregate, list, overwrite, recurse, smart). The default is smart.

  • merge_lists (optional): boolean to merge lists or overwrite them

It's possile to configure config.get when looking up the formula configuration. You need to define a subkey strategy with the following differences from YAML files:

  • strategy can be one of None, overwrite and recurse. The default is None.

  • if you use salt-ssh, this merge strategy is skipped and and error log message is emitted.

The parameters values are merged in the following order:

  1. initialize default values from defaults.yaml

  2. merge the values from each source defined by the ordered list map_jinja:sources

Pillar / config required to test the proposed changes

To test the skipping of the merge configuration of config.get when using salt-ssh:

libvirt:
  strategy: overwrite

Debug log showing how the proposed changes work

[DEBUG   ] map.jinja: initialise parameters from libvirt/parameters/defaults.yaml
[DEBUG   ] In saltenv 'base', looking at rel_path 'libvirt/parameters/defaults.yaml' to resolve 'salt://libvirt/parameters/defaults.yaml'
[DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/libvirt/parameters/defaults.yaml' to resolve 'salt://libvirt/parameters/defaults.yaml'
[DEBUG   ] map.jinja: lookup 'map_jinja' configuration sources
[DEBUG   ] key: map_jinja:sources, ret: _|-
[DEBUG   ] key: libvirt:map_jinja:sources, ret: _|-
[DEBUG   ] map.jinja: load parameters with sources from ['osarch', 'os_family', 'os', 'osfinger', 'config_get_lookup', 'config_get', 'id']
[DEBUG   ] key: libvirt:strategy, ret: _|-
[DEBUG   ] key: libvirt:merge_lists, ret: _|-
[DEBUG   ] map.jinja: load parameters from file libvirt/parameters/osarch/amd64.yaml
[DEBUG   ] Could not find file 'salt://libvirt/parameters/osarch/amd64.yaml' in saltenv 'base'
[DEBUG   ] map.jinja: load parameters from file libvirt/parameters/os_family/Debian.yaml
[DEBUG   ] In saltenv 'base', looking at rel_path 'libvirt/parameters/os_family/Debian.yaml' to resolve 'salt://libvirt/parameters/os_family/Debian.yaml'
[DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/libvirt/parameters/os_family/Debian.yaml' to resolve 'salt://libvirt/parameters/os_family/Debian.yaml'
[DEBUG   ] map.jinja: merge parameters from libvirt/parameters/os_family/Debian.yaml
[DEBUG   ] LazyLoaded slsutil.merge
[DEBUG   ] map.jinja: load parameters from file libvirt/parameters/os/Debian.yaml
[DEBUG   ] Could not find file 'salt://libvirt/parameters/os/Debian.yaml' in saltenv 'base'
[DEBUG   ] map.jinja: load parameters from file libvirt/parameters/osfinger/Debian-10.yaml
[DEBUG   ] Could not find file 'salt://libvirt/parameters/osfinger/Debian-10.yaml' in saltenv 'base'
[DEBUG   ] map.jinja: retrieve formula lookup with 'config.get'
[DEBUG   ] key: libvirt:lookup, ret: _|-
[DEBUG   ] map.jinja: merge formula lookup retrieved with 'config.get', merge: strategy='smart', lists='False'
[DEBUG   ] map.jinja: retrieve formula configuration with 'config.get'
[DEBUG   ] map.jinja: merge formula configuration retrieved with 'config.get', merge: strategy='smart', lists='False'
[DEBUG   ] map.jinja: load parameters from file libvirt/parameters/id/82a3476d7f58.yaml
[DEBUG   ] Could not find file 'salt://libvirt/parameters/id/82a3476d7f58.yaml' in saltenv 'base'
[DEBUG   ] map.jinja: save parameters in variable 'libvirt_settings'

When you put a pillar libvirt:strategy to overwrite, you have

  • with salt-ssh
    log.error "map.jinja: the 'merge' option of 'config.get' is skipped with salt cli 'salt-ssh'"
    
  • with minion or salt-call
    [DEBUG   ] map.jinja: retrieve formula lookup with 'config.get', merge: strategy='overwrite'
    [DEBUG   ] Missing configuration file: /etc/salt/master
    [DEBUG   ] Using cached minion ID from /etc/salt/minion_id: cdade01817fc
    [DEBUG   ] map.jinja: merge formula lookup retrieved with 'config.get', merge: strategy='overwrite', lists='False'
    [DEBUG   ] map.jinja: retrieve formula configuration with 'config.get', merge: strategy='overwrite'
    

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@baby-gnu
Copy link
Contributor Author

I updated my splityaml to puts defaults.yaml values under a values key.

I tested this formula with salt-ssh and it's working fine:

Summary for testmachine3
------------
Succeeded: 8 (changed=7)
Failed:    0
------------
Total states run:     8
Total run time:  60.949 s

@baby-gnu baby-gnu force-pushed the feature/configurable-map.jinja branch 2 times, most recently from ebf307b to b1cf8e1 Compare March 24, 2020 21:51
Lookup the configuration for `map.jinja` from:

1. builtin default

   1. `osarch` grain
   2. `os_family` grain
   3. `os` grain
   4. `osfinger` grain
   5. `lookup` table retrived by `config.get`
   6. configuration retrived by `config.get`
   7. `id` grain

2. `defaults.yaml`: optionally define a formula specific
   `map_jinja:sources`

3. global configuration lookup `map_jinja:sources`

4. formula specific `<tplroot>:map_jinja:sources`

The hard coded default is backward compatible and add the minion id at
the end of the list.

If an entry does not match a `salt['config.get']` parameter, it's used
as a literal file path (with `.yaml` added if it's missing).

Each YAML file are formatted with the following top level keys:

- `values`: contains the the parameter values

- `strategy` (optional): define the merge strategy for the function
  `salt.slsutils.merge` (aggregate, list, overwrite, recurse,
  smart). The default is `smart`.

- `merge_lists` (optional): boolean to merge lists or overwrite them

It's possile to configure `config.get` when looking up the formula
configuration. You need to define a subkey `strategy` with the
following differences from YAML files:

- `strategy` can only be one of `None`, `overwrite` and `recurse`.
  The default is `None`.

- if you use `salt-ssh`, this merge strategy is skipped and and error
  log message is emitted.

The parameters values are merged in the following order:

1. initialize default values from `defaults.yaml`

2. merge the values from each source defined by the ordered list
   `map_jinja:sources`
* libvirt/parameters/defaults.yaml: replace the top level key
  `libvirt` by 'values`.
@baby-gnu baby-gnu force-pushed the feature/configurable-map.jinja branch from b1cf8e1 to 365f711 Compare March 24, 2020 23:06
Copy link
Member

@myii myii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work, @baby-gnu! The next generation of map.jinja has begun! All OK in the yaml_dump tests as well:

@myii myii merged commit 40ffb58 into saltstack-formulas:master Mar 24, 2020
@saltstack-formulas-travis

🎉 This PR is included in version 3.7.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants