Skip to content

Commit

Permalink
listener manager: removing exceptions (#36314)
Browse files Browse the repository at this point in the history 
Risk Level: low
Testing: updated tests
Docs Changes: n/a
Release Notes: n/a
envoyproxy/envoy-mobile#176

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
@alyssawilk
alyssawilk authored Oct 8, 2024
1 parent a9ce686 commit 2386bd5
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 67 deletions.
4 changes: 2 additions & 2 deletions envoy/common/exception.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ class EnvoyException : public std::runtime_error {
};

#define SET_AND_RETURN_IF_NOT_OK(check_status, set_status) \
if (!check_status.ok()) { \
set_status = check_status; \
if (const absl::Status temp_status = check_status; !temp_status.ok()) { \
set_status = temp_status; \
return; \
}

Expand Down
107 changes: 71 additions & 36 deletions source/common/listener_manager/listener_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,27 @@ bool shouldBindToPort(const envoy::config::listener::v3::Listener& config) {
}
} // namespace

absl::StatusOr<std::unique_ptr<ListenSocketFactoryImpl>> ListenSocketFactoryImpl::create(
ListenerComponentFactory& factory, Network::Address::InstanceConstSharedPtr address,
Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options,
const std::string& listener_name, uint32_t tcp_backlog_size,
ListenerComponentFactory::BindType bind_type,
const Network::SocketCreationOptions& creation_options, uint32_t num_sockets) {
absl::Status creation_status = absl::OkStatus();
auto ret = std::unique_ptr<ListenSocketFactoryImpl>(new ListenSocketFactoryImpl(
factory, address, socket_type, options, listener_name, tcp_backlog_size, bind_type,
creation_options, num_sockets, creation_status));
RETURN_IF_NOT_OK(creation_status);
return ret;
}

ListenSocketFactoryImpl::ListenSocketFactoryImpl(
ListenerComponentFactory& factory, Network::Address::InstanceConstSharedPtr address,
Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options,
const std::string& listener_name, uint32_t tcp_backlog_size,
ListenerComponentFactory::BindType bind_type,
const Network::SocketCreationOptions& creation_options, uint32_t num_sockets)
const Network::SocketCreationOptions& creation_options, uint32_t num_sockets,
absl::Status& creation_status)
: factory_(factory), local_address_(address), socket_type_(socket_type), options_(options),
listener_name_(listener_name), tcp_backlog_size_(tcp_backlog_size), bind_type_(bind_type),
socket_creation_options_(creation_options) {
Expand All @@ -100,8 +115,9 @@ ListenSocketFactoryImpl::ListenSocketFactoryImpl(
}
}

sockets_.push_back(THROW_OR_RETURN_VALUE(
createListenSocketAndApplyOptions(factory, socket_type, 0), Network::SocketSharedPtr));
auto socket_or_error = createListenSocketAndApplyOptions(factory, socket_type, 0);
SET_AND_RETURN_IF_NOT_OK(socket_or_error.status(), creation_status);
sockets_.push_back(*socket_or_error);

