Re: Review Request 48593: Refactor Ranges Subtraction.

[Klaus Ma]

> On June 13, 2016, 11:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > 
> >
> > Any data on performance improvement?
> 
> Yanyan Hu wrote:
> Hi, Klaus, performance data is listed here: 
> https://issues.apache.org/jira/browse/MESOS-5425# thanks.

Thanks; according to the data. It seems worse performance when # < 200, but 
better performance when # > 200. I'm not sure which is general case in 
production :).


- Klaus


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


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Review Request 48614: Added validation for UUID's to master/agent validation helpers.

[Deshna Jain]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds logic to the master/agent validation helpers
to see if the status update acknowledgments have a valid UUID
set conforming to one of the supported versions.


Diffs
-

  src/master/validation.cpp 309fbed81c9ff0ccc4ff4ee3ee70cf8f1fb2ac55 
  src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 
  src/tests/scheduler_http_api_tests.cpp 
c12205f90e4f2da2c6ad7a0ea75777601eb0ef13 

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


Testing
---


Thanks,

Deshna Jain

Review Request 48613: Added validation logic for UUID's.

[Deshna Jain]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This change adds a `validateFromBytes` function for validating
UUID's by checking if they are one of the supported UUID
versions.


Diffs
-

  3rdparty/stout/include/stout/uuid.hpp 
e3cf21ff986112694a417981c22009323cb23987 
  3rdparty/stout/tests/uuid_tests.cpp 50295ad25843a3938a5a58f5ba7f082f8ffeba45 

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


Testing
---

make check


Thanks,

Deshna Jain

Re: Review Request 47893: Changed initialization order of `Anonymous` modules in Master.

[Avinash sridharan]

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

(Updated June 13, 2016, 6:43 a.m.)


Review request for mesos, Jie Yu and Kapil Arya.


Changes
---

re-based. Also, moved the loading and initialization of `Anonymous` modules 
next to each other.


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


Repository: mesos


Description
---

Changed initialization order of `Anonymous` modules in Master.


Diffs (updated)
-

  src/master/main.cpp ce6291f2595163a578abac515cb8250b066cbc52 

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


Testing
---

make check 
and sudo build/bin/mesos-tests.sh


Thanks,

Avinash sridharan

Re: Review Request 47893: Changed initialization order of `Anonymous` modules in Master.

[Avinash sridharan]

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

(Updated June 13, 2016, 6 a.m.)


Review request for mesos, Jie Yu and Kapil Arya.


Changes
---

re-based.


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


Repository: mesos


Description
---

Changed initialization order of `Anonymous` modules in Master.


Diffs (updated)
-

  src/master/main.cpp ce6291f2595163a578abac515cb8250b066cbc52 

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


Testing
---

make check 
and sudo build/bin/mesos-tests.sh


Thanks,

Avinash sridharan

Re: Review Request 48610: Renamed docker spec helper functions.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47806, 47807, 47808, 45951, 48570, 48604, 48605, 45952, 48610]

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

- Mesos ReviewBot


On June 13, 2016, 4:20 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48610/
> ---
> 
> (Updated June 13, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed docker spec helper functions.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
>   src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 944a559bb3a8ef602b4d8d52773afdb8bf8e5bf6 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/48610/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 48612: Removed conditional for of setting `--prefix=/`.

[Avinash sridharan]

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

Review request for mesos and Kapil Arya.


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


Repository: mesos


Description
---

For bundled 3rdparty packages the `--prefix` flag should always be set
to `/`, else the 3rdparty installation path might append the
$MESOS_INSTALL to the installation path.


Diffs
-

  3rdparty/Makefile.am fb4a37d50e751703b4ccddb0e004b58560707067 

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


Testing
---

make and make install

Bundled zookeeper installation takes place on the following path:
vagrant@mesos-dev:~/mesosphere/mesos/build/3rdparty$ find ~/mesos_install/ 
-name "zookeeper*"
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper.h
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper_version.h
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper_log.h
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper.jute.h
/home/vagrant/mesos_install/include/mesos/zookeeper
/home/vagrant/mesos_install/include/mesos/zookeeper/zookeeper.hpp
/home/vagrant/mesos_install/include/mesos/state/zookeeper.hpp


Thanks,

Avinash sridharan

Re: Review Request 48594: Added bundled zookeeper to the module-dependencies.

[Avinash sridharan]

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

(Updated June 13, 2016, 4:56 a.m.)


Review request for mesos, Jie Yu and Kapil Arya.


Changes
---

Updated the test description with results after running `make clean` in 
build/3rdparty.


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


Repository: mesos


Description
---

Modules that want to use the `State` API for storing information in
the replicated log require zookeeper client headers. With this change
the bundled zookeeper client headers will be installed along with the
Mesos libraries and headers under the Mesos installation path.


Diffs
-

  3rdparty/Makefile.am fb4a37d50e751703b4ccddb0e004b58560707067 

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


Testing (updated)
---

cd build/3rdparty; make clean
cd $MESOS_SRC/build/
make and make install

After running make install made sure that the bundled client libraries are 
installed under the $MESOS_INSTALL path. Here is an example output:

/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper.h
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper_version.h
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper_log.h
/home/vagrant/mesos_install/lib/mesos/3rdparty/include/zookeeper/zookeeper.jute.h
/home/vagrant/mesos_install/include/mesos/zookeeper
/home/vagrant/mesos_install/include/mesos/zookeeper/zookeeper.hpp
/home/vagrant/mesos_install/include/mesos/state/zookeeper.hpp


Thanks,

Avinash sridharan

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45377, 36440]

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

- Mesos ReviewBot


On June 13, 2016, 4:36 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36440/
> ---
> 
> (Updated June 13, 2016, 4:36 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5341
> https://issues.apache.org/jira/browse/MESOS-5341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled docker volume support for DockerContainerizer.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
>   include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
>   src/docker/docker.cpp a6dcbe788abb8802baa2c70ec2fced1da75c7210 
> 
> Diff: https://reviews.apache.org/r/36440/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 1) Start convoy
> 2) Create a volume named as c1
> root@kvm-002605:~# convoy list
> {
> "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
> "UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Name": "c1",
> "Driver": "devicemapper",
> "MountPoint": "",
> "CreatedTime": "Mon May 09 09:38:52 +0800 2016",
> "DriverInfo": {
> "DevID": "1",
> "Device": 
> "/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Driver": "devicemapper",
> "MountPoint": "",
> "Size": "107374182400"
> },
> "Snapshots": {}
> }
> }
> 3) Update  mesos-execte to set up volume
> index 4711e80..6c712a2 100644
> --- a/src/cli/execute.cpp
> +++ b/src/cli/execute.cpp
> @@ -72,6 +72,8 @@ using mesos::v1::TaskID;
>  using mesos::v1::TaskInfo;
>  using mesos::v1::TaskState;
>  using mesos::v1::TaskStatus;
> +using mesos::v1::Volume;
> +using mesos::v1::Parameters;
>  
>  using mesos::v1::scheduler::Call;
>  using mesos::v1::scheduler::Event;
> @@ -581,6 +583,18 @@ private:
>  
>containerInfo.set_type(ContainerInfo::DOCKER);
>containerInfo.mutable_docker()->set_image(dockerImage.get());
> +  //containerInfo.mutable_docker()->set_volume_driver("convoy");
> +
> +  Volume* volume1 = containerInfo.add_volumes();
> +  //volume1->set_host_path("c1");
> +  volume1->set_container_path("/tmp");
> +  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
> +  volume1->mutable_source()->mutable_docker_volume()->set_driver(
> +  "convoy");
> +  volume1->mutable_source()->mutable_docker_volume()->set_name(
> +  "c1");
> +  volume1->set_mode(Volume::RW);
> +  cout << "Add Voume 1" << endl;
>  
>if (networks.isSome() && !networks->empty()) {
>  vector tokens = strings::tokenize(networks.get(), ",");
> 4) Start master
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master
> 5) Start agent
> GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
> --isolation=filesystem/linux,docker/runtime --containerizers=docker 
> --work_dir=/tmp/mesos --image_providers=docker
> 5) Start mesos-execute
> src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
> --docker_image=ubuntu:14.04 --containerizer=docker
> I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
> I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
> master@10.0.0.65:5050
> Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
> Add Voume 1
> Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_LOST for task 'test'
> 
> 6) Inspect container
> docker inspect 
> ...
> {
> "Name": "c1",
> "Source": 
> "/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Destination": "/tmp",
> "Driver": "convoy",
> "Mode": "rw",
> "RW": true,
> "Propagation": "rprivate"
> }
> ...
> 7) Check sandbox executer log
> I0509 13:14:56.313102  8361 logging.cpp:195] Logging to STDERR
> I0509 13:14:56.315385  8361 process.cpp:999] libprocess is initialized on 
> 10.0.0.65:52853 with 16 worker threads
> I0509 13:14:56.316789  8361 exec.cpp:150] Version: 0.29.0
> I0509 13:14:56.321717  8387 exec.cpp:200] Executor started at: 
> executor(1)@10.0.0.65:52853 with pid 8

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Guangya Liu]

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

(Updated 六月 13, 2016, 4:36 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Enabled docker volume support for DockerContainerizer.


Diffs (updated)
-

  include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
  include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
  src/docker/docker.cpp a6dcbe788abb8802baa2c70ec2fced1da75c7210 

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


Testing
---

make
make check

1) Start convoy
2) Create a volume named as c1
root@kvm-002605:~# convoy list
{
"f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
"UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Name": "c1",
"Driver": "devicemapper",
"MountPoint": "",
"CreatedTime": "Mon May 09 09:38:52 +0800 2016",
"DriverInfo": {
"DevID": "1",
"Device": 
"/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Driver": "devicemapper",
"MountPoint": "",
"Size": "107374182400"
},
"Snapshots": {}
}
}
3) Update  mesos-execte to set up volume
index 4711e80..6c712a2 100644
--- a/src/cli/execute.cpp
+++ b/src/cli/execute.cpp
@@ -72,6 +72,8 @@ using mesos::v1::TaskID;
 using mesos::v1::TaskInfo;
 using mesos::v1::TaskState;
 using mesos::v1::TaskStatus;
+using mesos::v1::Volume;
+using mesos::v1::Parameters;
 
 using mesos::v1::scheduler::Call;
 using mesos::v1::scheduler::Event;
@@ -581,6 +583,18 @@ private:
 
   containerInfo.set_type(ContainerInfo::DOCKER);
   containerInfo.mutable_docker()->set_image(dockerImage.get());
+  //containerInfo.mutable_docker()->set_volume_driver("convoy");
+
+  Volume* volume1 = containerInfo.add_volumes();
+  //volume1->set_host_path("c1");
+  volume1->set_container_path("/tmp");
+  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
+  volume1->mutable_source()->mutable_docker_volume()->set_driver(
+  "convoy");
+  volume1->mutable_source()->mutable_docker_volume()->set_name(
+  "c1");
+  volume1->set_mode(Volume::RW);
+  cout << "Add Voume 1" << endl;
 
   if (networks.isSome() && !networks->empty()) {
 vector tokens = strings::tokenize(networks.get(), ",");
4) Start master
./bin/mesos-master.sh --work_dir=/tmp/mesos/master
5) Start agent
GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
--isolation=filesystem/linux,docker/runtime --containerizers=docker 
--work_dir=/tmp/mesos --image_providers=docker
5) Start mesos-execute
src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
--docker_image=ubuntu:14.04 --containerizer=docker
I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
master@10.0.0.65:5050
Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
Add Voume 1
Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
Received status update TASK_RUNNING for task 'test'
  source: SOURCE_EXECUTOR
Received status update TASK_LOST for task 'test'

6) Inspect container
docker inspect 
...
{
"Name": "c1",
"Source": 
"/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Destination": "/tmp",
"Driver": "convoy",
"Mode": "rw",
"RW": true,
"Propagation": "rprivate"
}
...
7) Check sandbox executer log
I0509 13:14:56.313102  8361 logging.cpp:195] Logging to STDERR
I0509 13:14:56.315385  8361 process.cpp:999] libprocess is initialized on 
10.0.0.65:52853 with 16 worker threads
I0509 13:14:56.316789  8361 exec.cpp:150] Version: 0.29.0
I0509 13:14:56.321717  8387 exec.cpp:200] Executor started at: 
executor(1)@10.0.0.65:52853 with pid 8361
I0509 13:14:56.325158  8388 exec.cpp:225] Executor registered on agent 
5454a953-83d0-42a4-b7fe-0825a6667f8c-S0
I0509 13:14:56.326740  8388 exec.cpp:237] Executor::registered took 325332ns
I0509 13:14:56.327030  8388 exec.cpp:312] Executor asked to run task 'test'
I0509 13:14:56.327163  8388 exec.cpp:321] Executor::launchTask took 98803ns
I0509 13:14:56.327354  8386 docker.cpp:546] Volume config 'c1:/tmp' driver 
'convoy' for container 
'mesos-5454a953-83d0-42a4-b7fe-0825a6667f8c-S0.d2fb7a57-d364-41b8-a671-30b95c5cebb4'
I0509 13:14:56.327437  8386 docker.cpp:707] Running docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
MESOS_SANDBOX=/mnt/mesos/sandbox -e 
MESOS_CONTAINER_NAME=mesos-5454a953-83d0-42a4-b7fe-0825a6667f8c-S0.d2fb7a57-d364-41b

