date:20161109


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


Ship it!




Ship It!

- Jie Yu


On Nov. 8, 2016, 9:54 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53352/
> ---
> 
> (Updated Nov. 8, 2016, 9:54 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6464
> https://issues.apache.org/jira/browse/MESOS-6464
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of only taking the namespaces to *clone* inside a newly forked
> container, 'LinuxLauncher->fork()' now takes a list of namespaces to
> enter inside a parent container before forking. When both an *enter*
> and a *clone* for the same namespace are passed, we will first enter
> the namespace of the parent, and then clone a new one.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> f4c4ad771b5dead4ea3ee7cd1b4383c4dc2359b4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e57064c768937969ba4a071ae80165ccab2f1dff 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> af9f3736b487b595e8768e56ce60dc4823db28a1 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> df16b8fee6799a69c7d96f33a5049bd9787c48f5 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> a1283e5ee92c916baaf9fca8ce314d597e8421b3 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> e3756c920081f2944bf4b640edf0a83f42784586 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> 0d9ec57d9aa83bcc6cc2e5a8d75f2e2251179b1b 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 50b43777b1e470898de139e2dae398ebb2c0d6bb 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 48202fb5bf1ede71b80760844c6d8a36ca7c700c 
>   src/slave/containerizer/mesos/isolators/volume/image.cpp 
> 210e67ad0d84f52135e77184f21e574c9e31628d 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
> 7b976d29226c3e0a4d52922e9d2f7e685de72297 
>   src/slave/containerizer/mesos/launcher.hpp 
> 6ceb02de5dc143e545e2fec43e2608916e46b898 
>   src/slave/containerizer/mesos/launcher.cpp 
> b45313fd717f9553ccb0cbe9e8ac095e2536944a 
>   src/slave/containerizer/mesos/linux_launcher.hpp 
> d2353055a838c872d5852982cfede8e38c6e8701 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 0305d14c1f791c93edcd3b32786b483b15f40a2d 
>   src/tests/containerizer/isolator_tests.cpp 
> 8fefeef8c83ed2ab01f56a1ec572d3acb307143c 
>   src/tests/containerizer/launcher.hpp 
> 773b458f19e11b219c3f13a43f2b751a4bbe7b85 
>   src/tests/containerizer/launcher.cpp 
> a92d9890f0931425d69ef8ce0896d081b8722079 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 8cb7d4e5585eda36b875812b7cd914f98cf70487 
>   src/tests/containerizer/port_mapping_tests.cpp 
> fbdc0db9238c85d2f6eaba7d13ee5ce23342b527 
> 
> Diff: https://reviews.apache.org/r/53352/diff/
> 
> 
> Testing
> ---
> 
> make -j check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 53351: Added 'ContainerClass' to help decide how best to launch a container.


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


Ship it!




Ship It!

- Jie Yu


On Nov. 4, 2016, 9:32 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53351/
> ---
> 
> (Updated Nov. 4, 2016, 9:32 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6464
> https://issues.apache.org/jira/browse/MESOS-6464
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A 'ContainerClass' must now be passed to all *nested*
> containerizer->launch() calls in order to specify the class of
> container being launched. For now, we simply have the default nested
> container class (called 'NESTED') in a subsequent commit we will
> introduce the 'DEBUG' class to help classify debug-based nested
> containers that should be launched slightly differently than default
> nested containers.
> 
> The 'ContainerClass' specified is passed through to each isolator in
> 'ContainerConfig' so they have a chance to custiomize their isolation
> policies based on the class as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> f4c4ad771b5dead4ea3ee7cd1b4383c4dc2359b4 
>   src/slave/containerizer/composing.hpp 
> 68c0523bb96e1078bd95c6094bce04c38c2fc9dd 
>   src/slave/containerizer/composing.cpp 
> 0c5489f8dffaf75bce26a042000eaf7376f05ff4 
>   src/slave/containerizer/containerizer.hpp 
> 7554446ac51a9063bd52b8b99845c73650740849 
>   src/slave/containerizer/mesos/containerizer.hpp 
> c4fea8b56c39d5a363f6ea80bd109fd2d3db52d9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 67cc595278f124cdf518d2f4fcfb257439f067e2 
>   src/slave/http.cpp a32aca437cffd52bc2bcde859eedddca2038e3f1 
>   src/tests/api_tests.cpp 57981f12d429614a537733ac4cec9c0e72ff6d6f 
>   src/tests/containerizer.hpp 080b8fa695d4c0dd2ed2bf028f1fc617dad23943 
>   src/tests/containerizer.cpp c3bcb85d245a0e95b377059802cd89617eb5e25c 
> 
> Diff: https://reviews.apache.org/r/53351/diff/
> 
> 
> Testing
> ---
> 
> make -j check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 53585: Added a default PATH environment variable when launching a container.


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


Ship it!




Ship It!

- Jie Yu


On Nov. 8, 2016, 9:49 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53585/
> ---
> 
> (Updated Nov. 8, 2016, 9:49 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6543
> https://issues.apache.org/jira/browse/MESOS-6543
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Having an environment set but no PATH variable set inside of it,
> will trigger an `execvpe` call in launch.cpp on the command we are
> trying to launch. Normally this doesn't casue problems because
> `execvpe` with call `confstr(_CS_PATH)` under the hood to set up a
> default path to use when lookiing up our command.
> 
> However, when provisioning a new filesystem for a container, its
> possible that the `confstr(_CS_PATH)` called from the agent may not
> return the same path as a `confstr(_CS_PATH)` call would return using
> the libc installed inside the container. This can lead to problems
> (for example) with finding the `sh` command in containers based on an
> alpine linux image.
> 
> We observed this in our test setup with `confstr(_CS_PATH)` on the
> agent returning `/usr/bin`, while alpine linux only has the `sh`
> command installed in `/bin`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 377a9d94aa780ab598b1c2034c10ce25a4e02cbe 
> 
> Diff: https://reviews.apache.org/r/53585/diff/
> 
> 
> Testing
> ---
> 
> make -j check
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 53623: Windows: Fixed provisioner copy backend build error.


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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/backends/copy.cpp (lines 269 - 272)


I'd move this to `provision` and bail out early (before mkdir)


- Jie Yu


On Nov. 10, 2016, 12:38 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53623/
> ---
> 
> (Updated Nov. 10, 2016, 12:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The provisioner's `copy` backend has never been supported on Windows,
> but we have included it in the build to reduce the number of `#ifdef`s
> needed to compile the agent.  If the user attempts to specify a rootfs
> on Windows, it will still fail in the containerizer's validation logic.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 7e98110af9d105c803518a29cd2e74b7464fcb1e 
> 
> Diff: https://reviews.apache.org/r/53623/diff/
> 
> 
> Testing
> ---
> 
> make check (CentOS 7)
> msbuild ... (Windows 10)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 53623: Windows: Fixed provisioner copy backend build error.


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



Patch looks great!

Reviews applied: [53623]

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

- Mesos ReviewBot


On Nov. 10, 2016, 12:38 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53623/
> ---
> 
> (Updated Nov. 10, 2016, 12:38 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The provisioner's `copy` backend has never been supported on Windows,
> but we have included it in the build to reduce the number of `#ifdef`s
> needed to compile the agent.  If the user attempts to specify a rootfs
> on Windows, it will still fail in the containerizer's validation logic.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 7e98110af9d105c803518a29cd2e74b7464fcb1e 
> 
> Diff: https://reviews.apache.org/r/53623/diff/
> 
> 
> Testing
> ---
> 
> make check (CentOS 7)
> msbuild ... (Windows 10)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 53623: Windows: Fixed provisioner copy backend build error.

2016-11-09 Thread Joseph Wu


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

Review request for mesos.


Repository: mesos


Description
---

The provisioner's `copy` backend has never been supported on Windows,
but we have included it in the build to reduce the number of `#ifdef`s
needed to compile the agent.  If the user attempts to specify a rootfs
on Windows, it will still fail in the containerizer's validation logic.


Diffs
-

  src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
7e98110af9d105c803518a29cd2e74b7464fcb1e 

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


Testing
---

make check (CentOS 7)
msbuild ... (Windows 10)


Thanks,

Joseph Wu

Re: Review Request 53636: Fix test ROOT_WriteAccessSharedPersistentVolumeReadOnlyMode.

2016-11-09 Thread Jiang Yan Xu


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


Ship it!




Ship It!

- Jiang Yan Xu


On Nov. 9, 2016, 4:24 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53636/
> ---
> 
> (Updated Nov. 9, 2016, 4:24 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added SHARED_RESOURCES capability to the framework since it creates
> a shared persistent volume in this test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
> 50b951dcd70cb7c494931f72d5807b22284c6b86 
> 
> Diff: https://reviews.apache.org/r/53636/diff/
> 
> 
> Testing
> ---
> 
> All tests passed including the failed test.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Review Request 53636: Fix test ROOT_WriteAccessSharedPersistentVolumeReadOnlyMode.

2016-11-09 Thread Anindya Sinha


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

Review request for mesos, Greg Mann and Jiang Yan Xu.


Repository: mesos


Description
---

Added SHARED_RESOURCES capability to the framework since it creates
a shared persistent volume in this test.


Diffs
-

  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
50b951dcd70cb7c494931f72d5807b22284c6b86 

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


Testing
---

All tests passed including the failed test.


Thanks,

Anindya Sinha

Re: Review Request 53487: Wired the libprocess code to use the streaming decoder.

2016-11-09 Thread Benjamin Mahler


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



Mostly some questions here about whether we can simplify the structure of visit.


3rdparty/libprocess/src/process.cpp (lines 673 - 675)


How about also describing the current semantics? Namely, the caller should 
ensure the provided request is valid until the future transitions.



3rdparty/libprocess/src/process.cpp (lines 682 - 684)


I assume you used to pull this out to take a non-const copy?

Seems like this could become:

```
return pipeRequest->reader->readAll()
  .then(...);
```



3rdparty/libprocess/src/process.cpp (lines 2707 - 2766)


Given my error handling comment below, would the following be easier?

```
if (libprocess(request)) {
  parse(request)
.onAny([=](const Future& message) {
  if (!message.isReady()) {
// Handle error.
  }
  
  // No longer need to care about nullptr case.
  CHECK_NOTNULL(message.get());
  
  // Handle accepted.
})
}
```

I'm a bit hesitant to need to do the PIPE->BODY conversion since we may 
want to make all requests have a pipe body. Parse would be the consumer of the 
request, and would look like:

```
Future parse(const Request& request);
```

Note that while its const, we can take a non-const copy of the request's 
pipe reader and consume it and this function doesn't care if the request is 
destructed since it has a copy of the pipe reader in the asynchronous path.



3rdparty/libprocess/src/process.cpp (lines 2711 - 2712)


Maybe a little comment here that we're guaranteed that for a given socket, 
the request's body consumption continuation will run before the next request 
arrives in handle. We rely on this, because otherwise requests could be 
processed out-of-order!



3rdparty/libprocess/src/process.cpp (line 2712)


What are you capturing here? Is it too onerous to be explicit?



3rdparty/libprocess/src/process.cpp (lines 2757 - 2759)


Yikes, if the message returned from parse is null we're not sending an 
error response?

Looking at the error handling you added above, it looks like this case also 
needs to be handled?



3rdparty/libprocess/src/process.cpp (lines 3611 - 3702)


How about we simplify this with resolvers?

```
void ProcessBase::visit(const HttpEvent& event)
{
  VLOG(1) << "Handling HTTP event for process '" << pid.id << "'"
  << " with path: '" << event.request->url.path << "'";

  // First try to resolve an endpoint for this path.
  Option endpoint = resolveEndpoint(event.request->url.path);

  if (endpoint.isSome()) {
if (endpoint->options->streaming) {
  // Consume the request body on behalf of the endpoint.
  convert(request.get())
.onAny(defer(self(), [this, endpoint, name, request, response](
const Future& future) {
  _visit(future, endpoint, name, request, response);
}));
} else {
  // Let the endpoint consume the request body.
  _visit(Nothing(), endpoint, name, request, response);
}

return;
  }
  
  // Ensure the body is consumed so that no backpressure is applied
  // to the socket (ignore the content since we do not care about it).
  http::Pipe::Reader reader = event->request->reader.get();
  reader.readAll();
  
  // If no endpoint is found, look for an asset.
  Option asset = resolveAsset(event.request->url.path);
  
  if (asset.isSome()) {
OK response;
response.type = Response::PATH;
response.path = asset->path;

// Construct the final path by appending remaining tokens.
for (int i = 2; i < tokens.size(); i++) {
  response.path += "/" + tokens[i];
}

// Try and determine the Content-Type from an extension.
Option extension = Path(response.path).extension();

if (extension.isSome() && asset->types.count(extension.get()) > 0) {
  response.headers["Content-Type"] = asset->types.at(extension.get());
}

// TODO(benh): Use "text/plain" for assets that don't have an
// extension or we don't have a mapping for? It might be better to
// just let the browser guess (or do its own

Re: Review Request 53624: Use JSON content type in mesos-execute.


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



Patch looks great!

Reviews applied: [53624]

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

- Mesos ReviewBot


On Nov. 9, 2016, 9:59 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53624/
> ---
> 
> (Updated Nov. 9, 2016, 9:59 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6562
> https://issues.apache.org/jira/browse/MESOS-6562
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use JSON content type in mesos-execute so that packet tracing
> protocol interactions is easier.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp b47c427c5ad29dda1985ee8fef6c4efe054df879 
> 
> Diff: https://reviews.apache.org/r/53624/diff/
> 
> 
> Testing
> ---
> 
> make check. Manually verified with tcpdump.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 53628: Document the namespaces/uts isolator.


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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Document the namespaces/uts isolator.


Diffs
-

  docs/mesos-containerizer.md 2bff35f6361f760a9001a249d2c01bbbc9e72932 

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


Testing
---

make check.


Thanks,

James Peach

Review Request 53627: Implement a namespaces/uts isolator.


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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Implement a very simple namespaces/uts isolator that can be used to set
the hostname and domainname of a container without the necessity of a
CNI plugin.

Since we already had a `hostname` field in the ContainerInfo, we can
use that to set the host name once we are in the UTS namespace. Add a
corresponding `domainname` to the ContainerInfo to allow setting the
domain name.


Diffs
-

  include/mesos/mesos.proto 380575904a5a29ee53bdc87ae8791ed14e3cafca 
  include/mesos/slave/containerizer.proto 
f4c4ad771b5dead4ea3ee7cd1b4383c4dc2359b4 
  src/Makefile.am 5a47c93388234a68c3c486a021ccdbe3213c5bac 
  src/cli/execute.cpp b47c427c5ad29dda1985ee8fef6c4efe054df879 
  src/slave/containerizer/mesos/containerizer.cpp 
44225ebf63d8dd93be9b60fff496c74dc6c3a5ad 
  src/slave/containerizer/mesos/isolators/namespaces/uts.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/namespaces/uts.cpp PRE-CREATION 
  src/slave/containerizer/mesos/launch.hpp 
8b23c1b6df6bc1fdd987af5a4469664356e7f27a 
  src/slave/containerizer/mesos/launch.cpp 
377a9d94aa780ab598b1c2034c10ce25a4e02cbe 
  src/tests/containerizer/isolator_tests.cpp 
8fefeef8c83ed2ab01f56a1ec572d3acb307143c 

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


Testing
---

make check.


Thanks,

James Peach

Review Request 53626: Add net::setDomainname() helper API.


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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Add net::setDomainname() helper API.


Diffs
-

  3rdparty/stout/include/stout/posix/net.hpp 
39b89828d350d83bf1d4b0badcd3e63eb9d1a630 
  3rdparty/stout/include/stout/windows/net.hpp 
1bed115cb848332bf9c31e455b2d001c173face9 

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


Testing
---

make check


Thanks,

James Peach

Re: Review Request 53624: Use JSON content type in mesos-execute.

2016-11-09 Thread Vinod Kone


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




src/cli/execute.cpp (line 358)


can you make it configurable via a flag (default to protobuf) instead?


- Vinod Kone


On Nov. 9, 2016, 9:59 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53624/
> ---
> 
> (Updated Nov. 9, 2016, 9:59 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6562
> https://issues.apache.org/jira/browse/MESOS-6562
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use JSON content type in mesos-execute so that packet tracing
> protocol interactions is easier.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp b47c427c5ad29dda1985ee8fef6c4efe054df879 
> 
> Diff: https://reviews.apache.org/r/53624/diff/
> 
> 
> Testing
> ---
> 
> make check. Manually verified with tcpdump.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 53624: Use JSON content type in mesos-execute.


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

Review request for mesos, Anand Mazumdar, Jie Yu, and Vinod Kone.


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


Repository: mesos


Description
---

Use JSON content type in mesos-execute so that packet tracing
protocol interactions is easier.


Diffs
-

  src/cli/execute.cpp b47c427c5ad29dda1985ee8fef6c4efe054df879 

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


Testing
---

make check. Manually verified with tcpdump.


Thanks,

James Peach

Re: Review Request 53490: Added a test for request streaming via the connection abstraction.

2016-11-09 Thread Benjamin Mahler

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

What you have looks good, I would just suggest that you also cover the case 
where the request body does not complete to EOF, but fails. In this case the 
server should not receive the request at all (if !RouteOptions.streaming) or 
should see the request.pipe fail (if RouteOptions.streaming).

I looked through the existing tests for the response streaming, they are pretty 
lacking, you can blame me for that :)
Also, they are a bit inconsistent with how you added the test here, so 
hopefully at some point we can circle back and clean up the existing tests.

3rdparty/libprocess/src/tests/http_tests.cpp (line 89)

streaming vs pipe (above) seems prone to confusion?

Probably we should do s/pipe/streamingResponse/ and 
s/streaming/streamingRequest/

3rdparty/libprocess/src/tests/http_tests.cpp (lines 1186 - 1190)

There is no guarantee that the writes on the client side will correspond to 
the reads on the server side. This is my bad because it looks like I made this 
assumption in the tests I added for the response streaming, what was I 
thinking?? :)

We have to instead just assert that the entire contents read on the server 
side correspond to the entire contents written on the client side. Or if we 
want to test more rigorously, that if I write "Hello", I will read "Hello" via 
1, 2, 3, 4, or 5 read calls.

For example (pseudo-code):

```
writer.write("Hello");

while (read != "Hello") {
  Future f = reader.read();
  AWAIT_READY(f);
  read += f.get();
  ASSERT_TRUE(string("Hello").startsWith(read));
}
```

- Benjamin Mahler

On Nov. 4, 2016, 5:55 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53490/
> ---
> 
> (Updated Nov. 4, 2016, 5:55 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6466
> https://issues.apache.org/jira/browse/MESOS-6466
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for request streaming via the connection abstraction.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 533104c93dd1eaf67bf3752163d2e0cad090078d 
> 
> Diff: https://reviews.apache.org/r/53490/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 52787: Reorganized the work directory and the runtime directory in local mode.

2016-11-09 Thread Kevin Klues


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




bin/mesos-local-flags.sh.in (lines 19 - 20)


Do we even these anymore since we set the work/runtime dir in the code via 
e.g.:

```
path::join(os::temp(), "mesos", "work"));
```

It seems better to use `os::temp()` than to hard code `/tmp`


- Kevin Klues


On Nov. 9, 2016, 4:53 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Nov. 9, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reorganized the work directory and the runtime directory in local mode.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp 3d8e5392f770a72354215829022b677caee9d8c6 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> No matter use `bin/mesos-local.sh` or use `src/mesos-local`, the folder 
> structure looks like
> 
> ```
> $ tree /tmp/mesos/
> /tmp/mesos/
> |-- runtime
> |   |-- agents
> |   |-- 0
> |   |-- 1
> |-- work
> |-- agents
> |   |-- 0
> |   |   |-- meta
> |   |   |   |-- boot_id
> |   |   |   |-- slaves
> |   |   |   |-- a63fbc0a-268e-4ebe-b31c-90d3a19d2fb3-S0
> |   |   |   |   |-- slave.info
> |   |   |   |-- latest -> 
> /tmp/mesos/work/agents/0/meta/slaves/a63fbc0a-268e-4ebe-b31c-90d3a19d2fb3-S0
> |   |   |-- provisioner
> |   |-- 1
> |   |-- meta
> |   |   |-- boot_id
> |   |   |-- slaves
> |   |   |-- b4d008bc-6be4-4bba-97c3-3a73d3f9611d-S0
> |   |   |   |-- slave.info
> |   |   |-- latest -> 
> /tmp/mesos/work/agents/1/meta/slaves/b4d008bc-6be4-4bba-97c3-3a73d3f9611d-S0
> |   |-- provisioner
> |-- master
> |-- replicated_log
> |-- 09.log
> |-- 11.sst
> |-- CURRENT
> |-- LOCK
> |-- LOG
> |-- LOG.old
> |-- MANIFEST-06
> 
> 20 directories, 11 files
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52919: Allowed to pass extra envs when launch development scripts.

2016-11-09 Thread Kevin Klues


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


Ship it!




I would update the description with a bit more detail about the actual change 
that was made.
Also, the summary line is a little hard to understand. I'd say something like:

`Updated scripts to allow override of MESOS environment variables.`

- Kevin Klues


On Nov. 9, 2016, 4:51 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52919/
> ---
> 
> (Updated Nov. 9, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6399
> https://issues.apache.org/jira/browse/MESOS-6399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed to pass extra envs when launch development scripts.
> 
> 
> Diffs
> -
> 
>   bin/mesos-agent-flags.sh.in 8c936aa06e994a87a8b09b31c907868bf9be38c7 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   bin/mesos-master-flags.sh.in 951c3e4c714809bcf2364eb28974aeefb7884bcb 
> 
> Diff: https://reviews.apache.org/r/52919/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ MESOS_TEMP_DIR=/tmp/mesos/bar ./bin/mesos-local.sh
> 
> $ tree /tmp/mesos/
> /tmp/mesos/
> ??? bar
> ??? agents
> ?   ??? 0
> ?   ??? runtime_dir
> ?   ??? work_dir
> ?   ??? meta
> ?   ?   ??? boot_id
> ?   ?   ??? slaves
> ?   ?   ??? 6b4ce717-dd6e-4d8c-9f89-66a42e3866a8-S0
> ?   ?   ?   ??? slave.info
> ?   ?   ??? latest -> 
> /tmp/mesos/bar/agents/0/work_dir/meta/slaves/6b4ce717-dd6e-4d8c-9f89-66a42e3866a8-S0
> ?   ??? provisioner
> ??? master
> ??? work_dir
> ??? replicated_log
> ??? 04.log
> ??? CURRENT
> ??? LOCK
> ??? LOG
> ??? MANIFEST-02
> 
> 13 directories, 7 files
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53615: Cleaned up formatting in home.md.


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



Patch looks great!

Reviews applied: [53610, 53611, 53612, 53613, 53614, 53615]

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

- Mesos ReviewBot


On Nov. 9, 2016, 5:08 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53615/
> ---
> 
> (Updated Nov. 9, 2016, 5:08 p.m.)
> 
> 
> Review request for mesos, Joerg Schad and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Restore the alphabetic order of entries and
> ensure two blank lines in between sections.
> 
> 
> Diffs
> -
> 
>   docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 
> 
> Diff: https://reviews.apache.org/r/53615/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53559: Transformed env variable parsing into Flags in process.cpp.

2016-11-09 Thread Armand Grillet


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

(Updated Nov. 9, 2016, 8:26 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Updated comment for libprocess port and the `advertise_port` cannot be 0 
anymore.


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


Repository: mesos


Description (updated)
---

Environment variables LIBPROCESS_ + (IP, ADVERTISE_IP,PORT,
ADVERTISE_PORT) are now in a Flags object.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 

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


Testing
---

make check (macOS)


Thanks,

Armand Grillet

Re: Review Request 52696: Harden stout

2016-11-09 Thread Benjamin Bannier


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


Ship it!




Lots great, thanks a lot.

- Benjamin Bannier


On Nov. 9, 2016, 8:05 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52696/
> ---
> 
> (Updated Nov. 9, 2016, 8:05 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add hardened flags for stout.
> Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am 4e10ae2 
>   3rdparty/stout/configure.ac f071f61 
>   3rdparty/stout/m4/ax_check_compile_flag.m4 PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52696/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> File Attachments
> 
> 
> --enable-optimized with hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/18a2f590-75ad-49c5-a697-56b746f28cae__hardened-optimized.txt
> Hardening applied but no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/a6e07766-80cc-4bd7-856d-8952cac12562__hardened-unoptimized.txt
> --enable-optimized with no hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/046b37a9-5aff-4543-b3bb-5ac60daaf498__optimized.txt
> No hardening applied and no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/3baa96cf-be05-4ac0-ad4c-ef571386e8f4__unoptimized.txt
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52645: Harden Mesos

2016-11-09 Thread Benjamin Bannier


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


Ship it!




Lots great, thanks a lot.

- Benjamin Bannier


On Nov. 9, 2016, 8:37 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52645/
> ---
> 
> (Updated Nov. 9, 2016, 8:37 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add hardened flags for Mesos.
> Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.
> 
> 
> Diffs
> -
> 
>   configure.ac 5380cbc 
>   m4/ax_check_compile_flag.m4 PRE-CREATION 
>   src/Makefile.am 5a47c93 
> 
> Diff: https://reviews.apache.org/r/52645/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52695: Harden libprocess

2016-11-09 Thread Benjamin Bannier


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


Ship it!




Lots great, thanks a lot.

- Benjamin Bannier


On Nov. 9, 2016, 8:07 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 9, 2016, 8:07 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add hardened flags for libprocess.
> Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 7131989 
>   3rdparty/libprocess/configure.ac e65e5ca 
>   3rdparty/libprocess/m4/ax_check_compile_flag.m4 PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52695/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> File Attachments
> 
> 
> --enable-optimized with hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/875c9e6e-c73b-4e3c-8265-0f7c6dc00351__hardened-optimized.txt
> Hardening applied but no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/932d28a7-2d31-471a-b438-647841a6853c__hardened-unoptimized.txt
> --enable-optimized with no hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/896944ea-9b31-4d62-b1b9-97fb4700a882__optimized.txt
> No hardening applied and no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/b32667ce-3e3b-4d2b-b4f8-4c2404a0fc1c__unoptimized.txt
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52645: Harden Mesos


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

(Updated Nov. 9, 2016, 7:37 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


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


Repository: mesos


Description (updated)
---

Add hardened flags for Mesos.
Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.


Diffs
-

  configure.ac 5380cbc 
  m4/ax_check_compile_flag.m4 PRE-CREATION 
  src/Makefile.am 5a47c93 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52645: Harden Mesos


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

(Updated Nov. 9, 2016, 7:37 p.m.)


Review request for mesos, James Peach, Michael Park, and Neil Conway.


Changes
---

Add `-Werror` back into `MESOS_CPPFLAGS`.


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


Repository: mesos


Description
---

Use a default set of flags to provide additional security and hardening to 
Mesos. Additionally, check and catch more warnings/errors.


Diffs (updated)
-

  configure.ac 5380cbc 
  m4/ax_check_compile_flag.m4 PRE-CREATION 
  src/Makefile.am 5a47c93 

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


Testing
---

Compared the benchmarks with and without the flags being used. Also did a 
comparsion with the flags being used with and without optimizations and without 
the flags being used with and without optimizations. Overall the performance 
hit was very small with a 3-8% overhead (optimizations brings this down 
slightly). Most benchmarks were about 5% (or less) slower.


Thanks,

Aaron Wood

Re: Review Request 52695: Harden libprocess

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

(Updated Nov. 9, 2016, 7:07 p.m.)

Review request for mesos, James Peach, Michael Park, and Neil Conway.

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

Repository: mesos

Description (updated)
---

Add hardened flags for libprocess.
Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.

Diffs
-

3rdparty/libprocess/Makefile.am 7131989
3rdparty/libprocess/configure.ac e65e5ca
3rdparty/libprocess/m4/ax_check_compile_flag.m4 PRE-CREATION

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

Testing
---

Compared the benchmarks with and without the flags being used. Also did a
comparsion with the flags being used with and without optimizations and without
the flags being used with and without optimizations. Overall the performance
hit was very small with a 3-8% overhead (optimizations brings this down
slightly). Most benchmarks were about 5% (or less) slower.

File Attachments

--enable-optimized with hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/875c9e6e-c73b-4e3c-8265-0f7c6dc00351__hardened-optimized.txt
Hardening applied but no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/932d28a7-2d31-471a-b438-647841a6853c__hardened-unoptimized.txt
--enable-optimized with no hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/896944ea-9b31-4d62-b1b9-97fb4700a882__optimized.txt
No hardening applied and no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/b32667ce-3e3b-4d2b-b4f8-4c2404a0fc1c__unoptimized.txt

Thanks,

Aaron Wood

Re: Review Request 52696: Harden stout



> On Nov. 2, 2016, 9:33 a.m., Benjamin Bannier wrote:
> > 3rdparty/stout/m4/ax_check_compile_flag.m4, line 1
> > 
> >
> > For future updates it would be great if we'd write down the 
> > autoconf-archive release this file came from (it looks like the latest 
> > release containing it is `v2016.09.16`).
> 
> Benjamin Bannier wrote:
> You marked this as resolved, but I couldn't find the change. Could you 
> please update e.g., the commit message to include something like
> 
> This commit adds ax_check_compiler_flag.m4 from 
> git://git.sv.gnu.org/autoconf-archive.git tag v2016.09.16.

I've updated the description. That hash points to a diff that was made in 2015 
so I figured it was best no to say v2016.09.16.


- Aaron


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


On Nov. 9, 2016, 7:05 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52696/
> ---
> 
> (Updated Nov. 9, 2016, 7:05 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add hardened flags for stout.
> Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/Makefile.am 4e10ae2 
>   3rdparty/stout/configure.ac f071f61 
>   3rdparty/stout/m4/ax_check_compile_flag.m4 PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/52696/diff/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> File Attachments
> 
> 
> --enable-optimized with hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/18a2f590-75ad-49c5-a697-56b746f28cae__hardened-optimized.txt
> Hardening applied but no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/a6e07766-80cc-4bd7-856d-8952cac12562__hardened-unoptimized.txt
> --enable-optimized with no hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/046b37a9-5aff-4543-b3bb-5ac60daaf498__optimized.txt
> No hardening applied and no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/3baa96cf-be05-4ac0-ad4c-ef571386e8f4__unoptimized.txt
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52696: Harden stout

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

(Updated Nov. 9, 2016, 7:05 p.m.)

Review request for mesos, James Peach, Michael Park, and Neil Conway.

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

Repository: mesos

Description (updated)
---

Add hardened flags for stout.
Take compile flag macro at 1a869696e4129279f7b99c3f9052717354b79a86.

Diffs
-

3rdparty/stout/Makefile.am 4e10ae2
3rdparty/stout/configure.ac f071f61
3rdparty/stout/m4/ax_check_compile_flag.m4 PRE-CREATION

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

Testing
---

File Attachments

--enable-optimized with hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/18a2f590-75ad-49c5-a697-56b746f28cae__hardened-optimized.txt
Hardening applied but no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/a6e07766-80cc-4bd7-856d-8952cac12562__hardened-unoptimized.txt
--enable-optimized with no hardening applied

https://reviews.apache.org/media/uploaded/files/2016/11/02/046b37a9-5aff-4543-b3bb-5ac60daaf498__optimized.txt
No hardening applied and no --enable-optimized

https://reviews.apache.org/media/uploaded/files/2016/11/02/3baa96cf-be05-4ac0-ad4c-ef571386e8f4__unoptimized.txt

Thanks,

Aaron Wood

Re: Review Request 51624: Implemented 'GetAgent' call in v1 agent API.

2016-11-09 Thread Zhitao Li


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


Ship it!




Ship It!

- Zhitao Li


On Oct. 8, 2016, 12:37 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51624/
> ---
> 
> (Updated Oct. 8, 2016, 12:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Vinod Kone, and Zhitao Li.
> 
> 
> Bugs: MESOS-6123
> https://issues.apache.org/jira/browse/MESOS-6123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented 'GetAgent' call in v1 agent API.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 5b91677487f8e23301e67af14c2115c6b4a533ac 
>   include/mesos/v1/agent/agent.proto 8145669073553dc8aa56cfe2c0a0b756d70fee0e 
>   src/slave/http.cpp 67463105d7fa38b2158a64f6994e3dd353a9fcc7 
>   src/slave/slave.hpp 13c76d1eaea2b49519948c9116e5db5caf9407ea 
>   src/slave/validation.cpp a9f318214182a87783a985cd8495a0ec00c95378 
>   src/tests/api_tests.cpp 26f99f7c337fbbc5278d1b30d3d5c8f659ddf5ca 
> 
> Diff: https://reviews.apache.org/r/51624/diff/
> 
> 
> Testing
> ---
> 
> Added a new test case `AgentAPITest.GetAgent`.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52919: Allowed to pass extra envs when launch development scripts.


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



Patch looks great!

Reviews applied: [52856, 52787, 52919]

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

- Mesos ReviewBot


On Nov. 9, 2016, 4:51 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52919/
> ---
> 
> (Updated Nov. 9, 2016, 4:51 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6399
> https://issues.apache.org/jira/browse/MESOS-6399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed to pass extra envs when launch development scripts.
> 
> 
> Diffs
> -
> 
>   bin/mesos-agent-flags.sh.in 8c936aa06e994a87a8b09b31c907868bf9be38c7 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   bin/mesos-master-flags.sh.in 951c3e4c714809bcf2364eb28974aeefb7884bcb 
> 
> Diff: https://reviews.apache.org/r/52919/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ MESOS_TEMP_DIR=/tmp/mesos/bar ./bin/mesos-local.sh
> 
> $ tree /tmp/mesos/
> /tmp/mesos/
> ??? bar
> ??? agents
> ?   ??? 0
> ?   ??? runtime_dir
> ?   ??? work_dir
> ?   ??? meta
> ?   ?   ??? boot_id
> ?   ?   ??? slaves
> ?   ?   ??? 6b4ce717-dd6e-4d8c-9f89-66a42e3866a8-S0
> ?   ?   ?   ??? slave.info
> ?   ?   ??? latest -> 
> /tmp/mesos/bar/agents/0/work_dir/meta/slaves/6b4ce717-dd6e-4d8c-9f89-66a42e3866a8-S0
> ?   ??? provisioner
> ??? master
> ??? work_dir
> ??? replicated_log
> ??? 04.log
> ??? CURRENT
> ??? LOCK
> ??? LOG
> ??? MANIFEST-02
> 
> 13 directories, 7 files
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 53509: Changed mesos-execute --master flag parsing.

2016-11-09 Thread Joseph Wu


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


Ship it!




Ship It!

- Joseph Wu


On Nov. 7, 2016, 6:01 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53509/
> ---
> 
> (Updated Nov. 7, 2016, 6:01 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Till Toenshoff.
> 
> 
> Bugs: MESOS-2723
> https://issues.apache.org/jira/browse/MESOS-2723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit makes the --master argument required.
> The parsing of the argument now happens in src/master/detector.cpp.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp b265bc6390ed8329a200408ef45512f900f9b999 
> 
> Diff: https://reviews.apache.org/r/53509/diff/
> 
> 
> Testing
> ---
> 
> Testing done by running a master with `--zk=zk://localhost:2181/mesos` then 
> an agent with `--master=zk://localhost:2181/mesos` and then executing a sleep 
> task on Mesos with `--master=zk://localhost:2181/mesos`.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 53559: Transformed env variable parsing into Flags in process.cpp.

2016-11-09 Thread Joseph Wu



> On Nov. 8, 2016, 1:43 a.m., Gastón Kleiman wrote:
> > 3rdparty/libprocess/src/process.cpp, line 201
> > 
> >
> > ditto
> 
> Gastón Kleiman wrote:
> @joseph: do you know what libprocess advertises if this is set to `0`? 
> Does advertising a randomly chosen port make sense? And what if 
> `LIBPROCESS_PORT = 31337` and `LIBPROCESS_ADVERTISE_PORT = 0`?

Hm... the current code is this:
```
  value = os::getenv("LIBPROCESS_ADVERTISE_PORT");
  if (value.isSome()) {
Try result = numify(value.get().c_str());
if (result.isSome() && result.get() >=0 && result.get() <= USHRT_MAX) {
  __address__.port = result.get();
} else {
  LOG(FATAL) << "LIBPROCESS_ADVERTISE_PORT=" << value.get()
 << " is not a valid port";
}
  }
```

A zero port here is clearly incorrect.  Hard to believe we haven't had anyone 
run into this.


- Joseph


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


On Nov. 7, 2016, 5:51 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53559/
> ---
> 
> (Updated Nov. 7, 2016, 5:51 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4609
> https://issues.apache.org/jira/browse/MESOS-4609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Environment variables LIBPROCESS_ + (IP, ADVERTISE_IP,PORT,
> ADVERTISE_PORT) are now in a Flags object. Heavily inspired
> by Joseph's review request [43261](https://reviews.apache.org/r/43261/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 
> 
> Diff: https://reviews.apache.org/r/53559/diff/
> 
> 
> Testing
> ---
> 
> make check (macOS)
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 53610: Added health checks documentation.


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




docs/health-checks.md (line 6)


Is `Health Checking For Tasks` a better title?



docs/health-checks.md (line 8)


`misbehave, or become unresponsive` should be `misbehave or become 
unresponsive`?



docs/health-checks.md (line 10)


Use 
`https://github.com/mesosphere/marathon/blob/v1.3.6/docs/docs/health-checks.md` 
would be shorter?



docs/health-checks.md (line 29)


s/functionality/functionalities/g



docs/health-checks.md (line 53)


Should remove `---` in this line?


- haosdent huang


On Nov. 9, 2016, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53610/
> ---
> 
> (Updated Nov. 9, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-5597
> https://issues.apache.org/jira/browse/MESOS-5597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md PRE-CREATION 
>   docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 
> 
> Diff: https://reviews.apache.org/r/53610/diff/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/7200c36b2fd1e81f78f2583e68b31fd1
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 53611: Improved comments around health checks in mesos.proto.


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


Ship it!




Ship It!

- haosdent huang


On Nov. 9, 2016, 5:05 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53611/
> ---
> 
> (Updated Nov. 9, 2016, 5:05 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and 
> Till Toenshoff.
> 
> 
> Bugs: MESOS-5597
> https://issues.apache.org/jira/browse/MESOS-5597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 380575904a5a29ee53bdc87ae8791ed14e3cafca 
>   include/mesos/v1/mesos.proto 5b542ff9cd0b5f301c088d3f4b2fe17ae760d368 
> 
> Diff: https://reviews.apache.org/r/53611/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 53610: Added health checks documentation.


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

Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/health-checks.md PRE-CREATION 
  docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 

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


Testing
---

https://gist.github.com/rukletsov/7200c36b2fd1e81f78f2583e68b31fd1


Thanks,

Alexander Rukletsov

Re: Review Request 53613: Fixed style issues in quota.md.


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

(Updated Nov. 9, 2016, 5:09 p.m.)


Review request for mesos, Joerg Schad and Neil Conway.


Repository: mesos


Description
---

Correct caption hierarchy, update headings to conform to
title case, and ensure two blank lines between sections.


Diffs (updated)
-

  docs/quota.md d7e8d58eeeac9d9ffc1a6f7614c7ddce6d7a9b89 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 53615: Cleaned up formatting in home.md.


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

Review request for mesos, Joerg Schad and Neil Conway.


Repository: mesos


Description
---

Restore the alphabetic order of entries and
ensure two blank lines in between sections.


Diffs
-

  docs/home.md a5811480de050352dca6c0f7e4e64d3d2351c2d5 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 53613: Fixed style issues in quota.md.


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

Review request for mesos, Joerg Schad and Neil Conway.


Repository: mesos


Description
---

Correct caption hierarchy, update headings to conform to
title case, and ensure two blank lines between sections.


Diffs
-

  docs/quota.md d7e8d58eeeac9d9ffc1a6f7614c7ddce6d7a9b89 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 53614: Updated the markdown style guide with headings capitalization.


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

Review request for mesos, Joerg Schad and Neil Conway.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/markdown-style-guide.md 667d6dfbcf6c0864085cdfb19f4e53fb49a49c44 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 53612: Fixed minor style issues in docs.


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

Review request for mesos, Joerg Schad and Neil Conway.


Repository: mesos


Description
---

According to our markdown style guide, Notes should be decorated as
**NOTE:** and not __NOTE:__ and formatted without extra indentation
or extra symbols.


Diffs
-

  docs/quota.md d7e8d58eeeac9d9ffc1a6f7614c7ddce6d7a9b89 
  docs/reservation.md 20db7c97d5c9d651c6d79864e5093c2fd4bdfc04 
  docs/sandbox.md af0036df05b31bc0bc028d283b7c5a0c60674327 
  docs/ssl.md 2f7d928ef94b9fb611e071b01755c002554bad40 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Review Request 53611: Improved comments around health checks in mesos.proto.


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

Review request for mesos, Gastón Kleiman, haosdent huang, Neil Conway, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/mesos.proto 380575904a5a29ee53bdc87ae8791ed14e3cafca 
  include/mesos/v1/mesos.proto 5b542ff9cd0b5f301c088d3f4b2fe17ae760d368 

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


Testing
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 52787: Reorganized the work directory and the runtime directory in local mode.



> On Nov. 3, 2016, 3:55 p.m., Kevin Klues wrote:
> > src/local/flags.hpp, lines 35-52
> > 
> >
> > I don't think we should have a flag for `temp_dir` here. We should 
> > still have separate flags for  `work_dir` and `runtime_dir`.
> > 
> > With those, you can still organize things similar to what you've 
> > described above, but each should be rooted differently, i.e.:
> > 
> > ```
> > /tmp/mesos/work
> > |-- master
> > |   |-- replicated_log
> > |   |   |-- 04.log
> > |   |   |-- CURRENT
> > |   |   |-- LOCK
> > |   |   |-- LOG
> > |   |   |-- MANIFEST-02
> > |-- agents
> > |-- 0
> > |   |-- meta
> > |   |   |-- boot_id
> > |   |   |-- slaves
> > |   |   |-- 5b7cf084-32d7-4530-8451-d6e5db19f48f-S0
> > |   |   |   |-- slave.info
> > |   |   |-- latest -> 
> > /tmp/mesos/work/meta/slaves/5b7cf084-32d7-4530-8451-d6e5db19f48f-S0
> > |   |-- provisioner
> > |-- 1
> > |-- meta
> > |   |-- boot_id
> > |   |-- slaves
> > |   |-- 5b7cf084-32d7-4530-8451-d6e5db19f48f-S1
> > |   |   |-- slave.info
> > |   |-- latest -> 
> > /tmp/mesos/work/meta/slaves/5b7cf084-32d7-4530-8451-d6e5db19f48f-S1
> > |-- provisioner
> > 
> > 
> > /tmp/mesos/runtime
> > |-- agents
> > |-- 0
> > |   |-- runtime_dir
> > |-- 1
> > |-- runtime_dir
> > ```

Thanks a lot, may you help review this patch again? I update the structure as 
you suggested.


- haosdent


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


On Nov. 9, 2016, 4:53 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52787/
> ---
> 
> (Updated Nov. 9, 2016, 4:53 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-6380
> https://issues.apache.org/jira/browse/MESOS-6380
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reorganized the work directory and the runtime directory in local mode.
> 
> 
> Diffs
> -
> 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
>   src/local/flags.hpp 3d8e5392f770a72354215829022b677caee9d8c6 
>   src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 
> 
> Diff: https://reviews.apache.org/r/52787/diff/
> 
> 
> Testing
> ---
> 
> No matter use `bin/mesos-local.sh` or use `src/mesos-local`, the folder 
> structure looks like
> 
> ```
> $ tree /tmp/mesos/
> /tmp/mesos/
> |-- runtime
> |   |-- agents
> |   |-- 0
> |   |-- 1
> |-- work
> |-- agents
> |   |-- 0
> |   |   |-- meta
> |   |   |   |-- boot_id
> |   |   |   |-- slaves
> |   |   |   |-- a63fbc0a-268e-4ebe-b31c-90d3a19d2fb3-S0
> |   |   |   |   |-- slave.info
> |   |   |   |-- latest -> 
> /tmp/mesos/work/agents/0/meta/slaves/a63fbc0a-268e-4ebe-b31c-90d3a19d2fb3-S0
> |   |   |-- provisioner
> |   |-- 1
> |   |-- meta
> |   |   |-- boot_id
> |   |   |-- slaves
> |   |   |-- b4d008bc-6be4-4bba-97c3-3a73d3f9611d-S0
> |   |   |   |-- slave.info
> |   |   |-- latest -> 
> /tmp/mesos/work/agents/1/meta/slaves/b4d008bc-6be4-4bba-97c3-3a73d3f9611d-S0
> |   |-- provisioner
> |-- master
> |-- replicated_log
> |-- 09.log
> |-- 11.sst
> |-- CURRENT
> |-- LOCK
> |-- LOG
> |-- LOG.old
> |-- MANIFEST-06
> 
> 20 directories, 11 files
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52787: Reorganized the work directory and the runtime directory in local mode.


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

(Updated Nov. 9, 2016, 4:53 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.


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


Repository: mesos


Description
---

Reorganized the work directory and the runtime directory in local mode.


Diffs
-

  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  src/local/flags.hpp 3d8e5392f770a72354215829022b677caee9d8c6 
  src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 

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


Testing (updated)
---

No matter use `bin/mesos-local.sh` or use `src/mesos-local`, the folder 
structure looks like

```
$ tree /tmp/mesos/
/tmp/mesos/
|-- runtime
|   |-- agents
|   |-- 0
|   |-- 1
|-- work
|-- agents
|   |-- 0
|   |   |-- meta
|   |   |   |-- boot_id
|   |   |   |-- slaves
|   |   |   |-- a63fbc0a-268e-4ebe-b31c-90d3a19d2fb3-S0
|   |   |   |   |-- slave.info
|   |   |   |-- latest -> 
/tmp/mesos/work/agents/0/meta/slaves/a63fbc0a-268e-4ebe-b31c-90d3a19d2fb3-S0
|   |   |-- provisioner
|   |-- 1
|   |-- meta
|   |   |-- boot_id
|   |   |-- slaves
|   |   |-- b4d008bc-6be4-4bba-97c3-3a73d3f9611d-S0
|   |   |   |-- slave.info
|   |   |-- latest -> 
/tmp/mesos/work/agents/1/meta/slaves/b4d008bc-6be4-4bba-97c3-3a73d3f9611d-S0
|   |-- provisioner
|-- master
|-- replicated_log
|-- 09.log
|-- 11.sst
|-- CURRENT
|-- LOCK
|-- LOG
|-- LOG.old
|-- MANIFEST-06

20 directories, 11 files
```


Thanks,

haosdent huang

Re: Review Request 52919: Allowed to pass extra envs when launch development scripts.


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

(Updated Nov. 9, 2016, 4:51 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Allowed to pass extra envs when launch development scripts.


Diffs (updated)
-

  bin/mesos-agent-flags.sh.in 8c936aa06e994a87a8b09b31c907868bf9be38c7 
  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  bin/mesos-master-flags.sh.in 951c3e4c714809bcf2364eb28974aeefb7884bcb 

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


Testing
---

```
$ MESOS_TEMP_DIR=/tmp/mesos/bar ./bin/mesos-local.sh

$ tree /tmp/mesos/
/tmp/mesos/
??? bar
??? agents
?   ??? 0
?   ??? runtime_dir
?   ??? work_dir
?   ??? meta
?   ?   ??? boot_id
?   ?   ??? slaves
?   ?   ??? 6b4ce717-dd6e-4d8c-9f89-66a42e3866a8-S0
?   ?   ?   ??? slave.info
?   ?   ??? latest -> 
/tmp/mesos/bar/agents/0/work_dir/meta/slaves/6b4ce717-dd6e-4d8c-9f89-66a42e3866a8-S0
?   ??? provisioner
??? master
??? work_dir
??? replicated_log
??? 04.log
??? CURRENT
??? LOCK
??? LOG
??? MANIFEST-02

13 directories, 7 files
```


Thanks,

haosdent huang

Re: Review Request 52787: Reorganized the work directory and the runtime directory in local mode.


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

(Updated Nov. 9, 2016, 4:51 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.


Changes
---

Address @klueska's comments.


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


Repository: mesos


Description (updated)
---

Reorganized the work directory and the runtime directory in local mode.


Diffs (updated)
-

  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  src/local/flags.hpp 3d8e5392f770a72354215829022b677caee9d8c6 
  src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 

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


Testing
---

```
$ ./src/mesos-local --num_slaves=2

$ tree /tmp/mesos/local
/tmp/mesos/local
|-- agents
|   |-- 0
|   |   |-- runtime_dir
|   |   |-- work_dir
|   |   |-- meta
|   |   |   |-- boot_id
|   |   |   |-- slaves
|   |   |   |-- 5b7cf084-32d7-4530-8451-d6e5db19f48f-S0
|   |   |   |   |-- slave.info
|   |   |   |-- latest -> 
/tmp/mesos/local/agents/0/work_dir/meta/slaves/5b7cf084-32d7-4530-8451-d6e5db19f48f-S0
|   |   |-- provisioner
|   |-- 1
|   |-- runtime_dir
|   |-- work_dir
|   |-- meta
|   |   |-- boot_id
|   |   |-- slaves
|   |   |-- 5b7cf084-32d7-4530-8451-d6e5db19f48f-S1
|   |   |   |-- slave.info
|   |   |-- latest -> 
/tmp/mesos/local/agents/1/work_dir/meta/slaves/5b7cf084-32d7-4530-8451-d6e5db19f48f-S1
|   |-- provisioner
|-- master
|-- work_dir
|-- replicated_log
|-- 04.log
|-- CURRENT
|-- LOCK
|-- LOG
|-- MANIFEST-02

20 directories, 9 files
```


```
$ ./bin/mesos-local.sh
tree /tmp/mesos/
/tmp/mesos/
|-- agents
|   |-- 0
|   |-- runtime_dir
|   |-- work_dir
|   |-- meta
|   |   |-- boot_id
|   |   |-- slaves
|   |   |-- 0e617524-b582-4793-8a47-184c542e95d2-S0
|   |   |   |-- slave.info
|   |   |-- latest -> 
/tmp/mesos/agents/0/work_dir/meta/slaves/0e617524-b582-4793-8a47-184c542e95d2-S0
|   |-- provisioner
|-- master
|-- work_dir
|-- replicated_log
|-- 04.log
|-- CURRENT
|-- LOCK
|-- LOG
|-- MANIFEST-02

12 directories, 7 files
```


Thanks,

haosdent huang

Re: Review Request 52856: Reverted incorrect changes in 1c2ee5c.


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

(Updated Nov. 9, 2016, 4:50 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Reverted incorrect changes in 1c2ee5c.


Diffs (updated)
-

  src/local/local.cpp 257179443827ffdfc946ed655a6840fcea70d454 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 53559: Transformed env variable parsing into Flags in process.cpp.



> On Nov. 8, 2016, 9:43 a.m., Gastón Kleiman wrote:
> > 3rdparty/libprocess/src/process.cpp, line 183
> > 
> >
> > Shoulnd't port `0` be rejected? AFAIK it is not a valid port.
> 
> Joseph Wu wrote:
> Port zero == random port.  It is actually valid.

Makes sense, I'd then update the flag description to reflect that.

Something like: ```if not specified or set to `0`, libprocess will bind to a 
random available port.```


> On Nov. 8, 2016, 9:43 a.m., Gastón Kleiman wrote:
> > 3rdparty/libprocess/src/process.cpp, line 201
> > 
> >
> > ditto

@joseph: do you know what libprocess advertises if this is set to `0`? Does 
advertising a randomly chosen port make sense? And what if `LIBPROCESS_PORT = 
31337` and `LIBPROCESS_ADVERTISE_PORT = 0`?


- Gastón


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


On Nov. 8, 2016, 1:51 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53559/
> ---
> 
> (Updated Nov. 8, 2016, 1:51 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4609
> https://issues.apache.org/jira/browse/MESOS-4609
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Environment variables LIBPROCESS_ + (IP, ADVERTISE_IP,PORT,
> ADVERTISE_PORT) are now in a Flags object. Heavily inspired
> by Joseph's review request [43261](https://reviews.apache.org/r/43261/).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> ab2b5a9d38a3001d6a5daa1807fecb630c4b154d 
> 
> Diff: https://reviews.apache.org/r/53559/diff/
> 
> 
> Testing
> ---
> 
> make check (macOS)
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 53541: Added authorization actions for debug API.


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



Patch looks great!

Reviews applied: [52600, 53057, 53058, 53541]

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

- Mesos ReviewBot


On Nov. 9, 2016, 12:43 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53541/
> ---
> 
> (Updated Nov. 9, 2016, 12:43 a.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, Kevin Klues, and Till Toenshoff.
> 
> 
> Bugs: MESOS-6474
> https://issues.apache.org/jira/browse/MESOS-6474
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added authorization actions for debug API.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> e3fd6a4a1b617a75714ebd6e08ab10cffa1a7d1b 
>   include/mesos/authorizer/authorizer.proto 
> b6a9f142eecbdfd59210872a92e3126f04de334c 
>   src/authorizer/local/authorizer.cpp 
> f1dff65d973fc84f4171f68fd0391a2343a96965 
>   src/tests/authorization_tests.cpp 5d7e17b67821357b8cb538798acc883945c8f8fd 
> 
> Diff: https://reviews.apache.org/r/53541/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 53500: Used an environment variable to pass command environment.



> On Nov. 9, 2016, 10:37 a.m., Gastón Kleiman wrote:
> > The Docker containerizer still passes the env variables to the executor 
> > through cmd line flags, we might want to fix that as well:
> > 
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/docker.cpp#L244
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/docker.cpp#L351-L372

I just checked and the env variables are not visible in the `DockerExecutor` 
cmd line, but the executor passes them to the `docker` binary using the cmd 
line.

See: https://issues.apache.org/jira/browse/MESOS-6566


- Gastón


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


On Nov. 8, 2016, 4:46 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53500/
> ---
> 
> (Updated Nov. 8, 2016, 4:46 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6526
> https://issues.apache.org/jira/browse/MESOS-6526
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used an environment variable to pass command environment.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 32fba7697b7488b63192c4e9630ff348c572e424 
>   src/slave/containerizer/mesos/main.cpp 
> 1a0e765ddb6d8519426b8d47067efdfa3432e07a 
> 
> Diff: https://reviews.apache.org/r/53500/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> No new tests are written but the fact the existing tests that run exectuors 
> depends on this to work correctly is a proof.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 53601: Fixed typos in the changelog and in comments.


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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 9, 2016, 11:07 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53601/
> ---
> 
> (Updated Nov. 9, 2016, 11:07 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 928be8940ba0f7618beab8e1f7ab4dcc389f5374 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e57064c768937969ba4a071ae80165ccab2f1dff 
> 
> Diff: https://reviews.apache.org/r/53601/diff/
> 
> 
> Testing
> ---
> 
> None, only comments/changelog affected.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 53500: Used an environment variable to pass command environment.



> On Nov. 9, 2016, 10:37 a.m., Gastón Kleiman wrote:
> > src/slave/containerizer/mesos/containerizer.cpp, line 1401
> > 
> >
> > There's a typo here, it should be `launch` instead of `lauch.`

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


- Gastón


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


On Nov. 8, 2016, 4:46 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53500/
> ---
> 
> (Updated Nov. 8, 2016, 4:46 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6526
> https://issues.apache.org/jira/browse/MESOS-6526
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used an environment variable to pass command environment.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 32fba7697b7488b63192c4e9630ff348c572e424 
>   src/slave/containerizer/mesos/main.cpp 
> 1a0e765ddb6d8519426b8d47067efdfa3432e07a 
> 
> Diff: https://reviews.apache.org/r/53500/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> No new tests are written but the fact the existing tests that run exectuors 
> depends on this to work correctly is a proof.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Review Request 53601: Fixed typos in the changelog and in comments.


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

Review request for mesos, Alexander Rukletsov and Jiang Yan Xu.


Repository: mesos


Description
---

See summary.


Diffs
-

  CHANGELOG 928be8940ba0f7618beab8e1f7ab4dcc389f5374 
  src/slave/containerizer/mesos/containerizer.cpp 
e57064c768937969ba4a071ae80165ccab2f1dff 

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


Testing
---

None, only comments/changelog affected.


Thanks,

Gastón Kleiman

Re: Review Request 53500: Used an environment variable to pass command environment.


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



The Docker containerizer still passes the env variables to the executor through 
cmd line flags, we might want to fix that as well:

https://github.com/apache/mesos/blob/master/src/slave/containerizer/docker.cpp#L244
https://github.com/apache/mesos/blob/master/src/slave/containerizer/docker.cpp#L351-L372


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


There's a typo here, it should be `launch` instead of `lauch.`


- Gastón Kleiman


On Nov. 8, 2016, 4:46 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53500/
> ---
> 
> (Updated Nov. 8, 2016, 4:46 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6526
> https://issues.apache.org/jira/browse/MESOS-6526
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used an environment variable to pass command environment.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 32fba7697b7488b63192c4e9630ff348c572e424 
>   src/slave/containerizer/mesos/main.cpp 
> 1a0e765ddb6d8519426b8d47067efdfa3432e07a 
> 
> Diff: https://reviews.apache.org/r/53500/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> No new tests are written but the fact the existing tests that run exectuors 
> depends on this to work correctly is a proof.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 52696: Harden stout