Re: Review Request 70045: Skipped tests when verifying `master` as part of review verification.

2019-02-23 Thread Benjamin Bannier 


> On Feb. 23, 2019, 11:55 p.m., Vinod Kone wrote:
> > support/jenkins/reviewbot.sh
> > Lines 38 (patched)
> > 
> >
> > I don't think this is needed because verify-reviews.py sets the 
> > ENVIRONMENT and other variables explicitly before verifying the review.
> > 
> > The comment at #33 should probably moved to #27.

Done. Added a comment on verification setting up its own env to document our 
assumptions.


- Benjamin


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


On Feb. 23, 2019, 9:18 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70045/
> ---
> 
> (Updated Feb. 23, 2019, 9:18 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to flakes test results reported by reviewbot need manual
> verification so we can skip executing tests when validating the
> `master` branch. This saves some execution time during validation.
> 
> Incidentally with this patch a previous comment matches now matches
> what is happening.
> 
> 
> Diffs
> -
> 
>   support/jenkins/reviewbot.sh 3cdd7f1c909c930a561ce2890212118579a47b73 
> 
> 
> Diff: https://reviews.apache.org/r/70045/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70046: Prevented closing invalid file descriptors.

2019-02-23 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70046]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 23, 2019, 12:41 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70046/
> ---
> 
> (Updated Feb. 23, 2019, 12:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During the refactoring of `d838f2958e` we reorganized the code flow to
> propagate failures from `os::close` and introduce code which migh
> close an invalid file descriptor. While this would produce an `Error`
> result from `mktemp` in any case, it would have lead to confusing
> error messages in that scenario. This patch prevents closing invalid
> file descriptors altogether which is consistent with the
> pre-refactoring behavior.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mktemp.hpp 
> 8dab2599f13c3e1dab109423c8a938ec16540aaf 
> 
> 
> Diff: https://reviews.apache.org/r/70046/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70046: Prevented closing invalid file descriptors.

2019-02-23 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 70046 was successfully built and tested.

Reviews applied: `['70046']`

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

- Mesos Reviewbot Windows


On Feb. 23, 2019, 8:41 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70046/
> ---
> 
> (Updated Feb. 23, 2019, 8:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During the refactoring of `d838f2958e` we reorganized the code flow to
> propagate failures from `os::close` and introduce code which migh
> close an invalid file descriptor. While this would produce an `Error`
> result from `mktemp` in any case, it would have lead to confusing
> error messages in that scenario. This patch prevents closing invalid
> file descriptors altogether which is consistent with the
> pre-refactoring behavior.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mktemp.hpp 
> 8dab2599f13c3e1dab109423c8a938ec16540aaf 
> 
> 
> Diff: https://reviews.apache.org/r/70046/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70045: Skipped tests when verifying `master` as part of review verification.

2019-02-23 Thread Vinod Kone 

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


Fix it, then Ship it!





support/jenkins/reviewbot.sh
Line 31 (original), 31 (patched)


Add a comment here that we are not running tests for when building HEAD?



support/jenkins/reviewbot.sh
Lines 38 (patched)


I don't think this is needed because verify-reviews.py sets the ENVIRONMENT 
and other variables explicitly before verifying the review.

The comment at #33 should probably moved to #27.


- Vinod Kone


On Feb. 23, 2019, 8:18 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70045/
> ---
> 
> (Updated Feb. 23, 2019, 8:18 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to flakes test results reported by reviewbot need manual
> verification so we can skip executing tests when validating the
> `master` branch. This saves some execution time during validation.
> 
> Incidentally with this patch a previous comment matches now matches
> what is happening.
> 
> 
> Diffs
> -
> 
>   support/jenkins/reviewbot.sh 3cdd7f1c909c930a561ce2890212118579a47b73 
> 
> 
> Diff: https://reviews.apache.org/r/70045/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70045: Skipped tests when verifying `master` as part of review verification.

2019-02-23 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70045]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 23, 2019, 8:18 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70045/
> ---
> 
> (Updated Feb. 23, 2019, 8:18 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to flakes test results reported by reviewbot need manual
> verification so we can skip executing tests when validating the
> `master` branch. This saves some execution time during validation.
> 
> Incidentally with this patch a previous comment matches now matches
> what is happening.
> 
> 
> Diffs
> -
> 
>   support/jenkins/reviewbot.sh 3cdd7f1c909c930a561ce2890212118579a47b73 
> 
> 
> Diff: https://reviews.apache.org/r/70045/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 70045: Skipped tests when verifying `master` as part of review verification.

2019-02-23 Thread Mesos Reviewbot Windows 

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



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

Reviews applied: `['70045']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
I0223 21:27:02.819763 15548 ex is terminating
I0223 21:27:02.820765 65472 master.cpp:1269] Agent 
4a6c8b55-95e3-4a7a-b0d1-21364aa645c0-S0 at slave(494)@192.10.1.6:52785 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net) disconnected
I0223 21:27:02.820765 65472 master.cpp:3292] Disconnecting agent 
4a6c8b55-95e3-4a7a-b0d1-21364aa645c0-S0 at slave(494)@192.10.1.6:52785 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0223 21:27:02.820765 65472 master.cpp:3311] Deactivating agent 
4a6c8b55-95e3-4a7a-b0d1-21364aa645c0-S0 at slave(494)@192.10.1.6:52785 
(windows-02.chtsmhjxogyevckjfayqqcnjda.xx.internal.cloudapp.net)
I0223 21:27:02.820765 63040 hierarchical.cpp:390] Removed framework 
4a6c8b55-95e3-4a7a-b0d1-21364aa645c0-
I0223 21:27:02.820765 63040 hierarchical.cpp:827] Agent 
4a6c8b55-95e3-4a7a-b0d1-21364aa645c0-S0 deactivated
I0223 21:27:02.822764 63040 containerizer.cpp:2526] Destroying container 
d5abeaa5-a9d6-484d-ba8f-5e1d78bf7d62 in RUNNING state
I0223 21:27:02.823782 63040 containerizer.cpp:3193] Transition[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (689 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (708 ms total)

[--] Global test environment tear-down
[==] 1115 tests from 106 test cases ran. (58 ms total)
[  PASSED  ] 1114 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 232 DISABLED TESTS

ing the state of container d5abeaa5-a9d6-484d-ba8f-5e1d78bf7d62 from RUNNING to 
DESTROYING
I0223 21:27:02.823782 63040 launcher.cpp:161] Asked to destroy container 
d5abeaa5-a9d6-484d-ba8f-5e1d78bf7d62
W0223 21:27:02.824772 64660 process.cpp:1423] Failed to recv on socket 
WindowsFD::Type::SOCKET=2600 to peer '192.10.1.6:55181': IO failed with error 
code: The specified network name is no longer available.

W0223 21:27:02.825767 64660 process.cpp:838] Failed to recv on socket 
WindowsFD::Type::SOCKET=2916 to peer '192.10.1.6:55182': IO failed with error 
code: The specified network name is no longer available.

I0223 21:27:02.890841 69728 containerizer.cpp:3032] Container 
d5abeaa5-a9d6-484d-ba8f-5e1d78bf7d62 has exited
I0223 21:27:02.920841 69728 master.cpp:1109] Master terminating
I0223 21:27:02.921860 60944 hierarchical.cpp:678] Removed agent 
4a6c8b55-95e3-4a7a-b0d1-21364aa645c0-S0
I0223 21:27:03.782843 64660 process.cpp:927] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Feb. 23, 2019, 12:18 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70045/
> ---
> 
> (Updated Feb. 23, 2019, 12:18 p.m.)
> 
> 
> Review request for mesos, Till Toenshoff and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to flakes test results reported by reviewbot need manual
> verification so we can skip executing tests when validating the
> `master` branch. This saves some execution time during validation.
> 
> Incidentally with this patch a previous comment matches now matches
> what is happening.
> 
> 
> Diffs
> -
> 
>   support/jenkins/reviewbot.sh 3cdd7f1c909c930a561ce2890212118579a47b73 
> 
> 
> Diff: https://reviews.apache.org/r/70045/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 70046: Prevented closing invalid file descriptors.

2019-02-23 Thread Benjamin Bannier 

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

Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.


Repository: mesos


Description
---

During the refactoring of `d838f2958e` we reorganized the code flow to
propagate failures from `os::close` and introduce code which migh
close an invalid file descriptor. While this would produce an `Error`
result from `mktemp` in any case, it would have lead to confusing
error messages in that scenario. This patch prevents closing invalid
file descriptors altogether which is consistent with the
pre-refactoring behavior.


Diffs
-

  3rdparty/stout/include/stout/os/posix/mktemp.hpp 
8dab2599f13c3e1dab109423c8a938ec16540aaf 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 70045: Skipped tests when verifying `master` as part of review verification.

2019-02-23 Thread Benjamin Bannier 

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

Review request for mesos, Till Toenshoff and Vinod Kone.


Repository: mesos


Description
---

Due to flakes test results reported by reviewbot need manual
verification so we can skip executing tests when validating the
`master` branch. This saves some execution time during validation.

Incidentally with this patch a previous comment matches now matches
what is happening.


Diffs
-

  support/jenkins/reviewbot.sh 3cdd7f1c909c930a561ce2890212118579a47b73 


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


Testing
---


Thanks,

Benjamin Bannier

Re: Review Request 70044: Moved status update streams of operations on agent's default resources.

2019-02-23 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [69977, 69978, 70044]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 23, 2019, 1:34 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70044/
> ---
> 
> (Updated Feb. 23, 2019, 1:34 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-9597
> https://issues.apache.org/jira/browse/MESOS-9597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves the operation status update streams for operations
> affecting agent default resources from the root of the metadata
> directory to the agent's metadata directory, i.e.,
> `meta/operations/` to
> `meta/slaves/`.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp bddaef220b4c9ac7d7988664cd82828e32f5a0df 
>   src/slave/paths.cpp 4ff9c5da05aef1496323daf269ab9b3215b0b31f 
>   src/slave/slave.hpp 7ad495504e4ff144ac31812fbd4a3a1f4da86f02 
>   src/slave/slave.cpp e3c2c005d865b5c333e92e50e49ef398fe06ad79 
> 
> 
> Diff: https://reviews.apache.org/r/70044/diff/1/
> 
> 
> Testing
> ---
> 
> Tests still pass on GNU/Linux + quite some manual testing.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 70040: Added test for terminal operation updates after master failover.

2019-02-23 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [69968, 69960, 69961, 69962, 69963, 69967, 69980, 70014, 
69872, 69869, 70040]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Feb. 22, 2019, 12:31 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70040/
> ---
> 
> (Updated Feb. 22, 2019, 12:31 a.m.)
> 
> 
> Review request for mesos, Benno Evers, Gastón Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-9542
> https://issues.apache.org/jira/browse/MESOS-9542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test covers a corner case where an agent reregisters with the
> master with a pending operation, but the operation's originating
> framework is unknown.  This can occur in a variety of situations like:
>   * the master fails over and a framework never reregisters,
>   * a completed framework is rotated out of the master's memory with
> pending operations, or
>   * an agent with pending operations is migrated from one cluster to
> another.
> 
> In this case, the master should "adopt" the orphan operation only
> after a delay.  This gives the framework some time to reregister.
> But if the framework does not reregister in time, the master will
> be in charge of acknowledging operation status updates.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> a661951a0a326cc342aa0c45dd0967692ae70941 
> 
> 
> Diff: https://reviews.apache.org/r/70040/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> make check
> src/mesos-tests --gtest_filter="*TerminalOrphanOperationAfterMasterFailover*" 
> --verbose
> src/mesos-tests --gtest_filter="*Operation*" --verbose
> ```
> 
> (Internal CI run pending)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 69411: Added new interface for constructing `cluster::Master`.

2019-02-23 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [69411]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


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/69411/
> ---
> 
> (Updated Nov. 22, 2018, 1:21 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This review adds a new method `cluster::Master::create()` that
> allows constructing masters while only specifying the non-standard
> components required for the specific test, like this:
> 
> auto master = cluster::Master::build()
>   .withZookeeperUrl(url)
>   .withAllocator(allocator);
> 
> This is sometimes better known as "fluent interface".
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
>   src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 
> 
> 
> Diff: https://reviews.apache.org/r/69411/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>