You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@mesos.apache.org by Connor Doyle <co...@mesosphere.io> on 2015/10/22 02:35:58 UTC

Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

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

Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
-------

In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

1) It's unclear how IP addresses map to virtual interfaces inside the container.
2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.

NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.


Diffs
-----

  include/mesos/mesos.proto 4a16be1 

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


Testing
-------

make && make check


Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.

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


Bad patch!

Reviews applied: [39531]

Failed command: ./support/apply-review.sh -n -r 39531

Error:
 2015-10-22 17:20:13 URL:https://reviews.apache.org/r/39531/diff/raw/ [12288/12288] -> "39531.patch" [1]
error: patch failed: include/mesos/mesos.proto:1351
error: include/mesos/mesos.proto: patch does not apply
39531.patch:296: new blank line at EOF.
+
Failed to apply patch

- Mesos ReviewBot


On Oct. 22, 2015, 5:18 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
>   include/mesos/v1/mesos.proto eadbc9d 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp 99b843a 
>   src/tests/common/http_tests.cpp 8a01ffc 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.


> On Oct. 22, 2015, 8:13 p.m., Kapil Arya wrote:
> > src/common/http.cpp, lines 165-180
> > <https://reviews.apache.org/r/39531/diff/5/?file=1103491#file1103491line165>
> >
> >     Dumb question -- could we have used `JSON::Protobuf()` instead?

Hey Kapil, I'll try it out.


- Connor


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


On Oct. 22, 2015, 5:45 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 5:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/tests/common/http_tests.cpp c2e7704 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.


> On Oct. 22, 2015, 8:13 p.m., Kapil Arya wrote:
> > include/mesos/mesos.proto, lines 1395-1409
> > <https://reviews.apache.org/r/39531/diff/5/?file=1103488#file1103488line1395>
> >
> >     I should have noticed it earlier, but does it make sense to make IPAddress a top level message? Nik?

Happy to change it, waiting on a second opinion unless you feel strongly about it Kapil.


- Connor


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


On Oct. 22, 2015, 11:30 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 11:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.


> On Oct. 22, 2015, 8:13 p.m., Kapil Arya wrote:
> > src/common/http.cpp, lines 165-180
> > <https://reviews.apache.org/r/39531/diff/5/?file=1103491#file1103491line165>
> >
> >     Dumb question -- could we have used `JSON::Protobuf()` instead?
> 
> Connor Doyle wrote:
>     Hey Kapil, I'll try it out.

Good catch, updated to use JSON::Protobuf() instead.


- Connor


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


On Oct. 22, 2015, 11:22 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 11:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Kapil Arya <ka...@mesosphere.io>.

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



include/mesos/mesos.proto (lines 1395 - 1409)
<https://reviews.apache.org/r/39531/#comment161709>

    I should have noticed it earlier, but does it make sense to make IPAddress a top level message? Nik?



src/common/http.cpp (lines 165 - 180)
<https://reviews.apache.org/r/39531/#comment161708>

    Dumb question -- could we have used `JSON::Protobuf()` instead?


- Kapil Arya


On Oct. 22, 2015, 1:45 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 1:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/tests/common/http_tests.cpp c2e7704 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.


> On Oct. 23, 2015, 12:06 a.m., Niklas Nielsen wrote:
> > include/mesos/v1/mesos.proto, line 1343
> > <https://reviews.apache.org/r/39531/diff/6/?file=1103672#file1103672line1343>
> >
> >     As a non-native speaker; what's the consistency around capitalizing framework :)

I personally find all occurrences of capitalized Master, Framework, Agent, etc to be strange because they're not proper nouns (where does it end... should Isolator also be capitalized?)  In any case let's be internally consistent and capitalize Framework here.  Will fix.


- Connor


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


On Oct. 22, 2015, 11:30 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 11:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.


> On Oct. 23, 2015, 12:06 a.m., Niklas Nielsen wrote:
> > include/mesos/mesos.proto, line 1389
> > <https://reviews.apache.org/r/39531/diff/6/?file=1103671#file1103671line1389>
> >
> >     'the actual assigned IP adress' reads a bit odd. How about something like: "A user can request an automatically assigned IP (for example, through an IPAM service) or a specific IP"
> >     
> >     No biggie; if you don't like the suggestion, feel free to drop

Updated, hopefully it's more clear now.


- Connor


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


