saltstack-formulas / mysql-formula

Install the MySQL client and/or server
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
85 stars 369 forks source link

feat(map.jinja): load a configurable list of YAML files #236

Closed baby-gnu closed 3 years ago

baby-gnu commented 4 years ago

PR progress checklist (to be filled in by reviewers)


What type of PR is this?

Primary type

Secondary type

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

Describe the changes you're proposing

I propose to modify map.jinja to load a configurable list of YAML files.

The configuration of map.jinja is lookup in the following order:

  1. formula specific defaults.yaml for the maintainer (can be overrode by the user)
  2. pillar map.jinja:sources to override globally the configuration for all formulas
  3. pillar mysql:map.jinja:sources to override the configuration only for this formula

I split all the os*map.yaml under parameters/<config>/<configvalue>.yaml.

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

The first sls using mapj.jinja is mysql.databases, here is the debug log during the import of map.jinja

       [DEBUG   ] In saltenv 'base', looking at rel_path 'mysql/database.sls' to resolve 'salt://mysql/database.sls'
       [DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/mysql/database.sls' to resolve 'salt://mysql/database.sls'
       [DEBUG   ] In saltenv 'base', looking at rel_path 'mysql/map.jinja' to resolve 'salt://mysql/map.jinja'
       [DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/mysql/map.jinja' to resolve 'salt://mysql/map.jinja'
       [DEBUG   ] Fetching file from saltenv 'base', ** attempting ** 'salt://mysql/map.jinja'
       [DEBUG   ] No dest file found
       [INFO    ] Fetching file from saltenv 'base', ** done ** 'mysql/map.jinja'
       [DEBUG   ] map.jinja: initialise parameters from mysql/parameters/defaults.yaml
       [DEBUG   ] In saltenv 'base', looking at rel_path 'mysql/parameters/defaults.yaml' to resolve 'salt://mysql/parameters/defaults.yaml'
       [DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/mysql/parameters/defaults.yaml' to resolve 'salt://mysql/parameters/defaults.yaml'
       [DEBUG   ] map.jinja: lookup map.jinja configuration sources
       [DEBUG   ] LazyLoaded config.get
       [DEBUG   ] key: map.jinja:sources, ret: _|-
       [DEBUG   ] key: mysql:map.jinja:sources, ret: _|-
       [DEBUG   ] map.jinja: load parameters with sources from ['osarch', 'os_family', 'os', 'osfinger']
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/osarch/amd64.yaml
       [DEBUG   ] Could not find file 'salt://mysql/parameters/osarch/amd64.yaml' in saltenv 'base'
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/os_family/Debian.yaml
       [DEBUG   ] Could not find file 'salt://mysql/parameters/os_family/Debian.yaml' in saltenv 'base'
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/os/Debian.yaml
       [DEBUG   ] In saltenv 'base', looking at rel_path 'mysql/parameters/os/Debian.yaml' to resolve 'salt://mysql/parameters/os/Debian.yaml'
       [DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/mysql/parameters/os/Debian.yaml' to resolve 'salt://mysql/parameters/os/Debian.yaml'
       [DEBUG   ] map.jinja: merge parameters from mysql/parameters/os/Debian.yaml
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/osfinger/Debian-9.yaml
       [DEBUG   ] In saltenv 'base', looking at rel_path 'mysql/parameters/osfinger/Debian-9.yaml' to resolve 'salt://mysql/parameters/osfinger/Debian-9.yaml'
       [DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/mysql/parameters/osfinger/Debian-9.yaml' to resolve 'salt://mysql/parameters/osfinger/Debian-9.yaml'
       [DEBUG   ] map.jinja: merge parameters from mysql/parameters/osfinger/Debian-9.yaml
       [DEBUG   ] map.jinja: save parameters in variable "mysql"

The following import of map.jinja is less verbose, for example, this is the mysql.user debug log comming just after the mysql.database:

       [DEBUG   ] In saltenv 'base', looking at rel_path 'mysql/user.sls' to resolve 'salt://mysql/user.sls'
       [DEBUG   ] In saltenv 'base', ** considering ** path '/tmp/kitchen/var/cache/salt/minion/files/base/mysql/user.sls' to resolve 'salt://mysql/user.sls'
       [DEBUG   ] map.jinja: initialise parameters from mysql/parameters/defaults.yaml
       [DEBUG   ] map.jinja: lookup map.jinja configuration sources
       [DEBUG   ] key: map.jinja:sources, ret: _|-
       [DEBUG   ] key: mysql:map.jinja:sources, ret: _|-
       [DEBUG   ] map.jinja: load parameters with sources from ['osarch', 'os_family', 'os', 'osfinger']
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/osarch/amd64.yaml
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/os_family/Debian.yaml
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/os/Debian.yaml
       [DEBUG   ] map.jinja: merge parameters from mysql/parameters/os/Debian.yaml
       [DEBUG   ] map.jinja: load parameters from file mysql/parameters/osfinger/Debian-9.yaml
       [DEBUG   ] map.jinja: merge parameters from mysql/parameters/osfinger/Debian-9.yaml
       [DEBUG   ] map.jinja: save parameters in variable "mysql"

Documentation checklist

Testing checklist

Additional context

baby-gnu commented 4 years ago

This is a first draft to see if it pass CI and to show to the community that it can be used quite easily.

I'll rebase this PR with properly split commits when everybody will agree.

The map.jinja for mysql-formula has few differences with the one from template-formula:

  1. the pythonpkg trick
  2. the postprocessing
--- template-formula/TEMPLATE/map.jinja 2020-02-15 13:37:13.000000000 +0100
+++ mysql-formula/mysql/map.jinja   2020-02-15 15:33:39.000000000 +0100
@@ -1,6 +1,13 @@
 # -*- coding: utf-8 -*-
 # vim: ft=jinja

+{# mysql-formula specific initialisation #}
+{%- set py_ver_settings = {
+      2: {'pythonpkg': 'python-mysqldb'},
+      3: {'pythonpkg': 'python3-mysqldb'},
+    } %}
+
+
 {#- Get the `tplroot` from `tpldir` #}
 {%- set tplroot = tpldir.split('/')[0] %}

@@ -12,6 +19,10 @@
 {%- do salt.log.debug('map.jinja: initialise parameters from ' ~ _defaults_filename ) %}
 {%- import_yaml _defaults_filename as default_settings %}

+{# Reproduce previous behaviour #}
+{%- set default_settings = salt.slsutil.merge(py_ver_settings[grains.pythonversion[0]],
+                                              default_settings) %}
+
 {# List of sources to lookup for parameters #}
 {%- do salt.log.debug('map.jinja: lookup map.jinja configuration sources') %}
 {# Fallback to previously used grains #}
@@ -66,12 +77,13 @@
 {# Merge the config dict #}
 {%- set config = salt.slsutil.merge(config, _config['formula']) %}

-{#- Change **TEMPLATE** to match with your formula's name and then remove this line #}
-{%- do salt.log.debug('map.jinja: save parameters in variable "TEMPLATE"') %}
-{%- set TEMPLATE = config %}
+{%- do salt.log.debug('map.jinja: save parameters in variable "mysql"') %}
+{%- set mysql = config %}

 {#- Post-processing for specific non-YAML customisations #}
 {%- if grains.os == 'MacOS' %}
-{%-   set macos_group = salt['cmd.run']("stat -f '%Sg' /dev/console") %}
-{%-   do TEMPLATE.update({'rootgroup': macos_group}) %}
+{%-   set macos_user = salt['pillar.get']('mysql:user', salt['cmd.run']("stat -f '%Su' /dev/console")) %}
+{%-   set macos_group = salt['pillar.get']('mysql:group', salt['cmd.run']("stat -f '%Sg' /dev/console")) %}
+{%-   do mysql.macos.update({'user': macos_user}) %}
+{%-   do mysql.macos.update({'group': macos_group}) %}
 {%- endif %}

Maybe we could have a uniq map.jinja loading an optional per formula custom.jinja?

Regards.

myii commented 4 years ago

@baby-gnu Thanks for setting this up. I've run a quick initial test locally and the map appears to be identical. When I get a chance, I intend to test this across all of the platforms, to compare the map output before and after.

baby-gnu commented 4 years ago

Thanks @myii.

I reproduce the failures on default-debian-10-master-py3 and default-ubuntu-1804-master-py3 with the branch master. So they do not look to be related to this PR.

myii commented 4 years ago

@baby-gnu So I've set up two jobs in Travis to show us the map.jinja output, before and after this PR (yd01 & yd02 respectively):

I'm running one instance for each platform and the same instances in both, so that we can get a comparison. So near the end of each Travis log you'll find the map output in YAML (the so called yaml_dump), for example (debian-10):

Results in the following diff:

--- yd01
+++ yd02
@@ -19,35 +19,13 @@
            client:
              port: 33306
              socket: /var/lib/mysql-socket/mysql.sock
-           isamchk:
-             key_buffer_size: 16M
            mysqld:
-             basedir: /usr
-             bind_address: 127.0.0.1
              datadir: ~/mysql/datadir
-             expire_logs_days: 10
-             key_buffer_size: 16M
-             lc_messages_dir: /usr/share/mysql
-             max_allowed_packet: 16M
-             max_binlog_size: 100M
-             pid_file: /var/run/mysqld/mysqld.pid
              port: 33306
-             query_cache_limit: 1M
-             query_cache_size: 16M
-             skip_external_locking: noarg_present
              socket: /var/run/mysqld/mysqld.sock
-             thread_cache_size: 8
-             thread_stack: 192K
-             tmpdir: /tmp
              user: myself
            mysqld_safe:
-             nice: 0
              plugin-dir: ~/mysql/plugins
-             socket: /var/run/mysqld/mysqld.sock
-           mysqldump:
-             max_allowed_packet: 16M
-             quick: noarg_present
-             quote_names: noarg_present
        database:
        - foo
        - name: bar

So the idea is to compare each platform to ensure that the map remains identical for each and every platform. At the moment, it will have to be a manual diff. Previously, I've been able to do something a little more sophisticated locally but I'm short on time at the moment, so this will have to do.

If you can look at the above and any other diff issues, that would be really useful. I'll try to help with the comparisons later on, when I get some time.

baby-gnu commented 4 years ago

This is because I made a huge mistake: osfamily instead of os_family :-/

https://travis-ci.org/myii/mysql-formula/jobs/651071506#L1217

myii commented 4 years ago

@baby-gnu No problem, let me re-run yd02 again with the fix.

myii commented 4 years ago

Re-running:

myii commented 4 years ago

@baby-gnu Excellent, that's doing better. debian-10, ubuntu-1804 & centos-8 are all identical. I'll return back to the rest of them later.

myii commented 4 years ago

I reproduce the failures on default-debian-10-master-py3 and default-ubuntu-1804-master-py3 with the branch master. So they do not look to be related to this PR.

@baby-gnu This is a bug reported upstream (for 3000) with a PR fix pending:

We could attempt to run the patch here to see if it works for us but that will have to be in a different PR.

baby-gnu commented 4 years ago

We could attempt to run the patch here to see if it works for us but that will have to be in a different PR.

I was wondering if it was due to my PR and it's not, I think we have enough work to do ;-)

myii commented 4 years ago

@baby-gnu @aboe76 Perfect! All 14 platforms are identical, so we're well on the way. So the remaining stages:

Please let me know if I missed something.

baby-gnu commented 3 years ago

I'll do a newt PR based on v5 map.jinja.