Re: Review Request 69452: Added a test for executors sending messages to recovered frameworks.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69452 was successfully built and tested.

Reviews applied: `['69451', '69452']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2652/mesos-review-69452

- Mesos Reviewbot Windows


On Nov. 27, 2018, 4:58 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69452/
> ---
> 
> (Updated Nov. 27, 2018, 4:58 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, 
> and Till Toenshoff.
> 
> 
> Bugs: MESOS-9419
> https://issues.apache.org/jira/browse/MESOS-9419
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds the `ExecutorMessageToRecoveredHttpFramework` test to
> ensure that a master will not crash when forwarding an executor message
> to a recovered framework to prevent any future regression of MESOS-9419.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 651bb9ba9298fc3d7179ed86487aa55be519ce12 
>   src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e 
> 
> 
> Diff: https://reviews.apache.org/r/69452/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> This test will fail without r/69451.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 69454: Added the autoconf `tar-pax` option.

[James Peach]

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

Review request for mesos, Benjamin Bannier and Till Toenshoff.


Repository: mesos


Description
---

The default tar format used in `make dist` is v7, which only supports
paths of up to 99 bytes in length. This causes errors when building
the CentOS RPM and adding files from 3rd party packages.


Diffs
-

  configure.ac c193adf93531170fb31a31c296e3d4ae1d0300b2 


Diff: https://reviews.apache.org/r/69454/diff/1/


Testing
---

Ran `support/packaging/centos/build-rpm-docker.sh` and observed no tar errors.


Thanks,

James Peach

Review Request 69451: Fixed master crash when executors send messages to recovered frameworks.

[Chun-Hung Hsiao]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Till Toenshoff.


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


Repository: mesos


Description
---

The `Framework::send` function assumes that either `http` or `pid` is
set, which is not true for a unregistered framework recovered from agent
reregistration. As a result, the master would crash when a recovered
executor tries to send a message to such a framework. This patch fixes
this crash bug.


Diffs
-

  src/master/master.hpp 3b3c1a4e61de9503c8d038dd3bee623ded5914c9 
  src/master/master.cpp b4b02d8b4d7d6d1aabda1f97b9bf824419f76a9e 


Diff: https://reviews.apache.org/r/69451/diff/1/


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 69452: Added a test for executors sending messages to recovered frameworks.

[Chun-Hung Hsiao]

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

Review request for mesos, Alexander Rukletsov, Benjamin Mahler, Greg Mann, and 
Till Toenshoff.


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


Repository: mesos


Description
---

This patch adds the `ExecutorMessageToRecoveredHttpFramework` test to
ensure that a master will not crash when forwarding an executor message
to a recovered framework to prevent any future regression of MESOS-9419.


Diffs
-

  src/tests/master_tests.cpp 651bb9ba9298fc3d7179ed86487aa55be519ce12 
  src/tests/mesos.hpp 576f4bde88c069ee2fa0dd33912a034437338e7e 


Diff: https://reviews.apache.org/r/69452/diff/1/


Testing
---

make check

This test will fail without r/69451.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69211: Improved the code comments for `getContainerDevicesPath`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69211 was successfully built and tested.

Reviews applied: `['69086', '69210', '69450', '69211']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2651/mesos-review-69211

- Mesos Reviewbot Windows


