Re: Review Request 43561: Improve Ranges parsing to handle single values.

2016-02-14 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43561]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 14, 2016, 6:38 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated Feb. 14, 2016, 6:38 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp c64407bc97ad858300f4661d616e0480920fc541 
>   src/tests/resources_tests.cpp 96864c3945729fe33be8b243b9c826fb12e90eff 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Timothy Chen


> On Feb. 8, 2016, 8:44 p.m., Jie Yu wrote:
> > I realized a tricky part in the recovery path while dealing with potential 
> > conflicts with MesosContainerizer (LinuxFilesystemIsolator). For instance, 
> > will the LinuxFilesystemIsolator try to umount some persistent volume 
> > mounts that belongs to docker containerizer because it'll treat them as 
> > orphans. After checking the code, we're lucky here because of the way 
> > LinxuFilesystemIsolator detects orphans in LinuxFilesystemIsolator. There 
> > will be a mount from 'directory' (e.g., /var/lib/mesos/...) -> 'sandbox' 
> > (e.g., rootfs/mnt/mesos/sandbox). If we cannot find such a mount in 
> > LinuxFilesystemIsolator, it'll not try to umount volumes under 'directory'.
> > 
> > However, I don't see any cleanup code during recovery in this patch. Do we 
> > need to cleanup volume mounts for orphan docker containers?

Yes you're right, I've added clean up code in recovery now.


- Timothy


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


On Feb. 9, 2016, 2:32 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Feb. 9, 2016, 2:32 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Timothy Chen

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

