Re: Review Request 71516: Refactored framework role tracking logic in the allocator.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71515, 71516]

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

- Mesos Reviewbot


On Sept. 19, 2019, 3:41 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71516/
> ---
> 
> (Updated Sept. 19, 2019, 3:41 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A framework is tracked under the role if the framework:
> (1) is subscribed to the role;
> Or
> (2) has resources allocated under the role.
> 
> (2) could be true without (1). This is because we allow
> frameworks to update their roles without revoking their
> allocated or offered resources.
> 
> This patch recognizes this fact and encapsulates the related
> checks in the helper.
> 
> Benchmark result: no major performance impact.
> 
> Master
> 
> BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Added 3000 agents in 90.816373ms
> Added 3000 frameworks in 17.202199348secs
> Made 3500 allocations in 15.46714944secs
> Made 0 allocation in 13.767830924secs
> 
> Master + this patch:
> 
> BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Added 3000 agents in 86.782773ms
> Added 3000 frameworks in 18.174924815secs
> Made 3500 allocations in 15.813011454secs
> Made 0 allocation in 12.882917161secs
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 5ea37912dadded47eaa2d354889c95533b191c59 
>   src/master/allocator/mesos/hierarchical.cpp 
> 256fdce61ccfb768605cac1579c9a6632cd26fec 
> 
> 
> Diff: https://reviews.apache.org/r/71516/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71441: Fixed URI stringification.

[James Peach]

> On Sept. 16, 2019, 5:24 a.m., Qian Zhang wrote:
> > In the description of this patch, I see you mentioned we only have this 
> > issue with a non-default Docker registry, but that seems not what I found. 
> > I started Mesos agent without setting the flag `--docker_registry` (so it 
> > just took the default value: `https://registry-1.docker.io`) and then 
> > launched a container with an image named `zhq527725/whiteout:latest`, and 
> > then I see:
> > > Pulling image 'zhq527725/whiteout:latest' from 
> > > 'docker-manifest://registry-1.docker.io:443zhq527725/whiteout?latest#https'
> > >  to '/opt/qzhang/mesos/store/docker/staging/pKccqL'
> > 
> > As you see, the separator between the hostname and the path was missed as 
> > well, and I found the container can be launched successfully. So what is 
> > actually the issue? Just the malformed registry URLs in the above message? 
> > If I read MESOS-9963 correctly, the issue should be launching container 
> > fails, right?
> 
> James Peach wrote:
> In my testing, the malformed URL is used to pull the images, and that 
> fails. Is there a code path I'm missing?
> 
> Qian Zhang wrote:
> Can you please let me know the detailed error message in your testing?
> 
> IIUC, the issue (missing separator) is in the method `ostream& 
> operator<<(ostream& stream, const URI& uri)`, and that method is used to 
> print the message `Pulling image ...` but not used when actually fetching 
> image, right?
> 
> James Peach wrote:
> The curl fetcher uses it via 
> [stringify](https://github.com/apache/mesos/blob/master/src/uri/fetchers/curl.cpp#L129).
> 
> Qian Zhang wrote:
> I think you are talking about [this 
> code](https://github.com/apache/mesos/blob/1.9.0/src/uri/fetchers/docker.cpp#L235),
>  right? However the URI used in that code is not the malformed URI that you 
> mentioned in this patch, it is actually re-constructed from the malformed URI 
> (see `DockerFetcherPluginProcess::getManifestUri` and 
> `DockerFetcherPluginProcess::getBlobUri`), so even we see the malformed URL 
> in the message like this 
> `"docker-manifest://registry-1.docker.io:443zhq527725/whiteout?latest#https"`,
>  the URI used by curl is still correct, like: 
> `"https://registry-1.docker.io:443/v2/zhq527725/whiteout/manifests/latest"`.
> 
> James Peach wrote:
> Ah I see. The Docker puller makes sure to place a leading '/' in the 
> path. I asked the relevant team to re-test and they confirmed that this is 
> purely a cosmetic issue.
> 
> I still think that this change is correct, but I'll update the commit 
> message :)
> 
> Qian Zhang wrote:
> Thanks, please also update the description of 
> https://issues.apache.org/jira/browse/MESOS-9963 .

Done!


- James


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


On Sept. 19, 2019, 5:11 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71441/
> ---
> 
> (Updated Sept. 19, 2019, 5:11 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-9963
> https://issues.apache.org/jira/browse/MESOS-9963
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the caller is not careful to ensure that the path component begins
> with '/', stringification of URI objects will concatenate the host and
> path, resulting in an malformed URI. The fix is to ensure that these
> fuelds are always separated by '/'.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 4be0716895132e0406299f7d77f8ceb2d95dbe8a 
>   src/tests/uri_tests.cpp 1d33ab110bed7c0b7ecfa5d608965da5a1729562 
>   src/uri/utils.cpp 3940dc041c1783eec9e7c950402fd7c42e620d8e 
> 
> 
> Diff: https://reviews.apache.org/r/71441/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 30)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 71441: Fixed URI stringification.

[James Peach]

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

(Updated Sept. 19, 2019, 5:11 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Updated commit message.


Summary (updated)
-

Fixed URI stringification.


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


Repository: mesos


Description (updated)
---

If the caller is not careful to ensure that the path component begins
with '/', stringification of URI objects will concatenate the host and
path, resulting in an malformed URI. The fix is to ensure that these
fuelds are always separated by '/'.


Diffs (updated)
-

  src/tests/containerizer/docker_spec_tests.cpp 
4be0716895132e0406299f7d77f8ceb2d95dbe8a 
  src/tests/uri_tests.cpp 1d33ab110bed7c0b7ecfa5d608965da5a1729562 
  src/uri/utils.cpp 3940dc041c1783eec9e7c950402fd7c42e620d8e 


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

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


Testing
---

make check (Fedora 30)


Thanks,

James Peach

Re: Review Request 71516: Refactored framework role tracking logic in the allocator.

[Meng Zhu]

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

(Updated Sept. 18, 2019, 8:41 p.m.)


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Changes
---

Added benchmark result in the description.


Repository: mesos


Description (updated)
---

A framework is tracked under the role if the framework:
(1) is subscribed to the role;
Or
(2) has resources allocated under the role.

(2) could be true without (1). This is because we allow
frameworks to update their roles without revoking their
allocated or offered resources.

This patch recognizes this fact and encapsulates the related
checks in the helper.

Benchmark result: no major performance impact.

Master

BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Added 3000 agents in 90.816373ms
Added 3000 frameworks in 17.202199348secs
Made 3500 allocations in 15.46714944secs
Made 0 allocation in 13.767830924secs

Master + this patch:

BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Added 3000 agents in 86.782773ms
Added 3000 frameworks in 18.174924815secs
Made 3500 allocations in 15.813011454secs
Made 0 allocation in 12.882917161secs


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
5ea37912dadded47eaa2d354889c95533b191c59 
  src/master/allocator/mesos/hierarchical.cpp 
256fdce61ccfb768605cac1579c9a6632cd26fec 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 71516: Refactored framework role tracking logic in the allocator.

[Meng Zhu]

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
---

A framework is tracked under the role if the framework:
(1) is subscribed to the role;
Or
(2) has resources allocated under the role.

(2) could be true without (1). This is because we allow
frameworks to update their roles without revoking their
allocated or offered resources.

This patch recognize this fact and encapsulates the related
checks in the helper.


Diffs
-

  src/master/allocator/mesos/hierarchical.hpp 
5ea37912dadded47eaa2d354889c95533b191c59 
  src/master/allocator/mesos/hierarchical.cpp 
256fdce61ccfb768605cac1579c9a6632cd26fec 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 71515: Fixed a bug where sorter may leak clients.

[Meng Zhu]

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


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


Repository: mesos


Description
---

It is possible to update allocations to empty in the sorter.
See MESOS-9015. When this happens, sorter will leak client
entries, since it does not erase the corresponding entries.

This patch fixes this issue. Also added a regression test.


Diffs
-

  src/master/allocator/mesos/sorter/drf/sorter.hpp 
2a861e2ea5fb61d20e50efbf3d94ee9757bca592 
  src/master/allocator/mesos/sorter/random/sorter.hpp 
bc5580959046492cbf6c38fe2cd7a7d9e11c99f4 
  src/tests/sorter_tests.cpp 64627c4ea57af19eb00bdff598936e23b55541f5 


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


Testing
---

make check
Added test fail on master, pass with the patch


Thanks,

Meng Zhu

Re: Review Request 71512: Windows: Libprocess: Fixed parallel test execution.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71510, 71511, 71512]

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

- Mesos Reviewbot


On Sept. 18, 2019, 11 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71512/
> ---
> 
> (Updated Sept. 18, 2019, 11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For the test runner script to find the executable now, it needs
> to be given the configuration and the file extension.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> b4ec9907d16c89b45562f4fa33c9f3d2913f6991 
> 
> 
> Diff: https://reviews.apache.org/r/71512/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build . --target check (Windows)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71488: Track per-role allocated resources in the roles tree.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [71486, 71487, 71488]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose 
--disable-libtool-wrappers --disable-parallel-test-execution' 
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh 2>&1 | tee 
build_71488"]

Error:
..
ll_timeout="1mins" --frameworks_home="/tmp/rNv7BZ/ecy48u/frameworks" 
--gc_delay="1weeks" --gc_disk_headroom="0.1" 
--gc_non_executor_container_sandboxes="false" --help="false" 
--hostname_lookup="true" --http_command_executor="false" 
--http_credentials="/tmp/rNv7BZ/ecy48u/http_credentials" 
--http_heartbeat_interval="30secs" --initialize_driver_logging="true" 
--isolation="posix/cpu,posix/mem" --launcher="posix" 
--launcher_dir="/mesos/mesos-1.10.0/_build/src" --logbufsecs="0" 
--logging_level="INFO" --max_completed_executors_per_framework="150" 
--memory_profiling="false" --network_cni_metrics="true" 
--network_cni_root_dir_persist="false" 
--oversubscribed_resources_interval="15secs" --perf_duration="10secs" 
--perf_interval="1mins" --port="5051" --qos_correction_interval_min="0ns" 
--quiet="false" --reconfiguration_policy="equal" --recover="reconnect" 
--recovery_timeout="15mins" --registration_backoff_factor="10ms" 
--resources="cpus:2;gpus:0;mem:1024;disk:1024;ports:[31000-32000]" --revocabl
 e_cpu_low_priority="true" 
--runtime_dir="/tmp/ContentType_OperationReconciliationTest_UnknownOperationAgentMarkedGone_0_xlS3XZ"
 --sandbox_directory="/mnt/mesos/sandbox" --strict="true" --switch_user="true" 
--systemd_enable_support="true" 
--systemd_runtime_directory="/run/systemd/system" --version="false" 
--work_dir="/tmp/ContentType_OperationReconciliationTest_UnknownOperationAgentMarkedGone_0_38i4Mu"
 --zk_session_timeout="10secs"
I0918 23:35:19.574690 18657 credentials.hpp:86] Loading credential for 
authentication from '/tmp/rNv7BZ/ecy48u/credential'
I0918 23:35:19.575434 18657 slave.cpp:300] Agent using credential for: 
test-principal
I0918 23:35:19.575819 18657 credentials.hpp:37] Loading credentials for 
authentication from '/tmp/rNv7BZ/ecy48u/http_credentials'
I0918 23:35:19.576562 18657 http.cpp:975] Creating default 'basic' HTTP 
authenticator for realm 'mesos-agent-readonly'
I0918 23:35:19.577693 18657 disk_profile_adaptor.cpp:78] Creating default disk 
profile adaptor module
I0918 23:35:19.580186 18657 slave.cpp:615] Agent resources: 
[{"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
I0918 23:35:19.581109 18657 slave.cpp:623] Agent attributes: [  ]
I0918 23:35:19.581487 18657 slave.cpp:632] Agent hostname: 905683fb3805
I0918 23:35:19.583003 18658 task_status_update_manager.cpp:181] Pausing sending 
task status updates
I0918 23:35:19.583117 18658 status_update_manager_process.hpp:379] Pausing 
operation status update manager
I0918 23:35:19.585777 18648 state.cpp:67] Recovering state from 
'/tmp/ContentType_OperationReconciliationTest_UnknownOperationAgentMarkedGone_0_38i4Mu/meta'
I0918 23:35:19.586390 18648 slave.cpp:7492] Finished recovering checkpointed 
state from 
'/tmp/ContentType_OperationReconciliationTest_UnknownOperationAgentMarkedGone_0_38i4Mu/meta',
 beginning agent recovery
