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

[Discussion] [Idea] Move all utilities for formulas outside of edited files (states, yaml) #122

Open
daks opened this issue May 20, 2019 · 10 comments

Comments

@daks
Copy link
Member

daks commented May 20, 2019

On several discussions about TOFS or config.get, I have expressed my main problem with those 'tools': they clutter/complicate the files that contributors needs to write.

My point of view on this is that such basic/simple formulas tools should not complicate any new comer which want to create a simple formula without any knowledge of them before.
They are too intrusive to keep this principe.

Part of TOFS is already abstracted in libtofs.jinja but it still needs 4-5 lines for each file.managed.
I know less config.get but I think it also had a lot of code (can't find now the PR I am looking) and especialy to deal with cases like salt-ssh.

I wonder if there is some way to do it. Puppet modules uses lib to add specific code. Ansible roles seems to use handlers.
Couldn't _modules or _states or whatever be used to do it? Even if we need to convert jinja/state code to Python.

cc @myii, @javierbertoli @aboe76 @n-rodriguez to open-mindly discuss this idea :)

@aboe76
Copy link
Member

aboe76 commented May 20, 2019

@daks I think you have point there, _modules should be picked up by salt if you put them in the formula.

But you have more those are called extension_mdules
https://docs.saltstack.com/en/latest/ref/configuration/master.html#extension-modules

and https://github.com/saltstack/salt-contrib has a bunch of them

@aboe76
Copy link
Member

aboe76 commented May 20, 2019

Or get them to add libtofs, into the core state system.

@alxwr
Copy link
Member

alxwr commented May 21, 2019

@daks Thanks for posting an alternative approach to #116!

I know less config.get but I think it also had a lot of code (can't find now the PR I am looking) and especialy to deal with cases like salt-ssh.

Maybe #102 and #115?

https://github.com/saltstack-formulas/template-formula/blob/91bc2f046b3629b9f1f66ed0eacddb626348da6c/template/map.jinja

This is roughly the same amount of code which was needed before with pillar.get. @myii and I just used more lines to increase readability. Really the main difference this: pillar.get got replaced by config.get and all merging is now done by grains.filter_by.
The recent fuzz about salt-ssh comes from minor implementation differences between it and regular salt (and my stumbling over them). The merging (which makes up most of the code) has to be done for salt too.

@daks I wholeheartedly agree with your point, that using _modules and/or _states is a nicer approach. I.e. states.file.tofs¹ and configstack.get² would be much cleaner and make a formula much more maintainable than the current way.

Formulas are stand-alone at the moment and IMHO we should keep it that way as much as possible to avoid dependency hell. The moment we move towards shared modules and states we'll have to take care of backwards-compatibility, which is doable but adds complexity.

In #102 #115 we deliberately used the methods Salt provides and stayed within the map.jinja approach. With the limitations came the code to cope with them. :-)
If there is a broad consensus that we want to move towards bundling states and modules with template-formula (or somewhere else)³, then I'd be happy to help.

In short I see two approaches at the moment:

  1. Only depend on the core and use what it provides. => map.jinja/config.get & libtofs.jinja
  2. Depend on additional modules (i.e. file.tofs and configstack.get) wherever they may 'live'.

In the hopes to enrich the debate.


¹ like file.managed, but with TOFS built in.
² encapsulating the map.jinja logic plus config.get and/or pillar.get (and maybe configurable to add even more data sources)
³ This could become a mini library (for formula purposes only), where a new developer can start without it but just use its methods when they are needed.
In a perfect world this would already be present in the core lib, but IMHO it's a good idea to keep formula-specific stuff with the formulas so they don't depend on particular versions of Salt.

@myii
Copy link
Member

myii commented May 21, 2019

@daks Thanks for this, important points to raise as the reshaping of SaltStack Formulas is progressing. I'd like to contribute to this discussion as well but time is a bit short at the moment. I hope to get a chance over the coming days.

@daks
Copy link
Member Author

daks commented May 23, 2019

@myii no problem. I don't have that much time myself currently.
My objective was to write down my ideas, share them with all of you, read what you think about. Moreover now everyone can start thinking about solutions at its own pace :)

@daks
Copy link
Member Author

daks commented May 24, 2019

@daks I wholeheartedly agree with your point, that using _modules and/or _states is a nicer approach. I.e. states.file.tofs¹ and configstack.get² would be much cleaner and make a formula much more maintainable than the current way.

I had thought about something like that too. Shouldn't be hard to POC.
Not sure if I like the idea of having states named differently (in a perfect world ;) but it's still a solution.

I wonder if trying to override the file.managed and config.get state modules could be possible.
Probably not simply: _modules should be after the original in loading path.

