Sure thing - apologize the inconvenience. Niklas
On 15 January 2015 at 12:46, Vinod Kone <[email protected]> wrote: > Hey Niklas. One of the hook tests is continously failing on our internal > CI. I've filed https://issues.apache.org/jira/browse/MESOS-2226. Can you > take a look? > > Also, https://issues.apache.org/jira/browse/MESOS-2227 > > On Wed, Jan 14, 2015 at 3:41 PM, <[email protected]> wrote: > > > Repository: mesos > > Updated Branches: > > refs/heads/master 60007b9e7 -> 4e110d5d0 > > > > > > Added example hook module. > > > > This module provides hooks for master label decorator, slave executor > > environment decorator and slave remove executor. A couple of test > > cases are also provided to verify the hooks from this module. > > > > Review: https://reviews.apache.org/r/29496 > > > > > > Project: http://git-wip-us.apache.org/repos/asf/mesos/repo > > Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/4e110d5d > > Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/4e110d5d > > Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/4e110d5d > > > > Branch: refs/heads/master > > Commit: 4e110d5d0f0c6cd65b8b6e5edff30bb4c589a278 > > Parents: 0773763 > > Author: Kapil Arya <[email protected]> > > Authored: Wed Jan 14 13:49:06 2015 -0800 > > Committer: Niklas Q. Nielsen <[email protected]> > > Committed: Wed Jan 14 15:11:37 2015 -0800 > > > > ---------------------------------------------------------------------- > > src/Makefile.am | 7 + > > src/examples/test_hook_module.cpp | 138 +++++++++++++++++ > > src/tests/hook_tests.cpp | 272 > +++++++++++++++++++++++++++++++++ > > src/tests/module.cpp | 21 +++ > > src/tests/module.hpp | 3 +- > > 5 files changed, 440 insertions(+), 1 deletion(-) > > ---------------------------------------------------------------------- > > > > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/4e110d5d/src/Makefile.am > > ---------------------------------------------------------------------- > > diff --git a/src/Makefile.am b/src/Makefile.am > > index 100db39..256384e 100644 > > --- a/src/Makefile.am > > +++ b/src/Makefile.am > > @@ -1237,6 +1237,12 @@ libtestauthentication_la_SOURCES = > > examples/test_authentication_modules.cpp > > libtestauthentication_la_CPPFLAGS = $(MESOS_CPPFLAGS) > > libtestauthentication_la_LDFLAGS = -release $(PACKAGE_VERSION) -shared > > > > +# Library containing test Hook module. > > +lib_LTLIBRARIES += libtesthook.la > > +libtesthook_la_SOURCES = examples/test_hook_module.cpp > > +libtesthook_la_CPPFLAGS = $(MESOS_CPPFLAGS) > > +libtesthook_la_LDFLAGS = -release $(PACKAGE_VERSION) -shared > > + > > mesos_tests_SOURCES = \ > > tests/attributes_tests.cpp \ > > tests/authentication_tests.cpp \ > > @@ -1260,6 +1266,7 @@ mesos_tests_SOURCES = \ > > tests/flags.cpp \ > > tests/gc_tests.cpp \ > > tests/hierarchical_allocator_tests.cpp \ > > + tests/hook_tests.cpp \ > > tests/isolator_tests.cpp \ > > tests/log_tests.cpp \ > > tests/logging_tests.cpp \ > > > > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/4e110d5d/src/examples/test_hook_module.cpp > > ---------------------------------------------------------------------- > > diff --git a/src/examples/test_hook_module.cpp > > b/src/examples/test_hook_module.cpp > > new file mode 100644 > > index 0000000..04fd43e > > --- /dev/null > > +++ b/src/examples/test_hook_module.cpp > > @@ -0,0 +1,138 @@ > > +/** > > + * Licensed to the Apache Software Foundation (ASF) under one > > + * or more contributor license agreements. See the NOTICE file > > + * distributed with this work for additional information > > + * regarding copyright ownership. The ASF licenses this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in compliance > > + * with the License. You may obtain a copy of the License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, software > > + * distributed under the License is distributed on an "AS IS" BASIS, > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > > implied. > > + * See the License for the specific language governing permissions and > > + * limitations under the License. > > + */ > > + > > +#include <string> > > + > > +#include <mesos/mesos.hpp> > > +#include <mesos/module.hpp> > > + > > +#include <stout/foreach.hpp> > > +#include <stout/os.hpp> > > +#include <stout/try.hpp> > > + > > +#include "hook/hook.hpp" > > +#include "master/master.hpp" > > +#include "module/hook.hpp" > > +#include "slave/slave.hpp" > > + > > +using std::string; > > + > > +using namespace mesos; > > +using namespace mesos::internal; > > + > > +// Must be kept in sync with variables of the same name in > > +// tests/hook_tests.cpp. > > +const char* testLabelKey = "MESOS_Test_Label"; > > +const char* testLabelValue = "ApacheMesos"; > > +const char* testEnvironmentVariableName = > > "MESOS_TEST_ENVIRONMENT_VARIABLE"; > > + > > +class TestHook : public Hook > > +{ > > +public: > > + virtual Result<Labels> masterLaunchTaskLabelDecorator( > > + const TaskInfo& taskInfo, > > + const FrameworkInfo& frameworkInfo, > > + const SlaveInfo& slaveInfo) > > + { > > + LOG(INFO) << "Executing 'masterLaunchTaskLabelDecorator' hook"; > > + > > + Labels labels; > > + Label *label = labels.add_labels(); > > + label->set_key(testLabelKey); > > + label->set_value(testLabelValue); > > + > > + return labels; > > + } > > + > > + > > + // In this hook, we create a temporary file and add its path to an > > + // environment variable. Later on, this environment variable is > > + // looked up by the removeExecutorHook to locate and delete this > > + // file. > > + virtual Result<Environment> slaveLaunchExecutorEnvironmentDecorator( > > + const ExecutorInfo& executorInfo, > > + const TaskInfo& taskInfo) > > + { > > + LOG(INFO) << "Executing 'slaveLaunchExecutorEnvironmentDecorator' > > hook"; > > + > > + // Find the label value for the label that was created in the > > + // label decorator hook above. > > + Option<string> labelValue; > > + foreach (const Label& label, taskInfo.labels().labels()) { > > + if (label.key() == testLabelKey) { > > + labelValue = label.value(); > > + CHECK_EQ(labelValue.get(), testLabelValue); > > + } > > + } > > + CHECK_SOME(labelValue); > > + > > + // Create a temporary file. > > + Try<string> file = os::mktemp(); > > + CHECK_SOME(file); > > + CHECK_SOME(os::write(file.get(), labelValue.get())); > > + > > + // Inject file path into command environment. > > + Environment environment; > > + Environment::Variable* variable = environment.add_variables(); > > + variable->set_name(testEnvironmentVariableName); > > + variable->set_value(file.get()); > > + > > + return environment; > > + } > > + > > + > > + // This hook locates the file created by environment decorator hook > > + // and deletes it. > > + virtual Try<Nothing> slaveRemoveExecutorHook( > > + const FrameworkInfo& frameworkInfo, > > + const ExecutorInfo& executorInfo) > > + { > > + LOG(INFO) << "Executing 'slaveRemoveExecutorHook'"; > > + > > + foreach (const Environment::Variable& variable, > > + executorInfo.command().environment().variables()) { > > + if (variable.name() == testEnvironmentVariableName) { > > + string path = variable.value(); > > + // The removeExecutor hook may be called multiple times; thus > > + // we ignore the subsequent calls. > > + if (os::isfile(path)) { > > + CHECK_SOME(os::rm(path)); > > + } > > + break; > > + } > > + } > > + return Nothing(); > > + } > > +}; > > + > > + > > +static Hook* createHook(const Parameters& parameters) > > +{ > > + return new TestHook(); > > +} > > + > > + > > +// Declares a Hook module named 'TestHook'. > > +mesos::modules::Module<Hook> org_apache_mesos_TestHook( > > + MESOS_MODULE_API_VERSION, > > + MESOS_VERSION, > > + "Apache Mesos", > > + "[email protected]", > > + "Test Hook module.", > > + NULL, > > + createHook); > > > > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/4e110d5d/src/tests/hook_tests.cpp > > ---------------------------------------------------------------------- > > diff --git a/src/tests/hook_tests.cpp b/src/tests/hook_tests.cpp > > new file mode 100644 > > index 0000000..4e9a3a7 > > --- /dev/null > > +++ b/src/tests/hook_tests.cpp > > @@ -0,0 +1,272 @@ > > +/** > > + * Licensed to the Apache Software Foundation (ASF) under one > > + * or more contributor license agreements. See the NOTICE file > > + * distributed with this work for additional information > > + * regarding copyright ownership. The ASF licenses this file > > + * to you under the Apache License, Version 2.0 (the > > + * "License"); you may not use this file except in compliance > > + * with the License. You may obtain a copy of the License at > > + * > > + * http://www.apache.org/licenses/LICENSE-2.0 > > + * > > + * Unless required by applicable law or agreed to in writing, software > > + * distributed under the License is distributed on an "AS IS" BASIS, > > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > > implied. > > + * See the License for the specific language governing permissions and > > + * limitations under the License. > > + */ > > + > > +#include <mesos/module.hpp> > > + > > +#include <process/future.hpp> > > +#include <process/gmock.hpp> > > +#include <process/pid.hpp> > > + > > +#include <stout/option.hpp> > > +#include <stout/os.hpp> > > +#include <stout/try.hpp> > > + > > +#include "hook/manager.hpp" > > +#include "module/manager.hpp" > > +#include "master/flags.hpp" > > +#include "master/master.hpp" > > +#include "slave/flags.hpp" > > +#include "slave/slave.hpp" > > + > > +#include "tests/containerizer.hpp" > > +#include "tests/flags.hpp" > > +#include "tests/mesos.hpp" > > + > > +using std::string; > > + > > +using namespace mesos; > > +using namespace mesos::internal; > > +using namespace mesos::internal::tests; > > +using namespace mesos::modules; > > + > > +using mesos::internal::master::Master; > > +using mesos::internal::slave::Slave; > > + > > +using process::Future; > > +using process::PID; > > + > > +using std::string; > > +using std::vector; > > + > > +using testing::_; > > +using testing::Return; > > + > > +const char* HOOK_MODULE_LIBRARY_NAME = "testhook"; > > +const char* HOOK_MODULE_NAME = "org_apache_mesos_TestHook"; > > + > > +// Must be kept in sync with variables of the same name in > > +// examples/test_hook_module.cpp. > > +const char* testLabelKey = "MESOS_Test_Label"; > > +const char* testLabelValue = "ApacheMesos"; > > +const char* testEnvironmentVariableName = > > "MESOS_TEST_ENVIRONMENT_VARIABLE"; > > + > > +class HookTest : public MesosTest > > +{ > > +protected: > > + // TODO(karya): Replace constructor/destructor with SetUp/TearDown. > > + // Currently, using SetUp/TearDown causes VerifySlave* test to > > + // fail with a duplicate slave id message. However, everything > > + // seems normal when using this construction/destructor combo. > > + HookTest() > > + { > > + // Install hooks. > > + EXPECT_SOME(HookManager::initialize(HOOK_MODULE_NAME)); > > + } > > + > > + ~HookTest() > > + { > > + // Unload the hooks so a subsequent install may succeed. > > + EXPECT_SOME(HookManager::unload(HOOK_MODULE_NAME)); > > + } > > +}; > > + > > + > > +// Test varioud hook install/uninstall mechanisms. > > +TEST_F(HookTest, HookLoading) > > +{ > > + // Installing unknown hooks should fail. > > + EXPECT_ERROR(HookManager::initialize("Unknown Hook")); > > + > > + // Uninstalling an unknown hook should fail. > > + EXPECT_ERROR(HookManager::unload("Unknown Hook")); > > + > > + // Installing an already installed hook should fail. > > + EXPECT_ERROR(HookManager::initialize(HOOK_MODULE_NAME)); > > + > > + // Uninstalling a hook should succeed. > > + EXPECT_SOME(HookManager::unload(HOOK_MODULE_NAME)); > > + > > + // Uninstalling an already uninstalled hook should fail. > > + EXPECT_ERROR(HookManager::unload(HOOK_MODULE_NAME)); > > + // This is needed to allow the tear-down to succeed. > > + EXPECT_SOME(HookManager::initialize(HOOK_MODULE_NAME)); > > +} > > + > > + > > +// Test that the label decorator hook hangs a new label off the > > +// taskinfo message during master launch task. > > +TEST_F(HookTest, VerifyMasterLaunchTaskHook) > > +{ > > + Try<PID<Master> > master = StartMaster(CreateMasterFlags()); > > + ASSERT_SOME(master); > > + > > + TestContainerizer containerizer; > > + > > + StandaloneMasterDetector detector(master.get()); > > + > > + // Start a mock slave since we aren't testing the slave hooks yet. > > + MockSlave slave(CreateSlaveFlags(), &detector, &containerizer); > > + process::spawn(slave); > > + > > + MockScheduler sched; > > + MesosSchedulerDriver driver( > > + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); > > + > > + EXPECT_CALL(sched, registered(&driver, _, _)) > > + .Times(1); > > + > > + Future<vector<Offer> > offers; > > + EXPECT_CALL(sched, resourceOffers(&driver, _)) > > + .WillOnce(FutureArg<1>(&offers)) > > + .WillRepeatedly(Return()); // Ignore subsequent offers. > > + > > + driver.start(); > > + > > + AWAIT_READY(offers); > > + EXPECT_NE(0u, offers.get().size()); > > + > > + CommandInfo command; > > + command.set_value("sleep 10"); > > + > > + // Launch a task with the command executor. > > + TaskInfo task; > > + task.set_name(""); > > + task.mutable_task_id()->set_value("1"); > > + task.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id()); > > + task.mutable_resources()->MergeFrom(offers.get()[0].resources()); > > + task.mutable_command()->MergeFrom(command); > > + > > + vector<TaskInfo> tasks; > > + tasks.push_back(task); > > + > > + Future<TaskInfo> taskInfo; > > + EXPECT_CALL(slave, runTask(_, _, _, _, _)) > > + .Times(1) > > + .WillOnce(FutureArg<4>(&taskInfo)); > > + > > + driver.launchTasks(offers.get()[0].id(), tasks); > > + > > + AWAIT_READY(taskInfo); > > + > > + // At launchTasks, the label decorator hook inside should have been > > + // executed and we should see the labels now. > > + Option<string> labelValue; > > + foreach (const Label& label, taskInfo.get().labels().labels()) { > > + if (label.key() == testLabelKey) { > > + labelValue = label.value(); > > + } > > + } > > + EXPECT_SOME_EQ(testLabelValue, labelValue); > > + > > + driver.stop(); > > + driver.join(); > > + > > + process::terminate(slave); > > + process::wait(slave); > > + > > + Shutdown(); // Must shutdown before 'containerizer' gets deallocated. > > +} > > + > > + > > +// Test executor environment decorator hook and remove executor hook > > +// for slave. We expect the environment-decorator hook to create a > > +// temporary file and the remove-executor hook to delete that file. > > +TEST_F(HookTest, VerifySlaveLaunchExecutorHook) > > +{ > > + master::Flags masterFlags = CreateMasterFlags(); > > + > > + Try<PID<Master> > master = StartMaster(masterFlags); > > + ASSERT_SOME(master); > > + > > + slave::Flags slaveFlags = CreateSlaveFlags(); > > + > > + MockExecutor exec(DEFAULT_EXECUTOR_ID); > > + > > + TestContainerizer containerizer(&exec); > > + > > + Try<PID<Slave> > slave = StartSlave(&containerizer); > > + ASSERT_SOME(slave); > > + > > + MockScheduler sched; > > + MesosSchedulerDriver driver( > > + &sched, DEFAULT_FRAMEWORK_INFO, master.get(), DEFAULT_CREDENTIAL); > > + > > + EXPECT_CALL(sched, registered(&driver, _, _)) > > + .Times(1); > > + > > + Future<vector<Offer> > offers; > > + EXPECT_CALL(sched, resourceOffers(&driver, _)) > > + .WillOnce(FutureArg<1>(&offers)) > > + .WillRepeatedly(Return()); // Ignore subsequent offers. > > + > > + driver.start(); > > + > > + AWAIT_READY(offers); > > + EXPECT_NE(0u, offers.get().size()); > > + > > + // Launch a task with the command executor. > > + TaskInfo task; > > + task.set_name(""); > > + task.mutable_task_id()->set_value("1"); > > + task.mutable_slave_id()->MergeFrom(offers.get()[0].slave_id()); > > + task.mutable_resources()->MergeFrom(offers.get()[0].resources()); > > + task.mutable_executor()->MergeFrom(DEFAULT_EXECUTOR_INFO); > > + > > + vector<TaskInfo> tasks; > > + tasks.push_back(task); > > + > > + EXPECT_CALL(exec, launchTask(_, _)); > > + > > + Future<ExecutorInfo> executorInfo; > > + EXPECT_CALL(exec, registered(_, _, _, _)) > > + .Times(1) > > + .WillOnce(FutureArg<1>(&executorInfo)); > > + > > + driver.launchTasks(offers.get()[0].id(), tasks); > > + > > + AWAIT_READY(executorInfo); > > + > > + // At launchTasks, the label decorator hook inside should have been > > + // executed and we should see the labels now. > > + // Further, the environment decorator hook should also have been > > + // executed. In that hook, we create a temp file and set the path > > + // as the value of the environment variable. > > + // Here we verify that the environment variable is present and the > > + // file is present on the disk. > > + Option<string> path; > > + foreach (const Environment::Variable& variable, > > + executorInfo.get().command().environment().variables()) { > > + if (variable.name() == testEnvironmentVariableName) { > > + path = variable.value(); > > + break; > > + } > > + } > > + > > + EXPECT_SOME(path); > > + // The file must have been create by the environment decorator hook. > > + EXPECT_TRUE(os::isfile(path.get())); > > + > > + driver.stop(); > > + driver.join(); > > + > > + Shutdown(); // Must shutdown before 'containerizer' gets deallocated. > > + > > + // The removeExecutor hook in the test module deletes the temp file. > > + // Verify that the file is not present. > > + EXPECT_FALSE(os::isfile(path.get())); > > +} > > > > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/4e110d5d/src/tests/module.cpp > > ---------------------------------------------------------------------- > > diff --git a/src/tests/module.cpp b/src/tests/module.cpp > > index 6cec1cb..e6dbf94 100644 > > --- a/src/tests/module.cpp > > +++ b/src/tests/module.cpp > > @@ -93,6 +93,24 @@ static void addAuthenticationModules(Modules& modules) > > } > > > > > > +static void addHookModules(Modules& modules) > > +{ > > + const string libraryPath = path::join( > > + tests::flags.build_dir, > > + "src", > > + ".libs", > > + os::libraries::expandName("testhook")); > > + > > + // Now add our test hook module. > > + Modules::Library* library = modules.add_libraries(); > > + library->set_file(libraryPath); > > + > > + // To add a new module from this library, create a new ModuleID enum > > + // and tie it with a module name. > > + addModule(library, TestHook, "org_apache_mesos_TestHook"); > > +} > > + > > + > > Try<Nothing> tests::initModules(const Option<Modules>& modules) > > { > > // First get the user provided modules. > > @@ -107,6 +125,9 @@ Try<Nothing> tests::initModules(const > Option<Modules>& > > modules) > > // Add authentication modules from testauthentication library. > > addAuthenticationModules(mergedModules); > > > > + // Add hook modules from testhook library. > > + addHookModules(mergedModules); > > + > > return ModuleManager::load(mergedModules); > > } > > > > > > > > > http://git-wip-us.apache.org/repos/asf/mesos/blob/4e110d5d/src/tests/module.hpp > > ---------------------------------------------------------------------- > > diff --git a/src/tests/module.hpp b/src/tests/module.hpp > > index bc1a37d..21e8fc4 100644 > > --- a/src/tests/module.hpp > > +++ b/src/tests/module.hpp > > @@ -42,7 +42,8 @@ enum ModuleID > > TestMemIsolator, > > TestCpuIsolator, > > TestCRAMMD5Authenticatee, > > - TestCRAMMD5Authenticator > > + TestCRAMMD5Authenticator, > > + TestHook > > }; > > > > > > > > >
