-
Notifications
You must be signed in to change notification settings - Fork 229
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(tofs): implementation for all file.managed #178
Conversation
@sticky-note Thanks for starting this. There's some work to be done here. Similar to the php.ng.fpm.pools_configphp_fpm_pool_conf_0:
file.managed:
...
- name: /location/of/php-fpm/pool.d/mypool.conf
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/mypool.conf
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/mypool.conf
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/mypool.conf
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/mypool.conf
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/mypool.conf
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/mypool.conf
+ - salt://php/ng/files/default/php.ini
php.ng.hhvm.configphp_hhvm_ini_config:
file.managed:
...
- name: /etc/hhvm/server.ini
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/server.ini
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/server.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/server.ini
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/server.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/server.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/server.ini
+ - salt://php/ng/files/default/php.ini
php_hhvm_conf_config:
file.managed:
...
- name: /etc/hhvm/php.ini
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/php.ini
+ - salt://php/ng/files/default/php.ini
php.ng.fpm.configphp_fpm_ini_config:
file.managed:
...
- name: /location/of/php-fpm/php.ini
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/php.ini
+ - salt://php/ng/files/default/php.ini
php_fpm_conf_config:
file.managed:
...
- name: /location/of/php-fpm/config.conf
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/config.conf
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/config.conf
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/config.conf
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/config.conf
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/config.conf
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/config.conf
+ - salt://php/ng/files/default/php.ini
php.ng.apache2.iniphp_apache2_ini:
file.managed:
...
- name: /etc/php/7.0/apache2/php.ini
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/php.ini
+ - salt://php/ng/files/default/php.ini
php.ng.cli.iniphp_cli_ini:
file.managed:
...
- name: /location/of/php-cli/php.ini
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/php.ini
+ - salt://php/ng/files/default/php.ini
php.ng.xcache.iniphp_xcache_ini:
file.managed:
...
- name: /location/of/php-xcache/php.ini
- - source: salt://php/ng/files/php.ini
+ - source:
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/ABC/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/files/default/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/ABC/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/Debian/php.ini
+ - salt://php/ng/files/default/php.ini
+ - salt://php/ng/files/default/php.ini
|
@myii I think the order of merging TOFS shouldn't matter that much, if it's merge first in |
@aboe76 Sure, it can still work. My issue is with the excessive entries under the |
@myii, you have a valid point there. let wait with |
@aboe76 The problem here is that |
👍 |
I'm working on it : #179 |
@n-rodriguez If the objective is to promote |
@n-rodriguez But there's no reason why we can't get the basic |
I agree
It's almost finished ;) |
Thanks to @n-rodriguez, |
@n-rodriguez Efffectively done: #177 (comment). So now onto the next stage. |
Great job! 👍 |
dd70f0f
to
356b8d3
Compare
Thanks to all for your work and reviews |
e2cd7dc
to
71b59fd
Compare
Brought some modification on my last push:
|
@sticky-note great work 👍
IMHO It would be nice to :
|
What about the |
@n-rodriguez Can I get your opinions on #175 (comment) and #175 (comment)? If you still think it's not worthwhile, we can request a closure of that PR. @sticky-note Thanks for your patience, I'll try to get around to checking your latest changes soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sticky-note Thanks for rebasing this. There are a number of changes, many of them necessary. After these are made, we can look at testing this PR for appropriate results.
@sticky-note When you rebase this, could you do it on the latest Update: and the merge of #195. |
@sticky-note Please also copy across |
d0b0ffd
to
bab5f08
Compare
@myii, Done requested changes:
|
@sticky-note Brilliant, thanks. I'll try to look at this over the next day or two. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sticky-note Just a couple of changes from the eyeball review. I'll run this over the next day or two as mentioned above.
bab5f08
to
7266b0c
Compare
@sticky-note Don't worry, I haven't forgotten! I'm just working through some other stuff, as I get time to work on it. This PR is near the top of the list. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a missing comma but otherwise fine.
php/apache2/ini.sls
Outdated
|
||
php_apache2_ini: | ||
{{ php_ini(php.lookup.apache2.ini, php.apache2.ini.opts, settings) }} | ||
{{ php_ini(php.lookup.apache2.ini, | ||
'php_apache2_ini' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'php_apache2_ini' | |
'php_apache2_ini', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sticky-note Don't worry, I've fixed it and rebased.
- Implementation of libtofs on ini macro, pools_config and apache2 mod_php.conf + Introduction of tplroot on modified files + `{%-` consistency when possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sticky-note Diff is as expected, this is ready to be merged after the tests have passed:
--- Before
+++ After
@@ -3,7 +3,10 @@
php_apache2_ini:
file.managed:
- name: /etc/php/5.6/apache2/php.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
[DEBUG ] Rendered data from file: /tmp/kitchen/var/cache/salt/minion/files/base/php/apache2/install.sls:
# -*- coding: utf-8 -*-
@@ -14,7 +17,13 @@
- pkgs: ["libapache2-mod-php5.6", "libapache2-mod-php5.6", "libapache2-mod-php7.3"]
/usr/local/etc/apache24/modules.d/050_mod_php.conf:
file.managed:
- - source: salt://php/apache2/files/mod_php.conf.jinja
+ - source:
+ - salt://php/apache2/files/ba9615bfb8b4/mod_php.conf.jinja
+ - salt://php/apache2/files/Debian/mod_php.conf.jinja
+ - salt://php/apache2/files/default/mod_php.conf.jinja
+ - salt://php/files/ba9615bfb8b4/mod_php.conf.jinja
+ - salt://php/files/Debian/mod_php.conf.jinja
+ - salt://php/files/default/mod_php.conf.jinja
- template: jinja
- makedirs: true
- require_in:
@@ -28,24 +37,36 @@
php_cli_ini_5.6:
file.managed:
- name: /etc/php/5.6/cli/php.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
php_cli_ini_7.3:
file.managed:
- name: /etc/php/7.3/cli/php.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
[DEBUG ] Rendered data from file: /tmp/kitchen/var/cache/salt/minion/files/base/php/fpm/config.sls:
# Manages the php-fpm main ini file
php_fpm_ini_config_5.6:
file.managed:
- name: /etc/php/5.6/fpm/php.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
php_fpm_conf_config_5.6:
file.managed:
- name: /etc/php/5.6/fpm/php-fpm.conf
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
- context:
config: [{"global": [{"pid": "/var/run/php5.6-fpm.pid"}, {"error_log": "/var/log/php5.6-fpm.log"}]}, {"include": "/etc/php/5.6/fpm/pool.d/*.conf"}]
@@ -59,12 +80,18 @@
php_fpm_ini_config_7.3:
file.managed:
- name: /etc/php/7.3/fpm/php.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
php_fpm_conf_config_7.3:
file.managed:
- name: /etc/php/7.3/fpm/php-fpm.conf
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
- context:
config: [{"global": [{"pid": "/var/run/php7.3-fpm.pid"}, {"error_log": "/var/log/php7.3-fpm.log"}]}, {"include": "/etc/php/7.3/fpm/pool.d/*.conf"}]
@@ -81,14 +108,20 @@
php_fpm_pool_conf_0:
file.managed:
- name: /etc/php/5.6/fpm/pool.d/radius-admin.conf
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
- context:
config: [{"radius-admin": [{"user": "www-data"}, {"group": "www-data"}, {"listen": "/tmp/php-fpm-radius-admin.sock"}, {"listen.mode": "0666"}, {"pm": "static"}, {"pm.max_children": 3}, {"pm.max_requests": 500}, {"pm.status_path": "/php-status"}, {"ping.path": "/php-ping"}, {"catch_workers_output": "yes"}, {"security.limit_extensions": ".php"}, {"php_admin_value[date.timezone]": "Europe/Paris"}]}]
php_fpm_pool_conf_1:
file.managed:
- name: /etc/php/7.3/fpm/pool.d/ldap-admin.conf
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
- context:
config: [{"ldap-admin": [{"user": "www-data"}, {"group": "www-data"}, {"listen": "/tmp/php-fpm-ldap-admin2.sock"}, {"listen.mode": "0666"}, {"pm": "static"}, {"pm.max_children": 3}, {"pm.max_requests": 500}, {"pm.status_path": "/php-status"}, {"ping.path": "/php-ping"}, {"catch_workers_output": "yes"}, {"security.limit_extensions": ".php"}, {"php_admin_value[date.timezone]": "Europe/Paris"}]}]
@@ -97,21 +130,30 @@
php_hhvm_ini_config:
file.managed:
- name: /etc/hhvm/server.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
- context:
config: [{"pid": "/var/run/hhvm/pid"}, {"hhvm.server.port": "9000"}, {"hhvm.server.type": "fastcgi"}, {"hhvm.server.default_document": "index.php"}, {"hhvm.log.use_log_file": "true"}, {"hhvm.log.file": "/var/log/hhvm/error.log"}, {"hhvm.repo.central.path": "/var/run/hhvm/hhvm.hhbc"}]
php_hhvm_conf_config:
file.managed:
- name: /etc/hhvm/php.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
[DEBUG ] Rendered data from file: /tmp/kitchen/var/cache/salt/minion/files/base/php/xcache/ini.sls:
# Manages the php cli main ini file
php_xcache_ini:
file.managed:
- name: /etc/php/conf.d/xcache.ini
- - source: salt://php/files/php.ini
+ - source:
+ - salt://php/files/ba9615bfb8b4/php.ini
+ - salt://php/files/Debian/php.ini
+ - salt://php/files/default/php.ini
- template: jinja
- context:
config: [{"xcache-common": [{"extension": "xcache.so"}]}, {"xcache": [{"xcache.size": "90M"}]}]
@sticky-note Finally merged, thanks for your patience. |
🎉 This PR is included in version 1.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
@myii Last thing, actually this way, TOFS is not usable in personal templates because of https://github.com/saltstack-formulas/php-formula/pull/178/files#diff-07689a57bcdaf996c5d9e3bd9099e831R15 without including original template or importing the php_block macro. |
@sticky-note Do you mean something like this? --- master
+++ proposed
@@ -5,14 +5,14 @@
{%- from tplroot ~ "/macro.jinja" import sls_block, serialize %}
{%- from tplroot ~ "/libtofs.jinja" import files_switch with context %}
-{% macro php_ini(filename, tofs_lookup, opts={}, settings={}) %}
+{% macro php_ini(filename, tofs_lookup, opts={}, settings={}, template='jinja') %}
file.managed:
{{ sls_block(opts) }}
- name: {{ filename }}
- source: {{ files_switch(['php.ini'],
tofs_lookup
) }}
- - template: jinja
+ - template: {{ template }}
- context:
config: {{ serialize(odict(settings)) }}
{%- endmacro -%} Or do you mean supplying the For clarity, could you provide a sample state of what you are hoping to achieve? It will be easier to discuss that way. Please provide it in a rendered example, similar to the examples shown in #178 (review), e.g. php_xcache_ini:
file.managed:
- name: /etc/php/conf.d/xcache.ini
- source:
- salt://php/files/ba9615bfb8b4/php.ini
- salt://php/files/Debian/php.ini
- salt://php/files/default/php.ini
- template: jinja
- context:
config: [{"xcache-common": [{"extension": "xcache.so"}]}, {"xcache": [{"xcache.size": "90M"}]}] |
CC: @baby-gnu (starting from #178 (comment)). |
Hello @myii, I'm not sure to understand the problem @sticky-note is speaking about. |
@baby-gnu Thanks, just wanted to draw this to your attention in case we need to improve |
No description provided.