On Oct. 23, 2015, 7:21 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2015, 7:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Niklas Nielsen <ni...@qni.dk>.

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


Wow! Awesome documentation work! A few suggestions below.

Mainly: We haven't planned 0.25.1, so let's bump to 0.26.0. The description is specific to isolators doing the heavy lifting, whereas we could be doing some of the IP assignment in the master (eventually).

Other than that; looks good!


docs/networking-for-mesos-managed-containers.md (line 140)
<https://reviews.apache.org/r/39531/#comment161753>

    Let's change this to 0.26.0



include/mesos/mesos.proto (line 1389)
<https://reviews.apache.org/r/39531/#comment161754>

    'the actual assigned IP adress' reads a bit odd. How about something like: "A user can request an automatically assigned IP (for example, through an IPAM service) or a specific IP"
    
    No biggie; if you don't like the suggestion, feel free to drop



include/mesos/mesos.proto (lines 1392 - 1394)
<https://reviews.apache.org/r/39531/#comment161755>

    We could, theoretically, have this be done from other places than the isolator. Wonder if we can abstract this out a bit



include/mesos/mesos.proto (line 1425)
<https://reviews.apache.org/r/39531/#comment161758>

    s/0.25.1/0.26.0/



include/mesos/v1/mesos.proto (line 1343)
<https://reviews.apache.org/r/39531/#comment161756>

    As a non-native speaker; what's the consistency around capitalizing framework :)



include/mesos/v1/mesos.proto (line 1350)
<https://reviews.apache.org/r/39531/#comment161757>

    Agent and Master?



src/common/http.cpp (lines 165 - 166)
<https://reviews.apache.org/r/39531/#comment161759>

    Can we inline JSON::Protobuf() instead?



src/examples/test_hook_module.cpp (line 261)
<https://reviews.apache.org/r/39531/#comment161760>

    0.27.0?


- Niklas Nielsen


On Oct. 22, 2015, 4:30 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 4:30 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Kapil Arya <ka...@mesosphere.io>.

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

Ship it!


Ship It!

- Kapil Arya


On Oct. 23, 2015, 3:21 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2015, 3:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Kapil Arya <ka...@mesosphere.io>.


> On Nov. 3, 2015, 2:53 p.m., Kapil Arya wrote:
> > src/common/http.cpp, lines 165-189
> > <https://reviews.apache.org/r/39531/diff/7/?file=1105326#file1105326line165>
> >
> >     Actually, my previous comment wasn't quite clear. We can use `JSON::Protobuf` for NetworkInfo::IPAddress, but not for the entire NetworkInfo since the labels will show up poorly.

I fixed it and committed the patch.


- Kapil


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


On Oct. 23, 2015, 3:21 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2015, 3:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Kapil Arya <ka...@mesosphere.io>.

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



src/common/http.cpp 
<https://reviews.apache.org/r/39531/#comment163317>

    Actually, my previous comment wasn't quite clear. We can use `JSON::Protobuf` for NetworkInfo::IPAddress, but not for the entire NetworkInfo since the labels will show up poorly.


- Kapil Arya


On Oct. 23, 2015, 3:21 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2015, 3:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Kapil Arya <ka...@mesosphere.io>.

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



docs/networking-for-mesos-managed-containers.md (lines 276 - 278)
<https://reviews.apache.org/r/39531/#comment163319>

    We'll need to update this comment after https://reviews.apache.org/r/39866/ has been merged.


- Kapil Arya


On Oct. 23, 2015, 3:21 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2015, 3:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> 
> Diffs
> -----
> 
>   docs/networking-for-mesos-managed-containers.md 33568a8 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/docker/executor.cpp 1e49013 
>   src/examples/test_hook_module.cpp 43d6cb9 
>   src/slave/slave.cpp e9f2d1b 
>   src/tests/common/http_tests.cpp c2e7704 
>   src/tests/hook_tests.cpp 5a5d019 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

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

(Updated Oct. 23, 2015, 7:21 p.m.)

Review request for mesos, Benjamin Hindman and Kapil Arya.

Changes
-------

- Removed JSON model() implementation for NetworkInfo in favor of inlined usage of JSON::Protobuf().
- Clarified wording in the NetworkInfo banner comment.
- Updated Mesos versions in inline deprecation comments.

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

Repository: mesos

Description
-------

In Mesos 0.25.0, a new message called NetworkInfo was introduced. This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

