On Thu, 16 Dec 2010 20:09:37 +1100, Daniel Pittman wrote: > > On Thu, Dec 16, 2010 at 17:52, Andrew Forgue <[email protected]> wrote: > > > Here's a new version of it. > > Thanks so much. I looked over the code, and I can't see anything > objectionable to it. It looks like it does the right thing™ in all > cases, and allows the right sort of SRV record lookup override. > > The one thing I am not certain about - and it is a kind of "what > colour is your bikeshed" question - is that you have the config option > specify '_puppet._tcp.$domain'. > > It might make sense to specify the "$domain" part of the lookup, but > make _puppet._tcp automatic - that isn't really an optional bit of the > label that gets searched for, after all. OTOH, it doesn't *hurt* > anything to do it this way... > > So, yeah: I would be happy with it as-is, or just specifying the > non-protocol part of the SRV record, as you prefer. > > That all looks great! > Daniel
I've updated the ticket with these comments[1], but I'm putting them here to get more discussion going: My branch[2] has another "Work-In-Progress" commit on top of the work Daniel had done on the branch. My WIP commit fixes a couple of missed method renames, and adjusts the weighted shuffle used for picking servers to better match what is described in RFC 2782 for handling 0 weighted entries. There is still some work left to do that Jesse and I discovered in our review of the branch. SRV record support shouldn’t be turned off if you specify a server to use, since you should be able to specify a fall-back server to use other than the default of "puppet". Also, the CA, reports, and file-serving look like they would be broken by this, unless you had "puppet" resolvable by DNS. We should probably open up discussion about whether the full SRV record name should be specified in the setting, or just the domain to use for the SRV record. Jesse and I are of the opinion that only the domain should be specified in the config. It would be really nice to have SRV services for _puppet_ca, _puppet_report, and _puppet_fileserver (all of which falling back to _puppet, then the server setting), though these don’t seem absolutely necessary for a first-round feature, provided you can still adjust the server setting when SRV record support is enabled. This looks like it will turn into a really cool feature. Thanks for putting in the work you have on it, Andrew! [1] http://projects.puppetlabs.com/issues/3669#note-14 [2] https://github.com/jhelwig/puppet/tree/ticket/next/3669-make-puppet-honor-DNS-SRV-records -- Jacob Helwig
signature.asc
Description: Digital signature
