fix(tofs): prepend the config-based `source_files` to the default #152

myii · 2019-07-23T16:11:42Z

feat(config): use separate defaults for all states nginx-formula#247 (comment)
- The main issue is that the nginx-formula has dynamic values being used as the default source_files -- there is no way to provide this from the pillar/config in a sensible fashion
- Prepending to this default (rather than overriding it) resolves this problem entirely, without adding excessive entries to the source
Closes Change file_switch to append source_file lookups, rather than override #151

CC: @baby-gnu.

myii · 2019-07-23T16:32:45Z

@baby-gnu If this implementation is accepted, I'll make the small changes necessary to the main TOFS document as well.

myii · 2019-07-23T17:01:21Z

@baby-gnu Actually, I've just gone ahead and done added the changes to the documentation as well.

baby-gnu · 2019-07-24T08:45:51Z

Thanks, I'm reviewing it.

baby-gnu

I agree with the changes but it's quite hard to see and understand the impact.

I set the kitchen log_level to debug and the difference between master and this PR is

--- master.out	2019-07-24 10:55:07.711690476 +0200
+++ pr.out	2019-07-24 10:55:21.223788394 +0200
@@ -3,9 +3,21 @@
            - name: /etc/template-formula.conf
            - source: 
              - salt://template/files/any/path/can/be/used/here/example.tmpl.jinja
-             - salt://template/files/ed264f5f1c9d/example.tmpl.jinja
+             - salt://template/files/any/path/can/be/used/here/example.tmpl
+             - salt://template/files/any/path/can/be/used/here/example.tmpl.jinja
+             - salt://template/files/cae0aef658c3/example.tmpl.jinja
+             - salt://template/files/cae0aef658c3/example.tmpl
+             - salt://template/files/cae0aef658c3/example.tmpl.jinja
+             - salt://template/files/Debian-9/example.tmpl.jinja
+             - salt://template/files/Debian-9/example.tmpl
              - salt://template/files/Debian-9/example.tmpl.jinja
              - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/Debian/example.tmpl
+             - salt://template/files/Debian/example.tmpl.jinja
              - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/Debian/example.tmpl
+             - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/default/example.tmpl.jinja
+             - salt://template/files/default/example.tmpl
              - salt://template/files/default/example.tmpl.jinja
            - mode: 644

I think we could modify the pillar.example to make more evident the differences:

diff --git a/pillar.example b/pillar.example
index 583a0d6..4b44ec3 100644
--- a/pillar.example
+++ b/pillar.example
@@ -42,15 +42,10 @@ template:
     # dirs:
     #   files: files_alt
     #   default: default_alt
-    # source_files:
-    #   template-config-file-file-managed:
-    #     - 'example_alt.tmpl'
-    #     - 'example_alt.tmpl.jinja'
-
-    # For testing purposes
     source_files:
       template-config-file-file-managed:
-        - 'example.tmpl.jinja'
+        - 'example_alt.tmpl'
+        - 'example_alt.tmpl.jinja'
 
   # Just for testing purposes
   winner: pillar

This will result in the following diff between master and the proposed changes of this PR:

--- master.out	2019-07-24 10:55:07.711690476 +0200
+++ alt.out	2019-07-24 10:55:33.151873999 +0200
@@ -2,10 +2,28 @@
          file.managed:
            - name: /etc/template-formula.conf
            - source: 
+             - salt://template/files/any/path/can/be/used/here/example_alt.tmpl
+             - salt://template/files/any/path/can/be/used/here/example_alt.tmpl.jinja
+             - salt://template/files/any/path/can/be/used/here/example.tmpl
              - salt://template/files/any/path/can/be/used/here/example.tmpl.jinja