On Nov. 17, 2018, 12:50 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69211/
> ---
> 
> (Updated Nov. 17, 2018, 12:50 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9319
> https://issues.apache.org/jira/browse/MESOS-9319
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved the code comments for `getContainerDevicesPath`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> de3981db7eb08e53901547037c947f594c8d46ab 
> 
> 
> Diff: https://reviews.apache.org/r/69211/diff/9/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69086: Moved the container root construction to the isolators.

[James Peach]

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

(Updated Nov. 27, 2018, 12:49 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.


Summary (updated)
-

Moved the container root construction to the isolators.


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


Repository: mesos


Description
---

Previously, if the container was configured with a root filesytem,
the root was populated by a combination of the `fs::chroot:prepare`
API and the various isolators. The implementation details of some
isolators had leaked into the chroot code, which had a special case
for adding GPU devices.

This change moves all the responsibility for defining the
root filesystem from the `fs::chroot::prepare()` API to the
`filesystem/linux` isolator. The `filesystem/linux` isolator is
now the single place that captures how to mount the container
pseudo-filesystems as well as how to construct a proper `/dev`
directory.

Since the `linux/filesystem` isolator is now entirely responsible
for creating and mounting the container `/dev`, any other isolators
that enable access to devices should populate device nodes in the
container devices directory and add a corresponding bind mount.


Diffs (updated)
-

  src/common/protobuf_utils.hpp 1662125ed3e47b179ee32d08c1d3af75553066ba 
  src/common/protobuf_utils.cpp a45607eed4c4bae5010bcc3f3ffeabd6d911062a 
  src/linux/fs.hpp 31969f6ba82bf5ee549bfdf9698a21adaa486a90 
  src/linux/fs.cpp 5cdffe1f4c7f00aee5b8f640e7cfa4a0018cfa0a 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
c7d753ac2e5575a8d687600bfb9e0617fa72c990 
  src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
4645c625877d9451516133b24bd3959e0f49c0a9 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
56d835779618fd965d928c6926664583e9141f79 
  src/slave/containerizer/mesos/isolators/linux/devices.cpp 
8f8ff95ec3856ba06647637a80315365d0e66e23 
  src/slave/containerizer/mesos/launch.cpp 
882bcdf89e2b0cca3d3f62e6d017849a51ceaead 


Diff: https://reviews.apache.org/r/69086/diff/12/

Changes: https://reviews.apache.org/r/69086/diff/11-12/


Testing
---

sudo make check (Fedora 28)


Thanks,

James Peach

Review Request 69450: Applied the `ContainerMountInfo` protobuf helper.

[James Peach]

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

Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Now that there is a protobuf helper to manufacture `ContainerMountInfo`
messages, apply it where appropriate.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
507665cdd3c08f6716840f2b10f2b9659e7fcf1a 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
d1938836997b177ec89c1e8d671dc6e0379aa061 
  src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
5b481b5e8e66104b8b7af6fa97dcb411fb0a1f5e 
  src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
73e09636808081ce411c022bf2bcb2a157a3ad25 
  src/slave/containerizer/mesos/isolators/volume/host_path.cpp 
88ecf91d91e2bebd484a4ac94510a14b3500dbfb 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
bb3fc659f1322d26fe3d035c961d3942c73353f0 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
300b3d95d74b73fbe0221096f3f3f172be745081 


Diff: https://reviews.apache.org/r/69450/diff/1/


Testing
---

sudo make check (Fedora 29)


Thanks,

James Peach

Re: Review Request 69449: Added the `DISCARD` blkio cgroup operation.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69449 was successfully built and tested.

Reviews applied: `['69449']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2650/mesos-review-69449

- Mesos Reviewbot Windows


On Nov. 26, 2018, 12:38 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69449/
> ---
> 
> (Updated Nov. 26, 2018, 12:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Linux 4.19 kernel added a new `Discard` operation to the blkio
> cgroup statistics. Added this new operation so that the containerizer
> won't fail to parse blkio statistics on the latest kernels.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6 
>   include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221 
>   src/linux/cgroups.hpp f4cc2d829fe4414b36532d0d77730f97e00fcb9a 
>   src/linux/cgroups.cpp 45dd5cf6747c1af8b18805dd83fcf542131d5708 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
> fbf1fed783934b10f9f726152ede33d3dade52d0 
> 
> 
> Diff: https://reviews.apache.org/r/69449/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 29)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69436: Fixed flaky check in cluster::Slave destructor.

[Joseph Wu]

> On Nov. 23, 2018, 5:44 a.m., Andrei Budnik wrote:
> > src/tests/cluster.cpp
> > Lines 699 (patched)
> > 
> >
> > What if there is more than one container whose 
> > `Future` is ready? Should we call `settle()` for each 
> > of these containers?
> 
> Benno Evers wrote:
> At this point we know from the `foreach`-loop above that all 
> `Future`s are ready, so all calls to `defer()` in 
> `composing.cpp` should have already happened. So I think a single call to 
> settle should guarantee that all of them will be processed by libprocess 
> before the clock is settled.
> 
> Andrei Budnik wrote:
> Can we check this assumption by trying to launch say 1000 containers in 
> the given test?

This might be a common (among Mesos test writers) misconception of what 
`Clock::settle()` actually does.  Operations involving the `Clock` will only 
affect timers and delayed events.  Other dispatches and deferals are not 
affected.

```
Clock::pause();

// If I delay something to the future here:
delay(Seconds(10), somePid, someFunction);

// And then I advance the clock to trigger the functions:
Clock::advance(Seconds(10));

// Checking state mutated by the delayed function will race with this thread 
(main thread vs libprocess worker thread).
// Unless, we settle the clock.  This explicitly waits for the delayed 
functions to finish, on the calling thread.
Clock::settle();
```

This means adding a `Clock::settle()` in this code is adding more a bit more 
computation between terminating the last container, and calling 
`containerizer->containers()`.  Which makes the flaky less likely, but still 
possible.

We still have a race between two threads (main/test thread & containerizer 
thread), gated on the same future, dispatching onto the composing 
containerizer.  
To completely remove the race, the main thread must dispatch on the 
Mesos/Docker containerizer actor, not the composing containerizer actor, 
because the Mesos/Docker containerizer actor's thread will be the one 
`defer`-ing onto the composing containerizer actor.  I think the following 
might work:

```
 foreach (const ContainerID& containerId, containers.get()) {
   process::Future> termination =
 containerizer->destroy(containerId);
 
   AWAIT(termination);
 
   if (!termination.isReady()) {
 LOG(ERROR) << "...";
   }
  
+  // Even though we just destroyed the container, fetch the status to
+  // synchronize this thread with the containerizer thread(s).
+  // This ensures that any continuations triggered by container termination
+  // have been dispatched (although not necessarily completed).
+  Future status = containerizer->status(containerId);
+  AWAIT(status);
+  
+  // We don't care about the success or failure of the `status` Future.
+  // There are three possible responses:
+  //   1) Mesos containerizer - This should return a failure since the 
container
+  //  no longer exists.
+  //   2) Docker containerizer - This containerizer seems to just return
+  //  a mostly-empty ContainerStatus object, regardless of whether the
+  //  container exists or not.
+  //   3) Composing containerizer - This could return either of the above, 
or a
+  //  failure originating from the Composing containerizer itself.  
This is
+  //  because the Composing containerizer does not update its internal 
state
+  //  immediately upon terminating a container, since termination and 
state
+  //  must be performed on different actors.
+  //  If this returns (1) or (2), then we know that the next dispatch 
onto
+  //  the Composing containerizer actor will come after the Composing 
+  //  containerizer has had a chance to update its internal state.
}
```

Feel free to reword the comment if it feels wordy or rambling or confusing.  
This is not a straightforward race to put into text :)


- Joseph


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


On Nov. 22, 2018, 11:43 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69436/
> ---
> 
> (Updated Nov. 22, 2018, 11:43 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Joseph Wu.
> 
> 
> Bugs: MESOS-9272
> https://issues.apache.org/jira/browse/MESOS-9272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The destructor of `cluster::Slave` contained an assertion
> that was not safe to assume in the presence of the
> composing containerizer. This commit adds an additional
> `Clock::settle()` 

Review Request 69449: Added the `DISCARD` blkio cgroup operation.

[James Peach]

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

Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Qian Zhang.


Repository: mesos


Description
---

The Linux 4.19 kernel added a new `Discard` operation to the blkio
cgroup statistics. Added this new operation so that the containerizer
won't fail to parse blkio statistics on the latest kernels.


Diffs
-

  include/mesos/mesos.proto 06a901d26693757edc653cd833d55aa42e4ff2c6 
  include/mesos/v1/mesos.proto 75cdb2889b2b645e23d9f5ab263ee63bf62b4221 
  src/linux/cgroups.hpp f4cc2d829fe4414b36532d0d77730f97e00fcb9a 
  src/linux/cgroups.cpp 45dd5cf6747c1af8b18805dd83fcf542131d5708 
  src/slave/containerizer/mesos/isolators/cgroups/subsystems/blkio.cpp 
fbf1fed783934b10f9f726152ede33d3dade52d0 


Diff: https://reviews.apache.org/r/69449/diff/1/


Testing
---

sudo make check (Fedora 29)


Thanks,

James Peach

Re: Review Request 69420: Added Seccomp isolator tests.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 67844.

Failed command: `python.exe .\support\apply-reviews.py -n -r 67844`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2649/mesos-review-69420

Relevant logs:

- 
[apply-review-67844.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2649/mesos-review-69420/logs/apply-review-67844.log):

```
error: missing binary patch data for '3rdparty/libseccomp-2.3.3.tar.gz'
error: binary patch does not apply to '3rdparty/libseccomp-2.3.3.tar.gz'
error: 3rdparty/libseccomp-2.3.3.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 21, 2018, 6:46 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69420/
> ---
> 
> (Updated Nov. 21, 2018, 6:46 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9409
> https://issues.apache.org/jira/browse/MESOS-9409
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5da75d3ef263b86af8d914d82cae889830097042 
>   src/tests/CMakeLists.txt 553516ad66cab4480b7211950fc726b7d9a3869b 
>   src/tests/containerizer/linux_seccomp_config.hpp PRE-CREATION 
>   src/tests/containerizer/linux_seccomp_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69420/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69447: Improved wording in comments for `AgentAdded` event.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69447 was successfully built and tested.

Reviews applied: `['69447']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2648/mesos-review-69447

- Mesos Reviewbot Windows


On Nov. 26, 2018, 9:44 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69447/
> ---
> 
> (Updated Nov. 26, 2018, 9:44 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The comment mentioned that `AgentAdded` events can be
> sent when an agent reregisters after a master failover.
> This is technically correct, but neglects to mention
> that the event can also be sent when the agent reregisters
> at any other point in time.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 80bd7c43cbb6d7df0dbd1c162cf4dda48f0e2e30 
>   include/mesos/v1/master/master.proto 
> cd41a7e8dc98753b5d2b8f93d92114fe97463636 
> 
> 
> Diff: https://reviews.apache.org/r/69447/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69441: Fixed sandbox creation chmod error handling in the agent.

[Ilya Pronin]

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


Ship it!




Ship It!

- Ilya Pronin


On Nov. 23, 2018, 9:22 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69441/
> ---
> 
> (Updated Nov. 23, 2018, 9:22 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik, Ilya Pronin, James Peach, and Qian 
> Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed sandbox creation chmod error handling in the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.cpp 9fd37f5456d45d520d6062577c1692a4be627c0e 
> 
> 
> Diff: https://reviews.apache.org/r/69441/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Review Request 69447: Improved wording in comments for `AgentAdded` event.

[Benno Evers]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

The comment mentioned that `AgentAdded` events can be
sent when an agent reregisters after a master failover.
This is technically correct, but neglects to mention
that the event can also be sent when the agent reregisters
at any other point in time.


Diffs
-

  include/mesos/master/master.proto 80bd7c43cbb6d7df0dbd1c162cf4dda48f0e2e30 
  include/mesos/v1/master/master.proto cd41a7e8dc98753b5d2b8f93d92114fe97463636 


Diff: https://reviews.apache.org/r/69447/diff/1/


Testing
---


Thanks,

Benno Evers

Re: Review Request 69436: Fixed flaky check in cluster::Slave destructor.

[Andrei Budnik]

> On Nov. 23, 2018, 1:44 p.m., Andrei Budnik wrote:
> > src/tests/cluster.cpp
> > Lines 699 (patched)
> > 
> >
> > What if there is more than one container whose 
> > `Future` is ready? Should we call `settle()` for each 
> > of these containers?
> 
> Benno Evers wrote:
> At this point we know from the `foreach`-loop above that all 
> `Future`s are ready, so all calls to `defer()` in 
> `composing.cpp` should have already happened. So I think a single call to 
> settle should guarantee that all of them will be processed by libprocess 
> before the clock is settled.

Can we check this assumption by trying to launch say 1000 containers in the 
given test?


- Andrei


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


On Nov. 22, 2018, 7:43 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69436/
> ---
> 
> (Updated Nov. 22, 2018, 7:43 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Joseph Wu.
> 
> 
> Bugs: MESOS-9272
> https://issues.apache.org/jira/browse/MESOS-9272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The destructor of `cluster::Slave` contained an assertion
> that was not safe to assume in the presence of the
> composing containerizer. This commit adds an additional
> `Clock::settle()` to fix the issue.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
> 
> 
> Diff: https://reviews.apache.org/r/69436/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `./src/mesos-tests --gtest_filter="SlaveTest.DefaultExecutorCommandInfo" 
> --verbose --gtest_repeat=5 --gtest_throw_on_failure` while simulatenously 
> running `stress-ng --random 64` on the same machine.
> 
> (before the change, `SlaveTest.DefaultExecutorCommandInfo` would fail roughly 
> once every 15000 runs without `stress-ng` and roughly once every 300 runs 
> with `stress-ng`)
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69395: Added '--all' flag to 'mesos task list'.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2647/mesos-review-69395

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2647/mesos-review-69395/logs/mesos-tests.log):

```
I1126 14:58:10.665406 54324 executor.cpp:918] Sending SIGTERM to process tree 
at pid 55c1-4ff9-bcc4-79f65eb49d86- because it is terminating
I1126 14:58:10.666405 59016 master.cpp:1275] Agent 
8c178cd5-55c1-4ff9-bcc4-79f65eb49d86-S0 at slave(461)@192.10.1.5:59448 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I1126 14:58:10.667402 59016 master.cpp:3273] Disconnecting agent 
8c178cd5-55c1-4ff9-bcc4-79f65eb49d86-S0 at slave(461)@192.10.1.5:59448 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1126 14:58:10.667402 59016 master.cpp:3292] Deactivating agent 
8c178cd5-55c1-4ff9-bcc4-79f65eb49d86-S0 at slave(461)@192.10.1.5:59448 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1126 14:58:10.667402 46364 hierarchical.cpp:357] Removed framework 
8c178cd5-55c1-4ff9-bcc4-79f65eb49d86-
I1126 14:58:10.667402 46364 hierarchical.cpp:801] Agent 
8c178cd5-55c1-4ff9-bcc4-79f65eb49d86-S0 deactivated
I1126 14:58:10.668414 59016 containerizer.cpp:2463] Destroying container 
9b953b62-5d06-4378-96ea-a3dbd15d5aaa in RUNNING state
I1126 14:58:10.668414[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (795 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (816 ms total)

[--] Global test environment tear-down
[==] 1054 tests from 103 test cases ran. (509518 ms total)
[  PASSED  ] 1053 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_InvokeFetchByName

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

 59016 containerizer.cpp:3130] Transitioning the state of container 
9b953b62-5d06-4378-96ea-a3dbd15d5aaa from RUNNING to DESTROYING
I1126 14:58:10.669421 59016 launcher.cpp:161] Asked to destroy container 
9b953b62-5d06-4378-96ea-a3dbd15d5aaa
W1126 14:58:10.670413 58692 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=11784 to peer '192.10.1.5:61260': IO failed with error 
code: The specified network name is no longer available.

