Re: Review Request 33459: Fix containerizers to only recover containers they would launch.

2015-04-22 Thread Timothy Chen
/33257/ My patch also handles docker container tasks, since the docker containerizer uses the command executor in docker tasks, and without more handling we can't tell if it's from mesos or docker. - Timothy Chen On April 23, 2015, 12:33 a.m., Benjamin Hindman wrote

Re: Review Request 33647: Only log processes in the slave cgroup on recovery.

2015-04-28 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33647/#review81905 --- Ship it! Ship It! - Timothy Chen On April 28, 2015, 11:48 p.m

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-04-30 Thread Timothy Chen
src/slave/containerizer/docker.cpp f9fc89a src/slave/flags.hpp d3b1ce1 src/slave/flags.cpp d0932b0 src/tests/docker_containerizer_tests.cpp c9d66b3 Diff: https://reviews.apache.org/r/29889/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-02 Thread Timothy Chen
-mail. To reply, visit: https://reviews.apache.org/r/29889/#review82287 --- On May 1, 2015, 9:43 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-02 Thread Timothy Chen
check Thanks, Timothy Chen

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-01 Thread Timothy Chen
check Thanks, Timothy Chen

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-06 Thread Timothy Chen
to the scheduler. Timothy Chen wrote: Reason field sounds good, I think what you proposed makes sense, in docker containerizer at least we also need to make sure termination message is set correctly as currently it doesn't contain all the error information that we pass back

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-05 Thread Timothy Chen
to the scheduler. Timothy Chen wrote: Reason field sounds good, I think what you proposed makes sense, in docker containerizer at least we also need to make sure termination message is set correctly as currently it doesn't contain all the error information that we pass back

Re: Review Request 32982: Added reservation user guide.

2015-05-08 Thread Timothy Chen
reservation is the principal here, are you going to cover that? docs/reservation.md https://reviews.apache.org/r/32982/#comment133926 What happens if you try to unreserve a reservation that is being used right now? - Timothy Chen On May 8, 2015, 6:57 p.m., Michael Park wrote

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-08 Thread Timothy Chen
check Thanks, Timothy Chen

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
/33249/#comment134171 We don't enqueue destroy anymore, let's fix this comment. src/tests/slave_tests.cpp https://reviews.apache.org/r/33249/#comment134173 I don't really understand this comment? And if you simply pause the clock your await won't work right? - Timothy Chen On May 11

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
to comment. - Timothy Chen On May 11, 2015, 10:06 p.m., Jay Buffington wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249

Re: Review Request 30643: Optionally specify executor for mesos execute.

2015-05-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30643/#review82830 --- Are you still planning to merge this? - Timothy Chen On Feb. 4

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-07 Thread Timothy Chen
to destroy if none of the containerizers return true. - Timothy Chen On May 7, 2015, 4:17 p.m., Jay Buffington wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-12 Thread Timothy Chen
On May 11, 2015, 11:07 p.m., Timothy Chen wrote: Thanks Jay the changes looks reasonable to me, will wait for Jie to comment. Jay Buffington wrote: Great, thanks! When you commit can you add the three separate commits that are here: https://github.com/apache/mesos/compare/master

Re: Review Request 33249: Send statusUpdate to scheduler on containerizer launch failure

2015-05-11 Thread Timothy Chen
message from it? - Timothy Chen On May 11, 2015, 5:10 p.m., Jay Buffington wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33249

Re: Review Request 34128: Enable different IP/Port for external access.

2015-05-12 Thread Timothy Chen
overriding __address__ static is dangerous, since we use this variable in other places for other purposes, such as checking if we are communicating to a remote address, etc. I suggest we create a local address struct just for binding for listening to public traffic. - Timothy Chen

Re: Review Request 34136: Add ContainerImage protobuf.

2015-05-14 Thread Timothy Chen
and reintroduce new ones like we did the in past. - Timothy Chen On May 13, 2015, 12:47 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34136

Re: Review Request 34139: AppC image discovery.

2015-05-14 Thread Timothy Chen
one that has a configurable discvoery mechanism and local dir - Timothy Chen On May 13, 2015, 12:47 a.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34139

Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.

2015-05-15 Thread Timothy Chen
://reviews.apache.org/r/34310/#comment135147 What if the same set of resources contains both revocable and non-revocable resources? - Timothy Chen On May 16, 2015, 1:14 a.m., Ian Downes wrote: --- This is an automatically generated e

