Repository: mesos Updated Branches: refs/heads/master b8888f1e3 -> 93701846c
Added style/syntax changes made to modules abstractions. This was done by Kapil Arya, Niklas Nielsen, and myself. Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/93701846 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/93701846 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/93701846 Branch: refs/heads/master Commit: 93701846c88e1da3f45cb7b14b54d81f9f3bc68f Parents: b8888f1 Author: Benjamin Hindman <[email protected]> Authored: Wed Oct 8 16:00:44 2014 -0700 Committer: Benjamin Hindman <[email protected]> Committed: Wed Oct 8 16:00:44 2014 -0700 ---------------------------------------------------------------------- include/mesos/module.hpp | 15 ++-- src/examples/example_module_impl.cpp | 2 +- src/examples/test_module.hpp | 6 +- src/module/manager.cpp | 104 +++++++++++++------------- src/module/manager.hpp | 36 +++++---- src/tests/master_tests.cpp | 2 - src/tests/module_tests.cpp | 120 ++++++++++++++++-------------- 7 files changed, 154 insertions(+), 131 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/93701846/include/mesos/module.hpp ---------------------------------------------------------------------- diff --git a/include/mesos/module.hpp b/include/mesos/module.hpp index a172a34..733b150 100644 --- a/include/mesos/module.hpp +++ b/include/mesos/module.hpp @@ -16,8 +16,8 @@ * limitations under the License. */ -#ifndef __MODULE_HPP__ -#define __MODULE_HPP__ +#ifndef __MESOS_MODULE_HPP__ +#define __MESOS_MODULE_HPP__ // Mesos Module API (MESOS-1384). // @@ -28,7 +28,7 @@ // argument (in JSON format or a path to a JSON file). // // JSON := [library, ...] -// library := {"path": <library path>, "modules" : [<module name>, ...]} +// library := {"path": <library path>, "modules": [<module name>, ...]} // // How to write a module library: // 1. Define a create() function that returns a pointer to an object @@ -47,6 +47,8 @@ #define MESOS_MODULE_API_VERSION "1" +namespace mesos { + // Internal utilities, not part of the module API: // Declare a loadable module library. @@ -72,6 +74,7 @@ struct ModuleBase const char* moduleApiVersion; const char* mesosVersion; + // String representation of module 'kind' returned from 'create'. const char* kind; const char* authorName; @@ -90,7 +93,9 @@ struct ModuleBase // This declaration is neeed only for later specializations. -template<class T> +template <typename T> struct Module; -#endif // __MODULE_HPP__ +} // namespace mesos { + +#endif // __MESOS_MODULE_HPP__ http://git-wip-us.apache.org/repos/asf/mesos/blob/93701846/src/examples/example_module_impl.cpp ---------------------------------------------------------------------- diff --git a/src/examples/example_module_impl.cpp b/src/examples/example_module_impl.cpp index dcba355..27c0520 100644 --- a/src/examples/example_module_impl.cpp +++ b/src/examples/example_module_impl.cpp @@ -56,7 +56,7 @@ static TestModule* create() // checks. // create() hook returns an object of type 'TestModule'. // Mesos core binds the module instance pointer as needed. -Module<TestModule> exampleModule( +mesos::Module<TestModule> org_apache_mesos_TestModule( MESOS_MODULE_API_VERSION, MESOS_VERSION, "author", http://git-wip-us.apache.org/repos/asf/mesos/blob/93701846/src/examples/test_module.hpp ---------------------------------------------------------------------- diff --git a/src/examples/test_module.hpp b/src/examples/test_module.hpp index 4760988..b5cc2fa 100644 --- a/src/examples/test_module.hpp +++ b/src/examples/test_module.hpp @@ -42,7 +42,9 @@ public: }; -template<> +namespace mesos { + +template <> struct Module<TestModule> : ModuleBase { Module( @@ -67,4 +69,6 @@ struct Module<TestModule> : ModuleBase TestModule* (*create)(); }; +} // namespace mesos { + #endif // __TEST_MODULE_HPP__ http://git-wip-us.apache.org/repos/asf/mesos/blob/93701846/src/module/manager.cpp ---------------------------------------------------------------------- diff --git a/src/module/manager.cpp b/src/module/manager.cpp index e178242..72041c0 100644 --- a/src/module/manager.cpp +++ b/src/module/manager.cpp @@ -19,6 +19,8 @@ #include <string> #include <vector> +#include <mesos/module.hpp> + #include <stout/json.hpp> #include <stout/numify.hpp> #include <stout/os.hpp> @@ -26,8 +28,6 @@ #include <stout/stringify.hpp> #include <stout/version.hpp> -#include "mesos/module.hpp" - #include "manager.hpp" using std::list; @@ -46,38 +46,40 @@ list<Owned<DynamicLibrary> > ModuleManager::dynamicLibraries; void ModuleManager::initialize() { -// ATTENTION: Every time a Mesos developer breaks compatibility with a -// module kind type, this table needs to be updated. Specifically, -// the version value in the entry corresponding to the kind needs to -// be set to the Mesos version that affects the current change. -// Typically that should be the version currently under development. + // ATTENTION: Every time a Mesos developer breaks compatibility with + // a module kind type, this table needs to be updated. + // Specifically, the version value in the entry corresponding to the + // kind needs to be set to the Mesos version that affects the + // current change. Typically that should be the version currently + // under development. kindToVersion["TestModule"] = MESOS_VERSION; -// What happens then when Mesos is built with a certain version, -// 'kindToVersion' states a certain other minimum version, and a -// module library is built against "module.hpp" belonging to yet -// another Mesos version? -// -// Mesos can admit modules built against earlier versions of itself -// by stating so explicitly in 'kindToVersion'. If a modules is built -// with a Mesos version greater than or equal to the one stated in -// 'kindToVersion', it passes this verification step. Otherwise it is -// rejected when attempting to load it. -// -// Here are some examples: -// -// Mesos kindToVersion library modules loadable? -// 0.18.0 0.18.0 0.18.0 YES -// 0.29.0 0.18.0 0.18.0 YES -// 0.29.0 0.18.0 0.21.0 YES -// 0.18.0 0.18.0 0.29.0 NO -// 0.29.0 0.21.0 0.18.0 NO -// 0.29.0 0.29.0 0.18.0 NO - -// ATTENTION: This mechanism only protects the interfaces of modules, -// not how they maintain functional compatibility with Mesos and among -// each other. This is covered by their own "isCompatible" call. + // What happens then when Mesos is built with a certain version, + // 'kindToVersion' states a certain other minimum version, and a + // module library is built against "module.hpp" belonging to yet + // another Mesos version? + // + // Mesos can admit modules built against earlier versions of itself + // by stating so explicitly in 'kindToVersion'. If a modules is + // built with a Mesos version greater than or equal to the one + // stated in 'kindToVersion', it passes this verification step. + // Otherwise it is rejected when attempting to load it. + // + // Here are some examples: + // + // Mesos kindToVersion library modules loadable? + // 0.18.0 0.18.0 0.18.0 YES + // 0.29.0 0.18.0 0.18.0 YES + // 0.29.0 0.18.0 0.21.0 YES + // 0.18.0 0.18.0 0.29.0 NO + // 0.29.0 0.21.0 0.18.0 NO + // 0.29.0 0.29.0 0.18.0 NO + + // ATTENTION: This mechanism only protects the interfaces of + // modules, not how they maintain functional compatibility with + // Mesos and among each other. This is covered by their own + // "isCompatible" call. } @@ -93,7 +95,8 @@ void ModuleManager::unloadAll() // TODO(karya): Show library author info for failed library/module. Try<Nothing> ModuleManager::verifyModule( - const string& moduleName, const ModuleBase* moduleBase) + const string& moduleName, + const ModuleBase* moduleBase) { CHECK_NOTNULL(moduleBase); if (moduleBase->mesosVersion == NULL || @@ -105,7 +108,7 @@ Try<Nothing> ModuleManager::verifyModule( return Error("Error loading module '" + moduleName + "'; missing fields"); } - // Verify module api version. + // Verify module API version. if (stringify(moduleBase->moduleApiVersion) != MESOS_MODULE_API_VERSION) { return Error( "Module API version mismatch. Mesos has: " MESOS_MODULE_API_VERSION ", " @@ -160,48 +163,47 @@ Try<Nothing> ModuleManager::verifyModule( } -void ModuleManager::load(const Modules& modules) +Try<Nothing> ModuleManager::load(const Modules& modules) { Lock lock(&mutex); initialize(); foreach (const Modules::Library& library, modules.libraries()) { - const string& path = library.path(); - if (path.empty()) { - LOG(WARNING) << "Library path not provided"; - continue; + if (!library.has_path()) { + return Error("Library path not provided"); } - Owned<DynamicLibrary> lib = Owned<DynamicLibrary>(new DynamicLibrary()); - Try<Nothing> result = lib.get()->open(path); + + Owned<DynamicLibrary> dynamicLibrary(new DynamicLibrary()); + Try<Nothing> result = dynamicLibrary->open(library.path()); if (!result.isSome()) { - LOG(WARNING) << "Error opening library: " << path; - continue; + return Error("Error opening library: '" + library.path() + "'"); } + // Currently we never delete the DynamicLibrary instance nor do we // expose a way to delete it so for now we just put it in a list. // TODO(karya): If we add the functionality to "unload" a module // library, we should make this pointer addressable by something // like the module name. - dynamicLibraries.push_back(lib); + dynamicLibraries.push_back(dynamicLibrary); // Load module manifests. foreach (const string& moduleName, library.modules()) { - Try<void*> symbol = lib->loadSymbol(moduleName); + Try<void*> symbol = dynamicLibrary->loadSymbol(moduleName); if (symbol.isError()) { - LOG(WARNING) << "Error loading module '" << moduleName << "': " - << symbol.error(); - continue; + return Error( + "Error loading module '" + moduleName + "': " + symbol.error()); } - ModuleBase* moduleBase = (ModuleBase*) symbol.get(); + ModuleBase* moduleBase = (ModuleBase*) symbol.get(); Try<Nothing> result = verifyModule(moduleName, moduleBase); if (result.isError()) { - LOG(WARNING) << "Error verifying module '" << moduleName << "': " - << result.error(); - continue; + return Error( + "Error verifying module '" + moduleName + "': " + result.error()); } moduleBases[moduleName] = (ModuleBase*) symbol.get(); } } + + return Nothing(); } } // namespace internal { http://git-wip-us.apache.org/repos/asf/mesos/blob/93701846/src/module/manager.hpp ---------------------------------------------------------------------- diff --git a/src/module/manager.hpp b/src/module/manager.hpp index 093c139..8275fd0 100644 --- a/src/module/manager.hpp +++ b/src/module/manager.hpp @@ -45,29 +45,34 @@ namespace internal { // Mesos module loading. // // Phases: -// 1. Load dynamic libraries that contain modules. The command-line -// flag "--modules" is used to declare all available library-module -// tuples. + +// 1. Load dynamic libraries that contain modules from the Modules +// instance which may have come from a commandline flag. // 2. Verify versions and compatibilities. // a) Library compatibility. (Module API version check) // b) Module compatibility. (Module Kind version check) // 3. Instantiate singleton per module. (happens in the library) // 4. Bind reference to use case. (happens in Mesos) + + class ModuleManager { public: // Loads dynamic libraries, and verifies the compatibility of the // modules in them. - static void load(const Modules& modules); + // + // NOTE: If loading fails at a particular library we don't unload + // all of the already loaded libraries. + static Try<Nothing> load(const Modules& modules); // create() should be called only after load(). - template<typename Kind> + template <typename Kind> static Try<Kind*> create(const std::string& moduleName) { Lock lock(&mutex); if (!moduleBases.contains(moduleName)) { return Error( - "Module '" + moduleName + "' not specified with --module flag"); + "Module '" + moduleName + "' unknown"); } Module<Kind>* module = (Module<Kind>*) moduleBases[moduleName]; @@ -76,34 +81,35 @@ public: "Error creating Module instance for '" + moduleName + "': " "create() method not found"); } - Kind* singleton = module->create(); - if (singleton == NULL) { + Kind* kind = module->create(); + if (kind == NULL) { return Error("Error creating Module instance for '" + moduleName + "'"); } - return singleton; + return kind; } + // Exposed just for testing so that we can close all open dynamic + // libraries. static void unloadAll(); private: - typedef hashmap<const std::string, std::vector<std::string> > - LibraryModuleMap; - static void initialize(); - static Try<LibraryModuleMap> parseFlag(const std::string& flag); - static Try<Nothing> verifyModule( - const std::string& moduleName, const ModuleBase* moduleBase); + const std::string& moduleName, + const ModuleBase* moduleBase); // TODO(karya): Replace pthread_mutex_t with std::mutex in // common/lock.hpp and other places that refer to it. static pthread_mutex_t mutex; + static hashmap<const std::string, std::string> kindToVersion; + // Mapping from "module name" to the actual ModuleBase. If two // modules from different libraries have the same name then the last // one specified in the protobuf Modules will be picked. static hashmap<const std::string, ModuleBase*> moduleBases; + // A list of dynamic libraries to keep the object from getting // destructed. // TODO(karya): Make it addressable only when we decide to implement http://git-wip-us.apache.org/repos/asf/mesos/blob/93701846/src/tests/master_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp index 75435f0..d9dc40c 100644 --- a/src/tests/master_tests.cpp +++ b/src/tests/master_tests.cpp @@ -26,8 +26,6 @@ #include <mesos/executor.hpp> #include <mesos/scheduler.hpp> -#include <messages/messages.hpp> - #include <process/clock.hpp> #include <process/future.hpp> #include <process/gmock.hpp> http://git-wip-us.apache.org/repos/asf/mesos/blob/93701846/src/tests/module_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/module_tests.cpp b/src/tests/module_tests.cpp index a651bef..6f9ee33 100644 --- a/src/tests/module_tests.cpp +++ b/src/tests/module_tests.cpp @@ -33,12 +33,16 @@ using namespace mesos; using namespace mesos::internal; using namespace mesos::internal::tests; + class ModuleTest : public MesosTest {}; -static const string getLibraryPath(string libraryName) +static const string getLibraryPath(const string& libraryName) { return path::join( - tests::flags.build_dir, "src", ".libs", "lib" + libraryName + + tests::flags.build_dir, + "src", + ".libs", + "lib" + libraryName + #ifdef __linux__ ".so" #else @@ -65,9 +69,10 @@ static Modules getModules(const string& libraryName, const string& moduleName) TEST_F(ModuleTest, ExampleModuleParseStringTest) { const string libraryName = "examplemodule"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + Modules modules = getModules(libraryName, moduleName); - ModuleManager::load(modules); + EXPECT_SOME(ModuleManager::load(modules)); Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName); EXPECT_SOME(module); @@ -77,6 +82,7 @@ TEST_F(ModuleTest, ExampleModuleParseStringTest) // product. EXPECT_EQ(module.get()->foo('A', 1024), 1089); EXPECT_EQ(module.get()->bar(0.5, 10.8), 5); + ModuleManager::unloadAll(); } @@ -85,18 +91,21 @@ TEST_F(ModuleTest, ExampleModuleParseStringTest) TEST_F(ModuleTest, AuthorInfoTest) { const string libraryName = "examplemodule"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + DynamicLibrary library; - Try<Nothing> result = library.open(getLibraryPath(libraryName)); - EXPECT_SOME(result); + EXPECT_SOME(library.open(getLibraryPath(libraryName))); + // Check the return values against the values defined in // test_module_impl.cpp. Try<void*> symbol = library.loadSymbol(moduleName); EXPECT_SOME(symbol); + ModuleBase* moduleBase = (ModuleBase*) symbol.get(); EXPECT_EQ(stringify(moduleBase->authorName), "author"); EXPECT_EQ(stringify(moduleBase->authorEmail), "[email protected]"); EXPECT_EQ(stringify(moduleBase->description), "This is a test module."); + ModuleManager::unloadAll(); } @@ -105,12 +114,12 @@ TEST_F(ModuleTest, AuthorInfoTest) TEST_F(ModuleTest, UnknownLibraryTest) { const string libraryName = "unknown"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + Modules modules = getModules(libraryName, moduleName); - ModuleManager::load(modules); - Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); + EXPECT_ERROR(ModuleManager::load(modules)); + ModuleManager::unloadAll(); } @@ -121,11 +130,10 @@ TEST_F(ModuleTest, UnknownModuleTest) { const string libraryName = "examplemodule"; const string moduleName = "unknown"; + Modules modules = getModules(libraryName, moduleName); - ModuleManager::load(modules); + EXPECT_ERROR(ModuleManager::load(modules)); - Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); ModuleManager::unloadAll(); } @@ -135,12 +143,13 @@ TEST_F(ModuleTest, UnknownModuleTest) TEST_F(ModuleTest, UnknownModuleInstantiationTest) { const string libraryName = "examplemodule"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + Modules modules = getModules(libraryName, moduleName); - ModuleManager::load(modules); + EXPECT_SOME(ModuleManager::load(modules)); + + EXPECT_ERROR(ModuleManager::create<TestModule>("unknown")); - Try<TestModule*> module = ModuleManager::create<TestModule>("unknown"); - EXPECT_ERROR(module); ModuleManager::unloadAll(); } @@ -149,36 +158,43 @@ TEST_F(ModuleTest, UnknownModuleInstantiationTest) TEST_F(ModuleTest, NonModuleLibrary) { const string libraryName = "mesos"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + Modules modules = getModules(libraryName, moduleName); - ModuleManager::load(modules); + EXPECT_ERROR(ModuleManager::load(modules)); - Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); ModuleManager::unloadAll(); } +// NOTE: We expect to pass 'version' which will outlive this function +// since we set it to the external module's 'mesosVersion'. static void updateModuleLibraryVersion( - DynamicLibrary* library, const string& moduleName, Try<const char*> version) + DynamicLibrary* library, + const string& moduleName, + const char* version) { - EXPECT_SOME(version); Try<void*> symbol = library->loadSymbol(moduleName); EXPECT_SOME(symbol); - ModuleBase* moduleBase = (ModuleBase*) symbol.get(); - moduleBase->mesosVersion = version.get(); + ModuleBase* moduleBase = (ModuleBase*) symbol.get(); + moduleBase->mesosVersion = version; } +// NOTE: Like above, we expect to pass 'version' which will outlive +// this function since we set it to the external module's +// 'mesosApiVersion'. static void updateModuleApiVersion( - DynamicLibrary* library, const string& moduleName, Try<const char*> version) + DynamicLibrary* library, + const string& moduleName, + const char* version) { - EXPECT_SOME(version); Try<void*> symbol = library->loadSymbol(moduleName); EXPECT_SOME(symbol); + ModuleBase* moduleBase = (ModuleBase*) symbol.get(); - moduleBase->moduleApiVersion = version.get(); + moduleBase->moduleApiVersion = version; } @@ -187,32 +203,29 @@ static void updateModuleApiVersion( TEST_F(ModuleTest, DifferentApiVersion) { const string libraryName = "examplemodule"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + DynamicLibrary library; - Try<Nothing> result = library.open(getLibraryPath(libraryName)); - EXPECT_SOME(result); + EXPECT_SOME(library.open(getLibraryPath(libraryName))); Modules modules = getModules(libraryName, moduleName); // Make the API version '0'. updateModuleApiVersion(&library, moduleName, "0"); - ModuleManager::load(modules); - Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); + EXPECT_ERROR(ModuleManager::load(modules)); + ModuleManager::unloadAll(); // Make the API version arbitrarily high. updateModuleApiVersion(&library, moduleName, "1000"); - ModuleManager::load(modules); - module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); + EXPECT_ERROR(ModuleManager::load(modules)); + ModuleManager::unloadAll(); // Make the API version some random string. updateModuleApiVersion(&library, moduleName, "ThisIsNotAnAPIVersion!"); - ModuleManager::load(modules); - module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); + EXPECT_ERROR(ModuleManager::load(modules)); + ModuleManager::unloadAll(); } @@ -222,18 +235,17 @@ TEST_F(ModuleTest, DifferentApiVersion) TEST_F(ModuleTest, NewerModuleLibrary) { const string libraryName = "examplemodule"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + DynamicLibrary library; - Try<Nothing> result = library.open(getLibraryPath(libraryName)); - EXPECT_SOME(result); + EXPECT_SOME(library.open(getLibraryPath(libraryName))); Modules modules = getModules(libraryName, moduleName); // Make the library version arbitrarily high. updateModuleLibraryVersion(&library, moduleName, "100.1.0"); - ModuleManager::load(modules); - Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); + EXPECT_ERROR(ModuleManager::load(modules)); + ModuleManager::unloadAll(); } @@ -243,20 +255,16 @@ TEST_F(ModuleTest, NewerModuleLibrary) TEST_F(ModuleTest, OlderModuleLibrary) { const string libraryName = "examplemodule"; - const string moduleName = "exampleModule"; + const string moduleName = "org_apache_mesos_TestModule"; + DynamicLibrary library; - Try<Nothing> result = library.open(getLibraryPath(libraryName)); - EXPECT_SOME(result); + EXPECT_SOME(library.open(getLibraryPath(libraryName))); Modules modules = getModules(libraryName, moduleName); // Make the library version arbitrarily low. updateModuleLibraryVersion(&library, moduleName, "0.1.0"); - ModuleManager::load(modules); - Try<TestModule*> module = ModuleManager::create<TestModule>(moduleName); - EXPECT_ERROR(module); + EXPECT_ERROR(ModuleManager::load(modules)); + ModuleManager::unloadAll(); } - - -// TODO(bernd): Add MESOS_MODULE_IS_COMPATIBILE() tests.
