You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anindya Sinha <an...@apple.com> on 2015/11/19 01:58:42 UTC

Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

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

Review request for mesos, haosdent huang and Timothy Chen.


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


Repository: mesos


Description
-------

If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.

Command line arguments added:
advertise_ip: IP address advertised to reach mesos slave.
advertise_port: Port advertised to reach mesos slave (used with advertise_ip).


Diffs
-----

  docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
  src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 

Diff: https://reviews.apache.org/r/40459/diff/


Testing
-------

make check run successfully.


Thanks,

Anindya Sinha


Re: Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40459/#review107234
-----------------------------------------------------------

Ship it!


Ship It!

- Vinod Kone


On Nov. 19, 2015, 12:58 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40459/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Timothy Chen.
> 
> 
> Bugs: MESOS-3809
>     https://issues.apache.org/jira/browse/MESOS-3809
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.
> 
> Command line arguments added:
> advertise_ip: IP address advertised to reach mesos slave.
> advertise_port: Port advertised to reach mesos slave (used with advertise_ip).
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
>   src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 
> 
> Diff: https://reviews.apache.org/r/40459/diff/
> 
> 
> Testing
> -------
> 
> make check run successfully.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Re: Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40459/#review107158
-----------------------------------------------------------


Patch looks great!

Reviews applied: [40459]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 19, 2015, 12:58 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40459/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Timothy Chen.
> 
> 
> Bugs: MESOS-3809
>     https://issues.apache.org/jira/browse/MESOS-3809
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.
> 
> Command line arguments added:
> advertise_ip: IP address advertised to reach mesos slave.
> advertise_port: Port advertised to reach mesos slave (used with advertise_ip).
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
>   src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 
> 
> Diff: https://reviews.apache.org/r/40459/diff/
> 
> 
> Testing
> -------
> 
> make check run successfully.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Re: Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

Posted by Anindya Sinha <an...@apple.com>.

> On Nov. 19, 2015, 2:57 a.m., Guangya Liu wrote:
> > docs/configuration.md, lines 37-54
> > <https://reviews.apache.org/r/40459/diff/1/?file=1131592#file1131592line37>
> >
> >     I think that the master/main.cpp should also be updated for those flages.
> 
> Anindya Sinha wrote:
>     It is already there added as a part of MESOS-809. Refer https://github.com/apache/mesos/blob/master/src/master/main.cpp#L131 and https://github.com/apache/mesos/blob/master/src/master/main.cpp#L138.
> 
> Guangya Liu wrote:
>     I mean this part should also be updated to use the latest description as here you have updated "mesos master" to "mesos master/slave"

Sorry, I am not sure I understand the concern. Are you suggesting we should have the exact same text when we do "mesos-master --help" or "mesos-slave --help" or look in the documentation.

In docs/configuration.md, I moved description of advertise_ip and advertise_port to the section of "Master and Slave Options" from the original "Master options" to denote the flags correspond to both master as well as slave (and the text reflects that). And it uses "mesos master/slave" since this is a common command line arg.
In src/slave/main.cpp: It has the same language as in docs/configuration.md except that I mention "slave" instead of "master/slave".
In src/master/main.cpp: It has the same language as in docs/configuration.md except that I mention "master" instead of "master/slave".

So when we do "mesos-master --help", we see mesos master (not mesos master/slave).
And when we do "mesos-slave --help", we see mesos slave (not mesos master/slave).
But in documentation, we see mesos master/slave (since these 2 flags are common to both mesos master as well as mesos slave).


- Anindya


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


On Nov. 19, 2015, 12:58 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40459/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Timothy Chen.
> 
> 
> Bugs: MESOS-3809
>     https://issues.apache.org/jira/browse/MESOS-3809
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.
> 
> Command line arguments added:
> advertise_ip: IP address advertised to reach mesos slave.
> advertise_port: Port advertised to reach mesos slave (used with advertise_ip).
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
>   src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 
> 
> Diff: https://reviews.apache.org/r/40459/diff/
> 
> 
> Testing
> -------
> 
> make check run successfully.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Re: Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

Posted by Guangya Liu <gy...@gmail.com>.

> On 十一月 19, 2015, 2:57 a.m., Guangya Liu wrote:
> > docs/configuration.md, lines 37-54
> > <https://reviews.apache.org/r/40459/diff/1/?file=1131592#file1131592line37>
> >
> >     I think that the master/main.cpp should also be updated for those flages.
> 
> Anindya Sinha wrote:
>     It is already there added as a part of MESOS-809. Refer https://github.com/apache/mesos/blob/master/src/master/main.cpp#L131 and https://github.com/apache/mesos/blob/master/src/master/main.cpp#L138.
> 
> Guangya Liu wrote:
>     I mean this part should also be updated to use the latest description as here you have updated "mesos master" to "mesos master/slave"
> 
> Anindya Sinha wrote:
>     Sorry, I am not sure I understand the concern. Are you suggesting we should have the exact same text when we do "mesos-master --help" or "mesos-slave --help" or look in the documentation.
>     
>     In docs/configuration.md, I moved description of advertise_ip and advertise_port to the section of "Master and Slave Options" from the original "Master options" to denote the flags correspond to both master as well as slave (and the text reflects that). And it uses "mesos master/slave" since this is a common command line arg.
>     In src/slave/main.cpp: It has the same language as in docs/configuration.md except that I mention "slave" instead of "master/slave".
>     In src/master/main.cpp: It has the same language as in docs/configuration.md except that I mention "master" instead of "master/slave".
>     
>     So when we do "mesos-master --help", we see mesos master (not mesos master/slave).
>     And when we do "mesos-slave --help", we see mesos slave (not mesos master/slave).
>     But in documentation, we see mesos master/slave (since these 2 flags are common to both mesos master as well as mesos slave).