Re: Review Request 34319: Refactored os::getenv() to return an option and updated its callers in stout.

2015-05-16 Thread Timothy Chen
/stout/os.hpp https://reviews.apache.org/r/34319/#comment135177 Please fix the comment as we no longer have expected. - Timothy Chen On May 17, 2015, 4:51 a.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 34310: Use IDLE scheduling for revocable CPU in cgroups isolator.

2015-05-19 Thread Timothy Chen
On May 16, 2015, 4:46 a.m., Timothy Chen wrote: src/slave/containerizer/isolators/cgroups/cpushare.cpp, line 345 https://reviews.apache.org/r/34310/diff/1/?file=961963#file961963line345 What if the same set of resources contains both revocable and non-revocable resources? Ian

Re: Review Request 34317: Updated callers of os::getenv() in /src.

2015-05-20 Thread Timothy Chen
/#comment135872 optionally you can avoid the extra check by calling os::getenv(PATH).get() - Timothy Chen On May 17, 2015, 4:54 a.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 34319: Refactored os::getenv() to return an option and updated its callers in stout.

2015-05-20 Thread Timothy Chen
https://reviews.apache.org/r/34319/#comment135863 I don't think we neeed to change this. - Timothy Chen On May 17, 2015, 1:40 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 34319: Refactored os::getenv() to return an option and updated its callers in stout.

2015-05-20 Thread Timothy Chen
://reviews.apache.org/r/34319/#comment135871 Btw after looking at your other patches, I realize we don't really need hasenv anymore right? And I see you've been replacing hasenv with getenv, I think why not just remove hasenv all together? - Timothy Chen On May 17, 2015, 1:40 p.m., Greg Mann

Re: Review Request 34317: Updated callers of os::getenv() in /src.

2015-05-20 Thread Timothy Chen
://reviews.apache.org/r/34317/#comment135869 ditto src/examples/low_level_scheduler_libprocess.cpp https://reviews.apache.org/r/34317/#comment135868 ditto - Timothy Chen On May 17, 2015, 4:54 a.m., Greg Mann wrote

Re: Review Request 34317: Updated callers of os::getenv() in /src and removed calls to os::hasenv()

2015-06-03 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34317/#review86486 --- Ship it! Ship It! - Timothy Chen On May 25, 2015, 4:44 p.m

Re: Review Request 34318: Update callers of os::getenv() in libprocess.

2015-06-03 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34318/#review86487 --- Ship it! Ship It! - Timothy Chen On May 25, 2015, 4:36 p.m

Re: Review Request 34319: Refactored os::getenv() to return an option and removed os::hasenv()

2015-06-03 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34319/#review86485 --- Ship it! Ship It! - Timothy Chen On May 25, 2015, 4:43 p.m

Re: Review Request 34317: Updated callers of os::getenv() in /src and removed calls to os::hasenv()

2015-06-03 Thread Timothy Chen
the reviewboards? I'm getting merge conflicts when I Try to apply them. - Timothy Chen On May 25, 2015, 4:44 p.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34317

Review Request 35182: Fix handling large docker image info on inspect.

2015-06-06 Thread Timothy Chen
71383294c6234d08b2156565b170ada329b8e30f Diff: https://reviews.apache.org/r/35182/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 34534: Reflected in documentation that isolators are only relevant for Mesos Containerizer.

2015-06-03 Thread Timothy Chen
instead of mechanisms. - Timothy Chen On May 21, 2015, 8:45 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34534

Review Request 35799: Support mounting relative paths with docker.

2015-06-23 Thread Timothy Chen
acf1e3c8f455e8b40e4b3cb8748fcfeacbab142c Diff: https://reviews.apache.org/r/35799/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 34137: Add support for container image provisioners.

2015-06-24 Thread Timothy Chen
://reviews.apache.org/r/34137/#comment141822 We actually don't need the underscores anymore right? According to the updated style? - Timothy Chen On June 22, 2015, 4:44 p.m., Ian Downes wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 34136: Add ContainerImage protobuf.

2015-06-24 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34136/#review89226 --- Ship it! Ship It! - Timothy Chen On June 22, 2015, 4:42 p.m

Re: Review Request 35799: Support mounting relative paths with docker.

2015-06-24 Thread Timothy Chen
/#review89042 --- On June 23, 2015, 7:18 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35799

Re: Review Request 35799: Support mounting relative paths with docker.