Re: Review Request 48604: Implemented docker spec helper to parse URL.

[Gilbert Song]

> On June 12, 2016, 8:25 p.m., Jie Yu wrote:
> >

Both are fixed by patch https://reviews.apache.org/r/48610/


- Gilbert


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


On June 12, 2016, 12:07 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48604/
> ---
> 
> (Updated June 12, 2016, 12:07 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented docker spec helper to parse URL.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
>   src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
> 
> Diff: https://reviews.apache.org/r/48604/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 48610: Renamed docker spec helper functions.

[Gilbert Song]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Renamed docker spec helper functions.


Diffs
-

  include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
  src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
  src/tests/containerizer/docker_spec_tests.cpp 
944a559bb3a8ef602b4d8d52773afdb8bf8e5bf6 
  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48593: Refactor Ranges Subtraction.

[Yanyan Hu]

> On June 12, 2016, 9:32 a.m., Guangya Liu wrote:
> > Hi Yanyan, can you please add some test cases for your code chage? You can 
> > refer to 
> > https://github.com/apache/mesos/blob/master/src/tests/values_tests.cpp for 
> > detail.
> > 
> > You can take a look at 
> > https://github.com/apache/mesos/blob/master/docs/newbie-guide.md#making-changes
> >  for how to run a specified unit test case.

Thanks, Guangya, will add some test cases for this change.


- Yanyan


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


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

[Yanyan Hu]

> On June 13, 2016, 3:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > 
> >
> > Any data on performance improvement?

Hi, Klaus, performance data is listed here: 
https://issues.apache.org/jira/browse/MESOS-5425# thanks.


- Yanyan


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


On June 12, 2016, 8:03 a.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 8:03 a.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Guangya Liu]

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

(Updated 六月 13, 2016, 3:43 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Enabled docker volume support for DockerContainerizer.


Diffs (updated)
-

  include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 
  include/mesos/v1/mesos.proto 39967fa09d2774d564f3df28277edea8ebcfb50d 
  src/docker/docker.cpp a6dcbe788abb8802baa2c70ec2fced1da75c7210 

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


Testing
---

make
make check

1) Start convoy
2) Create a volume named as c1
root@kvm-002605:~# convoy list
{
"f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
"UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Name": "c1",
"Driver": "devicemapper",
"MountPoint": "",
"CreatedTime": "Mon May 09 09:38:52 +0800 2016",
"DriverInfo": {
"DevID": "1",
"Device": 
"/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Driver": "devicemapper",
"MountPoint": "",
"Size": "107374182400"
},
"Snapshots": {}
}
}
3) Update  mesos-execte to set up volume
index 4711e80..6c712a2 100644
--- a/src/cli/execute.cpp
+++ b/src/cli/execute.cpp
@@ -72,6 +72,8 @@ using mesos::v1::TaskID;
 using mesos::v1::TaskInfo;
 using mesos::v1::TaskState;
 using mesos::v1::TaskStatus;
+using mesos::v1::Volume;
+using mesos::v1::Parameters;
 
 using mesos::v1::scheduler::Call;
 using mesos::v1::scheduler::Event;
@@ -581,6 +583,18 @@ private:
 
   containerInfo.set_type(ContainerInfo::DOCKER);
   containerInfo.mutable_docker()->set_image(dockerImage.get());
+  //containerInfo.mutable_docker()->set_volume_driver("convoy");
+
+  Volume* volume1 = containerInfo.add_volumes();
+  //volume1->set_host_path("c1");
+  volume1->set_container_path("/tmp");
+  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
+  volume1->mutable_source()->mutable_docker_volume()->set_driver(
+  "convoy");
+  volume1->mutable_source()->mutable_docker_volume()->set_name(
+  "c1");
+  volume1->set_mode(Volume::RW);
+  cout << "Add Voume 1" << endl;
 
   if (networks.isSome() && !networks->empty()) {
 vector tokens = strings::tokenize(networks.get(), ",");
4) Start master
./bin/mesos-master.sh --work_dir=/tmp/mesos/master
5) Start agent
GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
--isolation=filesystem/linux,docker/runtime --containerizers=docker 
--work_dir=/tmp/mesos --image_providers=docker
5) Start mesos-execute
src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
--docker_image=ubuntu:14.04 --containerizer=docker
I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
master@10.0.0.65:5050
Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
Add Voume 1
Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
Received status update TASK_RUNNING for task 'test'
  source: SOURCE_EXECUTOR
Received status update TASK_LOST for task 'test'

6) Inspect container
docker inspect 
...
{
"Name": "c1",
"Source": 
"/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
"Destination": "/tmp",
"Driver": "convoy",
"Mode": "rw",
"RW": true,
"Propagation": "rprivate"
}
...
7) Check sandbox executer log
I0509 13:14:56.313102  8361 logging.cpp:195] Logging to STDERR
I0509 13:14:56.315385  8361 process.cpp:999] libprocess is initialized on 
10.0.0.65:52853 with 16 worker threads
I0509 13:14:56.316789  8361 exec.cpp:150] Version: 0.29.0
I0509 13:14:56.321717  8387 exec.cpp:200] Executor started at: 
executor(1)@10.0.0.65:52853 with pid 8361
I0509 13:14:56.325158  8388 exec.cpp:225] Executor registered on agent 
5454a953-83d0-42a4-b7fe-0825a6667f8c-S0
I0509 13:14:56.326740  8388 exec.cpp:237] Executor::registered took 325332ns
I0509 13:14:56.327030  8388 exec.cpp:312] Executor asked to run task 'test'
I0509 13:14:56.327163  8388 exec.cpp:321] Executor::launchTask took 98803ns
I0509 13:14:56.327354  8386 docker.cpp:546] Volume config 'c1:/tmp' driver 
'convoy' for container 
'mesos-5454a953-83d0-42a4-b7fe-0825a6667f8c-S0.d2fb7a57-d364-41b8-a671-30b95c5cebb4'
I0509 13:14:56.327437  8386 docker.cpp:707] Running docker -H 
unix:///var/run/docker.sock run --cpu-shares 1024 --memory 134217728 -e 
MESOS_SANDBOX=/mnt/mesos/sandbox -e 
MESOS_CONTAINER_NAME=mesos-5454a953-83d0-42a4-b7fe-0825a6667f8c-S0.d2fb7a57-d364-41b

Re: Review Request 48604: Implemented docker spec helper to parse URL.

[Gilbert Song]

> On June 12, 2016, 8:25 p.m., Jie Yu wrote:
> > include/mesos/docker/spec.hpp, line 83
> > 
> >
> > I would rename this to parseAuthConfig as well. Can you followup with a 
> > patch to do the renaming?

Sure, I will post a followup patch to rename this two functions now.


- Gilbert


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


On June 12, 2016, 12:07 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48604/
> ---
> 
> (Updated June 12, 2016, 12:07 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented docker spec helper to parse URL.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
>   src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
> 
> Diff: https://reviews.apache.org/r/48604/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Guangya Liu]

> On 六月 12, 2016, 7:10 p.m., Jie Yu wrote:
> > src/docker/docker.cpp, lines 559-562
> > 
> >
> > I don't get this check. if we end up here, has_host_path() is false and 
> > has_source() is false, why we bother having this check?

I was following the original logic as here: 
https://github.com/apache/mesos/blob/0.28.x/src/docker/docker.cpp#L529-L531

I think that the reason we need this check is because if a volume has mode but 
no source or host path, we should ignore this request and report error.


- Guangya


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


On 五月 10, 2016, 3:50 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36440/
> ---
> 
> (Updated 五月 10, 2016, 3:50 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5341
> https://issues.apache.org/jira/browse/MESOS-5341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled docker volume support for DockerContainerizer.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 73d4bc0ec3f34033eddf04aa41893c1fc66933b3 
>   include/mesos/v1/mesos.proto 44b4f8a059f9dfdcbf02f0c30c1b859898c2e617 
>   src/docker/docker.cpp 19cf424dfd5748ae66a7023840aa2b0652e8f2c0 
> 
> Diff: https://reviews.apache.org/r/36440/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 1) Start convoy
> 2) Create a volume named as c1
> root@kvm-002605:~# convoy list
> {
> "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
> "UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Name": "c1",
> "Driver": "devicemapper",
> "MountPoint": "",
> "CreatedTime": "Mon May 09 09:38:52 +0800 2016",
> "DriverInfo": {
> "DevID": "1",
> "Device": 
> "/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Driver": "devicemapper",
> "MountPoint": "",
> "Size": "107374182400"
> },
> "Snapshots": {}
> }
> }
> 3) Update  mesos-execte to set up volume
> index 4711e80..6c712a2 100644
> --- a/src/cli/execute.cpp
> +++ b/src/cli/execute.cpp
> @@ -72,6 +72,8 @@ using mesos::v1::TaskID;
>  using mesos::v1::TaskInfo;
>  using mesos::v1::TaskState;
>  using mesos::v1::TaskStatus;
> +using mesos::v1::Volume;
> +using mesos::v1::Parameters;
>  
>  using mesos::v1::scheduler::Call;
>  using mesos::v1::scheduler::Event;
> @@ -581,6 +583,18 @@ private:
>  
>containerInfo.set_type(ContainerInfo::DOCKER);
>containerInfo.mutable_docker()->set_image(dockerImage.get());
> +  //containerInfo.mutable_docker()->set_volume_driver("convoy");
> +
> +  Volume* volume1 = containerInfo.add_volumes();
> +  //volume1->set_host_path("c1");
> +  volume1->set_container_path("/tmp");
> +  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
> +  volume1->mutable_source()->mutable_docker_volume()->set_driver(
> +  "convoy");
> +  volume1->mutable_source()->mutable_docker_volume()->set_name(
> +  "c1");
> +  volume1->set_mode(Volume::RW);
> +  cout << "Add Voume 1" << endl;
>  
>if (networks.isSome() && !networks->empty()) {
>  vector tokens = strings::tokenize(networks.get(), ",");
> 4) Start master
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master
> 5) Start agent
> GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
> --isolation=filesystem/linux,docker/runtime --containerizers=docker 
> --work_dir=/tmp/mesos --image_providers=docker
> 5) Start mesos-execute
> src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
> --docker_image=ubuntu:14.04 --containerizer=docker
> I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
> I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
> master@10.0.0.65:5050
> Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
> Add Voume 1
> Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_LOST for task 'test'
> 
> 6) Inspect container
> docker inspect 
> ...
> {
> "Name": "c1",
> "Source": 
> "/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Destination": "/tmp",
> "Driver": "convoy",
> "Mode": "rw",
> "RW": true,
> "Propagation": "rprivate"
> }
> ...
> 7) Check sandbox executer log

Re: Review Request 48593: Refactor Ranges Subtraction.

[Klaus Ma]

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




src/common/values.cpp (lines 401 - 409)


Any data on performance improvement?


- Klaus Ma


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47511]

Failed command: ./support/apply-review.sh -n -r 47511

Error:
2016-06-13 03:34:00 URL:https://reviews.apache.org/r/47511/diff/raw/ 
[11891/11891] -> "47511.patch" [1]
error: missing binary patch data for 'docs/images/docker-volume-isolator.png'
error: binary patch does not apply to 'docs/images/docker-volume-isolator.png'
error: docs/images/docker-volume-isolator.png: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13729/console

- Mesos ReviewBot


On June 13, 2016, 3:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated June 13, 2016, 3:18 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document here: 
> https://github.com/jay-lau/mesos/blob/master/docs/docker-volume.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47396: Added aufs provisioning backend.

[Guangya Liu]

> On 六月 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > 
> >
> > I think that here also needs to be updated to use your new function 
> > `fs::aufs::supported()` to check `aufs`?
> 
> Jie Yu wrote:
> Hum... this looks weird to me. I am wondering if we can make 
> `fs::supported` understand the sutble details related to overlayfs? Can you 
> guys follow up with a patch. Basically, here, it should be `Try check = 
> fs::supported(fsname);` and inside `fs::supported`, we check special case for 
> overlayfs.
> 
> Guangya Liu wrote:
> I'm now thinking that we may not need the API of `fs::aufs::supported`, 
> the reason that we introduced `fs::overlay::supported` is becuase of keyword 
> issue, the overlay fs keyword in `/proc/filesystems` can be `overlay` or 
> `overlayfs`.
> 
> But for `aufs`, I think we do not need to introduce the new API of 
> `fs::aufs::supported` but call `fs::supported` directly, as the API of 
> `fs::aufs::supported` is just a wrapper of `fs::supported`. Comments?
> 
> Jie Yu wrote:
> Yeah, I agree with you Guangya. I think we should make 
> `fs::supported(fsname)` understand both overlayfs and overlay and get rid of 
> all fs::xxx::supported logics. Do you have time to follow up with a patch?
> 
> Guangya Liu wrote:
> Hi Yu Jie, it may be difficult to make `fs::supported(fsname)` to 
> understand both overlayfs and overlay, so my thinking is that we only keep 
> `fs::overlay::supported` but remove the `fs::aufs::supported`, comments?
> 
> Jie Yu wrote:
> Why it's difficult to make fs::supported(fsname) to understand both 
> overlayfs and overlay?
> 
> Guangya Liu wrote:
> My first thinking is as https://reviews.apache.org/r/44421/diff/1/
> 
> But considering this will change the parameter of `fs::supported`, so we 
> decide to add `fs::overlay::supported`.  Comments?
> 
> Jie Yu wrote:
> Let's not change the parameter of fs::supported, instead, change the 
> logic inside fs::supported so that it handles overlay/overlayfs well.

Got it, then the `fs::supported` may have some special logic for `overlay` and 
`overlayfs`, I will post a patch later for this.


- Guangya


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


On 六月 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> ---
> 
> (Updated 六月 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
> https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> b2a20b7c08fa790da09ba05b725248e42b8d3bc4 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> bc04760c9504ea55cd4bb72c7e9012e43a5911aa 
>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> ---
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script 
> that launches tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 47396: Added aufs provisioning backend.

[Jie Yu]

> On June 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > 
> >
> > I think that here also needs to be updated to use your new function 
> > `fs::aufs::supported()` to check `aufs`?
> 
> Jie Yu wrote:
> Hum... this looks weird to me. I am wondering if we can make 
> `fs::supported` understand the sutble details related to overlayfs? Can you 
> guys follow up with a patch. Basically, here, it should be `Try check = 
> fs::supported(fsname);` and inside `fs::supported`, we check special case for 
> overlayfs.
> 
> Guangya Liu wrote:
> I'm now thinking that we may not need the API of `fs::aufs::supported`, 
> the reason that we introduced `fs::overlay::supported` is becuase of keyword 
> issue, the overlay fs keyword in `/proc/filesystems` can be `overlay` or 
> `overlayfs`.
> 
> But for `aufs`, I think we do not need to introduce the new API of 
> `fs::aufs::supported` but call `fs::supported` directly, as the API of 
> `fs::aufs::supported` is just a wrapper of `fs::supported`. Comments?
> 
> Jie Yu wrote:
> Yeah, I agree with you Guangya. I think we should make 
> `fs::supported(fsname)` understand both overlayfs and overlay and get rid of 
> all fs::xxx::supported logics. Do you have time to follow up with a patch?
> 
> Guangya Liu wrote:
> Hi Yu Jie, it may be difficult to make `fs::supported(fsname)` to 
> understand both overlayfs and overlay, so my thinking is that we only keep 
> `fs::overlay::supported` but remove the `fs::aufs::supported`, comments?
> 
> Jie Yu wrote:
> Why it's difficult to make fs::supported(fsname) to understand both 
> overlayfs and overlay?
> 
> Guangya Liu wrote:
> My first thinking is as https://reviews.apache.org/r/44421/diff/1/
> 
> But considering this will change the parameter of `fs::supported`, so we 
> decide to add `fs::overlay::supported`.  Comments?

Let's not change the parameter of fs::supported, instead, change the logic 
inside fs::supported so that it handles overlay/overlayfs well.


- Jie


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


On June 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> ---
> 
> (Updated June 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
> https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> b2a20b7c08fa790da09ba05b725248e42b8d3bc4 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> bc04760c9504ea55cd4bb72c7e9012e43a5911aa 
>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> ---
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script 
> that launches tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 48605: Added docker spec test for parseUrl.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 12, 2016, 7:08 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48605/
> ---
> 
> (Updated June 12, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docker spec test for parseUrl.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 944a559bb3a8ef602b4d8d52773afdb8bf8e5bf6 
> 
> Diff: https://reviews.apache.org/r/48605/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48604: Implemented docker spec helper to parse URL.

[Jie Yu]

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


Fix it, then Ship it!





include/mesos/docker/spec.hpp (line 83)


I would rename this to parseAuthConfig as well. Can you followup with a 
patch to do the renaming?



include/mesos/docker/spec.hpp (line 89)


parseUrl sounds too general.

s/parseUrl/parseAuthUrl/


- Jie Yu


On June 12, 2016, 7:07 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48604/
> ---
> 
> (Updated June 12, 2016, 7:07 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented docker spec helper to parse URL.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
>   src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 
> 
> Diff: https://reviews.apache.org/r/48604/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Guangya Liu]

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

(Updated 六月 13, 2016, 3:18 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for `docker/volume` isolator.


Diffs
-

  docs/docker-volume.md PRE-CREATION 
  docs/images/docker-volume-isolator.png PRE-CREATION 

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


Testing (updated)
---

You can review the document here: 
https://github.com/jay-lau/mesos/blob/master/docs/docker-volume.md


Thanks,

Guangya Liu

Re: Review Request 47396: Added aufs provisioning backend.

[Guangya Liu]

> On 六月 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > 
> >
> > I think that here also needs to be updated to use your new function 
> > `fs::aufs::supported()` to check `aufs`?
> 
> Jie Yu wrote:
> Hum... this looks weird to me. I am wondering if we can make 
> `fs::supported` understand the sutble details related to overlayfs? Can you 
> guys follow up with a patch. Basically, here, it should be `Try check = 
> fs::supported(fsname);` and inside `fs::supported`, we check special case for 
> overlayfs.
> 
> Guangya Liu wrote:
> I'm now thinking that we may not need the API of `fs::aufs::supported`, 
> the reason that we introduced `fs::overlay::supported` is becuase of keyword 
> issue, the overlay fs keyword in `/proc/filesystems` can be `overlay` or 
> `overlayfs`.
> 
> But for `aufs`, I think we do not need to introduce the new API of 
> `fs::aufs::supported` but call `fs::supported` directly, as the API of 
> `fs::aufs::supported` is just a wrapper of `fs::supported`. Comments?
> 
> Jie Yu wrote:
> Yeah, I agree with you Guangya. I think we should make 
> `fs::supported(fsname)` understand both overlayfs and overlay and get rid of 
> all fs::xxx::supported logics. Do you have time to follow up with a patch?
> 
> Guangya Liu wrote:
> Hi Yu Jie, it may be difficult to make `fs::supported(fsname)` to 
> understand both overlayfs and overlay, so my thinking is that we only keep 
> `fs::overlay::supported` but remove the `fs::aufs::supported`, comments?
> 
> Jie Yu wrote:
> Why it's difficult to make fs::supported(fsname) to understand both 
> overlayfs and overlay?

My first thinking is as https://reviews.apache.org/r/44421/diff/1/

But considering this will change the parameter of `fs::supported`, so we decide 
to add `fs::overlay::supported`.  Comments?


- Guangya


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


On 六月 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> ---
> 
> (Updated 六月 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
> https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> b2a20b7c08fa790da09ba05b725248e42b8d3bc4 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> bc04760c9504ea55cd4bb72c7e9012e43a5911aa 
>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> ---
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script 
> that launches tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Guangya Liu]

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

(Updated 六月 13, 2016, 3:16 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added documentation for `docker/volume` isolator.


Diffs (updated)
-

  docs/docker-volume.md PRE-CREATION 
  docs/images/docker-volume-isolator.png PRE-CREATION 

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


Testing
---

You can review the document here: 
https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md


Thanks,

Guangya Liu

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Guangya Liu]

> On 六月 12, 2016, 6:57 p.m., Jie Yu wrote:
> > docs/docker-volume-isolator.md, line 181
> > 
> >
> > WHy operator?

Updating to `framework developer`


> On 六月 12, 2016, 6:57 p.m., Jie Yu wrote:
> > docs/docker-volume-isolator.md, line 184
> > 
> >
> > WHy operator?

Updating to `framework developer`


> On 六月 12, 2016, 6:57 p.m., Jie Yu wrote:
> > docs/docker-volume-isolator.md, line 67
> > 
> >
> > Have a link to below (i.e., framework API)

For this part, I think that we should refer to `TaskInfo` and it was already 
covered by `Enabling frameworks for Docker volume support`, comments?


- Guangya


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


On 六月 13, 2016, 3:16 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated 六月 13, 2016, 3:16 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document here: 
> https://github.com/jay-lau/mesos/blob/master/docs/docker-volume-isolator.md
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47396: Added aufs provisioning backend.

[Jie Yu]

> On June 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > 
> >
> > I think that here also needs to be updated to use your new function 
> > `fs::aufs::supported()` to check `aufs`?
> 
> Jie Yu wrote:
> Hum... this looks weird to me. I am wondering if we can make 
> `fs::supported` understand the sutble details related to overlayfs? Can you 
> guys follow up with a patch. Basically, here, it should be `Try check = 
> fs::supported(fsname);` and inside `fs::supported`, we check special case for 
> overlayfs.
> 
> Guangya Liu wrote:
> I'm now thinking that we may not need the API of `fs::aufs::supported`, 
> the reason that we introduced `fs::overlay::supported` is becuase of keyword 
> issue, the overlay fs keyword in `/proc/filesystems` can be `overlay` or 
> `overlayfs`.
> 
> But for `aufs`, I think we do not need to introduce the new API of 
> `fs::aufs::supported` but call `fs::supported` directly, as the API of 
> `fs::aufs::supported` is just a wrapper of `fs::supported`. Comments?
> 
> Jie Yu wrote:
> Yeah, I agree with you Guangya. I think we should make 
> `fs::supported(fsname)` understand both overlayfs and overlay and get rid of 
> all fs::xxx::supported logics. Do you have time to follow up with a patch?
> 
> Guangya Liu wrote:
> Hi Yu Jie, it may be difficult to make `fs::supported(fsname)` to 
> understand both overlayfs and overlay, so my thinking is that we only keep 
> `fs::overlay::supported` but remove the `fs::aufs::supported`, comments?

Why it's difficult to make fs::supported(fsname) to understand both overlayfs 
and overlay?


- Jie


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


On June 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> ---
> 
> (Updated June 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
> https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> b2a20b7c08fa790da09ba05b725248e42b8d3bc4 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> bc04760c9504ea55cd4bb72c7e9012e43a5911aa 
>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> ---
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script 
> that launches tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

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

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, lines 612-627
> > 
> >
> > Should we add a benchmark here to understand the effect of now having 3 
> > levels of `tokenize`?
> > Is this code in any critical paths?
> 
> Klaus Ma wrote:
> This function is only uesd when parsing resources from string; it only 
> impacts the duration of `make check`. I'll try it and let you known the 
> impact.

Run `time build/src/mesos-test`, the result is almost the same :).


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> 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 587cb68551d438621e215953e89818b623b7f71b 
>   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 46375: Updated MACHINE_UP_HELP's comments.

[Klaus Ma]

> On April 20, 2016, 4:25 a.m., Joseph Wu wrote:
> > Ship It!

@Joseph, would you help to commit it?


- Klaus


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


On April 19, 2016, 3:51 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46375/
> ---
> 
> (Updated April 19, 2016, 3:51 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated MACHINE_UP_HELP's comments.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
> 
> Diff: https://reviews.apache.org/r/46375/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[Qian Zhang]

> On June 11, 2016, 10:31 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 131
> > 
> >
> > I think there is a special case that we need to handle: In the OS using 
> > systemd (e.g., RHEL 7.1), `cpu` and `cpuacct` subsystems are co-mounted, 
> > like this:
> > ```
> > ls -la /sys/fs/cgroup/
> > ...
> > lrwxrwxrwx.  1 root root  11 Dec 28 15:12 cpu -> cpu,cpuacct
> > lrwxrwxrwx.  1 root root  11 Dec 28 15:12 cpuacct -> cpu,cpuacct
> > drwxr-xr-x.  4 root root   0 Jan  2 17:01 cpu,cpuacct
> > ...
> > ```
> > That means in this `hierarchies` hashmap, the values of the two keys 
> > `cpu` and `cpuacct` are same (both of them are 
> > `/sys/fs/cgroup/cpu,cpuacct`), this will cause an issue in 
> > `CgroupsIsolatorProcess::prepare()`: In this method, we will call 
> > `prepareHierarchy()` which will check if the cgroup for the container to be 
> > creatd exists or not, if not, create the cgroup, if yes, return an Error. 
> > For the OS using systemd, I think we will always get the Error since for 
> > both `cpu` and `cpuacct` subsystems, we will try to create cgroup in the 
> > same location, i.e., `/sys/fs/cgroup/cpu,cpuacct/mesos/`.
> > 
> > You can take a look at the following code in the existing 
> > `CgroupsCpushareIsolatorProcess` class for how we handle this case. 
> > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp#L94:L144
> 
> haosdent huang wrote:
> Because we use
> ```
> foreach (const string& hierarchy, info->subsystems.keys()) {
> ```
> this problem would not happen.
> 
> Suppose both `cpu` and `cpuacct` mount to `/sys/fs/cgroup/cpu,cpuacct`. 
> The `info->subsystems.keys()` only have one `/sys/fs/cgroup/cpu,cpuacct` here.

Ha, I got it, `multihashmap::keys()` does the trick which will return a `set`, 
so there will never be two `/sys/fs/cgroup/cpu,cpuacct`, thanks haosdent!


- Qian


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


On April 16, 2016, 6:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated April 16, 2016, 6:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47743: Supported private registry per-container in unified containerizer.

[Gilbert Song]

> On June 12, 2016, 1:05 p.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [47743, 47742, 45952, 48605, 48604, 48570, 45951, 47808, 
> > 47807, 47806, 45950, 45949]
> > 
> > Failed command: ./support/apply-review.sh -n -r 47807
> > 
> > Error:
> > 2016-06-12 20:05:27 URL:https://reviews.apache.org/r/47807/diff/raw/ 
> > [2067/2067] -> "47807.patch" [1]
> > error: patch failed: include/mesos/docker/spec.hpp:76
> > error: include/mesos/docker/spec.hpp: patch does not apply
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/13720/console

Should be ok after discarding two un-used dependencies.


- Gilbert


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


On June 12, 2016, 12:11 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47743/
> ---
> 
> (Updated June 12, 2016, 12:11 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported private registry per-container in unified containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6545a6d29cb91d6cf8919c7796c283084178e499 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
>   src/uri/schemes/docker.hpp 68dbf3ffb20c59c7af9558f5ed0bb5706d8aa057 
> 
> Diff: https://reviews.apache.org/r/47743/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested by mesos-execute.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47806: Add docker config auth protobuf to docker spec.

[Gilbert Song]

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

(Updated June 12, 2016, 6:25 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


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


Repository: mesos


Description
---

Add docker config auth protobuf to docker spec.


Diffs
-

  include/mesos/docker/spec.proto 4f2ff2d7d88ceca2f25864b60a04ff5ec05317ff 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[Qian Zhang]

> On June 2, 2016, 10:58 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 105-106
> > 
> >
> > Since this is a multiple lines code, I think you need to add a newline 
> > after that, please check the following code as a reference:
> > 
> > https://github.com/apache/mesos/blob/0.28.1/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp#L285;L288
> > And you may need to do this change for all other places like this.
> > 
> > And what about just name this variable as `subsystem`?
> 
> haosdent huang wrote:
> For the style, I think both 
> 
> ```
> string subsystemName =
>   strings::remove(type, cgroupsPrefix, strings::Mode::ANY);
> ```
> ```
> string subsystemName = strings::remove(
> type, cgroupsPrefix, strings::Mode::ANY);
> ```
> 
> are match mesos style, refer to the mesos style document 
> https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#continuation
> 
> 
> I think use `subsystem` make bring confusing because it is the name of 
> subsystem, instead of an instance of `Subsystem`.

Sorry, I did not make it clear. My point is we need to change:
```
string subsystemName =
  strings::remove(type, cgroupsPrefix, strings::Mode::ANY);
if (hierarchies.contains(subsystemName)) {
  // Skip when the subsystem exists.
  continue;
}
```
to:
```
string subsystemName =
  strings::remove(type, cgroupsPrefix, strings::Mode::ANY);

if (hierarchies.contains(subsystemName)) {
  // Skip when the subsystem exists.
  continue;
}
```
You can refer to: 
https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#empty-lines
 :-)


- Qian


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


On April 16, 2016, 6:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated April 16, 2016, 6:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Jie Yu]

> On June 12, 2016, 6:08 p.m., Jie Yu wrote:
> > src/uri/fetchers/docker.cpp, lines 647-649
> > 
> >
> > I don't get this part. This is not what you're doing here, right? I 
> > think a request to authorization server will return an 401 and we will  set 
> > auth header and send the request again. Putting the comments here is 
> > confusing. I would simply add a TODO here saying that here we assume the 
> > auth is basic, and we simply include the auth header while sending the 
> > request. It'll be ignored if auth is not required.
> 
> Gilbert Song wrote:
> Step 3 ~ 7 is what I haven't done here.
> 
> >> I think a request to authorization server will return an 401 and we 
> will  set auth header and send the request again.
> The auth server would always return a token, but the token is incorrect 
> (not accessible) if no auth attached or the basic auth is incorrect. We could 
> only know that once the wrong token sent to registry and get an 401. Then we 
> will attach basic auth to request another token again.
> 
> Yeah, a little confusing here. Just want to make it in detail, otherwise 
> people may not understand what this TODO exactly is.
> 
> Gilbert Song wrote:
> Step 3 ~ 7 is what I haven't done here.
> 
> >> I think a request to authorization server will return an 401 and we 
> will set auth header and send the request again.
> 
> The auth server would always return a token, but the token is incorrect 
> (not accessible) if no auth attached or the basic auth is incorrect. We could 
> only know that once the wrong token sent to registry and get an 401. Then we 
> will attach basic auth to request another token again.
> 
> >> Putting the comments here is confusing.
> 
> Yeah, a little confusing here. Just want to make it in detail, otherwise 
> people may not understand what this TODO exactly is.
> 
> Jie Yu wrote:
> Regarding using an incorrect toke, are you sure? I want to confirm this. 
> It sounds unintuitive to me.
> 
> Gilbert Song wrote:
> Yeah, I remember it behaves this way which also sounds weird to me. Just 
> re-confirmed by commandline:
> 
> # no basic auth header attached to authentication server, 401 is from 
> registry because of incorrect token:
> ```
> vagrant@vagrant-ubuntu-trusty-64:~$ curl -s -S -L -D - 
> 'https://auth.docker.io/token?service=registry.docker.io&scope=repository:gilbertsong/inky:pull'
> HTTP/1.1 200 OK
> Content-Type: application/json
> Date: Sun, 12 Jun 2016 19:17:30 GMT
> Content-Length: 1358
> Strict-Transport-Security: max-age=31536000
> 
> 
> {"token":"eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCIsIng1YyI6WyJNSUlDTHpDQ0FkU2dBd0lCQWdJQkFEQUtCZ2dxaGtqT1BRUURBakJHTVVRd1FnWURWUVFERXp0Uk5Gb3pPa2RYTjBrNldGUlFSRHBJVFRSUk9rOVVWRmc2TmtGRlF6cFNUVE5ET2tGU01rTTZUMFkzTnpwQ1ZrVkJPa2xHUlVrNlExazFTekFlRncweE5qQTFNekV5TXpVNE5UZGFGdzB4TnpBMU16RXlNelU0TlRkYU1FWXhSREJDQmdOVkJBTVRPMUV6UzFRNlFqSkpNenBhUjFoT09qSlhXRTA2UTBWWFF6cFVNMHhPT2tvMlYxWTZNbGsyVHpwWlFWbEpPbGhQVTBRNlZFUlJTVG8wVWtwRE1Ga3dFd1lIS29aSXpqMENBUVlJS29aSXpqMERBUWNEUWdBRVo0NkVLV3VKSXhxOThuUC9GWEU3U3VyOXlkZ3c3K2FkcndxeGlxN004VHFUa0N0dzBQZm1SS2VLdExwaXNTRFU4LzZseWZ3QUFwZWh6SHdtWmxZR2dxT0JzakNCcnpBT0JnTlZIUThCQWY4RUJBTUNCNEF3RHdZRFZSMGxCQWd3QmdZRVZSMGxBREJFQmdOVkhRNEVQUVE3VVROTFZEcENNa2t6T2xwSFdFNDZNbGRZVFRwRFJWZERPbFF6VEU0NlNqWlhWam95V1RaUE9sbEJXVWs2V0U5VFJEcFVSRkZKT2pSU1NrTXdSZ1lEVlIwakJEOHdQWUE3VVRSYU16cEhWemRKT2xoVVVFUTZTRTAwVVRwUFZGUllPalpCUlVNNlVrMHpRenBCVWpKRE9rOUdOemM2UWxaRlFUcEpSa1ZKT2tOWk5Vc3dDZ1lJS29aSXpqMEVBd0lEU1FBd1JnSWhBTzYxSWloN1FUcHNTMFFIYUNwTDFZTWNMMnZXZlNydlhHbHpSRDEwN2
 
NRUEFpRUFtZXduelNYRHplRGxqcDc4T1NsTFFzbnROYWM5eHRyYW0xU0kxY0ZXQ2tJPSJdfQ.eyJhY2Nlc3MiOltdLCJhdWQiOiJyZWdpc3RyeS5kb2NrZXIuaW8iLCJleHAiOjE0NjU3NTkzNTAsImlhdCI6MTQ2NTc1OTA1MCwiaXNzIjoiYXV0aC5kb2NrZXIuaW8iLCJqdGkiOiJ4MmlfMkxmOEhoUU9IT1VRajV0SiIsIm5iZiI6MTQ2NTc1OTA1MCwic3ViIjoiIn0.WiGorARqU93j6hCXOgXvx_lt9QyCD59oGOLbmbZox1Cqq44rV13hLeqCCtemaWS3gpwtZ12z9exogJNgRRryfA"}
> 
> vagrant@vagrant-ubuntu-trusty-64:~$ curl -s -S -L -D - -H 'Authorization: 
> Bearer 
> eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCIsIng1YyI6WyJNSUlDTHpDQ0FkU2dBd0lCQWdJQkFEQUtCZ2dxaGtqT1BRUURBakJHTVVRd1FnWURWUVFERXp0Uk5Gb3pPa2RYTjBrNldGUlFSRHBJVFRSUk9rOVVWRmc2TmtGRlF6cFNUVE5ET2tGU01rTTZUMFkzTnpwQ1ZrVkJPa2xHUlVrNlExazFTekFlRncweE5qQTFNekV5TXpVNE5UZGFGdzB4TnpBMU16RXlNelU0TlRkYU1FWXhSREJDQmdOVkJBTVRPMUV6UzFRNlFqSkpNenBhUjFoT09qSlhXRTA2UTBWWFF6cFVNMHhPT2tvMlYxWTZNbGsyVHpwWlFWbEpPbGhQVTBRNlZFUlJTVG8wVWtwRE1Ga3dFd1lIS29aSXpqMENBUVlJS29aSXpqMERBUWNEUWdBRVo0NkVLV3VKSXhxOThuUC9GWEU3U3VyOXlkZ3c3K2FkcndxeGlxN004VHFUa0N0dzBQZm1SS2VLdExwaXNTRFU4LzZseWZ3QUFwZWh6SHdtWmxZR2dxT0JzakNCcnpBT0JnTlZIUThCQWY4RUJBTUNCNEF3RHdZRFZSMGxCQWd3QmdZRVZSMGxBREJFQmdOVkhRNEVQUVE3VVROTFZEcENNa2t6T2xwSFdFNDZNbGRZVFRwRFJWZERPbFF6VEU0NlNqWlhWam95V1RaUE9sbEJXVWs2V0U5VFJEcFVSRkZKT2pSU1NrTXdSZ1lEVlIwakJEOHdQWUE3VVRSYU16cEhWemRKT2xo

Re: Review Request 47396: Added aufs provisioning backend.

[Guangya Liu]

> On 六月 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > 
> >
> > I think that here also needs to be updated to use your new function 
> > `fs::aufs::supported()` to check `aufs`?
> 
> Jie Yu wrote:
> Hum... this looks weird to me. I am wondering if we can make 
> `fs::supported` understand the sutble details related to overlayfs? Can you 
> guys follow up with a patch. Basically, here, it should be `Try check = 
> fs::supported(fsname);` and inside `fs::supported`, we check special case for 
> overlayfs.
> 
> Guangya Liu wrote:
> I'm now thinking that we may not need the API of `fs::aufs::supported`, 
> the reason that we introduced `fs::overlay::supported` is becuase of keyword 
> issue, the overlay fs keyword in `/proc/filesystems` can be `overlay` or 
> `overlayfs`.
> 
> But for `aufs`, I think we do not need to introduce the new API of 
> `fs::aufs::supported` but call `fs::supported` directly, as the API of 
> `fs::aufs::supported` is just a wrapper of `fs::supported`. Comments?
> 
> Jie Yu wrote:
> Yeah, I agree with you Guangya. I think we should make 
> `fs::supported(fsname)` understand both overlayfs and overlay and get rid of 
> all fs::xxx::supported logics. Do you have time to follow up with a patch?

Hi Yu Jie, it may be difficult to make `fs::supported(fsname)` to understand 
both overlayfs and overlay, so my thinking is that we only keep 
`fs::overlay::supported` but remove the `fs::aufs::supported`, comments?


- Guangya


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


On 六月 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> ---
> 
> (Updated 六月 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
> https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> b2a20b7c08fa790da09ba05b725248e42b8d3bc4 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> bc04760c9504ea55cd4bb72c7e9012e43a5911aa 
>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> ---
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script 
> that launches tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 47743: Supported private registry per-container in unified containerizer.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47743, 47742, 45952, 48605, 48604, 48570, 45951, 47808, 
47807, 47806, 45950, 45949]

Failed command: ./support/apply-review.sh -n -r 47807

Error:
2016-06-12 20:05:27 URL:https://reviews.apache.org/r/47807/diff/raw/ 
[2067/2067] -> "47807.patch" [1]
error: patch failed: include/mesos/docker/spec.hpp:76
error: include/mesos/docker/spec.hpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13720/console

- Mesos ReviewBot


On June 12, 2016, 7:11 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47743/
> ---
> 
> (Updated June 12, 2016, 7:11 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Supported private registry per-container in unified containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> 6545a6d29cb91d6cf8919c7796c283084178e499 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
>   src/uri/schemes/docker.hpp 68dbf3ffb20c59c7af9558f5ed0bb5706d8aa057 
> 
> Diff: https://reviews.apache.org/r/47743/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested by mesos-execute.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Gilbert Song]

> On June 12, 2016, 11:08 a.m., Jie Yu wrote:
> > src/uri/fetchers/docker.cpp, lines 647-649
> > 
> >
> > I don't get this part. This is not what you're doing here, right? I 
> > think a request to authorization server will return an 401 and we will  set 
> > auth header and send the request again. Putting the comments here is 
> > confusing. I would simply add a TODO here saying that here we assume the 
> > auth is basic, and we simply include the auth header while sending the 
> > request. It'll be ignored if auth is not required.
> 
> Gilbert Song wrote:
> Step 3 ~ 7 is what I haven't done here.
> 
> >> I think a request to authorization server will return an 401 and we 
> will  set auth header and send the request again.
> The auth server would always return a token, but the token is incorrect 
> (not accessible) if no auth attached or the basic auth is incorrect. We could 
> only know that once the wrong token sent to registry and get an 401. Then we 
> will attach basic auth to request another token again.
> 
> Yeah, a little confusing here. Just want to make it in detail, otherwise 
> people may not understand what this TODO exactly is.
> 
> Gilbert Song wrote:
> Step 3 ~ 7 is what I haven't done here.
> 
> >> I think a request to authorization server will return an 401 and we 
> will set auth header and send the request again.
> 
> The auth server would always return a token, but the token is incorrect 
> (not accessible) if no auth attached or the basic auth is incorrect. We could 
> only know that once the wrong token sent to registry and get an 401. Then we 
> will attach basic auth to request another token again.
> 
> >> Putting the comments here is confusing.
> 
> Yeah, a little confusing here. Just want to make it in detail, otherwise 
> people may not understand what this TODO exactly is.
> 
> Jie Yu wrote:
> Regarding using an incorrect toke, are you sure? I want to confirm this. 
> It sounds unintuitive to me.

Yeah, I remember it behaves this way which also sounds weird to me. Just 
re-confirmed by commandline:

# no basic auth header attached to authentication server, 401 is from registry 
because of incorrect token:
```
vagrant@vagrant-ubuntu-trusty-64:~$ curl -s -S -L -D - 
'https://auth.docker.io/token?service=registry.docker.io&scope=repository:gilbertsong/inky:pull'
HTTP/1.1 200 OK
Content-Type: application/json
Date: Sun, 12 Jun 2016 19:17:30 GMT
Content-Length: 1358
Strict-Transport-Security: max-age=31536000

{"token":"eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCIsIng1YyI6WyJNSUlDTHpDQ0FkU2dBd0lCQWdJQkFEQUtCZ2dxaGtqT1BRUURBakJHTVVRd1FnWURWUVFERXp0Uk5Gb3pPa2RYTjBrNldGUlFSRHBJVFRSUk9rOVVWRmc2TmtGRlF6cFNUVE5ET2tGU01rTTZUMFkzTnpwQ1ZrVkJPa2xHUlVrNlExazFTekFlRncweE5qQTFNekV5TXpVNE5UZGFGdzB4TnpBMU16RXlNelU0TlRkYU1FWXhSREJDQmdOVkJBTVRPMUV6UzFRNlFqSkpNenBhUjFoT09qSlhXRTA2UTBWWFF6cFVNMHhPT2tvMlYxWTZNbGsyVHpwWlFWbEpPbGhQVTBRNlZFUlJTVG8wVWtwRE1Ga3dFd1lIS29aSXpqMENBUVlJS29aSXpqMERBUWNEUWdBRVo0NkVLV3VKSXhxOThuUC9GWEU3U3VyOXlkZ3c3K2FkcndxeGlxN004VHFUa0N0dzBQZm1SS2VLdExwaXNTRFU4LzZseWZ3QUFwZWh6SHdtWmxZR2dxT0JzakNCcnpBT0JnTlZIUThCQWY4RUJBTUNCNEF3RHdZRFZSMGxCQWd3QmdZRVZSMGxBREJFQmdOVkhRNEVQUVE3VVROTFZEcENNa2t6T2xwSFdFNDZNbGRZVFRwRFJWZERPbFF6VEU0NlNqWlhWam95V1RaUE9sbEJXVWs2V0U5VFJEcFVSRkZKT2pSU1NrTXdSZ1lEVlIwakJEOHdQWUE3VVRSYU16cEhWemRKT2xoVVVFUTZTRTAwVVRwUFZGUllPalpCUlVNNlVrMHpRenBCVWpKRE9rOUdOemM2UWxaRlFUcEpSa1ZKT2tOWk5Vc3dDZ1lJS29aSXpqMEVBd0lEU1FBd1JnSWhBTzYxSWloN1FUcHNTMFFIYUNwTDFZTWNMMnZXZlNydlhHbHpSRDEwN2NRUEFp
 
RUFtZXduelNYRHplRGxqcDc4T1NsTFFzbnROYWM5eHRyYW0xU0kxY0ZXQ2tJPSJdfQ.eyJhY2Nlc3MiOltdLCJhdWQiOiJyZWdpc3RyeS5kb2NrZXIuaW8iLCJleHAiOjE0NjU3NTkzNTAsImlhdCI6MTQ2NTc1OTA1MCwiaXNzIjoiYXV0aC5kb2NrZXIuaW8iLCJqdGkiOiJ4MmlfMkxmOEhoUU9IT1VRajV0SiIsIm5iZiI6MTQ2NTc1OTA1MCwic3ViIjoiIn0.WiGorARqU93j6hCXOgXvx_lt9QyCD59oGOLbmbZox1Cqq44rV13hLeqCCtemaWS3gpwtZ12z9exogJNgRRryfA"}

vagrant@vagrant-ubuntu-trusty-64:~$ curl -s -S -L -D - -H 'Authorization: 
Bearer 
eyJhbGciOiJFUzI1NiIsInR5cCI6IkpXVCIsIng1YyI6WyJNSUlDTHpDQ0FkU2dBd0lCQWdJQkFEQUtCZ2dxaGtqT1BRUURBakJHTVVRd1FnWURWUVFERXp0Uk5Gb3pPa2RYTjBrNldGUlFSRHBJVFRSUk9rOVVWRmc2TmtGRlF6cFNUVE5ET2tGU01rTTZUMFkzTnpwQ1ZrVkJPa2xHUlVrNlExazFTekFlRncweE5qQTFNekV5TXpVNE5UZGFGdzB4TnpBMU16RXlNelU0TlRkYU1FWXhSREJDQmdOVkJBTVRPMUV6UzFRNlFqSkpNenBhUjFoT09qSlhXRTA2UTBWWFF6cFVNMHhPT2tvMlYxWTZNbGsyVHpwWlFWbEpPbGhQVTBRNlZFUlJTVG8wVWtwRE1Ga3dFd1lIS29aSXpqMENBUVlJS29aSXpqMERBUWNEUWdBRVo0NkVLV3VKSXhxOThuUC9GWEU3U3VyOXlkZ3c3K2FkcndxeGlxN004VHFUa0N0dzBQZm1SS2VLdExwaXNTRFU4LzZseWZ3QUFwZWh6SHdtWmxZR2dxT0JzakNCcnpBT0JnTlZIUThCQWY4RUJBTUNCNEF3RHdZRFZSMGxCQWd3QmdZRVZSMGxBREJFQmdOVkhRNEVQUVE3VVROTFZEcENNa2t6T2xwSFdFNDZNbGRZVFRwRFJWZERPbFF6VEU0NlNqWlhWam95V1RaUE9sbEJXVWs2V0U5VFJEcFVSRkZKT2pSU1NrTXdSZ1lEVlIwakJEOHdQWUE3VVRSYU16cEhWemRKT2xoVVVFUTZTRTAwVVRwUFZGUllPalpCUlVNNlVrMHpRenBCVWpKRE9rOUdOemM2UWxaRlFUcEpSa1ZKT2tOWk5Vc3dDZ1lJS29aSXpqMEVBd0lEU
 
1FBd1JnS

Re: Review Request 48580: Added test case `MasterAPITest.StartAndStopMaintenance`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48115, 48116, 48257, 48084, 48259, 48260, 48285, 48286, 48580]

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

- Mesos ReviewBot


On June 12, 2016, 5:47 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48580/
> ---
> 
> (Updated June 12, 2016, 5:47 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Vinod Kone.
> 
> 
> Bugs: MESOS-5507
> https://issues.apache.org/jira/browse/MESOS-5507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case `MasterAPITest.StartAndStopMaintenance`.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48580/diff/
> 
> 
> Testing
> ---
> 
> Test by
> 
> ```
> ./bin/mesos-tests.sh --gtest_filter="*APITest*"
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46762: Enabled volume support for mesos-execute.

[Jie Yu]

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




src/cli/execute.cpp (line 239)


Would love to support JSON::Array parsing. Right now, i think in only 
support JSON::Object parsing. See here:

https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/parse.hpp#L80



src/cli/execute.cpp (lines 612 - 613)


Hum, any reason this is MesosContainerizer only?


- Jie Yu


On April 28, 2016, 1:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46762/
> ---
> 
> (Updated April 28, 2016, 1:56 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5265
> https://issues.apache.org/jira/browse/MESOS-5265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled volume support for mesos-execute.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 4711e80314e2fc4cde077bebd9a8167324c3254e 
> 
> Diff: https://reviews.apache.org/r/46762/diff/
> 
> 
> Testing
> ---
> 
> root@mesos002:~/src/mesos/m3/mesos/build# cat /root/test/volume4.json
>   [{
>   "container_path":"\/tmp\/abc1",
>   "mode":"RW",
>   "source":
> {
>   "docker_volume":
> {
>   "driver":"convoy",
>   "name":"dvd1"
> },
> "type":"DOCKER_VOLUME"
> }
> },
> {
>   "container_path":"\/tmp\/abc2",
>   "mode":"RW",
>   "source":
> {
>   "docker_volume":
> {
>   "driver":"convoy",
>   "driver_options":
> {"parameter":[
>   {
> "key":"iops",
> "value":"150"
>   }
> ]},
> "name":"dvd2"
>  },
>  "type":"DOCKER_VOLUME"
> }
> }]
> 
> root@mesos002:~/src/mesos/m3/mesos/build# src/mesos-execute 
> --master=192.168.56.12:5050 --name=test  --command="sleep 100" 
> --volumes=/root/test/volume4.json
> I0428 21:27:52.406168  3775 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-0003'
> Submitted task 'test' to agent '2a69e042-4e51-4cd9-bcab-e303d6b4b1dd-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47743: Supported private registry per-container in unified containerizer.

[Gilbert Song]

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

(Updated June 12, 2016, 12:11 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Supported private registry per-container in unified containerizer.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6545a6d29cb91d6cf8919c7796c283084178e499 
  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
  src/uri/schemes/docker.hpp 68dbf3ffb20c59c7af9558f5ed0bb5706d8aa057 

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


Testing
---

make check

Tested by mesos-execute.


Thanks,

Gilbert Song

Re: Review Request 47742: Modified docker puller interface to pass credential.

[Gilbert Song]

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

(Updated June 12, 2016, 12:11 p.m.)


Review request for mesos, Jie Yu and Timothy Chen.


Repository: mesos


Description
---

Modified docker puller interface to pass credential.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
5f5aaa3688ba11463942f0e08716d6650d7762c3 
  src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
4be26faf0d46ee29fb4169bfe69264c57a6b9fce 
  src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
bbd6005317bed3fff3d86e2527ca3ead839d49e3 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
6545a6d29cb91d6cf8919c7796c283084178e499 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
cd5849bb9cdd12f2240885a0eae90569d2a9502e 
  src/tests/containerizer/provisioner_docker_tests.cpp 
ffe3382da8b1199e257a72ca9034fbccec9494b1 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Gilbert Song]

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

(Updated June 12, 2016, 12:11 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Implemented support for passing agent default docker config.


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
cd5849bb9cdd12f2240885a0eae90569d2a9502e 
  src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check

Tested with mesos-execute, using a private repo from docker hub. Both cases are 
tested:
1. --docker_registry=private_registry
2. private_registry/repo


Thanks,

Gilbert Song

Re: Review Request 36440: Enabled docker volume support for DockerContainerizer.

[Jie Yu]

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




include/mesos/mesos.proto (lines 1781 - 1783)


Can you use [deprecated = true] here?



include/mesos/v1/mesos.proto (lines 1780 - 1781)


Ditto.



src/docker/docker.cpp (lines 559 - 562)


I don't get this check. if we end up here, has_host_path() is false and 
has_source() is false, why we bother having this check?


- Jie Yu


On May 10, 2016, 3:50 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36440/
> ---
> 
> (Updated May 10, 2016, 3:50 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5341
> https://issues.apache.org/jira/browse/MESOS-5341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled docker volume support for DockerContainerizer.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 73d4bc0ec3f34033eddf04aa41893c1fc66933b3 
>   include/mesos/v1/mesos.proto 44b4f8a059f9dfdcbf02f0c30c1b859898c2e617 
>   src/docker/docker.cpp 19cf424dfd5748ae66a7023840aa2b0652e8f2c0 
> 
> Diff: https://reviews.apache.org/r/36440/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 1) Start convoy
> 2) Create a volume named as c1
> root@kvm-002605:~# convoy list
> {
> "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad": {
> "UUID": "f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Name": "c1",
> "Driver": "devicemapper",
> "MountPoint": "",
> "CreatedTime": "Mon May 09 09:38:52 +0800 2016",
> "DriverInfo": {
> "DevID": "1",
> "Device": 
> "/dev/mapper/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Driver": "devicemapper",
> "MountPoint": "",
> "Size": "107374182400"
> },
> "Snapshots": {}
> }
> }
> 3) Update  mesos-execte to set up volume
> index 4711e80..6c712a2 100644
> --- a/src/cli/execute.cpp
> +++ b/src/cli/execute.cpp
> @@ -72,6 +72,8 @@ using mesos::v1::TaskID;
>  using mesos::v1::TaskInfo;
>  using mesos::v1::TaskState;
>  using mesos::v1::TaskStatus;
> +using mesos::v1::Volume;
> +using mesos::v1::Parameters;
>  
>  using mesos::v1::scheduler::Call;
>  using mesos::v1::scheduler::Event;
> @@ -581,6 +583,18 @@ private:
>  
>containerInfo.set_type(ContainerInfo::DOCKER);
>containerInfo.mutable_docker()->set_image(dockerImage.get());
> +  //containerInfo.mutable_docker()->set_volume_driver("convoy");
> +
> +  Volume* volume1 = containerInfo.add_volumes();
> +  //volume1->set_host_path("c1");
> +  volume1->set_container_path("/tmp");
> +  volume1->mutable_source()->set_type(Volume::Source::DOCKER_VOLUME);
> +  volume1->mutable_source()->mutable_docker_volume()->set_driver(
> +  "convoy");
> +  volume1->mutable_source()->mutable_docker_volume()->set_name(
> +  "c1");
> +  volume1->set_mode(Volume::RW);
> +  cout << "Add Voume 1" << endl;
>  
>if (networks.isSome() && !networks->empty()) {
>  vector tokens = strings::tokenize(networks.get(), ",");
> 4) Start master
> ./bin/mesos-master.sh --work_dir=/tmp/mesos/master
> 5) Start agent
> GLOG_v=1 ./bin/mesos-slave.sh --master=10.0.0.65:5050 
> --isolation=filesystem/linux,docker/runtime --containerizers=docker 
> --work_dir=/tmp/mesos --image_providers=docker
> 5) Start mesos-execute
> src/mesos-execute --master=10.0.0.65:5050 --name=test --command="sleep 1" 
> --docker_image=ubuntu:14.04 --containerizer=docker
> I0509 13:14:56.118808  8320 scheduler.cpp:177] Version: 0.29.0
> I0509 13:14:56.120827  8346 scheduler.cpp:479] New master detected at 
> master@10.0.0.65:5050
> Subscribed with ID 'acff7546-e8bf-4fad-a651-307c73297db0-'
> Add Voume 1
> Submitted task 'test' to agent '5454a953-83d0-42a4-b7fe-0825a6667f8c-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_LOST for task 'test'
> 
> 6) Inspect container
> docker inspect 
> ...
> {
> "Name": "c1",
> "Source": 
> "/var/lib/convoy/devicemapper/mounts/f3459d44-bbed-4e01-a7f6-6c6aa4b70cad",
> "Destination": "/tmp",
> "Driver": "convoy",
> "Mode": "rw",
> "RW": true,
> "Propagation": "rprivate"
> }
> ...
> 7) Check sandbox executer log
> I0509 13:14:56.313102  8361 logging.cpp:195] Logging to STDERR
> I0509 13:14:56.315385  8361 pr

Review Request 48605: Added docker spec test for parseUrl.

[Gilbert Song]

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

Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Added docker spec test for parseUrl.


Diffs
-

  src/tests/containerizer/docker_spec_tests.cpp 
944a559bb3a8ef602b4d8d52773afdb8bf8e5bf6 

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


Testing
---

make check


Thanks,

Gilbert Song

Review Request 48604: Implemented docker spec helper to parse URL.

[Gilbert Song]

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

Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


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


Repository: mesos


Description
---

Implemented docker spec helper to parse URL.


Diffs
-

  include/mesos/docker/spec.hpp b37c7731590ec144df720d7b28eeed5db76f750e 
  src/docker/spec.cpp 7270d182b8c3a5d5c34cbda2bd736ba489089720 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 48570: Fixed docker fetcher plugin process indentation.

[Gilbert Song]

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

(Updated June 12, 2016, 12:06 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
Chen.


Repository: mesos


Description
---

Fixed docker fetcher plugin process indentation.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 

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


Testing
---

make check


Thanks,

Gilbert Song

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[haosdent huang]

> On June 11, 2016, 2:31 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 131
> > 
> >
> > I think there is a special case that we need to handle: In the OS using 
> > systemd (e.g., RHEL 7.1), `cpu` and `cpuacct` subsystems are co-mounted, 
> > like this:
> > ```
> > ls -la /sys/fs/cgroup/
> > ...
> > lrwxrwxrwx.  1 root root  11 Dec 28 15:12 cpu -> cpu,cpuacct
> > lrwxrwxrwx.  1 root root  11 Dec 28 15:12 cpuacct -> cpu,cpuacct
> > drwxr-xr-x.  4 root root   0 Jan  2 17:01 cpu,cpuacct
> > ...
> > ```
> > That means in this `hierarchies` hashmap, the values of the two keys 
> > `cpu` and `cpuacct` are same (both of them are 
> > `/sys/fs/cgroup/cpu,cpuacct`), this will cause an issue in 
> > `CgroupsIsolatorProcess::prepare()`: In this method, we will call 
> > `prepareHierarchy()` which will check if the cgroup for the container to be 
> > creatd exists or not, if not, create the cgroup, if yes, return an Error. 
> > For the OS using systemd, I think we will always get the Error since for 
> > both `cpu` and `cpuacct` subsystems, we will try to create cgroup in the 
> > same location, i.e., `/sys/fs/cgroup/cpu,cpuacct/mesos/`.
> > 
> > You can take a look at the following code in the existing 
> > `CgroupsCpushareIsolatorProcess` class for how we handle this case. 
> > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp#L94:L144

Because we use
```
foreach (const string& hierarchy, info->subsystems.keys()) {
```
this problem would not happen.

Suppose both `cpu` and `cpuacct` mount to `/sys/fs/cgroup/cpu,cpuacct`. The 
`info->subsystems.keys()` only have one `/sys/fs/cgroup/cpu,cpuacct` here.


- haosdent


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


On April 16, 2016, 10:14 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated April 16, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46428: Fixed the broken Docker Volume Rootfs Test on Centos7.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 31, 2016, 12:49 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46428/
> ---
> 
> (Updated May 31, 2016, 12:49 a.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5351
> https://issues.apache.org/jira/browse/MESOS-5351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the broken Docker Volume Rootfs Test on Centos7.
> 
> This patch is splitting the test case of DockerVolumeIsolatorTest
> ROOT_CommandTaskNoRootfsWithVolumes to two cases: one for
> absolute path and the other is for relative path. 
> 
> The reason that I need to split is that there is no good way to
> enable one command with shell as false to execute two commands
> to test for both absolute and relative path.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_volume_isolator_tests.cpp 
> c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 
> 
> Diff: https://reviews.apache.org/r/46428/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [==] Running 2 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 2 tests from DockerVolumeIsolatorTest
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithAbsolutePathVolume
>  (1732 ms)
> [ RUN  ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
> [   OK ] 
> DockerVolumeIsolatorTest.ROOT_INTERNET_CURL_CommandTaskRootfsWithRelativeVolume
>  (1708 ms)
> [--] 2 tests from DockerVolumeIsolatorTest (3451 ms total)
>  
> [--] Global test environment tear-down
> [==] 2 tests from 1 test case ran. (3460 ms total)
> [  PASSED  ] 2 tests.
> [root@mesos-24 build]# cat /etc/*release
> CentOS Linux release 7.2.1511 (Core) 
> NAME="CentOS Linux"
> VERSION="7 (Core)"
> ID="centos"
> ID_LIKE="rhel fedora"
> VERSION_ID="7"
> PRETTY_NAME="CentOS Linux 7 (Core)"
> ANSI_COLOR="0;31"
> CPE_NAME="cpe:/o:centos:centos:7"
> HOME_URL="https://www.centos.org/";
> BUG_REPORT_URL="https://bugs.centos.org/";
>  
> CENTOS_MANTISBT_PROJECT="CentOS-7"
> CENTOS_MANTISBT_PROJECT_VERSION="7"
> REDHAT_SUPPORT_PRODUCT="centos"
> REDHAT_SUPPORT_PRODUCT_VERSION="7"
>  
> CentOS Linux release 7.2.1511 (Core) 
> CentOS Linux release 7.2.1511 (Core) 
> [root@mesos-24 build]#
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Jie Yu]

> On June 12, 2016, 6:08 p.m., Jie Yu wrote:
> > src/uri/fetchers/docker.cpp, lines 647-649
> > 
> >
> > I don't get this part. This is not what you're doing here, right? I 
> > think a request to authorization server will return an 401 and we will  set 
> > auth header and send the request again. Putting the comments here is 
> > confusing. I would simply add a TODO here saying that here we assume the 
> > auth is basic, and we simply include the auth header while sending the 
> > request. It'll be ignored if auth is not required.
> 
> Gilbert Song wrote:
> Step 3 ~ 7 is what I haven't done here.
> 
> >> I think a request to authorization server will return an 401 and we 
> will  set auth header and send the request again.
> The auth server would always return a token, but the token is incorrect 
> (not accessible) if no auth attached or the basic auth is incorrect. We could 
> only know that once the wrong token sent to registry and get an 401. Then we 
> will attach basic auth to request another token again.
> 
> Yeah, a little confusing here. Just want to make it in detail, otherwise 
> people may not understand what this TODO exactly is.
> 
> Gilbert Song wrote:
> Step 3 ~ 7 is what I haven't done here.
> 
> >> I think a request to authorization server will return an 401 and we 
> will set auth header and send the request again.
> 
> The auth server would always return a token, but the token is incorrect 
> (not accessible) if no auth attached or the basic auth is incorrect. We could 
> only know that once the wrong token sent to registry and get an 401. Then we 
> will attach basic auth to request another token again.
> 
> >> Putting the comments here is confusing.
> 
> Yeah, a little confusing here. Just want to make it in detail, otherwise 
> people may not understand what this TODO exactly is.

Regarding using an incorrect toke, are you sure? I want to confirm this. It 
sounds unintuitive to me.


- Jie


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


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45952/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented support for passing agent default docker config.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> cd5849bb9cdd12f2240885a0eae90569d2a9502e 
>   src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/45952/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with mesos-execute, using a private repo from docker hub. Both cases 
> are tested:
> 1. --docker_registry=private_registry
> 2. private_registry/repo
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47511: Added documentation for `docker/volume` isolator.

[Jie Yu]

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




docs/docker-volume-isolator.md (line 2)


I would rename the filename to be `docker-volume.md`.

I would rename the title to be `Docker Volume Support in Mesos 
Containerizer`



docs/docker-volume-isolator.md (line 6)


Please adjust the title here as well.



docs/docker-volume-isolator.md (lines 8 - 10)


```
Mesos 1.0 adds the docker volume support for MesosContainerizer (a.k.a., 
the unified containerizer) by introducing a new isolator, called 
`docker/volume` isolator.
```



docs/docker-volume-isolator.md (line 12)


s/of docker volume support//



docs/docker-volume-isolator.md (line 13)


s/of all the involved components//



docs/docker-volume-isolator.md (line 34)


have a link to the persistent volume doc.

ALso, i would consistently use `Mesos` (not mesos) in this doc. Can you do 
a sweep to fix all of them?



docs/docker-volume-isolator.md (line 36)


I would say:
```
The external storage is needed for applications that need much larger 
storage (e.g., databases).
```



docs/docker-volume-isolator.md (line 39)


can be integrated with



docs/docker-volume-isolator.md (lines 39 - 40)


This is the 'motivation' section. I would focus on why we need docker 
volume support. You mentioned local disk size limit, which is very good.

I think you should also mention that the other motivation is that we want 
Mesos to be able to integrate with external storage providers like EBS, GCE 
volume, etc. so that the data persisted is beyond the lifetime of a single host.

And then, you should have a paragraph exlaining why we choose Docker volume 
plugin API.



docs/docker-volume-isolator.md (lines 42 - 45)


I would move this to the following section (i.e., how does it work).



docs/docker-volume-isolator.md (line 49)


s/Engine//



docs/docker-volume-isolator.md (line 67)


Have a link to below (i.e., framework API)



docs/docker-volume-isolator.md (line 95)


Please use `Docker` (not docker) consistently in this doc.



docs/docker-volume-isolator.md (line 105)


by frameworks to specify the docker volumes



docs/docker-volume-isolator.md (lines 125 - 127)


two space indentation. I would put each flag in its own line.



docs/docker-volume-isolator.md (line 151)


can you kill extra lines here to make it compact



docs/docker-volume-isolator.md (line 181)


WHy operator?



docs/docker-volume-isolator.md (line 184)


WHy operator?



docs/docker-volume-isolator.md (line 225)


that want to

need to set



docs/docker-volume-isolator.md (lines 233 - 251)


Please make it a JSON (adding quotes for keys) format.



docs/docker-volume-isolator.md (lines 305 - 306)


It is not recommended to use the same Docker volume from both 
DockerContainerizer and MesosContainerizer...


- Jie Yu


On May 31, 2016, 12:46 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47511/
> ---
> 
> (Updated May 31, 2016, 12:46 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-5216
> https://issues.apache.org/jira/browse/MESOS-5216
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `docker/volume` isolator.
> 
> 
> Diffs
> -
> 
>   docs/docker-volume-isolator.md PRE-CREATION 
>   docs/images/docker-volume-isolator.png PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47511/diff/
> 
> 
> Testing
> ---
> 
> You can review the document h

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[haosdent huang]

> On June 2, 2016, 2:58 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 97
> > 
> >
> > Why make it a static variable? I think 
> > `CgroupsIsolatorProcess::create()` will not be called more than once, right?

Nice catch, it only called when initialzation. Let me remvoe static.


> On June 2, 2016, 2:58 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 105-106
> > 
> >
> > Since this is a multiple lines code, I think you need to add a newline 
> > after that, please check the following code as a reference:
> > 
> > https://github.com/apache/mesos/blob/0.28.1/src/slave/containerizer/mesos/isolators/network/port_mapping.cpp#L285;L288
> > And you may need to do this change for all other places like this.
> > 
> > And what about just name this variable as `subsystem`?

For the style, I think both 

```
string subsystemName =
  strings::remove(type, cgroupsPrefix, strings::Mode::ANY);
```
```
string subsystemName = strings::remove(
type, cgroupsPrefix, strings::Mode::ANY);
```

are match mesos style, refer to the mesos style document 
https://github.com/apache/mesos/blob/master/docs/c%2B%2B-style-guide.md#continuation


I think use `subsystem` make bring confusing because it is the name of 
subsystem, instead of an instance of `Subsystem`.


> On June 2, 2016, 2:58 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 106
> > 
> >
> > I would suggest to use `string::PREFIX` since we are sure `cgroups/` is 
> > the prefix.

Make sense, let me use PREFIX.


- haosdent


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


On April 16, 2016, 10:14 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated April 16, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Gilbert Song]

> On June 12, 2016, 11:08 a.m., Jie Yu wrote:
> > src/uri/fetchers/docker.cpp, lines 647-649
> > 
> >
> > I don't get this part. This is not what you're doing here, right? I 
> > think a request to authorization server will return an 401 and we will  set 
> > auth header and send the request again. Putting the comments here is 
> > confusing. I would simply add a TODO here saying that here we assume the 
> > auth is basic, and we simply include the auth header while sending the 
> > request. It'll be ignored if auth is not required.
> 
> Gilbert Song wrote:
> Step 3 ~ 7 is what I haven't done here.
> 
> >> I think a request to authorization server will return an 401 and we 
> will  set auth header and send the request again.
> The auth server would always return a token, but the token is incorrect 
> (not accessible) if no auth attached or the basic auth is incorrect. We could 
> only know that once the wrong token sent to registry and get an 401. Then we 
> will attach basic auth to request another token again.
> 
> Yeah, a little confusing here. Just want to make it in detail, otherwise 
> people may not understand what this TODO exactly is.

Step 3 ~ 7 is what I haven't done here.

>> I think a request to authorization server will return an 401 and we will set 
>> auth header and send the request again.

The auth server would always return a token, but the token is incorrect (not 
accessible) if no auth attached or the basic auth is incorrect. We could only 
know that once the wrong token sent to registry and get an 401. Then we will 
attach basic auth to request another token again.

>> Putting the comments here is confusing.

Yeah, a little confusing here. Just want to make it in detail, otherwise people 
may not understand what this TODO exactly is.


- Gilbert


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


On June 10, 2016, 5:42 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45952/
> ---
> 
> (Updated June 10, 2016, 5:42 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented support for passing agent default docker config.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> cd5849bb9cdd12f2240885a0eae90569d2a9502e 
>   src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/45952/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with mesos-execute, using a private repo from docker hub. Both cases 
> are tested:
> 1. --docker_registry=private_registry
> 2. private_registry/repo
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48603: Implemented v1::agent::Call::GET_METRICS.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48583, 48584, 48585, 48587, 48588, 48595, 48596, 48597, 
48600, 48601, 48602, 48603]

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

- Mesos ReviewBot


On June 12, 2016, 6:04 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48603/
> ---
> 
> (Updated June 12, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
> Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5484
> https://issues.apache.org/jira/browse/MESOS-5484
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented v1::agent::Call::GET_METRICS.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
>   src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
>   src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
>   src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 
> 
> Diff: https://reviews.apache.org/r/48603/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Gilbert Song]

> On June 12, 2016, 11:08 a.m., Jie Yu wrote:
> > src/uri/fetchers/docker.cpp, lines 647-649
> > 
> >
> > I don't get this part. This is not what you're doing here, right? I 
> > think a request to authorization server will return an 401 and we will  set 
> > auth header and send the request again. Putting the comments here is 
> > confusing. I would simply add a TODO here saying that here we assume the 
> > auth is basic, and we simply include the auth header while sending the 
> > request. It'll be ignored if auth is not required.

Step 3 ~ 7 is what I haven't done here.

>> I think a request to authorization server will return an 401 and we will  
>> set auth header and send the request again.
The auth server would always return a token, but the token is incorrect (not 
accessible) if no auth attached or the basic auth is incorrect. We could only 
know that once the wrong token sent to registry and get an 401. Then we will 
attach basic auth to request another token again.

Yeah, a little confusing here. Just want to make it in detail, otherwise people 
may not understand what this TODO exactly is.


- Gilbert


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


On June 10, 2016, 5:42 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45952/
> ---
> 
> (Updated June 10, 2016, 5:42 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented support for passing agent default docker config.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> cd5849bb9cdd12f2240885a0eae90569d2a9502e 
>   src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/45952/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with mesos-execute, using a private repo from docker hub. Both cases 
> are tested:
> 1. --docker_registry=private_registry
> 2. private_registry/repo
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 47396: Added aufs provisioning backend.

[Jie Yu]

> On June 7, 2016, 2:09 a.m., Guangya Liu wrote:
> > src/tests/environment.cpp, lines 492-495
> > 
> >
> > I think that here also needs to be updated to use your new function 
> > `fs::aufs::supported()` to check `aufs`?
> 
> Jie Yu wrote:
> Hum... this looks weird to me. I am wondering if we can make 
> `fs::supported` understand the sutble details related to overlayfs? Can you 
> guys follow up with a patch. Basically, here, it should be `Try check = 
> fs::supported(fsname);` and inside `fs::supported`, we check special case for 
> overlayfs.
> 
> Guangya Liu wrote:
> I'm now thinking that we may not need the API of `fs::aufs::supported`, 
> the reason that we introduced `fs::overlay::supported` is becuase of keyword 
> issue, the overlay fs keyword in `/proc/filesystems` can be `overlay` or 
> `overlayfs`.
> 
> But for `aufs`, I think we do not need to introduce the new API of 
> `fs::aufs::supported` but call `fs::supported` directly, as the API of 
> `fs::aufs::supported` is just a wrapper of `fs::supported`. Comments?

Yeah, I agree with you Guangya. I think we should make `fs::supported(fsname)` 
understand both overlayfs and overlay and get rid of all fs::xxx::supported 
logics. Do you have time to follow up with a patch?


- Jie


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


On June 6, 2016, 11:57 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47396/
> ---
> 
> (Updated June 6, 2016, 11:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, and Jie Yu.
> 
> 
> Bugs: MESOS-4672
> https://issues.apache.org/jira/browse/MESOS-4672
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added aufs provisioning backend.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a08ea407d631f6ae56ac36b122bfdf0e849e8b56 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> b2a20b7c08fa790da09ba05b725248e42b8d3bc4 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp PRE-CREATION 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> bc04760c9504ea55cd4bb72c7e9012e43a5911aa 
>   src/tests/environment.cpp 849e9ce05864f6ec1a736dfc1a7a31d2364c84bf 
> 
> Diff: https://reviews.apache.org/r/47396/diff/
> 
> 
> Testing
> ---
> 
> - Make check on ubuntu 14.04 64bit
> - Test manually: start slave with aufs backend, and write a simple script 
> that launches tasks with alpine/ubuntu images
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 45952: Implemented support for passing agent default docker config.

[Jie Yu]

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


Fix it, then Ship it!





src/uri/fetchers/docker.cpp (lines 645 - 647)


I don't get this part. This is not what you're doing here, right? I think a 
request to authorization server will return an 401 and we will  set auth header 
and send the request again. Putting the comments here is confusing. I would 
simply add a TODO here saying that here we assume the auth is basic, and we 
simply include the auth header while sending the request. It'll be ignored if 
auth is not required.



src/uri/fetchers/docker.cpp (line 664)


This check is redundant, isn't it? I would just remove it.



src/uri/fetchers/docker.cpp (line 693)


I would kill this line.


- Jie Yu


On June 11, 2016, 12:42 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45952/
> ---
> 
> (Updated June 11, 2016, 12:42 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy 
> Chen.
> 
> 
> Bugs: MESOS-4938
> https://issues.apache.org/jira/browse/MESOS-4938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented support for passing agent default docker config.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> cd5849bb9cdd12f2240885a0eae90569d2a9502e 
>   src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 
>   src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 
> 
> Diff: https://reviews.apache.org/r/45952/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Tested with mesos-execute, using a private repo from docker hub. Both cases 
> are tested:
> 1. --docker_registry=private_registry
> 2. private_registry/repo
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 48603: Implemented v1::agent::Call::GET_METRICS.

[haosdent huang]

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

(Updated June 12, 2016, 6:04 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::agent::Call::GET_METRICS.


Diffs (updated)
-

  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48602: Implemented v1::master::Call::GET_METRICS.

[haosdent huang]

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

(Updated June 12, 2016, 6:04 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_METRICS.


Diffs (updated)
-

  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

[haosdent huang]

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

(Updated June 12, 2016, 6:03 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48600: Used `DurationInfo` as `timeout` type in `GetMetrics` operation APIs.

[haosdent huang]

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

(Updated June 12, 2016, 6:03 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Used `DurationInfo` as `timeout` type in `GetMetrics` operation APIs.


Diffs (updated)
-

  include/mesos/master/master.proto PRE-CREATION 
  include/mesos/slave/slave.proto PRE-CREATION 
  include/mesos/v1/agent.proto b9b1680afec4e73c31a9e0aeb74153d129c54ee6 
  include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48597: Implemented v1::agent::Call::SET_LOGGING_LEVEL.

[haosdent huang]

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

(Updated June 12, 2016, 6:02 p.m.)


Review request for mesos, Abhishek Dasgupta, Anand Mazumdar, zhou xing, Jay 
Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::agent::Call::SET_LOGGING_LEVEL.


Diffs (updated)
-

  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48595: Exposed the logging process `PID` in libprocess.

[haosdent huang]

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

(Updated June 12, 2016, 6:02 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Exposed the logging process `PID` in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/logging.hpp 
651cb3736705124bec9e4366c0e7adc5d5b488f0 
  3rdparty/libprocess/include/process/process.hpp 
646059e2740a32dd9c483dfa56c71dc46727862c 
  3rdparty/libprocess/src/logging.cpp 222449b904268a47868be374bf8202aafe6516fe 
  3rdparty/libprocess/src/process.cpp b4cdba6b0cd4f8f373f37118cd2e9d4955f2425a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48588: Devolved v1 operator protos to unversioned operator protos in Agent.

[haosdent huang]

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

(Updated June 12, 2016, 6:01 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Agent.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
  src/slave/validation.hpp 59b12f0109f6fa4c570f02d90834502a8edebe45 
  src/slave/validation.cpp 189d10caf4517a1aff62b523d8eb99d9d1ca4b6a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48596: Implemented v1::master::Call::SET_LOGGING_LEVEL.

[haosdent huang]

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

(Updated June 12, 2016, 6:02 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::SET_LOGGING_LEVEL.


Diffs (updated)
-

  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48587: Added unversioned protos for agent API.

[haosdent huang]

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

(Updated June 12, 2016, 6:01 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added unversioned protos for agent API.


Diffs (updated)
-

  include/mesos/slave/slave.hpp PRE-CREATION 
  include/mesos/slave/slave.proto PRE-CREATION 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48583: Added `VersionInfo`, `Flag`, `Metrics` to unversioned mesos protos.

[haosdent huang]

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

(Updated June 12, 2016, 6 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

We add `VersionInfo`, `Flag`, `Metrics` to v1 mesos protos in 549a0340,
but didn't add them to unversioned mesos protos as well. This change
attempt to fix this.


Diffs (updated)
-

  include/mesos/mesos.proto dbffb16f4317df786ee38999a49e4e89733ff9ff 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48585: Devolved v1 operator protos to unversioned operator protos in Master.

[haosdent huang]

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

(Updated June 12, 2016, 6:01 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Devolved v1 operator protos to unversioned operator protos in Master.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/master/validation.hpp e1271bbaebaac52a078eedfdc2299c7c6f4474f8 
  src/master/validation.cpp 7b9c2281b2ab1295211f0dd385cd77947fbd63be 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48584: Added unversioned protos for master API.

[haosdent huang]

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

(Updated June 12, 2016, 6:01 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added unversioned protos for master API.


Diffs (updated)
-

  include/mesos/master/master.hpp PRE-CREATION 
  include/mesos/master/master.proto PRE-CREATION 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[haosdent huang]

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

(Updated June 12, 2016, 5:57 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_MAINTENANCE_STATUS.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[haosdent huang]

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

(Updated June 12, 2016, 5:52 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_MAINTENANCE_STATUS.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[haosdent huang]

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

(Updated June 12, 2016, 5:52 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48260: Added test case `MasterAPITest.GetMaintenanceStatus`.

[haosdent huang]

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

(Updated June 12, 2016, 5:52 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.GetMaintenanceStatus`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

Test in CentOS 7.2

```
./bin/mesos-tests.sh --gtest_filter="*MasterAPITest*"
```


Thanks,

haosdent huang

Re: Review Request 48580: Added test case `MasterAPITest.StartAndStopMaintenance`.

[haosdent huang]

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

(Updated June 12, 2016, 5:47 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.StartAndStopMaintenance`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

Test by

```
./bin/mesos-tests.sh --gtest_filter="*APITest*"
```


Thanks,

haosdent huang

Re: Review Request 48286: Implemented STOP_MAINTENANCE Call in v1 master API.

[haosdent huang]

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

(Updated June 12, 2016, 5:47 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented STOP_MAINTENANCE Call in v1 master API.


Diffs (updated)
-

  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48259: Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.

[haosdent huang]

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

(Updated June 12, 2016, 5:46 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.UpdateAndGetMaintenanceSchedule`.


Diffs (updated)
-

  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48285: Implemented START_MAINTENANCE Call in v1 master API.

[haosdent huang]

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

(Updated June 12, 2016, 5:47 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented START_MAINTENANCE Call in v1 master API.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[haosdent huang]

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

(Updated June 12, 2016, 5:46 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_MAINTENANCE_STATUS.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48260: Added test case `MasterAPITest.GetMaintenanceStatus`.

[haosdent huang]

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

(Updated June 12, 2016, 5:47 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added test case `MasterAPITest.GetMaintenanceStatus`.


Diffs (updated)
-

  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---

Test in CentOS 7.2

```
./bin/mesos-tests.sh --gtest_filter="*MasterAPITest*"
```


Thanks,

haosdent huang

Re: Review Request 48257: Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.

[haosdent huang]

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

(Updated June 12, 2016, 5:46 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented GET_MAINTENANCE_SCHEDULE Call in v1 master API.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48115: Added maintenance V1 header.

[haosdent huang]

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

(Updated June 12, 2016, 5:46 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added maintenance V1 header.


Diffs (updated)
-

  include/mesos/v1/maintenance/maintenance.hpp PRE-CREATION 
  src/Makefile.am b656702d918e747cbd4b3d8f2c4257f61c83b385 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48116: Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.

[haosdent huang]

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

(Updated June 12, 2016, 5:46 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented UPDATE_MAINTENANCE_SCHEDULE Call in v1 master API.


Diffs (updated)
-

  src/internal/devolve.hpp 00842bb0de1dd587f2b47c79f17c0e7bd7f51189 
  src/internal/devolve.cpp 4233246c6838f85189f1b4c7e66d2bc0a3bf5408 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48084: Implemented v1::master::Call::GET_MAINTENANCE_STATUS.

[haosdent huang]

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

(Updated June 12, 2016, 5:44 p.m.)


Review request for mesos, Joseph Wu and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_MAINTENANCE_STATUS.


Diffs (updated)
-

  src/internal/evolve.hpp 66a3deaa94939ad2233d944ba35ac7e5cbe682e7 
  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp cd089cf6afd654b07608548f441a0e225df6425e 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---


Thanks,

haosdent huang

Review Request 48603: Implemented v1::agent::Call::GET_METRICS.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented v1::agent::Call::GET_METRICS.


Diffs
-

  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/slave/http.cpp 67ad7a92195abc266fd82fa5dc4c71f24a02aef3 
  src/slave/slave.hpp da8b0d78af8611d22d66935e908e14977b242586 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On June 12, 2016, 4:05 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48599/
> ---
> 
> (Updated June 12, 2016, 4:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Bugs: MESOS-5604
> https://issues.apache.org/jira/browse/MESOS-5604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via \`.then(\[=\]\`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
> 
> Diff: https://reviews.apache.org/r/48599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 48602: Implemented v1::master::Call::GET_METRICS.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Implemented v1::master::Call::GET_METRICS.


Diffs
-

  src/internal/evolve.cpp 7f16cbda7da6c838648cca909368973e7298730b 
  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 
  src/tests/api_tests.cpp 3a482ca2a640b3f3e3b08a80ac84068d7e9ff8b0 

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


Testing
---


Thanks,

haosdent huang

Review Request 48600: Used `DurationInfo` as `timeout` type in `GetMetrics` operation APIs.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Used `DurationInfo` as `timeout` type in `GetMetrics` operation APIs.


Diffs
-

  include/mesos/master/master.proto PRE-CREATION 
  include/mesos/slave/slave.proto PRE-CREATION 
  include/mesos/v1/agent.proto b9b1680afec4e73c31a9e0aeb74153d129c54ee6 
  include/mesos/v1/master.proto 7b07b90557e0202cabc8f6164582a058631ab0e8 

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


Testing
---


Thanks,

haosdent huang

Review Request 48601: Exposed metrics information via `process::metrics::snapshot`.

[haosdent huang]

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

Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


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


Repository: mesos


Description
---

Exposed metrics information via `process::metrics::snapshot`.


Diffs
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
25e8fcaeb9c71eaa43096165745e79c8198c139a 
  3rdparty/libprocess/src/metrics/metrics.cpp 
be03406c58439b7ab7c1ba12c19dd9f30aff109e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48595: Exposed the logging process `PID` in libprocess.

[haosdent huang]

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

(Updated June 12, 2016, 5:19 p.m.)


Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.


Changes
---

Fix typo.


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


Repository: mesos


Description
---

Exposed the logging process `PID` in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/logging.hpp 
651cb3736705124bec9e4366c0e7adc5d5b488f0 
  3rdparty/libprocess/include/process/process.hpp 
646059e2740a32dd9c483dfa56c71dc46727862c 
  3rdparty/libprocess/src/logging.cpp 222449b904268a47868be374bf8202aafe6516fe 
  3rdparty/libprocess/src/process.cpp b4cdba6b0cd4f8f373f37118cd2e9d4955f2425a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48598, 48599]

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

- Mesos ReviewBot


On June 12, 2016, 4:05 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48599/
> ---
> 
> (Updated June 12, 2016, 4:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Bugs: MESOS-5604
> https://issues.apache.org/jira/browse/MESOS-5604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via \`.then(\[=\]\`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
> 
> Diff: https://reviews.apache.org/r/48599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On June 12, 2016, 9:05 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48599/
> ---
> 
> (Updated June 12, 2016, 9:05 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Bugs: MESOS-5604
> https://issues.apache.org/jira/browse/MESOS-5604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via \`.then(\[=\]\`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
> 
> Diff: https://reviews.apache.org/r/48599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On June 12, 2016, 4:05 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48599/
> ---
> 
> (Updated June 12, 2016, 4:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Bugs: MESOS-5604
> https://issues.apache.org/jira/browse/MESOS-5604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via \`.then(\[=\]\`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
> 
> Diff: https://reviews.apache.org/r/48599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[haosdent huang]

> On June 12, 2016, 3:18 p.m., haosdent huang wrote:
> > `src/files/files.cpp` have similar problems.
> > 
> > In discription, `differen` shoud be `different`?
> 
> Joerg Schad wrote:
> If you mean 
> https://github.com/apache/mesos/blob/master/src/files/files.cpp#L591 that is 
> on my investigation list,
> all other occurences in that file shoud have been fixed with 
> https://github.com/apache/mesos/commit/97124ac72bbc42f809b800fe2da383dc43b1b34e,
>  which was committed this morning.
> Thanks for pointing out the typo!

Oh, sorry, my bad, I have not yet updated my local repo.


- haosdent


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


On June 12, 2016, 4:05 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48599/
> ---
> 
> (Updated June 12, 2016, 4:05 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Bugs: MESOS-5604
> https://issues.apache.org/jira/browse/MESOS-5604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via \`.then(\[=\]\`,
> which potentially executes the continuation on a different
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
> 
> Diff: https://reviews.apache.org/r/48599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[Joerg Schad]

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

(Updated June 12, 2016, 4:05 p.m.)


Review request for mesos, Jie Yu and Joseph Wu.


Changes
---

Fixed typo in description.


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


Repository: mesos


Description (updated)
---

Previously the continuation followed via \`.then(\[=\]\`,
which potentially executes the continuation on a different
process. This patch fixes this behavior (it should
run on the same process) and avoids potential race
conditions.


Diffs
-

  src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[Joerg Schad]

> On June 12, 2016, 3:18 p.m., haosdent huang wrote:
> > `src/files/files.cpp` have similar problems.
> > 
> > In discription, `differen` shoud be `different`?

If you mean 
https://github.com/apache/mesos/blob/master/src/files/files.cpp#L591 that is on 
my investigation list,
all other occurences in that file shoud have been fixed with 
https://github.com/apache/mesos/commit/97124ac72bbc42f809b800fe2da383dc43b1b34e,
 which was committed this morning.
Thanks for pointing out the typo!


- Joerg


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


On June 12, 2016, 3:09 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48599/
> ---
> 
> (Updated June 12, 2016, 3:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Bugs: MESOS-5604
> https://issues.apache.org/jira/browse/MESOS-5604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via \`.then(\[=\]\`,
> which potentially executes the continuation on a differen
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
> 
> Diff: https://reviews.apache.org/r/48599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48598: Added changes to upgrades.md.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [48598]

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

- Mesos ReviewBot


On June 12, 2016, 2:13 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48598/
> ---
> 
> (Updated June 12, 2016, 2:13 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos 1.0 adds a number of changes which
> will impact operators. This patch adds two
> changes (see reviews 48381 and 48380) to
> the upgrades.md.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md 3d31d15ea3327d1d0734b0d79be3cc86a66ab1f4 
> 
> Diff: https://reviews.apache.org/r/48598/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 48599: Fixed continuation logic in docker.cpp.

[haosdent huang]

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



`src/files/files.cpp` have similar problems.

In discription, `differen` shoud be `different`?

- haosdent huang


On June 12, 2016, 3:09 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48599/
> ---
> 
> (Updated June 12, 2016, 3:09 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joseph Wu.
> 
> 
> Bugs: MESOS-5604
> https://issues.apache.org/jira/browse/MESOS-5604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously the continuation followed via \`.then(\[=\]\`,
> which potentially executes the continuation on a differen
> process. This patch fixes this behavior (it should
> run on the same process) and avoids potential race
> conditions.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 
> 
> Diff: https://reviews.apache.org/r/48599/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 48599: Fixed continuation logic in docker.cpp.

[Joerg Schad]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Previously the continuation followed via \`.then(\[=\]\`,
which potentially executes the continuation on a differen
process. This patch fixes this behavior (it should
run on the same process) and avoids potential race
conditions.


Diffs
-

  src/slave/containerizer/docker.cpp 1af5b451922e1eeb0af025f29c53a85ab9deec3b 

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


Testing
---

make check


Thanks,

Joerg Schad

Re: Review Request 46158: Completed implementation of the cgroups unified isolator.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 351 - 363)


Why do we prepare hierarchy here? I think preparing hierarchy for a 
specific container is a low-level work which should be handled in 
`Subsystem::prepare()` method rather than here. And in that way, we only need 
one `for` loop (the following one) rather than 2 here.



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 388 - 390)


I am wondering if we really need this method. I think If we introduce a 
static member `hashmap> infos` in the class 
`Subsystem` as I mentioned in this comment 
https://reviews.apache.org/r/46158/#comment201273, then we may not need this 
`CgroupsIsolatorProcess::_prepare()` method, because in 
`CgroupsIsolatorProcess::watch()`, we can directly watch the `limitation` of 
the Info object in that static `infos` hashmap, and in each Subsystem (e.g., in 
`MemorySubsystem::oom()`), we can directly trigger this `limiation`.



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 428 - 445)


Again I think assigning container to its own cgroup should be handled in 
`Subsystem::isolate()` method. In that way, we only need one `for` loop (the 
following one) rather than 2 here.



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 592 - 600)


Again I think destroying container's cgroup should be handled in 
`Subsystem::cleanup()` method.


- Qian Zhang


On April 16, 2016, 6:14 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46158/
> ---
> 
> (Updated April 16, 2016, 6:14 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, and 
> Kevin Klues.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Completed implementation of the cgroups unified isolator.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 48381: Updated CHANGELOG for 'work_dir' flag changes.

[Joerg Schad]

> On June 8, 2016, 1:10 p.m., Joerg Schad wrote:
> > Should we add this as well to upgrades.md?
> 
> Greg Mann wrote:
> Yep, great idea!

https://reviews.apache.org/r/48598/


- Joerg


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


On June 9, 2016, 6:16 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48381/
> ---
> 
> (Updated June 9, 2016, 6:16 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CHANGELOG for 'work_dir' flag changes.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 0513c1594098c8afdb7085ecab5edf07f93f7d0e 
> 
> Diff: https://reviews.apache.org/r/48381/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>