Re: Review Request 69123: Fixed an early fd close in the cgroups event notifier.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69123 was successfully built and tested.

Reviews applied: `['69123']`

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

- Mesos Reviewbot Windows


On Oct. 23, 2018, 3:08 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69123/
> ---
> 
> (Updated Oct. 23, 2018, 3:08 a.m.)
> 
> 
> Review request for mesos and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The cgroups event notifier was closing the eventfd while an
> `io::read()` operation may be in progress. This can lead to
> bugs where the fd gets re-used and read from a stale io::read.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 95efc1def3318dd93d30d91e9c8a91376cc610b8 
> 
> 
> Diff: https://reviews.apache.org/r/69123/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 69123: Fixed an early fd close in the cgroups event notifier.

[Benjamin Mahler]

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

Review request for mesos and Qian Zhang.


Repository: mesos


Description
---

The cgroups event notifier was closing the eventfd while an
`io::read()` operation may be in progress. This can lead to
bugs where the fd gets re-used and read from a stale io::read.


Diffs
-

  src/linux/cgroups.cpp 95efc1def3318dd93d30d91e9c8a91376cc610b8 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 69036: WIP: Changed the semantics of `CREATE_DISK` and `DESTROY_DISK` operations.

[Chun-Hung Hsiao]

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

(Updated Oct. 23, 2018, 2:37 a.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


Changes
---

Renamed `CreateDisk.profile` to `CreateDisk.target_profile`.


Summary (updated)
-

WIP: Changed the semantics of `CREATE_DISK` and `DESTROY_DISK` operations.


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


Repository: mesos


Description
---

The semantics of these two operations has been updated to provide
primitives to import CSI volumes and recover CSI volumes against agent
ID changes and metadata loss.


Diffs (updated)
-

  include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae 
  include/mesos/v1/mesos.proto a5ebb786a98bd3fd34745ddc553aa7a751c0e337 


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

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


Testing
---

Test will be done later in the chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68957 was successfully built and tested.

Reviews applied: `['68953', '68956', '68957']`

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

- Mesos Reviewbot Windows


On Oct. 22, 2018, 4:53 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 22, 2018, 4:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/9/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

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

(Updated Oct. 22, 2018, 11:53 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

In clusters with high numbers of frameworks, it
can be necessary to control publishing of per
framework metrics. Metrics are still collected for
active frameworks, but will not be pulished if
this flag is set. This allows future extraction
of these metrics e.g. an API call or logs while
keeping the code change simple.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/allocator/mesos/metrics.hpp 
34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
  src/master/allocator/mesos/metrics.cpp 
73e68eb4ef53b56f2a764b0504e92d4688eb183c 
  src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
  src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
  src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


Diff: https://reviews.apache.org/r/68956/diff/8/

Changes: https://reviews.apache.org/r/68956/diff/7-8/


Testing
---

make check on OSX

test-framework in local cluster with flag on/off

flag off: 

{
  "allocator/event_queue_dispatches": 0,
  "allocator/mesos/allocation_run_latency_ms": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/count": 28,
  "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
  "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
  "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
  "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
  "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
  "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
  "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
  "allocator/mesos/allocation_run_ms": 0.038144,
  "allocator/mesos/allocation_run_ms/count": 28,
  "allocator/mesos/allocation_run_ms/max": 0.326912,
  "allocator/mesos/allocation_run_ms/min": 0.01408,
  "allocator/mesos/allocation_run_ms/p50": 0.038144,
  "allocator/mesos/allocation_run_ms/p90": 0.0663552,
  "allocator/mesos/allocation_run_ms/p95": 0.133964786,
  "allocator/mesos/allocation_run_ms/p99": 0.284541443,
  "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
  "allocator/mesos/allocation_run_ms/p": 0.326488294398,
  "allocator/mesos/allocation_runs": 28,
  "allocator/mesos/event_queue_dispatches": 6,
  "allocator/mesos/resources/cpus/offered_or_allocated": 0,
  "allocator/mesos/resources/cpus/total": 8,
  "allocator/mesos/resources/disk/offered_or_allocated": 0,
  "allocator/mesos/resources/disk/total": 471782,
  "allocator/mesos/resources/mem/offered_or_allocated": 0,
  "allocator/mesos/resources/mem/total": 15360,
  "master/cpus_percent": 0,
  "master/cpus_revocable_percent": 0,
  "master/cpus_revocable_total": 0,
  "master/cpus_revocable_used": 0,
  "master/cpus_total": 8,
  "master/cpus_used": 0,
  "master/disk_percent": 0,
  "master/disk_revocable_percent": 0,
  "master/disk_revocable_total": 0,
  "master/disk_revocable_used": 0,
  "master/disk_total": 471782,
  "master/disk_used": 0,
  "master/dropped_messages": 0,
  "master/elected": 1,
  "master/event_queue_dispatches": 10,
  "master/event_queue_http_requests": 0,
  "master/event_queue_messages": 0,
  "master/frameworks_active": 0,
  "master/frameworks_connected": 0,
  "master/frameworks_disconnected": 0,
  "master/frameworks_inactive": 0,
  "master/gpus_percent": 0,
  "master/gpus_revocable_percent": 0,
  "master/gpus_revocable_total": 0,
  "master/gpus_revocable_used": 0,
  "master/gpus_total": 0,
  "master/gpus_used": 0,
  "master/invalid_executor_to_framework_messages": 0,
  "master/invalid_framework_to_executor_messages": 0,
  "master/invalid_operation_status_update_acknowledgements": 0,
  "master/invalid_status_update_acknowledgements": 0,
  "master/invalid_status_updates": 0,
  "master/mem_percent": 0,
  "master/mem_revocable_percent": 0,
  "master/mem_revocable_total": 0,
  "master/mem_revocable_used": 0,
  "master/mem_total": 15360,
  "master/mem_used": 0,
  "master/messages_authenticate": 0,
  "master/messages_deactivate_framework": 1,
  "master/messages_decline_offers":

Re: Review Request 68953: Refactor allocator configuration into a struct.

[Jacob Janco]

> On Oct. 22, 2018, 3:44 a.m., Greg Mann wrote:
> > include/mesos/allocator/allocator.hpp
> > Lines 55 (patched)
> > 
> >
> > Is this necessary in this patch, or should it be added in the child 
> > patch which adds the flag?

I like that, I'll fold it into the child.


> On Oct. 22, 2018, 3:44 a.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 763-764 (patched)
> > 
> >
> > Fits on one line.

Fixed.


> On Oct. 22, 2018, 3:44 a.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 766-767 (patched)
> > 
> >
> > Fits on one line.

Fixed


- Jacob


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


On Oct. 22, 2018, 11:52 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68953/
> ---
> 
> (Updated Oct. 22, 2018, 11:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There has been a proliferation of configuration
> passed into the allocator. This commit collects
> that configuration into an `Options` struct.
> Test fixes have been added as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
>   src/master/allocator/mesos/allocator.hpp 
> a4d7f2be3e8ff71cc2c45cb8ed808b9dbb821aaf 
>   src/master/allocator/mesos/hierarchical.hpp 
> 7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
>   src/master/allocator/mesos/hierarchical.cpp 
> c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
>   src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
>   src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
>   src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> bf9167b63747f7b8a402d950947028436307082a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 27fbd9cf0c4442e7675362a806d35bad141ffb6d 
>   src/tests/master_allocator_tests.cpp 
> 88288ae8528900b017941db9b07f5f83649fa096 
>   src/tests/master_quota_tests.cpp f669396569c92609ee75ea7ae427eb5f5769bfd9 
>   src/tests/reservation_tests.cpp d6931220139d91620c886591d7916079b8541982 
>   src/tests/resource_offers_tests.cpp 
> 24800c2aa291431e4865e4104da62054b14e5eca 
>   src/tests/slave_recovery_tests.cpp 5842ccffaf8c409aaa9c84720ba6c7b07ba6dc7c 
> 
> 
> Diff: https://reviews.apache.org/r/68953/diff/7/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

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

(Updated Oct. 22, 2018, 11:53 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

Add documentation for per-framework metrics flag.


Diffs (updated)
-

  docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 


Diff: https://reviews.apache.org/r/68957/diff/9/

Changes: https://reviews.apache.org/r/68957/diff/8-9/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68953: Refactor allocator configuration into a struct.

[Jacob Janco]

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

(Updated Oct. 22, 2018, 11:52 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

There has been a proliferation of configuration
passed into the allocator. This commit collects
that configuration into an `Options` struct.
Test fixes have been added as well.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/allocator.hpp 
a4d7f2be3e8ff71cc2c45cb8ed808b9dbb821aaf 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
  src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/hierarchical_allocator_benchmarks.cpp 
bf9167b63747f7b8a402d950947028436307082a 
  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 
  src/tests/master_allocator_tests.cpp 88288ae8528900b017941db9b07f5f83649fa096 
  src/tests/master_quota_tests.cpp f669396569c92609ee75ea7ae427eb5f5769bfd9 
  src/tests/reservation_tests.cpp d6931220139d91620c886591d7916079b8541982 
  src/tests/resource_offers_tests.cpp 24800c2aa291431e4865e4104da62054b14e5eca 
  src/tests/slave_recovery_tests.cpp 5842ccffaf8c409aaa9c84720ba6c7b07ba6dc7c 


Diff: https://reviews.apache.org/r/68953/diff/7/

Changes: https://reviews.apache.org/r/68953/diff/6-7/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > In the discussion on Slack we talked about adding some explanation of why 
> > we choose to omit publishing rathe than omit metrics collection altogether. 
> > I don't see an obvious, central place to capture that in a comment, so lets 
> > add some explanatory discussion of the issue in the commit message.

I agree and had the same feeling. I'll add the info. to the commit.


> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 101 (patched)
> > 
> >
> > `const bool publishPerFrameworkMetrics`

Fixed.


> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > src/master/allocator/mesos/metrics.hpp
> > Lines 117 (patched)
> > 
> >
> > Remove this comment, I don't think it's giving much value.

Fixed.


> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > src/master/allocator/mesos/metrics.hpp
> > Lines 118 (patched)
> > 
> >
> > `const bool enabled`
> > 
> > For consistency, I would like to call this 
> > `publishPerFrameworkMetrics`, but that's a mouthful in this context. I'd be 
> > OK with `publish` though.
> > 
> > Greg WDYT?

For now, I'm going with the verbose option.


> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > src/master/allocator/mesos/metrics.cpp
> > Lines 280 (patched)
> > 
> >
> > Should be 2 newlines above each of these functions.

Fixed.


> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > src/master/flags.cpp
> > Lines 688 (patched)
> > 
> >
> > ```
> > If true, metrics will be published for each framework.\n"
> > ```

Fixed.


> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > src/master/metrics.cpp
> > Lines 758 (patched)
> > 
> >
> > The `template ` should be on its own line.

Fixed.


> On Oct. 22, 2018, 5:04 p.m., James Peach wrote:
> > src/master/metrics.cpp
> > Lines 770 (patched)
> > 
> >
> > 2 newlines here.

Fixed.


- Jacob


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


On Oct. 22, 2018, 11:42 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68956/
> ---
> 
> (Updated Oct. 22, 2018, 11:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In clusters with high numbers of frameworks, it
> can be necessary to control publishing of per
> framework metrics. Metrics are still collected for
> active frameworks, but will not be pulished if
> this flag is set. This allows future extraction
> of these metrics e.g. an API call or logs while
> keeping the code change simple.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
>   src/master/allocator/mesos/hierarchical.hpp 
> 7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
>   src/master/allocator/mesos/hierarchical.cpp 
> c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
>   src/master/allocator/mesos/metrics.hpp 
> 34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
>   src/master/allocator/mesos/metrics.cpp 
> 73e68eb4ef53b56f2a764b0504e92d4688eb183c 
>   src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
>   src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
>   src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
> 
> 
> Diff: https://reviews.apache.org/r/68956/diff/7/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> test-framework in local cluster with flag on/off
> 
> flag off: 
> 
> {
>   "allocator/event_queue_dispatches": 0,
>   "allocator/mesos/allocation_run_latency_ms": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/count": 28,
>   "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
>   "allocator/mesos/allocation_run_latency_ms/

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

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

(Updated Oct. 22, 2018, 11:42 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


Changes
---

Fixed review comments.


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


Repository: mesos


Description (updated)
---

In clusters with high numbers of frameworks, it
can be necessary to control publishing of per
framework metrics. Metrics are still collected for
active frameworks, but will not be pulished if
this flag is set. This allows future extraction
of these metrics e.g. an API call or logs while
keeping the code change simple.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/allocator/mesos/metrics.hpp 
34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
  src/master/allocator/mesos/metrics.cpp 
73e68eb4ef53b56f2a764b0504e92d4688eb183c 
  src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
  src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
  src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


Diff: https://reviews.apache.org/r/68956/diff/7/

Changes: https://reviews.apache.org/r/68956/diff/6-7/


Testing
---

make check on OSX

test-framework in local cluster with flag on/off

flag off: 

{
  "allocator/event_queue_dispatches": 0,
  "allocator/mesos/allocation_run_latency_ms": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/count": 28,
  "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
  "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
  "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
  "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
  "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
  "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
  "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
  "allocator/mesos/allocation_run_ms": 0.038144,
  "allocator/mesos/allocation_run_ms/count": 28,
  "allocator/mesos/allocation_run_ms/max": 0.326912,
  "allocator/mesos/allocation_run_ms/min": 0.01408,
  "allocator/mesos/allocation_run_ms/p50": 0.038144,
  "allocator/mesos/allocation_run_ms/p90": 0.0663552,
  "allocator/mesos/allocation_run_ms/p95": 0.133964786,
  "allocator/mesos/allocation_run_ms/p99": 0.284541443,
  "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
  "allocator/mesos/allocation_run_ms/p": 0.326488294398,
  "allocator/mesos/allocation_runs": 28,
  "allocator/mesos/event_queue_dispatches": 6,
  "allocator/mesos/resources/cpus/offered_or_allocated": 0,
  "allocator/mesos/resources/cpus/total": 8,
  "allocator/mesos/resources/disk/offered_or_allocated": 0,
  "allocator/mesos/resources/disk/total": 471782,
  "allocator/mesos/resources/mem/offered_or_allocated": 0,
  "allocator/mesos/resources/mem/total": 15360,
  "master/cpus_percent": 0,
  "master/cpus_revocable_percent": 0,
  "master/cpus_revocable_total": 0,
  "master/cpus_revocable_used": 0,
  "master/cpus_total": 8,
  "master/cpus_used": 0,
  "master/disk_percent": 0,
  "master/disk_revocable_percent": 0,
  "master/disk_revocable_total": 0,
  "master/disk_revocable_used": 0,
  "master/disk_total": 471782,
  "master/disk_used": 0,
  "master/dropped_messages": 0,
  "master/elected": 1,
  "master/event_queue_dispatches": 10,
  "master/event_queue_http_requests": 0,
  "master/event_queue_messages": 0,
  "master/frameworks_active": 0,
  "master/frameworks_connected": 0,
  "master/frameworks_disconnected": 0,
  "master/frameworks_inactive": 0,
  "master/gpus_percent": 0,
  "master/gpus_revocable_percent": 0,
  "master/gpus_revocable_total": 0,
  "master/gpus_revocable_used": 0,
  "master/gpus_total": 0,
  "master/gpus_used": 0,
  "master/invalid_executor_to_framework_messages": 0,
  "master/invalid_framework_to_executor_messages": 0,
  "master/invalid_operation_status_update_acknowledgements": 0,
  "master/invalid_status_update_acknowledgements": 0,
  "master/invalid_status_updates": 0,
  "master/mem_percent": 0,
  "master/mem_revocable_percent": 0,
  "master/mem_revocable_total": 0,
  "master/mem_revocable_used": 0,
  "master/mem_total": 15360,
  "master/mem_used": 0,
  "master/messages_authenticate": 0,
  "master/messages_deactivat

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

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

(Updated Oct. 22, 2018, 11:42 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

Add documentation for per-framework metrics flag.


Diffs (updated)
-

  docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 


Diff: https://reviews.apache.org/r/68957/diff/8/

Changes: https://reviews.apache.org/r/68957/diff/7-8/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68957 was successfully built and tested.

Reviews applied: `['68953', '68956', '68957']`

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

- Mesos Reviewbot Windows


On Oct. 23, 2018, 6:05 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 23, 2018, 6:05 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/7/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 68022: Enabled Seccomp filter in the containerizer launcher.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [68022, 68021, 68020, 68019, 68018, 68017, 68016, 67844]

Failed command: /usr/bin/python3 support/apply-reviews.py -n -r 67844

Error:
2018-10-22 22:52:21 URL:https://reviews.apache.org/r/67844/diff/raw/ [260/260] 
-> "67844.patch" [1]
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

Full log: https://builds.apache.org/job/Mesos-Reviewbot/23502/console

- Mesos Reviewbot


On Aug. 6, 2018, 1:39 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68022/
> ---
> 
> (Updated Aug. 6, 2018, 1:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9106
> https://issues.apache.org/jira/browse/MESOS-9106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Containerizer launcher creates an instance of `SeccompFilter`, which is
> used to setup Seccomp profile using `ContainerSeccompProfile` message
> prepared by the `linux/seccomp` isolator. The Seccomp filter is loaded
> right before calling `execve()`, so that a container will be running
> with a syscall filtering enabled.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 7193da0a094df3e441e185c62b3a0379a0bdc4a2 
> 
> 
> Diff: https://reviews.apache.org/r/68022/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69098: Added a benchmark to compare quota and nonquota allocation performance.

[Meng Zhu]

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

(Updated Oct. 22, 2018, 3:39 p.m.)


Review request for mesos, Benjamin Mahler and Gastón Kleiman.


Changes
---

Posted optimized build benchmark result.


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


Repository: mesos


Description
---

This benchmark evaluates the performance difference between nonquota
and quota settings. In both settings, the same allocations are made
for fair comparison.


Diffs
-

  src/tests/hierarchical_allocator_benchmarks.cpp 
bf9167b63747f7b8a402d950947028436307082a 


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


Testing (updated)
---

Result from optimized build on a multicore 2.2GHz machine:

Benchmark setup: 20 agents, 10 roles, 20 frameworks
Made 20 allocations in 3.508676ms for nonquota roles
Made 20 allocations in 7.901269ms for quota roles

Benchmark setup: 200 agents, 100 roles, 200 frameworks
Made 200 allocations in 63.407391ms for nonquota roles
Made 200 allocations in 279.002319ms for quota roles

Benchmark setup: 2000 agents, 1000 roles, 2000 frameworks
Made 2000 allocations in 4.003802373secs for nonquota roles
Made 2000 allocations in 20.503188535secs for quota roles


Thanks,

Meng Zhu

Re: Review Request 69097: Added an allocator benchmark for quota performance.

[Meng Zhu]

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

(Updated Oct. 22, 2018, 3:32 p.m.)


Review request for mesos, Benjamin Mahler and Gastón Kleiman.


Changes
---

Posted optimized build result.


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


Repository: mesos


Description
---

This benchmark evaluates the allocator performance in
the presence of roles with both small quota (which can
be satisfied by half an agent) as well as large quota
(which need resources from two agents). We setup the cluster,
trigger one allocation cycle and measure the elapsed time.


Diffs
-

  src/tests/hierarchical_allocator_benchmarks.cpp 
bf9167b63747f7b8a402d950947028436307082a 


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


Testing (updated)
---

Result from optimized build on a multicore 2.2GHz machine:

Benchmark setup: 20 agents, 25 roles, 25 frameworks
Made 30 allocations in 11.01225ms
Made 0 allocation in 5.758192ms

Benchmark setup: 200 agents, 250 roles, 250 frameworks
Made 300 allocations in 337.974387ms
Made 0 allocation in 313.298636ms

Benchmark setup: 2000 agents, 2500 roles, 2500 frameworks
Made 3000 allocations in 27.881684393secs
Made 0 allocation in 25.013398926secs


Thanks,

Meng Zhu

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

> On Oct. 22, 2018, 4:45 p.m., James Peach wrote:
> > docs/configuration/master.md
> > Lines 669 (patched)
> > 
> >
> > Since this is a boolean flag, it should be:
> > ```
> > --[no-]publish_per_framework_metrics
> > ```

Fixed.


- Jacob


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


On Oct. 22, 2018, 10:05 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 22, 2018, 10:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/7/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

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

(Updated Oct. 22, 2018, 10:05 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


Changes
---

Updated review comments.


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


Repository: mesos


Description
---

Add documentation for per-framework metrics flag.


Diffs (updated)
-

  docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 


Diff: https://reviews.apache.org/r/68957/diff/7/

Changes: https://reviews.apache.org/r/68957/diff/6-7/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

> On Oct. 22, 2018, 4:42 p.m., James Peach wrote:
> > docs/configuration/master.md
> > Lines 672 (patched)
> > 
> >
> > I think that we should give the operator a bit more context here. Maybe 
> > something like this:
> > 
> > 
> > ```
> > If true, an extensive set of metrics for each active 
> > framework will be published. These metrics are useful for understanding 
> > cluster behavior, but can be overwhelming for very large numbers of 
> > frameworks. (default: true)
> > ```

Sounds good, took it verbatim.


- Jacob


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


On Oct. 22, 2018, 10:05 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 22, 2018, 10:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/7/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 69115: Added test for interactive 'task exec'.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [69114, 69048, 69119, 69049, 69115]

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

- Mesos Reviewbot


On Oct. 22, 2018, 2:40 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69115/
> ---
> 
> (Updated Oct. 22, 2018, 2:40 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9342
> https://issues.apache.org/jira/browse/MESOS-9342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for interactive 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/task.py 
> 4b03cf13185f1c718f24a990a75a4dd74e7e132a 
> 
> 
> Diff: https://reviews.apache.org/r/69115/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 6 tests in 12.602s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69009: Stout: Added a sync option for `write` and `rename`.

[Chun-Hung Hsiao]

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

(Updated Oct. 22, 2018, 9:21 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Jie Yu, and 
Jan Schlicht.


Changes
---

Removed unnecessary blank lines.


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


Repository: mesos


Description
---

This patch adds an option for the caller to sync the file and directory
contents to the disk after a write to prevent filesystem inconsistency
against reboots.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/fsync.hpp 
9a6bbf6c51b0a7d6085924733e33d8b4f1bbc1ac 
  3rdparty/stout/include/stout/os/posix/rename.hpp 
9cff6db16c8a4e5a79bf0082e18a1133bd287796 
  3rdparty/stout/include/stout/os/windows/rename.hpp 
523912ac3bf315f70f542e8eab7d2d02249909b4 
  3rdparty/stout/include/stout/os/write.hpp 
cd35285a9595004bac627abf687588050aef8161 
  3rdparty/stout/include/stout/protobuf.hpp 
1d03e1e3a8dd642f7239d777fb04759caf100a8b 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

[Chun-Hung Hsiao]

> On Oct. 22, 2018, 6:05 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/os/posix/mkdir.hpp
> > Lines 45 (patched)
> > 
> >
> > I wonder if this has issues with symlinks.
> > 
> > Imagine e.g., that we have a directory `/dir` which is symlinked as 
> > `/symlink`.
> > 
> > * If we call `os::mkdir("/symlink/subdir")`, this code would just 
> > `fsync` `/symlink`, but never `/dir`.
> > * While above might be fixable, the situation becomes worse should a 
> > user `os::mkdir("/dir/subdir")` where we might need to `fsync` `/symlink`. 
> > It looks like this might be hard, 
> > http://austingroupbugs.net/view.php?id=672. Maybe there's a way to address 
> > this outside of POSIX, e.g., for Linux.

Actually, the `os::open` call in `os::fsync` will follow the symlink (since no 
`O_NOFOLLOW` is specified), so the problem does not exist. See: 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html


> On Oct. 22, 2018, 6:05 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/os/posix/mkdir.hpp
> > Lines 75 (patched)
> > 
> >
> > I am not sure we should abort on the first error here. Many documented 
> > failure scenarios of `::fsync` are either very unlikley in this scope or 
> > retryable.
> > 
> > To get to the error code we might need to invoke `::fsync` directly.

It seems to me that we should abort on any of the errors listed in 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fsync.html except for 
`EBADF`, which cannot happen.


- Chun-Hung


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


On Oct. 19, 2018, 5:54 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69085/
> ---
> 
> (Updated Oct. 19, 2018, 5:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9281
> https://issues.apache.org/jira/browse/MESOS-9281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure the directories created by `mkdir` are commited to their
> filesystems, an `fsync` will be called on the parent of each created
> directory.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mkdir.hpp 
> 418db9af310ed763a5ae4735c2ebdd1ca38738ba 
> 
> 
> Diff: https://reviews.apache.org/r/69085/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69110 was successfully built and tested.

Reviews applied: `['69110']`

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

- Mesos Reviewbot Windows


On Oct. 22, 2018, 6:37 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69110/
> ---
> 
> (Updated Oct. 22, 2018, 6:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8780
> https://issues.apache.org/jira/browse/MESOS-8780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Task protobuf message is updated to include the health check
> definition of a task when it is specified. Associated helpers are
> also updated along with a test which verifies that this field is
> reflected in master API responses.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
>   src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
>   src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
>   src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
>   src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 
> 
> 
> Diff: https://reviews.apache.org/r/69110/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69036: WIP: Updated the semantics of `CREATE_DISK` and `DESTROY_DISK` operations.

[Chun-Hung Hsiao]

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

(Updated Oct. 22, 2018, 6:50 p.m.)


Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.


Changes
---

Addressed James' comments.


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


Repository: mesos


Description
---

The semantics of these two operations has been updated to provide
primitives to import CSI volumes and recover CSI volumes against agent
ID changes and metadata loss.


Diffs (updated)
-

  include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae 
  include/mesos/v1/mesos.proto a5ebb786a98bd3fd34745ddc553aa7a751c0e337 


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

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


Testing
---

Test will be done later in the chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 68642: Added `lsof()` into stout.

[James Peach]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/lsof.hpp
Lines 24 (patched)


This should be `std::vector` now.


- James Peach


On Oct. 20, 2018, 7:02 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68642/
> ---
> 
> (Updated Oct. 20, 2018, 7:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `lsof()` into stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am a9c61fcba253a811494cdcdb0afb3d3a018f4585 
>   3rdparty/stout/include/stout/os.hpp 
> aee041891b7e7ff93a0b1ac31019a7a3d4eae962 
>   3rdparty/stout/include/stout/os/lsof.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/lsof.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/lsof.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68642/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Greg Mann]

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

(Updated Oct. 22, 2018, 6:37 p.m.)


Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

The Task protobuf message is updated to include the health check
definition of a task when it is specified. Associated helpers are
also updated along with a test which verifies that this field is
reflected in master API responses.


Diffs
-

  include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
  include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
  src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
  src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 


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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 69110: Added task health check definitions to master API responses.

[Greg Mann]

> On Oct. 22, 2018, 8:42 a.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto
> > Lines 2200-2201 (patched)
> > 
> >
> > Let's add a `TODO` here for `CheckInfo`. Here and below.

I went ahead and modified `MasterAPITest.Subscribe` to verify the TASK_ADDED 
event, PTAL.


- Greg


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


On Oct. 22, 2018, 6:20 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69110/
> ---
> 
> (Updated Oct. 22, 2018, 6:20 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8780
> https://issues.apache.org/jira/browse/MESOS-8780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added task health check definitions to master API responses.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
>   src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
>   src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
>   src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
>   src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 
> 
> 
> Diff: https://reviews.apache.org/r/69110/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Greg Mann]

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

(Updated Oct. 22, 2018, 6:20 p.m.)


Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Added task health check definitions to master API responses.


Diffs (updated)
-

  include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
  include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
  src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
  src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 


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

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

[Benjamin Bannier]

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




3rdparty/stout/include/stout/os/posix/mkdir.hpp
Lines 45 (patched)


I wonder if this has issues with symlinks.

Imagine e.g., that we have a directory `/dir` which is symlinked as 
`/symlink`.

* If we call `os::mkdir("/symlink/subdir")`, this code would just `fsync` 
`/symlink`, but never `/dir`.
* While above might be fixable, the situation becomes worse should a user 
`os::mkdir("/dir/subdir")` where we might need to `fsync` `/symlink`. It looks 
like this might be hard, http://austingroupbugs.net/view.php?id=672. Maybe 
there's a way to address this outside of POSIX, e.g., for Linux.



3rdparty/stout/include/stout/os/posix/mkdir.hpp
Lines 64 (patched)


See above.



3rdparty/stout/include/stout/os/posix/mkdir.hpp
Lines 75 (patched)


I am not sure we should abort on the first error here. Many documented 
failure scenarios of `::fsync` are either very unlikley in this scope or 
retryable.

To get to the error code we might need to invoke `::fsync` directly.


- Benjamin Bannier


On Oct. 19, 2018, 7:54 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69085/
> ---
> 
> (Updated Oct. 19, 2018, 7:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9281
> https://issues.apache.org/jira/browse/MESOS-9281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure the directories created by `mkdir` are commited to their
> filesystems, an `fsync` will be called on the parent of each created
> directory.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mkdir.hpp 
> 418db9af310ed763a5ae4735c2ebdd1ca38738ba 
> 
> 
> Diff: https://reviews.apache.org/r/69085/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68022: Enabled Seccomp filter in the containerizer launcher.

[Mesos Reviewbot Windows]

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



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/2510/mesos-review-68022

Relevant logs:

- 
[apply-review-67844.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2510/mesos-review-68022/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 Aug. 6, 2018, 1:39 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68022/
> ---
> 
> (Updated Aug. 6, 2018, 1:39 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9106
> https://issues.apache.org/jira/browse/MESOS-9106
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Containerizer launcher creates an instance of `SeccompFilter`, which is
> used to setup Seccomp profile using `ContainerSeccompProfile` message
> prepared by the `linux/seccomp` isolator. The Seccomp filter is loaded
> right before calling `execve()`, so that a container will be running
> with a syscall filtering enabled.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 7193da0a094df3e441e185c62b3a0379a0bdc4a2 
> 
> 
> Diff: https://reviews.apache.org/r/68022/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.1 where needed.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['69075']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
I1022 17:21:30.089454 21200 executor.cpp:184] Received SHUTDOWN event
I1022 17:21:30.089454 21200 executor.cpp:805] Shutting down
I1022 17:21:30.089454 21200 executor.cpp:918] Sending SIGTERM to process tree 
at pid 276task 28bfbb95-cf0c-4807-8fd7-fa2b82ee7881 with resources 
cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; 
ports(allocated: *):[31000-32000] of framework 
3c466bf9-2352-43d0-84ec-6ed9760f3391- on agent 
3c466bf9-2352-43d0-84ec-6ed9760f3391-S0 at slave(461)@192.10.1.8:49162 
(windows-05.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1022 17:21:30.089454 30408 slave.cpp:915] Agent terminating
W1022 17:21:30.089454 30408 slave.cpp:3923] Ignoring shutdown framework 
3c466bf9-2352-43d0-84ec-6ed9760f3391- because it is terminating
I1022 17:21:30.091459 25328 master.cpp:1266] Agent 
3c466bf9-2352-43d0-84ec-6ed9760f3391-S0 at slave(461)@192.10.1.8:49162 
(windows-05.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I1022 17:21:30.091459 25328 master.cpp:3282] Disconnecting agent 
3c466bf9-2352-43d0-84ec-6ed9760f3391-S0 at slave(461)@192.10.1.8:49162 
(windows-05.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1022 17:21:30.091459 25328 master.cpp:3301] Deactivating agent 
3c466bf9-2352-43d0-84ec-6ed9760f3391-S0 at slave(461)@192.10.1.8:49162 
(windows-05.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1022 17:21:30.092567 27548 hierarchical.cpp:359] Removed framework 
3c466bf9-2352-43d0-84ec-6ed9760f3391-
I1022 17:21:30.092567 27548 hierarchical.cpp:803] Agent 
3c466bf9-2352-43d0-84ec-6ed9760f3391-S0 deactivated
I1022 17:21:30.093459 29184 containerizer.cpp:2455] Destroying container 
a811e304-31d7-4c6d-a9b6-a9698407afa1 in RUNNING state
I1022 17:21:30.094449 29184 containerizer.cpp:3122] Transitioning the state of 
container a811e304-31d7-4c6d-a9b6-a9698407afa1 from RUNNING to DESTROYING
I1022 17:21:30.094449 29184 launcher.cpp:166] Asked to destroy container 
a811e304-31d7-4c6d-a9b6-a9698407afa1
I1022 17:21:30.191465 25328 containerizer.cpp:2961] Container 
a811e304-31d7-4c6d-a9b6-a9698407afa1 has exited
I1022 17:21:30.221736 24116 master.cpp:1108] Master terminating
I1022 17:21:30.223459 30628 hierarchical.cpp:645] Removed agent 
3c466bf9-2352-43d0-84ec-6ed9760f3391-S[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (684 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (704 ms total)

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

 1 FAILED TEST
  YOU HAVE 232 DISABLED TESTS

0
I1022 17:21:30.594519 30068 process.cpp:926] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Oct. 22, 2018, 4:22 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 4:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.1. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl is old enough to not support that flag, we
> can deduct that it will not enforce HTTP 2 and therefore need no
> further actions.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/

Re: Review Request 68017: Added Seccomp-related protobuf messages.

[James Peach]

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




include/mesos/mesos.proto
Lines 3085 (patched)


I'm not sure we should have a default here. The default seccomp profile 
should probably be defined by te operator, which implies an agent flag (which 
could have a default value). Specifying a default in the protobuf seems likely 
to cause interoperatbility problems.


- James Peach


On Aug. 6, 2018, 1:38 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68017/
> ---
> 
> (Updated Aug. 6, 2018, 1:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9033
> https://issues.apache.org/jira/browse/MESOS-9033
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/slave/containerizer.proto 
> 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 
> 
> 
> Diff: https://reviews.apache.org/r/68017/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68018: Added `SeccompFilter` class.

[James Peach]

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




src/linux/seccomp/seccomp.cpp
Lines 21 (patched)


We avoid `using namespace foo` statements as it is not explicit about which 
symbols are pulled in, and it can often pull in a lot of symbols, which 
sometimes lead to conflicts.



src/linux/seccomp/seccomp.cpp
Lines 80 (patched)


`static const`



src/linux/seccomp/seccomp.cpp
Lines 99 (patched)


`static const`



src/linux/seccomp/seccomp.cpp
Lines 135 (patched)


Drop the fullstop at the end, otherwise this error will stringify as:
```
Failed to load Seccomp filter.: invalid argument
```


- James Peach


On Aug. 6, 2018, 1:38 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68018/
> ---
> 
> (Updated Aug. 6, 2018, 1:38 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.
> 
> 
> Bugs: MESOS-9034
> https://issues.apache.org/jira/browse/MESOS-9034
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `SeccompFilter` class is a wrapper for `libseccomp` API. Its main
> purpose is to provide a translation of the `ContainerSeccompProfile`
> message into calls of `libseccomp` API.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 
>   src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 
>   src/linux/seccomp/seccomp.hpp PRE-CREATION 
>   src/linux/seccomp/seccomp.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68018/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[James Peach]

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



In the discussion on Slack we talked about adding some explanation of why we 
choose to omit publishing rathe than omit metrics collection altogether. I 
don't see an obvious, central place to capture that in a comment, so lets add 
some explanatory discussion of the issue in the commit message.


src/master/allocator/mesos/hierarchical.hpp
Lines 101 (patched)


`const bool publishPerFrameworkMetrics`



src/master/allocator/mesos/metrics.hpp
Lines 117 (patched)


Remove this comment, I don't think it's giving much value.



src/master/allocator/mesos/metrics.hpp
Lines 118 (patched)


`const bool enabled`

For consistency, I would like to call this `publishPerFrameworkMetrics`, 
but that's a mouthful in this context. I'd be OK with `publish` though.

Greg WDYT?



src/master/allocator/mesos/metrics.cpp
Lines 280 (patched)


Should be 2 newlines above each of these functions.



src/master/flags.cpp
Lines 688 (patched)


```
If true, metrics will be published for each framework.\n"
```



src/master/metrics.cpp
Lines 758 (patched)


The `template ` should be on its own line.



src/master/metrics.cpp
Lines 770 (patched)


2 newlines here.


- James Peach


On Oct. 18, 2018, 10:59 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68956/
> ---
> 
> (Updated Oct. 18, 2018, 10:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In clusters with high numbers of frameworks, it is
> necessary to control the registration of per
> framework metrics.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
>   src/master/allocator/mesos/hierarchical.hpp 
> 7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
>   src/master/allocator/mesos/hierarchical.cpp 
> c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
>   src/master/allocator/mesos/metrics.hpp 
> 34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
>   src/master/allocator/mesos/metrics.cpp 
> 73e68eb4ef53b56f2a764b0504e92d4688eb183c 
>   src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
>   src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
>   src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
> 
> 
> Diff: https://reviews.apache.org/r/68956/diff/6/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> test-framework in local cluster with flag on/off
> 
> flag off: 
> 
> {
>   "allocator/event_queue_dispatches": 0,
>   "allocator/mesos/allocation_run_latency_ms": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/count": 28,
>   "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
>   "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
>   "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
>   "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
>   "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
>   "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
>   "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
>   "allocator/mesos/allocation_run_ms": 0.038144,
>   "allocator/mesos/allocation_run_ms/count": 28,
>   "allocator/mesos/allocation_run_ms/max": 0.326912,
>   "allocator/mesos/allocation_run_ms/min": 0.01408,
>   "allocator/mesos/allocation_run_ms/p50": 0.038144,
>   "allocator/mesos/allocation_run_ms/p90": 0.0663552,
>   "allocator/mesos/allocation_run_ms/p95": 0.133964786,
>   "allocator/mesos/allocation_run_ms/p99": 0.284541443,
>   "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
>   "allocator/mesos/allocation_run_ms/p": 0.326488294398,
>   "allocator/mesos/allocation_runs": 28,
>   "allocator/mesos/event_queue_dispatches": 6,
>   "al

Re: Review Request 68019: Added a parser for the Docker Seccomp config format.

[Andrei Budnik]

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

(Updated Oct. 22, 2018, 5:04 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang.


Changes
---

small refactoring & bug fixes & more comments


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


Repository: mesos


Description
---

Docker Seccomp config is a JSON file containing Seccomp filtering
rules. This patch introduces a parser for Docker Seccomp config format.
This parser accepts a JSON-string, parses and validates it, then
returns a prepared `ContainerSeccompProfile` message.


Diffs (updated)
-

  src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 
  src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 
  src/linux/seccomp/seccomp_parser.hpp PRE-CREATION 
  src/linux/seccomp/seccomp_parser.cpp PRE-CREATION 


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

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


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 68016: Added libseccomp to the build.

[Andrei Budnik]

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

(Updated Oct. 22, 2018, 5:04 p.m.)


Review request for mesos, Andrew Schwartzmeyer, Gilbert Song, Jie Yu, James 
Peach, and Qian Zhang.


Changes
---

rebased


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


Repository: mesos


Description
---

This library is needed to implement Seccomp syscall filtering in the
Mesos containerizer. This patch introduces `seccomp-isolator` build
flag, which is used to include or exclude sources related to Seccomp
from the build. Since Seccomp is a Linux-specific feature, the flag
is disabled by default. Enabling `seccomp-isolator` means either:

1. Compiling and linking against the bundled version of libseccomp from
   sources (default).

2. Linking against the libseccomp installed in the OS,
   if `--with-libseccomp` build flag is provided.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 9584f537cc2a862ce17037224fd0628ffda54b2b 
  3rdparty/Makefile.am e625e7be1743348d02c6dbb8e0a92d1a395b0ef4 
  3rdparty/cmake/Versions.cmake 69fc594ec5ba2887b20b88ec0767a5d801411411 
  3rdparty/versions.am 99ef92087f6958d83ba415e84db5cbbb0c597573 
  cmake/CompilationConfigure.cmake 5d2be0adc55ac5302c2e0e62131feb65657466be 
  configure.ac 1caab4d0630821a937d92f3c788a01ae3b5dc228 
  src/CMakeLists.txt 732368293049b7d9d6f62057344d433637ad44e8 
  src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 


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

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


Testing
---


Thanks,

Andrei Budnik

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[James Peach]

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




docs/configuration/master.md
Lines 669 (patched)


Since this is a boolean flag, it should be:
```
--[no-]publish_per_framework_metrics
```


- James Peach


On Oct. 18, 2018, 10:59 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 18, 2018, 10:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/6/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[James Peach]

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




docs/configuration/master.md
Lines 672 (patched)


I think that we should give the operator a bit more context here. Maybe 
something like this:

```
If true, an extensive set of metrics for each active framework 
will be published. These metrics are useful for understanding cluster behavior, 
but can be overwhelming for very large numbers of frameworks. (default: true)
```


- James Peach


On Oct. 18, 2018, 10:59 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 18, 2018, 10:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/6/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 69116: Added 'popen_tty' to test util functions for the new CLI.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [69116]

Failed command: /usr/bin/python3 support/apply-reviews.py -n -r 69116

Error:
2018-10-22 16:36:31 URL:https://reviews.apache.org/r/69116/diff/raw/ 
[1588/1588] -> "69116.patch" [1]
error: patch failed: src/python/cli_new/lib/cli/tests/base.py:486
error: src/python/cli_new/lib/cli/tests/base.py: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/23500/console

- Mesos Reviewbot


On Oct. 22, 2018, 12:23 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69116/
> ---
> 
> (Updated Oct. 22, 2018, 12:23 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9343
> https://issues.apache.org/jira/browse/MESOS-9343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code was pulled directly from:
> https://github.com/dcos/dcos-core-cli/blob/
> 7fd55421939a7782c237e2b8719c0fe2f543acd7/
> python/lib/dcoscli/dcoscli/test/common.py
> 
> This function will be used by tests requiring a TTY. This will be the
> case for tests concerning the 'task attach' subcommand.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> 3fb471c1f49e930d908322055bb9a188f88ee602 
> 
> 
> Diff: https://reviews.apache.org/r/69116/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.1.

[James Peach]

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


Fix it, then Ship it!





src/uri/fetchers/docker.cpp
Lines 124 (patched)


I think we can improve this comment ..

> the HTTP 1.1 locking

??

> therefor

therefore

Maybe:
```
Older curl versions do not support the HTTP.1.1 flag, but these versions 
are also old enough to not default to HTTP/2.
```


- James Peach


On Oct. 22, 2018, 4:22 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 4:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.1. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl is old enough to not support that flag, we
> can deduct that it will not enforce HTTP 2 and therefor need no
> further actions.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69115: Added test for interactive 'task exec'.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69115 was successfully built and tested.

Reviews applied: `['69115']`

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

- Mesos Reviewbot Windows


On Oct. 22, 2018, 2:40 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69115/
> ---
> 
> (Updated Oct. 22, 2018, 2:40 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9342
> https://issues.apache.org/jira/browse/MESOS-9342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for interactive 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/task.py 
> 4b03cf13185f1c718f24a990a75a4dd74e7e132a 
> 
> 
> Diff: https://reviews.apache.org/r/69115/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 6 tests in 12.602s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.1.

[Till Toenshoff via Review Board]

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

(Updated Oct. 22, 2018, 4:22 p.m.)


Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
James Peach.


Changes
---

Updated again to address recent comment by James - thanks a bunch!


Summary (updated)
-

Updated docker image fetcher to enforce HTTP 1.1.


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


Repository: mesos


Description (updated)
---

Modifies the 'curl' invocation that is returning an http::Response,
locking it into HTTP 1.1. Our current HTTP parser is unable to process
HTTP 2 responses.

With the advent of curl 7.47, HTTPS connections are being enforced
towards HTTP 2 rather aggressively. As a result, our image fetcher
fails when recent curl versions are being used for pulling images from
a registry that supports HTTP 2.

HTTP 1.1 is chosen as long as the underlying curl supports the
'--http1.1' flag. If curl is old enough to not support that flag, we
can deduct that it will not enforce HTTP 2 and therefor need no
further actions.

For allowing all the benefits of HTTP 2 where possible, we do not
adapt any 'curl' invocations that do not attempt to parse headers.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 


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

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


Testing
---

`make check`
`sudo ./bin/mesos-tests.sh 
--gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
 on a system with curl 7.59.0 installed.


Thanks,

Till Toenshoff

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Till Toenshoff via Review Board]

> On Oct. 22, 2018, 4:03 p.m., James Peach wrote:
> > src/uri/fetchers/docker.cpp
> > Lines 123 (patched)
> > 
> >
> > If curl is so old that it doesn't accept the `--http1.1` flags, then 
> > it's not going to use HTTP/2 by default. In this case, there's no point 
> > forcing HTTP/1.0.

That way we could avoid forcing 1.0 when 1.1 was actually working (just not 
offered as a flag) - very good point - thank you!


- Till


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


On Oct. 22, 2018, 11:22 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 11:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[James Peach]

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




src/uri/fetchers/docker.cpp
Lines 123 (patched)


If curl is so old that it doesn't accept the `--http1.1` flags, then it's 
not going to use HTTP/2 by default. In this case, there's no point forcing 
HTTP/1.0.


- James Peach


On Oct. 22, 2018, 11:22 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 11:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [69110]

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

- Mesos Reviewbot


On Oct. 22, 2018, 5:35 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69110/
> ---
> 
> (Updated Oct. 22, 2018, 5:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8780
> https://issues.apache.org/jira/browse/MESOS-8780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Task protobuf message is updated to include the health check
> definition of a task when it is specified. Associated helpers are
> also updated along with a test which verifies that this field is
> reflected in master API responses.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
>   src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
>   src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
>   src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 
> 
> 
> Diff: https://reviews.apache.org/r/69110/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69115: Added test for interactive 'task exec'.

[Armand Grillet]

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

(Updated Oct. 22, 2018, 4:40 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

Added test for interactive 'task exec'.


Diffs (updated)
-

  src/python/cli_new/lib/cli/tests/task.py 
4b03cf13185f1c718f24a990a75a4dd74e7e132a 


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

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


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-6551) ? 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_interactive (cli.tests.task.TestTaskPlugin) ... ok
test_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 6 tests in 12.602s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69115: Added test for interactive 'task exec'.

[Kevin Klues]

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




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


Can we add the same as for the non-interactive case:
```
raise CLIException("Unable to find running task in master state 
'{master}'".format(master=master))
```



src/python/cli_new/lib/cli/tests/task.py
Lines 117-118 (patched)


Need to change this based on recent constants added for TEST_DIRECTORY and 
TEST_DATA_DIRECTORY



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


This comment seems unnecessary



src/python/cli_new/lib/cli/tests/task.py
Lines 124-126 (patched)


I don't really like the way we do this. I'd rather just open it twice and 
get the content twice.


- Kevin Klues


On Okt. 22, 2018, 11:59 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69115/
> ---
> 
> (Updated Okt. 22, 2018, 11:59 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9342
> https://issues.apache.org/jira/browse/MESOS-9342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for interactive 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/task.py 
> b54ade557f579a489e459f6022807146e0211fb0 
> 
> 
> Diff: https://reviews.apache.org/r/69115/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 6 tests in 12.602s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69049: Added test for 'task exec'.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Okt. 22, 2018, 2:13 nachm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69049/
> ---
> 
> (Updated Okt. 22, 2018, 2:13 nachm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/task.py 
> b54ade557f579a489e459f6022807146e0211fb0 
> 
> 
> Diff: https://reviews.apache.org/r/69049/diff/3/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 5 tests in 8.754s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69049: Added test for 'task exec'.

[Armand Grillet]

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

(Updated Oct. 22, 2018, 4:13 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description
---

Added test for 'task exec'.


Diffs (updated)
-

  src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION 
  src/python/cli_new/lib/cli/tests/task.py 
b54ade557f579a489e459f6022807146e0211fb0 


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

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


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-6551) ? 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_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 5 tests in 8.754s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69119: Added new CLI constants 'TEST_DIRECTORY' and 'TEST_DATA_DIRECTORY'.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Okt. 22, 2018, 2:12 nachm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69119/
> ---
> 
> (Updated Okt. 22, 2018, 2:12 nachm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new CLI constants 'TEST_DIRECTORY' and 'TEST_DATA_DIRECTORY'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/constants.py 
> 1398701b167eae397730afc5b1fab6a21e723266 
> 
> 
> Diff: https://reviews.apache.org/r/69119/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 69119: Added new CLI constants 'TEST_DIRECTORY' and 'TEST_DATA_DIRECTORY'.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

Added new CLI constants 'TEST_DIRECTORY' and 'TEST_DATA_DIRECTORY'.


Diffs
-

  src/python/cli_new/lib/cli/tests/constants.py 
1398701b167eae397730afc5b1fab6a21e723266 


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


Testing
---


Thanks,

Armand Grillet

Re: Review Request 69049: Added test for 'task exec'.

[Kevin Klues]

> On Okt. 22, 2018, 1:45 nachm., Kevin Klues wrote:
> > src/python/cli_new/lib/cli/tests/task.py
> > Lines 41 (patched)
> > 
> >
> > It seems strange to just assume we are in the correct directory when 
> > this is ran since this is just a relative path. Does this work if we run 
> > the tests from elsewhere?

We can probably just add a nother path element to the front of this with:
`os.path.dirname(os.path.realpath(__file__))`

What I don't understand yet though is how this is working until now.


- Kevin


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


On Okt. 22, 2018, 1:41 nachm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69049/
> ---
> 
> (Updated Okt. 22, 2018, 1:41 nachm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/task.py 
> b54ade557f579a489e459f6022807146e0211fb0 
> 
> 
> Diff: https://reviews.apache.org/r/69049/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 5 tests in 8.754s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69049: Added test for 'task exec'.

[Kevin Klues]

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




src/python/cli_new/lib/cli/tests/data/task/lorem-ipsum.txt
Lines 1 (patched)


Let's put this one level up (i.e. not in a nested `task` directory).



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


It seems strange to just assume we are in the correct directory when this 
is ran since this is just a relative path. Does this work if we run the tests 
from elsewhere?



src/python/cli_new/lib/cli/tests/task.py
Lines 58-64 (patched)


This seems really messy at first glance. Is there some way to clean this up 
so it's easier to read?

Maybe just a comment on what we are doing here will suffice.



src/python/cli_new/lib/cli/tests/task.py
Lines 74-75 (patched)


What is going on here? Why are we actually using a bare-raise here?



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


We shouldn't have to decode here anymore after moving the `exec_command` to 
use `universal_newline`.


- Kevin Klues


On Okt. 22, 2018, 1:41 nachm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69049/
> ---
> 
> (Updated Okt. 22, 2018, 1:41 nachm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/task.py 
> b54ade557f579a489e459f6022807146e0211fb0 
> 
> 
> Diff: https://reviews.apache.org/r/69049/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 5 tests in 8.754s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69049: Added test for 'task exec'.

[Armand Grillet]

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

(Updated Oct. 22, 2018, 3:41 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Moved test data.


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


Repository: mesos


Description
---

Added test for 'task exec'.


Diffs (updated)
-

  src/python/cli_new/lib/cli/tests/data/lorem-ipsum.txt PRE-CREATION 
  src/python/cli_new/lib/cli/tests/task.py 
b54ade557f579a489e459f6022807146e0211fb0 


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

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


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-6551) ? 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_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 5 tests in 8.754s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69116: Added 'popen_tty' to test util functions for the new CLI.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: patch failed: src/python/cli_new/lib/cli/tests/base.py:486
error: src/python/cli_new/lib/cli/tests/base.py: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 22, 2018, 12:23 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69116/
> ---
> 
> (Updated Oct. 22, 2018, 12:23 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9343
> https://issues.apache.org/jira/browse/MESOS-9343
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code was pulled directly from:
> https://github.com/dcos/dcos-core-cli/blob/
> 7fd55421939a7782c237e2b8719c0fe2f543acd7/
> python/lib/dcoscli/dcoscli/test/common.py
> 
> This function will be used by tests requiring a TTY. This will be the
> case for tests concerning the 'task attach' subcommand.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> 3fb471c1f49e930d908322055bb9a188f88ee602 
> 
> 
> Diff: https://reviews.apache.org/r/69116/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69075 was successfully built and tested.

Reviews applied: `['69075']`

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

- Mesos Reviewbot Windows


On Oct. 22, 2018, 7:22 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 7:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69048: Added tenacity to 'pip-requirements' for new CLI.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Okt. 22, 2018, 10:31 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69048/
> ---
> 
> (Updated Okt. 22, 2018, 10:31 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This requirement will be used in upcoming new CLI integration tests.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/pip-requirements.txt 
> dfbc28fb041b5cdf6bf9440fab0730eab0a4c162 
> 
> 
> Diff: https://reviews.apache.org/r/69048/diff/1/
> 
> 
> Testing
> ---
> 
> Tested later in the chain.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69115: Added test for interactive 'task exec'.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69115 was successfully built and tested.

Reviews applied: `['69114', '69048', '69049', '69115']`

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

- Mesos Reviewbot Windows


On Oct. 22, 2018, 11:59 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69115/
> ---
> 
> (Updated Oct. 22, 2018, 11:59 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9342
> https://issues.apache.org/jira/browse/MESOS-9342
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for interactive 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/task.py 
> b54ade557f579a489e459f6022807146e0211fb0 
> 
> 
> Diff: https://reviews.apache.org/r/69115/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_interactive (cli.tests.task.TestTaskPlugin) ... ok
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 6 tests in 12.602s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69114: Added 'exec_command' to test util functions for the new CLI.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Okt. 22, 2018, 1:26 nachm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69114/
> ---
> 
> (Updated Okt. 22, 2018, 1:26 nachm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code was mostly pulled directly from:
> https://github.com/dcos/dcos-core-cli/blob/
> 7fd55421939a7782c237e2b8719c0fe2f543acd7/
> python/lib/dcoscli/dcoscli/test/common.py
> 
> This function will be used by tests that do not return a specific output
> but an error code, stdout, and stderr. This will be the case for tests
> concerning the 'task exec' and 'task attach' subcommands.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> 3fb471c1f49e930d908322055bb9a188f88ee602 
> 
> 
> Diff: https://reviews.apache.org/r/69114/diff/2/
> 
> 
> Testing
> ---
> 
> Tested later in the chain.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69114: Added 'exec_command' to test util functions for the new CLI.

[Armand Grillet]

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

(Updated Oct. 22, 2018, 3:26 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description (updated)
---

This code was mostly pulled directly from:
https://github.com/dcos/dcos-core-cli/blob/
7fd55421939a7782c237e2b8719c0fe2f543acd7/
python/lib/dcoscli/dcoscli/test/common.py

This function will be used by tests that do not return a specific output
but an error code, stdout, and stderr. This will be the case for tests
concerning the 'task exec' and 'task attach' subcommands.


Diffs (updated)
-

  src/python/cli_new/lib/cli/tests/base.py 
3fb471c1f49e930d908322055bb9a188f88ee602 


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

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


Testing
---

Tested later in the chain.


Thanks,

Armand Grillet

Re: Review Request 69114: Added 'exec_command' to test util functions for the new CLI.

[Kevin Klues]

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




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


We shouldn't do this. We should raise a CLIException like we do everywhere 
else.



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


Instead of doing this, we should use the argument to Popen with 
`universal_newlines=True`.


- Kevin Klues


On Okt. 22, 2018, 10:31 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69114/
> ---
> 
> (Updated Okt. 22, 2018, 10:31 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This code was pulled directly from:
> https://github.com/dcos/dcos-core-cli/blob/
> 7fd55421939a7782c237e2b8719c0fe2f543acd7/
> python/lib/dcoscli/dcoscli/test/common.py
> 
> This function will be used by tests that do not return a specific output
> but an error code, stdout, and stderr. This will be the case for tests
> concerning the 'task exec' and 'task attach' subcommands.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/base.py 
> 3fb471c1f49e930d908322055bb9a188f88ee602 
> 
> 
> Diff: https://reviews.apache.org/r/69114/diff/1/
> 
> 
> Testing
> ---
> 
> Tested later in the chain.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 69116: Added 'popen_tty' to test util functions for the new CLI.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This code was pulled directly from:
https://github.com/dcos/dcos-core-cli/blob/
7fd55421939a7782c237e2b8719c0fe2f543acd7/
python/lib/dcoscli/dcoscli/test/common.py

This function will be used by tests requiring a TTY. This will be the
case for tests concerning the 'task attach' subcommand.


Diffs
-

  src/python/cli_new/lib/cli/tests/base.py 
3fb471c1f49e930d908322055bb9a188f88ee602 


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


Testing
---


Thanks,

Armand Grillet

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Armand Grillet]

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


Ship it!




Ship It!

- Armand Grillet


On Oct. 22, 2018, 1:22 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69075/
> ---
> 
> (Updated Oct. 22, 2018, 1:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
> James Peach.
> 
> 
> Bugs: MESOS-8907
> https://issues.apache.org/jira/browse/MESOS-8907
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Modifies the 'curl' invocation that is returning an http::Response,
> locking it into HTTP 1.x. Our current HTTP parser is unable to process
> HTTP 2 responses.
> 
> With the advent of curl 7.47, HTTPS connections are being enforced
> towards HTTP 2 rather aggressively. As a result, our image fetcher
> fails when recent curl versions are being used for pulling images from
> a registry that supports HTTP 2.
> 
> HTTP 1.1 is chosen as long as the underlying curl supports the
> '--http1.1' flag. If curl does not support that flag, fall back to
> using HTTP 1.0.
> 
> For allowing all the benefits of HTTP 2 where possible, we do not
> adapt any 'curl' invocations that do not attempt to parse headers.
> 
> 
> Diffs
> -
> 
>   src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 
> 
> 
> Diff: https://reviews.apache.org/r/69075/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> `sudo ./bin/mesos-tests.sh 
> --gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
>  on a system with curl 7.59.0 installed.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69049: Added test for 'task exec'.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69049 was successfully built and tested.

Reviews applied: `['69114', '69048', '69049']`

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

- Mesos Reviewbot Windows


On Oct. 22, 2018, 10:32 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69049/
> ---
> 
> (Updated Oct. 22, 2018, 10:32 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-9341
> https://issues.apache.org/jira/browse/MESOS-9341
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for 'task exec'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/tests/data/task/lorem-ipsum.txt PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/task.py 
> b54ade557f579a489e459f6022807146e0211fb0 
> 
> 
> Diff: https://reviews.apache.org/r/69049/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> (mesos-cli) ?  cli_new (MESOS-6551) ? 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_list (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 5 tests in 8.754s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 69115: Added test for interactive 'task exec'.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

Added test for interactive 'task exec'.


Diffs
-

  src/python/cli_new/lib/cli/tests/task.py 
b54ade557f579a489e459f6022807146e0211fb0 


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


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-6551) ? 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_interactive (cli.tests.task.TestTaskPlugin) ... ok
test_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 6 tests in 12.602s

OK
```


Thanks,

Armand Grillet

Re: Review Request 69075: Updated docker image fetcher to enforce HTTP 1.x.

[Till Toenshoff via Review Board]

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

(Updated Oct. 22, 2018, 11:22 a.m.)


Review request for mesos, Alexander Rukletsov, Armand Grillet, Jie Yu, and 
James Peach.


Changes
---

Make HTTP 1.1 default but allow for fallback on HTTP 1.0 when curl does not 
support otherwise.


Summary (updated)
-

Updated docker image fetcher to enforce HTTP 1.x.


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


Repository: mesos


Description (updated)
---

Modifies the 'curl' invocation that is returning an http::Response,
locking it into HTTP 1.x. Our current HTTP parser is unable to process
HTTP 2 responses.

With the advent of curl 7.47, HTTPS connections are being enforced
towards HTTP 2 rather aggressively. As a result, our image fetcher
fails when recent curl versions are being used for pulling images from
a registry that supports HTTP 2.

HTTP 1.1 is chosen as long as the underlying curl supports the
'--http1.1' flag. If curl does not support that flag, fall back to
using HTTP 1.0.

For allowing all the benefits of HTTP 2 where possible, we do not
adapt any 'curl' invocations that do not attempt to parse headers.


Diffs (updated)
-

  src/uri/fetchers/docker.cpp 6b1277f822dcf80d44daa8133b3ecc8c9a34ef07 


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

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


Testing
---

`make check`
`sudo ./bin/mesos-tests.sh 
--gtest_filter="ImageAlpine/ProvisionerDockerTest.ROOT_INTERNET_CURL_SimpleCommand/2"`
 on a system with curl 7.59.0 installed.


Thanks,

Till Toenshoff

Review Request 69049: Added test for 'task exec'.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

Added test for 'task exec'.


Diffs
-

  src/python/cli_new/lib/cli/tests/data/task/lorem-ipsum.txt PRE-CREATION 
  src/python/cli_new/lib/cli/tests/task.py 
b54ade557f579a489e459f6022807146e0211fb0 


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


Testing
---

```
(mesos-cli) ?  cli_new (MESOS-6551) ? 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_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 5 tests in 8.754s

OK
```


Thanks,

Armand Grillet

Review Request 69048: Added tenacity to 'pip-requirements' for new CLI.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This requirement will be used in upcoming new CLI integration tests.


Diffs
-

  src/python/cli_new/pip-requirements.txt 
dfbc28fb041b5cdf6bf9440fab0730eab0a4c162 


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


Testing
---

Tested later in the chain.


Thanks,

Armand Grillet

Review Request 69114: Added 'exec_command' to test util functions for the new CLI.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

This code was pulled directly from:
https://github.com/dcos/dcos-core-cli/blob/
7fd55421939a7782c237e2b8719c0fe2f543acd7/
python/lib/dcoscli/dcoscli/test/common.py

This function will be used by tests that do not return a specific output
but an error code, stdout, and stderr. This will be the case for tests
concerning the 'task exec' and 'task attach' subcommands.


Diffs
-

  src/python/cli_new/lib/cli/tests/base.py 
3fb471c1f49e930d908322055bb9a188f88ee602 


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


Testing
---

Tested later in the chain.


Thanks,

Armand Grillet

Re: Review Request 69110: Added task health check definitions to master API responses.

[Alexander Rukletsov]

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


Fix it, then Ship it!




Let's make sure we also add a test for the contents of `TASK_ADDED` soon and 
add expose check definitions as well.


include/mesos/mesos.proto
Lines 2200-2201 (patched)


Let's add a `TODO` here for `CheckInfo`. Here and below.


- Alexander Rukletsov


On Oct. 22, 2018, 5:35 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69110/
> ---
> 
> (Updated Oct. 22, 2018, 5:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8780
> https://issues.apache.org/jira/browse/MESOS-8780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Task protobuf message is updated to include the health check
> definition of a task when it is specified. Associated helpers are
> also updated along with a test which verifies that this field is
> reflected in master API responses.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
>   src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
>   src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
>   src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 
> 
> 
> Diff: https://reviews.apache.org/r/69110/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69110 was successfully built and tested.

Reviews applied: `['69110']`

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

- Mesos Reviewbot Windows


On Oct. 22, 2018, 5:35 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69110/
> ---
> 
> (Updated Oct. 22, 2018, 5:35 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8780
> https://issues.apache.org/jira/browse/MESOS-8780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Task protobuf message is updated to include the health check
> definition of a task when it is specified. Associated helpers are
> also updated along with a test which verifies that this field is
> reflected in master API responses.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
>   src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
>   src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
>   src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 
> 
> 
> Diff: https://reviews.apache.org/r/69110/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>