-             - salt://template/files/ed264f5f1c9d/example.tmpl.jinja
+             - salt://template/files/cae0aef658c3/example_alt.tmpl
+             - salt://template/files/cae0aef658c3/example_alt.tmpl.jinja
+             - salt://template/files/cae0aef658c3/example.tmpl
+             - salt://template/files/cae0aef658c3/example.tmpl.jinja
+             - salt://template/files/Debian-9/example_alt.tmpl
+             - salt://template/files/Debian-9/example_alt.tmpl.jinja
+             - salt://template/files/Debian-9/example.tmpl
              - salt://template/files/Debian-9/example.tmpl.jinja
+             - salt://template/files/Debian/example_alt.tmpl
+             - salt://template/files/Debian/example_alt.tmpl.jinja
+             - salt://template/files/Debian/example.tmpl
              - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/Debian/example_alt.tmpl
+             - salt://template/files/Debian/example_alt.tmpl.jinja
+             - salt://template/files/Debian/example.tmpl
              - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/default/example_alt.tmpl
+             - salt://template/files/default/example_alt.tmpl.jinja
+             - salt://template/files/default/example.tmpl
              - salt://template/files/default/example.tmpl.jinja
            - mode: 644

Regards.

n-rodriguez · 2019-07-24T16:39:51Z

            - name: /etc/template-formula.conf
            - source: 
              - salt://template/files/any/path/can/be/used/here/example.tmpl.jinja
-             - salt://template/files/ed264f5f1c9d/example.tmpl.jinja
+             - salt://template/files/any/path/can/be/used/here/example.tmpl
+             - salt://template/files/any/path/can/be/used/here/example.tmpl.jinja
+             - salt://template/files/cae0aef658c3/example.tmpl.jinja
+             - salt://template/files/cae0aef658c3/example.tmpl
+             - salt://template/files/cae0aef658c3/example.tmpl.jinja
+             - salt://template/files/Debian-9/example.tmpl.jinja
+             - salt://template/files/Debian-9/example.tmpl
              - salt://template/files/Debian-9/example.tmpl.jinja
              - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/Debian/example.tmpl
+             - salt://template/files/Debian/example.tmpl.jinja
              - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/Debian/example.tmpl
+             - salt://template/files/Debian/example.tmpl.jinja
+             - salt://template/files/default/example.tmpl.jinja
+             - salt://template/files/default/example.tmpl
              - salt://template/files/default/example.tmpl.jinja
            - mode: 644

@myii :
Out of subject: do you think this could be done in pillar stack?
I know macros work in pillar stack since I use them in a private project ;)
Thanks!

myii · 2019-07-24T20:30:33Z

Out of subject: do you think this could be done in pillar stack?
I know macros work in pillar stack since I use them in a private project ;)

@n-rodriguez I'm sorry, I don't quite understand the question. I use PillarStack as well, so I'll be able to check what you're asking. Do you mean supplying different values to be processed by files_switch? Or actually avoiding TOFS files_switch entirely and making a PillarStack-based files_switch?

n-rodriguez · 2019-07-24T23:36:51Z

Do you mean supplying different values to be processed by files_switch? Or actually avoiding TOFS files_switch entirely and making a PillarStack-based files_switch?

I mean implementing files_switch in pillar stack. But it might be confused in my mind too ^^ Need to sleep on it ;)

* saltstack-formulas/nginx-formula#247 (comment) - The main issue is that the `nginx-formula` has dynamic values being used as the default `source_files` -- there is no way to provide this from the pillar/config in a sensible fashion - Prepending to this default (rather than overriding it) resolves this problem entirely, without adding excessive entries to the `source` * Closes saltstack-formulas#151

myii · 2019-07-25T01:50:04Z

@baby-gnu Resolved, to ensure that example.tmpl.jinja remains the priority, since that's required for testing. The (simplified) diff now becomes:

--- before
+++ after
@@ -1,4 +1,7 @@
     - source:
       - salt://template/files/ABC/example.tmpl.jinja
+      - salt://template/files/ABC/example.tmpl
       - salt://template/files/Debian/example.tmpl.jinja
+      - salt://template/files/Debian/example.tmpl
       - salt://template/files/default/example.tmpl.jinja