if (sockets_[0] != nullptr && local_address_->ip() && local_address_->ip()->port() == 0) {
local_address_ = sockets_[0]->connectionInfoProvider().localAddress();
Expand All @@ -114,8 +130,9 @@ ListenSocketFactoryImpl::ListenSocketFactoryImpl(
if (bind_type_ != ListenerComponentFactory::BindType::ReusePort && sockets_[0] != nullptr) {
sockets_.push_back(sockets_[0]->duplicate());
} else {
sockets_.push_back(THROW_OR_RETURN_VALUE(
createListenSocketAndApplyOptions(factory, socket_type, i), Network::SocketSharedPtr));
auto socket_or_error = createListenSocketAndApplyOptions(factory, socket_type, i);
SET_AND_RETURN_IF_NOT_OK(socket_or_error.status(), creation_status);
sockets_.push_back(*socket_or_error);
}
}
ASSERT(sockets_.size() == num_sockets);
Expand Down Expand Up @@ -165,12 +182,7 @@ absl::StatusOr<Network::SocketSharedPtr> ListenSocketFactoryImpl::createListenSo
fmt::format("{}: Setting socket options {}", listener_name_, ok ? "succeeded" : "failed");
if (!ok) {
ENVOY_LOG(warn, "{}", message);
#ifdef ENVOY_DISABLE_EXCEPTIONS
PANIC(message);
#else
throw Network::SocketOptionException(message);
#endif

return absl::InvalidArgumentError(message);
} else {
ENVOY_LOG(debug, "{}", message);
}
Expand Down Expand Up @@ -236,8 +248,11 @@ std::string listenerStatsScope(const envoy::config::listener::v3::Listener& conf
return absl::StrCat("envoy_internal_", config.name());
}
auto address_or_error = Network::Address::resolveProtoAddress(config.address());
THROW_IF_NOT_OK_REF(address_or_error.status());
return address_or_error.value()->asString();
if (address_or_error.status().ok()) {
return address_or_error.value()->asString();
}
// Listener creation will fail shortly when the address is used.
return absl::StrCat("invalid_address_listener");
}
} // namespace