2015-06-24 Thread Timothy Chen
/35799/#review89147 --- On June 23, 2015, 7:18 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35799

Re: Review Request 35799: Support mounting relative paths with docker.

2015-06-26 Thread Timothy Chen
/docker_tests.cpp acf1e3c8f455e8b40e4b3cb8748fcfeacbab142c Diff: https://reviews.apache.org/r/35799/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

2015-06-16 Thread Timothy Chen
/35330/#comment140544 Forgot it's a vector! Keep thinking it's a list. Then how about pop_back() and start iterating from the back? Erasing from the front causes moving elements and I tihnk we don't really care about the order here. - Timothy Chen On June 11, 2015, 9:39 p.m., Lily Chen

Re: Review Request 35330: Capped number of parallel inspect instances on a docker ps call.

2015-06-16 Thread Timothy Chen
/ --- (Updated June 11, 2015, 9:39 p.m.) Review request for mesos and Timothy Chen. Bugs: MESOS-2797 https://issues.apache.org/jira/browse/MESOS-2797 Repository: mesos Description --- Capped number of parallel inspect instances on a docker ps call. Diffs - src

Re: Review Request 35574: Added missing libraries to mesos-docker-executor

2015-06-17 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35574/#review88256 --- Ship it! Ship It! - Timothy Chen On June 17, 2015, 6:16 p.m

Re: Review Request 35438: Fixed fetcher cache test race for resource offers when starting tasks and changed corresponding CHECK to EXPECT.

2015-06-15 Thread Timothy Chen
/35438/#comment140309 Does this mean when the default changes we need to change this value too? Also what's the rationale making it a Error/Try? Is this because a timeout can be a valid case here? - Timothy Chen On June 14, 2015, 1:54 p.m., Bernd Mathiske wrote

Re: Review Request 34534: Reflected in documentation that isolators are only relevant for Mesos Containerizer.

2015-06-03 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34534/#review86440 --- Ship it! Ship It! - Timothy Chen On June 3, 2015, 5:23 p.m

Re: Review Request 34654: Send docker inspect output with TaskStatus data.

2015-05-28 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34654/#review85390 --- On May 26, 2015, 6:14 p.m., Timothy Chen wrote: --- This is an automatically generated e

Re: Review Request 34134: Add container rootfs to Isolator::prepare().

2015-05-28 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34134/#review85564 --- Ship it! Ship It! - Timothy Chen On May 13, 2015, 12:44 a.m

Re: Review Request 36071: Add flow diagram for docker containerizer.

2015-07-01 Thread Timothy Chen
e-mail. To reply, visit: https://reviews.apache.org/r/36071/#review90052 --- On July 1, 2015, 1:07 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 35799: Support mounting relative paths with docker.

2015-06-30 Thread Timothy Chen
/docker_tests.cpp acf1e3c8f455e8b40e4b3cb8748fcfeacbab142c Diff: https://reviews.apache.org/r/35799/diff/ Testing --- make check Thanks, Timothy Chen

Review Request 36071: Add flow diagram for docker containerizer.

2015-06-30 Thread Timothy Chen
--- make Thanks, Timothy Chen

Re: Review Request 36106: cgroups: added cpuacct subsystem

2015-07-01 Thread Timothy Chen
On July 1, 2015, 9:46 p.m., Timothy Chen wrote: src/linux/cgroups.cpp, line 2060 https://reviews.apache.org/r/36106/diff/1/?file=997647#file997647line2060 Why add trailing underscore? Jojy Varghese wrote: As a member variable(is accepted according to mesos coding style

Re: Review Request 36106: cgroups: added cpuacct subsystem

2015-07-01 Thread Timothy Chen
On July 1, 2015, 9:46 p.m., Timothy Chen wrote: src/linux/cgroups.cpp, line 2014 https://reviews.apache.org/r/36106/diff/1/?file=997647#file997647line2014 I don't think we usually define a inline function like this anywhere else, so curious to see what others think

Re: Review Request 34427: AppC provisioner backend using bind mounts.

2015-07-02 Thread Timothy Chen
/bind_backend.hpp (line 31) https://reviews.apache.org/r/34427/#comment143207 Even with overlayfs is available, I don't think users has a choice to use it yet. Perhaps we should give this advice when we have overlayfs backend. - Timothy Chen On June 22, 2015, 4:53 p.m., Ian Downes wrote

Re: Review Request 34142: AppC provisioner.

