mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gastón Kleiman <gas...@mesosphere.com>
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:
https://reviews.apache.org/r/55901/
-----------------------------------------------------------

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


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