runtime: removing most of the exceptions (envoyproxy#32708)

Signed-off-by: Alyssa Wilk <[email protected]>
zhxie · Mar 11, 2024 · 708fa7b · 708fa7b
1 parent ec68439
commit 708fa7b
Show file tree

Hide file tree

Showing 11 changed files with 167 additions and 106 deletions.
diff --git a/envoy/runtime/runtime.h b/envoy/runtime/runtime.h
@@ -247,8 +247,10 @@ class Loader {
    * Merge the given map of key-value pairs into the runtime's state. To remove a previous merge for
    * a key, use an empty string as the value.
    * @param values the values to merge
+   * @return a status indicating success or failure.
    */
-  virtual void mergeValues(const absl::node_hash_map<std::string, std::string>& values) PURE;
+  virtual absl::Status
+  mergeValues(const absl::node_hash_map<std::string, std::string>& values) PURE;
 
   /**
    * Initiate all RTDS subscriptions. The `on_done` callback is invoked when all RTDS requests

diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc
@@ -399,7 +399,7 @@ SnapshotImpl::Entry SnapshotImpl::createEntry(const ProtobufWkt::Value& value,
   return entry;
 }
 
-void AdminLayer::mergeValues(const absl::node_hash_map<std::string, std::string>& values) {
+absl::Status AdminLayer::mergeValues(const absl::node_hash_map<std::string, std::string>& values) {
 #ifdef ENVOY_ENABLE_YAML
   for (const auto& kv : values) {
     values_.erase(kv.first);
@@ -408,37 +408,38 @@ void AdminLayer::mergeValues(const absl::node_hash_map<std::string, std::string>
     }
   }
   stats_.admin_overrides_active_.set(values_.empty() ? 0 : 1);
+  return absl::OkStatus();
 #else
-  IS_ENVOY_BUG("Runtime admin reload requires YAML support");
   UNREFERENCED_PARAMETER(values);
-  return;
+  return absl::InvalidArgumentError("Runtime admin reload requires YAML support");
 #endif
 }
 
-DiskLayer::DiskLayer(absl::string_view name, const std::string& path, Api::Api& api)
+DiskLayer::DiskLayer(absl::string_view name, const std::string& path, Api::Api& api,
+                     absl::Status& creation_status)
     : OverrideLayerImpl{name} {
-  walkDirectory(path, "", 1, api);
+  creation_status = walkDirectory(path, "", 1, api);
 }
 
-void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix, uint32_t depth,
-                              Api::Api& api) {
+absl::Status DiskLayer::walkDirectory(const std::string& path, const std::string& prefix,
+                                      uint32_t depth, Api::Api& api) {
   // Maximum recursion depth for walkDirectory().
   static constexpr uint32_t MaxWalkDepth = 16;
 
   ENVOY_LOG(debug, "walking directory: {}", path);
   if (depth > MaxWalkDepth) {
-    throwEnvoyExceptionOrPanic(absl::StrCat("Walk recursion depth exceeded ", MaxWalkDepth));
+    return absl::InvalidArgumentError(absl::StrCat("Walk recursion depth exceeded ", MaxWalkDepth));
   }
   // Check if this is an obviously bad path.
   if (api.fileSystem().illegalPath(path)) {
-    throwEnvoyExceptionOrPanic(absl::StrCat("Invalid path: ", path));
+    return absl::InvalidArgumentError(absl::StrCat("Invalid path: ", path));
   }
 
   Filesystem::Directory directory(path);
   Filesystem::DirectoryIteratorImpl it = directory.begin();
-  THROW_IF_NOT_OK_REF(it.status());
+  RETURN_IF_STATUS_NOT_OK(it);
   for (; it != directory.end(); ++it) {
-    THROW_IF_NOT_OK_REF(it.status());
+    RETURN_IF_STATUS_NOT_OK(it);
     Filesystem::DirectoryEntry entry = *it;
     std::string full_path = path + "/" + entry.name_;
     std::string full_prefix;
@@ -450,7 +451,8 @@ void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix
 
     if (entry.type_ == Filesystem::FileType::Directory && entry.name_ != "." &&
         entry.name_ != "..") {
-      walkDirectory(full_path, full_prefix, depth + 1, api);
+      absl::Status status = walkDirectory(full_path, full_prefix, depth + 1, api);
+      RETURN_IF_NOT_OK(status);
     } else if (entry.type_ == Filesystem::FileType::Regular) {
       // Suck the file into a string. This is not very efficient but it should be good enough
       // for small files. Also, as noted elsewhere, none of this is non-blocking which could
@@ -461,7 +463,7 @@ void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix
       // Read the file and remove any comments. A comment is a line starting with a '#' character.
       // Comments are useful for placeholder files with no value.
       auto file_or_error = api.fileSystem().fileReadToEnd(full_path);
-      THROW_IF_STATUS_NOT_OK(file_or_error, throw);
+      RETURN_IF_STATUS_NOT_OK(file_or_error);
       const std::string text_file{file_or_error.value()};
 
       const auto lines = StringUtil::splitToken(text_file, "\n");
@@ -484,26 +486,32 @@ void DiskLayer::walkDirectory(const std::string& path, const std::string& prefix
 #else
       IS_ENVOY_BUG("Runtime admin reload requires YAML support");
       UNREFERENCED_PARAMETER(value);
-      return;
+      return absl::OkStatus();
 #endif
     }
   }
-  THROW_IF_NOT_OK_REF(it.status());
+  RETURN_IF_STATUS_NOT_OK(it);
+  return absl::OkStatus();
 }
 
-ProtoLayer::ProtoLayer(absl::string_view name, const ProtobufWkt::Struct& proto)
+ProtoLayer::ProtoLayer(absl::string_view name, const ProtobufWkt::Struct& proto,
+                       absl::Status& creation_status)
     : OverrideLayerImpl{name} {
+  creation_status = absl::OkStatus();
   for (const auto& f : proto.fields()) {
-    walkProtoValue(f.second, f.first);
+    creation_status = walkProtoValue(f.second, f.first);
+    if (!creation_status.ok()) {
+      return;
+    }
   }
 }
 
-void ProtoLayer::walkProtoValue(const ProtobufWkt::Value& v, const std::string& prefix) {
+absl::Status ProtoLayer::walkProtoValue(const ProtobufWkt::Value& v, const std::string& prefix) {
   switch (v.kind_case()) {
   case ProtobufWkt::Value::KIND_NOT_SET:
   case ProtobufWkt::Value::kListValue:
   case ProtobufWkt::Value::kNullValue:
-    throwEnvoyExceptionOrPanic(absl::StrCat("Invalid runtime entry value for ", prefix));
+    return absl::InvalidArgumentError(absl::StrCat("Invalid runtime entry value for ", prefix));
     break;
   case ProtobufWkt::Value::kStringValue:
     SnapshotImpl::addEntry(values_, prefix, v, "");
@@ -525,35 +533,42 @@ void ProtoLayer::walkProtoValue(const ProtobufWkt::Value& v, const std::string&
       break;
     }
     for (const auto& f : s.fields()) {
-      walkProtoValue(f.second, prefix + "." + f.first);
+      absl::Status status = walkProtoValue(f.second, prefix + "." + f.first);
+      RETURN_IF_NOT_OK(status);
     }
     break;
   }
   }
+  return absl::OkStatus();
 }
 
 LoaderImpl::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)
+                       ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api,
+                       absl::Status& creation_status)
     : 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();
   absl::node_hash_set<std::string> layer_names;
   for (const auto& layer : config_.layers()) {
     auto ret = layer_names.insert(layer.name());
     if (!ret.second) {
-      throwEnvoyExceptionOrPanic(absl::StrCat("Duplicate layer name: ", layer.name()));
+      creation_status =
+          absl::InvalidArgumentError(absl::StrCat("Duplicate layer name: ", layer.name()));
+      return;
     }
     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) {
-        throwEnvoyExceptionOrPanic(
+        creation_status = 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;
@@ -562,19 +577,20 @@ LoaderImpl::LoaderImpl(Event::Dispatcher& dispatcher, ThreadLocal::SlotAllocator
         watcher_ = dispatcher.createFilesystemWatcher();
       }
       watcher_->addWatch(layer.disk_layer().symlink_root(), Filesystem::Watcher::Events::MovedTo,
-                         [this](uint32_t) -> void { loadNewSnapshot(); });
+                         [this](uint32_t) -> void { THROW_IF_NOT_OK(loadNewSnapshot()); });
       break;
     case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::kRtdsLayer:
       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:
-      throwEnvoyExceptionOrPanic("layer specifier not set");
+      creation_status = absl::InvalidArgumentError("layer specifier not set");
+      return;
     }
   }
 
-  loadNewSnapshot();
+  creation_status = loadNewSnapshot();
 }
 
 void LoaderImpl::initialize(Upstream::ClusterManager& cm) {
@@ -626,7 +642,7 @@ RtdsSubscription::onConfigUpdate(const std::vector<Config::DecodedResourceRef>&
   }
   ENVOY_LOG(debug, "Reloading RTDS snapshot for onConfigUpdate");
   proto_.CopyFrom(runtime.layer());
-  parent_.loadNewSnapshot();
+  RETURN_IF_NOT_OK(parent_.loadNewSnapshot());
   init_target_.ready();
   return absl::OkStatus();
 }
@@ -680,13 +696,15 @@ absl::Status RtdsSubscription::onConfigRemoved(
   }
   ENVOY_LOG(debug, "Clear RTDS snapshot for onConfigUpdate");
   proto_.Clear();
-  parent_.loadNewSnapshot();
+  RETURN_IF_NOT_OK(parent_.loadNewSnapshot());
   init_target_.ready();
   return absl::OkStatus();
 }
 
-void LoaderImpl::loadNewSnapshot() {
-  std::shared_ptr<SnapshotImpl> ptr = createNewSnapshot();
+absl::Status LoaderImpl::loadNewSnapshot() {
+  auto snapshot_or_error = createNewSnapshot();
+  RETURN_IF_STATUS_NOT_OK(snapshot_or_error);
+  std::shared_ptr<SnapshotImpl> ptr = std::move(snapshot_or_error.value());
   tls_->set([ptr](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr {
     return std::static_pointer_cast<ThreadLocal::ThreadLocalObject>(ptr);
   });
@@ -697,6 +715,7 @@ void LoaderImpl::loadNewSnapshot() {
     absl::MutexLock lock(&snapshot_mutex_);
     thread_safe_snapshot_ = ptr;
   }
+  return absl::OkStatus();
 }
 
 const Snapshot& LoaderImpl::snapshot() {
@@ -716,12 +735,12 @@ SnapshotConstSharedPtr LoaderImpl::threadsafeSnapshot() {
   }
 }
 
-void LoaderImpl::mergeValues(const absl::node_hash_map<std::string, std::string>& values) {
+absl::Status LoaderImpl::mergeValues(const absl::node_hash_map<std::string, std::string>& values) {
   if (admin_layer_ == nullptr) {
-    throwEnvoyExceptionOrPanic("No admin layer specified");
+    return absl::InvalidArgumentError("No admin layer specified");
   }
-  admin_layer_->mergeValues(values);
-  loadNewSnapshot();
+  RETURN_IF_NOT_OK(admin_layer_->mergeValues(values));
+  return loadNewSnapshot();
 }
 
 Stats::Scope& LoaderImpl::getRootScope() { return *store_.rootScope(); }
@@ -735,15 +754,18 @@ RuntimeStats LoaderImpl::generateStats(Stats::Store& store) {
   return stats;
 }
 
-SnapshotImplPtr LoaderImpl::createNewSnapshot() {
+absl::StatusOr<SnapshotImplPtr> LoaderImpl::createNewSnapshot() {
   std::vector<Snapshot::OverrideLayerConstPtr> layers;
   uint32_t disk_layers = 0;
   uint32_t error_layers = 0;
   uint32_t rtds_layer = 0;
+  absl::Status creation_status;
   for (const auto& layer : config_.layers()) {
     switch (layer.layer_specifier_case()) {
     case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::kStaticLayer:
-      layers.emplace_back(std::make_unique<const ProtoLayer>(layer.name(), layer.static_layer()));
+      layers.emplace_back(
+          std::make_unique<const ProtoLayer>(layer.name(), layer.static_layer(), creation_status));
+      RETURN_IF_NOT_OK(creation_status);
       break;
     case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::kDiskLayer: {
       std::string path =
@@ -752,18 +774,27 @@ SnapshotImplPtr LoaderImpl::createNewSnapshot() {
         absl::StrAppend(&path, "/", service_cluster_);
       }
       if (api_.fileSystem().directoryExists(path)) {
+        std::unique_ptr<DiskLayer> disk_layer;
+        std::string error;
         TRY_ASSERT_MAIN_THREAD {
-          layers.emplace_back(std::make_unique<DiskLayer>(layer.name(), path, api_));
-          ++disk_layers;
+          absl::Status creation_status;
+          disk_layer = std::make_unique<DiskLayer>(layer.name(), path, api_, creation_status);
+          if (!creation_status.ok()) {
+            error = creation_status.message();
+          }
+          END_TRY
         }
-        END_TRY
-        CATCH(EnvoyException & e, {
+        CATCH(EnvoyException & e, { error = e.what(); });
+        if (error.empty()) {
+          layers.emplace_back(std::move(disk_layer));
+          ++disk_layers;
+        } else {
           // TODO(htuch): Consider latching here, rather than ignoring the
           // layer. This would be consistent with filesystem RTDS.
           ++error_layers;
           ENVOY_LOG(debug, "error loading runtime values for layer {} from disk: {}",
-                    layer.DebugString(), e.what());
-        });
+                    layer.DebugString(), error);
+        }
       }
       break;
     }
@@ -772,7 +803,9 @@ SnapshotImplPtr LoaderImpl::createNewSnapshot() {
       break;
     case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::kRtdsLayer: {
       auto* subscription = subscriptions_[rtds_layer++].get();
-      layers.emplace_back(std::make_unique<const ProtoLayer>(layer.name(), subscription->proto_));
+      layers.emplace_back(
+          std::make_unique<const ProtoLayer>(layer.name(), subscription->proto_, creation_status));
+      RETURN_IF_NOT_OK(creation_status);
       break;
     }
     case envoy::config::bootstrap::v3::RuntimeLayer::LayerSpecifierCase::LAYER_SPECIFIER_NOT_SET:

diff --git a/source/common/runtime/runtime_impl.h b/source/common/runtime/runtime_impl.h
@@ -117,8 +117,8 @@ class OverrideLayerImpl : public Snapshot::OverrideLayer {
 
 /**
  * Extension of OverrideLayerImpl that maintains an in-memory set of values. These values can be
- * modified programmatically via mergeValues(). AdminLayer is so named because it can be accessed
- * and manipulated by Envoy's admin interface.
+ * modified programmatically via mergeValues(). AdminLayer is so named because it
+ * can be accessed and manipulated by Envoy's admin interface.
  */
 class AdminLayer : public OverrideLayerImpl {
 public:
@@ -135,7 +135,7 @@ class AdminLayer : public OverrideLayerImpl {
    * Merge the provided values into our entry map. An empty value indicates that a key should be
    * removed from our map.
    */
-  void mergeValues(const absl::node_hash_map<std::string, std::string>& values);
+  absl::Status mergeValues(const absl::node_hash_map<std::string, std::string>& values);
 
 private:
   RuntimeStats& stats_;
@@ -148,11 +148,12 @@ using AdminLayerPtr = std::unique_ptr<AdminLayer>;
  */
 class DiskLayer : public OverrideLayerImpl, Logger::Loggable<Logger::Id::runtime> {
 public:
-  DiskLayer(absl::string_view name, const std::string& path, Api::Api& api);
+  DiskLayer(absl::string_view name, const std::string& path, Api::Api& api,
+            absl::Status& creation_status);
 
 private:
-  void walkDirectory(const std::string& path, const std::string& prefix, uint32_t depth,
-                     Api::Api& api);
+  absl::Status walkDirectory(const std::string& path, const std::string& prefix, uint32_t depth,
+                             Api::Api& api);
 
   const std::string path_;
   const Filesystem::WatcherPtr watcher_;
@@ -163,10 +164,11 @@ class DiskLayer : public OverrideLayerImpl, Logger::Loggable<Logger::Id::runtime
  */
 class ProtoLayer : public OverrideLayerImpl, Logger::Loggable<Logger::Id::runtime> {
 public:
-  ProtoLayer(absl::string_view name, const ProtobufWkt::Struct& proto);
+  ProtoLayer(absl::string_view name, const ProtobufWkt::Struct& proto,
+             absl::Status& creation_status);
 
 private:
-  void walkProtoValue(const ProtobufWkt::Value& v, const std::string& prefix);
+  absl::Status walkProtoValue(const ProtobufWkt::Value& v, const std::string& prefix);
 };
 
 class LoaderImpl;
@@ -216,13 +218,14 @@ class LoaderImpl : public Loader, Logger::Loggable<Logger::Id::runtime> {
              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);
+             ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api,
+             absl::Status& creation_status);
 
   // Runtime::Loader
   void initialize(Upstream::ClusterManager& cm) override;
   const Snapshot& snapshot() override;
   SnapshotConstSharedPtr threadsafeSnapshot() override;
-  void mergeValues(const absl::node_hash_map<std::string, std::string>& values) override;
+  absl::Status mergeValues(const absl::node_hash_map<std::string, std::string>& values) override;
   void startRtdsSubscriptions(ReadyCallback on_done) override;
   Stats::Scope& getRootScope() override;
   void countDeprecatedFeatureUse() const override;
@@ -231,9 +234,9 @@ class LoaderImpl : public Loader, Logger::Loggable<Logger::Id::runtime> {
   friend RtdsSubscription;
 
   // Create a new Snapshot
-  SnapshotImplPtr createNewSnapshot();
+  absl::StatusOr<SnapshotImplPtr> createNewSnapshot();
   // Load a new Snapshot into TLS
-  void loadNewSnapshot();
+  absl::Status loadNewSnapshot();
   RuntimeStats generateStats(Stats::Store& store);
   void onRtdsReady();