2015-07-02 Thread Timothy Chen
/34142/#comment143208 I believe we discussed this, but different acVersion will most likely have different schema. Unless we intend to only support one version (or anything that's backward compatible), we should version the protobuf too. - Timothy Chen On June 22, 2015, 4:57 p.m

Review Request 36173: Update attributes doc to reflect current supported attributes types.

2015-07-03 Thread Timothy Chen
Toenshoff. Repository: mesos Description --- Update attributes doc to reflect current supported attributes types. Diffs - docs/attributes-resources.md 0ae8b5908fe0b3a3499e6d813afbb328a13bdcde Diff: https://reviews.apache.org/r/36173/diff/ Testing --- make Thanks, Timothy

Re: Review Request 36181: Port CFS support to Docker Containerizer

2015-07-04 Thread Timothy Chen
://reviews.apache.org/r/36181/#comment143442 These flags are supported only after a certain docker version right? I think we need to log and disable this when the user is using a older version that doens't support these flags. - Timothy Chen On July 4, 2015, 4:55 p.m., haosdent huang wrote

Re: Review Request 36179: Fix duplicate -e environment variables option in Docker::run.

2015-07-04 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36179/#review90393 --- Ship it! Ship It! - Timothy Chen On July 4, 2015, noon

Re: Review Request 36128: Change docker version parsing

2015-07-02 Thread Timothy Chen
to support both 1.8 and = 1.8 and do different parsing since not all users will be upgrading. So I think if there is a way to detect this or have one way to parse both that will good. Either way we should put in the comments what does the output look like. - Timothy Chen On July

Re: Review Request 36128: Change docker version parsing

2015-07-02 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36128/#review90257 --- Ship it! Ship It! - Timothy Chen On July 2, 2015, 8:58 a.m

Re: Review Request 36128: Change docker version parsing

2015-07-02 Thread Timothy Chen
On July 2, 2015, 5:47 p.m., Timothy Chen wrote: src/docker/docker.cpp, line 189 https://reviews.apache.org/r/36128/diff/1/?file=998179#file998179line189 It's a bit harder to understand how this works. Looks like the new output is: Client: Version: 1.8.0

Re: Review Request 34142: AppC provisioner.

2015-07-02 Thread Timothy Chen
On July 2, 2015, 8:48 a.m., Timothy Chen wrote: include/mesos/mesos.proto, line 1300 https://reviews.apache.org/r/34142/diff/2/?file=989783#file989783line1300 I believe we discussed this, but different acVersion will most likely have different schema. Unless we

Review Request 36216: Only run netcat tests when nc is available.

2015-07-06 Thread Timothy Chen
/environment.cpp 3726e5d95b9388f0881184e1b4874e1eb2e7f531 src/tests/port_mapping_tests.cpp ac49cdfdcf6baf00ac2907e193c683c8b6c83ffb Diff: https://reviews.apache.org/r/36216/diff/ Testing --- make check Thanks, Timothy Chen

Review Request 36214: Fix running docker executor tests.

2015-07-06 Thread Timothy Chen
is in the PATH. Diffs - src/tests/docker_containerizer_tests.cpp d4ccb0b30fe27980846d913e292d2e18fd3d1055 Diff: https://reviews.apache.org/r/36214/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-07-06 Thread Timothy Chen
that a docker container can be launched, which is in launchExecutorProcess. Can you add the hook call in that path as well? - Timothy Chen On July 5, 2015, 9:14 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply

Re: Review Request 36071: Add flow diagram for docker containerizer.

2015-07-06 Thread Timothy Chen
://reviews.apache.org/r/36071/diff/ Testing --- make File Attachments (updated) docker_containerizer_flow.png https://reviews.apache.org/media/uploaded/files/2015/07/06/d888a674-17d8-4faf-ab03-f1892537a6e5__docker_containerizer_flow.png Thanks, Timothy Chen

Re: Review Request 36216: Only run netcat tests when nc is available.

2015-07-06 Thread Timothy Chen
--- make check Thanks, Timothy Chen

Re: Review Request 36214: Fix running docker executor tests.

2015-07-06 Thread Timothy Chen
Thanks, Timothy Chen

Re: Review Request 36106: cgroups: added cpuacct subsystem

2015-07-02 Thread Timothy Chen
On July 1, 2015, 9:46 p.m., Timothy Chen wrote: src/linux/cgroups.cpp, line 2014 https://reviews.apache.org/r/36106/diff/1/?file=997647#file997647line2014 I don't think we usually define a inline function like this anywhere else, so curious to see what others think