1) It's unclear how IP addresses map to virtual interfaces inside the container.
2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.

Diffs (updated)
-----

docs/networking-for-mesos-managed-containers.md 33568a8
include/mesos/mesos.proto 9400434
include/mesos/v1/mesos.proto 8131778
src/common/http.hpp 0cc98a8
src/common/http.cpp f56d8a1
src/docker/executor.cpp 1e49013
src/examples/test_hook_module.cpp 43d6cb9
src/slave/slave.cpp e9f2d1b
src/tests/common/http_tests.cpp c2e7704
src/tests/hook_tests.cpp 5a5d019

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

Testing
-------

make && make check

Thanks,

Connor Doyle

Re: Review Request 39531: Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

(Updated Oct. 22, 2015, 11:30 p.m.)

Review request for mesos, Benjamin Hindman and Kapil Arya.

Summary (updated)
-----------------

Clarify NetworkInfo semantics for IP addresses and group policies.

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

Repository: mesos

Description
-------

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

Diffs
-----

docs/networking-for-mesos-managed-containers.md 33568a8
include/mesos/mesos.proto 9400434
include/mesos/v1/mesos.proto 8131778
src/common/http.cpp f56d8a1
src/docker/executor.cpp 1e49013
src/examples/test_hook_module.cpp 43d6cb9
src/slave/slave.cpp e9f2d1b
src/tests/common/http_tests.cpp c2e7704
src/tests/hook_tests.cpp 5a5d019

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

Testing
-------

make && make check

Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

(Updated Oct. 22, 2015, 11:24 p.m.)

Review request for mesos, Benjamin Hindman and Kapil Arya.

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

Repository: mesos

Description (updated)
-------

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

Diffs
-----

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

Testing
-------

make && make check

Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

(Updated Oct. 22, 2015, 11:22 p.m.)

Review request for mesos, Benjamin Hindman and Kapil Arya.

Changes
-------

- Fill in NetworkInfo->ip_addresses in Docker executor and slave.
- Updated hook module test for NetworkInfo changes.
- Simplified NetworkInfo JSON serialization.
- Updated documentation.

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

Repository: mesos

Description
-------

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.

Diffs (updated)
-----

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

Testing
-------

make && make check

Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

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

(Updated Oct. 22, 2015, 5:45 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
-------

In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

1) It's unclear how IP addresses map to virtual interfaces inside the container.
2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.

NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.


Diffs (updated)
-----

  include/mesos/mesos.proto 9400434 
  include/mesos/v1/mesos.proto 8131778 
  src/common/http.hpp 0cc98a8 
  src/common/http.cpp f56d8a1 
  src/tests/common/http_tests.cpp c2e7704 

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


Testing
-------

make && make check


Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.

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

Bad patch!

Reviews applied: [39531]

Failed command: ./support/apply-review.sh -n -r 39531

Error:
 2015-10-22 17:40:22 URL:https://reviews.apache.org/r/39531/diff/raw/ [12284/12284] -> "39531.patch" [1]
39531.patch:296: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Successfully applied: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

1) It's unclear how IP addresses map to virtual interfaces inside the container.
2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.

NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.

Review: https://reviews.apache.org/r/39531
src/tests/common/http_tests.cpp:236: new blank line at EOF.
Failed to commit patch

- Mesos ReviewBot

On Oct. 22, 2015, 5:24 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 5:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 9400434 
>   include/mesos/v1/mesos.proto 8131778 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp f56d8a1 
>   src/tests/common/http_tests.cpp c2e7704 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

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

(Updated Oct. 22, 2015, 5:24 p.m.)


Review request for mesos, Benjamin Hindman and Kapil Arya.


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


Repository: mesos


Description
-------

In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

1) It's unclear how IP addresses map to virtual interfaces inside the container.
2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.

NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.


Diffs (updated)
-----

  include/mesos/mesos.proto 9400434 
  include/mesos/v1/mesos.proto 8131778 
  src/common/http.hpp 0cc98a8 
  src/common/http.cpp f56d8a1 
  src/tests/common/http_tests.cpp c2e7704 

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


Testing
-------

make && make check


Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

(Updated Oct. 22, 2015, 5:18 p.m.)

Review request for mesos, Benjamin Hindman and Kapil Arya.

Changes
-------

Sync v1 API with mesos.proto updates for NetworkInfo.

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

Repository: mesos