+      - salt://template/files/default/example.tmpl

myii · 2019-07-25T01:51:43Z

@baby-gnu If you're happy with this, please go ahead and merge it. I think everyone else leaves TOFS for us two to play with! Meaning, I don't think anyone else is waiting to review this...

myii · 2019-07-25T02:00:25Z

I mean implementing files_switch in pillar stack. But it might be confused in my mind too ^^ Need to sleep on it ;)

@n-rodriguez I can definitely see the angle on this. In a way, it would be nice to defer the processing to PillarStack instead. The biggest problem I can see is that it makes a dependency on (having to configure) PillarStack. And we're trying to avoid all dependencies for our formulas, at least at the current time.

Right now, I've got three components doing the same sort of thing but at different levels in the system:

PillarStack
TOFS
stack-formula (for providing YAML-based SDB configurations, that are accessed using config.get)

Combining all of these to use the same processing and directory structure? That would be very interesting.

myii · 2019-07-25T07:29:20Z

Thanks for the review and merge @baby-gnu. I'm about to propagate these changes to all of the formulas. I already started (saltstack-formulas/chrony-formula#23) but now I'll include this as well in one go.

saltstack-formulas-travis · 2019-07-25T07:55:55Z

🎉 This PR is included in version 3.1.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

myii requested a review from baby-gnu July 23, 2019 16:11

myii mentioned this pull request Jul 23, 2019

feat(config): use separate defaults for all states saltstack-formulas/nginx-formula#247

Open

myii force-pushed the bug/append-tofs-source-files branch from b778901 to 88ecbbe Compare July 23, 2019 16:17

myii force-pushed the bug/append-tofs-source-files branch from 88ecbbe to cb1f1bf Compare July 23, 2019 16:55

baby-gnu requested changes Jul 24, 2019

View reviewed changes

myii force-pushed the bug/append-tofs-source-files branch from cb1f1bf to 3483e76 Compare July 25, 2019 01:42

baby-gnu approved these changes Jul 25, 2019

View reviewed changes

baby-gnu merged commit 3425b5a into saltstack-formulas:develop Jul 25, 2019

myii deleted the bug/append-tofs-source-files branch July 25, 2019 07:28

saltstack-formulas-travis added the released label Jul 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tofs): prepend the config-based `source_files` to the default #152

fix(tofs): prepend the config-based `source_files` to the default #152

myii commented Jul 23, 2019 •

edited

Loading

myii commented Jul 23, 2019

myii commented Jul 23, 2019

baby-gnu commented Jul 24, 2019

baby-gnu left a comment

n-rodriguez commented Jul 24, 2019 •

edited

Loading

myii commented Jul 24, 2019

n-rodriguez commented Jul 24, 2019

myii commented Jul 25, 2019

myii commented Jul 25, 2019

myii commented Jul 25, 2019

myii commented Jul 25, 2019

saltstack-formulas-travis commented Jul 25, 2019

fix(tofs): prepend the config-based source_files to the default #152

fix(tofs): prepend the config-based source_files to the default #152

Conversation

myii commented Jul 23, 2019 • edited Loading

myii commented Jul 23, 2019

myii commented Jul 23, 2019

baby-gnu commented Jul 24, 2019

baby-gnu left a comment

Choose a reason for hiding this comment

n-rodriguez commented Jul 24, 2019 • edited Loading

myii commented Jul 24, 2019

n-rodriguez commented Jul 24, 2019

myii commented Jul 25, 2019

myii commented Jul 25, 2019

myii commented Jul 25, 2019

myii commented Jul 25, 2019

saltstack-formulas-travis commented Jul 25, 2019

fix(tofs): prepend the config-based `source_files` to the default #152

fix(tofs): prepend the config-based `source_files` to the default #152

myii commented Jul 23, 2019 •

edited

Loading

n-rodriguez commented Jul 24, 2019 •

edited

Loading