Re: Review Request 36173: Update attributes doc to reflect current supported attributes types.

2015-07-05 Thread Timothy Chen
., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36173/ --- (Updated July 3, 2015, 9:30 p.m.) Review

Re: Review Request 36173: Update attributes doc to reflect current supported attributes types.

2015-07-06 Thread Timothy Chen
-mail. To reply, visit: https://reviews.apache.org/r/36173/#review90437 --- On July 3, 2015, 9:30 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-22 Thread Timothy Chen
5520c58 Diff: https://reviews.apache.org/r/29889/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-24 Thread Timothy Chen
to use the promise associate to handle both cases. I'll leave a comment. - Timothy Chen On May 23, 2015, 6:14 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-24 Thread Timothy Chen
5520c58 Diff: https://reviews.apache.org/r/29889/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-24 Thread Timothy Chen
On May 24, 2015, 6:07 p.m., Timothy Chen wrote: src/slave/containerizer/docker.cpp, lines 845-848 https://reviews.apache.org/r/29889/diff/13/?file=970849#file970849line845 Ah, I realize why now I want a promise. So the semantics I want to have is that I want to able

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-24 Thread Timothy Chen
, 2015, 6:26 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889/ --- (Updated May 24, 2015, 6:26 p.m

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-24 Thread Timothy Chen
5520c58 Diff: https://reviews.apache.org/r/29889/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 29889: Recover Docker containers when mesos slave is in a container

2015-05-23 Thread Timothy Chen
5520c58 Diff: https://reviews.apache.org/r/29889/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 34427: AppC provisioner backend using bind mounts.

2015-05-21 Thread Timothy Chen
to avoid process:: and std:: everywhere, just a nit. src/slave/containerizer/provisioners/appc/bind_backend.hpp https://reviews.apache.org/r/34427/#comment136122 Should we make rootfs a constant somewhere? - Timothy Chen On May 19, 2015, 6:46 p.m., Ian Downes wrote

Re: Review Request 34140: AppC image store

2015-05-26 Thread Timothy Chen
/#comment134949 Check exists first? src/slave/containerizer/provisioners/appc/store.cpp https://reviews.apache.org/r/34140/#comment134950 Why not use os::rmdir? - Timothy Chen On May 26, 2015, 6:25 p.m., Ian Downes wrote

Re: Review Request 34654: Send docker inspect output with TaskStatus data.

2015-05-26 Thread Timothy Chen
/docker_containerizer_tests.cpp 7524803 Diff: https://reviews.apache.org/r/34654/diff/ Testing --- make check Thanks, Timothy Chen

Re: Review Request 34654: Send docker inspect output with TaskStatus data.

2015-05-26 Thread Timothy Chen
075c6b5 src/tests/docker_containerizer_tests.cpp 7524803 Diff: https://reviews.apache.org/r/34654/diff/ Testing (updated) --- make check Thanks, Timothy Chen

Re: Review Request 37236: Added the linux filesystem isolator.

2015-08-12 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37236/#review95192 --- Ship it! Ship It! - Timothy Chen On Aug. 12, 2015, 6:54 p.m

Re: Review Request 37237: Added a few MesosContainerizer filesystem tests to test the linux filesystem isolator.

2015-08-12 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37237/#review95194 --- Ship it! Ship It! - Timothy Chen On Aug. 8, 2015, 12:32 a.m

Re: Review Request 37114: MESOS-3187, support docker host command line option

2015-08-12 Thread Timothy Chen
On Aug. 12, 2015, 6:13 p.m., Timothy Chen wrote: src/slave/flags.cpp, line 391 https://reviews.apache.org/r/37114/diff/2/?file=1034775#file1034775line391 This will cause problem in the docker containerizer since we also try to mount in the socket when we launch executors

Re: Review Request 37549: Add jenkins script to run docker tests.

2015-08-17 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37549/#review95648 --- On Aug. 17, 2015, 8:16 p.m., Timothy Chen wrote

Re: Review Request 37505: Fix broken health check in docker executor.

2015-08-17 Thread Timothy Chen
the healthcheck outside of the container doesn't make much sense to me. I think we should try to run it inside of the container (docker exec), but since docker exec is introduced until few later versions we also need to check to make sure the right version is running on the slave. - Timothy Chen

Re: Review Request 37426: MESOS-3251 : Fixing host field of request header.

2015-08-17 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37426/#review95652 --- Ship it! Ship It! - Timothy Chen On Aug. 17, 2015, 9:56 p.m

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-08-16 Thread Timothy Chen
, once you fix this I'll run this locally and this should be ready to go. - Timothy Chen On Aug. 16, 2015, 6:05 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-08-16 Thread Timothy Chen
. - Timothy Chen On Aug. 16, 2015, 6:05 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36185/ --- (Updated

Re: Review Request 37493: Detect gflags when present and link when building Python module

2015-08-15 Thread Timothy Chen
])]) Otherwise this looks good to me. - Timothy Chen On Aug. 15, 2015, 12:28 a.m., Greg Mann wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 37493: Detect gflags when present and link when building Python module

