date:20150414

Re: Review Request 31985: Mesos container ID available to the executor through an environment variable.

2015-04-14 Thread Alexander Rojas

On March 14, 2015, 1:49 a.m., Timothy Chen wrote:
The change looks good, but I'm not sure how exposing the container id is
the right thing to do overall yet. Container id as I know of is meant to be
a internal id that is used only in mesos, and I believe the whole
motivation was for users to be able to guess the docker container name from
the container id. However, container id - docker container name mapping
might change since we manage that, and I'm progress changing it to include
the slave id.

I personally think we should think about exposing container specific
information.

Do you know if there is any other use case to know the container id?

Alexander Rojas wrote:
1. I am not aware of any other use case. However I asked benh about this
and he mentioned it was ok to have this as extra info on the update, as long
as it was no docker specific data.
2. After asking some questions to the original reporter, the idea was
more to be able to group tasks assigned to the same container, but not as a
way to extract the specific container.
3. I am not sure, then, what would be considered mesos private info and
info which can be shared. For example, why can the framework id and the
executor id be shared but no the container id?

Vinod Kone wrote:
group tasks assigned to the same container .. What does this mean?
IIUC, our docker containerizer only supports single task containers.

regarding why framework id and executor id are exposed: framework id is
needed by frameworks to reregister with master. executor id (and task id) is
generated by the framework and not mesos.

Alexander Rojas wrote:
So I guess then, I will discard this patch and set the issue to won't fix?

Vinod Kone wrote:
I think we should try to understand the root of the issue that the
reporter is having before jumping onto a specific implementation.

Alexander Rojas wrote:
Well, I felt like I had it clear but apparently not. Can you please ask
the questions in the Jira entry vinod?

I was also wondering, As mentioned above, we want to keep the
`containerId` private within mesos, but patch
[32426](https://reviews.apache.org/r/32426) effectively makes it public.

Alexander Rojas wrote:
I got it now… it is still quite private. Forget my question on the second
paragraph.

Alexander Rojas wrote:
One more thing Vinod, you wrote one container per task, however if you
check the code (Slave.cpp at `Executor*
Framework::launchExecutor(ExecutorInfo, TaskInfo)`) what we have is one
container per executor.

Timothy Chen wrote:
Docker containerizer supports multiple tasks only if the container is a
custom executor, so you get grouping naturally through that.
From what I understand most people wants the container ID so they can get
docker specific information like name, IP, etc. I think by sending this all
back through the docker executor we shouldn't need this anymore.

Hey guys, I asked the reporter of the issue the reason why he needs it. He
wrote a full explanation on the [JIRA
Issue](https://issues.apache.org/jira/browse/MESOS-2191?focusedCommentId=14387724page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14387724),
so if you can read that and then decide if it is worth commiting this patch or
not.

- Alexander

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31985/#review76470
---

On March 20, 2015, 10:27 a.m., Alexander Rojas wrote:

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31985/
---

(Updated March 20, 2015, 10:27 a.m.)

Review request for mesos, Bernd Mathiske, Isabel Jimenez, Joerg Schad, Till
Toenshoff, and Vinod Kone.

Bugs: MESOS-2191
https://issues.apache.org/jira/browse/MESOS-2191

Repository: mesos

Description
---

When the executor is created, the container ID where it runs is made
accesible through an environment variable.

Diffs
-

src/exec/exec.cpp d678f0682d803b0b080c3a6c296067ac9ab5dbf8
src/slave/containerizer/containerizer.hpp
129e60f20835f5d151701e934330b81825887af1
src/slave/containerizer/containerizer.cpp
4d66e767de1f877cb66b37826ba7c9d00639a7c0
src/slave/containerizer/docker.hpp b7bf54ac65d6c61622e485ac253513eaac2e4f88
src/slave/containerizer/docker.cpp 5f4b4ce49a9523e4743e5c79da4050e6f9e29ed7
src/slave/containerizer/external_containerizer.cpp
42c67f548caf7bddbe131e0dfa7d74227d8c2593
src/slave/containerizer/mesos/containerizer.cpp
fbd1c0a0e5f4f227adb022f0baaa6d2c7e3ad748
src/tests/containerizer.cpp

Re: Review Request 31985: Mesos container ID available to the executor through an environment variable.

2015-04-14 Thread Timothy Chen

On March 14, 2015, 12:49 a.m., Timothy Chen wrote:
The change looks good, but I'm not sure how exposing the container id is
the right thing to do overall yet. Container id as I know of is meant to be
a internal id that is used only in mesos, and I believe the whole
motivation was for users to be able to guess the docker container name from
the container id. However, container id - docker container name mapping
might change since we manage that, and I'm progress changing it to include
the slave id.