feat(tofs): implementation for all file.managed

[myii]

@sticky-note Thanks for starting this. There's some work to be done here. Similar to the nginx-formula PR, I've gone through and done a breakdown of the changes this brings. The issues need to be addressed. Also, pillar.example will have to be extended to cover all of the relevant states in some way or another. One further idea is that applying TOFS to ng formulas is not really the best idea -- better to promote the ng first before doing this -- let's see what the other think about this (CC: @alxwr @aboe76).

---

php.ng.fpm.pools_config

php_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

• Let's restrict to php.ini during this PR.

---

php.ng.hhvm.config

php_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

• Let's restrict to php.ini during this PR.

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

• Let's restrict to php.ini during this PR.

---

php.ng.fpm.config

php_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

• The duplications need to be removed here.

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

• Let's restrict to php.ini during this PR.

---

php.ng.apache2.ini

php_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

• The duplications need to be removed here.

---

php.ng.cli.ini

php_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

• The duplications need to be removed here.

---

php.ng.xcache.ini

php_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

• The duplications need to be removed here.

[aboe76]

@sticky-note Thanks for starting this. There's some work to be done here. Similar to the nginx-formula PR, I've gone through and done a breakdown of the changes this brings. The issues need to be addressed. Also, pillar.example will have to be extended to cover all of the relevant states in some way or another. One further idea is that applying TOFS to ng formulas is not really the best idea -- better to promote the ng first before doing this -- let's see what the other think about this (CC: @alxwr @aboe76).

@myii I think the order of merging TOFS shouldn't matter that much, if it's merge first in ng and later the ng will be merged in the non-ng formula. The only downside I can think of is that you have to handle the same code twice, to compensate for the restructuring of the pillar data.

[myii]

@aboe76 Sure, it can still work. My issue is with the excessive entries under the source, which can't be avoided when working with ng formulas. Once they've been promoted, we no longer face this issue.

[aboe76]

@myii, you have a valid point there. let wait with TOFS after merging ng

[myii]

@aboe76 The problem here is that semantic-release hasn't been enabled for this formula, which really should be done before promoting ng.

[n-rodriguez]

better to promote the ng first before doing this

:+1:

[n-rodriguez]

hasn't been enabled for this formula, which really should be done before promoting ng.

I'm working on it : https://github.com/saltstack-formulas/php-formula/pull/179 But I got one question : what should I test? The old formula or the new one? (ng)

[myii]

@n-rodriguez If the objective is to promote ng, then it's only worth testing that. I think the right thing to do would be to ask the main maintainers of the formula to see if they're ready for ng to be promoted (e.g. @alxwr @aboe76). Otherwise, we may be pushing too quickly.

[myii]

@n-rodriguez But there's no reason why we can't get the basic semantic-release up and running. In fact, I already started the process in anticipation of your PR yesterday, only to be disappointed to find that it was a WIP!

[n-rodriguez]

@n-rodriguez But there's no reason why we can't get the basic semantic-release up and running

I agree

In fact, I already started the process in anticipation of your PR yesterday, only to be disappointed to find that it was a WIP!

It's almost finished ;)

[myii]

