----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55901/#review162922 -----------------------------------------------------------
Patch looks great! Reviews applied: [55899, 55900, 55901] Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh - Mesos Reviewbot On Jan. 25, 2017, 12:33 a.m., Gastón Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55901/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2017, 12:33 a.m.) > > > Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent > huang, and Vinod Kone. > > > Bugs: MESOS-6280 > https://issues.apache.org/jira/browse/MESOS-6280 > > > Repository: mesos > > > Description > ------- > > [WIP] Added support for command health checks to the default executor. > > This is still a work in progress, but I am posting it in order to get some > feedback on the general approach. > > Open questions/issues: > > - The code that handles communication with the Agent is on the > `DefaultExecutor`, so the health checker calls `evolve` when serializing the > API calls. Should it use the v1 protos instead, and skip the `evolve` call? > - CMD health checks for command tasks inherit the env from the executor, > but allow overriding env variables. I don't think that inheriting the > executor env makes sense for cmd health checks launched by the > `DefaultExecutor` - each task can have its own env, so the health check > command should inherit the task env and merge it with the one in the check's > `CommandInfo`. > I added some code that takes the task env from `TaskInfo` and merges it > check's env, but I don't think that this is an ideal approach, since it will > miss env variables set/modified by containerizers/hooks/isolators. Do you > think that it would make sense for all Debug Containers to inherit the > environment of the parent container? > - There's a TODO for stopping/resuming health checking when the > `DefaultExecutor` is disconnected from the agent. After talking to AlexR, I > believe that we should do this for all types of checks, not just for `CMD`. > - The test that I introduced passes on Linux, but not on macOS, I still > have to do some more debugging. > > > Diffs > ----- > > src/checks/health_checker.hpp 6e558f2061a9e31157c47d31cb64b3a8568aace3 > src/checks/health_checker.cpp 50aa2858e807b27bbab58a3618f5200cfe4eca9e > src/launcher/default_executor.cpp a03794934adb93868734f8cf00b337a1bff9b5ab > src/tests/health_check_tests.cpp debbd3c09b7555145aaf3f62a24d795d1423a269 > > Diff: https://reviews.apache.org/r/55901/diff/ > > > Testing > ------- > > Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It > passes on Linux, but not on macOS. > > > Thanks, > > Gastón Kleiman > >
