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