(Updated Feb. 14, 2016, 8:38 a.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Fixed persistent volumes with docker tasks.


Diffs (updated)
-

  src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/tests/containerizer/docker_containerizer_tests.cpp 
645bdcf095145097d8b8c65d592c787417883145 

Diff: https://reviews.apache.org/r/43015/diff/


Testing
---

make check


Thanks,

Timothy Chen



Re: Review Request 43293: Ignored invalid env vars.

2016-02-14 Thread Timothy Chen

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




src/tests/containerizer/docker_tests.cpp (lines 506 - 507)


Is this even a valid docker inspect output?
In what situations will we get this?


- Timothy Chen


On Feb. 7, 2016, 9:40 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated Feb. 7, 2016, 9:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0207 10:30:01.894125 2034615040 process.cpp:2489] Spawned process 
> files@192.168.0.100:49551
> I0207 10:30:01.894136 211529728 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.894163968+00:00
> I0207 10:30:01.894317 210456576 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.894332928+00:00
> I0207 10:30:01.898862 2034615040 docker.cpp:397] Skipping duplicate 
> environment variable 'JAVA_VERSION'
> I0207 10:30:01.898892 2034615040 docker.cpp:390] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0207 10:30:01.899473 20992 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.0.100:49551 at 2016-02-07 
> 02:30:01.899522048+00:00
> I0207 10:30:01.899983 213139456 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.9+00:00
> I0207 10:30:01.900032 213139456 process.cpp:2604] Cleaning up 
> files@192.168.0.100:49551
> I0207 10:30:01.900244 211529728 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.900256000+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (8 ms)
> [--] 1 test from DockerImageTest (8 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 42691: Fix GMock warning in MasterTest.OrphanTasks.

2016-02-14 Thread Timothy Chen

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



Sorry for the late review, I can't apply the review at this moment. Can you 
rebase?

- Timothy Chen


On Jan. 24, 2016, 3:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42691/
> ---
> 
> (Updated Jan. 24, 2016, 3:45 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4484
> https://issues.apache.org/jira/browse/MESOS-4484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix GMock warning in MasterTest.OrphanTasks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp cd426fda633c2d665b9533271610863062b1ed4e 
> 
> Diff: https://reviews.apache.org/r/42691/diff/
> 
> 
> Testing
> ---
> 
> # warning didn't appear now.
> sudo ./bin/mesos-tests.sh --gtest_filter="MasterTest.OrphanTasks"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 42691: Fix GMock warning in MasterTest.OrphanTasks.

2016-02-14 Thread haosdent huang


> On Feb. 14, 2016, 8:44 a.m., Timothy Chen wrote:
> > Sorry for the late review, I can't apply the review at this moment. Can you 
> > rebase?

I saw current code have already added this. Let me discard this patch.


- haosdent


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


On Jan. 24, 2016, 3:45 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42691/
> ---
> 
> (Updated Jan. 24, 2016, 3:45 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4484
> https://issues.apache.org/jira/browse/MESOS-4484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix GMock warning in MasterTest.OrphanTasks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp cd426fda633c2d665b9533271610863062b1ed4e 
> 
> Diff: https://reviews.apache.org/r/42691/diff/
> 
> 
> Testing
> ---
> 
> # warning didn't appear now.
> sudo ./bin/mesos-tests.sh --gtest_filter="MasterTest.OrphanTasks"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43015]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 14, 2016, 8:38 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated Feb. 14, 2016, 8:38 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 43284: Wait for perf statistics processes exit.

2016-02-14 Thread haosdent huang


> On Feb. 8, 2016, 10:47 p.m., Ian Downes wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp, lines 
> > 132-134
> > 
> >
> > I think you should discard the future and let it do the correct thing 
> > -- kill and reap the perf process -- rather than do a blocking await. The 
> > perf sample interval is configurable and typical values are 10's of seconds.
> > 
> > Note: I'm not sure if the correct behavior is implemented...
> 
> Ian Downes wrote:
> Actually, it looks like it might:
> 
> ```
> virtual void initialize()
> {
>   // Stop when no one cares.
>   promise.future().onDiscard(lambda::bind(
>   static_cast(terminate), self(), 
> true));
> 
>   execute();
> }
> 
> virtual void finalize()
> {
>   // Kill the perf process (if it's still running) by sending
>   // SIGTERM to the signal handler which will then SIGKILL the
>   // perf process group created by setupChild.
>   if (perf.isSome() && perf->status().isPending()) {
> kill(perf->pid(), SIGTERM);
>   }
> 
>   promise.discard();
> }
> ```

I try discard that before.
```
sampleFuture.get().discard();
sampleFuture.get().await();
```
But failed. It is because in `collect()`

```
// Stop this nonsense if nobody cares.
promise->future().onDiscard(defer(this, &CollectProcess::discarded));
```

Every future discarded is scheduled to run after 
CgroupsPerfEventIsolatorProcess finalize.


- haosdent


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


On Feb. 14, 2016, 7:59 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43284/
> ---
> 
> (Updated Feb. 14, 2016, 7:59 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jan Schlicht, and Paul Brett.
> 
> 
> Bugs: MESOS-4655
> https://issues.apache.org/jira/browse/MESOS-4655
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Wait for perf statistics processes exit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
> 65e731886b9e5cac07ae3ad6398faf8f50de5650 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 5ef4ae5c468580352cd16e7716b9ca4c0acde659 
> 
> Diff: https://reviews.apache.org/r/43284/diff/
> 
> 
> Testing
> ---
> 
> Without this patch, when running 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="PerfEventIsolatorTest.ROOT_CGROUPS_Sample" --verbose
> ```
> , would got this error
> ```
> [--] Global test environment tear-down
> ../../src/tests/environment.cpp:732: Failure
> Failed
> Tests completed with child processes remaining:
> -+- 16501 /home/haosdent/mesos/build/src/.libs/lt-mesos-tests 
> --gtest_filter=PerfEventIsolatorTest.ROOT_CGROUPS_Sample --verbose
>  |-+- 16580 /home/haosdent/mesos/build/src/.libs/lt-mesos-tests 
> --gtest_filter=PerfEventIsolatorTest.ROOT_CGROUPS_Sample --verbose
>  | -+- 16582 perf stat --all-cpus --field-separator , --log-fd 1 --event 
> cycles --cgroup mesos/239d30bb-f7a1-413b-9d99-0914149d5899 --event task-clock 
> --cgroup mesos/239d30bb-f7a1-413b-9d99-0914149d5899 -- sleep 0.25
>  |   --- 16584 sleep 0.25
>  --- 16581 ()
> [==] 1 test from 1 test case ran. (4095 ms total)
> ```
> 
> This also fix similar error in 
> `MesosContainerizerSlaveRecoveryTest.CGROUPS_ROOT_PerfRollForward` and 
> `CgroupsAnyHierarchyWithPerfEventTest.ROOT_CGROUPS_Perf`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>



Re: Review Request 41597: Extending allocator interface to support dynamic weights.

2016-02-14 Thread Yongqiao Wang

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

(Updated Feb. 14, 2016, 11:37 a.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Resoved conflict.


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


Repository: mesos


Description
---

Extending allocator interface to support dynamic weights.


Diffs (updated)
-

  include/mesos/master/allocator.hpp e163669c9c4e4c98572968f18987704b60722a79 
  include/mesos/mesos.proto 0bd5abadb5abe052161963ca995c396f1ed832f2 
  include/mesos/v1/mesos.proto 38e04cb19e303d1c71d2afad6ea73137aaa7403a 
  src/master/allocator/mesos/allocator.hpp 
581eaad376e7b2febe0b6359014617b935a677a3 
  src/master/allocator/mesos/hierarchical.hpp 
20d7ceb1a75ea5cb9efb1fc7fb085265b32864fe 
  src/master/allocator/mesos/hierarchical.cpp 
a9d2c23162892e0f97d89a076d2311091d91 
  src/master/allocator/sorter/drf/sorter.hpp 
4669149b81de39b4bb921ef7cd6787aa583f6e40 
  src/master/allocator/sorter/drf/sorter.cpp 
18797e42a9c2bd20392020237cfae600a5ffe12c 
  src/master/allocator/sorter/sorter.hpp 
a0a779b81f6d048271f15256b38ff907ae144b83 
  src/tests/allocator.hpp 206e9ac3a83038a691f7929bdd627042b0f363b0 

Diff: https://reviews.apache.org/r/41597/diff/


Testing
---

Make & Make check successfully!

Test case: https://reviews.apache.org/r/41672/


Thanks,

Yongqiao Wang



Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

2016-02-14 Thread Yongqiao Wang


> On Feb. 9, 2016, 7:07 a.m., Adam B wrote:
> > src/master/master.cpp, lines 1560-1561
> > 
> >
> > Can you explain more clearly why this needs to be done? Is it because 
> > we already initialized the allocator with the weights from --weights?

Yes, when master recovered the weight infos from registry, allocator has been 
initialized with the weights specified by --weights flag, so we need to do this 
update.


- Yongqiao


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


On Jan. 25, 2016, 3:36 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41681/
> ---
> 
> (Updated Jan. 25, 2016, 3:36 p.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4214
> https://issues.apache.org/jira/browse/MESOS-4214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduce HTTP endpoint /weights for updating weight.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.hpp 
> 5ee3c7afadd131802c93febbb6b4dbad069c2d81 
>   include/mesos/authorizer/authorizer.proto 
> 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 8657a869f931aa7482fbb09f2c6df95b6a8c50c6 
>   src/authorizer/local/authorizer.hpp 
> c7321c276d566eca6a91f45c546468bea1b0da15 
>   src/authorizer/local/authorizer.cpp 
> 9557bbdf68ff182c4538bbf70cee576d717abc05 
>   src/master/http.cpp 12c1fe5a514903f657911302e8770e9b245fdbb7 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
>   src/master/weights_handler.cpp PRE-CREATION 
>   src/tests/mesos.hpp 5a737a6490060b0194db097990b327c9921221f4 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/41681/diff/
> 
> 
> Testing
> ---
> 
> Make & Make check successfully!
> 
> $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
> --weights="role1=4.2,role2=3.1" --authenticate_http 
> --credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
> "roles": [
> {
> "frameworks": [ ], 
> "name": "*", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 1
> }, 
> {
> "frameworks": [ ], 
> "name": "role1", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 4.2
> }, 
> {
> "frameworks": [ ], 
> "name": "role2", 
> "resources": {
> "cpus": 0, 
> "disk": 0, 
> "mem": 0
> }, 
> "weight": 3.1
> }
> ]
> }
> 
> Test update:
> $ curl --user framework1:secret_string1 --data 
> "[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
>  -X PUT http://127.0.0.1:5050/weights
> $ curl http://localhost:5050/roles | python -mjson.tool
> {
> "roles": [
> {
> "frameworks": [],
> "name": "*",
> "resources": {
> "cpus": 0,
> "disk": 0,
> "mem": 0
> },
> "weight": 1.0
> },
> {
> "frameworks": [],
> "name": "role1",
> "resources": {
> "cpus": 0,
> "disk": 0,
> "mem": 0
> },
> "weight": 1.8
> },
> {
> "frameworks": [],
> "name": "role2",
> "resources": {
> "cpus": 0,
> "disk": 0,
> "mem": 0
> },
> "weight": 1.0
> },
> {
> "frameworks": [],
> "name": "role3",
> "resources": {
> "cpus": 0,
> "disk": 0,
> "mem": 0
> },
> "weight": 3.4
> }
> ]
> }
> 
> Test recovuery:
> $ ps -ef | grep mesos-master
> 501 56292 1   0  6:18PM ttys0010:00.31 
> /Users/yqwyq/Desktop/mesos/build/src/.libs/mesos-master --ip=127.0.0.1 
> --work_dir=/tmp/mesos-master --weights=role1=4.2,role2=3.1 
> --authenticate_ht

Re: Review Request 41681: Introduce HTTP endpoint /weights for updating weight.

2016-02-14 Thread Yongqiao Wang

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

(Updated Feb. 14, 2016, 12:02 p.m.)


Review request for mesos, Adam B, Neil Conway, and Qian Zhang.


Changes
---

Addressed the comments of Adam.


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


Repository: mesos


Description
---

Introduce HTTP endpoint /weights for updating weight.


Diffs (updated)
-

  include/mesos/authorizer/authorizer.hpp 
5ee3c7afadd131802c93febbb6b4dbad069c2d81 
  include/mesos/authorizer/authorizer.proto 
226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 
  src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 
  src/Makefile.am 5813ab2c33a7de6b612064e894e5f15b5a474e2b 
  src/authorizer/local/authorizer.hpp c7321c276d566eca6a91f45c546468bea1b0da15 
  src/authorizer/local/authorizer.cpp 9557bbdf68ff182c4538bbf70cee576d717abc05 
  src/master/http.cpp f92212bf69f9db51d729347fb553e74e28e105fd 
  src/master/master.hpp 2f2ad2ada508e1923bf995ab124367a3b082b572 
  src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 
  src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 
  src/master/weights_handler.cpp PRE-CREATION 
  src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 
  src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 

Diff: https://reviews.apache.org/r/41681/diff/


Testing
---

Make & Make check successfully!

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--weights="role1=4.2,role2=3.1" --authenticate_http 
--credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [ ], 
"name": "*", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 1
}, 
{
"frameworks": [ ], 
"name": "role1", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 4.2
}, 
{
"frameworks": [ ], 
"name": "role2", 
"resources": {
"cpus": 0, 
"disk": 0, 
"mem": 0
}, 
"weight": 3.1
}
]
}

Test update:
$ curl --user framework1:secret_string1 --data 
"[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]"
 -X PUT http://127.0.0.1:5050/weights
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [],
"name": "*",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role1",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.8
},
{
"frameworks": [],
"name": "role2",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role3",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 3.4
}
]
}

Test recovuery:
$ ps -ef | grep mesos-master
501 56292 1   0  6:18PM ttys0010:00.31 
/Users/yqwyq/Desktop/mesos/build/src/.libs/mesos-master --ip=127.0.0.1 
--work_dir=/tmp/mesos-master --weights=role1=4.2,role2=3.1 --authenticate_http 
--credentials=/opt/credentials.json
$ kill -9 56292

$ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master  
--weights="role1=4.2,role2=3.1,role6=9.0" --authenticate_http 
--credentials=/opt/credentials.json  >> /tmp/mesos-master.log 2>&1 &)
$ curl http://localhost:5050/roles | python -mjson.tool
{
"roles": [
{
"frameworks": [],
"name": "*",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"frameworks": [],
"name": "role1",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.8
},
{
"frameworks": [],
"name": "role2",
"resources": {
"cpus": 0,
"disk": 0,
"mem": 0
},
"weight": 1.0
},
{
"framew

Re: Review Request 42719: Add doc for weights.

2016-02-14 Thread Yongqiao Wang


> On Feb. 10, 2016, 9:03 a.m., Adam B wrote:
> > Looks good, but I think we can pull this information into the roles.md and 
> > endpoint help docs, rather than create a new docs page.

Maybe weight will also be supported for framework in the future, so I think it 
is better to create a new doc for weights.


> On Feb. 10, 2016, 9:03 a.m., Adam B wrote:
> > docs/weights.md, line 5
> > 
> >
> > Why not just include this in the existing roles.md?
> > If you are creating a new doc, you ought to add it to home.md so it can 
> > easily be found and navigated to.

I considered that weight will also be support for framework, so I created a new 
doc for that.


> On Feb. 10, 2016, 9:03 a.m., Adam B wrote:
> > docs/weights.md, line 19
> > 
> >
> > I wonder if all this information should just go in the endpoint help 
> > description, and then you can link to it in the new auto-generated 
> > endpoints docs, e.g. 
> > http://mesos.apache.org/documentation/latest/endpoints/master/roles/

The /weights endpoint help description will be auto-generated from strings 
stored in the Mesos source code, so if we still decide to create a new doc for 
weights, then does not need to add this link.


- Yongqiao


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


On Jan. 25, 2016, 3 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42719/
> ---
> 
> (Updated Jan. 25, 2016, 3 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add doc for weights.
> 
> 
> Diffs
> -
> 
>   docs/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>



Re: Review Request 42719: Add doc for weights.

2016-02-14 Thread Yongqiao Wang

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

(Updated Feb. 14, 2016, 12:47 p.m.)


Review request for mesos and Adam B.


Changes
---

Addressed the comments of Adam.


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


Repository: mesos


Description
---

Add doc for weights.


Diffs (updated)
-

  docs/home.md a2000a35a6eeaa7b36cb1796532263f5a703ac88 
  docs/weights.md PRE-CREATION 

Diff: https://reviews.apache.org/r/42719/diff/


Testing
---


Thanks,

Yongqiao Wang



Re: Review Request 42719: Add doc for weights.

2016-02-14 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [42719]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 14, 2016, 12:47 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42719/
> ---
> 
> (Updated Feb. 14, 2016, 12:47 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-3945
> https://issues.apache.org/jira/browse/MESOS-3945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add doc for weights.
> 
> 
> Diffs
> -
> 
>   docs/home.md a2000a35a6eeaa7b36cb1796532263f5a703ac88 
>   docs/weights.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42719/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>



Review Request 43564: Defined `status` method for `ComposingContainerizer`.

2016-02-14 Thread Avinash sridharan

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Defined `status` method for `ComposingContainerizer`.


Diffs
-

  src/slave/containerizer/composing.hpp 
6b8757ebc75710b612f6ea5854ee2ff0ca0083ec 

Diff: https://reviews.apache.org/r/43564/diff/


Testing
---

make, make check, and sudo make check.


Thanks,

Avinash sridharan



Review Request 43565: Implemented `status` method in `ComposingContainerizer`.

2016-02-14 Thread Avinash sridharan

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This method will be used by the agent to retrieve `ContainerStatus`
from the `Containerizer`, that was responsible for launching the
container.


Diffs
-

  src/slave/containerizer/composing.cpp 
f0a7bba4a56702872c9b73a12128b5292708d0e7 

Diff: https://reviews.apache.org/r/43565/diff/


Testing
---

make and make check.


Thanks,

Avinash sridharan



Review Request 43566: Modified `NetClsIsolatorTest` to use `ComposingContainerizer`.

2016-02-14 Thread Avinash sridharan

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This tests the case when both both `docker` and `mesos` containerizers
are specified in the agent flag.


Diffs
-

  src/tests/containerizer/isolator_tests.cpp 
653b037c489072f43e53dec01a811a9249dcd660 

Diff: https://reviews.apache.org/r/43566/diff/


Testing
---

make, make check and `sudo make check`.


Thanks,

Avinash sridharan



Re: Review Request 43566: Modified `NetClsIsolatorTest` to use `ComposingContainerizer`.

2016-02-14 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43564, 43565, 43566]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 14, 2016, 4:38 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43566/
> ---
> 
> (Updated Feb. 14, 2016, 4:38 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4670
> https://issues.apache.org/jira/browse/MESOS-4670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This tests the case when both both `docker` and `mesos` containerizers
> are specified in the agent flag.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/isolator_tests.cpp 
> 653b037c489072f43e53dec01a811a9249dcd660 
> 
> Diff: https://reviews.apache.org/r/43566/diff/
> 
> 
> Testing
> ---
> 
> make, make check and `sudo make check`.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 43293: Ignored invalid env vars.

2016-02-14 Thread Guangya Liu


> On 二月 14, 2016, 8:42 a.m., Timothy Chen wrote:
> > src/tests/containerizer/docker_tests.cpp, lines 506-507
> > 
> >
> > Is this even a valid docker inspect output?
> > In what situations will we get this?

Yes, there might be such output if the end user input some invalid env vars.

root@mesos002:/home/gyliu# docker run -it -e env1=xxx -e env1=xxx -e env1+xx 
ubuntu:14.04 /bin/bash
root@84330a72f8b4:/# 

root@mesos002:/home/gyliu# docker inspect 84330a72f8b4 | grep env1
"env1=xxx",
"env1=xxx",
"env1+xx"


- Guangya


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


On 二月 7, 2016, 9:40 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43293/
> ---
> 
> (Updated 二月 7, 2016, 9:40 a.m.)
> 
> 
> Review request for mesos, haosdent huang, Jie Yu, and Timothy Chen.
> 
> 
> Bugs: MESOS-4607
> https://issues.apache.org/jira/browse/MESOS-4607
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored invalid env vars when creating docker image.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
>   src/tests/containerizer/docker_tests.cpp 
> 620819330847a10d9dcd817968df9d2b180a9a29 
> 
> Diff: https://reviews.apache.org/r/43293/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> $ GLOG_v=2 ./bin/mesos-tests.sh  
> --gtest_filter="DockerImageTest.ParseInspectonImage" --verbose
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from DockerImageTest
> [ RUN  ] DockerImageTest.ParseInspectonImage
> I0207 10:30:01.894125 2034615040 process.cpp:2489] Spawned process 
> files@192.168.0.100:49551
> I0207 10:30:01.894136 211529728 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.894163968+00:00
> I0207 10:30:01.894317 210456576 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.894332928+00:00
> I0207 10:30:01.898862 2034615040 docker.cpp:397] Skipping duplicate 
> environment variable 'JAVA_VERSION'
> I0207 10:30:01.898892 2034615040 docker.cpp:390] Skipping invalid environment 
> variable 'JAVA_VERSION+8u66' for 'ContainerConfig.Env'
> I0207 10:30:01.899473 20992 process.cpp:2499] Resuming 
> AuthenticationRouter(1)@192.168.0.100:49551 at 2016-02-07 
> 02:30:01.899522048+00:00
> I0207 10:30:01.899983 213139456 process.cpp:2499] Resuming 
> files@192.168.0.100:49551 at 2016-02-07 02:30:01.9+00:00
> I0207 10:30:01.900032 213139456 process.cpp:2604] Cleaning up 
> files@192.168.0.100:49551
> I0207 10:30:01.900244 211529728 process.cpp:2499] Resuming 
> help@192.168.0.100:49551 at 2016-02-07 02:30:01.900256000+00:00
> [   OK ] DockerImageTest.ParseInspectonImage (8 ms)
> [--] 1 test from DockerImageTest (8 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (18 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43565: Implemented `status` method in `ComposingContainerizer`.

2016-02-14 Thread Guangya Liu

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




src/slave/containerizer/composing.cpp (line 230)


It is already 81 chars, it is better to split this to several lines.



src/slave/containerizer/composing.cpp (line 513)


I was that mesos containizers are using `return Failure("Unknown container: 
" + stringify(containerId));` , can you please update the message to keep 
consistent?


- Guangya Liu


On 二月 14, 2016, 4:31 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43565/
> ---
> 
> (Updated 二月 14, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4670
> https://issues.apache.org/jira/browse/MESOS-4670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method will be used by the agent to retrieve `ContainerStatus`
> from the `Containerizer`, that was responsible for launching the
> container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
> 
> Diff: https://reviews.apache.org/r/43565/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 43565: Implemented `status` method in `ComposingContainerizer`.

2016-02-14 Thread Guangya Liu


> On 二月 15, 2016, 1:17 a.m., Guangya Liu wrote:
> > src/slave/containerizer/composing.cpp, line 513
> > 
> >
> > I was that mesos containizers are using `return Failure("Unknown 
> > container: " + stringify(containerId));` , can you please update the 
> > message to keep consistent?

Mesos containizer using `"Unknown container: " + stringify(containerId)` while 
most of the compose containizer using `"Container " + containerId.value() + " 
not found"`, just ignore this if you do not want an update.


- Guangya


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


On 二月 14, 2016, 4:31 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43565/
> ---
> 
> (Updated 二月 14, 2016, 4:31 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4670
> https://issues.apache.org/jira/browse/MESOS-4670
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This method will be used by the agent to retrieve `ContainerStatus`
> from the `Containerizer`, that was responsible for launching the
> container.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> f0a7bba4a56702872c9b73a12128b5292708d0e7 
> 
> Diff: https://reviews.apache.org/r/43565/diff/
> 
> 
> Testing
> ---
> 
> make and make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Review Request 43569: Updated log message if container not found.

2016-02-14 Thread Guangya Liu

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

Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Updated log message if container not found.


Diffs
-

  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/slave/containerizer/mesos/containerizer.cpp 
3de214d6c71773f5deb5db9fe3f527dd5064737f 

Diff: https://reviews.apache.org/r/43569/diff/


Testing
---


Thanks,

Guangya Liu



Re: Review Request 43569: Updated log message if container not found.

2016-02-14 Thread Guangya Liu

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

(Updated 二月 15, 2016, 1:56 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Updated log message if container not found.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/slave/containerizer/mesos/containerizer.cpp 
3de214d6c71773f5deb5db9fe3f527dd5064737f 

Diff: https://reviews.apache.org/r/43569/diff/


Testing (updated)
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 43569: Updated log message if container not found.

2016-02-14 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43569]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 15, 2016, 1:56 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated Feb. 15, 2016, 1:56 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3de214d6c71773f5deb5db9fe3f527dd5064737f 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43569: Updated log message if container not found.

2016-02-14 Thread Avinash sridharan

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



Just some flyby comments.


src/slave/containerizer/docker.cpp (line 1119)


You don't need to use `stringify` here. `ContainerId` has the ostream 
operator.



src/slave/containerizer/mesos/containerizer.cpp (line 1185)


If we are adding `ContainerId` to other messages, we should add it here as 
well?



src/slave/containerizer/mesos/containerizer.cpp (line 1588)


Maybe
s/Ignore destroy of/Cannot reap/ ?


- Avinash sridharan


On Feb. 15, 2016, 1:56 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated Feb. 15, 2016, 1:56 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3de214d6c71773f5deb5db9fe3f527dd5064737f 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>



Re: Review Request 43015: Fixed persistent volumes with docker tasks.

2016-02-14 Thread Guangya Liu

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




src/slave/containerizer/docker.cpp (line 505)


What about add `containerId` in the log message?



src/tests/containerizer/docker_containerizer_tests.cpp (line 3149)


s/by default/the default

There are indeed patches trying to update all `by default` to `the 
default`: https://reviews.apache.org/r/42689/



src/tests/containerizer/docker_containerizer_tests.cpp (line 3160)


Why not `ASSERT_EQ(1u, offers2.get().size());`? This seems more accurate


- Guangya Liu


On 二月 14, 2016, 8:38 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43015/
> ---
> 
> (Updated 二月 14, 2016, 8:38 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3413
> https://issues.apache.org/jira/browse/MESOS-3413
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed persistent volumes with docker tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 77a50d80179672cf3c270cbdd7fa003c7d9ad6f3 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
> 
> Diff: https://reviews.apache.org/r/43015/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>



Re: Review Request 43569: Updated log message if container not found.

2016-02-14 Thread Guangya Liu

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

(Updated 二月 15, 2016, 4:17 a.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Updated log message if container not found.


Diffs (updated)
-

  src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
  src/slave/containerizer/mesos/containerizer.cpp 
3de214d6c71773f5deb5db9fe3f527dd5064737f 

Diff: https://reviews.apache.org/r/43569/diff/


Testing
---

make
make check


Thanks,

Guangya Liu



Re: Review Request 43569: Updated log message if container not found.

2016-02-14 Thread Mesos ReviewBot

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



Patch looks great!

Reviews applied: [43569]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On Feb. 15, 2016, 4:17 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43569/
> ---
> 
> (Updated Feb. 15, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated log message if container not found.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp ed1c9a551f03a37d572470e4c495f5df834198cc 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 3de214d6c71773f5deb5db9fe3f527dd5064737f 
> 
> Diff: https://reviews.apache.org/r/43569/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>