diff --git a/include/envoy/init/init.h b/include/envoy/init/init.h index 824dbd01fac59..338511c3545b8 100644 --- a/include/envoy/init/init.h +++ b/include/envoy/init/init.h @@ -10,7 +10,8 @@ namespace Envoy { namespace Init { /** - * A single initialization target. + * A single initialization target. Deprecated, use SafeInit::Target instead. + * TODO(mergeconflict): convert all Init::Target implementations to SafeInit::TargetImpl. */ class Target { public: @@ -25,7 +26,8 @@ class Target { }; /** - * A manager that initializes multiple targets. + * A manager that initializes multiple targets. Deprecated, use SafeInit::Manager instead. + * TODO(mergeconflict): convert all Init::Manager uses to SafeInit::Manager. */ class Manager { public: diff --git a/include/envoy/safe_init/BUILD b/include/envoy/safe_init/BUILD new file mode 100644 index 0000000000000..2229d7c7a12e4 --- /dev/null +++ b/include/envoy/safe_init/BUILD @@ -0,0 +1,31 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "watcher_interface", + hdrs = ["watcher.h"], +) + +envoy_cc_library( + name = "target_interface", + hdrs = ["target.h"], + deps = [ + ":watcher_interface", + ], +) + +envoy_cc_library( + name = "manager_interface", + hdrs = ["manager.h"], + deps = [ + ":target_interface", + ":watcher_interface", + ], +) diff --git a/include/envoy/safe_init/manager.h b/include/envoy/safe_init/manager.h new file mode 100644 index 0000000000000..a94718fbd2869 --- /dev/null +++ b/include/envoy/safe_init/manager.h @@ -0,0 +1,79 @@ +#pragma once + +#include "envoy/common/pure.h" +#include "envoy/safe_init/target.h" +#include "envoy/safe_init/watcher.h" + +namespace Envoy { +namespace SafeInit { + +/** + * SafeInit::Manager coordinates initialization of one or more "targets." A typical flow would be: + * + * - One or more initialization targets are registered with a manager using `add`. + * - The manager is told to `initialize` all its targets, given a Watcher to notify when all + * registered targets are initialized. + * - Each target will initialize, either immediately or asynchronously, and will signal + * `ready` to the manager when initialized. + * - When all targets are initialized, the manager signals `ready` to the watcher it was given + * previously. + * + * Since there are several entities involved in this flow -- the owner of the manager, the targets + * registered with the manager, and the manager itself -- it may be difficult or impossible in some + * cases to guarantee that their lifetimes line up correctly to avoid use-after-free errors. The + * interface design here in SafeInit allows implementations to avoid the issue: + * + * - A Target can only be initialized via a TargetHandle, which acts as a weak reference. + * Attempting to initialize a destroyed Target via its handle has no ill effects. + * - Likewise, a Watcher can only be notified that initialization was complete via a + * WatcherHandle, which acts as a weak reference as well. + * + * See target.h and watcher.h, as well as implementation in source/common/safe_init for details. + */ +struct Manager { + virtual ~Manager() = default; + + /** + * The manager's state, used e.g. for reporting in the admin server. + */ + enum class State { + /** + * Targets have not been initialized. + */ + Uninitialized, + /** + * Targets are currently being initialized. + */ + Initializing, + /** + * All targets have been initialized. + */ + Initialized + }; + + /** + * @return the current state of the manager. + */ + virtual State state() const PURE; + + /** + * Register an initialization target. If the manager's current state is uninitialized, the target + * will be saved for invocation later, when `initialize` is called. If the current state is + * initializing, the target will be invoked immediately. It is an error to register a target with + * a manager that is already in initialized state. + * @param target the target to be invoked when initialization begins. + */ + virtual void add(const Target& target) PURE; + + /** + * Start initialization of all previously registered targets, and notify the given Watcher when + * initialization is complete. It is an error to call initialize on a manager that is already in + * initializing or initialized state. If the manager contains no targets, initialization completes + * immediately. + * @param watcher the watcher to notify when initialization is complete. + */ + virtual void initialize(const Watcher& watcher) PURE; +}; + +} // namespace SafeInit +} // namespace Envoy diff --git a/include/envoy/safe_init/target.h b/include/envoy/safe_init/target.h new file mode 100644 index 0000000000000..25dd958d3a646 --- /dev/null +++ b/include/envoy/safe_init/target.h @@ -0,0 +1,52 @@ +#pragma once + +#include + +#include "envoy/common/pure.h" +#include "envoy/safe_init/watcher.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace SafeInit { + +/** + * A TargetHandle functions as a weak reference to a Target. It is how an implementation of + * SafeInit::Manager would safely tell a target to `initialize` with no guarantees about the + * target's lifetime. Typical usage (outside of SafeInit::ManagerImpl) does not require touching + * TargetHandles at all. + */ +struct TargetHandle { + virtual ~TargetHandle() = default; + + /** + * Tell the target to begin initialization, if it is still available. + * @param watcher A Watcher for the target to notify when it has initialized. + * @return true if the target received this call, false if the target was already destroyed. + */ + virtual bool initialize(const Watcher& watcher) const PURE; +}; +using TargetHandlePtr = std::unique_ptr; + +/** + * An initialization Target is an entity that can be registered with a Manager for initialization. + * It can only be invoked through a TargetHandle. + */ +struct Target { + virtual ~Target() = default; + + /** + * @return a human-readable target name, for logging / debugging. + */ + virtual absl::string_view name() const PURE; + + /** + * Create a new handle that can initialize this target. + * @param name a human readable handle name, for logging / debugging. + * @return a new handle that can initialize this target. + */ + virtual TargetHandlePtr createHandle(absl::string_view name) const PURE; +}; + +} // namespace SafeInit +} // namespace Envoy diff --git a/include/envoy/safe_init/watcher.h b/include/envoy/safe_init/watcher.h new file mode 100644 index 0000000000000..b9eb0cf08959e --- /dev/null +++ b/include/envoy/safe_init/watcher.h @@ -0,0 +1,53 @@ +#pragma once + +#include + +#include "envoy/common/pure.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace SafeInit { + +/** + * A WatcherHandle functions as a weak reference to a Watcher. It is how an implementation of + * SafeInit::Target would safely notify a Manager that it has initialized, and likewise it's how + * an implementation of SafeInit::Manager would safely tell its client that all registered targets + * have initialized, with no guarantees about the lifetimes of the manager or client. Typical usage + * (outside of SafeInit::TargetImpl and ManagerImpl) does not require touching WatcherHandles at + * all. + */ +struct WatcherHandle { + virtual ~WatcherHandle() = default; + + /** + * Tell the watcher that initialization has completed, if it is still available. + * @return true if the watcher received this call, false if the watcher was already destroyed. + */ + virtual bool ready() const PURE; +}; +using WatcherHandlePtr = std::unique_ptr; + +/** + * A Watcher is an entity that listens for notifications that either an initialization target or + * all targets registered with a manager have initialized. It can only be invoked through a + * WatcherHandle. + */ +struct Watcher { + virtual ~Watcher() = default; + + /** + * @return a human-readable target name, for logging / debugging. + */ + virtual absl::string_view name() const PURE; + + /** + * Create a new handle that can notify this watcher. + * @param name a human readable handle name, for logging / debugging. + * @return a new handle that can notify this watcher. + */ + virtual WatcherHandlePtr createHandle(absl::string_view name) const PURE; +}; + +} // namespace SafeInit +} // namespace Envoy diff --git a/source/common/safe_init/BUILD b/source/common/safe_init/BUILD new file mode 100644 index 0000000000000..269cd9fbaace6 --- /dev/null +++ b/source/common/safe_init/BUILD @@ -0,0 +1,40 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "watcher_lib", + srcs = ["watcher_impl.cc"], + hdrs = ["watcher_impl.h"], + deps = [ + "//include/envoy/safe_init:watcher_interface", + "//source/common/common:logger_lib", + ], +) + +envoy_cc_library( + name = "target_lib", + srcs = ["target_impl.cc"], + hdrs = ["target_impl.h"], + deps = [ + "//include/envoy/safe_init:target_interface", + "//source/common/common:logger_lib", + ], +) + +envoy_cc_library( + name = "manager_lib", + srcs = ["manager_impl.cc"], + hdrs = ["manager_impl.h"], + deps = [ + ":watcher_lib", + "//include/envoy/safe_init:manager_interface", + "//source/common/common:logger_lib", + ], +) diff --git a/source/common/safe_init/manager_impl.cc b/source/common/safe_init/manager_impl.cc new file mode 100644 index 0000000000000..a21827c67f2f0 --- /dev/null +++ b/source/common/safe_init/manager_impl.cc @@ -0,0 +1,79 @@ +#include "common/safe_init/manager_impl.h" + +#include "common/common/assert.h" + +namespace Envoy { +namespace SafeInit { + +ManagerImpl::ManagerImpl(absl::string_view name) + : name_(fmt::format("init manager {}", name)), state_(State::Uninitialized), count_(0), + watcher_(name_, [this]() { onTargetReady(); }) {} + +Manager::State ManagerImpl::state() const { return state_; } + +void ManagerImpl::add(const Target& target) { + ++count_; + TargetHandlePtr target_handle(target.createHandle(name_)); + switch (state_) { + case State::Uninitialized: + // If the manager isn't initialized yet, save the target handle to be initialized later. + ENVOY_LOG(debug, "added {} to {}", target.name(), name_); + target_handles_.push_back(std::move(target_handle)); + return; + case State::Initializing: + // If the manager is already initializing, initialize the new target immediately. Note that + // it's important in this case that count_ was incremented above before calling the target, + // because if the target calls the init manager back immediately, count_ will be decremented + // here (see the definition of watcher_ above). + target_handle->initialize(watcher_); + return; + case State::Initialized: + // If the manager has already completed initialization, consider this a programming error. + ASSERT(false, fmt::format("attempted to add {} to initialized {}", target.name(), name_)); + } +} + +void ManagerImpl::initialize(const Watcher& watcher) { + // If the manager is already initializing or initialized, consider this a programming error. + ASSERT(state_ == State::Uninitialized, fmt::format("attempted to initialize {} twice", name_)); + + // Create a handle to notify when initialization is complete. + watcher_handle_ = watcher.createHandle(name_); + + if (count_ == 0) { + // If we have no targets, initialization trivially completes. This can happen, and is fine. + ENVOY_LOG(debug, "{} contains no targets", name_); + ready(); + } else { + // If we have some targets, start initialization... + ENVOY_LOG(debug, "{} initializing", name_); + state_ = State::Initializing; + + // Attempt to initialize each target. If a target is unavailable, treat it as though it + // completed immediately. + for (const auto& target_handle : target_handles_) { + if (!target_handle->initialize(watcher_)) { + onTargetReady(); + } + } + } +} + +void ManagerImpl::onTargetReady() { + // If there are no remaining targets and one mysteriously calls us back, this manager is haunted. + ASSERT(count_ != 0, fmt::format("{} called back by target after initialization complete")); + + // If there are no uninitialized targets remaining when called back by a target, that means it was + // the last. Signal `ready` to the handle we saved in `initialize`. + if (--count_ == 0) { + ready(); + } +} + +void ManagerImpl::ready() { + state_ = State::Initialized; + watcher_handle_->ready(); +} + +} // namespace SafeInit +} // namespace Envoy diff --git a/source/common/safe_init/manager_impl.h b/source/common/safe_init/manager_impl.h new file mode 100644 index 0000000000000..7a88572422ad7 --- /dev/null +++ b/source/common/safe_init/manager_impl.h @@ -0,0 +1,62 @@ +#pragma once + +#include + +#include "envoy/safe_init/manager.h" + +#include "common/common/logger.h" +#include "common/safe_init/watcher_impl.h" + +namespace Envoy { +namespace SafeInit { + +/** + * SafeInit::ManagerImpl coordinates initialization of one or more "targets." See comments in + * include/envoy/safe_init/manager.h for an overview. + * + * When the logging level is set to "debug" or "trace," the log will contain entries for all + * significant events in the initialization flow: + * + * - Targets added to the manager + * - Initialization started for the manager and for each target + * - Initialization completed for each target and for the manager + * - Destruction of targets and watchers + * - Callbacks to "unavailable" (deleted) targets, manager, or watchers + */ +class ManagerImpl : public Manager, Logger::Loggable { +public: + /** + * @param name a human-readable manager name, for logging / debugging. + */ + ManagerImpl(absl::string_view name); + + // SafeInit::Manager + State state() const override; + void add(const Target& target) override; + void initialize(const Watcher& watcher) override; + +private: + void onTargetReady(); + void ready(); + + // Human-readable name for logging + const std::string name_; + + // Current state + State state_; + + // Current number of registered targets that have not yet initialized + uint32_t count_; + + // Handle to the watcher passed in `initialize`, to be called when initialization completes + WatcherHandlePtr watcher_handle_; + + // Watcher to receive ready notifications from each target + const WatcherImpl watcher_; + + // All registered targets + std::list target_handles_; +}; + +} // namespace SafeInit +} // namespace Envoy diff --git a/source/common/safe_init/target_impl.cc b/source/common/safe_init/target_impl.cc new file mode 100644 index 0000000000000..bdc839018e38e --- /dev/null +++ b/source/common/safe_init/target_impl.cc @@ -0,0 +1,54 @@ +#include "common/safe_init/target_impl.h" + +namespace Envoy { +namespace SafeInit { + +TargetHandleImpl::TargetHandleImpl(absl::string_view handle_name, absl::string_view name, + std::weak_ptr fn) + : handle_name_(handle_name), name_(name), fn_(std::move(fn)) {} + +bool TargetHandleImpl::initialize(const Watcher& watcher) const { + auto locked_fn(fn_.lock()); + if (locked_fn) { + // If we can "lock" a shared pointer to the target's callback function, call it + // with a new handle to the ManagerImpl's watcher that was passed in. + ENVOY_LOG(debug, "{} initializing {}", handle_name_, name_); + (*locked_fn)(watcher.createHandle(name_)); + return true; + } else { + // If not, the target was already destroyed. + ENVOY_LOG(debug, "{} can't initialize {} (unavailable)", handle_name_, name_); + return false; + } +} + +TargetImpl::TargetImpl(absl::string_view name, InitializeFn fn) + : name_(fmt::format("target {}", name)), + fn_(std::make_shared([this, fn](WatcherHandlePtr watcher_handle) { + watcher_handle_ = std::move(watcher_handle); + fn(); + })) {} + +TargetImpl::~TargetImpl() { ENVOY_LOG(debug, "{} destroyed", name_); } + +absl::string_view TargetImpl::name() const { return name_; } + +TargetHandlePtr TargetImpl::createHandle(absl::string_view handle_name) const { + // Note: can't use std::make_unique here because TargetHandleImpl ctor is private. + return std::unique_ptr( + new TargetHandleImpl(handle_name, name_, std::weak_ptr(fn_))); +} + +bool TargetImpl::ready() { + if (watcher_handle_) { + // If we have a handle for the ManagerImpl's watcher, signal it and then reset so it can't be + // accidentally signaled again. + const bool result = watcher_handle_->ready(); + watcher_handle_.reset(); + return result; + } + return false; +} + +} // namespace SafeInit +} // namespace Envoy diff --git a/source/common/safe_init/target_impl.h b/source/common/safe_init/target_impl.h new file mode 100644 index 0000000000000..675cfceb91eec --- /dev/null +++ b/source/common/safe_init/target_impl.h @@ -0,0 +1,89 @@ +#pragma once + +#include + +#include "envoy/safe_init/target.h" + +#include "common/common/logger.h" + +namespace Envoy { +namespace SafeInit { + +/** + * A target is just a glorified callback function, called by the manager it was registered with. + */ +using InitializeFn = std::function; + +/** + * Internally, the callback is slightly more sophisticated: it actually takes a WatcherHandlePtr + * that it uses to notify the manager when the target is ready. It saves this pointer when invoked + * and resets it later in `ready`. Users needn't care about this implementation detail, they only + * need to provide an `InitializeFn` above when constructing a target. + */ +using InternalInitalizeFn = std::function; + +/** + * A TargetHandleImpl functions as a weak reference to a TargetImpl. It is how a ManagerImpl safely + * tells a target to `initialize` with no guarantees about the target's lifetime. + */ +class TargetHandleImpl : public TargetHandle, Logger::Loggable { +private: + friend class TargetImpl; + TargetHandleImpl(absl::string_view handle_name, absl::string_view name, + std::weak_ptr fn); + +public: + // SafeInit::TargetHandle + bool initialize(const Watcher& watcher) const override; + +private: + // Name of the handle (almost always the name of the ManagerImpl calling the target) + const std::string handle_name_; + + // Name of the target + const std::string name_; + + // The target's callback function, only called if the weak pointer can be "locked" + const std::weak_ptr fn_; +}; + +/** + * A TargetImpl is an entity that can be registered with a Manager for initialization. It can only + * be invoked through a TargetHandle. + */ +class TargetImpl : public Target, Logger::Loggable { +public: + /** + * @param name a human-readable target name, for logging / debugging + * @fn a callback function to invoke when `initialize` is called on the handle. Note that this + * doesn't take a WatcherHandlePtr (like TargetFn does). Managing the watcher handle is done + * internally to simplify usage. + */ + TargetImpl(absl::string_view name, InitializeFn fn); + ~TargetImpl() override; + + // SafeInit::Target + absl::string_view name() const override; + TargetHandlePtr createHandle(absl::string_view handle_name) const override; + + /** + * Signal to the init manager that this target has finished initializing. This is safe to call + * any time. Calling it before initialization begins or after initialization has already ended + * will have no effect. + * @return true if the init manager received this call, false otherwise. + */ + bool ready(); + +private: + // Human-readable name for logging + const std::string name_; + + // Handle to the ManagerImpl's internal watcher, to call when this target is initialized + WatcherHandlePtr watcher_handle_; + + // The callback function, called via TargetHandleImpl by the manager + const std::shared_ptr fn_; +}; + +} // namespace SafeInit +} // namespace Envoy diff --git a/source/common/safe_init/watcher_impl.cc b/source/common/safe_init/watcher_impl.cc new file mode 100644 index 0000000000000..ee7899f55637f --- /dev/null +++ b/source/common/safe_init/watcher_impl.cc @@ -0,0 +1,38 @@ +#include "common/safe_init/watcher_impl.h" + +namespace Envoy { +namespace SafeInit { + +WatcherHandleImpl::WatcherHandleImpl(absl::string_view handle_name, absl::string_view name, + std::weak_ptr fn) + : handle_name_(handle_name), name_(name), fn_(std::move(fn)) {} + +bool WatcherHandleImpl::ready() const { + auto locked_fn(fn_.lock()); + if (locked_fn) { + // If we can "lock" a shared pointer to the watcher's callback function, call it. + ENVOY_LOG(debug, "{} initialized, notifying {}", handle_name_, name_); + (*locked_fn)(); + return true; + } else { + // If not, the watcher was already destroyed. + ENVOY_LOG(debug, "{} initialized, but can't notify {} (unavailable)", handle_name_, name_); + return false; + } +} + +WatcherImpl::WatcherImpl(absl::string_view name, ReadyFn fn) + : name_(name), fn_(std::make_shared(std::move(fn))) {} + +WatcherImpl::~WatcherImpl() { ENVOY_LOG(debug, "{} destroyed", name_); } + +absl::string_view WatcherImpl::name() const { return name_; } + +WatcherHandlePtr WatcherImpl::createHandle(absl::string_view handle_name) const { + // Note: can't use std::make_unique because WatcherHandleImpl ctor is private + return std::unique_ptr( + new WatcherHandleImpl(handle_name, name_, std::weak_ptr(fn_))); +} + +} // namespace SafeInit +} // namespace Envoy diff --git a/source/common/safe_init/watcher_impl.h b/source/common/safe_init/watcher_impl.h new file mode 100644 index 0000000000000..582fd64910816 --- /dev/null +++ b/source/common/safe_init/watcher_impl.h @@ -0,0 +1,73 @@ +#pragma once + +#include + +#include "envoy/safe_init/watcher.h" + +#include "common/common/logger.h" + +namespace Envoy { +namespace SafeInit { + +/** + * A watcher is just a glorified callback function, called by a target or a manager when + * initialization completes. + */ +using ReadyFn = std::function; + +/** + * A WatcherHandleImpl functions as a weak reference to a Watcher. It is how a TargetImpl safely + * notifies a ManagerImpl that it has initialized, and likewise it's how ManagerImpl safely tells + * its client that all registered targets have initialized, with no guarantees about the lifetimes + * of the manager or client. + */ +class WatcherHandleImpl : public WatcherHandle, Logger::Loggable { +private: + friend class WatcherImpl; + WatcherHandleImpl(absl::string_view handle_name, absl::string_view name, + std::weak_ptr fn); + +public: + // SafeInit::WatcherHandle + bool ready() const override; + +private: + // Name of the handle (either the name of the target calling the manager, or the name of the + // manager calling the client) + const std::string handle_name_; + + // Name of the watcher (either the name of the manager, or the name of the client) + const std::string name_; + + // The watcher's callback function, only called if the weak pointer can be "locked" + const std::weak_ptr fn_; +}; + +/** + * A WatcherImpl is an entity that listens for notifications that either an initialization target or + * all targets registered with a manager have initialized. It can only be invoked through a + * WatcherHandleImpl. + */ +class WatcherImpl : public Watcher, Logger::Loggable { +public: + /** + * @param name a human-readable watcher name, for logging / debugging + * @param fn a callback function to invoke when `ready` is called on the handle + */ + WatcherImpl(absl::string_view name, ReadyFn fn); + ~WatcherImpl() override; + + // SafeInit::Watcher + absl::string_view name() const override; + WatcherHandlePtr createHandle(absl::string_view handle_name) const override; + +private: + // Human-readable name for logging + const std::string name_; + + // The callback function, called via WatcherHandleImpl by either the target or the manager + const std::shared_ptr fn_; +}; + +} // namespace SafeInit +} // namespace Envoy diff --git a/source/server/init_manager_impl.h b/source/server/init_manager_impl.h index 0f3ec0d2321f7..e84ec4fbd32d7 100644 --- a/source/server/init_manager_impl.h +++ b/source/server/init_manager_impl.h @@ -11,7 +11,8 @@ namespace Server { /** * Implementation of Init::Manager for use during post cluster manager init / pre listening. - * TODO(JimmyCYJ): Move InitManagerImpl into a new subdirectory in source/ called init/. + * Deprecated, use SafeInit::ManagerImpl instead. + * TODO(mergeconflict): convert all Init::ManagerImpl uses to SafeInit::ManagerImpl. */ class InitManagerImpl : public Init::Manager, Logger::Loggable { public: diff --git a/test/common/safe_init/BUILD b/test/common/safe_init/BUILD new file mode 100644 index 0000000000000..35dd33cd09b7c --- /dev/null +++ b/test/common/safe_init/BUILD @@ -0,0 +1,34 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_test( + name = "watcher_impl_test", + srcs = ["watcher_impl_test.cc"], + deps = [ + "//test/mocks/safe_init:safe_init_mocks", + ], +) + +envoy_cc_test( + name = "target_impl_test", + srcs = ["target_impl_test.cc"], + deps = [ + "//test/mocks/safe_init:safe_init_mocks", + ], +) + +envoy_cc_test( + name = "manager_impl_test", + srcs = ["manager_impl_test.cc"], + deps = [ + "//source/common/safe_init:manager_lib", + "//test/mocks/safe_init:safe_init_mocks", + ], +) diff --git a/test/common/safe_init/manager_impl_test.cc b/test/common/safe_init/manager_impl_test.cc new file mode 100644 index 0000000000000..88e6cc97c7d34 --- /dev/null +++ b/test/common/safe_init/manager_impl_test.cc @@ -0,0 +1,194 @@ +#include "common/safe_init/manager_impl.h" + +#include "test/mocks/safe_init/mocks.h" + +#include "gtest/gtest.h" + +using ::testing::InSequence; +using ::testing::InvokeWithoutArgs; + +namespace Envoy { +namespace SafeInit { +namespace { + +void expectUninitialized(const Manager& m) { EXPECT_EQ(Manager::State::Uninitialized, m.state()); } +void expectInitializing(const Manager& m) { EXPECT_EQ(Manager::State::Initializing, m.state()); } +void expectInitialized(const Manager& m) { EXPECT_EQ(Manager::State::Initialized, m.state()); } + +TEST(SafeInitManagerImplTest, AddImmediateTargetsWhenUninitialized) { + InSequence s; + + ManagerImpl m("test"); + expectUninitialized(m); + + ExpectableTargetImpl t1("t1"); + m.add(t1); + + ExpectableTargetImpl t2("t2"); + m.add(t2); + + ExpectableWatcherImpl w; + + // initialization should complete immediately + t1.expectInitializeWillCallReady(); + t2.expectInitializeWillCallReady(); + w.expectReady(); + m.initialize(w); + expectInitialized(m); +} + +TEST(SafeInitManagerImplTest, AddAsyncTargetsWhenUninitialized) { + InSequence s; + + ManagerImpl m("test"); + expectUninitialized(m); + + ExpectableTargetImpl t1("t1"); + m.add(t1); + + ExpectableTargetImpl t2("t2"); + m.add(t2); + + ExpectableWatcherImpl w; + + // initialization should begin + t1.expectInitialize(); + t2.expectInitialize(); + m.initialize(w); + expectInitializing(m); + + // should still be initializing after first target initializes + t1.ready(); + expectInitializing(m); + + // initialization should finish after second target initializes + w.expectReady(); + t2.ready(); + expectInitialized(m); +} + +TEST(SafeInitManagerImplTest, AddMixedTargetsWhenUninitialized) { + InSequence s; + + ManagerImpl m("test"); + expectUninitialized(m); + + ExpectableTargetImpl t1("t1"); + m.add(t1); + + ExpectableTargetImpl t2("t2"); + m.add(t2); + + ExpectableWatcherImpl w; + + // initialization should begin, and first target will initialize immediately + t1.expectInitializeWillCallReady(); + t2.expectInitialize(); + m.initialize(w); + expectInitializing(m); + + // initialization should finish after second target initializes + w.expectReady(); + t2.ready(); + expectInitialized(m); +} + +TEST(SafeInitManagerImplTest, AddImmediateTargetWhenInitializing) { + InSequence s; + + ManagerImpl m("test"); + expectUninitialized(m); + + ExpectableTargetImpl t1("t1"); + m.add(t1); + + ExpectableWatcherImpl w; + + // initialization should begin + t1.expectInitialize(); + m.initialize(w); + expectInitializing(m); + + // adding an immediate target shouldn't finish initialization + ExpectableTargetImpl t2("t2"); + t2.expectInitializeWillCallReady(); + m.add(t2); + expectInitializing(m); + + // initialization should finish after original target initializes + w.expectReady(); + t1.ready(); + expectInitialized(m); +} + +TEST(SafeInitManagerImplTest, UnavailableTarget) { + InSequence s; + + ManagerImpl m("test"); + expectUninitialized(m); + + // add a target and destroy it + { + ExpectableTargetImpl t("t"); + m.add(t); + t.expectInitialize().Times(0); + } + + ExpectableWatcherImpl w; + + // initialization should complete despite the destroyed target + w.expectReady(); + m.initialize(w); + expectInitialized(m); +} + +TEST(SafeInitManagerImplTest, UnavailableManager) { + InSequence s; + + ExpectableTargetImpl t("t"); + ExpectableWatcherImpl w; + + { + ManagerImpl m("test"); + expectUninitialized(m); + + m.add(t); + + // initialization should begin before destroying the manager + t.expectInitialize(); + m.initialize(w); + expectInitializing(m); + } + + // the watcher should not be notified when the target is initialized + w.expectReady().Times(0); + t.ready(); +} + +TEST(SafeInitManagerImplTest, UnavailableWatcher) { + InSequence s; + + ManagerImpl m("test"); + expectUninitialized(m); + + ExpectableTargetImpl t("t"); + m.add(t); + + { + ExpectableWatcherImpl w; + + // initialization should begin before destroying the watcher + t.expectInitialize(); + m.initialize(w); + expectInitializing(m); + + w.expectReady().Times(0); + } + + // initialization should finish without notifying the watcher + t.ready(); +} + +} // namespace +} // namespace SafeInit +} // namespace Envoy diff --git a/test/common/safe_init/target_impl_test.cc b/test/common/safe_init/target_impl_test.cc new file mode 100644 index 0000000000000..df0c41fad2f1e --- /dev/null +++ b/test/common/safe_init/target_impl_test.cc @@ -0,0 +1,65 @@ +#include "test/mocks/safe_init/mocks.h" + +#include "gtest/gtest.h" + +using ::testing::InSequence; + +namespace Envoy { +namespace SafeInit { +namespace { + +TEST(SafeInitTargetImplTest, Name) { + ExpectableTargetImpl target; + EXPECT_EQ("target test", target.name()); +} + +TEST(SafeInitTargetImplTest, InitializeWhenAvailable) { + InSequence s; + + ExpectableTargetImpl target; + ExpectableWatcherImpl watcher; + + // initializing the target through its handle should invoke initialize()... + target.expectInitialize(); + EXPECT_TRUE(target.createHandle("test")->initialize(watcher)); + + // calling ready() on the target should invoke the saved watcher handle... + watcher.expectReady(); + EXPECT_TRUE(target.ready()); + + // calling ready() a second time should have no effect. + watcher.expectReady().Times(0); + EXPECT_FALSE(target.ready()); +} + +TEST(SafeInitTargetImplTest, InitializeWhenUnavailable) { + ExpectableWatcherImpl watcher; + TargetHandlePtr handle; + { + ExpectableTargetImpl target; + + // initializing the target after it's been destroyed should do nothing. + handle = target.createHandle("test"); + target.expectInitialize().Times(0); + } + EXPECT_FALSE(handle->initialize(watcher)); +} + +TEST(SafeInitTargetImplTest, ReadyWhenWatcherUnavailable) { + ExpectableTargetImpl target; + { + ExpectableWatcherImpl watcher; + + // initializing the target through its handle should invoke initialize()... + target.expectInitialize(); + EXPECT_TRUE(target.createHandle("test")->initialize(watcher)); + + // calling ready() on the target after the watcher has been destroyed should do nothing. + watcher.expectReady().Times(0); + } + EXPECT_FALSE(target.ready()); +} + +} // namespace +} // namespace SafeInit +} // namespace Envoy diff --git a/test/common/safe_init/watcher_impl_test.cc b/test/common/safe_init/watcher_impl_test.cc new file mode 100644 index 0000000000000..39abccbf40f93 --- /dev/null +++ b/test/common/safe_init/watcher_impl_test.cc @@ -0,0 +1,36 @@ +#include "test/mocks/safe_init/mocks.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace SafeInit { +namespace { + +TEST(SafeInitWatcherImplTest, Name) { + ExpectableWatcherImpl watcher; + EXPECT_EQ("test", watcher.name()); +} + +TEST(SafeInitWatcherImplTest, ReadyWhenAvailable) { + ExpectableWatcherImpl watcher; + + // notifying the watcher through its handle should invoke ready(). + watcher.expectReady(); + EXPECT_TRUE(watcher.createHandle("test")->ready()); +} + +TEST(SafeInitWatcherImplTest, ReadyWhenUnavailable) { + WatcherHandlePtr handle; + { + ExpectableWatcherImpl watcher; + + // notifying the watcher after it's been destroyed should do nothing. + handle = watcher.createHandle("test"); + watcher.expectReady().Times(0); + } + EXPECT_FALSE(handle->ready()); +} + +} // namespace +} // namespace SafeInit +} // namespace Envoy diff --git a/test/mocks/safe_init/BUILD b/test/mocks/safe_init/BUILD new file mode 100644 index 0000000000000..fbb24c52d1861 --- /dev/null +++ b/test/mocks/safe_init/BUILD @@ -0,0 +1,20 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_mock", + "envoy_package", +) + +envoy_package() + +envoy_cc_mock( + name = "safe_init_mocks", + srcs = ["mocks.cc"], + hdrs = ["mocks.h"], + deps = [ + "//include/envoy/safe_init:manager_interface", + "//source/common/safe_init:target_lib", + "//source/common/safe_init:watcher_lib", + ], +) diff --git a/test/mocks/safe_init/mocks.cc b/test/mocks/safe_init/mocks.cc new file mode 100644 index 0000000000000..1ef93da2f3074 --- /dev/null +++ b/test/mocks/safe_init/mocks.cc @@ -0,0 +1,25 @@ +#include "test/mocks/safe_init/mocks.h" + +namespace Envoy { +namespace SafeInit { + +using ::testing::Invoke; + +ExpectableWatcherImpl::ExpectableWatcherImpl(absl::string_view name) + : WatcherImpl(name, {[this]() { ready(); }}) {} +::testing::internal::TypedExpectation& ExpectableWatcherImpl::expectReady() const { + return EXPECT_CALL(*this, ready()); +} + +ExpectableTargetImpl::ExpectableTargetImpl(absl::string_view name) + : TargetImpl(name, {[this]() { initialize(); }}) {} +::testing::internal::TypedExpectation& ExpectableTargetImpl::expectInitialize() { + return EXPECT_CALL(*this, initialize()); +} +::testing::internal::TypedExpectation& +ExpectableTargetImpl::expectInitializeWillCallReady() { + return expectInitialize().WillOnce(Invoke([this]() { ready(); })); +} + +} // namespace SafeInit +} // namespace Envoy diff --git a/test/mocks/safe_init/mocks.h b/test/mocks/safe_init/mocks.h new file mode 100644 index 0000000000000..92a41cf4d7389 --- /dev/null +++ b/test/mocks/safe_init/mocks.h @@ -0,0 +1,66 @@ +#pragma once + +#include "envoy/safe_init/manager.h" + +#include "common/safe_init/target_impl.h" +#include "common/safe_init/watcher_impl.h" + +#include "gmock/gmock.h" + +namespace Envoy { +namespace SafeInit { + +/** + * ExpectableWatcherImpl is a real WatcherImpl, subclassed to add a mock `ready` method that you can + * set expectations on in tests. Tests should never want a watcher with different behavior than the + * real implementation. + */ +class ExpectableWatcherImpl : public WatcherImpl { +public: + ExpectableWatcherImpl(absl::string_view name = "test"); + MOCK_CONST_METHOD0(ready, void()); + + /** + * Convenience method to provide a shorthand for EXPECT_CALL(watcher, ready()). Can be chained, + * for example: watcher.expectReady().Times(0); + */ + ::testing::internal::TypedExpectation& expectReady() const; +}; + +/** + * ExpectableTargetImpl is a real TargetImpl, subclassed to add a mock `initialize` method that you + * can set expectations on in tests. Tests should never want a target with a different behavior than + * the real implementation. + */ +class ExpectableTargetImpl : public TargetImpl { +public: + ExpectableTargetImpl(absl::string_view name = "test"); + MOCK_METHOD0(initialize, void()); + + /** + * Convenience method to provide a shorthand for EXPECT_CALL(target, initialize()). Can be + * chained, for example: target.expectInitialize().Times(0); + */ + ::testing::internal::TypedExpectation& expectInitialize(); + + /** + * Convenience method to provide a shorthand for expectInitialize() with mocked behavior of + * calling `ready` immediately. + */ + ::testing::internal::TypedExpectation& expectInitializeWillCallReady(); +}; + +/** + * MockManager is a typical mock. In many cases, it won't be necessary to mock any of its methods. + * In cases where its `add` and `initialize` methods are actually called in a test, it's usually + * sufficient to mock `add` by saving the target argument locally, and to mock `initialize` by + * invoking the saved target with the watcher argument. + */ +struct MockManager : Manager { + MOCK_CONST_METHOD0(state, Manager::State()); + MOCK_METHOD1(add, void(const Target&)); + MOCK_METHOD1(initialize, void(const Watcher&)); +}; + +} // namespace SafeInit +} // namespace Envoy