W1126 14:58:10.671396 58692 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=11720 to peer '192.10.1.5:61261': IO failed with error 
code: The specified network name is no longer available.

I1126 14:58:10.759449 60456 containerizer.cpp:2969] Container 
9b953b62-5d06-4378-96ea-a3dbd15d5aaa has exited
I1126 14:58:10.797418 60904 master.cpp:1117] Master terminating
I1126 14:58:10.799414 59016 hierarchical.cpp:643] Removed agent 
8c178cd5-55c1-4ff9-bcc4-79f65eb49d86-S0
I1126 14:58:11.723407 58692 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Nov. 26, 2018, 1:54 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69395/
> ---
> 
> (Updated Nov. 26, 2018, 1:54 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this option, the command can show all the tasks that have ever been
> run. This makes the command's behavior closer to the one of 'docker ps'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> 0ce21b365d64c5c331ce7f1a5937258960e15b6f 
>   src/python/cli_new/lib/cli/tests/task.py 
> eb01bf5cae095a067572f534a13f4ec542101ca5 
> 
> 
> Diff: https://reviews.apache.org/r/69395/diff/5/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> test_list_all (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 8 tests in 20.889s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 

Re: Review Request 69395: Added '--all' flag to 'mesos task list'.

[Kevin Klues]

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




src/python/cli_new/lib/cli/tests/task.py
Lines 250 (patched)


Can we be more descriptive here?



src/python/cli_new/lib/cli/tests/task.py
Lines 252 (patched)


Comment should mention launching two tasks or something.


- Kevin Klues


On Nov. 26, 2018, 1:51 nachm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69395/
> ---
> 
> (Updated Nov. 26, 2018, 1:51 nachm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this option, the command can show all the tasks that have ever been
> run. This makes the command's behavior closer to the one of 'docker ps'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> 0ce21b365d64c5c331ce7f1a5937258960e15b6f 
>   src/python/cli_new/lib/cli/tests/task.py 
> eb01bf5cae095a067572f534a13f4ec542101ca5 
> 
> 
> Diff: https://reviews.apache.org/r/69395/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> test_list_all (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 8 tests in 20.889s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69395: Added '--all' flag to 'mesos task list'.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Nov. 26, 2018, 1:54 nachm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69395/
> ---
> 
> (Updated Nov. 26, 2018, 1:54 nachm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With this option, the command can show all the tasks that have ever been
> run. This makes the command's behavior closer to the one of 'docker ps'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> 0ce21b365d64c5c331ce7f1a5937258960e15b6f 
>   src/python/cli_new/lib/cli/tests/task.py 
> eb01bf5cae095a067572f534a13f4ec542101ca5 
> 
> 
> Diff: https://reviews.apache.org/r/69395/diff/5/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> test_list_all (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 8 tests in 20.889s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69445: WIP: Moved `ReadOnlyHandler` into separate header.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69445 was successfully built and tested.

Reviews applied: `['69071', '68795', '69411', '69421', '69064', '69422', 
'69445']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2646/mesos-review-69445

- Mesos Reviewbot Windows


On Nov. 26, 2018, 12:18 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69445/
> ---
> 
> (Updated Nov. 26, 2018, 12:18 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Exposed the former inner class `Master::ReadOnlyHandler`
> in a separate header so it can be used from unit tests.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 75ab6ea422a77e25049cd6afe007507d11650a06 
>   src/master/master.hpp e77babf22126838c63cd05e483875c9beb3ac5ff 
>   src/master/readonly_handler.hpp PRE-CREATION 
>   src/master/readonly_handler.cpp 8895374499dc6baa2c4d8a8dd86fddac4e39be29 
>   src/tests/master_load_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69445/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69395: Added '--all' flag to 'mesos task list'.

[Armand Grillet]

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

(Updated Nov. 26, 2018, 2:54 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

With this option, the command can show all the tasks that have ever been
run. This makes the command's behavior closer to the one of 'docker ps'.


Diffs (updated)
-

  src/python/cli_new/lib/cli/plugins/task/main.py 
0ce21b365d64c5c331ce7f1a5937258960e15b6f 
  src/python/cli_new/lib/cli/tests/task.py 
eb01bf5cae095a067572f534a13f4ec542101ca5 


Diff: https://reviews.apache.org/r/69395/diff/5/

Changes: https://reviews.apache.org/r/69395/diff/4-5/


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
Running the Mesos CLI unit tests

TestAgentPlugin
test_list (cli.tests.agent.TestAgentPlugin) ... ok

TestInfrastructure
test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok

TestTaskPlugin
test_exec (cli.tests.task.TestTaskPlugin) ... ok
test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
test_list (cli.tests.task.TestTaskPlugin) ... ok
test_list_all (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 8 tests in 20.889s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69395: Added '--all' flag to 'mesos task list'.

[Armand Grillet]

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

(Updated Nov. 26, 2018, 2:51 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Rebased.


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


Repository: mesos


Description
---

With this option, the command can show all the tasks that have ever been
run. This makes the command's behavior closer to the one of 'docker ps'.


Diffs (updated)
-

  src/python/cli_new/lib/cli/plugins/task/main.py 
0ce21b365d64c5c331ce7f1a5937258960e15b6f 
  src/python/cli_new/lib/cli/tests/task.py 
eb01bf5cae095a067572f534a13f4ec542101ca5 


Diff: https://reviews.apache.org/r/69395/diff/4/

Changes: https://reviews.apache.org/r/69395/diff/3-4/


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
Running the Mesos CLI unit tests

TestAgentPlugin
test_list (cli.tests.agent.TestAgentPlugin) ... ok

TestInfrastructure
test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok

TestTaskPlugin
test_exec (cli.tests.task.TestTaskPlugin) ... ok
test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
test_list (cli.tests.task.TestTaskPlugin) ... ok
test_list_all (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 8 tests in 20.889s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

[Armand Grillet]

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

(Updated Nov. 26, 2018, 2:45 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

Replaces 'running_tasks(master)', a function that was not generic nor
explicit, by 'wait_for_task(master, name, state, delay)'. This helper
function waits a 'delay' for a task with a given 'name' to be in a
certain 'state'.

All uses of 'running_tasks' have been replaced by the new function.


Diffs (updated)
-

  src/python/cli_new/lib/cli/tests/base.py 
58c96d7ba45a57cb824d1f7f146e6dc5e41572cd 
  src/python/cli_new/lib/cli/tests/task.py 
eb01bf5cae095a067572f534a13f4ec542101ca5 


Diff: https://reviews.apache.org/r/69426/diff/3/

Changes: https://reviews.apache.org/r/69426/diff/2-3/


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
Running the Mesos CLI unit tests

TestAgentPlugin
test_list (cli.tests.agent.TestAgentPlugin) ... ok

TestInfrastructure
test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok

TestTaskPlugin
test_exec (cli.tests.task.TestTaskPlugin) ... ok
test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
test_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 7 tests in 19.319s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69426: Replaced CLI test helper function 'running_tasks' by 'wait_for_task'.

[Kevin Klues]

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




src/python/cli_new/lib/cli/tests/base.py
Lines 516 (patched)


Let's return the task here. I'm not 100% sure when / how we will use it, 
but it's better than just retunring `None` in my opinion.



src/python/cli_new/lib/cli/tests/task.py
Lines 70 (patched)


Add the name of the task in the error message.



src/python/cli_new/lib/cli/tests/task.py
Lines 77 (patched)


This could fail for two reasons (which is fine), but we should change the 
error message to reflect exactly what's gone wrong (i.e. that we can't get the 
tasks from that endpoint -- not just that we couldn't open it).



src/python/cli_new/lib/cli/tests/task.py
Lines 78 (patched)


Let's put a line break here.



src/python/cli_new/lib/cli/tests/task.py
Lines 96-99 (original), 108-122 (patched)


Same comments as above.



src/python/cli_new/lib/cli/tests/task.py
Lines 129-132 (original), 152-166 (patched)


Same comments as above.



src/python/cli_new/lib/cli/tests/task.py
Lines 164-168 (original), 198-210 (patched)


Same comments as above.


- Kevin Klues


On Nov. 25, 2018, 11:08 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69426/
> ---
> 
> (Updated Nov. 25, 2018, 11:08 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaces 'running_tasks(master)', a function that was not generic nor
> explicit, by 'wait_for_task(master, name, state, delay)'. This helper
> function waits a 'delay' for a task with a given 'name' to be in a
> certain 'state'.
> 
> All uses of 'running_tasks' have been replaced by the new function.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> c28e2a65b5c1b881c2f4eb09baf9806338f26e40 
>   src/python/cli_new/lib/cli/tests/task.py 
> 1b48c0a4729043c874330552074b05368e38b715 
> 
> 
> Diff: https://reviews.apache.org/r/69426/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 7 tests in 19.319s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69425: Fixed name of task created when running mesos-cli-tests.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Nov. 22, 2018, 11:33 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69425/
> ---
> 
> (Updated Nov. 22, 2018, 11:33 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed name of task created when running mesos-cli-tests.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> c28e2a65b5c1b881c2f4eb09baf9806338f26e40 
> 
> 
> Diff: https://reviews.apache.org/r/69425/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 7 tests in 19.732s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69394: Updated 'mesos task list' to only display running tasks.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Nov. 25, 2018, 9:53 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69394/
> ---
> 
> (Updated Nov. 25, 2018, 9:53 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9399
> https://issues.apache.org/jira/browse/MESOS-9399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated 'mesos task list' to only display running tasks.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> 4abc70a2de7e63bb629d5ed82d0045b29fb237fa 
>   src/python/cli_new/lib/cli/tests/task.py 
> 1b48c0a4729043c874330552074b05368e38b715 
> 
> 
> Diff: https://reviews.apache.org/r/69394/diff/2/
> 
> 
> Testing
> ---
> 
> (mesos-cli) ?  cli_new (MESOS-9399) ? mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_exec (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_exit_status (cli.tests.task.TestTaskPlugin) ... ok
> test_exec_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 7 tests in 19.845s
> 
> OK
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69421: Exposed private data members for testing.

[Benno Evers]

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




src/master/master.hpp
Lines 1387 (patched)


I've implemented the "cleaner" approach of pulling out `ReadOnlyHandler` 
into a separate header in https://reviews.apache.org/r/69445/

The cleaner structuring comes at the cost of asymmetry between 
`ReadOnlyHandler` and the other inner handler classes, and a raw pointer member 
in a non-inner class, so I opted for this version instead.

It's not a strong preference though, so I'd be happy to merge the linked 
alternative into this review as well.


- Benno Evers


On Nov. 22, 2018, 1:21 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69421/
> ---
> 
> (Updated Nov. 22, 2018, 1:21 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For the unit tests introduced in the subsequent
> commit, access to the underlying master object
> of `cluster::Master` and to the definition of the
> class `Master::ReadOnlyHandler` are required.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3b3c1a4e61de9503c8d038dd3bee623ded5914c9 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
> 
> 
> Diff: https://reviews.apache.org/r/69421/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69064: Added unit tests for Master HTTP endpoints.

[Benno Evers]

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




src/tests/master_load_tests.cpp
Lines 216 (patched)


I'm not 100% happy about this part, but I also couldn't think of a better 
way to generate the references. Any ideas/comments?


- Benno Evers


On Nov. 22, 2018, 1:22 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69064/
> ---
> 
> (Updated Nov. 22, 2018, 1:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds a set of unit test to verify that
> basic Master HTTP endpoints still work correctly
> under the presence of request caching.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8da1a05b618f17542fec9b5057484a9bae57658a 
>   src/tests/master_load_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69064/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 69445: WIP: Moved `ReadOnlyHandler` into separate header.

[Benno Evers]

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

Review request for mesos.


Repository: mesos


Description
---

Exposed the former inner class `Master::ReadOnlyHandler`
in a separate header so it can be used from unit tests.


Diffs
-

  src/master/http.cpp 75ab6ea422a77e25049cd6afe007507d11650a06 
  src/master/master.hpp e77babf22126838c63cd05e483875c9beb3ac5ff 
  src/master/readonly_handler.hpp PRE-CREATION 
  src/master/readonly_handler.cpp 8895374499dc6baa2c4d8a8dd86fddac4e39be29 
  src/tests/master_load_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/69445/diff/1/


Testing
---


Thanks,

Benno Evers

Re: Review Request 69436: Fixed flaky check in cluster::Slave destructor.

[Benno Evers]

> On Nov. 23, 2018, 1:44 p.m., Andrei Budnik wrote:
> > src/tests/cluster.cpp
> > Lines 699 (patched)
> > 
> >
> > What if there is more than one container whose 
> > `Future` is ready? Should we call `settle()` for each 
> > of these containers?

At this point we know from the `foreach`-loop above that all 
`Future`s are ready, so all calls to `defer()` in 
`composing.cpp` should have already happened. So I think a single call to 
settle should guarantee that all of them will be processed by libprocess before 
the clock is settled.


- Benno


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


On Nov. 22, 2018, 7:43 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69436/
> ---
> 
> (Updated Nov. 22, 2018, 7:43 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Joseph Wu.
> 
> 
> Bugs: MESOS-9272
> https://issues.apache.org/jira/browse/MESOS-9272
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The destructor of `cluster::Slave` contained an assertion
> that was not safe to assume in the presence of the
> composing containerizer. This commit adds an additional
> `Clock::settle()` to fix the issue.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
> 
> 
> Diff: https://reviews.apache.org/r/69436/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `./src/mesos-tests --gtest_filter="SlaveTest.DefaultExecutorCommandInfo" 
> --verbose --gtest_repeat=5 --gtest_throw_on_failure` while simulatenously 
> running `stress-ng --random 64` on the same machine.
> 
> (before the change, `SlaveTest.DefaultExecutorCommandInfo` would fail roughly 
> once every 15000 runs without `stress-ng` and roughly once every 300 runs 
> with `stress-ng`)
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 69365: Recovered disk through `CREATE_DISK` in test `AgentRegisteredWithNewId`.

[Chun-Hung Hsiao]

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

(Updated Nov. 26, 2018, 8:30 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Fixed another test flake.


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


Repository: mesos


Description
---

Test `AgentRegisteredWithNewId` is now improved to exercise the code
path for recovering disks created by the last agent through
`CREATE_DISK`.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
077a46585bd56181ba199dc529e09f38f4971338 


Diff: https://reviews.apache.org/r/69365/diff/6/

Changes: https://reviews.apache.org/r/69365/diff/5-6/


Testing
---

make check

This test passes iff r/69362 is applied.


Thanks,

Chun-Hung Hsiao

Re: Review Request 69402: Fixed `CreateVolume` of the test CSI plugin.

[Chun-Hung Hsiao]

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

(Updated Nov. 26, 2018, 8:29 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed Benjamin's comments.


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


Repository: mesos


Description
---

This patch makes sure that `CreateVolume` is idempotent, and check if
the specified volume capability is supported.


Diffs (updated)
-

  src/examples/test_csi_plugin.cpp 66f4ee0e2381e94bf9110d1e6e9f79eac939f683 


Diff: https://reviews.apache.org/r/69402/diff/3/

Changes: https://reviews.apache.org/r/69402/diff/2-3/


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 69400: Refactored the test CSI plugin.

[Chun-Hung Hsiao]

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

(Updated Nov. 26, 2018, 8:28 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed Benjamin's comments.


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


Repository: mesos


Description
---

This patch does not introduce any functional change to the test CSI
plugin. It simply refactored the check for the default mount volume
capability and the parsing of `--volumes` flag.


Diffs (updated)
-

  src/examples/test_csi_plugin.cpp 66f4ee0e2381e94bf9110d1e6e9f79eac939f683 


Diff: https://reviews.apache.org/r/69400/diff/2/

Changes: https://reviews.apache.org/r/69400/diff/1-2/


Testing
---

make check


Thanks,

Chun-Hung Hsiao