2015-08-19 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37493/#review95901 --- Ship it! Ship It! - Timothy Chen On Aug. 19, 2015, 12:33 a.m

Re: Review Request 36185: Create pre-launch hook before a docker container launches in slave.

2015-08-19 Thread Timothy Chen
/36185/#comment151104 Btw we no longer need a extra space between as we upgraded our gcc version. src/tests/hook_tests.cpp (line 539) https://reviews.apache.org/r/36185/#comment151105 Same here - Timothy Chen On Aug. 16, 2015, 7:15 a.m., haosdent huang wrote

Re: Review Request 37669: Ignore overflow components in version parsing.

2015-08-21 Thread Timothy Chen
17) https://reviews.apache.org/r/37669/#comment151327 We put this below the stdlib includes. - Timothy Chen On Aug. 21, 2015, 2:53 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 37669: Ignore overflow components in version parsing.

2015-08-22 Thread Timothy Chen
60) https://reviews.apache.org/r/37669/#comment151328 ignored - Timothy Chen On Aug. 22, 2015, 5:56 a.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37669

Re: Review Request 37657: Document iptable rule need added when use docker bridge network mode.

2015-08-20 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37657/#review95994 --- Ship it! Ship It! - Timothy Chen On Aug. 20, 2015, 5:30 p.m

Re: Review Request 37198: Add Docker image provisioner and copy backend.

2015-08-19 Thread Timothy Chen
) https://reviews.apache.org/r/37198/#comment150978 Seems like a useless comment :) src/slave/containerizer/provisioners/docker.cpp (line 234) https://reviews.apache.org/r/37198/#comment150979 Unable to join discovery local path: + error() - Timothy Chen On Aug. 17, 2015, 9:11 p.m

Re: Review Request 37197: Docker image store.

2015-08-19 Thread Timothy Chen
) https://reviews.apache.org/r/37197/#comment150974 Btw this is actually renamed to be local store later right? How about just remove the impl completely in this review so later review we introduce the local store? - Timothy Chen On Aug. 16, 2015, 8:31 a.m., Lily Chen wrote

Re: Review Request 37382: Introduced provisioner Backend interface.

2015-08-19 Thread Timothy Chen
, 5:51 p.m.) Review request for mesos, Lily Chen, Ian Downes, Jie Yu, and Timothy Chen. Bugs: MESOS-2968 https://issues.apache.org/jira/browse/MESOS-2968 Repository: mesos Description --- The Backend interface is made generic so it can be used by different

Review Request 37549: Add jenkins script to run docker tests.

2015-08-17 Thread Timothy Chen
script to run docker tests. Diffs - support/docker_tests_jenkins_build.sh PRE-CREATION Diff: https://reviews.apache.org/r/37549/diff/ Testing --- ./support/docker_jenkins_build_test.sh in ubuntu 14.04 Thanks, Timothy Chen

Re: Review Request 37426: MESOS-3251 : Fixing host field of request header.

2015-08-17 Thread Timothy Chen
://reviews.apache.org/r/37426/#comment150741 Let's add the port to make sure we're adhering to the HTTP 1.1 standard - Timothy Chen On Aug. 13, 2015, 12:46 a.m., Jojy Varghese wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 36620: WIP Added Non-Freezeer Task Killer.

2015-08-17 Thread Timothy Chen
? - Timothy Chen On Aug. 13, 2015, 1:52 p.m., Joerg Schad wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36620/ --- (Updated

Re: Review Request 37199: Added store interface and moved store implementation to LocalStore subclass.

2015-08-20 Thread Timothy Chen
message. - Timothy Chen On Aug. 17, 2015, 9:12 p.m., Lily Chen wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37199

  1   2   3   4   5   6   7   8   >