I0918 23:35:19.587337 18648 task_status_update_manager.cpp:207] Recovering task 
status update manager
I0918 23:35:19.588311 18648 containerizer.cpp:821] Recovering Mesos containers
I0918 23:35:19.589103 18648 containerizer.cpp:1161] Recovering isolators
I0918 23:35:19.590978 18660 containerizer.cpp:1200] Recovering provisioner
I0918 23:35:19.592371 18660 provisioner.cpp:500] Provisioner recovery complete
I0918 23:35:19.594522 18649 composing.cpp:339] Finished recovering all 
containerizers
I0918 23:35:19.594980 18665 slave.cpp:7973] Recovering executors
I0918 23:35:19.595139 18665 slave.cpp:8126] Finished recovery
I0918 23:35:19.596966 18662 task_status_update_manager.cpp:181] Pausing sending 
task status updates
I0918 23:35:19.597014 18656 slave.cpp:1351] New master detected at 
master@172.17.0.2:34081
I0918 23:35:19.598083 18656 slave.cpp:1416] Detecting new master
I0918 23:35:19.597108 18663 status_update_manager_process.hpp:379] Pausing 
operation status update manager
I0918 23:35:19.599468 18661 slave.cpp:1443] Authenticating with master 
master@172.17.0.2:34081
I0918 23:35:19.599647 18661 slave.cpp:1452] Using default CRAM-MD5 authenticatee
I0918 23:35:19.600613 18661 authenticatee.cpp:121] Creating new client SASL 
connection
I0918 23:35:19.601212 18661 master.cpp:10590] Authenticating 
slave(1047)@172.17.0.2:34081
I0918 23:3

