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

Fix configured_zones behaviour #121

Closed
wants to merge 3 commits into from
Closed

Fix configured_zones behaviour #121

wants to merge 3 commits into from

Conversation

aanriot
Copy link
Contributor

@aanriot aanriot commented Dec 28, 2018

Hello,

With the latest commits, we unconditionally perform a file.managed state because the file argument is always defined. I think that it breaks the previous configured_zones behaviour, as we may want to declare a zone (and bind has to know the filename) without managing it.

Feel free to comment.

@aanriot
Copy link
Contributor Author

aanriot commented Dec 28, 2018

GitHub doesn't allow me to add you as a reviewer @crux-capacitor, but please feel free to comment as well.

@crux-capacitor
Copy link
Contributor

crux-capacitor commented Dec 28, 2018

Hi @aanriot,

The file argument is optional underneath configured_views:<view>:configured_zones:<zone>. If you do specify it, it allows you to declare the file to be used that you configured underneath available_zones:<zone>, otherwise the code will choose the file based on the zone under available_zones that matches the zone name for the current view in the loop.

Your code adds a way to not create the zone file?

@aanriot
Copy link
Contributor Author

aanriot commented Dec 31, 2018

Hi @aanriot,

The file argument is optional underneath configured_views:<view>:configured_zones:<zone>. If you do specify it, it allows you to declare the file to be used that you configured underneath available_zones:<zone>, otherwise the code will choose the file based on the zone under available_zones that matches the zone name for the current view in the loop.

Your code adds a way to not create the zone file?

Thanks @crux-capacitor . That's true. Previously, it was possible to declare a zone (and its path) and edit the occurring zone file manually. Currently, I have a server on which we manage the bind configuration, but not the content of the zone files, mainly because they contain static records as well as dynamic records.

Copy link
Member

@aboe76 aboe76 left a comment

Choose a reason for hiding this comment

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

@aanriot can this behavior be documented somewhere?

@aanriot
Copy link
Contributor Author

aanriot commented Dec 31, 2018

@aanriot can this behavior be documented somewhere?

@aboe76 If I understand correctly the doc, it's already at https://github.com/saltstack-formulas/bind-formula#external-zone-files.

But it should probably be rewritten accordingly with pillar.example if we choose to move forward with the clarification attempt @javierbertoli mentionned.

@aanriot
Copy link
Contributor Author

aanriot commented Jan 3, 2019

Closing this PR as it will not be merged, refactoring attempts will be tracked in #114.

@aanriot aanriot closed this Jan 3, 2019
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.

3 participants