Expand All @@ -254,10 +269,22 @@ Network::DrainDecision& ListenerFactoryContextBaseImpl::drainDecision() { return
Server::DrainManager& ListenerFactoryContextBaseImpl::drainManager() { return *drain_manager_; }
Init::Manager& ListenerFactoryContextBaseImpl::initManager() { return server_.initManager(); }

absl::StatusOr<std::unique_ptr<ListenerImpl>>
ListenerImpl::create(const envoy::config::listener::v3::Listener& config,
const std::string& version_info, ListenerManagerImpl& parent,
const std::string& name, bool added_via_api, bool workers_started,
uint64_t hash) {
absl::Status creation_status = absl::OkStatus();
auto ret = std::unique_ptr<ListenerImpl>(new ListenerImpl(
config, version_info, parent, name, added_via_api, workers_started, hash, creation_status));
RETURN_IF_NOT_OK(creation_status);
return ret;
}

ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
const std::string& version_info, ListenerManagerImpl& parent,
const std::string& name, bool added_via_api, bool workers_started,
uint64_t hash)
uint64_t hash, absl::Status& creation_status)
: parent_(parent),
socket_type_(config.has_internal_listener()
? Network::Socket::Type::Stream
Expand Down Expand Up @@ -324,25 +351,28 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
// All the addresses should be same socket type, so get the first address's socket type is
// enough.
auto address_or_error = Network::Address::resolveProtoAddress(config.address());
THROW_IF_NOT_OK_REF(address_or_error.status());
SET_AND_RETURN_IF_NOT_OK(address_or_error.status(), creation_status);
auto address = std::move(address_or_error.value());
THROW_IF_NOT_OK(checkIpv4CompatAddress(address, config.address()));
SET_AND_RETURN_IF_NOT_OK(checkIpv4CompatAddress(address, config.address()), creation_status);
addresses_.emplace_back(address);
address_opts_list.emplace_back(std::ref(config.socket_options()));

for (auto i = 0; i < config.additional_addresses_size(); i++) {
if (socket_type_ !=
Network::Utility::protobufAddressSocketType(config.additional_addresses(i).address())) {
throwEnvoyExceptionOrPanic(
creation_status = absl::InvalidArgumentError(
fmt::format("listener {}: has different socket type. The listener only "
"support same socket type for all the addresses.",
name_));
return;
}
auto addresses_or_error =
Network::Address::resolveProtoAddress(config.additional_addresses(i).address());
THROW_IF_NOT_OK_REF(addresses_or_error.status());
SET_AND_RETURN_IF_NOT_OK(addresses_or_error.status(), creation_status);
auto additional_address = std::move(addresses_or_error.value());
THROW_IF_NOT_OK(checkIpv4CompatAddress(address, config.additional_addresses(i).address()));
SET_AND_RETURN_IF_NOT_OK(
checkIpv4CompatAddress(address, config.additional_addresses(i).address()),
creation_status);
addresses_.emplace_back(additional_address);
if (config.additional_addresses(i).has_socket_options()) {
address_opts_list.emplace_back(
Expand All @@ -367,20 +397,21 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config,
addresses_, listener_factory_context_->parentFactoryContext(), initManager()),

buildAccessLog(config);
THROW_IF_NOT_OK(validateConfig());
SET_AND_RETURN_IF_NOT_OK(validateConfig(), creation_status);

// buildUdpListenerFactory() must come before buildListenSocketOptions() because the UDP
// listener factory can provide additional options.
THROW_IF_NOT_OK(buildUdpListenerFactory(config, parent_.server_.options().concurrency()));
SET_AND_RETURN_IF_NOT_OK(buildUdpListenerFactory(config, parent_.server_.options().concurrency()),
creation_status);
buildListenSocketOptions(config, address_opts_list);
THROW_IF_NOT_OK(createListenerFilterFactories(config));
THROW_IF_NOT_OK(validateFilterChains(config));
THROW_IF_NOT_OK(buildFilterChains(config));
SET_AND_RETURN_IF_NOT_OK(createListenerFilterFactories(config), creation_status);
SET_AND_RETURN_IF_NOT_OK(validateFilterChains(config), creation_status);
SET_AND_RETURN_IF_NOT_OK(buildFilterChains(config), creation_status);
if (socket_type_ != Network::Socket::Type::Datagram) {
buildSocketOptions(config);
buildOriginalDstListenerFilter(config);
buildProxyProtocolListenerFilter(config);
THROW_IF_NOT_OK(buildInternalListener(config));
SET_AND_RETURN_IF_NOT_OK(buildInternalListener(config), creation_status);
}
if (!workers_started_) {
// Initialize dynamic_init_manager_ from Server's init manager if it's not initialized.
Expand All @@ -395,7 +426,7 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin,
const envoy::config::listener::v3::Listener& config,
const std::string& version_info, ListenerManagerImpl& parent,
const std::string& name, bool added_via_api, bool workers_started,
uint64_t hash)
uint64_t hash, absl::Status& creation_status)
: parent_(parent), addresses_(origin.addresses_), socket_type_(origin.socket_type_),
bind_to_port_(shouldBindToPort(config)), mptcp_enabled_(config.enable_mptcp()),
hand_off_restored_destination_connections_(
Expand Down Expand Up @@ -443,11 +474,11 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin,
missing_listener_config_stats_({ALL_MISSING_LISTENER_CONFIG_STATS(
POOL_COUNTER(listener_factory_context_->listenerScope()))}) {
buildAccessLog(config);
THROW_IF_NOT_OK(validateConfig());
THROW_IF_NOT_OK(createListenerFilterFactories(config));
THROW_IF_NOT_OK(validateFilterChains(config));
THROW_IF_NOT_OK(buildFilterChains(config));
THROW_IF_NOT_OK(buildInternalListener(config));
SET_AND_RETURN_IF_NOT_OK(validateConfig(), creation_status);
SET_AND_RETURN_IF_NOT_OK(createListenerFilterFactories(config), creation_status);
SET_AND_RETURN_IF_NOT_OK(validateFilterChains(config), creation_status);
SET_AND_RETURN_IF_NOT_OK(buildFilterChains(config), creation_status);
SET_AND_RETURN_IF_NOT_OK(buildInternalListener(config), creation_status);
if (socket_type_ == Network::Socket::Type::Stream) {
// Apply the options below only for TCP.
buildSocketOptions(config);
Expand Down Expand Up @@ -1020,14 +1051,18 @@ bool ListenerImpl::supportUpdateFilterChain(const envoy::config::listener::v3::L
return false;
}

ListenerImplPtr
absl::StatusOr<ListenerImplPtr>
ListenerImpl::newListenerWithFilterChain(const envoy::config::listener::v3::Listener& config,
bool workers_started, uint64_t hash) {

absl::Status creation_status = absl::OkStatus();
// Use WrapUnique since the constructor is private.
return absl::WrapUnique(new ListenerImpl(*this, config, version_info_, parent_, name_,
added_via_api_,
/* new new workers started state */ workers_started,
/* use new hash */ hash));
auto ret = absl::WrapUnique(new ListenerImpl(*this, config, version_info_, parent_, name_,
added_via_api_,
/* new new workers started state */ workers_started,
/* use new hash */ hash, creation_status));
RETURN_IF_NOT_OK(creation_status);
return ret;
}

void ListenerImpl::diffFilterChain(const ListenerImpl& another_listener,
Expand Down
38 changes: 25 additions & 13 deletions source/common/listener_manager/listener_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,12 @@ class ListenerManagerImpl;
class ListenSocketFactoryImpl : public Network::ListenSocketFactory,
protected Logger::Loggable<Logger::Id::config> {
public:
ListenSocketFactoryImpl(ListenerComponentFactory& factory,
Network::Address::InstanceConstSharedPtr address,
Network::Socket::Type socket_type,
const Network::Socket::OptionsSharedPtr& options,
const std::string& listener_name, uint32_t tcp_backlog_size,
ListenerComponentFactory::BindType bind_type,
const Network::SocketCreationOptions& creation_options,
uint32_t num_sockets);
static absl::StatusOr<std::unique_ptr<ListenSocketFactoryImpl>>
create(ListenerComponentFactory& factory, Network::Address::InstanceConstSharedPtr address,
Network::Socket::Type socket_type, const Network::Socket::OptionsSharedPtr& options,
const std::string& listener_name, uint32_t tcp_backlog_size,
ListenerComponentFactory::BindType bind_type,
const Network::SocketCreationOptions& creation_options, uint32_t num_sockets);

// Network::ListenSocketFactory
Network::Socket::Type socketType() const override { return socket_type_; }
Expand All @@ -89,6 +87,15 @@ class ListenSocketFactoryImpl : public Network::ListenSocketFactory,
absl::Status doFinalPreWorkerInit() override;

private:
ListenSocketFactoryImpl(ListenerComponentFactory& factory,
Network::Address::InstanceConstSharedPtr address,
Network::Socket::Type socket_type,
const Network::Socket::OptionsSharedPtr& options,
const std::string& listener_name, uint32_t tcp_backlog_size,
ListenerComponentFactory::BindType bind_type,
const Network::SocketCreationOptions& creation_options,
uint32_t num_sockets, absl::Status& creation_status);

ListenSocketFactoryImpl(const ListenSocketFactoryImpl& factory_to_clone);

absl::StatusOr<Network::SocketSharedPtr>
Expand Down Expand Up @@ -205,9 +212,10 @@ class ListenerImpl final : public Network::ListenerConfig,
* @param hash supplies the hash to use for duplicate checking.
* @param concurrency is the number of listeners instances to be created.
*/
ListenerImpl(const envoy::config::listener::v3::Listener& config, const std::string& version_info,
ListenerManagerImpl& parent, const std::string& name, bool added_via_api,
bool workers_started, uint64_t hash);
static absl::StatusOr<std::unique_ptr<ListenerImpl>>
create(const envoy::config::listener::v3::Listener& config, const std::string& version_info,
ListenerManagerImpl& parent, const std::string& name, bool added_via_api,
bool workers_started, uint64_t hash);
~ListenerImpl() override;

// TODO(lambdai): Explore using the same ListenerImpl object to execute in place filter chain
Expand All @@ -216,7 +224,7 @@ class ListenerImpl final : public Network::ListenerConfig,
* Execute in place filter chain update. The filter chain update is less expensive than full
* listener update because connections may not need to be drained.
*/
std::unique_ptr<ListenerImpl>
absl::StatusOr<std::unique_ptr<ListenerImpl>>
newListenerWithFilterChain(const envoy::config::listener::v3::Listener& config,
bool workers_started, uint64_t hash);
/**
Expand Down Expand Up @@ -344,6 +352,9 @@ class ListenerImpl final : public Network::ListenerConfig,
SystemTime last_updated_;

private:
ListenerImpl(const envoy::config::listener::v3::Listener& config, const std::string& version_info,
ListenerManagerImpl& parent, const std::string& name, bool added_via_api,
bool workers_started, uint64_t hash, absl::Status& creation_status);
struct UdpListenerConfigImpl : public Network::UdpListenerConfig {
UdpListenerConfigImpl(const envoy::config::listener::v3::UdpListenerConfig config)
: config_(config) {}
Expand Down Expand Up @@ -384,7 +395,8 @@ class ListenerImpl final : public Network::ListenerConfig,
*/
ListenerImpl(ListenerImpl& origin, const envoy::config::listener::v3::Listener& config,
const std::string& version_info, ListenerManagerImpl& parent,
const std::string& name, bool added_via_api, bool workers_started, uint64_t hash);
const std::string& name, bool added_via_api, bool workers_started, uint64_t hash,
absl::Status& creation_status);
// Helpers for constructor.
void buildAccessLog(const envoy::config::listener::v3::Listener& config);
absl::Status buildInternalListener(const envoy::config::listener::v3::Listener& config);
Expand Down
17 changes: 13 additions & 4 deletions source/common/listener_manager/listener_manager_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,13 +580,17 @@ absl::StatusOr<bool> ListenerManagerImpl::addOrUpdateListenerInternal(
(*existing_active_listener)->supportUpdateFilterChain(config, workers_started_)) {
ENVOY_LOG(debug, "use in place update filter chain update path for listener name={} hash={}",
name, hash);
new_listener =
auto listener_or_error =
(*existing_active_listener)->newListenerWithFilterChain(config, workers_started_, hash);
RETURN_IF_NOT_OK_REF(listener_or_error.status());
new_listener = std::move(*listener_or_error);
stats_.listener_in_place_updated_.inc();
} else {
ENVOY_LOG(debug, "use full listener update path for listener name={} hash={}", name, hash);
new_listener = std::make_unique<ListenerImpl>(config, version_info, *this, name, added_via_api,
auto listener_or_error = ListenerImpl::create(config, version_info, *this, name, added_via_api,
workers_started_, hash);
RETURN_IF_NOT_OK_REF(listener_or_error.status());
new_listener = std::move(*listener_or_error);
}

ListenerImpl& new_listener_ref = *new_listener;
Expand Down Expand Up @@ -1197,10 +1201,15 @@ absl::Status ListenerManagerImpl::createListenSocketFactory(ListenerImpl& listen
creation_options.mptcp_enabled_ = listener.mptcpEnabled();
for (std::vector<Network::Address::InstanceConstSharedPtr>::size_type i = 0;
i < listener.addresses().size(); i++) {
socket_status = listener.addSocketFactory(std::make_unique<ListenSocketFactoryImpl>(
auto factory_or_error = ListenSocketFactoryImpl::create(
*factory_, listener.addresses()[i], socket_type, listener.listenSocketOptions(i),
listener.name(), listener.tcpBacklogSize(), bind_type, creation_options,
server_.options().concurrency()));
server_.options().concurrency());
if (!factory_or_error.status().ok()) {
socket_status = factory_or_error.status();
} else {
socket_status = listener.addSocketFactory(std::move(*factory_or_error));
}
if (!socket_status.ok()) {
break;
}
Expand Down
Loading

0 comments on commit 2386bd5

Please sign in to comment.