Skip to content
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

Feature/template override and files switch #19

Conversation

baby-gnu
Copy link
Contributor

@baby-gnu baby-gnu commented Feb 6, 2019

This is the TOFS pattern from @moreda with the modifications I did in systemd-formula.

@aboe76
Copy link
Member

aboe76 commented Feb 6, 2019

@baby-gnu nice documentation.

TOFS_pattern.md Outdated

Among other functions, the _master_ (or _salt-master_) serves files to the _minions_ (or _salt-minions_). The [file_roots](http://docs.saltstack.com/en/latest/ref/file_server/file_roots.html) is the list of directories used in sequence to find a file when a minion requires it: the first match is served to the minion. Those files could be [state files](http://docs.saltstack.com/en/latest/topics/tutorials/starting_states.html) or configuration templates, among others.

Using Saltstack is a simple and effective way to implement configuration management, but even in a [non multitenant](http://en.wikipedia.org/wiki/Multitenancy) scenario, it's not a good idea to generally accesible some data (e.g. the database password in our [Zabbix](http://www.zabbix.com/) server configuration file or the private key of our [Nginx](http://nginx.org/en/) TLS certificate).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accesible => accessible.

TOFS_pattern.md Outdated
{% set default_servers = ['0.ubuntu.pool.ntp.org',
'1.ubuntu.pool.ntp.org',
'2.ubuntu.pool.ntp.org',
'3.ubuntu.pool.ntp.org']}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing %:

'3.ubuntu.pool.ntp.org'] %}

