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

Rewrite of formula #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tomduijf
Copy link

@tomduijf tomduijf commented Oct 11, 2016

Hi all,

I imagine rewrites of existing formulas are not taken lightly, and they shouldn't.
There were many great idea's taken from the current formula, which will still work. Some things did change that makes this PR not backward compatible with current pillar.

This is an initial PR, i'd very much like some feedback on if we want this at all, and possible improvements. See todo below.

So, what does this bring? Some of these items are also present in the current version.

  • Renaming '_' -> '-' for option and statement names is still here.
  • All config blocks (declarations in dhcpd) are printed using a macro.
    • This allows easier mixing of declarations inside declarations. dhcpd gives a lot of freedom in buiiding a configuration. The current formula is very limited in this
  • Available options and statements are defined in lists. This allows for
    • shorter code
    • automatic quoting, if the statement / option needs it.
    • easier extending supported statements / options
    • it doesn't safeguard setting invalid statements in invalid places anymore. You can now put any statement anywhere (drawback of the lists approach).
  • type of custom_options are tracked in the big option list, allowing these to be automatically quoted as well.
  • option spaces. Well these are pretty much custom_options, but these are grouped together nicely
  • much more statements, declarations and options supported.
  • all options and statements can be either single key/val pairs or key/list items (useful array/list definitions like option domain-name-servers
  • special statements like allow, deny, set, `match, etc are repeated when values are in list form
  • failover options that may only be set for primary, are not printed for secondary role

Incompatibility with the current formula:

  • failover peer primary/secondary are set using role:
  • all option <something> statements require to be set under options: dict
  • customized_options -> custom_options

Todo:

  • more testing
  • cleanup some of the jinja code
  • optimize some stuff (like use |join(', '), etc)

Probably forgot to mention several things.

This was quite a bit of work, especially the jinja code, getting to the eventual imports / macro combination as a solution for dealing with the blocks, custom option tracking, and passing configuration.
Hope you like!

Fixed some typos
reverted back to dictonary declarations, liked it better
updated pillar.example and dhcpd.sample
added some statements and options i think
@mthibaut
Copy link
Contributor

mthibaut commented Feb 7, 2018

I am in the process of adding testing to dhcpd-formula. Next I would like to incorporate your changes and add IPv6 support.

@tomduijf
Copy link
Author

tomduijf commented Feb 8, 2018

Hi @mthibaut

Sounds great! I'd be happy to help with testing/reviewing if you want.

Mind passing a little credit back my when submitting? (took me quite some effort)

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.

2 participants