Description
-------

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.

Diffs (updated)
-----

include/mesos/mesos.proto 4a16be1
include/mesos/v1/mesos.proto eadbc9d
src/common/http.hpp 0cc98a8
src/common/http.cpp 99b843a
src/tests/common/http_tests.cpp 8a01ffc

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

Testing
-------

make && make check

Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.

(Updated Oct. 22, 2015, 5:08 p.m.)

Review request for mesos, Benjamin Hindman and Kapil Arya.

Changes
-------

Updated JSON formatters, added JSON tests, addressed naming tweak for IPAddress message.

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

Repository: mesos

Description
-------

Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.

NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.

Diffs (updated)
-----

include/mesos/mesos.proto 4a16be1
src/common/http.hpp 0cc98a8
src/common/http.cpp 99b843a
src/tests/common/http_tests.cpp 8a01ffc

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

Testing
-------

make && make check

Thanks,

Connor Doyle

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.


> On Oct. 22, 2015, 12:45 a.m., Joseph Wu wrote:
> > Can you also sync this with the V1 API?  (If so, I'll remove that section from this: https://reviews.apache.org/r/39502 ).

Yes, I will update the v1 proto file as well.  Thanks for the reminder.


- Connor


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


On Oct. 22, 2015, 5:08 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 5:08 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp 99b843a 
>   src/tests/common/http_tests.cpp 8a01ffc 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Connor Doyle <co...@mesosphere.io>.


> On Oct. 22, 2015, 12:45 a.m., Joseph Wu wrote:
> > Can you also sync this with the V1 API?  (If so, I'll remove that section from this: https://reviews.apache.org/r/39502 ).
> 
> Connor Doyle wrote:
>     Yes, I will update the v1 proto file as well.  Thanks for the reminder.

Updated.


- Connor


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


On Oct. 22, 2015, 5:18 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 5:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
>   include/mesos/v1/mesos.proto eadbc9d 
>   src/common/http.hpp 0cc98a8 
>   src/common/http.cpp 99b843a 
>   src/tests/common/http_tests.cpp 8a01ffc 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Joseph Wu <jo...@mesosphere.io>.

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


Can you also sync this with the V1 API?  (If so, I'll remove that section from this: https://reviews.apache.org/r/39502 ).

- Joseph Wu


On Oct. 21, 2015, 5:35 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2015, 5:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.

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


Bad patch!

Reviews applied: [39531]

Failed command: ./support/apply-review.sh -n -r 39531

Error:
 2015-10-22 01:30:35 URL:https://reviews.apache.org/r/39531/diff/raw/ [4873/4873] -> "39531.patch" [1]
error: patch failed: include/mesos/mesos.proto:1351
error: include/mesos/mesos.proto: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Oct. 22, 2015, 12:35 a.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2015, 12:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>

Re: Review Request 39531: [WIP] Clarify NetworkInfo semantics for IP addresses and group policies.

Posted by Kapil Arya <ka...@mesosphere.io>.

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


Should we also mention it explicitly that each NetworkInfo message represents a request to create a new virtual network interface inside the network namespace for the Container?

As a separate note, we also need to update the proto files inside include/v1 :-).


include/mesos/mesos.proto (line 1382)
<https://reviews.apache.org/r/39531/#comment161549>

    Should we call it IPAddress instead?


- Kapil Arya


On Oct. 21, 2015, 8:35 p.m., Connor Doyle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39531/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2015, 8:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Kapil Arya.
> 
> 
> Bugs: MESOS-3788
>     https://issues.apache.org/jira/browse/MESOS-3788
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In Mesos 0.25.0, a new message called NetworkInfo was introduced.  This message allows framework authors to communicate with network isolation modules via a first-class message type to request IP addresses and network group isolation policies.
> 
> Unfortunately, the structure is somewhat confusing to both framework authors and module implementors.
> 
> 1) It's unclear how IP addresses map to virtual interfaces inside the container.
> 2) It's difficult for application developers to understand the final policy when multiple IP addresses can be assigned with differing isolation policies.
> 
> NOTE: the slave should also be updated in order to set the new IpAddress message in ContainerStatus in case network isolation modules are not installed.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 4a16be1 
> 
> Diff: https://reviews.apache.org/r/39531/diff/
> 
> 
> Testing
> -------
> 
> make && make check
> 
> 
> Thanks,
> 
> Connor Doyle
> 
>