[Discussion] TOFS/pillar.stack/stack-formula for map.jinja

alxwr commented 5 years ago

Currently map.jinja relies heavily on Pillar data. This shifts the load to the master which does not scale well. With the recent introduction of config.get by @myii other data sources are accessible¹, but there are some caveats:

no merging (not implemented in salt-ssh)
merging not configurable (as in pillar.stack
fixed lookup sequence¹

@claudekenni implemented a prototype mimicking pillar.stack: https://github.com/claudekenni/stack-formula He also wrote an execution module https://github.com/claudekenni/stack_module which provides stack.items, stack.get and others, and a SDB module: https://github.com/claudekenni/sdbstack.

How can we take the experience of TOFS, pillar.stack and @claudekenni's prototypes and move ~80% of our Pillar data onto the minions?

with configurable lookup sequence
with configurable merging
with support for various data sources (Like Pillar, which supports i.e. Jinja and the Py renderer.)
with computation on the minion
with support for salt (ZMQ), salt-call (local) and salt-ssh (SSH)
while still supporting Pillar for secrets
while still keeping the formulas independent of one another and independent of code which is not part of the Salt Module Index

¹ SDB <; Minion configuration <; Minion's grains <; Minion's pillar data < ... (credit: @myii)

Common checklist

@alxwr Hope you don't mind me hijacking this part of the comment as a common checklist for anyone to edit! It shows up properly in the GitHub UI if it's in the first comment:

[x] [Ref] Make the tests pass with the new config.get on existing formulas with semantic-release and a valid test suite -- PR.

alxwr commented 5 years ago

Maybe it's a good idea to implement #103 on the way.

myii commented 5 years ago

This is an incredibly important discussion, so I'm just going to invite the cavalry to start bringing in their opinions (only inviting to the discussion, not forcing to participate). By the way, I reckon we're going have to break this down into multiple stages. There's already a lot going on at a basic level, with the introduction of config.get. There are so many options available, that it may take some time to work through them.

myii commented 5 years ago

Please don't be offended if I've invited you (when you rather wouldn't have been) or if I've forgotten to invite you (because salt-ssh has blown a hole through my brain!).

TL;DR: config.get has opened up a whole new direction for SaltStack Formulas to develop towards. We can help one another find the right path(s) to take.

n-rodriguez commented 5 years ago

Congratulations at @myii, @alxwr and others for their work.

But I would say as a fist step : why not make the tests pass with the new config.get on exisiting formulas with semantic-release and a valid test suite :)

myii commented 5 years ago

Thanks @n-rodriguez, that's prompted me to start the checklist at the top of this issue.

alxwr commented 5 years ago

@myii You are allowed to hijack the first comment. :+1: :smile:

daks commented 5 years ago

Currently map.jinja relies heavily on Pillar data.

Can we really say that? map.jinja relies on pillars when user wants to override default values with the nearly-standard <formula>:lookup dict. As this means overriding core configuration, I consider it to be for edge-cases not standard formula use. map.jinja itself in 'normal' use is YAML merging, pillars are not involved.

This shifts the load to the master which does not scale well. With the recent introduction of config.get by @myii other data sources are accessible¹,

As I already said before: we (Saltstack itself, community...) don't already have solutions to propose to get out of pillars, so it always seems premature to me to push people out of pillars when we don't have anything to propose. And this had a lot of code, so complexity to the formula, with no real benefit at the moment.

This all config.get change seems today really buggy (the hacks inside template-formula to manage salt-ssh are really hacky/ugly)[1] and premature.

[1] Those should not be in the formulas but in salt itself, in my opinion

baby-gnu commented 4 years ago

Hello.

I'm seting up of a new infrastructure and I have a little time to think about how to properly do things.

I thought quite a lot abouth differents discussions on the maintainability of the formula:

In the maintainance of libvirt-formula, one of the problem I have is “how to maintain only the distributions in which I'm interested whithout putting barrier for people to manage others?”.

I think we could start by generalizing how map.jinja works the same way we do with libtofs.jinja:

load <tplroot>/defaults.yaml as default_settings
use config.get to retrieve a list of places to look for, for example <tplroot>:values:sources with the default value ['osarch', 'os_family', 'os', 'osfinger']
for each <item> return by step 2
1. import_yaml the file <tplroot>/<item>/<config.get(<item>)>.yaml
2. merge the imported values
retrieve with config.get the lookup dict and merge it

The actual os*map.yaml will result in the following tree:

<tplroot>/
├── map.jinja
├── defaults.yaml
├── os
│   ├── Amazon.yaml
│   ├── CentOS.yaml
│   ├── Fedora.yaml
│   ├── Funtoo.yaml
│   ├── Manjaro.yaml
│   ├── openSUSE.yaml
│   ├── Raspbian.yaml
│   ├── SmartOS.yaml
│   ├── SUSE.yaml
│   └── Ubuntu.yaml
├── osarch
│   ├── 386.yaml
│   ├── amd64.yaml
│   ├── arm64.yaml
│   ├── armv6l.yaml
│   ├── armv7l.yaml
│   ├── ppc64le.yaml
│   ├── s390x.yaml
│   └── x86_64.yaml
├── osfamily
│   ├── Alpine.yaml
│   ├── Arch.yaml
│   ├── Debian.yaml
│   ├── FreeBSD.yaml
│   ├── Gentoo.yaml
│   ├── MacOS.yaml
│   ├── OpenBSD.yaml
│   ├── RedHat.yaml
│   ├── Solaris.yaml
│   ├── Suse.yaml
│   └── Windows.yaml
└── osfinger
    ├── Amazon Linux-2.yaml
    ├── Amazon Linux AMI-2018.yaml
    ├── CentOS-6.yaml
    ├── CentOS Linux-7.yaml
    ├── CentOS Linux-8.yaml
    ├── Debian-10.yaml
    ├── Debian-8.yaml
    ├── Debian-9.yaml
    ├── Fedora-30.yaml
    ├── Fedora-31.yaml
    ├── FreeBSD-12.yaml
    ├── Gentoo-2.yaml
    ├── Leap-15.yaml
    ├── Ubuntu-16.04.yaml
    ├── Ubuntu-18.04.yaml
    └── Windows-8.1.yaml

NB: we could groups defaults.yaml and all the directories under a parameters/ directory (or any other meaningful name)

This will permits:

to maintain only the supported distributions
to let people provide missing YAML by putting the file in the proper place in file_roots
to override single YAML without the need to maintain others by putting the file in proper place in file_roots (before to override)
to avoid using pillars for non secret values by modifing the <tplroot>:values:sources, for example:
- add some value sources: ['osarch', 'os_family', 'os', 'osfinger', 'domain', 'roles', 'id']
- add the proper YAML files: domain/example.net.yaml, domain/example.org.yaml, roles/web.yaml, roles/ssh.yaml, id/minion1.example.org.yaml
to make a great use of gitfs

We could think about adding a config.get option to make map.jinja use import_json instead of import_yaml if required.

The only thing I don't know is how to configure the merging. Do we have access to some salt functions to do that like pillar.stack does?

Regards.

baby-gnu commented 4 years ago

We could do the merge with salt.slsutil.merge which support:

merging strategy (aggregate, list, overwrite, recurse, smart)
merging of lists

For example, YAML files could look like:

strategy: 'overwrite'
merge_lists: 'true'
values:
  foo: 1
  bar:
    - 'a'
    - 'b'
    - 'c'

we then load the YAML files with something like:

{%- import_yaml tlproot ~ "/defaults.yaml" as default_settings %}

{%- import_yaml tlproot ~ "/somefile.yaml" as somefile %}
{%- set config = salt.slsutil.merge(default_settings,
                                    somefile.get('values', {}),
                                    strategy=somefile.get('strategy', 'smart'),
                                    merge_lists=somefile.get('merge_lists', False) | to_bool
                                   ) %}

I made some tests and this is working with salt-ssh.

baby-gnu commented 4 years ago

Now, I need a way to make it works even when the file does not exists.

baby-gnu commented 4 years ago

I found a way to handle missing files:

{%- import_yaml tlproot ~ "/defaults.yaml" as default_settings %}

{%- set yamlfiles = ["file1.yaml", "file2.yaml", "file3.yaml"] %}
{%- set _config = {'values': default_settings} %}

{%- for yamlfile in yamlfiles %}
  {%- load_yaml as  loaded_values %}
    {%- include yamlfile ignore missing %}
  {%- endload %}

  {%- if loaded_values %}
    {%- do _config.update({'values': salt.slsutil.merge(_config['values'],
                                                        loaded_values.get('values', {}),
                                                        strategy=loaded_values.get('strategy', 'smart'),
                                                        merge_lists=loaded_values.get('merge_lists', False) | to_bool)})
      %}
  {%- endif %}
{%- endfor %}

{%- set config = _config['values'] %}

So, now I'll be able to provide a pull request ;-)

saltstack-formulas / template-formula

[Discussion] TOFS/pillar.stack/stack-formula for map.jinja #116

Common checklist