Got it, thanks!


- Guangya


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


On 十一月 19, 2015, 12:58 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40459/
> -----------------------------------------------------------
> 
> (Updated 十一月 19, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Timothy Chen.
> 
> 
> Bugs: MESOS-3809
>     https://issues.apache.org/jira/browse/MESOS-3809
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.
> 
> Command line arguments added:
> advertise_ip: IP address advertised to reach mesos slave.
> advertise_port: Port advertised to reach mesos slave (used with advertise_ip).
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
>   src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 
> 
> Diff: https://reviews.apache.org/r/40459/diff/
> 
> 
> Testing
> -------
> 
> make check run successfully.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Re: Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

Posted by Guangya Liu <gy...@gmail.com>.

> On 十一月 19, 2015, 2:57 a.m., Guangya Liu wrote:
> > docs/configuration.md, lines 37-54
> > <https://reviews.apache.org/r/40459/diff/1/?file=1131592#file1131592line37>
> >
> >     I think that the master/main.cpp should also be updated for those flages.
> 
> Anindya Sinha wrote:
>     It is already there added as a part of MESOS-809. Refer https://github.com/apache/mesos/blob/master/src/master/main.cpp#L131 and https://github.com/apache/mesos/blob/master/src/master/main.cpp#L138.

I mean this part should also be updated to use the latest description as here you have updated "mesos master" to "mesos master/slave"


- Guangya


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


On 十一月 19, 2015, 12:58 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40459/
> -----------------------------------------------------------
> 
> (Updated 十一月 19, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Timothy Chen.
> 
> 
> Bugs: MESOS-3809
>     https://issues.apache.org/jira/browse/MESOS-3809
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.
> 
> Command line arguments added:
> advertise_ip: IP address advertised to reach mesos slave.
> advertise_port: Port advertised to reach mesos slave (used with advertise_ip).
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
>   src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 
> 
> Diff: https://reviews.apache.org/r/40459/diff/
> 
> 
> Testing
> -------
> 
> make check run successfully.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Re: Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

Posted by Anindya Sinha <an...@apple.com>.

> On Nov. 19, 2015, 2:57 a.m., Guangya Liu wrote:
> > docs/configuration.md, lines 37-54
> > <https://reviews.apache.org/r/40459/diff/1/?file=1131592#file1131592line37>
> >
> >     I think that the master/main.cpp should also be updated for those flages.

It is already there added as a part of MESOS-809. Refer https://github.com/apache/mesos/blob/master/src/master/main.cpp#L131 and https://github.com/apache/mesos/blob/master/src/master/main.cpp#L138.


- Anindya


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


On Nov. 19, 2015, 12:58 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40459/
> -----------------------------------------------------------
> 
> (Updated Nov. 19, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Timothy Chen.
> 
> 
> Bugs: MESOS-3809
>     https://issues.apache.org/jira/browse/MESOS-3809
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.
> 
> Command line arguments added:
> advertise_ip: IP address advertised to reach mesos slave.
> advertise_port: Port advertised to reach mesos slave (used with advertise_ip).
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
>   src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 
> 
> Diff: https://reviews.apache.org/r/40459/diff/
> 
> 
> Testing
> -------
> 
> make check run successfully.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Re: Review Request 40459: Added 2 slave flags --advertise_ip and --advertise_port.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40459/#review107119
-----------------------------------------------------------



docs/configuration.md (lines 37 - 54)
<https://reviews.apache.org/r/40459/#comment166074>

    I think that the master/main.cpp should also be updated for those flages.


- Guangya Liu


On 十一月 19, 2015, 12:58 a.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40459/
> -----------------------------------------------------------
> 
> (Updated 十一月 19, 2015, 12:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Timothy Chen.
> 
> 
> Bugs: MESOS-3809
>     https://issues.apache.org/jira/browse/MESOS-3809
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> If set, these IP/Port shall be advertised by libprocess (although bind is not done on this IP/Port). If not set, libprocess advertises the IP/Port on which bind was done.
> 
> Command line arguments added:
> advertise_ip: IP address advertised to reach mesos slave.
> advertise_port: Port advertised to reach mesos slave (used with advertise_ip).
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 72847e5efe7008fdec8287cce100857f9e7c0fe0 
>   src/slave/main.cpp 746eec684a0c9119f85506022676d276f808b6ea 
> 
> Diff: https://reviews.apache.org/r/40459/diff/
> 
> 
> Testing
> -------
> 
> make check run successfully.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>