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

2015-11-04 Thread Kapil Arya 


> On Nov. 3, 2015, 2:53 p.m., Kapil Arya wrote:
> > src/common/http.cpp, lines 165-189
> > 
> >
> > 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.

2015-11-03 Thread Kapil Arya 

---
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.

2015-11-03 Thread Kapil Arya 

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



src/common/http.cpp 


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.

2015-11-03 Thread Kapil Arya 

---
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)


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.

2015-10-23 Thread Connor Doyle 

---
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.

2015-10-23 Thread Connor Doyle 


> On Oct. 23, 2015, 12:06 a.m., Niklas Nielsen wrote:
> > include/mesos/mesos.proto, line 1389
> > 
> >
> > '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.

2015-10-23 Thread Connor Doyle 


> On Oct. 23, 2015, 12:06 a.m., Niklas Nielsen wrote:
> > include/mesos/v1/mesos.proto, line 1343
> > 
> >
> > 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.

2015-10-22 Thread Connor Doyle 

---
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.


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
---

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.

2015-10-22 Thread Connor Doyle 


> On Oct. 22, 2015, 8:13 p.m., Kapil Arya wrote:
> > include/mesos/mesos.proto, lines 1395-1409
> > 
> >
> > 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: Clarify NetworkInfo semantics for IP addresses and group policies.

2015-10-22 Thread Niklas Nielsen 

---
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)


Let's change this to 0.26.0



include/mesos/mesos.proto (line 1389)


'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)


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)


s/0.25.1/0.26.0/



include/mesos/v1/mesos.proto (line 1343)


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



include/mesos/v1/mesos.proto (line 1350)


Agent and Master?



src/common/http.cpp (lines 165 - 166)


Can we inline JSON::Protobuf() instead?



src/examples/test_hook_module.cpp (line 261)


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
> 
>