Re: Review Request 71478: Windows: Moved definition out of inline function call.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Sept. 12, 2019, 7:35 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71478/
> ---
> 
> (Updated Sept. 12, 2019, 7:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MSVC does not deal with #ifdefs from inside function calls.
> So here, the `#if defined(...)` was taken literally and is
> considered a syntax error by MSVC.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
> 
> 
> Diff: https://reviews.apache.org/r/71478/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71509: Windows: Fixed AllocationRoleEnvironmentVariable tests.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 18, 2019, 3:57 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71509/
> ---
> 
> (Updated Sept. 18, 2019, 3:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Qian Zhang, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9874
> https://issues.apache.org/jira/browse/MESOS-9874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests were using a shell script, which can easily be
> converted to a batch script on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> e9726b0b6c4623d5196186cd43722952f77669da 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 5e31a95149bdc0de64eeab3c289f7816077c82c4 
>   src/tests/default_executor_tests.cpp 
> e5c0bf2df4b35fb3920104e1b72bea7797724933 
> 
> 
> Diff: https://reviews.apache.org/r/71509/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build . --target check (Windows)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 71512: Windows: Libprocess: Fixed parallel test execution.

[Joseph Wu]

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

Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.


Repository: mesos


Description
---

For the test runner script to find the executable now, it needs
to be given the configuration and the file extension.


Diffs
-

  3rdparty/libprocess/src/tests/CMakeLists.txt 
b4ec9907d16c89b45562f4fa33c9f3d2913f6991 


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


Testing
---

cmake --build . --target check (Windows)


Thanks,

Joseph Wu

Re: Review Request 71509: Windows: Fixed AllocationRoleEnvironmentVariable tests.

[Till Toenshoff via Review Board]

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


Ship it!




Ship It!

- Till Toenshoff


On Sept. 18, 2019, 10:57 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71509/
> ---
> 
> (Updated Sept. 18, 2019, 10:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Qian Zhang, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9874
> https://issues.apache.org/jira/browse/MESOS-9874
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests were using a shell script, which can easily be
> converted to a batch script on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/command_executor_tests.cpp 
> e9726b0b6c4623d5196186cd43722952f77669da 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 5e31a95149bdc0de64eeab3c289f7816077c82c4 
>   src/tests/default_executor_tests.cpp 
> e5c0bf2df4b35fb3920104e1b72bea7797724933 
> 
> 
> Diff: https://reviews.apache.org/r/71509/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build . --target check (Windows)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 71511: Windows: Stout: Fixed parallel test execution.

[Joseph Wu]

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

Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.


Repository: mesos


Description
---

For the test runner script to find the executable now, it needs
to be given the configuration and the file extension.


Diffs
-

  3rdparty/stout/tests/CMakeLists.txt e3291c72d1875f5b196dd5ea2ba20e7d53881b87 


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


Testing
---

cmake --build . --target check (Windows)


Thanks,

Joseph Wu

Review Request 71510: Windows: Fixed parallel test execution.

[Joseph Wu]

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

Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.


Repository: mesos


Description
---

Since the default 'check' target now uses the parallel test runner,
there are some changes necessary to use this on Windows.

In the script itself, references to RLimits need to be removed,
since those structures do not exist on Windows.

The TEST_RUNNER variable must include "python" on Windows, since
'.py' files are not automatically run with Python on Windows.

The script also needs the full test executable name (+ '.exe').
We could omit this previously, because you can run executables
while omitting the extension.  But the script checks if the file
exists, and that operation requires the full name plus extension.


Diffs
-

  cmake/MesosConfigure.cmake 83d41addcd2c14358fba8bab2ac654475626a3e8 
  src/tests/CMakeLists.txt 1e53b396569bf3e2f47199956a630afb6197b992 
  support/mesos-gtest-runner.py 42c0a143477b5ccd411db482a8877e596f520342 


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


Testing
---

cmake --build . --target check (Windows)


Thanks,

Joseph Wu

Review Request 71509: Windows: Fixed AllocationRoleEnvironmentVariable tests.

[Joseph Wu]

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

Review request for mesos, Gilbert Song, Greg Mann, Qian Zhang, and Till 
Toenshoff.


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


Repository: mesos


Description
---

These tests were using a shell script, which can easily be
converted to a batch script on Windows.


Diffs
-

  src/tests/command_executor_tests.cpp e9726b0b6c4623d5196186cd43722952f77669da 
  src/tests/containerizer/docker_containerizer_tests.cpp 
5e31a95149bdc0de64eeab3c289f7816077c82c4 
  src/tests/default_executor_tests.cpp e5c0bf2df4b35fb3920104e1b72bea7797724933 


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


Testing
---

cmake --build . --target check (Windows)


Thanks,

Joseph Wu

Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71496, 71497]

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

- Mesos Reviewbot


On Sept. 18, 2019, 12:35 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71497/
> ---
> 
> (Updated Sept. 18, 2019, 12:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Bugs: MESOS-9972
> https://issues.apache.org/jira/browse/MESOS-9972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
> `LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.
> 
> The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
> `LIBPROCESS_SSL_VERIFY_SERVER_CERT`.
> 
> The new names better describe the actual effect of both flags, and
> make upgrades easier by allowing operators to only enable verification
> on agents that are new enough to contain the updated hostname
> validation code paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 1a0e3820cc8cd1459625f46a54b194133500f11e 
>   3rdparty/libprocess/src/openssl.hpp 
> 271cc95238d287c06df36478554502e8b7205b09 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 9d5ab679165a709f7c3740020961ec89a7db4f54 
>   docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
>   docs/upgrades.md e630731c332fdd7df788f96644a8084f30b5c621 
> 
> 
> Diff: https://reviews.apache.org/r/71497/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71507: Windows: Disabled the dist and distcheck targets with a note.

[Joseph Wu]

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

(Updated Sept. 18, 2019, 1:21 p.m.)


Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.


Changes
---

Comment tweak.


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


Repository: mesos


Description
---

The targets are implemented with shell scripts, so do not work
on Windows.  The targets are also currently unneeded for the Windows
build.


Diffs (updated)
-

  CMakeLists.txt a1ca5c28819b946b10a3533f4793896c676d4682 


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

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


Testing
---

cmake --build . --target dist


Thanks,

Joseph Wu

Re: Review Request 71507: Windows: Disabled the dist and distcheck targets with a note.

[Benjamin Bannier]

> On Sept. 18, 2019, 10:07 p.m., Benjamin Bannier wrote:
> > CMakeLists.txt
> > Lines 100 (patched)
> > 
> >
> > This is consistent with how we do it elsewhere, but I wonder: is the 
> > reason we cannot have these targets here due to shell scripts not being 
> > available on the Windows _platform_, or due to the default _generator_ on 
> > Windows not supporting them (i.e., would this e.g., work when building with 
> > Ninja on Windows). Maybe you could leave a comment with a hint why we 
> > disable them here.
> 
> Joseph Wu wrote:
> The default way to handle `add_custom_target(... COMMAND ...)` depends on 
> the platform.  On Windows, it will attempt to run the command with `cmd`, 
> which won't work.  So this doesn't have much to do with the chosen generator.
> 
> It might be possible for a different generator to try appending `bash` 
> instead of `cmd`.  But the existence of `bash` on Windows is not guaranteed.

Great, let's capture the gist of that in a comment in the code.


- Benjamin


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


On Sept. 18, 2019, 9:26 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71507/
> ---
> 
> (Updated Sept. 18, 2019, 9:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9971
> https://issues.apache.org/jira/browse/MESOS-9971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The targets are implemented with shell scripts, so do not work
> on Windows.  The targets are also currently unneeded for the Windows
> build.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a1ca5c28819b946b10a3533f4793896c676d4682 
> 
> 
> Diff: https://reviews.apache.org/r/71507/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build . --target dist
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71507: Windows: Disabled the dist and distcheck targets with a note.

[Joseph Wu]

> On Sept. 18, 2019, 1:07 p.m., Benjamin Bannier wrote:
> > CMakeLists.txt
> > Lines 100 (patched)
> > 
> >
> > This is consistent with how we do it elsewhere, but I wonder: is the 
> > reason we cannot have these targets here due to shell scripts not being 
> > available on the Windows _platform_, or due to the default _generator_ on 
> > Windows not supporting them (i.e., would this e.g., work when building with 
> > Ninja on Windows). Maybe you could leave a comment with a hint why we 
> > disable them here.

The default way to handle `add_custom_target(... COMMAND ...)` depends on the 
platform.  On Windows, it will attempt to run the command with `cmd`, which 
won't work.  So this doesn't have much to do with the chosen generator.

It might be possible for a different generator to try appending `bash` instead 
of `cmd`.  But the existence of `bash` on Windows is not guaranteed.


- Joseph


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


On Sept. 18, 2019, 12:26 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71507/
> ---
> 
> (Updated Sept. 18, 2019, 12:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9971
> https://issues.apache.org/jira/browse/MESOS-9971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The targets are implemented with shell scripts, so do not work
> on Windows.  The targets are also currently unneeded for the Windows
> build.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a1ca5c28819b946b10a3533f4793896c676d4682 
> 
> 
> Diff: https://reviews.apache.org/r/71507/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build . --target dist
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71507: Windows: Disabled the dist and distcheck targets with a note.

[Benjamin Bannier]

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


Fix it, then Ship it!




In addition to testing the `dist` target, could you also confirm that this 
patch fixes the default workflow (probably: `cmake --build . --target check`)?


CMakeLists.txt
Lines 99 (patched)


Let's move this before the actual targets.



CMakeLists.txt
Lines 100 (patched)


This is consistent with how we do it elsewhere, but I wonder: is the reason 
we cannot have these targets here due to shell scripts not being available on 
the Windows _platform_, or due to the default _generator_ on Windows not 
supporting them (i.e., would this e.g., work when building with Ninja on 
Windows). Maybe you could leave a comment with a hint why we disable them here.


- Benjamin Bannier


On Sept. 18, 2019, 9:26 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71507/
> ---
> 
> (Updated Sept. 18, 2019, 9:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9971
> https://issues.apache.org/jira/browse/MESOS-9971
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The targets are implemented with shell scripts, so do not work
> on Windows.  The targets are also currently unneeded for the Windows
> build.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a1ca5c28819b946b10a3533f4793896c676d4682 
> 
> 
> Diff: https://reviews.apache.org/r/71507/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build . --target dist
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 71507: Windows: Disable the dist and distcheck targets with a note.

[Joseph Wu]

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

Review request for mesos, Benjamin Bannier, Greg Mann, and Till Toenshoff.


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


Repository: mesos


Description
---

The targets are implemented with shell scripts, so do not work
on Windows.  The targets are also currently unneeded for the Windows
build.


Diffs
-

  CMakeLists.txt a1ca5c28819b946b10a3533f4793896c676d4682 


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


Testing
---

cmake --build . --target dist


Thanks,

Joseph Wu

Re: Review Request 71498: Introduced a role tree helper to modify a role and all its ancestors.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71489, 71486, 71487, 71488, 71490, 71491, 71498]

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

- Mesos Reviewbot


On Sept. 18, 2019, 12:59 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71498/
> ---
> 
> (Updated Sept. 18, 2019, 12:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9949
> https://issues.apache.org/jira/browse/MESOS-9949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a role tree helper to modify a role and all its ancestors.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 5ea37912dadded47eaa2d354889c95533b191c59 
>   src/master/allocator/mesos/hierarchical.cpp 
> 256fdce61ccfb768605cac1579c9a6632cd26fec 
> 
> 
> Diff: https://reviews.apache.org/r/71498/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71479: Added a test to ensure resources are recovered during agent removal.

[Andrei Sekretenko]

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


Ship it!




Ship It!

- Andrei Sekretenko


On Sept. 12, 2019, 10:51 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71479/
> ---
> 
> (Updated Sept. 12, 2019, 10:51 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-621
> https://issues.apache.org/jira/browse/MESOS-621
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure resources are recovered during agent removal.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f2e2b259af9920133dd22a5b03774d7035821dc 
> 
> 
> Diff: https://reviews.apache.org/r/71479/diff/1/
> 
> 
> Testing
> ---
> 
> Failed on master.
> Pass after r/71476
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71476: Simplified recover resources when removing frameworks or agents.

[Andrei Sekretenko]

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


Fix it, then Ship it!




Oh, that's a long-awaited fix!

Looks good to me, albeit there is a corner case around which I cannot wrap my 
head (see below).


src/master/allocator/mesos/hierarchical.cpp
Lines 1489-1494 (original), 1523-1528 (patched)


Previously, this code was executed even in case of recovering resources of 
already removed slave.

Now I don't see any framework untracking code in `removeSlave()` or 
`untrackAllocatedResources()`. 

Does this mean that now we are potentially leaking frameworks? Or not?


- Andrei Sekretenko


On Sept. 12, 2019, 10:42 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71476/
> ---
> 
> (Updated Sept. 12, 2019, 10:42 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-621
> https://issues.apache.org/jira/browse/MESOS-621
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Simplified recover resources when removing frameworks or agents.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 5ea37912dadded47eaa2d354889c95533b191c59 
>   src/master/allocator/mesos/hierarchical.cpp 
> 256fdce61ccfb768605cac1579c9a6632cd26fec 
>   src/master/master.cpp a2c289a8b6e30eac690f77c7fc1b12b16f62f541 
> 
> 
> Diff: https://reviews.apache.org/r/71476/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> New test in https://reviews.apache.org/r/71479/
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71490: Added quota consumption metrics to the hierarchial allocator.

[Andrei Sekretenko]

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

(Updated Sept. 18, 2019, 5:13 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Implemented specialized `Role::quotaConsumed()` method due to the chaenges in 
the previous patch.


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


Repository: mesos


Description
---

Added quota consumption metrics to the hierarchial allocator.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
5ea37912dadded47eaa2d354889c95533b191c59 
  src/master/allocator/mesos/hierarchical.cpp 
256fdce61ccfb768605cac1579c9a6632cd26fec 
  src/master/allocator/mesos/metrics.hpp 
adb9515c82029f32e57e73cddd441c1bceec1aee 
  src/master/allocator/mesos/metrics.cpp 
ba95dc4415e3f85522f5d1291b6e9c171b900482 


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

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


Testing
---


Thanks,

Andrei Sekretenko

Re: Review Request 71488: Track per-role allocated resources in the roles tree.

[Andrei Sekretenko]

> On Sept. 17, 2019, 10:12 p.m., Meng Zhu wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 134 (patched)
> > 
> >
> > why not resource quantities? what matter data do we need?

I've removed exposing allocated scalars altogether (in favour of 
`consumedQuota()` method in the next patch). 

However, I don't see a simple way to correctly account for shared resources 
without storing them, so `allocaredScalars_` has to be `Resources` anyway.


- Andrei


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


On Sept. 18, 2019, 5:07 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71488/
> ---
> 
> (Updated Sept. 18, 2019, 5:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9949
> https://issues.apache.org/jira/browse/MESOS-9949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Track per-role allocated resources in the roles tree.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 5ea37912dadded47eaa2d354889c95533b191c59 
>   src/master/allocator/mesos/hierarchical.cpp 
> 256fdce61ccfb768605cac1579c9a6632cd26fec 
> 
> 
> Diff: https://reviews.apache.org/r/71488/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71488: Track per-role allocated resources in the roles tree.

[Andrei Sekretenko]

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

(Updated Sept. 18, 2019, 5:07 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Removed exposing `allocatedScalars_` altogether. Fixed CHECK() message.


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


Repository: mesos


Description
---

Track per-role allocated resources in the roles tree.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
5ea37912dadded47eaa2d354889c95533b191c59 
  src/master/allocator/mesos/hierarchical.cpp 
256fdce61ccfb768605cac1579c9a6632cd26fec 


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

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


Testing
---


Thanks,

Andrei Sekretenko

Re: Review Request 71487: Added method to notify allocator that offered transitioned to allocated.

[Andrei Sekretenko]

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

(Updated Sept. 18, 2019, 5:06 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Renamed usedForLaunch into launchResources; removed stray newlines.


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


Repository: mesos


Description
---

This is a prerequisite for tracking quota consumption in allocator.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
2bab53ab5fb25931a724c20a039e1301983ba574 
  src/master/allocator/mesos/allocator.hpp 
6921581745bf876ee42bcfb62b59245f23fcbf47 
  src/master/allocator/mesos/hierarchical.hpp 
5ea37912dadded47eaa2d354889c95533b191c59 
  src/master/allocator/mesos/hierarchical.cpp 
256fdce61ccfb768605cac1579c9a6632cd26fec 
  src/master/master.cpp a2c289a8b6e30eac690f77c7fc1b12b16f62f541 
  src/tests/allocator.hpp 01e6d2ce3d0655ad408f605c7be84cd7c0f0d479 


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

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


Testing
---


Thanks,

Andrei Sekretenko

Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

[Benno Evers]

> On Sept. 17, 2019, 5:39 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 118 (patched)
> > 
> >
> > can you use the `alias` argument in `add()` for these?

Actually, after having implemented the change I'm considering reverting back to 
the previous version.

The `alias()` mechanism is strict about allowing users to specify only one 
version of the command name:

```
E0918 11:04:41.622146 62400 openssl.cpp:454] EXIT with status 1: Failed to load 
flags from environment variables prefixed by LIBPROCESS_SSL_ or SSL_ 
(deprecated): Flag 'verify_server_cert' is already loaded via name 'verify_cert'
```

This makes sense in general, but it can lead to problems when an operator 
manually wants to enable server certificate validation by setting 
`LIBPROCESS_SSL_VERIFY_CERT=true` and a newer agent leaks its own configuration 
`LIBPROCESS_SSL_VERIFY_SERVER_CERT=true` into the process environment, 
accidentally killing the task.


- Benno


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


On Sept. 18, 2019, 12:35 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71497/
> ---
> 
> (Updated Sept. 18, 2019, 12:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Bugs: MESOS-9972
> https://issues.apache.org/jira/browse/MESOS-9972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
> `LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.
> 
> The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
> `LIBPROCESS_SSL_VERIFY_SERVER_CERT`.
> 
> The new names better describe the actual effect of both flags, and
> make upgrades easier by allowing operators to only enable verification
> on agents that are new enough to contain the updated hostname
> validation code paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 1a0e3820cc8cd1459625f46a54b194133500f11e 
>   3rdparty/libprocess/src/openssl.hpp 
> 271cc95238d287c06df36478554502e8b7205b09 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 9d5ab679165a709f7c3740020961ec89a7db4f54 
>   docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
>   docs/upgrades.md e630731c332fdd7df788f96644a8084f30b5c621 
> 
> 
> Diff: https://reviews.apache.org/r/71497/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71179: WIP: Added a flag for running example framework with a list of roles.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On July 29, 2019, 4:44 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71179/
> ---
> 
> (Updated July 29, 2019, 4:44 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-8503
> https://issues.apache.org/jira/browse/MESOS-8503
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Added a flag for running example framework with a list of roles.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 621a5ca973640b810d429827dacf4ab72cfebbb8 
> 
> 
> Diff: https://reviews.apache.org/r/71179/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71501: Gc'ed nested container sandbox only if we have root container sandbox.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71501]

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

- Mesos Reviewbot


On Sept. 18, 2019, 10:49 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71501/
> ---
> 
> (Updated Sept. 18, 2019, 10:49 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-9966
> https://issues.apache.org/jira/browse/MESOS-9966
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Gc'ed nested container sandbox only if we have root container sandbox.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 16149a1428db687b131de08d14893a2dd684ce28 
> 
> 
> Diff: https://reviews.apache.org/r/71501/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71178: Implemented displaying roles of multi-role frameworks as a tree.

[Andrei Sekretenko]

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

(Updated Sept. 18, 2019, 1:40 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Added screenshot with two roles


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


Repository: mesos


Description
---

This patch makes the UI pages `frameworks` and `framework` display
roles of multi-role frameworks as collapsible tree (instead of a list).


Diffs
-

  src/webui/app/app.js f6f11384f35b260dcaff432c884dc063ea5e0f0e 
  src/webui/app/frameworks/framework.html 
82f6b279a9416d147fcfab094a326d67d0951dcc 
  src/webui/app/frameworks/frameworks.html 
d37c6137b638a27e5bd0f70f08733d81550b3ace 
  src/webui/app/frameworks/roles-tree-root.html PRE-CREATION 
  src/webui/app/frameworks/roles-tree.html PRE-CREATION 
  src/webui/app/frameworks/roles.html PRE-CREATION 
  src/webui/assets/css/mesos.css 0ff47cda36cc897f2e8f43804d38046f3e27e575 


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


Testing
---

Tested manually with conbination of frameworks with thousands of roles and 
frameworks with 1-2 roles.

Performance: re-rendering time of `frameworks` page with 50 frameworks with 
~4000 roles each is around 500 ms on my hardware. 
With all subtrees collapsed, more than half of this time is spent inside 
`intermediateRoleTree()` and `aggregateRoleTree()` (roughly equal amounts of 
time).
If/when the master starts to store the roles as a tree, it might make sense to 
convert this to getting roles in the tree form.


File Attachments (updated)


Frameworks table: collapsed tree, expanded tree and a framework with only two 
rolesnd a framework e
  
https://reviews.apache.org/media/uploaded/files/2019/08/23/ce65874e-56f2-4ddd-9135-f428515a193b__brief_roles.png
"Framework" page with a roles tree
  
https://reviews.apache.org/media/uploaded/files/2019/08/23/f99ddea3-afb6-44d2-9e1b-fb1f9fdb77e8__framework_page.png
Framework with two roles
  
https://reviews.apache.org/media/uploaded/files/2019/09/18/4bd31618-69d4-41e2-971e-e0573c2635ce__two_roles.png


Thanks,

Andrei Sekretenko

Re: Review Request 71178: Implemented displaying roles of multi-role frameworks as a tree.

[Andrei Sekretenko]

> On Sept. 14, 2019, 8:08 p.m., Meng Zhu wrote:
> > src/webui/app/app.js
> > Lines 276 (patched)
> > 
> >
> > s/g/'g'/

Thanks for noticing!


> On Sept. 14, 2019, 8:08 p.m., Meng Zhu wrote:
> > src/webui/app/frameworks/roles.html
> > Lines 1 (patched)
> > 
> >
> > Dose this mean, if the framework has less than three roles, we will 
> > just join them with space?
> > 
> > I would still prefer a list even if there are only two roles, will be 
> > clearer and consistent?

Exactly.

Changed this to two, so that a string is shown only if a framework has 
subscribed with only one role or no roles.
Not entirely sure about clarity, but more consistent indeed.


- Andrei


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


On Sept. 18, 2019, 1:23 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71178/
> ---
> 
> (Updated Sept. 18, 2019, 1:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-8503
> https://issues.apache.org/jira/browse/MESOS-8503
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the UI pages `frameworks` and `framework` display
> roles of multi-role frameworks as collapsible tree (instead of a list).
> 
> 
> Diffs
> -
> 
>   src/webui/app/app.js f6f11384f35b260dcaff432c884dc063ea5e0f0e 
>   src/webui/app/frameworks/framework.html 
> 82f6b279a9416d147fcfab094a326d67d0951dcc 
>   src/webui/app/frameworks/frameworks.html 
> d37c6137b638a27e5bd0f70f08733d81550b3ace 
>   src/webui/app/frameworks/roles-tree-root.html PRE-CREATION 
>   src/webui/app/frameworks/roles-tree.html PRE-CREATION 
>   src/webui/app/frameworks/roles.html PRE-CREATION 
>   src/webui/assets/css/mesos.css 0ff47cda36cc897f2e8f43804d38046f3e27e575 
> 
> 
> Diff: https://reviews.apache.org/r/71178/diff/4/
> 
> 
> Testing
> ---
> 
> Tested manually with conbination of frameworks with thousands of roles and 
> frameworks with 1-2 roles.
> 
> Performance: re-rendering time of `frameworks` page with 50 frameworks with 
> ~4000 roles each is around 500 ms on my hardware. 
> With all subtrees collapsed, more than half of this time is spent inside 
> `intermediateRoleTree()` and `aggregateRoleTree()` (roughly equal amounts of 
> time).
> If/when the master starts to store the roles as a tree, it might make sense 
> to convert this to getting roles in the tree form.
> 
> 
> File Attachments
> 
> 
> Frameworks table: collapsed tree, expanded tree and a framework with only two 
> rolesnd a framework e
>   
> https://reviews.apache.org/media/uploaded/files/2019/08/23/ce65874e-56f2-4ddd-9135-f428515a193b__brief_roles.png
> "Framework" page with a roles tree
>   
> https://reviews.apache.org/media/uploaded/files/2019/08/23/f99ddea3-afb6-44d2-9e1b-fb1f9fdb77e8__framework_page.png
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71178: Implemented displaying roles of multi-role frameworks as a tree.

[Andrei Sekretenko]

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

(Updated Sept. 18, 2019, 1:23 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


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


Repository: mesos


Description
---

This patch makes the UI pages `frameworks` and `framework` display
roles of multi-role frameworks as collapsible tree (instead of a list).


Diffs (updated)
-

  src/webui/app/app.js f6f11384f35b260dcaff432c884dc063ea5e0f0e 
  src/webui/app/frameworks/framework.html 
82f6b279a9416d147fcfab094a326d67d0951dcc 
  src/webui/app/frameworks/frameworks.html 
d37c6137b638a27e5bd0f70f08733d81550b3ace 
  src/webui/app/frameworks/roles-tree-root.html PRE-CREATION 
  src/webui/app/frameworks/roles-tree.html PRE-CREATION 
  src/webui/app/frameworks/roles.html PRE-CREATION 
  src/webui/assets/css/mesos.css 0ff47cda36cc897f2e8f43804d38046f3e27e575 


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

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


Testing
---

Tested manually with conbination of frameworks with thousands of roles and 
frameworks with 1-2 roles.

Performance: re-rendering time of `frameworks` page with 50 frameworks with 
~4000 roles each is around 500 ms on my hardware. 
With all subtrees collapsed, more than half of this time is spent inside 
`intermediateRoleTree()` and `aggregateRoleTree()` (roughly equal amounts of 
time).
If/when the master starts to store the roles as a tree, it might make sense to 
convert this to getting roles in the tree form.


File Attachments


Frameworks table: collapsed tree, expanded tree and a framework with only two 
rolesnd a framework e
  
https://reviews.apache.org/media/uploaded/files/2019/08/23/ce65874e-56f2-4ddd-9135-f428515a193b__brief_roles.png
"Framework" page with a roles tree
  
https://reviews.apache.org/media/uploaded/files/2019/08/23/f99ddea3-afb6-44d2-9e1b-fb1f9fdb77e8__framework_page.png


Thanks,

Andrei Sekretenko

Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

[Benno Evers]

> On Sept. 17, 2019, 5:39 p.m., Vinod Kone wrote:
> > docs/ssl.md
> > Lines 100 (patched)
> > 
> >
> > These deprecations need to be documented in CHANGELOG and upgrades.md.

I updated `upgrades.md`, but I think the `CHANGELOG` updates are usually done 
in a separate patch.


- Benno


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


On Sept. 18, 2019, 12:35 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71497/
> ---
> 
> (Updated Sept. 18, 2019, 12:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Bugs: MESOS-9972
> https://issues.apache.org/jira/browse/MESOS-9972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
> `LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.
> 
> The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
> `LIBPROCESS_SSL_VERIFY_SERVER_CERT`.
> 
> The new names better describe the actual effect of both flags, and
> make upgrades easier by allowing operators to only enable verification
> on agents that are new enough to contain the updated hostname
> validation code paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 1a0e3820cc8cd1459625f46a54b194133500f11e 
>   3rdparty/libprocess/src/openssl.hpp 
> 271cc95238d287c06df36478554502e8b7205b09 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 9d5ab679165a709f7c3740020961ec89a7db4f54 
>   docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
>   docs/upgrades.md e630731c332fdd7df788f96644a8084f30b5c621 
> 
> 
> Diff: https://reviews.apache.org/r/71497/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71498: Introduced a role tree helper to modify a role and all its ancestors.

[Andrei Sekretenko]

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

(Updated Sept. 18, 2019, 12:59 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Changes
---

Made the helper accept `Role*` instead of `std::string`.


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


Repository: mesos


Description
---

Introduced a role tree helper to modify a role and all its ancestors.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
5ea37912dadded47eaa2d354889c95533b191c59 
  src/master/allocator/mesos/hierarchical.cpp 
256fdce61ccfb768605cac1579c9a6632cd26fec 


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

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


Testing (updated)
---

make check


Thanks,

Andrei Sekretenko

Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

[Benno Evers]

> On Sept. 18, 2019, 9:50 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/src/openssl.cpp
> > Lines 545-547 (patched)
> > 
> >
> > We are stating that there would be a deprecation; does that mean at 
> > some point this flag won't work anymore? If so when?
> > 
> > Our experience with such changes shows that we will very likely never 
> > kill the old name to make sure we stay reliably compatible. The only 
> > realistic option appears to be Mesos 2.0 for a removal of the old flag. 
> > Let's create a blocker ticket for 2.0, add that as a comment referencing 
> > the JIRA and we are golden.

https://issues.apache.org/jira/browse/MESOS-9973


- Benno


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


On Sept. 18, 2019, 12:35 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71497/
> ---
> 
> (Updated Sept. 18, 2019, 12:35 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Bugs: MESOS-9972
> https://issues.apache.org/jira/browse/MESOS-9972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
> `LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.
> 
> The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
> `LIBPROCESS_SSL_VERIFY_SERVER_CERT`.
> 
> The new names better describe the actual effect of both flags, and
> make upgrades easier by allowing operators to only enable verification
> on agents that are new enough to contain the updated hostname
> validation code paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 1a0e3820cc8cd1459625f46a54b194133500f11e 
>   3rdparty/libprocess/src/openssl.hpp 
> 271cc95238d287c06df36478554502e8b7205b09 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 9d5ab679165a709f7c3740020961ec89a7db4f54 
>   docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
>   docs/upgrades.md e630731c332fdd7df788f96644a8084f30b5c621 
> 
> 
> Diff: https://reviews.apache.org/r/71497/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

[Benno Evers]

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

(Updated Sept. 18, 2019, 12:35 p.m.)


Review request for mesos, Greg Mann and Till Toenshoff.


Changes
---

Use built-in libprocess alias mechanism.


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


Repository: mesos


Description
---

The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
`LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.

The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
`LIBPROCESS_SSL_VERIFY_SERVER_CERT`.

The new names better describe the actual effect of both flags, and
make upgrades easier by allowing operators to only enable verification
on agents that are new enough to contain the updated hostname
validation code paths.


Diffs (updated)
-

  3rdparty/libprocess/include/process/ssl/flags.hpp 
1a0e3820cc8cd1459625f46a54b194133500f11e 
  3rdparty/libprocess/src/openssl.hpp 271cc95238d287c06df36478554502e8b7205b09 
  3rdparty/libprocess/src/openssl.cpp 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
9d5ab679165a709f7c3740020961ec89a7db4f54 
  docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
  docs/upgrades.md e630731c332fdd7df788f96644a8084f30b5c621 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 71503: Manually created clang-tidy config in mesos-tidy setup.

[Benno Evers]

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


Ship it!




Ship It!

- Benno Evers


On Sept. 18, 2019, 12:09 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71503/
> ---
> 
> (Updated Sept. 18, 2019, 12:09 p.m.)
> 
> 
> Review request for mesos and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Manually created clang-tidy config in mesos-tidy setup.
> 
> 
> Diffs
> -
> 
>   support/mesos-tidy/entrypoint.sh a8408678ec7dfda398a117788d5f5f955e304bc4 
> 
> 
> Diff: https://reviews.apache.org/r/71503/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `support/mesos-tidy.sh` with local docker image.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 71503: Manually created clang-tidy config in mesos-tidy setup.

[Benjamin Bannier]

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

Review request for mesos and Benno Evers.


Repository: mesos


Description
---

Manually created clang-tidy config in mesos-tidy setup.


Diffs
-

  support/mesos-tidy/entrypoint.sh a8408678ec7dfda398a117788d5f5f955e304bc4 


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


Testing
---

Ran `support/mesos-tidy.sh` with local docker image.


Thanks,

Benjamin Bannier

Review Request 71501: Gc'ed nested container sandbox only if we have root container sandbox.

[Qian Zhang]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Gc'ed nested container sandbox only if we have root container sandbox.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
16149a1428db687b131de08d14893a2dd684ce28 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 71497: Introduced new names for SSL-related libprocess flags.

[Till Toenshoff via Review Board]

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




3rdparty/libprocess/src/openssl.cpp
Lines 545-547 (patched)


We are stating that there would be a deprecation; does that mean at some 
point this flag won't work anymore? If so when?

Our experience with such changes shows that we will very likely never kill 
the old name to make sure we stay reliably compatible. The only realistic 
option appears to be Mesos 2.0 for a removal of the old flag. Let's create a 
blocker ticket for 2.0, add that as a comment referencing the JIRA and we are 
golden.



3rdparty/libprocess/src/openssl.cpp
Line 593 (original), 615 (patched)


s/compatility/compatibility/



3rdparty/libprocess/src/openssl.cpp
Line 594 (original), 616 (patched)


s/be//



docs/ssl.md
Lines 100 (patched)


s/below//



docs/ssl.md
Lines 117 (patched)


s/below//


- Till Toenshoff


On Sept. 17, 2019, 12:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71497/
> ---
> 
> (Updated Sept. 17, 2019, 12:50 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `LIBPROCESS_SSL_REQUIRE_CERT` flag was renamed to
> `LIBPROCESS_SSL_REQUIRE_CLIENT_CERT`.
> 
> The `LIBPROCESS_SSL_VERIFY_CERT` flag was renamed to
> `LIBPROCESS_SSL_VERIFY_SERVER_CERT`.
> 
> The new names better describe the actual effect of both flags, and
> make upgrades easier by allowing operators to only enable verification
> on agents that are new enough to contain the updated hostname
> validation code paths.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/ssl/flags.hpp 
> 1a0e3820cc8cd1459625f46a54b194133500f11e 
>   3rdparty/libprocess/src/openssl.hpp 
> 271cc95238d287c06df36478554502e8b7205b09 
>   3rdparty/libprocess/src/openssl.cpp 
> 5854711971c9ebc4d676edc43af5ab5cfd5ea4c6 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 9d5ab679165a709f7c3740020961ec89a7db4f54 
>   docs/ssl.md 90a2eb9800b7d8d9aa9d7b1060a6e5eb9e124b02 
> 
> 
> Diff: https://reviews.apache.org/r/71497/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>