Thanks to @n-rodriguez, semantic-release is now active for this formula (merged in #179, with updates in #180). So what's next? Continue with this PR or promote ng first?

[n-rodriguez]

So what's next? Continue with this PR or promote ng first?

I'd suggest to merge https://github.com/saltstack-formulas/php-formula/pull/177 and close https://github.com/saltstack-formulas/php-formula/pull/175 first.

[myii]

I'd suggest to merge #177 and close #175 first.

@n-rodriguez Efffectively done: https://github.com/saltstack-formulas/php-formula/pull/177#issuecomment-506979472. So now onto the next stage.

[n-rodriguez]

@n-rodriguez Efffectively done: #177 (comment). So now onto the next stage.

Great job! :+1:

[sticky-note]

Thanks to all for your work and reviews @myii Rebased and Force pushed your requested changes, How about passing crafted URLs array in a uniq filter before returning them in files_switch?

[sticky-note]

Brought some modification on my last push:

• Added tofs_lookup parameter to php_ini macro so we can pass in tofs lookup for files_switch
• Introduced tplroot on files I modified
• Updated pillar.example to reflect these changes and removed TOFS comments outside TOFS block
• Added {%- on files I modified @myii @n-rodriguez @aboe76 What to do next ?

[n-rodriguez]

@sticky-note great work :+1:

@myii @n-rodriguez @aboe76 What to do next ?

IMHO It would be nice to :

1. get rid of https://github.com/saltstack-formulas/php-formula/pull/175
2. promote ng
3. merge this PR

[sticky-note]

What about the serialize macro ? config objects are serialized when sent to jinja template so we retrieve an array of odicts on our custom templates. Commit was done 5 years ago -> https://github.com/saltstack-formulas/php-formula/pull/14/commits/c2435a397f7c0d4e50f714d456e7779682f8d814 What is the effective need of this ? Is it still relevant to use this or we can do something more TOFS friendly or we can refactor this entire part? Look forward to have your advices.

[myii]

1. get rid of #175

@n-rodriguez Can I get your opinions on https://github.com/saltstack-formulas/php-formula/pull/175#issuecomment-506979807 and https://github.com/saltstack-formulas/php-formula/pull/175#issuecomment-507184031? 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.

[sticky-note]

It's okay, but now I'm wondering if tofs is worth it here because of this serialize thing

[n-rodriguez]

If you still think it's not worthwhile, we can request a closure of that PR.

If https://github.com/saltstack-formulas/php-formula/pull/175 is too long to merge then I think we can close it and go further.

[myii]

@sticky-note Apologies, with the merge of #183, this will need amending before it can be reviewed and merged. If you're not happy about that, you're welcome to complain to the author of #183!!

[n-rodriguez]

@sticky-note be careful of #194

[sticky-note]

@myii :laughing: I've reworked this with v1.0.1 Questions: What do we do with php/apache2/files/php_mod.conf.jinja. I've managed to put it under php/apache2/files/default with v1_path_prefix = apache2. It ends with a source list not so much clean:

 /usr/local/etc/apache24/modules.d/050_mod_php.conf:
 (...)
              source:
                  - salt://php/apache2/files/minion/mod_php.conf.jinja
                  - salt://php/apache2/files/FreeBSD/mod_php.conf.jinja
                  - salt://php/apache2/files/default/mod_php.conf.jinja
                  - salt://php/files/minion/mod_php.conf.jinja
                  - salt://php/files/FreeBSD/mod_php.conf.jinja
                  - salt://php/files/default/mod_php.conf.jinja

Do we need to move it under php/files/default ?

With the introduction of versions list, I've written a different lookup name if someone use only one version and someone uses a list a versions. Is that looks good to you

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. How can we handle this problem ?

[myii]

@sticky-note I'll have a look at this when I get a chance... @n-rodriguez has been keeping me busy with multiple semantic-release PRs!!

[sticky-note]

Any chance to get that reviewed @myii @aboe76 @n-rodriguez

[myii]

@sticky-note When you rebase this, could you do it on the latest upstream? We want to get the tests running from the merge of #197.

Update: and the merge of #195.

[myii]

@sticky-note Please also copy across docs/TOFS_pattern.rst from the template-formula as well.

[sticky-note]

@myii, Done requested changes:

• Maintain layout consistency when using the php_ini and files_switch macro.
• Fixed the use of version.
• Updated php/libtofs.jinja
• Copied docs/TOFS_pattern.rst from template-formula
• Changed v1_path_prefix to use_subpath
• Removed inserted /ng while copied import paths
• Rebased with latest master commit

[myii]

@sticky-note Brilliant, thanks. I'll try to look at this over the next day or two.

[sticky-note]

@myii

[myii]

@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.

[myii]

@sticky-note Finally merged, thanks for your patience.

[saltstack-formulas-travis]

:tada: This PR is included in version 1.2.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

[sticky-note]

@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. How can we handle this problem ?

[myii]

@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 template from the pillar/config as well? Meaning, that it should be provided to the files_switch macro?

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 https://github.com/saltstack-formulas/php-formula/pull/178#pullrequestreview-287922147, 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"}]}]

[myii]

CC: @baby-gnu (starting from https://github.com/saltstack-formulas/php-formula/pull/178#issuecomment-531679947).

[baby-gnu]

Hello @myii, I'm not sure to understand the problem @sticky-note is speaking about.

[myii]

@baby-gnu Thanks, just wanted to draw this to your attention in case we need to improve libtofs a little more. Let's wait for a response from @sticky-note first.