Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions STYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
includes both const and non-const references.
* Function names using camel case starting with a lower case letter (e.g., "doFoo()").
* Struct/Class member variables have a '\_' postfix (e.g., "int foo\_;").
* Enum values using PascalCase (e.g., `RoundRobin`).
* 100 columns is the line limit.
* Use your GitHub name in TODO comments, e.g. `TODO(foobar): blah`.
* Smart pointers are type aliased:
Expand Down
12 changes: 12 additions & 0 deletions docs/configuration/cluster_manager/cluster.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Cluster
"features": "...",
"http_codec_options": "...",
"dns_refresh_rate_ms": "...",
"dns_lookup_family": "...",
"outlier_detection": "{...}"
}

Expand Down Expand Up @@ -144,6 +145,17 @@ dns_refresh_rate_ms
the value defaults to 5000. For cluster types other than *strict_dns* and *logical_dns* this setting is
ignored.

.. _config_cluster_manager_cluster_dns_lookup_family:

dns_lookup_family
*(optional, string)* The DNS IP address resolution policy. The options are *v4_only*, *v6_only*,
and *auto*. If this setting is not specified, the value defaults to *v4_only*. When *v4_only* is selected,
the DNS resolver will only perform a lookup for addresses in the IPv4 family. If *v6_only* is selected,
the DNS resolver will only perform a lookup for addresses in the IPv6 family. If *auto* is specified,
the DNS resolver will first perform a lookup for addresses in the IPv6 family and fallback to a lookup for
addresses in the IPv4 family. For cluster types other than *strict_dns* and *logical_dns*, this setting
is ignored.

.. _config_cluster_manager_cluster_outlier_detection:

outlier_detection
Expand Down
6 changes: 5 additions & 1 deletion include/envoy/network/dns.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class ActiveDnsQuery {
virtual void cancel() PURE;
};

enum class DnsLookupFamily { V4Only, V6Only, Auto };

/**
* An asynchronous DNS resolver.
*/
Expand All @@ -41,11 +43,13 @@ class DnsResolver {
/**
* Initiate an async DNS resolution.
* @param dns_name supplies the DNS name to lookup.
* @param dns_lookup_family the DNS IP version lookup policy.
* @param callback supplies the callback to invoke when the resolution is complete.
* @return if non-null, a handle that can be used to cancel the resolution.
* This is only valid until the invocation of callback or ~DnsResolver().
*/
virtual ActiveDnsQuery* resolve(const std::string& dns_name, ResolveCb callback) PURE;
virtual ActiveDnsQuery* resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family,
ResolveCb callback) PURE;
};