Formulas are stand-alone at the moment and IMHO we should keep it that way as much as possible to avoid dependency hell. The moment we move towards shared modules and states we'll have to take care of backwards-compatibility, which is doable but adds complexity.

Not having dependencies will be limiting at one moment in the future, but for now we don't have tooling and not a lot of needs (not sure if some formulas has dependencies).

Also having a "stdlib" formula could completely remove the problem of having to port template-formula changes to other formulas.

In short I see two approaches at the moment:

1. Only depend on the core and use what it provides. => `map.jinja`/`config.get` & `libtofs.jinja`

2. Depend on additional modules (i.e. `file.tofs` and `configstack.get`) wherever they may 'live'.

I think this is worth testing. Not sure I'll have time soon though.

Thanks all for commenting.

In the hopes to enrich the debate.

¹ like file.managed, but with TOFS built in.
² encapsulating the map.jinja logic plus config.get and/or pillar.get (and maybe configurable to add even more data sources)
³ This could become a mini library (for formula purposes only), where a new developer can start without it but just use its methods when they are needed.
In a perfect world this would already be present in the core lib, but IMHO it's a good idea to keep formula-specific stuff with the formulas so they don't depend on particular versions of Salt.

@aboe76
Copy link
Member

aboe76 commented Jul 31, 2019

@daks I wholeheartedly agree with your point, that using _modules and/or _states is a nicer approach. I.e. states.file.tofs¹ and configstack.get² would be much cleaner and make a formula much more maintainable than the current way.

I had thought about something like that too. Shouldn't be hard to POC.
Not sure if I like the idea of having states named differently (in a perfect world ;) but it's still a solution.

I wonder if trying to override the file.managed and config.get state modules could be possible.
Probably not simply: _modules should be after the original in loading path.

Formulas are stand-alone at the moment and IMHO we should keep it that way as much as possible to avoid dependency hell. The moment we move towards shared modules and states we'll have to take care of backwards-compatibility, which is doable but adds complexity.

Not having dependencies will be limiting at one moment in the future, but for now we don't have tooling and not a lot of needs (not sure if some formulas has dependencies).

Also having a "stdlib" formula could completely remove the problem of having to port template-formula changes to other formulas.

The "stdlib" approach is what puppet has done, which brings a whole lot of things, like versioning on the table, it works because they have the infrastructure to pull versioned modules.
Same thing as with ansible approach is the infrastructure to pull from galaxy or git on versions or branches,

At the moment saltstack-formulas is getting in better shape for doing this kind of work but where missing a key part, that is a tool/infrastructure to serve this from.

In short I see two approaches at the moment:

1. Only depend on the core and use what it provides. => `map.jinja`/`config.get` & `libtofs.jinja`

2. Depend on additional modules (i.e. `file.tofs` and `configstack.get`) wherever they may 'live'.

I propose a third solution:
3. have the formulas stay as standalone as possible to have maximum flexibility having a common way of structuring a formula to make it quicker to learn is the way forward.

I think this is worth testing. Not sure I'll have time soon though.

I don't see an issue with testing the other solutions.

Thanks all for commenting.

In the hopes to enrich the debate.
¹ like file.managed, but with TOFS built in.
² encapsulating the map.jinja logic plus config.get and/or pillar.get (and maybe configurable to add even more data sources)
³ This could become a mini library (for formula purposes only), where a new developer can start without it but just use its methods when they are needed.
In a perfect world this would already be present in the core lib, but IMHO it's a good idea to keep formula-specific stuff with the formulas so they don't depend on particular versions of Salt.

@n-rodriguez
Copy link
Member

n-rodriguez commented Aug 1, 2019

  1. have the formulas stay as standalone as possible to have maximum flexibility having a common way of structuring a formula to make it quicker to learn is the way forward.

👍

I think we should keep it simple and focus on the strengths we have (vs Puppet, Chef, or Ansible) :

  • unicity
  • uniformity
  • maintainability
  • tests
  • versionning

@n-rodriguez
Copy link
Member

n-rodriguez commented Oct 8, 2019

On the other hand it could be nice to start centralizing them in a macros dir in this formula and why not adding tests. Instead of reinventing the wheel, hopefully people will come here to copy/paste in their own formulas and maybe upstream some issues.

@daks
Copy link
Member Author

daks commented Oct 9, 2019

for an update: I tried (some time ago) to create a _states/formulalib.py with a function named tofs_managed. My idea was just to wrap file.managed but implementing the TOFS path expansion and give it to file.managed. I never succeed in doing it, I'm blocked calling file.managed from my own function. I thought it would be simple, but didn't find how and hadn't more time to search for it.

Any help appreciated on this specific subject (even if I have less time than some months ago :-/

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

No branches or pull requests

5 participants