mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gastón Kleiman <>
Subject Review Request 55901: [WIP] Added support for command health checks to the default executor.
Date Wed, 25 Jan 2017 00:33:46 GMT

This is an automatically generated e-mail. To reply, visit:

Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, and Vinod Kone.

Bugs: MESOS-6280

Repository: mesos


[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.


  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 



Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It passes on Linux,
but not on macOS.


Gastón Kleiman

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message