typedef std::unique_ptr<DnsResolver> DnsResolverPtr;
Expand Down
4 changes: 4 additions & 0 deletions source/common/json/config_schemas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,10 @@ const std::string Json::Schema::CLUSTER_SCHEMA(R"EOF(
"minimum" : 0,
"exclusiveMinimum" : true
},
"dns_lookup_family" : {
"type" : "string",
"enum" : ["v4_only", "v6_only", "auto"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Docs say fallback, but it's auto here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I actually, like 'auto' for consistency with other settings (just fix the doc)

Copy link
Copy Markdown
Contributor Author

@hennna hennna May 22, 2017

Choose a reason for hiding this comment

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

The problem with auto is that it's a keyword and can't be used when making the string to enum conversion.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bummer :(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually AUTO might be okay

},
"outlier_detection" : {
"type" : "object",
"properties" : {
Expand Down
87 changes: 64 additions & 23 deletions source/common/network/dns_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,51 @@ void DnsResolverImpl::PendingResolution::onAresHostCallback(int status, hostent*
delete this;
return;
}
if (status == ARES_SUCCESS || !fallback_if_failed_) {
completed_ = true;
}

std::list<Address::InstanceConstSharedPtr> address_list;
completed_ = true;
if (status == ARES_SUCCESS) {
ASSERT(hostent->h_addrtype == AF_INET);
for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) {
ASSERT(hostent->h_length == sizeof(in_addr));
sockaddr_in address;
memset(&address, 0, sizeof(address));
// TODO(mattklein123): IPv6 support.
address.sin_family = AF_INET;
address.sin_port = 0;
address.sin_addr = *reinterpret_cast<in_addr*>(hostent->h_addr_list[i]);
address_list.emplace_back(new Address::Ipv4Instance(&address));
if (hostent->h_addrtype == AF_INET) {
for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) {
ASSERT(hostent->h_length == sizeof(in_addr));
sockaddr_in address;
memset(&address, 0, sizeof(address));
address.sin_family = AF_INET;
address.sin_port = 0;
address.sin_addr = *reinterpret_cast<in_addr*>(hostent->h_addr_list[i]);
address_list.emplace_back(new Address::Ipv4Instance(&address));
}
} else if (hostent->h_addrtype == AF_INET6) {
for (int i = 0; hostent->h_addr_list[i] != nullptr; ++i) {
ASSERT(hostent->h_length == sizeof(in6_addr));
sockaddr_in6 address;
memset(&address, 0, sizeof(address));
address.sin6_family = AF_INET6;
address.sin6_port = 0;
address.sin6_addr = *reinterpret_cast<in6_addr*>(hostent->h_addr_list[i]);
address_list.emplace_back(new Address::Ipv6Instance(address));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unrelated to your review, but it seems weird that the sockeadd[_in6] constructors for Ipv4Instance and Ipv6Instance differ in whether they take a pointer or reference.

}
}
}
if (!cancelled_) {
callback_(std::move(address_list));

if (completed_) {
if (!cancelled_) {
callback_(std::move(address_list));
}
if (owned_) {
delete this;
return;
}
}
if (owned_) {
delete this;

if (status != ARES_SUCCESS && fallback_if_failed_) {
fallback_if_failed_ = false;
getHostByName(AF_INET);
// Note: Nothing can follow this call to getHostByName due to deletion of this
// object upon synchronous resolution.
return;
}
}

Expand Down Expand Up @@ -115,17 +140,26 @@ void DnsResolverImpl::onAresSocketStateChange(int fd, int read, int write) {
(write ? Event::FileReadyType::Write : 0));
}

ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, ResolveCb callback) {
std::unique_ptr<PendingResolution> pending_resolution(new PendingResolution());
pending_resolution->callback_ = callback;
ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name,
DnsLookupFamily dns_lookup_family, ResolveCb callback) {
// TODO(hennna): Add DNS caching which will allow testing the edge case of a
// failed intial call to getHostbyName followed by a synchronous IPv4
// resolution.
std::unique_ptr<PendingResolution> pending_resolution(
new PendingResolution(callback, channel_, dns_name));
if (dns_lookup_family == DnsLookupFamily::Auto) {
pending_resolution->fallback_if_failed_ = true;
}

ares_gethostbyname(channel_, dns_name.c_str(),
AF_INET, [](void* arg, int status, int timeouts, hostent* hostent) {
static_cast<PendingResolution*>(arg)->onAresHostCallback(status, hostent);
UNREFERENCED_PARAMETER(timeouts);
}, pending_resolution.get());
if (dns_lookup_family == DnsLookupFamily::V4Only) {
pending_resolution->getHostByName(AF_INET);
} else {
pending_resolution->getHostByName(AF_INET6);
}

if (pending_resolution->completed_) {
// Resolution does not need asynchronous behavior or network events. For
// example, localhost lookup.
return nullptr;
} else {
// The PendingResolution will self-delete when the request completes
Expand All @@ -135,5 +169,12 @@ ActiveDnsQuery* DnsResolverImpl::resolve(const std::string& dns_name, ResolveCb
}
}

void DnsResolverImpl::PendingResolution::getHostByName(int family) {
ares_gethostbyname(channel_, dns_name_.c_str(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As discussed IRL last week, ares_gethostbyname() can implement the IPv6-IPv4 fallback with AF_UNSPEC family set (but is badly documented). However, I think your current implementation gets us closer to where we want to be in #978, since it will allow for the return of both IPv6 and IPv4 addresses in a list in a future PR, where we can support the caller deciding upon fallback as a result of connect() failure. So, I reckon it's good to keep the explicit fallback in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It also seems that AF_UNSPEC has unexpected behavior (not a straightforward IPv6 lookup followed by a IPv4 lookup). Once that and returning multiple address types is added to the c-ares library, we can switch over to using AF_UNSPEC.

family, [](void* arg, int status, int, hostent* hostent) {
static_cast<PendingResolution*>(arg)->onAresHostCallback(status, hostent);
}, this);
}

} // Network
} // Envoy
24 changes: 21 additions & 3 deletions source/common/network/dns_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,48 @@ class DnsResolverImpl : public DnsResolver {
~DnsResolverImpl() override;

// Network::DnsResolver
ActiveDnsQuery* resolve(const std::string& dns_name, ResolveCb callback) override;
ActiveDnsQuery* resolve(const std::string& dns_name, DnsLookupFamily dns_lookup_family,
ResolveCb callback) override;

private:
friend class DnsResolverImplPeer;
struct PendingResolution : public ActiveDnsQuery {
// Network::ActiveDnsQuery
PendingResolution(ResolveCb callback, ares_channel channel, const std::string& dns_name)
: callback_(callback), channel_(channel), dns_name_(dns_name) {}

void cancel() override {
// c-ares only supports channel-wide cancellation, so we just allow the
// network events to continue but don't invoke the callback on completion.
cancelled_ = true;
}

// c-ares ares_gethostbyname() query callback.
/**
* c-ares ares_gethostbyname() query callback.
* @param status return status of call to ares_gethostbyname.
* @param hostent structure that stores information about a given host.
*/
void onAresHostCallback(int status, hostent* hostent);
/**
* wrapper function of call to ares_gethostbyname.
* @param family currently AF_INET and AF_INET6 are supported.
*/
void getHostByName(int family);

// Caller supplied callback to invoke on query completion or error.
ResolveCb callback_;
const ResolveCb callback_;
// Does the object own itself? Resource reclamation occurs via self-deleting
// on query completion or error.
bool owned_ = false;
// Has the query completed? Only meaningful if !owned_;
bool completed_ = false;
// Was the query cancelled via cancel()?
bool cancelled_ = false;
// If dns_lookup_family is "fallback", fallback to v4 address if v6
// resolution failed.
bool fallback_if_failed_ = false;
const ares_channel channel_;
const std::string dns_name_;
};

// Callback for events on sockets tracked in events_.
Expand Down
11 changes: 11 additions & 0 deletions source/common/network/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,17 @@ Address::InstanceConstSharedPtr Utility::getIpv6AnyAddress() {
return any;
}

Address::InstanceConstSharedPtr Utility::getAddressWithPort(const Address::Instance& address,
uint32_t port) {
switch (address.ip()->version()) {
case Network::Address::IpVersion::v4:
return std::make_shared<Address::Ipv4Instance>(address.ip()->addressAsString(), port);
case Network::Address::IpVersion::v6:
return std::make_shared<Address::Ipv6Instance>(address.ip()->addressAsString(), port);
}
NOT_REACHED;
}

Address::InstanceConstSharedPtr Utility::getOriginalDst(int fd) {
sockaddr_storage orig_addr;
socklen_t addr_len = sizeof(sockaddr_storage);
Expand Down
8 changes: 8 additions & 0 deletions source/common/network/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,14 @@ class Utility {
*/
static Address::InstanceConstSharedPtr getIpv6AnyAddress();

/**
* @param address IP address instance.
* @param port to update.
* @return Address::InstanceConstSharedPtr a new address instance with updated port.
*/
static Address::InstanceConstSharedPtr getAddressWithPort(const Address::Instance& address,
uint32_t port);

/**
* Retrieve the original destination address from an accepted fd.
* The address (IP and port) may be not local and the port may differ from
Expand Down
37 changes: 27 additions & 10 deletions source/common/upstream/logical_dns_cluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,20 @@ LogicalDnsCluster::LogicalDnsCluster(const Json::Object& config, Runtime::Loader
std::chrono::milliseconds(config.getInteger("dns_refresh_rate_ms", 5000))),
tls_(tls), tls_slot_(tls.allocateSlot()), initialized_(false),
resolve_timer_(dispatcher.createTimer([this]() -> void { startResolve(); })) {

std::vector<Json::ObjectSharedPtr> hosts_json = config.getObjectArray("hosts");
if (hosts_json.size() != 1) {
throw EnvoyException("logical_dns clusters must have a single host");
}

std::string dns_lookup_family = config.getString("dns_lookup_family", "v4_only");
if (dns_lookup_family == "v6_only") {
dns_lookup_family_ = Network::DnsLookupFamily::V6Only;
} else if (dns_lookup_family == "auto") {
dns_lookup_family_ = Network::DnsLookupFamily::Auto;
} else {
ASSERT(dns_lookup_family == "v4_only");
dns_lookup_family_ = Network::DnsLookupFamily::V4Only;
}
dns_url_ = hosts_json[0]->getString("url");
hostname_ = Network::Utility::hostFromTcpUrl(dns_url_);
Network::Utility::portFromTcpUrl(dns_url_);
Expand All @@ -51,18 +59,19 @@ void LogicalDnsCluster::startResolve() {
info_->stats().update_attempt_.inc();

active_dns_query_ = dns_resolver_.resolve(
dns_address, [this, dns_address](
std::list<Network::Address::InstanceConstSharedPtr>&& address_list) -> void {
dns_address, dns_lookup_family_,
[this, dns_address](
std::list<Network::Address::InstanceConstSharedPtr>&& address_list) -> void {
active_dns_query_ = nullptr;
log_debug("async DNS resolution complete for {}", dns_address);
info_->stats().update_success_.inc();

if (!address_list.empty()) {
// TODO(mattklein123): IPv6 support as well as moving port handling into the DNS
// interface.
Network::Address::InstanceConstSharedPtr new_address(
new Network::Address::Ipv4Instance(address_list.front()->ip()->addressAsString(),
Network::Utility::portFromTcpUrl(dns_url_)));
// TODO(mattklein123): Move port handling into the DNS interface.
ASSERT(address_list.front() != nullptr);
Network::Address::InstanceConstSharedPtr new_address =
Network::Utility::getAddressWithPort(*address_list.front(),
Network::Utility::portFromTcpUrl(dns_url_));
if (!current_resolved_address_ || !(*new_address == *current_resolved_address_)) {
current_resolved_address_ = new_address;
// Capture URL to avoid a race with another update.
Expand All @@ -77,8 +86,16 @@ void LogicalDnsCluster::startResolve() {
// to show the friendly DNS name in that output, but currently there is no way to
// express a DNS name inside of an Address::Instance. For now this is OK but we might
// want to do better again later.
logical_host_.reset(new LogicalHost(
info_, hostname_, Network::Utility::resolveUrl("tcp://0.0.0.0:0"), *this));
switch (address_list.front()->ip()->version()) {
case Network::Address::IpVersion::v4:
logical_host_.reset(
new LogicalHost(info_, hostname_, Network::Utility::getIpv4AnyAddress(), *this));
break;
case Network::Address::IpVersion::v6:
logical_host_.reset(
new LogicalHost(info_, hostname_, Network::Utility::getIpv6AnyAddress(), *this));
break;
}
HostVectorSharedPtr new_hosts(new std::vector<HostSharedPtr>());
new_hosts->emplace_back(logical_host_);
updateHosts(new_hosts, createHealthyHostList(*new_hosts), empty_host_lists_,
Expand Down
1 change: 1 addition & 0 deletions source/common/upstream/logical_dns_cluster.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class LogicalDnsCluster : public ClusterImplBase {

Network::DnsResolver& dns_resolver_;
const std::chrono::milliseconds dns_refresh_rate_ms_;
Network::DnsLookupFamily dns_lookup_family_;
ThreadLocal::Instance& tls_;
uint32_t tls_slot_;
std::function<void()> initialize_callback_;
Expand Down
21 changes: 15 additions & 6 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,16 @@ StrictDnsClusterImpl::StrictDnsClusterImpl(const Json::Object& config, Runtime::
: BaseDynamicClusterImpl(config, runtime, stats, ssl_context_manager),
dns_resolver_(dns_resolver), dns_refresh_rate_ms_(std::chrono::milliseconds(
config.getInteger("dns_refresh_rate_ms", 5000))) {
std::string dns_lookup_family = config.getString("dns_lookup_family", "v4_only");
if (dns_lookup_family == "v6_only") {
dns_lookup_family_ = Network::DnsLookupFamily::V6Only;
} else if (dns_lookup_family == "auto") {
dns_lookup_family_ = Network::DnsLookupFamily::Auto;
} else {
ASSERT(dns_lookup_family == "v4_only");
dns_lookup_family_ = Network::DnsLookupFamily::V4Only;
}

for (Json::ObjectSharedPtr host : config.getObjectArray("hosts")) {
resolve_targets_.emplace_back(new ResolveTarget(*this, dispatcher, host->getString("url")));
}
Expand Down Expand Up @@ -413,7 +423,7 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() {
parent_.info_->stats().update_attempt_.inc();

active_query_ = parent_.dns_resolver_.resolve(
dns_address_,
dns_address_, parent_.dns_lookup_family_,
[this](std::list<Network::Address::InstanceConstSharedPtr>&& address_list) -> void {
active_query_ = nullptr;
log_debug("async DNS resolution complete for {}", dns_address_);
Expand All @@ -424,11 +434,10 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() {
// TODO(mattklein123): Currently the DNS interface does not consider port. We need to make
// a new address that has port in it. We need to both support IPv6 as well as potentially
// move port handling into the DNS interface itself, which would work better for SRV.
new_hosts.emplace_back(new HostImpl(
parent_.info_, dns_address_,
Network::Address::InstanceConstSharedPtr{
new Network::Address::Ipv4Instance(address->ip()->addressAsString(), port_)},
false, 1, ""));
ASSERT(address != nullptr);
new_hosts.emplace_back(new HostImpl(parent_.info_, dns_address_,
Network::Utility::getAddressWithPort(*address, port_),
false, 1, ""));
}

std::vector<HostSharedPtr> hosts_added;
Expand Down
1 change: 1 addition & 0 deletions source/common/upstream/upstream_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ class StrictDnsClusterImpl : public BaseDynamicClusterImpl {
Network::DnsResolver& dns_resolver_;
std::list<ResolveTargetPtr> resolve_targets_;
const std::chrono::milliseconds dns_refresh_rate_ms_;
Network::DnsLookupFamily dns_lookup_family_;
};

} // Upstream
Expand Down
Loading