Skip to content

Commit

Permalink
Use factory to create RuntimeLoader
Browse files Browse the repository at this point in the history 
Signed-off-by: Yan Avlasov <yavlasov@google.com>
  • Loading branch information
@yanavlasov
yanavlasov committed Mar 26, 2024
1 parent 2d368c3 commit 32a82d8
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 71 deletions.
45 changes: 29 additions & 16 deletions source/common/runtime/runtime_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,33 +542,46 @@ absl::Status ProtoLayer::walkProtoValue(const ProtobufWkt::Value& v, const std::
return absl::OkStatus();
}

LoaderImpl::LoaderImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls,
LoaderImpl::LoaderImpl(ThreadLocal::SlotAllocator& tls,
const envoy::config::bootstrap::v3::LayeredRuntime& config,
const LocalInfo::LocalInfo& local_info, Stats::Store& store,
Random::RandomGenerator& generator,
ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api,
absl::Status& creation_status)
Random::RandomGenerator& generator, Api::Api& api)
: generator_(generator), stats_(generateStats(store)), tls_(tls.allocateSlot()),
config_(config), service_cluster_(local_info.clusterName()), api_(api),
init_watcher_("RTDS", [this]() { onRtdsReady(); }), store_(store) {
creation_status = absl::OkStatus();
init_watcher_("RTDS", [this]() { onRtdsReady(); }), store_(store) {}

absl::StatusOr<std::unique_ptr<LoaderImpl>>
LoaderImpl::create(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls,
const envoy::config::bootstrap::v3::LayeredRuntime& config,
const LocalInfo::LocalInfo& local_info, Stats::Store& store,
Random::RandomGenerator& generator,
ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api) {
auto loader =
std::unique_ptr<LoaderImpl>(new LoaderImpl(tls, config, local_info, store, generator, api));
auto result = loader->initLayers(dispatcher, validation_visitor);
if (!result.ok()) {
return result;
}
return loader;
}

absl::Status LoaderImpl::initLayers(Event::Dispatcher& dispatcher,
ProtobufMessage::ValidationVisitor& validation_visitor) {
absl::Status creation_status;
absl::node_hash_set<std::string> layer_names;
for (const auto& layer : config_.layers()) {
auto ret = layer_names.insert(layer.name());
if (!ret.second) {
creation_status =
absl::InvalidArgumentError(absl::StrCat("Duplicate layer name: ", layer.name()));
return;
return absl::InvalidArgumentError(absl::StrCat("Duplicate layer name: ", layer.name()));
}
switch (layer.layer_specifier_case()) {
case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::kStaticLayer:
// Nothing needs to be done here.
break;
case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::kAdminLayer:
if (admin_layer_ != nullptr) {
creation_status = absl::InvalidArgumentError(
return absl::InvalidArgumentError(
"Too many admin layers specified in LayeredRuntime, at most one may be specified");
return;
}
admin_layer_ = std::make_unique<AdminLayer>(layer.name(), stats_);
break;
Expand All @@ -580,21 +593,21 @@ LoaderImpl::LoaderImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator
Filesystem::Watcher::Events::MovedTo,
[this](uint32_t) { return loadNewSnapshot(); });
if (!creation_status.ok()) {
return;
return creation_status;
}
break;
case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::kRtdsLayer:
subscriptions_.emplace_back(
std::make_unique<RtdsSubscription>(*this, layer.rtds_layer(), store, validation_visitor));
subscriptions_.emplace_back(std::make_unique<RtdsSubscription>(*this, layer.rtds_layer(),
store_, validation_visitor));
init_manager_.add(subscriptions_.back()->init_target_);
break;
case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::LAYER_SPECIFIER_NOT_SET:
creation_status = absl::InvalidArgumentError("layer specifier not set");
return;
return absl::InvalidArgumentError("layer specifier not set");
}
}

creation_status = loadNewSnapshot();
return creation_status;
}

void LoaderImpl::initialize(Upstream::ClusterManager& cm) {
Expand Down
19 changes: 13 additions & 6 deletions source/common/runtime/runtime_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "source/common/singleton/threadsafe_singleton.h"

#include "absl/container/node_hash_map.h"
#include "absl/status/statusor.h"
#include "spdlog/spdlog.h"

namespace Envoy {
Expand Down Expand Up @@ -214,12 +215,12 @@ using RtdsSubscriptionPtr = std::unique_ptr<RtdsSubscription>;
*/
class LoaderImpl : public Loader, Logger::Loggable<Logger::Id::runtime> {
public:
LoaderImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls,
const envoy::config::bootstrap::v3::LayeredRuntime& config,
const LocalInfo::LocalInfo& local_info, Stats::Store& store,
Random::RandomGenerator& generator,
ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api,
absl::Status& creation_status);
static absl::StatusOr<std::unique_ptr<LoaderImpl>>
create(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator& tls,
const envoy::config::bootstrap::v3::LayeredRuntime& config,
const LocalInfo::LocalInfo& local_info, Stats::Store& store,
Random::RandomGenerator& generator, ProtobufMessage::ValidationVisitor& validation_visitor,
Api::Api& api);

// Runtime::Loader
void initialize(Upstream::ClusterManager& cm) override;
Expand All @@ -232,7 +233,13 @@ class LoaderImpl : public Loader, Logger::Loggable<Logger::Id::runtime> {

private:
friend RtdsSubscription;
LoaderImpl(ThreadLocal::SlotAllocator& tls,
const envoy::config::bootstrap::v3::LayeredRuntime& config,
const LocalInfo::LocalInfo& local_info, Stats::Store& store,
Random::RandomGenerator& generator, Api::Api& api);

absl::Status initLayers(Event::Dispatcher& dispatcher,
ProtobufMessage::ValidationVisitor& validation_visitor);
// Create a new Snapshot
absl::StatusOr<SnapshotImplPtr> createNewSnapshot();
// Load a new Snapshot into TLS
Expand Down
9 changes: 4 additions & 5 deletions source/server/server.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,13 +864,12 @@ Runtime::LoaderPtr InstanceUtil::createRuntime(Instance& server,
#ifdef ENVOY_ENABLE_YAML
ENVOY_LOG(info, "runtime: {}", MessageUtil::getYamlStringFromMessage(config.runtime()));
#endif
absl::Status creation_status;
auto loader = std::make_unique<Runtime::LoaderImpl>(
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader = Runtime::LoaderImpl::create(
server.dispatcher(), server.threadLocal(), config.runtime(), server.localInfo(),
server.stats(), server.api().randomGenerator(),
server.messageValidationContext().dynamicValidationVisitor(), server.api(), creation_status);
THROW_IF_NOT_OK(creation_status);
return loader;
server.messageValidationContext().dynamicValidationVisitor(), server.api());
THROW_IF_NOT_OK(loader.status());
return std::move(loader.value());
}

void InstanceBase::loadServerFlags(const absl::optional<std::string>& flags_path) {
Expand Down
42 changes: 21 additions & 21 deletions test/common/runtime/runtime_impl_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,11 @@ class DiskLoaderImplTest : public LoaderImplTest {

envoy::config::bootstrap::v3::LayeredRuntime layered_runtime;
Config::translateRuntime(runtime, layered_runtime);
absl::Status creation_status;
loader_ = std::make_unique<LoaderImpl>(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_, creation_status);
THROW_IF_NOT_OK(creation_status);
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader =
Runtime::LoaderImpl::create(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_);
THROW_IF_NOT_OK(loader.status());
loader_ = std::move(loader.value());
}

void write(const std::string& path, const std::string& value) {
Expand Down Expand Up @@ -519,11 +520,10 @@ TEST_F(DiskLoaderImplTest, MultipleAdminLayersFail) {
layer->set_name("admin_1");
layer->mutable_admin_layer();
}
absl::Status creation_status;
auto loader =
std::make_unique<LoaderImpl>(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_, creation_status);
EXPECT_EQ(creation_status.message(),
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader =
Runtime::LoaderImpl::create(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_);
EXPECT_EQ(loader.status().message(),
"Too many admin layers specified in LayeredRuntime, at most one may be specified");
}

Expand All @@ -542,10 +542,11 @@ class StaticLoaderImplTest : public LoaderImplTest {
layer->set_name("admin");
layer->mutable_admin_layer();
}
absl::Status creation_status;
loader_ = std::make_unique<LoaderImpl>(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_, creation_status);
THROW_IF_NOT_OK(creation_status);
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader =
Runtime::LoaderImpl::create(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_);
THROW_IF_NOT_OK(loader.status());
loader_ = std::move(loader.value());
}

ProtobufWkt::Struct base_;
Expand Down Expand Up @@ -957,10 +958,10 @@ class RtdsLoaderImplTest : public LoaderImplTest {
rtds_callbacks_.push_back(&callbacks);
return ret;
}));
absl::Status creation_status;
loader_ = std::make_unique<LoaderImpl>(dispatcher_, tls_, config, local_info_, store_,
generator_, validation_visitor_, *api_, creation_status);
THROW_IF_NOT_OK(creation_status);
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader = Runtime::LoaderImpl::create(
dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_, *api_);
THROW_IF_NOT_OK(loader.status());
loader_ = std::move(loader.value());
loader_->initialize(cm_);
for (auto* sub : rtds_subscriptions_) {
EXPECT_CALL(*sub, start(_));
Expand Down Expand Up @@ -1294,11 +1295,10 @@ TEST_F(RtdsLoaderImplTest, BadConfigSource) {
rtds_layer->mutable_rtds_config();

EXPECT_CALL(cm_, subscriptionFactory());
absl::Status creation_status;
LoaderImpl loader(dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_,
*api_, creation_status);
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader = Runtime::LoaderImpl::create(
dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_, *api_);

EXPECT_THROW_WITH_MESSAGE(loader.initialize(cm_), EnvoyException, "bad config");
EXPECT_THROW_WITH_MESSAGE(loader.value()->initialize(cm_), EnvoyException, "bad config");
}

} // namespace
Expand Down
10 changes: 5 additions & 5 deletions test/common/stats/thread_local_store_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2177,11 +2177,11 @@ class HistogramParameterisedTest : public HistogramTest,
layer->set_name("admin");
layer->mutable_admin_layer();
}
absl::Status creation_status;
loader_ = std::make_unique<Runtime::LoaderImpl>(dispatcher_, tls_, layered_runtime, local_info_,
*store_, generator_, validation_visitor_, *api_,
creation_status);
THROW_IF_NOT_OK(creation_status);
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader =
Runtime::LoaderImpl::create(dispatcher_, tls_, layered_runtime, local_info_, *store_,
generator_, validation_visitor_, *api_);
THROW_IF_NOT_OK(loader.status());
loader_ = std::move(loader.value());
}

NiceMock<Server::Configuration::MockServerFactoryContext> context_;
Expand Down
11 changes: 5 additions & 6 deletions test/extensions/filters/network/http_connection_manager/config_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,19 +659,18 @@ TEST_F(HttpConnectionManagerConfigTest, OverallSampling) {
envoy::config::bootstrap::v3::LayeredRuntime runtime_config;
NiceMock<LocalInfo::MockLocalInfo> local_info;
NiceMock<ProtobufMessage::MockValidationVisitor> validation_visitor;
absl::Status creation_status;
Runtime::LoaderImpl runtime(dispatcher, tls, runtime_config, local_info, store, generator,
validation_visitor, *api, creation_status);
ASSERT_TRUE(creation_status.ok());
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader = Runtime::LoaderImpl::create(
dispatcher, tls, runtime_config, local_info, store, generator, validation_visitor, *api);
ASSERT_TRUE(loader.ok());

int sampled_count = 0;
NiceMock<Router::MockRoute> route;
Envoy::Random::RandomGeneratorImpl rand;
for (int i = 0; i < 1000000; i++) {
Envoy::Http::TestRequestHeaderMapImpl header{{"x-request-id", rand.uuid()}};
config.requestIDExtension()->setTraceReason(header, Envoy::Tracing::Reason::Sampling);
auto reason = Envoy::Http::ConnectionManagerUtility::mutateTracingRequestHeader(header, runtime,
config, &route);
auto reason = Envoy::Http::ConnectionManagerUtility::mutateTracingRequestHeader(
header, *loader.value(), config, &route);
if (reason == Envoy::Tracing::Reason::Sampling) {
sampled_count++;
}
Expand Down
20 changes: 8 additions & 12 deletions test/test_common/test_runtime.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,11 @@ class TestScopedRuntime {
// The existence of an admin layer is required for mergeValues() to work.
config.add_layers()->mutable_admin_layer();

absl::Status creation_status;
Runtime::LoaderPtr runtime_ptr = std::make_unique<Runtime::LoaderImpl>(
dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_, *api_,
creation_status);
THROW_IF_NOT_OK(creation_status);
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader = Runtime::LoaderImpl::create(
dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_, *api_);
THROW_IF_NOT_OK(loader.status());
// This will ignore values set in test, but just use flag defaults!
runtime_ = std::move(runtime_ptr);
runtime_ = std::move(loader.value());
}

Runtime::Loader& loader() { return *runtime_; }
Expand Down Expand Up @@ -88,13 +86,11 @@ class TestScopedStaticReloadableFeaturesRuntime {
}
runtime->mutable_static_layer()->MergeFrom(envoy_layer);

absl::Status creation_status;
Runtime::LoaderPtr runtime_ptr = std::make_unique<Runtime::LoaderImpl>(
dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_, *api_,
creation_status);
THROW_IF_NOT_OK(creation_status);
absl::StatusOr<std::unique_ptr<Runtime::LoaderImpl>> loader = Runtime::LoaderImpl::create(
dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_, *api_);
THROW_IF_NOT_OK(loader.status());
// This will ignore values set in test, but just use flag defaults!
runtime_ = std::move(runtime_ptr);
runtime_ = std::move(loader.value());
}

protected:
Expand Down

0 comments on commit 32a82d8

Please sign in to comment.