[PUP-8705] Add SRV record support to Puppet::Rest::Client Created: 2018/05/09  Updated: 2018/09/19  Resolved: 2018/06/26

Status: Closed
Project: Puppet
Component/s: None
Affects Version/s: None
Fix Version/s: PUP 6.0.0

Type: Task Priority: Normal
Reporter: Maggie Dreyer Assignee: Maggie Dreyer
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Acceptance Criteria:
  • Puppet::Rest::Client respects SRV records when available.
  • The record list for a service is cached once it has been resolved.
  • The cached of records should be invalidated when the shortest TTL among cached records has expired.
  • The found server for each service is cached once it has been selected.
  • The cache entry for a selected server is invalidated if the server is unreachable, and a new server is selected from the cached record list for that service, falling back to configured value if none is available.
Epic Link: HTTPClient in Puppet
Team: Froyo
Release Notes: Not Needed
QA Risk Assessment: Needs Assessment



In order to improve performance over the indirector's re-resolving SRV records on each request, we should implement something similar to Puppet::Network::Resolver but that caches the results of the SRV lookup for each service. The cache of SRV records should be invalidated based on the shortest TTL among the cache entries. This resolver should be an object whose lifetime is tied to the HTTP client making requests with its results. (Note that currently the HTTP clients are created where needed and their configuration is locked, but eventually we will probably use one client for all requests, at which point the DNS cache will therefore also be global). The client should then in turn cache the selected server and port, and that cache entry should only be invalidated if the cached server becomes unreachable for any reason, including a 500 error. At this point the server and port should be reselected from the cached SRV record inside the Resolver.

**Original text**

Currently, Puppet::Rest::Client does not do any SRV lookups, unlike the other indirector-based HTTP client in Puppet. We need to implement this, once we fully understand the tradeoffs between performance and the need to restrict caching due to the nature of SRV lookups.

A quick look at the current implementation (https://github.com/puppetlabs/puppet/blob/master/lib/puppet/indirector/request.rb#L187-L197 and https://github.com/puppetlabs/puppet/blob/master/lib/puppet/network/resolver.rb#L8-L41) makes it look as though DNS::Resolv objects and SRV lookups are performed on every request. We need to evaluate whether this is necessary, or more could be done to cut down on work that must be done per-request.

Comment by Justin Stoller [ 2018/05/10 ]

We'd like more information on the applicability of SRV lookups during cert download to figure out when to prioritize this.

Hoping to talk to Jacob Helwig and Josh Cooper specifically, as they seem like our current subject matter experts on SRV.

Comment by Jacob Helwig [ 2018/05/11 ]

Justin Stoller, happy to sync up on this. Feel free to throw something on my calendar whenever it's a a good time for you all.

Generated at Sun May 31 14:10:02 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.