Hey Vinod, I've closed https://issues.apache.org/jira/browse/MESOS-2227 as a duplicate of https://issues.apache.org/jira/browse/MESOS-2049 and the review request is available at: https://reviews.apache.org/r/29948.
Thanks, MPark On 15 January 2015 at 15:51, Niklas Nielsen <[email protected]> wrote: > 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 > > > }; > > > > > > > > > > > > > > >
