Skip to content
This repository has been archived by the owner on Aug 29, 2018. It is now read-only.

Added generic call script custom DNS Plugin Gem #6119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grahamnscp
Copy link

Hi guys, created a useful generic DNS plugin that calls a script specified in the config file. An example script that calls nsupdate is included in the conf directory too.
Creating a plugin was not well documented so also serves as an example as well as allowing a free form script to wrap any custom DNS deployment requirements.
Hope that it would be a useful addition to the upstream project, also my first upstream pull request :)
Best regards, Graham

@danmcp
Copy link
Contributor

danmcp commented Jun 11, 2015

@sosiouxme Review please.

@sosiouxme
Copy link
Member

@grahamrht The concept is good, the implementation looks good at first glance, but it will have to be added to my list to review fully after a few other things are done. Sorry for the wait, thanks for getting this moving!

@grahamnscp
Copy link
Author

Thanks Luke,

Best regards,
Graham

On 11/06/2015 11:44, Luke Meyer wrote:

@grahamrht https://github.com/grahamrht The concept is good, the
implementation looks good at first glance, but it will have to be
added to my list to review fully after a few other things are done.
Sorry for the wait, thanks for getting this moving!


Reply to this email directly or view it on GitHub
#6119 (comment).

Graham Hares, RHCA
Red Hat Consulting
Mobile: 07584 680158

hostname=127.0.0.1
priv_key=/etc/openshift/example.com.key
ttl=60
zone=example.com
Copy link
Member

Choose a reason for hiding this comment

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

Should these parameters be read in from a configuration file? Few actual openshift environments have named installed on the same host as a broker, so this plugin won't work for most.

They should either be read in from a configuration file or passed into the script as parameters. Normally, dns plugins establish connection information in the plugin class's initialize method. I can see why that wasn't done here, since it would be cumbersome to pass the connection information every time the script is called. If the connection information isn't passed as parameters, they should at least be read in from a configuration file.

One issue with defining the connection parameters in this script is that this file is not marked as a configuration file in the rpm spec. If a newer version of the rpm is installed, it will overwrite the modified file with the one in the newer rpm.

@tiwillia
Copy link
Member

@grahamrht the openshift-origin-dns-custom-1.0.0.gem file was included with this pr. There is no need to include the built gem, it should be removed.

Other than that and the few line notes I made, this looks good to me.

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

Successfully merging this pull request may close these issues.

4 participants