TOFS_pattern.md Outdated
{% endfor %}
```

This way we are localy **overriding the template files** offered by the formula in order to make a more complex adaptation. Of course, this could be applied as well to any of the files, including the state files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

localy => locally.


/etc/yyy/zzz.conf:
file:
- managed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use file.managed instead.

TOFS_pattern.md Outdated

/etc/yyy/zzz.conf:
file:
- managed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use file.managed instead.

TOFS_pattern.md Outdated

/etc/yyy/zzz.conf:
file:
- managed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use file.managed instead.


/etc/yyy/zzz.conf:
file:
- managed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use file.managed instead.

Copy link
Member

@noelmcloughlin noelmcloughlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - hope this formula evolves. thanks @baby-gnu

Copy link
Contributor

@vutny vutny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great review @myii

@myii
Copy link
Member

myii commented Feb 7, 2019

@vutny Thanks for the compliment.


I don't intend to hijack this discussion but this PR has reminded me of a more general situation in respect to these SaltStack formulas. With a number of active contributors involved in this PR discussion, hopefully someone can suggest a better place to talk about the points I mention below:

  • In discussions on the SaltStack Community Slack workspace, I've found some resistance to these SaltStack formulas.
  • Main issues appear to be that these formulas are too heavily reliant on using pillars (leading to excess load on the master) alongside general quality issues.

I'm not expecting anywhere near a complete solution to this. However, I believe it would be useful to compare ideas, to identify any (small) improvements that realistically could be made and maintained.

@baby-gnu
Copy link
Contributor Author

baby-gnu commented Feb 7, 2019

Thanks to all the contribution, I'll integrate them and push them as soon as possible (and I found an issue in macros.jinja ;-)).

Regards.

* TOFS_pattern.md: add myself as modifier.
  Trim trailing whitespaces.
  Separate titles from first paragraph.
The example could not be used as-is. This commit improve conformity to
formula conventions.

* TOFS_pattern.md: add missing commas “,” in “map.jinja” and extra one
  to ease the addition of new entries.
  Import “map.jinja” in “init.sls” and “conf.sls”.
  Declare descriptive state IDs.
  Use the “module.function” notation.
  Use the “name” parameter.
The prefix was used for 2 purposes:

- define the pillar prefix where to lookup “:files_switch”. It
  supports the colon “:” separator to lookup in pillar subtree like
  “foo:bar”
- define the path prefix where to look for “files/”, It did not support
  separator to lookup inside directory tree.

This patch only replace any colon “:” with “/” when looking up
“files/” directory, with the “foo:bar” prefix:

- lookup “foo:bar:files_switch” pillar to get list of grains to match
- lookup files under “salt://foo/bar/files/”

* TOFS_pattern.md: document the new use of “prefix” supporting colon “:”.

* template/macros.jinja: transform any colon “:” in “prefix” by slash
  “/” to lookup files.
When the files are “full path” with leading slash “/”, the generated
URL contain a double slash because of the join.

* template/macros.jinja: remove leading slash before joining parts.

* TOFS_pattern.md: mirror changes of “macros.jinja”.
@baby-gnu baby-gnu force-pushed the feature/Template-Override-and-Files-Switch branch from 224915c to 8bd2157 Compare February 7, 2019 16:18
The “template” is kept during rendering.

* TOFS_pattern.md: add “template” to rendered state.

* template/macros.jinja: ditoo.
@aboe76
Copy link
Member

aboe76 commented Feb 8, 2019

@myii I agree hijacking a discussion for the greater good...:)
I would love to see these salt-formula's become something like a community maintenance project.
something allong the line of vox pupuli https://voxpupuli.org/ but then for salt

@myii
Copy link
Member

myii commented Feb 8, 2019

@aboe76 I just had a quick look at https://voxpupuli.org/ and I'm impressed by the effort they have put in over the years. That far surpasses what I was envisaging! I'm intrigued by the idea -- but I don't think it's right to continue off-topic for this PR. Is there a more appropriate place for this discussion to be continued?

@aboe76
Copy link
Member

aboe76 commented Feb 8, 2019

@myii
Copy link
Member

myii commented Feb 8, 2019

OK, @aboe76 has contacted me in Slack and we're using the main formulas channel there to continue this discussion. Please feel free to join us there.

@baby-gnu
Copy link
Contributor Author

baby-gnu commented Feb 8, 2019

@myii Sorry but I do not use Slack for “political” reasons (we can't even see discussions without an account). I already accept to use GitHub to contribute to projects I like ;-)

Thanks for making this discussion alive.

Regards.

@myii
Copy link
Member

myii commented Feb 8, 2019

@baby-gnu I'm not particularly interested in keeping it on Slack. I'm just not sure Google Groups is the easiest place either. Can you suggest another place?

@myii
Copy link
Member

myii commented Feb 8, 2019

@baby-gnu @aboe76 How about something like a Matrix (riot.im) or Telegram group?

@baby-gnu
Copy link
Contributor Author

baby-gnu commented Feb 8, 2019

@myii @aboe76 I'm not sure I understood the purpose of this discussion, but if the SaltStack community wants to use Slack to organise the formula community I will not complain, I'll just be off the discussions.

I'm used to mailing-list and IRC, I make few exceptions to my avoidance of centralised services ;-)

@aboe76
Copy link
Member

aboe76 commented Feb 8, 2019

@baby-gnu salt has an irc channel on freenode, maybe create a salt-formulas channel there as well
and maybe link slack channel formulas with that one, so you and others won't be left out...

@aboe76 aboe76 merged commit 95e5e37 into saltstack-formulas:master Feb 8, 2019
@aboe76
Copy link
Member

aboe76 commented Feb 8, 2019

@baby-gnu merged, thanks for you contributions, and please continu your work.

@myii
Copy link
Member

myii commented Feb 8, 2019

@baby-gnu Matrix is decentralised as well. Besides, bridging between Matrix and IRC is simple, albeit a little ugly (in presentation). I'm good either way but there have been some very useful points made so far.

@myii
Copy link
Member

myii commented Feb 8, 2019

@baby-gnu Thanks for the work. @aboe76 Thanks for the merge.

@myii
Copy link
Member

myii commented Feb 9, 2019

All of the initial discussions have been collected on a wiki page. There have been some very interesting points made so I encourage you to have a look over it. I believe it contains a good basis for making improvements across the whole of SaltStack Formulas.

@baby-gnu
Copy link
Contributor Author

baby-gnu commented Feb 9, 2019

@aboe76 thanks for the merge. By the way, there is already a #salt-formulas IRC channel for github.com/salt-formulas, looks to be a separated from saltstack-formula ;-)

See you.

@baby-gnu baby-gnu deleted the feature/Template-Override-and-Files-Switch branch February 9, 2019 17:42
@javierbertoli
Copy link
Member

github.com/salt-formulas is an interesting project I saw in the past and to which I get back from time to time for inspiration. The first time I saw it was when I moved from chef to salt and was trying to find the community's formulas. I found that group and this.

The thing that prevented me for using salt-formulas and moved me to participate in saltstack-formulas is that their quickstart bootstrap is anything but simple. They write formulas in a consistent way, but at the cost (IMHO) of a too complex initial setup and a heavy inter-dependence between formulas (as I understand them, maybe I'm wrong). I think some ideas can be learned from their ecosystem too.

@aboe76
Copy link
Member

aboe76 commented Feb 10, 2019

@baby-gnu the github.com/salt-formulas are nice,
they rely on reclass or the newer saltclass to do the heavy lifting of pillar data merging or overwriting.

And I agree with @javierbertoli we can learn a few things from how they develop the formulas and stay consistent.

@myii
Copy link
Member

myii commented Feb 11, 2019

Ongoing discussions have been captured on another wiki page. More importantly, a SaltStack Formulas project has been created and populated with all of the discussion points so far.

@myii
Copy link
Member

myii commented Feb 13, 2019

CC: @baby-gnu @vutny @aboe76 @noelmcloughlin @javierbertoli @alxwr.

We're live!!! Thanks to @gtmanfred, #saltstack-formulas on Freenode is now bridged to the Slack #formulas room. Note, this is not #salt-formula, which is linked to https://github.com/salt-formulas as @baby-gnu mentioned above.

Any suggestions for decent logging of the IRC channel? There's some cracking discussions and I don't want them lost into the ether (or is that aether?). Nor do I fancy manually keeping logs in the wiki! I'm sure I've come across something in the past but I'll have to search again.

In any case, no excuses. See you on the other side.

BTW, @baby-gnu: there have been a number of conversations about this TOFS pattern approach. I can't do it justice. Your feedback would be invaluable.

@myii
Copy link
Member

myii commented Feb 13, 2019

CC: @baby-gnu @vutny @aboe76 @noelmcloughlin @javierbertoli @alxwr.

Sorry for the mass pingouts again but now there really is no excuse. IRC logging is in place, so you never have to miss any discussions: https://freenode.logbot.info/saltstack-formulas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants