Re: Review Request 35766: Added --with-libnl=DIR configure flag.

[Kapil Arya]

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

(Updated June 23, 2015, 2:18 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Added LDFLAGS.


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


Repository: mesos


Description
---

This allows us to specify alternate libnl installation location.


Diffs (updated)
-

  configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 

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


Testing
---

Tested with: 
```
../configure --with-network-isolator --with-libnl=/usr/local 
LDFLAGS="-L/usr/local/lib64"
```

On systems that do not use `lib64` for 64-bit libraries, the LDFLAGS options 
isn't needed.


Thanks,

Kapil Arya

Review Request 35766: Added --with-libnl=DIR configure flag.

[Kapil Arya]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

This allows us to specify alternate libnl installation location.


Diffs
-

  configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 

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


Testing
---

Tested with: 
```
../configure --with-network-isolator --with-libnl=/usr/local 
LDFLAGS="-L/usr/local/lib64"
```

On systems that do not use `lib64` for 64-bit libraries, the LDFLAGS options 
isn't needed.


Thanks,

Kapil Arya

Re: Review Request 35763: Document workaround for LIBNL_CFLAGS path

[Marco Massenzio]

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


Can you please add one of the committers as a reviewer?

- Marco Massenzio


On June 23, 2015, 2:39 a.m., Roger Ignazio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35763/
> ---
> 
> (Updated June 23, 2015, 2:39 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1856
> https://issues.apache.org/jira/browse/MESOS-1856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit documents a workaround to MESOS-1856, telling the user that
> they will need to override LIBNL_CFLAGS at build time if libnl3 was
> *not* compiled and installed with `--prefix=/usr`.
> 
> 
> Diffs
> -
> 
>   docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 
> 
> Diff: https://reviews.apache.org/r/35763/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Roger Ignazio
> 
>

Re: Review Request 35763: Document workaround for LIBNL_CFLAGS path

[Marco Massenzio]

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

Ship it!


Thanks for this.

A couple of minor formatting nits.


docs/network-monitoring.md (lines 44 - 48)


Were you after the `blockquote` effect here, or was this just an artifact 
of copy & paste?

I would prefer that you remove the blockquote, and have the code surrounded 
by the ``` markers.



docs/network-monitoring.md (line 48)


please format as a markdown URL:

please see [MESOS-1856](https://issues.apache.org/jira/browse/MESOS-1856)

to be consistent with the rest of the document.


- Marco Massenzio


On June 23, 2015, 2:39 a.m., Roger Ignazio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35763/
> ---
> 
> (Updated June 23, 2015, 2:39 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1856
> https://issues.apache.org/jira/browse/MESOS-1856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit documents a workaround to MESOS-1856, telling the user that
> they will need to override LIBNL_CFLAGS at build time if libnl3 was
> *not* compiled and installed with `--prefix=/usr`.
> 
> 
> Diffs
> -
> 
>   docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 
> 
> Diff: https://reviews.apache.org/r/35763/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Roger Ignazio
> 
>

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Kapil Arya]

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

(Updated June 23, 2015, 1:03 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


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


Repository: mesos


Description
---

This would enable the MesosContainerizer to pass on a list of namespaces
to LinuxLauncher instead of having LinuxLauncher guess it from the
isolation flags.


Diffs
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/slave/containerizer/isolator.cpp d51ecc9347cafef90c92a6965e37f417b4929e79 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
5049306dea7b94dfaa4f7c2d8187bbae8c360633 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
c6b28aa70415f4d88b14f0eeff7f20c35018fbdc 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
ee869ce71a68f51d58159c701d3c5d1e502b 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
137cdc9b064b02e829af857cff7b9023ee7b7b16 

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


Testing (updated)
---

make check

When I tried to run port-mapping tests as root, I got a bunch of errors. Any 
ideas on what might be going on here?


```
thinkpad:/home/kapil/mesos/netns/build-port-mapping # GLOG_logtostderr=1 
GTEST_v=2 make check GTEST_FILTER="*Port*"
Making check in .
make[1]: Entering directory '/local/mesos/netns/build-port-mapping'
make[1]: Nothing to be done for 'check-am'.
make[1]: Leaving directory '/local/mesos/netns/build-port-mapping'
Making check in 3rdparty
make[1]: Entering directory '/local/mesos/netns/build-port-mapping/3rdparty'
make  check-recursive
make[2]: Entering directory '/local/mesos/netns/build-port-mapping/3rdparty'
Making check in libprocess
make[3]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess'
Making check in 3rdparty
make[4]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
make  check-recursive
make[5]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
Making check in stout
make[6]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty/stout'
Making check in .
make[7]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty/stout'
make[7]: Nothing to be done for 'check-am'.
make[7]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty/stout'
Making check in include
make[7]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty/stout/include'
make[7]: Nothing to be done for 'check'.
make[7]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty/stout/include'
make[6]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty/stout'
make[6]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
make  libgmock.la stout-tests
make[7]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
make[7]: 'libgmock.la' is up to date.
make[7]: 'stout-tests' is up to date.
make[7]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
make  check-local
make[7]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
./stout-tests
Note: Google Test filter = *Port*
[==] Running 0 tests from 0 test cases.
[==] 0 tests from 0 test cases ran. (0 ms total)
[  PASSED  ] 0 tests.
make[7]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
make[6]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
make[5]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
make[4]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess/3rdparty'
Making check in .
make[4]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess'
make  tests benchmarks
make[5]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess'
make[5]: 'tests' is up to date.
make[5]: 'benchmarks' is up to date.
make[5]: Leaving directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess'
make  check-local
make[5]: Entering directory 
'/local/mesos/netns/build-port-mapping/3rdparty/libprocess'
./tests
Note: Google Test filter = *Port*
[==] Running 0 tests from 0 test cases.
[==] 0 tests from 0 test cases ran. (0 ms total)
[  PASSED  ] 0 tests.

  YOU HAVE 3 DISABLED TES

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Kapil Arya]

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

(Updated June 23, 2015, 12:59 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

This would enable the MesosContainerizer to pass on a list of namespaces
to LinuxLauncher instead of having LinuxLauncher guess it from the
isolation flags.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/slave/containerizer/isolator.cpp d51ecc9347cafef90c92a6965e37f417b4929e79 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
  src/slave/containerizer/isolators/filesystem/shared.cpp 
5049306dea7b94dfaa4f7c2d8187bbae8c360633 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
  src/slave/containerizer/isolators/namespaces/pid.cpp 
c6b28aa70415f4d88b14f0eeff7f20c35018fbdc 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
ee869ce71a68f51d58159c701d3c5d1e502b 
  src/slave/containerizer/isolators/network/port_mapping.cpp 
137cdc9b064b02e829af857cff7b9023ee7b7b16 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 35586: Updated LinuxLauncher to receive list of namespaces.

[Kapil Arya]

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

(Updated June 23, 2015, 12:59 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


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


Repository: mesos


Description
---

MesosContainerizer looks up the list of required namespaces by calling
Isolator::namespaces() for all enabled isolators and passes on this
value to LinuxLauncher.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.hpp 
ec08e24b9ba525893d218636ebddea480e641bbf 
  src/slave/containerizer/linux_launcher.cpp 
8eae258d81229e19f8c587f5e023b1df7deed025 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/tests/isolator_tests.cpp c635a4d5c78d71ca5474993eba57d1f81be9cbf1 
  src/tests/port_mapping_tests.cpp 6caab134fdbf3894f9fae801daf9491a13888c7d 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 35586: Updated LinuxLauncher to receive list of namespaces.

[Kapil Arya]

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

(Updated June 23, 2015, 12:27 a.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

Updated port_mapping tests.


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


Repository: mesos


Description
---

MesosContainerizer looks up the list of required namespaces by calling
Isolator::namespaces() for all enabled isolators and passes on this
value to LinuxLauncher.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.hpp 
ec08e24b9ba525893d218636ebddea480e641bbf 
  src/slave/containerizer/linux_launcher.cpp 
8eae258d81229e19f8c587f5e023b1df7deed025 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/tests/isolator_tests.cpp c635a4d5c78d71ca5474993eba57d1f81be9cbf1 
  src/tests/port_mapping_tests.cpp 6caab134fdbf3894f9fae801daf9491a13888c7d 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 35765: Updated network-monitoring docs to reflect correct libnl version.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35585, 35586, 35762, 35765]

All tests passed.

- Mesos ReviewBot


On June 23, 2015, 3:09 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35765/
> ---
> 
> (Updated June 23, 2015, 3:09 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2917
> https://issues.apache.org/jira/browse/MESOS-2917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The correct libnl required version is >= 3.2.26.
> 
> 
> Diffs
> -
> 
>   docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 
> 
> Diff: https://reviews.apache.org/r/35765/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Jie Yu]

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

Ship it!



include/mesos/slave/isolator.hpp (lines 78 - 104)


Any reason you want to keep these defined here in the public header? IIUC, 
these are workaround for those who are using an old glibc.

Can you instead include src/linux/ns.hpp in the corresponding isolator 
source file?



include/mesos/slave/isolator.hpp (lines 113 - 114)


Can you comment on what does that mean if None() is returned?



src/slave/containerizer/isolators/filesystem/shared.hpp (lines 42 - 45)


Can you move this to the cpp file and include src/linux/ns.hpp?



src/slave/containerizer/isolators/namespaces/pid.hpp (lines 59 - 62)


Ditto.



src/slave/containerizer/isolators/network/port_mapping.hpp (lines 155 - 158)


Ditto on moving to cpp file.


- Jie Yu


On June 23, 2015, 2:48 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35585/
> ---
> 
> (Updated June 23, 2015, 2:48 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2884
> https://issues.apache.org/jira/browse/MESOS-2884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would enable the MesosContainerizer to pass on a list of namespaces
> to LinuxLauncher instead of having LinuxLauncher guess it from the
> isolation flags.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/slave/containerizer/isolator.cpp 
> d51ecc9347cafef90c92a6965e37f417b4929e79 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> ee869ce71a68f51d58159c701d3c5d1e502b 
> 
> Diff: https://reviews.apache.org/r/35585/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35762: Updated libnl configure check to reflect correct requirements.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On June 23, 2015, 3:05 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35762/
> ---
> 
> (Updated June 23, 2015, 3:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2917
> https://issues.apache.org/jira/browse/MESOS-2917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current codebase require version >= 3.2.26 due to
> https://reviews.apache.org/r/31503. Libnl-3.2.26 was released on
> 3/30/2015.
> 
> 
> Diffs
> -
> 
>   configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
> 
> Diff: https://reviews.apache.org/r/35762/diff/
> 
> 
> Testing
> ---
> 
> Tested by running configure check with 3.2.25 and 3.2.26.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35765: Updated network-monitoring docs to reflect correct libnl version.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On June 23, 2015, 3:09 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35765/
> ---
> 
> (Updated June 23, 2015, 3:09 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2917
> https://issues.apache.org/jira/browse/MESOS-2917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The correct libnl required version is >= 3.2.26.
> 
> 
> Diffs
> -
> 
>   docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 
> 
> Diff: https://reviews.apache.org/r/35765/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35763: Document workaround for LIBNL_CFLAGS path

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35763]

All tests passed.

- Mesos ReviewBot


On June 23, 2015, 2:39 a.m., Roger Ignazio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35763/
> ---
> 
> (Updated June 23, 2015, 2:39 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-1856
> https://issues.apache.org/jira/browse/MESOS-1856
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit documents a workaround to MESOS-1856, telling the user that
> they will need to override LIBNL_CFLAGS at build time if libnl3 was
> *not* compiled and installed with `--prefix=/usr`.
> 
> 
> Diffs
> -
> 
>   docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 
> 
> Diff: https://reviews.apache.org/r/35763/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Roger Ignazio
> 
>

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

[Michael Park]

> On June 22, 2015, 2:13 p.m., Alexander Rukletsov wrote:
> > Looks like a copy-paste bug. How have you found it? Was the test flaky?
> 
> Michael Park wrote:
> I was using these tests as a reference for writing my new tests for 
> [r35702](https://reviews.apache.org/r/35702/) and noticed it.
> 
> Alexander Rukletsov wrote:
> I wonder why they were not flaky.

Because `filtersForever` gets default-constructed, it gets initialized with the 
default value of 5 seconds. We set it to `std::numeric_limits::max()` 
to be certain + documentation. Practically, since the test finishes within a 
second, 5 seconds was "forever" enough.


- Michael


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


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> ---
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than 
> `filters`.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35765: Updated network-monitoring docs to reflect correct libnl version.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On June 23, 2015, 3:09 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35765/
> ---
> 
> (Updated June 23, 2015, 3:09 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2917
> https://issues.apache.org/jira/browse/MESOS-2917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The correct libnl required version is >= 3.2.26.
> 
> 
> Diffs
> -
> 
>   docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 
> 
> Diff: https://reviews.apache.org/r/35765/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Review Request 35765: Updated network-monitoring docs to reflect correct libnl version.

[Kapil Arya]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The correct libnl required version is >= 3.2.26.


Diffs
-

  docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 35762: Updated libnl configure check to reflect correct requirements.

[Till Toenshoff]

> On June 23, 2015, 2:47 a.m., Till Toenshoff wrote:
> > Ship It!

As discussed, another RR updating docs/network-monitoring.md would be great to 
keep things in sync.


- Till


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


On June 23, 2015, 3:05 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35762/
> ---
> 
> (Updated June 23, 2015, 3:05 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2917
> https://issues.apache.org/jira/browse/MESOS-2917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current codebase require version >= 3.2.26 due to
> https://reviews.apache.org/r/31503. Libnl-3.2.26 was released on
> 3/30/2015.
> 
> 
> Diffs
> -
> 
>   configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
> 
> Diff: https://reviews.apache.org/r/35762/diff/
> 
> 
> Testing
> ---
> 
> Tested by running configure check with 3.2.25 and 3.2.26.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35762: Updated libnl configure check to reflect correct requirements.

[Kapil Arya]

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

(Updated June 22, 2015, 11:05 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Fixed jira issue


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


Repository: mesos


Description
---

The current codebase require version >= 3.2.26 due to
https://reviews.apache.org/r/31503. Libnl-3.2.26 was released on
3/30/2015.


Diffs
-

  configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 

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


Testing
---

Tested by running configure check with 3.2.25 and 3.2.26.


Thanks,

Kapil Arya

Re: Review Request 35715: Added revocable resource state validation.

[Michael Park]

> On June 21, 2015, 6:47 p.m., Vinod Kone wrote:
> > src/common/resources.cpp, lines 479-487
> > 
> >
> > These checks are done in master's validation.cpp
> 
> Michael Park wrote:
> Ah sorry, I missed that.
> 
> This reminded me of the discussion Jie and I had for 
> [r32140](https://reviews.apache.org/r/32140/) regarding where validations 
> should live. I think this validation belongs here rather than in master 
> validation.
> What we concluded from the discussion was that `Resources::validate` 
> should perform necessary validation to satisfy the invariant of the 
> `Resource` object.
> This enables methods that operate on `Resource` (e.g. 
> `Resources::isRevocable`) to assume its validity.
> 
> My notes:
> > Synced with Jie on IRC regarding this topic. We agreed that 
> `Resources::validate` needs to capture the invariant of the `Resource` object 
> which means it needs to invalidate the `role == "*" && has_reservation()` 
> state. This invariant is required for all the predicates as well as functions 
> such as `reserved()` and `unreserved()` to have well-defined behavior.
> 
> Jie's note:
> > Discussed with Mpark offline. We agreed that rule for 
> Resources::validate is that it should only perform necessary validation to 
> make sure all methods in Resources are well hahaved, and the validation 
> around * and reservation info is necessary for 'reserved/unreserved' to work 
> properly. Thus dropping the issue around validation.
> 
> Michael Park wrote:
> I found Jie's comment regarding this: 
> https://reviews.apache.org/r/33865/#comment133597
> 
> @Jie: My thought here was that these checks are necessary to make 
> `isRevocable` well-defined. The same way the check for `"*" resource cannot 
> be dynamically reserved` is necessary to make `isDynamicallyReserved` and 
> others well-defined?
> 
> Jie Yu wrote:
> @Mpark,
> 
> I think the following check is in Resources::validate because otherwise 
> isReserved will break (e.g., role = `*` and reservation is not set, 
> isReserved(resource, `*`) will return true).
> 
> ```
> if (resource.role() == "*" && resource.has_reservation()) {
>   return Error(
>   "Invalid reservation: role \"*\" cannot be dynamically reserved");
> }
> ```

@Jie,
> e.g., role = * and reservation is not set, isReserved(resource, *) will 
> return true

If you meant `role = * and reservation _is_ set`, then yes.

I'm saying that exact reasoning is also why these checks should be in 
`Resources::validate`, because otherwise `isRevocable` will break.
e.g. `reservation is set and revocable is set`, `isRevocable` will return true.


- Michael


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


On June 21, 2015, 7 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35715/
> ---
> 
> (Updated June 21, 2015, 7 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In `mesos.proto`, it specifies the expected state of revocable resource:
> 
> ```
> // ... Note that if this is set, 'disk' or 'reservation' cannot be set.
> optional RevocableInfo revocable = 9;
> ```
>   
> This expectation should be validated in `Resources::validate(const Resource& 
> resoure)`
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp eb5476a0365fe65f474afd0ab7a52ad7f1e04521 
>   src/tests/resources_tests.cpp 9f96b14a6a4ce416d044934dd7ab4d28e4bc7332 
> 
> Diff: https://reviews.apache.org/r/35715/diff/
> 
> 
> Testing
> ---
> 
> Added `RevocableResourceTest.Validation` + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35586: Updated LinuxLauncher to receive list of namespaces.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35585, 35586]

All tests passed.

- Mesos ReviewBot


On June 23, 2015, 2:48 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35586/
> ---
> 
> (Updated June 23, 2015, 2:48 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2884
> https://issues.apache.org/jira/browse/MESOS-2884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MesosContainerizer looks up the list of required namespaces by calling
> Isolator::namespaces() for all enabled isolators and passes on this
> value to LinuxLauncher.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/linux_launcher.hpp 
> ec08e24b9ba525893d218636ebddea480e641bbf 
>   src/slave/containerizer/linux_launcher.cpp 
> 8eae258d81229e19f8c587f5e023b1df7deed025 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 8c102fb7d1f79ee768cb06de3a976ea12f958712 
>   src/tests/isolator_tests.cpp c635a4d5c78d71ca5474993eba57d1f81be9cbf1 
> 
> Diff: https://reviews.apache.org/r/35586/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35586: Updated LinuxLauncher to receive list of namespaces.

[Kapil Arya]

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

(Updated June 22, 2015, 10:48 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


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


Repository: mesos


Description
---

MesosContainerizer looks up the list of required namespaces by calling
Isolator::namespaces() for all enabled isolators and passes on this
value to LinuxLauncher.


Diffs (updated)
-

  src/slave/containerizer/linux_launcher.hpp 
ec08e24b9ba525893d218636ebddea480e641bbf 
  src/slave/containerizer/linux_launcher.cpp 
8eae258d81229e19f8c587f5e023b1df7deed025 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/tests/isolator_tests.cpp c635a4d5c78d71ca5474993eba57d1f81be9cbf1 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Kapil Arya]

> On June 22, 2015, 10:23 p.m., Jie Yu wrote:
> > Hum, I don't see any change from r2 to r3? Did you upload the correct diff?

I accidentally committed the diffs into the dependent RR. Fixed now. Thanks! :)


- Kapil


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


On June 22, 2015, 10:48 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35585/
> ---
> 
> (Updated June 22, 2015, 10:48 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2884
> https://issues.apache.org/jira/browse/MESOS-2884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would enable the MesosContainerizer to pass on a list of namespaces
> to LinuxLauncher instead of having LinuxLauncher guess it from the
> isolation flags.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/slave/containerizer/isolator.cpp 
> d51ecc9347cafef90c92a6965e37f417b4929e79 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> ee869ce71a68f51d58159c701d3c5d1e502b 
> 
> Diff: https://reviews.apache.org/r/35585/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Kapil Arya]

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

(Updated June 22, 2015, 10:48 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

Reuploading the diff.


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


Repository: mesos


Description
---

This would enable the MesosContainerizer to pass on a list of namespaces
to LinuxLauncher instead of having LinuxLauncher guess it from the
isolation flags.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/slave/containerizer/isolator.cpp d51ecc9347cafef90c92a6965e37f417b4929e79 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
ee869ce71a68f51d58159c701d3c5d1e502b 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 35762: Updated libnl configure check to reflect correct requirements.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On June 23, 2015, 2:41 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35762/
> ---
> 
> (Updated June 23, 2015, 2:41 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2422
> https://issues.apache.org/jira/browse/MESOS-2422
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current codebase require version >= 3.2.26 due to
> https://reviews.apache.org/r/31503. Libnl-3.2.26 was released on
> 3/30/2015.
> 
> 
> Diffs
> -
> 
>   configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 
> 
> Diff: https://reviews.apache.org/r/35762/diff/
> 
> 
> Testing
> ---
> 
> Tested by running configure check with 3.2.25 and 3.2.26.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35762: Updated libnl configure check to reflect correct requirements.

[Kapil Arya]

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

(Updated June 22, 2015, 10:41 p.m.)


Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

The current codebase require version >= 3.2.26 due to
https://reviews.apache.org/r/31503. Libnl-3.2.26 was released on
3/30/2015.


Diffs
-

  configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 

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


Testing
---

Tested by running configure check with 3.2.25 and 3.2.26.


Thanks,

Kapil Arya

Review Request 35762: Updated libnl configure check to reflect correct requirements.

[Kapil Arya]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

The current codebase require version >= 3.2.26 due to
https://reviews.apache.org/r/31503. Libnl-3.2.26 was released on
3/30/2015.


Diffs
-

  configure.ac 563e9c529444b3e980db6d04173f0d016a737c74 

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


Testing
---

Tested by running configure check with 3.2.25 and 3.2.26.


Thanks,

Kapil Arya

Review Request 35763: Document workaround for LIBNL_CFLAGS path

[Roger Ignazio]

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

Review request for mesos.


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


Repository: mesos


Description
---

This commit documents a workaround to MESOS-1856, telling the user that
they will need to override LIBNL_CFLAGS at build time if libnl3 was
*not* compiled and installed with `--prefix=/usr`.


Diffs
-

  docs/network-monitoring.md ac36ba3aa9afda9231de634157a2c083988e7e22 

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


Testing
---


Thanks,

Roger Ignazio

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Jie Yu]

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


Hum, I don't see any change from r2 to r3? Did you upload the correct diff?

- Jie Yu


On June 23, 2015, 2:20 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35585/
> ---
> 
> (Updated June 23, 2015, 2:20 a.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2884
> https://issues.apache.org/jira/browse/MESOS-2884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would enable the MesosContainerizer to pass on a list of namespaces
> to LinuxLauncher instead of having LinuxLauncher guess it from the
> isolation flags.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/slave/containerizer/isolator.cpp 
> d51ecc9347cafef90c92a6965e37f417b4929e79 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> ee869ce71a68f51d58159c701d3c5d1e502b 
> 
> Diff: https://reviews.apache.org/r/35585/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35586: Updated LinuxLauncher to receive list of namespaces.

[Kapil Arya]

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

(Updated June 22, 2015, 10:21 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

rebased


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


Repository: mesos


Description
---

MesosContainerizer looks up the list of required namespaces by calling
Isolator::namespaces() for all enabled isolators and passes on this
value to LinuxLauncher.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/slave/containerizer/isolator.cpp d51ecc9347cafef90c92a6965e37f417b4929e79 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
ee869ce71a68f51d58159c701d3c5d1e502b 
  src/slave/containerizer/linux_launcher.hpp 
ec08e24b9ba525893d218636ebddea480e641bbf 
  src/slave/containerizer/linux_launcher.cpp 
8eae258d81229e19f8c587f5e023b1df7deed025 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/tests/isolator_tests.cpp c635a4d5c78d71ca5474993eba57d1f81be9cbf1 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Kapil Arya]

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

(Updated June 22, 2015, 10:20 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

Addressed Jie's comments.


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


Repository: mesos


Description
---

This would enable the MesosContainerizer to pass on a list of namespaces
to LinuxLauncher instead of having LinuxLauncher guess it from the
isolation flags.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/slave/containerizer/isolator.cpp d51ecc9347cafef90c92a6965e37f417b4929e79 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
ee869ce71a68f51d58159c701d3c5d1e502b 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 35756: Fixed a race condition in hook tests for remove-executor hook.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35756]

All tests passed.

- Mesos ReviewBot


On June 23, 2015, 12:53 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35756/
> ---
> 
> (Updated June 23, 2015, 12:53 a.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2226
> https://issues.apache.org/jira/browse/MESOS-2226
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the code was not checking for TASK_RUNNING status message
> from the MockExecutor before stopping the scheduler driver. This caused
> the Executor to be terminated prematurely (before the tasks were
> launched) and thus the remove-executor hook was never called. The fix
> was to wait for the TASK_RUNNING status update and then wait for the
> shutdown() within MockExecutor. Only then we wait for the future from
> remove-executor hook.
> 
> 
> Diffs
> -
> 
>   src/tests/hook_tests.cpp 3ffde6d6b2faeb5a8a40eb27c3b0a2b7f9ecd2b1 
> 
> Diff: https://reviews.apache.org/r/35756/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35738: Add slave metric to count container launch failures.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35738]

All tests passed.

- Mesos ReviewBot


On June 23, 2015, 12:42 a.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35738/
> ---
> 
> (Updated June 23, 2015, 12:42 a.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-2904
> https://issues.apache.org/jira/browse/MESOS-2904
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add slave metric to count container launch failures.
> 
> 
> Diffs
> -
> 
>   src/slave/metrics.hpp 780c438b07993253c75275fd3956f1631b3c0d73 
>   src/slave/metrics.cpp 87289f2da83f211e1244d425c1d003e2ae79b507 
>   src/slave/slave.cpp 946e372252e6188b28f94452f1479824a7e201cd 
>   src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 
>   src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 
>   src/tests/metrics_tests.cpp 6896727a23c1d6b0e2055ee661042a0a66b09067 
>   src/tests/slave_tests.cpp 50301983c674ef50a64294816db9587bf065aa9e 
> 
> Diff: https://reviews.apache.org/r/35738/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 35701: Minor formatting cleanup in the Master.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> ---
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35701: Minor formatting cleanup in the Master.

[Till Toenshoff]

> On June 22, 2015, 6:26 p.m., Till Toenshoff wrote:
> > src/master/master.cpp, lines 972-975
> > 
> >
> > I understand this was updated due to some discussions on related RRs. I 
> > would however love to see the definitive answer to 
> > https://issues.apache.org/jira/browse/MESOS-2618 for preventing any 
> > subjective back and forth. The key-question here is "when are we reaching 
> > the jaggedness problem"?
> > 
> > Would you mind adding a comment to the ticket with your recent findings 
> > and experience from those other RRs?
> 
> Ben Mahler wrote:
> Hey Till, the particular case Michael fixed here was bad alignment: it 
> doesn't fit any of the options in the style guide. So I doubt there would be 
> any controversy here :)
> 
> Till Toenshoff wrote:
> Thanks for chiming in Ben. While I see that the original formatting was 
> off in any case, I still see a potential problem with this variable 
> indentation for the argument lists.
> 
> For fixing the above, according to our current styleguide, MPark could 
> have chosen any of the below.
> 
> A. Variable Indentation:
> ```
> delay(failoverTimeout,
>   self(),
>   &Master::frameworkFailoverTimeout,
> framework->id(),
> framework->reregisteredTime);
> ```
> 
> B. Fixed Indentation:
> ```
> delay(
> failoverTimeout,
> self(),
> &Master::frameworkFailoverTimeout,
>   framework->id(),
>   framework->reregisteredTime);
> ```
> 
> However, how to be able to assert that A. would be fine even though we 
> try to avoid "jaggedness" is a real problem to me. I wondered if we could get 
> closer to that "definitive answer". Any input is greatly appreciated :)
> 
> Michael Park wrote:
> Hey Till, I've captured my thoughts on this topic on the [JIRA 
> ticket](https://issues.apache.org/jira/browse/MESOS-2618?focusedCommentId=14596770&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14596770).

Terrific, thanks MPark! For now, I shall drop that issue given that you payed 
the toll :D.


- Till


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


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> ---
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 35756: Fixed a race condition in hook tests for remove-executor hook.

[Kapil Arya]

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

Review request for mesos and Niklas Nielsen.


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


Repository: mesos


Description
---

Previously, the code was not checking for TASK_RUNNING status message
from the MockExecutor before stopping the scheduler driver. This caused
the Executor to be terminated prematurely (before the tasks were
launched) and thus the remove-executor hook was never called. The fix
was to wait for the TASK_RUNNING status update and then wait for the
shutdown() within MockExecutor. Only then we wait for the future from
remove-executor hook.


Diffs
-

  src/tests/hook_tests.cpp 3ffde6d6b2faeb5a8a40eb27c3b0a2b7f9ecd2b1 

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


Testing
---

make check.


Thanks,

Kapil Arya

Re: Review Request 35738: Add slave metric to count container launch failures.

[Paul Brett]

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

(Updated June 23, 2015, 12:42 a.m.)


Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


Changes
---

Added a test to verify that failed container creation increments the counter.


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


Repository: mesos


Description
---

Add slave metric to count container launch failures.


Diffs (updated)
-

  src/slave/metrics.hpp 780c438b07993253c75275fd3956f1631b3c0d73 
  src/slave/metrics.cpp 87289f2da83f211e1244d425c1d003e2ae79b507 
  src/slave/slave.cpp 946e372252e6188b28f94452f1479824a7e201cd 
  src/tests/containerizer.hpp 24b014f44d9eec56840e18cf39fbf9100f2c0711 
  src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 
  src/tests/metrics_tests.cpp 6896727a23c1d6b0e2055ee661042a0a66b09067 
  src/tests/slave_tests.cpp 50301983c674ef50a64294816db9587bf065aa9e 

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


Testing
---

sudo make check


Thanks,

Paul Brett

Re: Review Request 35567: Added 'executor_environment_variables' flag to slave.

[Till Toenshoff]

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

Ship it!



src/slave/flags.cpp (line 28)


IIUC, then this should be the first include.



src/tests/containerizer.cpp (lines 127 - 128)


Cant follow this one - are you saying that  `executorEnvironment(..)` 
should possibly set environment variables?


- Till Toenshoff


On June 20, 2015, 7 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35567/
> ---
> 
> (Updated June 20, 2015, 7 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-2832
> https://issues.apache.org/jira/browse/MESOS-2832
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> c5ffc49899976225e319f122cb5f7556c3b09d46 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/tests/containerizer.cpp 80b910515694c81cf5b1eace1d79a5178cf35426 
> 
> Diff: https://reviews.apache.org/r/35567/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 35715: Added revocable resource state validation.

[Jie Yu]

> On June 21, 2015, 6:47 p.m., Vinod Kone wrote:
> > src/common/resources.cpp, lines 479-487
> > 
> >
> > These checks are done in master's validation.cpp
> 
> Michael Park wrote:
> Ah sorry, I missed that.
> 
> This reminded me of the discussion Jie and I had for 
> [r32140](https://reviews.apache.org/r/32140/) regarding where validations 
> should live. I think this validation belongs here rather than in master 
> validation.
> What we concluded from the discussion was that `Resources::validate` 
> should perform necessary validation to satisfy the invariant of the 
> `Resource` object.
> This enables methods that operate on `Resource` (e.g. 
> `Resources::isRevocable`) to assume its validity.
> 
> My notes:
> > Synced with Jie on IRC regarding this topic. We agreed that 
> `Resources::validate` needs to capture the invariant of the `Resource` object 
> which means it needs to invalidate the `role == "*" && has_reservation()` 
> state. This invariant is required for all the predicates as well as functions 
> such as `reserved()` and `unreserved()` to have well-defined behavior.
> 
> Jie's note:
> > Discussed with Mpark offline. We agreed that rule for 
> Resources::validate is that it should only perform necessary validation to 
> make sure all methods in Resources are well hahaved, and the validation 
> around * and reservation info is necessary for 'reserved/unreserved' to work 
> properly. Thus dropping the issue around validation.
> 
> Michael Park wrote:
> I found Jie's comment regarding this: 
> https://reviews.apache.org/r/33865/#comment133597
> 
> @Jie: My thought here was that these checks are necessary to make 
> `isRevocable` well-defined. The same way the check for `"*" resource cannot 
> be dynamically reserved` is necessary to make `isDynamicallyReserved` and 
> others well-defined?

@Mpark,

I think the following check is in Resources::validate because otherwise 
isReserved will break (e.g., role = `*` and reservation is not set, 
isReserved(resource, `*`) will return true).

```
if (resource.role() == "*" && resource.has_reservation()) {
  return Error(
  "Invalid reservation: role \"*\" cannot be dynamically reserved");
}
```


- Jie


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


On June 21, 2015, 7 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35715/
> ---
> 
> (Updated June 21, 2015, 7 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In `mesos.proto`, it specifies the expected state of revocable resource:
> 
> ```
> // ... Note that if this is set, 'disk' or 'reservation' cannot be set.
> optional RevocableInfo revocable = 9;
> ```
>   
> This expectation should be validated in `Resources::validate(const Resource& 
> resoure)`
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp eb5476a0365fe65f474afd0ab7a52ad7f1e04521 
>   src/tests/resources_tests.cpp 9f96b14a6a4ce416d044934dd7ab4d28e4bc7332 
> 
> Diff: https://reviews.apache.org/r/35715/diff/
> 
> 
> Testing
> ---
> 
> Added `RevocableResourceTest.Validation` + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35750: Added a test to verify recovering mixed known and unknown orphans for port mapping isolator.

[Chi Zhang]

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

Ship it!


It's not very clean to reply on `___recover`'s implementation details IMO, but 
looks like this is not the first time we test based on this.

- Chi Zhang


On June 22, 2015, 11:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35750/
> ---
> 
> (Updated June 22, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to verify recovering mixed known and unknown orphans for port 
> mapping isolator.
> 
> 
> Diffs
> -
> 
>   src/tests/port_mapping_tests.cpp 6caab134fdbf3894f9fae801daf9491a13888c7d 
> 
> Diff: https://reviews.apache.org/r/35750/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35750: Added a test to verify recovering mixed known and unknown orphans for port mapping isolator.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35749, 35750]

All tests passed.

- Mesos ReviewBot


On June 22, 2015, 11:09 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35750/
> ---
> 
> (Updated June 22, 2015, 11:09 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to verify recovering mixed known and unknown orphans for port 
> mapping isolator.
> 
> 
> Diffs
> -
> 
>   src/tests/port_mapping_tests.cpp 6caab134fdbf3894f9fae801daf9491a13888c7d 
> 
> Diff: https://reviews.apache.org/r/35750/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35749: Fixed a memory bug in port mapping recovery logic.

[Jie Yu]

> On June 22, 2015, 11:33 p.m., Chi Zhang wrote:
> > let us definitely improve the info pointer management, there is 7 'delete 
> > info' in this file right now... :(

Yeah, I'll follow up with patches.


- Jie


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


On June 22, 2015, 11:29 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35749/
> ---
> 
> (Updated June 22, 2015, 11:29 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a memory bug in port mapping recovery logic. Info will be deleted in 
> `_cleanup` (a little weired, we can fix that later).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 4de737a158da2ab329de7c57b9b52de305606bf0 
> 
> Diff: https://reviews.apache.org/r/35749/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35749: Fixed a memory bug in port mapping recovery logic.

[Chi Zhang]

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

Ship it!


let us definitely improve the info pointer management, there is 7 'delete info' 
in this file right now... :(

- Chi Zhang


On June 22, 2015, 11:29 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35749/
> ---
> 
> (Updated June 22, 2015, 11:29 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a memory bug in port mapping recovery logic. Info will be deleted in 
> `_cleanup` (a little weired, we can fix that later).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 4de737a158da2ab329de7c57b9b52de305606bf0 
> 
> Diff: https://reviews.apache.org/r/35749/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35749: Fixed a memory bug in port mapping recovery logic.

[Jie Yu]

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

(Updated June 22, 2015, 11:29 p.m.)


Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.


Changes
---

Added a TODO.


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


Repository: mesos


Description
---

Fixed a memory bug in port mapping recovery logic. Info will be deleted in 
`_cleanup` (a little weired, we can fix that later).


Diffs (updated)
-

  src/slave/containerizer/isolators/network/port_mapping.cpp 
4de737a158da2ab329de7c57b9b52de305606bf0 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 35749: Fixed a memory bug in port mapping recovery logic.

[Jie Yu]

> On June 22, 2015, 11:19 p.m., Chi Zhang wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1905-1909
> > 
> >
> > Do we need another for loop here to clean up unknownOrphans like you 
> > did above?

No, we cannot, because Info in 'unknownOrphans' might have been already deleted.

This whole 'delete Info' thing needs to be refactored. Let's do that later. 
I'll add a TODO here.


- Jie


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


On June 22, 2015, 11:08 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35749/
> ---
> 
> (Updated June 22, 2015, 11:08 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a memory bug in port mapping recovery logic. Info will be deleted in 
> `_cleanup` (a little weired, we can fix that later).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 4de737a158da2ab329de7c57b9b52de305606bf0 
> 
> Diff: https://reviews.apache.org/r/35749/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Jie Yu]

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



include/mesos/slave/isolator.hpp (lines 105 - 116)


This is not exactly an enum since we need to perform bit-wise OR on the 
enum which seems to be quite weired for an enum.

I would suggest we still stick to an 'int' as you originally did.



include/mesos/slave/isolator.hpp (line 130)


Given the TODO here, does it make sense to return an Option. We return 
None() if namespaces does not apply?
```
Future> namespaces();
```


- Jie Yu


On June 22, 2015, 5:56 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35585/
> ---
> 
> (Updated June 22, 2015, 5:56 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2884
> https://issues.apache.org/jira/browse/MESOS-2884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would enable the MesosContainerizer to pass on a list of namespaces
> to LinuxLauncher instead of having LinuxLauncher guess it from the
> isolation flags.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/slave/containerizer/isolator.cpp 
> d51ecc9347cafef90c92a6965e37f417b4929e79 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> ee869ce71a68f51d58159c701d3c5d1e502b 
> 
> Diff: https://reviews.apache.org/r/35585/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35749: Fixed a memory bug in port mapping recovery logic.

[Chi Zhang]

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



src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1905 - 1909)


Do we need another for loop here to clean up unknownOrphans like you did 
above?


- Chi Zhang


On June 22, 2015, 11:08 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35749/
> ---
> 
> (Updated June 22, 2015, 11:08 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a memory bug in port mapping recovery logic. Info will be deleted in 
> `_cleanup` (a little weired, we can fix that later).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 4de737a158da2ab329de7c57b9b52de305606bf0 
> 
> Diff: https://reviews.apache.org/r/35749/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35697: mesos: Fixed test names.

[Michael Park]

> On June 22, 2015, 8:12 p.m., Ben Mahler wrote:
> > Thanks, will commit this for you.
> > 
> > Also we are inconcistent on the "Test" suffix, e.g. "MasterTest" vs 
> > "CRAMMD5Authentication". See two options:
> > 
> > (1) We can use "Test" to signify that there is a fixture, if that's 
> > helpful. So remove any "Test" suffixes from non-fixture tests.
> > (2) We can use "Test" as a suffix for all tests.
> > 
> > Thoughts?

Ah, that's a good catch Ben!

(1) I think whether the test is a fixture or not is already clear from the use 
of `TEST_F` as opposed to `TEST`. So I don't think it's useful to have the 
`Test` suffix to signify fixtures.
(2) The fact that it's a test is obvious from `TEST`, `TEST_F`, so I don't 
think there's a benefit of having the `Test` suffix for all tests either. 
However, most of our tests have the `Test` suffix, and removing all of those 
would touch pretty much all of the tests. So I would vote for having the `Test` 
suffix for all tests.

There are currently 136 instances of tests without the `Test` suffix. `grep 
--exclude=./src/tests/script.hpp 
--exclude=./src/webui/master/static/js/jquery-1.7.1.min.js 
--exclude-dir=./build -r "TEST.*(.*," . | grep -v "TEST.*(.*Test,"`


- Michael


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


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35697/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   src/tests/cram_md5_authentication_tests.cpp 
> 992302365682df2c800c8828a21b9d38cb318472 
>   src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f 
>   src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a 
> 
> Diff: https://reviews.apache.org/r/35697/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 35750: Added a test to verify recovering mixed known and unknown orphans for port mapping isolator.

[Jie Yu]

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

Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.


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


Repository: mesos


Description
---

Added a test to verify recovering mixed known and unknown orphans for port 
mapping isolator.


Diffs
-

  src/tests/port_mapping_tests.cpp 6caab134fdbf3894f9fae801daf9491a13888c7d 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Review Request 35749: Fixed a memory bug in port mapping recovery logic.

[Jie Yu]

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

Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.


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


Repository: mesos


Description
---

Fixed a memory bug in port mapping recovery logic. Info will be deleted in 
`_cleanup` (a little weired, we can fix that later).


Diffs
-

  src/slave/containerizer/isolators/network/port_mapping.cpp 
4de737a158da2ab329de7c57b9b52de305606bf0 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 35738: Add slave metric to count container launch failures.

[Chi Zhang]

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

Ship it!


Ship It!

- Chi Zhang


On June 22, 2015, 7:26 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35738/
> ---
> 
> (Updated June 22, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-2904
> https://issues.apache.org/jira/browse/MESOS-2904
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add slave metric to count container launch failures.
> 
> 
> Diffs
> -
> 
>   src/slave/metrics.hpp 780c438b07993253c75275fd3956f1631b3c0d73 
>   src/slave/metrics.cpp 87289f2da83f211e1244d425c1d003e2ae79b507 
>   src/slave/slave.cpp 946e372252e6188b28f94452f1479824a7e201cd 
>   src/tests/metrics_tests.cpp 6896727a23c1d6b0e2055ee661042a0a66b09067 
>   src/tests/slave_tests.cpp 50301983c674ef50a64294816db9587bf065aa9e 
> 
> Diff: https://reviews.apache.org/r/35738/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

Re: Review Request 35701: Minor formatting cleanup in the Master.

[Michael Park]

> On June 22, 2015, 6:26 p.m., Till Toenshoff wrote:
> > src/master/master.cpp, lines 972-975
> > 
> >
> > I understand this was updated due to some discussions on related RRs. I 
> > would however love to see the definitive answer to 
> > https://issues.apache.org/jira/browse/MESOS-2618 for preventing any 
> > subjective back and forth. The key-question here is "when are we reaching 
> > the jaggedness problem"?
> > 
> > Would you mind adding a comment to the ticket with your recent findings 
> > and experience from those other RRs?
> 
> Ben Mahler wrote:
> Hey Till, the particular case Michael fixed here was bad alignment: it 
> doesn't fit any of the options in the style guide. So I doubt there would be 
> any controversy here :)
> 
> Till Toenshoff wrote:
> Thanks for chiming in Ben. While I see that the original formatting was 
> off in any case, I still see a potential problem with this variable 
> indentation for the argument lists.
> 
> For fixing the above, according to our current styleguide, MPark could 
> have chosen any of the below.
> 
> A. Variable Indentation:
> ```
> delay(failoverTimeout,
>   self(),
>   &Master::frameworkFailoverTimeout,
> framework->id(),
> framework->reregisteredTime);
> ```
> 
> B. Fixed Indentation:
> ```
> delay(
> failoverTimeout,
> self(),
> &Master::frameworkFailoverTimeout,
>   framework->id(),
>   framework->reregisteredTime);
> ```
> 
> However, how to be able to assert that A. would be fine even though we 
> try to avoid "jaggedness" is a real problem to me. I wondered if we could get 
> closer to that "definitive answer". Any input is greatly appreciated :)

Hey Till, I've captured my thoughts on this topic on the [JIRA 
ticket](https://issues.apache.org/jira/browse/MESOS-2618?focusedCommentId=14596770&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14596770).


- Michael


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


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> ---
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35354: Smaller fixes in libprocess firewall initialization

[Ben Mahler]

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


Till, will you be able to help Alex get these committed? If not, I'm happy to 
help out.

- Ben Mahler


On June 15, 2015, 9:24 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35354/
> ---
> 
> (Updated June 15, 2015, 9:24 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 1c33e3bf7baae45b1671f9566b4993f39d9f1294 
>   src/messages/flags.hpp 41be419ba5593a600aa0c6c411210fa4faa829a8 
>   src/slave/main.cpp c379243e01919a5ab30bb9dea1b738665ba4e746 
> 
> Diff: https://reviews.apache.org/r/35354/diff/
> 
> 
> Testing
> ---
> 
> Manual checks && make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 35743: flags: fixed const'ness of load

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35743]

All tests passed.

- Mesos ReviewBot


On June 22, 2015, 9:27 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35743/
> ---
> 
> (Updated June 22, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes:
>   - replacing argument arrays with static initializers in tests.
>   - making the argument 'argv' const in load method to reflect the semantics.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 7584cb871d02ad01021f0c3439ea205736d4f6b4 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> c2c6a6ac97044f2317418295f48d75e94de4112b 
> 
> Diff: https://reviews.apache.org/r/35743/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 35701: Minor formatting cleanup in the Master.

[Till Toenshoff]

> On June 22, 2015, 6:26 p.m., Till Toenshoff wrote:
> > src/master/master.cpp, lines 972-975
> > 
> >
> > I understand this was updated due to some discussions on related RRs. I 
> > would however love to see the definitive answer to 
> > https://issues.apache.org/jira/browse/MESOS-2618 for preventing any 
> > subjective back and forth. The key-question here is "when are we reaching 
> > the jaggedness problem"?
> > 
> > Would you mind adding a comment to the ticket with your recent findings 
> > and experience from those other RRs?
> 
> Ben Mahler wrote:
> Hey Till, the particular case Michael fixed here was bad alignment: it 
> doesn't fit any of the options in the style guide. So I doubt there would be 
> any controversy here :)

Thanks for chiming in Ben. While I see that the original formatting was off in 
any case, I still see a potential problem with this variable indentation for 
the argument lists.

For fixing the above, according to our current styleguide, MPark could have 
chosen any of the below.

A. Variable Indentation:
```
delay(failoverTimeout,
  self(),
  &Master::frameworkFailoverTimeout,
  framework->id(),
  framework->reregisteredTime);
```

B. Fixed Indentation:
```
delay(
failoverTimeout,
self(),
&Master::frameworkFailoverTimeout,
framework->id(),
framework->reregisteredTime);
```

However, how to be able to assert that A. would be fine even though we try to 
avoid "jaggedness" is a real problem to me. I wondered if we could get closer 
to that "definitive answer". Any input is greatly appreciated :)


- Till


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


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> ---
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35744: Cleaned up the unknown orphans after all known orphans are recovered.

[Paul Brett]

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

Ship it!


LGTM

- Paul Brett


On June 22, 2015, 9:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35744/
> ---
> 
> (Updated June 22, 2015, 9:58 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the unknown orphans after all known orphans are recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 137cdc9b064b02e829af857cff7b9023ee7b7b16 
> 
> Diff: https://reviews.apache.org/r/35744/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Will follow up a test later.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35743: flags: fixed const'ness of load

[Anand Mazumdar]

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

Ship it!



3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp (line 413)


boost::size(...) would suffice here too albeit minus the small trivial 
run-time cost.


- Anand Mazumdar


On June 22, 2015, 9:27 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35743/
> ---
> 
> (Updated June 22, 2015, 9:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Changes:
>   - replacing argument arrays with static initializers in tests.
>   - making the argument 'argv' const in load method to reflect the semantics.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
> 7584cb871d02ad01021f0c3439ea205736d4f6b4 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> c2c6a6ac97044f2317418295f48d75e94de4112b 
> 
> Diff: https://reviews.apache.org/r/35743/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 35744: Cleaned up the unknown orphans after all known orphans are recovered.

[Chi Zhang]

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

Ship it!


Ship It!

- Chi Zhang


On June 22, 2015, 9:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35744/
> ---
> 
> (Updated June 22, 2015, 9:58 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the unknown orphans after all known orphans are recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 137cdc9b064b02e829af857cff7b9023ee7b7b16 
> 
> Diff: https://reviews.apache.org/r/35744/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Will follow up a test later.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35744: Cleaned up the unknown orphans after all known orphans are recovered.

[Jie Yu]

> On June 22, 2015, 9:37 p.m., Chi Zhang wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1886-1889
> > 
> >
> > If we have 0 known orphans, 3 unknown orphans, we'd try to delete ICMP 
> > and ARP 3 times in the new loop?
> 
> Jie Yu wrote:
> Yeah, that's the reason I put that TODO. The second `_cleanup` will fail 
> and the recovery will fail. The slave will restart and cleanup the 3rd 
> unknown orphan. So it's OK for now.

I added a NOTE about that.


- Jie


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


On June 22, 2015, 9:58 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35744/
> ---
> 
> (Updated June 22, 2015, 9:58 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the unknown orphans after all known orphans are recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 137cdc9b064b02e829af857cff7b9023ee7b7b16 
> 
> Diff: https://reviews.apache.org/r/35744/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Will follow up a test later.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35744: Cleaned up the unknown orphans after all known orphans are recovered.

[Jie Yu]

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

(Updated June 22, 2015, 9:58 p.m.)


Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.


Changes
---

Added a NOTE per Chi's comment.


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


Repository: mesos


Description
---

Cleaned up the unknown orphans after all known orphans are recovered.


Diffs (updated)
-

  src/slave/containerizer/isolators/network/port_mapping.cpp 
137cdc9b064b02e829af857cff7b9023ee7b7b16 

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


Testing
---

sudo make check

Will follow up a test later.


Thanks,

Jie Yu

Re: Review Request 35744: Cleaned up the unknown orphans after all known orphans are recovered.

[Jie Yu]

> On June 22, 2015, 9:37 p.m., Chi Zhang wrote:
> > src/slave/containerizer/isolators/network/port_mapping.cpp, lines 1886-1889
> > 
> >
> > If we have 0 known orphans, 3 unknown orphans, we'd try to delete ICMP 
> > and ARP 3 times in the new loop?

Yeah, that's the reason I put that TODO. The second `_cleanup` will fail and 
the recovery will fail. The slave will restart and cleanup the 3rd unknown 
orphan. So it's OK for now.


- Jie


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


On June 22, 2015, 9:25 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35744/
> ---
> 
> (Updated June 22, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the unknown orphans after all known orphans are recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 137cdc9b064b02e829af857cff7b9023ee7b7b16 
> 
> Diff: https://reviews.apache.org/r/35744/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Will follow up a test later.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 35744: Cleaned up the unknown orphans after all known orphans are recovered.

[Chi Zhang]

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



src/slave/containerizer/isolators/network/port_mapping.cpp (lines 1881 - 1884)


If we have 0 known orphans, 3 unknown orphans, we'd try to delete ICMP and 
ARP 3 times in the new loop?


- Chi Zhang


On June 22, 2015, 9:25 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35744/
> ---
> 
> (Updated June 22, 2015, 9:25 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.
> 
> 
> Bugs: MESOS-2914
> https://issues.apache.org/jira/browse/MESOS-2914
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up the unknown orphans after all known orphans are recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/isolators/network/port_mapping.cpp 
> 137cdc9b064b02e829af857cff7b9023ee7b7b16 
> 
> Diff: https://reviews.apache.org/r/35744/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Will follow up a test later.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 35743: flags: fixed const'ness of load

[Jojy Varghese]

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

Review request for mesos, Benjamin Hindman and Till Toenshoff.


Repository: mesos


Description
---

Changes:
  - replacing argument arrays with static initializers in tests.
  - making the argument 'argv' const in load method to reflect the semantics.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
7584cb871d02ad01021f0c3439ea205736d4f6b4 
  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
c2c6a6ac97044f2317418295f48d75e94de4112b 

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


Testing
---

make check


Thanks,

Jojy Varghese

Review Request 35744: Cleaned up the unknown orphans after all known orphans are recovered.

[Jie Yu]

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

Review request for mesos, Chi Zhang, Paul Brett, and Vinod Kone.


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


Repository: mesos


Description
---

Cleaned up the unknown orphans after all known orphans are recovered.


Diffs
-

  src/slave/containerizer/isolators/network/port_mapping.cpp 
137cdc9b064b02e829af857cff7b9023ee7b7b16 

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


Testing
---

sudo make check

Will follow up a test later.


Thanks,

Jie Yu

Re: Review Request 35714: Added a new HTTP response type: PreconditionFailed.

[Michael Park]

> On June 22, 2015, 8:04 p.m., Ben Mahler wrote:
> > Adding this seems good since it's agnostic to how it is used. But for 
> > reservations specifically, any reason not to use just BadRequest?
> > 
> > From what I can tell, this is generally used for conditional requests 
> > (based on headers or some other conditionals): 
> > http://stackoverflow.com/a/5369582
> > What will the pre-conditions be for /reserve? If the request does not 
> > contain explicit pre-conditions, it seems a little non-idiomatic to return 
> > 412..?

I was hoping to keep `BadRequest` to situations where the user has provided 
bad/invalid arguments. e.g. invalid JSON format, missing parameters, invalid 
resources, etc.

I was aiming to use `PreconditionFailed` to represent the case where the user 
have provided perfectly valid arguments, but we don't currently have sufficient 
resources to satisfy the request. The precondition here is whether we have 
sufficient resources or not. I'm of course open to using `BadRequest` with an 
explanation or what went wrong, or perhaps using a different status. (e.g. 
`Conflict` was suggested)

I'm not an expert on HTTP statuses and so I don't have a strong stance here, do 
you have a strong opinion or stance on this? I'm really open to anything.


- Michael


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


On June 22, 2015, 5:08 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35714/
> ---
> 
> (Updated June 22, 2015, 5:08 a.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Needed in subsequent patch for /reserve master endpoint.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> e47cc7afbc8110759edf25a2dc05d09eda25c417 
> 
> Diff: https://reviews.apache.org/r/35714/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35701: Minor formatting cleanup in the Master.

[Ben Mahler]

> On June 22, 2015, 6:26 p.m., Till Toenshoff wrote:
> > src/master/master.cpp, lines 972-975
> > 
> >
> > I understand this was updated due to some discussions on related RRs. I 
> > would however love to see the definitive answer to 
> > https://issues.apache.org/jira/browse/MESOS-2618 for preventing any 
> > subjective back and forth. The key-question here is "when are we reaching 
> > the jaggedness problem"?
> > 
> > Would you mind adding a comment to the ticket with your recent findings 
> > and experience from those other RRs?

Hey Till, the particular case Michael fixed here was bad alignment: it doesn't 
fit any of the options in the style guide. So I doubt there would be any 
controversy here :)


- Ben


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


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> ---
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35738: Add slave metric to count container launch failures.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35738]

All tests passed.

- Mesos ReviewBot


On June 22, 2015, 7:26 p.m., Paul Brett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35738/
> ---
> 
> (Updated June 22, 2015, 7:26 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.
> 
> 
> Bugs: MESOS-2904
> https://issues.apache.org/jira/browse/MESOS-2904
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add slave metric to count container launch failures.
> 
> 
> Diffs
> -
> 
>   src/slave/metrics.hpp 780c438b07993253c75275fd3956f1631b3c0d73 
>   src/slave/metrics.cpp 87289f2da83f211e1244d425c1d003e2ae79b507 
>   src/slave/slave.cpp 946e372252e6188b28f94452f1479824a7e201cd 
>   src/tests/metrics_tests.cpp 6896727a23c1d6b0e2055ee661042a0a66b09067 
>   src/tests/slave_tests.cpp 50301983c674ef50a64294816db9587bf065aa9e 
> 
> Diff: https://reviews.apache.org/r/35738/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Paul Brett
> 
>

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

[Anindya Sinha]

> On June 11, 2015, 7:34 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 820-836
> > 
> >
> > If two libprocess based unix processes (e.g., scheudler and master) are 
> > within the *same* bridged container, would they able to communicate with 
> > this change? Can you test this to confirm?
> > 
> > 
> > If not, a better option might be to instead have LIBPROCESS_BIND_IP and 
> > LIBPROCESS_BIND_PORT that just changes the address we bind to. 
> > LIBPROCESS_IP and LIBPROCESS_PORT semantics could be left untouched.
> 
> Anindya Sinha wrote:
> If 2 libprocess based unix processes are running, they would point to a 
> different  (most likely same public_ip but a different 
> public_port, ie. same LIBPROCESS_PUBLIC_IP but a different 
> LIBPROCESS_PUBLIC_PORT). The processes themselves would bind as it does today 
> on  (based in LIBPROCESS_IP and LIBPROCESS_PORT). Once a request 
> lands on a corresponding , a proxy listening on that 
> would forward that to the actual  corresponding to the 
> .
> 
> As an example, mesos-master binds on 10.11.12.13:5050 (ip:port) with 
> public_ip:public_port as 192.168.100.100:6050, and say scheduler binds on 
> 10.11.12.13:8081 with public_ip:public_port as 192.168.100.100:9081. Requests 
> received on 192.168.100.100:6050 shall be proxied over to 10.11.12.13:5050 
> (to reach mesos-master) and requests received on 192.168.100.100:9081 shall 
> be proxied over to 10.11.12.13:8081 (to reach scheduler).
> 
> Vinod Kone wrote:
> ```
> a proxy listening on that would forward that...
> ```
> 
> who sets up this proxy? or do you mean this is what happens currently in 
> bridged mode containers (e.g., docker)?

No this is not something that is part of docker. The proxy would be something 
external to the container which would proxy the request to your application 
within the container.
This patch enables external entities viz. zookeeper reach the host (based on 
public_ip:public_port) from where a proxy will be required to route to the 
application running within the container. If we use , zookeeper won't 
be able to reach. The setting up of the proxy is not a part of docker or 
libprocess.
Please also refer to relevant issue 
https://issues.apache.org/jira/browse/MESOS-2587.


- Anindya


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


On May 18, 2015, 10:08 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34128/
> ---
> 
> (Updated May 18, 2015, 10:08 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-809
> https://issues.apache.org/jira/browse/MESOS-809
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose environment variables LIBPROCESS_PUBLIC_IP and LIBPROCESS_PUBLIC_PORT 
> as the IP and
> port which libprocess would advertise (if set). If not set, it defaults to
> the IP and port on which it binded to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> e3de3cd6b536aaaf59784360aed546512dd04dc9 
> 
> Diff: https://reviews.apache.org/r/34128/diff/
> 
> 
> Testing
> ---
> 
> Testing:
>   make test
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 35695: stout: Fixed test names.

[Ben Mahler]

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

Ship it!


Thanks, will commit this for you.

Ditto the previous question about the "Test" suffix, curious to hear your 
thoughts on it.

- Ben Mahler


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35695/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 60c0336082caf30b2d7943d85ef3bb7a534648d8 
>   3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp 
> 4dbe4bab79f7536a6a7a53348f44f8ac888d3d1d 
>   3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
> f2386d55c658a5bbdedb6b433c70f7739f6ac0d4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
> ad79a1677fb0bb92215e29f98a17c97d5309279c 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 62987e0df28f28816c59d7cbad89fa2af41ade04 
>   3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp 
> d3c9aede8e8a7f419f8eae7a7f52629d5cc1513f 
>   3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 
> 7519b12d7334b16496377f60b2e830401dd0db86 
>   3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 
> ad1d986de72a51540ebeb56db7eb399fb8e66f87 
> 
> Diff: https://reviews.apache.org/r/35695/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35697: mesos: Fixed test names.

[Ben Mahler]

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

Ship it!


Thanks, will commit this for you.

Also we are inconcistent on the "Test" suffix, e.g. "MasterTest" vs 
"CRAMMD5Authentication". See two options:

(1) We can use "Test" to signify that there is a fixture, if that's helpful. So 
remove any "Test" suffixes from non-fixture tests.
(2) We can use "Test" as a suffix for all tests.

Thoughts?

- Ben Mahler


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35697/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   src/tests/cram_md5_authentication_tests.cpp 
> 992302365682df2c800c8828a21b9d38cb318472 
>   src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f 
>   src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a 
> 
> Diff: https://reviews.apache.org/r/35697/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35714: Added a new HTTP response type: PreconditionFailed.

[Ben Mahler]

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


Adding this seems good since it's agnostic to how it is used. But for 
reservations specifically, any reason not to use just BadRequest?

>From what I can tell, this is generally used for conditional requests (based 
>on headers or some other conditionals): http://stackoverflow.com/a/5369582
What will the pre-conditions be for /reserve? If the request does not contain 
explicit pre-conditions, it seems a little non-idiomatic to return 412..?

- Ben Mahler


On June 22, 2015, 5:08 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35714/
> ---
> 
> (Updated June 22, 2015, 5:08 a.m.)
> 
> 
> Review request for mesos, Adam B and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Needed in subsequent patch for /reserve master endpoint.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> e47cc7afbc8110759edf25a2dc05d09eda25c417 
> 
> Diff: https://reviews.apache.org/r/35714/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35699: Added an invariant CHECK_EQ for available resources in HierarchicalAllocator::updateAllocation.

[Ben Mahler]

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

Ship it!


Thanks for taking care of this!

- Ben Mahler


On June 20, 2015, 7:52 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35699/
> ---
> 
> (Updated June 20, 2015, 7:52 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 646ee8c1c0fb824e1d17150b4e96e6281c65358f 
> 
> Diff: https://reviews.apache.org/r/35699/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35696: libprocess: Fixed test names.

[Ben Mahler]

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

Ship it!


I'll get this committed for you, one other inconsistency is that some tests are 
"Process" and some tests are named "JsonTest". The latter tends to be forced 
when we have a test fixture (e.g. MasterTest).

- Ben Mahler


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35696/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 86b2c7d3beae6671474cf0a5ad52a10334bc9230 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 84e5d746220dfa274465632280de87dc5e7f673a 
>   3rdparty/libprocess/src/tests/mutex_tests.cpp 
> 451b234ca46dd91bff6d8463a00a40fb14070db6 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 660af45e7fd45bdf5d43ad9aa54477fd94f87058 
>   3rdparty/libprocess/src/tests/queue_tests.cpp 
> 79741d37cb761593b38f21eef466d0313220d721 
>   3rdparty/libprocess/src/tests/statistics_tests.cpp 
> 0e4960a4e2e807ee9f3f6d83cdbfa6b563352760 
>   3rdparty/libprocess/src/tests/timeseries_tests.cpp 
> fd906b869e8bbf5c07c67c5319dec82681e811b7 
> 
> Diff: https://reviews.apache.org/r/35696/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 34908: Rename --docker_sandbox_directory flag for general use.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [34908]

All tests passed.

- Mesos ReviewBot


On June 22, 2015, 4:55 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34908/
> ---
> 
> (Updated June 22, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rename --docker_sandbox_directory flag for general use.
> 
> This will require users to update configuration scripts etc if they specify 
> the old flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md aaf65bfa2848318c8d07c772ba2c521aa72c2677 
>   src/slave/containerizer/docker.cpp 00db9811824995fe19a6143aa2bcba3733a93147 
>   src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
>   src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
>   src/tests/docker_containerizer_tests.cpp 
> 3a983c6813dab6fa03ccb2c87e1ea71866766d6e 
> 
> Diff: https://reviews.apache.org/r/34908/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Review Request 35738: Add slave metric to count container launch failures.

[Paul Brett]

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

Review request for mesos, Chi Zhang, Ian Downes, Jie Yu, and Cong Wang.


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


Repository: mesos


Description
---

Add slave metric to count container launch failures.


Diffs
-

  src/slave/metrics.hpp 780c438b07993253c75275fd3956f1631b3c0d73 
  src/slave/metrics.cpp 87289f2da83f211e1244d425c1d003e2ae79b507 
  src/slave/slave.cpp 946e372252e6188b28f94452f1479824a7e201cd 
  src/tests/metrics_tests.cpp 6896727a23c1d6b0e2055ee661042a0a66b09067 
  src/tests/slave_tests.cpp 50301983c674ef50a64294816db9587bf065aa9e 

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


Testing
---

sudo make check


Thanks,

Paul Brett

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

[Vinod Kone]

> On June 11, 2015, 7:34 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 820-836
> > 
> >
> > If two libprocess based unix processes (e.g., scheudler and master) are 
> > within the *same* bridged container, would they able to communicate with 
> > this change? Can you test this to confirm?
> > 
> > 
> > If not, a better option might be to instead have LIBPROCESS_BIND_IP and 
> > LIBPROCESS_BIND_PORT that just changes the address we bind to. 
> > LIBPROCESS_IP and LIBPROCESS_PORT semantics could be left untouched.
> 
> Anindya Sinha wrote:
> If 2 libprocess based unix processes are running, they would point to a 
> different  (most likely same public_ip but a different 
> public_port, ie. same LIBPROCESS_PUBLIC_IP but a different 
> LIBPROCESS_PUBLIC_PORT). The processes themselves would bind as it does today 
> on  (based in LIBPROCESS_IP and LIBPROCESS_PORT). Once a request 
> lands on a corresponding , a proxy listening on that 
> would forward that to the actual  corresponding to the 
> .
> 
> As an example, mesos-master binds on 10.11.12.13:5050 (ip:port) with 
> public_ip:public_port as 192.168.100.100:6050, and say scheduler binds on 
> 10.11.12.13:8081 with public_ip:public_port as 192.168.100.100:9081. Requests 
> received on 192.168.100.100:6050 shall be proxied over to 10.11.12.13:5050 
> (to reach mesos-master) and requests received on 192.168.100.100:9081 shall 
> be proxied over to 10.11.12.13:8081 (to reach scheduler).

```
a proxy listening on that would forward that...
```

who sets up this proxy? or do you mean this is what happens currently in 
bridged mode containers (e.g., docker)?


- Vinod


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


On May 18, 2015, 10:08 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34128/
> ---
> 
> (Updated May 18, 2015, 10:08 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-809
> https://issues.apache.org/jira/browse/MESOS-809
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose environment variables LIBPROCESS_PUBLIC_IP and LIBPROCESS_PUBLIC_PORT 
> as the IP and
> port which libprocess would advertise (if set). If not set, it defaults to
> the IP and port on which it binded to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> e3de3cd6b536aaaf59784360aed546512dd04dc9 
> 
> Diff: https://reviews.apache.org/r/34128/diff/
> 
> 
> Testing
> ---
> 
> Testing:
>   make test
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

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

[Anindya Sinha]

> On June 11, 2015, 7:34 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 820-836
> > 
> >
> > If two libprocess based unix processes (e.g., scheudler and master) are 
> > within the *same* bridged container, would they able to communicate with 
> > this change? Can you test this to confirm?
> > 
> > 
> > If not, a better option might be to instead have LIBPROCESS_BIND_IP and 
> > LIBPROCESS_BIND_PORT that just changes the address we bind to. 
> > LIBPROCESS_IP and LIBPROCESS_PORT semantics could be left untouched.

If 2 libprocess based unix processes are running, they would point to a 
different  (most likely same public_ip but a different 
public_port, ie. same LIBPROCESS_PUBLIC_IP but a different 
LIBPROCESS_PUBLIC_PORT). The processes themselves would bind as it does today 
on  (based in LIBPROCESS_IP and LIBPROCESS_PORT). Once a request lands 
on a corresponding , a proxy listening on that would 
forward that to the actual  corresponding to the 
.

As an example, mesos-master binds on 10.11.12.13:5050 (ip:port) with 
public_ip:public_port as 192.168.100.100:6050, and say scheduler binds on 
10.11.12.13:8081 with public_ip:public_port as 192.168.100.100:9081. Requests 
received on 192.168.100.100:6050 shall be proxied over to 10.11.12.13:5050 (to 
reach mesos-master) and requests received on 192.168.100.100:9081 shall be 
proxied over to 10.11.12.13:8081 (to reach scheduler).


- Anindya


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


On May 18, 2015, 10:08 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34128/
> ---
> 
> (Updated May 18, 2015, 10:08 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-809
> https://issues.apache.org/jira/browse/MESOS-809
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expose environment variables LIBPROCESS_PUBLIC_IP and LIBPROCESS_PUBLIC_PORT 
> as the IP and
> port which libprocess would advertise (if set). If not set, it defaults to
> the IP and port on which it binded to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> e3de3cd6b536aaaf59784360aed546512dd04dc9 
> 
> Diff: https://reviews.apache.org/r/34128/diff/
> 
> 
> Testing
> ---
> 
> Testing:
>   make test
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [34136, 34134, 34135]

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

Error:
 2015-06-22 18:32:03 URL:https://reviews.apache.org/r/34135/diff/raw/ 
[37213/37213] -> "34135.patch" [1]
error: patch failed: src/slave/containerizer/mesos/containerizer.cpp:120
error: src/slave/containerizer/mesos/containerizer.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On June 22, 2015, 4:53 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34427/
> ---
> 
> (Updated June 22, 2015, 4:53 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Note: This is a specialized backend; see notes in bind_backend.hpp.
>  
> Reproduced here for your convenience:
> 
> This is a specialized backend that may be useful for deployments
> using large (multi-GB) single-layer images *and* where more
> recent kernel features such as overlayfs are not available. For small
> images (10's to 100's of MB) the Copy backend may be sufficient.
> 1) It supports only a single layer. Multi-layer images will fail
>to provision and the container will fail to launch!
> 2) The filesystem is read-only because all containers using this
>image share the source. Select writable areas can be achieved
>by
>mounting read-write volumes to places like /tmp, /var/tmp,
>/home, etc. using the ContainerInfo. These can be relative to
>the executor work directory.
> 3) It relies on the image persisting in the store.
> 4) It's fast because the bind mount requires (nearly) zero IO.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/slave/containerizer/provisioners/appc/backend.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc/bind_backend.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34427/diff/
> 
> 
> Testing
> ---
> 
> manual testing of a single layer image with RW relative bind mount for /tmp.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 35701: Minor formatting cleanup in the Master.

[Till Toenshoff]

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



src/master/master.cpp (lines 972 - 975)


I understand this was updated due to some discussions on related RRs. I 
would however love to see the definitive answer to 
https://issues.apache.org/jira/browse/MESOS-2618 for preventing any subjective 
back and forth. The key-question here is "when are we reaching the jaggedness 
problem"?

Would you mind adding a comment to the ticket with your recent findings and 
experience from those other RRs?


- Till Toenshoff


On June 20, 2015, 8 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35701/
> ---
> 
> (Updated June 20, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 4c8102e3cd75e9284dac3d535545370ca37f502c 
>   src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d 
> 
> Diff: https://reviews.apache.org/r/35701/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35033: Define potentially missing MS_* mount flags.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [35033]

All tests passed.

- Mesos ReviewBot


On June 22, 2015, 4:51 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35033/
> ---
> 
> (Updated June 22, 2015, 4:51 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> May be missing in sys/mount.h but supported by the kernel.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp d7832a4b3761c48be6c1ccef58a30ee31c70dc1b 
> 
> Diff: https://reviews.apache.org/r/35033/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Kapil Arya]

> On June 18, 2015, 6:08 p.m., Niklas Nielsen wrote:
> > A few high level comments:
> > 1) Let's use an enum instead of an unstructured int to encode the namespaces
> > 2) Let's get a test wired up
> > 3) Think about Mac compatibility while introducing the enum :)

- Added an enum that is protected by __linux__.


- Kapil


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


On June 22, 2015, 1:56 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35585/
> ---
> 
> (Updated June 22, 2015, 1:56 p.m.)
> 
> 
> Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2884
> https://issues.apache.org/jira/browse/MESOS-2884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would enable the MesosContainerizer to pass on a list of namespaces
> to LinuxLauncher instead of having LinuxLauncher guess it from the
> isolation flags.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
>   src/slave/containerizer/isolator.cpp 
> d51ecc9347cafef90c92a6965e37f417b4929e79 
>   src/slave/containerizer/isolators/filesystem/shared.hpp 
> 08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
>   src/slave/containerizer/isolators/namespaces/pid.hpp 
> 6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
>   src/slave/containerizer/isolators/network/port_mapping.hpp 
> ee869ce71a68f51d58159c701d3c5d1e502b 
> 
> Diff: https://reviews.apache.org/r/35585/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 35585: Updated Isolator to return required namespaces.

[Kapil Arya]

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

(Updated June 22, 2015, 1:56 p.m.)


Review request for mesos, Ian Downes, Jie Yu, and Niklas Nielsen.


Changes
---

Addressed two of Nik's comments.


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


Repository: mesos


Description
---

This would enable the MesosContainerizer to pass on a list of namespaces
to LinuxLauncher instead of having LinuxLauncher guess it from the
isolation flags.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/slave/containerizer/isolator.cpp d51ecc9347cafef90c92a6965e37f417b4929e79 
  src/slave/containerizer/isolators/filesystem/shared.hpp 
08c6ffea0e2fada8e0b04f4ab15d8569c5416a8e 
  src/slave/containerizer/isolators/namespaces/pid.hpp 
6b24e2990311d0b09b3f6f2bb4ab29ee83fc 
  src/slave/containerizer/isolators/network/port_mapping.hpp 
ee869ce71a68f51d58159c701d3c5d1e502b 

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


Testing
---

make check


Thanks,

Kapil Arya

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

[Jie Yu]

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



src/tests/launch_tests.cpp (lines 118 - 120)


This is not needed?



src/tests/launch_tests.cpp (lines 186 - 187)


No snake case please. Also, do you still need this?


- Jie Yu


On June 22, 2015, 4:38 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31444/
> ---
> 
> (Updated June 22, 2015, 4:38 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, 
> and James Peach.
> 
> 
> Bugs: MESOS-2350
> https://issues.apache.org/jira/browse/MESOS-2350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Optionally take a path that the launch helper should chroot to before 
> exec'ing the executor. It is assumed that the work directory is mounted to 
> the appropriate location under the chroot. In particular, the path to the 
> executor must be relative to the chroot.
> 
> Configuration that should be private to the chroot is done during the launch, 
> e.g. mounting proc and statically configuring basic devices. It is assumed 
> that other configuration, e.g., preparing the image, mounting in volumes or 
> persistent resources, is done by the caller.
> 
> Mounts can be made to the chroot (e.g., updating the volumes or persistent 
> resources) and they will propagate in to the container but mounts made inside 
> the container will not propagate out to the host.
> 
> It currently assumes that at least {{chroot}}/tmp is writeable and that mount 
> points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
> 
> This is specific to Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/linux/fs.cpp 568565f878b34708170a886dc4d62849aa01f263 
>   src/slave/containerizer/mesos/launch.hpp 
> 7c8b535746b5ce9add00afef86fdb6faefb5620e 
>   src/slave/containerizer/mesos/launch.cpp 
> 2f2d60e2011f60ec711d3b29fd2c157e30c83c34 
>   src/tests/launch_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31444/diff/
> 
> 
> Testing
> ---
> 
> Manual testing only so far. This is harder to automate because we need a 
> self-contained chroot to execute something in... Suggestions welcome.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 35686: Change the if statement to a CHECK at the end of _runTask.

[Jie Yu]

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

Ship it!


Ship It!

- Jie Yu


On June 20, 2015, 4:42 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35686/
> ---
> 
> (Updated June 20, 2015, 4:42 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Follow-up from [r35433](https://reviews.apache.org/r/35433/#comment141187).
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e73913b072e323033a81e64b5345cb9457137e3e 
> 
> Diff: https://reviews.apache.org/r/35686/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [32891, 32978, 31444]

All tests passed.

- Mesos ReviewBot


On June 22, 2015, 4:38 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31444/
> ---
> 
> (Updated June 22, 2015, 4:38 p.m.)
> 
> 
> Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, 
> and James Peach.
> 
> 
> Bugs: MESOS-2350
> https://issues.apache.org/jira/browse/MESOS-2350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Optionally take a path that the launch helper should chroot to before 
> exec'ing the executor. It is assumed that the work directory is mounted to 
> the appropriate location under the chroot. In particular, the path to the 
> executor must be relative to the chroot.
> 
> Configuration that should be private to the chroot is done during the launch, 
> e.g. mounting proc and statically configuring basic devices. It is assumed 
> that other configuration, e.g., preparing the image, mounting in volumes or 
> persistent resources, is done by the caller.
> 
> Mounts can be made to the chroot (e.g., updating the volumes or persistent 
> resources) and they will propagate in to the container but mounts made inside 
> the container will not propagate out to the host.
> 
> It currently assumes that at least {{chroot}}/tmp is writeable and that mount 
> points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.
> 
> This is specific to Linux.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
>   src/linux/fs.cpp 568565f878b34708170a886dc4d62849aa01f263 
>   src/slave/containerizer/mesos/launch.hpp 
> 7c8b535746b5ce9add00afef86fdb6faefb5620e 
>   src/slave/containerizer/mesos/launch.cpp 
> 2f2d60e2011f60ec711d3b29fd2c157e30c83c34 
>   src/tests/launch_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/31444/diff/
> 
> 
> Testing
> ---
> 
> Manual testing only so far. This is harder to automate because we need a 
> self-contained chroot to execute something in... Suggestions welcome.
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 35695: stout: Fixed test names.

[Till Toenshoff]

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

Ship it!


Ship It!

- Till Toenshoff


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35695/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
> 60c0336082caf30b2d7943d85ef3bb7a534648d8 
>   3rdparty/libprocess/3rdparty/stout/tests/mac_tests.cpp 
> 4dbe4bab79f7536a6a7a53348f44f8ac888d3d1d 
>   3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
> f2386d55c658a5bbdedb6b433c70f7739f6ac0d4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/signals_tests.cpp 
> ad79a1677fb0bb92215e29f98a17c97d5309279c 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 62987e0df28f28816c59d7cbad89fa2af41ade04 
>   3rdparty/libprocess/3rdparty/stout/tests/proc_tests.cpp 
> d3c9aede8e8a7f419f8eae7a7f52629d5cc1513f 
>   3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 
> 7519b12d7334b16496377f60b2e830401dd0db86 
>   3rdparty/libprocess/3rdparty/stout/tests/uuid_tests.cpp 
> ad1d986de72a51540ebeb56db7eb399fb8e66f87 
> 
> Diff: https://reviews.apache.org/r/35695/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 35697: mesos: Fixed test names.

[Till Toenshoff]

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

Ship it!


Thanks a bunch for these.

- Till Toenshoff


On June 20, 2015, 7:14 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35697/
> ---
> 
> (Updated June 20, 2015, 7:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> First noticed `JsonTest.parse`, then found the other cases with the following 
> regex: `TEST.*(.*, [a-z].*`.
> 
> 
> Diffs
> -
> 
>   src/tests/cram_md5_authentication_tests.cpp 
> 992302365682df2c800c8828a21b9d38cb318472 
>   src/tests/credentials_tests.cpp ed672733ff895b1f252fa75d5653a9f3527d5b5f 
>   src/tests/registrar_tests.cpp 97d0b685dd2744cf92b3a463d586e8bcf4c0af1a 
> 
> Diff: https://reviews.apache.org/r/35697/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 34142: AppC provisioner.

[Ian Downes]

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

(Updated June 22, 2015, 9:57 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

- Use protobuf representation of Appc Image Manifest
- Implemented recover


Repository: mesos


Description
---

Discovers image(s), fetches to the image store, then provisions using
a backend.


Diffs (updated)
-

  include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/slave/containerizer/provisioners/appc.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34908: Rename --docker_sandbox_directory flag for general use.

[Ian Downes]

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

(Updated June 22, 2015, 9:55 a.m.)


Review request for mesos and Timothy Chen.


Repository: mesos


Description
---

Rename --docker_sandbox_directory flag for general use.

This will require users to update configuration scripts etc if they specify the 
old flag.


Diffs (updated)
-

  docs/configuration.md aaf65bfa2848318c8d07c772ba2c521aa72c2677 
  src/slave/containerizer/docker.cpp 00db9811824995fe19a6143aa2bcba3733a93147 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/tests/docker_containerizer_tests.cpp 
3a983c6813dab6fa03ccb2c87e1ea71866766d6e 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34139: AppC image discovery.

[Ian Downes]

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

(Updated June 22, 2015, 9:54 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Flag name changes. Switched to discovery returning at most a single URI.


Repository: mesos


Description
---

Local and simple discovery only.


Diffs (updated)
-

  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/provisioners/appc/discovery.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc/discovery.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34141: AppC provsioning backend.

[Ian Downes]

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

(Updated June 22, 2015, 9:53 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Flag rename, code cleanup, async rm.


Repository: mesos


Description
---

Simple copy backend that forms the rootfs by copying each layer.


Diffs (updated)
-

  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/provisioners/appc/backend.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc/backend.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 

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


Testing
---


Thanks,

Ian Downes

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

[Ian Downes]

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

(Updated June 22, 2015, 9:53 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Minor code changes to address comments.


Repository: mesos


Description
---

Note: This is a specialized backend; see notes in bind_backend.hpp.
 
Reproduced here for your convenience:

This is a specialized backend that may be useful for deployments
using large (multi-GB) single-layer images *and* where more
recent kernel features such as overlayfs are not available. For small
images (10's to 100's of MB) the Copy backend may be sufficient.
1) It supports only a single layer. Multi-layer images will fail
   to provision and the container will fail to launch!
2) The filesystem is read-only because all containers using this
   image share the source. Select writable areas can be achieved
   by
   mounting read-write volumes to places like /tmp, /var/tmp,
   /home, etc. using the ContainerInfo. These can be relative to
   the executor work directory.
3) It relies on the image persisting in the store.
4) It's fast because the bind mount requires (nearly) zero IO.


Diffs (updated)
-

  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/provisioners/appc/backend.cpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc/bind_backend.hpp PRE-CREATION 

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


Testing
---

manual testing of a single layer image with RW relative bind mount for /tmp.


Thanks,

Ian Downes

Re: Review Request 35033: Define potentially missing MS_* mount flags.

[Ian Downes]

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

(Updated June 22, 2015, 9:51 a.m.)


Review request for mesos, Jie Yu and Vinod Kone.


Changes
---

Added comment on source of #defines. No code change.


Repository: mesos


Description
---

May be missing in sys/mount.h but supported by the kernel.


Diffs (updated)
-

  src/linux/fs.hpp d7832a4b3761c48be6c1ccef58a30ee31c70dc1b 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34140: AppC image store

[Ian Downes]

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

(Updated June 22, 2015, 9:51 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Renamed flag and invoke fetcher properly.


Repository: mesos


Description
---

Images are fetched into the store (after discovery). Stored images are
currently kept indefinitely.


Diffs (updated)
-

  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/provisioners/appc/store.hpp PRE-CREATION 
  src/slave/containerizer/provisioners/appc/store.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34138: AppC hash computation.

[Ian Downes]

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

(Updated June 22, 2015, 9:46 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Improve multiplatform support: check a few alternatives to find something to 
compute a sha-512 file hash.


Repository: mesos


Description
---

AppC hash computation.


Diffs (updated)
-

  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/provisioners/appc/hash.hpp PRE-CREATION 

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


Testing
---


Thanks,

Ian Downes

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

[Ian Downes]

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

(Updated June 22, 2015, 9:44 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

- Support multiple provisioners
- Add recover() to provisioner interface
- Checkpoint and recover optional container rootfs


Repository: mesos


Description
---

The MesosContainerizer can optionally provision a root filesystem for the 
containers.


Diffs (updated)
-

  include/mesos/slave/isolator.hpp 18edc030367e42240090f2f3dbc92aec7a4c6234 
  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/mesos/containerizer.hpp 
3ac2387eabded61c897a5016655aae10cd1bca91 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 
  src/slave/containerizer/provisioner.hpp PRE-CREATION 
  src/slave/containerizer/provisioner.cpp PRE-CREATION 
  src/slave/flags.hpp 7634e368c72e83932dcd992d78eaca146326606b 
  src/slave/flags.cpp cbf431eb0627bdaf07241cc0fc4630df06fb20e2 
  src/slave/paths.hpp 00476f107ed8233123a6eab0925c9f28aac0a86f 
  src/slave/paths.cpp 0741616b656e947cb460dd6ee6a9a4852be001c2 
  src/slave/state.hpp fed4b7ecf9572a8dbb1a99dbb1769d3e55ef7bc5 
  src/slave/state.cpp 8eda22a550e5add0f84c46cc2ed762b006c0dcec 
  src/tests/containerizer_tests.cpp 0cdb2d2a3f19a4835e85c6b040759019b03f051e 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34136: Add ContainerImage protobuf.

[Ian Downes]

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

(Updated June 22, 2015, 9:42 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Changed the hierarchy so the ContainerInfo::Image is a union of different 
types, initially just Appc.


Repository: mesos


Description
---

Add ContainerImage protobuf.


Diffs (updated)
-

  include/mesos/mesos.proto 8df1211165169c9595e0e6e85b5ddc404345ff70 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 34135: Add filesystem/ isolators for persistent volumes.

[Ian Downes]

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

(Updated June 22, 2015, 9:41 a.m.)


Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.


Changes
---

Removed rootfs recovery from isolator in favor of checkpointing the rootfs and 
recoverying as part of the RunState.


Repository: mesos


Description
---

Moved code from Mesos Containerizer to filesystem isolators
 - filesystem/posix (symlinks, doesn't support container rootfs)
 - filesystem/linux (bind mounts, does support container rootfs)

The filesystem/posix isolator will be automatically included if no filesystem/ 
isolator is specified.


Diffs (updated)
-

  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/slave/containerizer/isolators/filesystem/linux.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/linux.cpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/posix.hpp PRE-CREATION 
  src/slave/containerizer/isolators/filesystem/posix.cpp PRE-CREATION 
  src/slave/containerizer/linux_launcher.cpp 
8eae258d81229e19f8c587f5e023b1df7deed025 
  src/slave/containerizer/mesos/containerizer.cpp 
8c102fb7d1f79ee768cb06de3a976ea12f958712 

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


Testing
---

existing persistent volumes tests.


Thanks,

Ian Downes

Re: Review Request 32891: Support for entering and configuring a Linux chroot.

[Ian Downes]

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

(Updated June 22, 2015, 9:38 a.m.)


Review request for mesos, Chi Zhang, Jay Buffington, Jie Yu, and James Peach.


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


Repository: mesos


Description
---

Support for entering and configuring a Linux chroot.


Diffs (updated)
-

  src/linux/fs.hpp d7832a4b3761c48be6c1ccef58a30ee31c70dc1b 
  src/linux/fs.cpp 568565f878b34708170a886dc4d62849aa01f263 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

[Ian Downes]

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

(Updated June 22, 2015, 9:38 a.m.)


Review request for mesos, Chi Zhang, Dominic Hamon, Jay Buffington, Jie Yu, and 
James Peach.


Changes
---

Addressed review comments, notably to ensure the host mount table is not 
polluted.


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


Repository: mesos


Description
---

Optionally take a path that the launch helper should chroot to before exec'ing 
the executor. It is assumed that the work directory is mounted to the 
appropriate location under the chroot. In particular, the path to the executor 
must be relative to the chroot.

Configuration that should be private to the chroot is done during the launch, 
e.g. mounting proc and statically configuring basic devices. It is assumed that 
other configuration, e.g., preparing the image, mounting in volumes or 
persistent resources, is done by the caller.

Mounts can be made to the chroot (e.g., updating the volumes or persistent 
resources) and they will propagate in to the container but mounts made inside 
the container will not propagate out to the host.

It currently assumes that at least {{chroot}}/tmp is writeable and that mount 
points {{chroot}}/{tmp,dev,proc,sys} exist in the chroot.

This is specific to Linux.


Diffs (updated)
-

  src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 
  src/linux/fs.cpp 568565f878b34708170a886dc4d62849aa01f263 
  src/slave/containerizer/mesos/launch.hpp 
7c8b535746b5ce9add00afef86fdb6faefb5620e 
  src/slave/containerizer/mesos/launch.cpp 
2f2d60e2011f60ec711d3b29fd2c157e30c83c34 
  src/tests/launch_tests.cpp PRE-CREATION 

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


Testing
---

Manual testing only so far. This is harder to automate because we need a 
self-contained chroot to execute something in... Suggestions welcome.


Thanks,

Ian Downes

Re: Review Request 35703: Set refuse seconds on the correct filter in reservation test.

[Alexander Rukletsov]

> On June 22, 2015, 2:13 p.m., Alexander Rukletsov wrote:
> > Looks like a copy-paste bug. How have you found it? Was the test flaky?
> 
> Michael Park wrote:
> I was using these tests as a reference for writing my new tests for 
> [r35702](https://reviews.apache.org/r/35702/) and noticed it.

I wonder why they were not flaky.


- Alexander


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


On June 20, 2015, 8:51 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35703/
> ---
> 
> (Updated June 20, 2015, 8:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `set_refuse_seconds` should have been called on `filtersForever` rather than 
> `filters`.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_tests.cpp 755a375346e0be290d4b8ffac3ecf5e7d191970d 
> 
> Diff: https://reviews.apache.org/r/35703/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>