You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kapil Arya <ka...@mesosphere.io> on 2017/05/18 17:40:19 UTC
Review Request 59372: Disable port-mapping isolator by default.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Tim Hansen.
Repository: mesos
Description
-------
The corresponding flag (--enable-port-mapping-isolator) was accidentally
enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
(https://reviews.apache.org/r/59193/).
Diffs
-----
configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
Diff: https://reviews.apache.org/r/59372/diff/1/
Testing
-------
Tested the following configurations:
Configure flag Isolator status
`./configure` Disabled
`./configure --with-port-mapping-isolator` Enabled
`./configure --with-network-isolator` Enabled
Thanks,
Kapil Arya
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Tim Hansen via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175417
-----------------------------------------------------------
Ship it!
Ship It!
- Tim Hansen
On May 18, 2017, 5:56 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 5:56 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/2/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Kapil Arya <ka...@mesosphere.io>.
> On May 18, 2017, 2:36 p.m., Jie Yu wrote:
> > configure.ac
> > Lines 347 (patched)
> > <https://reviews.apache.org/r/59372/diff/3/?file=1724123#file1724123line347>
> >
> > Should we set enable_port_mapping_isolator=no here explicitly?
Doing that would override the setting from `--enable-port-mapping-isolator` as it would just set the flag to `no` always.
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175419
-----------------------------------------------------------
On May 18, 2017, 2:28 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 2:28 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/3/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175419
-----------------------------------------------------------
Fix it, then Ship it!
configure.ac
Lines 347 (patched)
<https://reviews.apache.org/r/59372/#comment248866>
Should we set enable_port_mapping_isolator=no here explicitly?
- Jie Yu
On May 18, 2017, 6:28 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 6:28 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/3/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175420
-----------------------------------------------------------
Ship it!
Confirmed that this works on macOS with default flags, and triggering of correct deprecation warning.
The handling of the deprecated flag is a pretty horrible mix of overly long lines, and commands using both autoconf macros and naked shell. AFAIK there are no standard tools to avoid any of that.
- Benjamin Bannier
On May 18, 2017, 8:28 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 8:28 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/3/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/
-----------------------------------------------------------
(Updated May 18, 2017, 2:28 p.m.)
Review request for mesos, Jie Yu and Tim Hansen.
Repository: mesos
Description
-------
The corresponding flag (--enable-port-mapping-isolator) was accidentally
enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
(https://reviews.apache.org/r/59193/).
Diffs (updated)
-----
configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
Diff: https://reviews.apache.org/r/59372/diff/3/
Changes: https://reviews.apache.org/r/59372/diff/2-3/
Testing
-------
Tested the following configurations:
Configure flag Isolator status
`./configure` Disabled
`./configure --with-port-mapping-isolator` Enabled
`./configure --with-network-isolator` Enabled
Thanks,
Kapil Arya
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175418
-----------------------------------------------------------
configure.ac
Line 345 (original), 345 (patched)
<https://reviews.apache.org/r/59372/#comment248865>
Not yours, but this should be "deprecated".
- Neil Conway
On May 18, 2017, 5:56 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 5:56 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/2/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/
-----------------------------------------------------------
(Updated May 18, 2017, 1:56 p.m.)
Review request for mesos, Jie Yu and Tim Hansen.
Changes
-------
Updated help message.
Repository: mesos
Description
-------
The corresponding flag (--enable-port-mapping-isolator) was accidentally
enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
(https://reviews.apache.org/r/59193/).
Diffs (updated)
-----
configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
Diff: https://reviews.apache.org/r/59372/diff/2/
Changes: https://reviews.apache.org/r/59372/diff/1-2/
Testing
-------
Tested the following configurations:
Configure flag Isolator status
`./configure` Disabled
`./configure --with-port-mapping-isolator` Enabled
`./configure --with-network-isolator` Enabled
Thanks,
Kapil Arya
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175413
-----------------------------------------------------------
configure.ac
Line 346 (original), 346 (patched)
<https://reviews.apache.org/r/59372/#comment248861>
No. The first action is when the flag is specified. So, if the flag is specified, we do want to set it to "yes" after emmitting the warning.
- Kapil Arya
On May 18, 2017, 1:40 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 1:40 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/1/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Avinash sridharan <av...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175411
-----------------------------------------------------------
configure.ac
Line 192 (original), 192 (patched)
<https://reviews.apache.org/r/59372/#comment248858>
not yours, but could change this to:
`enable port mapping network isolator`. Just to be more explicit.
- Avinash sridharan
On May 18, 2017, 5:40 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 5:40 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/1/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Kapil Arya <ka...@mesosphere.io>.
> On May 18, 2017, 1:50 p.m., Tim Hansen wrote:
> > configure.ac
> > Line 346 (original), 346 (patched)
> > <https://reviews.apache.org/r/59372/diff/1/?file=1724107#file1724107line346>
> >
> > This should also be enable_port_mapping_isolator=no
No. The first action is when the flag is specified. So, if the flag is specified, we do want to set it to "yes" after emmitting the warning.
- Kapil
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175412
-----------------------------------------------------------
On May 18, 2017, 1:56 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 1:56 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/2/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 59372: Disable port-mapping isolator by default.
Posted by Tim Hansen via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59372/#review175412
-----------------------------------------------------------
configure.ac
Line 346 (original), 346 (patched)
<https://reviews.apache.org/r/59372/#comment248860>
This should also be enable_port_mapping_isolator=no
- Tim Hansen
On May 18, 2017, 5:40 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59372/
> -----------------------------------------------------------
>
> (Updated May 18, 2017, 5:40 p.m.)
>
>
> Review request for mesos, Jie Yu and Tim Hansen.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The corresponding flag (--enable-port-mapping-isolator) was accidentally
> enabled by-default in 20dee4190838b5bc7eb9cb524af413e3fe3fe082
> (https://reviews.apache.org/r/59193/).
>
>
> Diffs
> -----
>
> configure.ac d5236700d2ab06232960f0bed43e8394e9a29aa4
>
>
> Diff: https://reviews.apache.org/r/59372/diff/1/
>
>
> Testing
> -------
>
> Tested the following configurations:
> Configure flag Isolator status
> `./configure` Disabled
> `./configure --with-port-mapping-isolator` Enabled
> `./configure --with-network-isolator` Enabled
>
>
> Thanks,
>
> Kapil Arya
>
>