Re: Review Request 39531: 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/ --- (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.
> 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.
> 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: [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/ --- (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.
> 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.
--- 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. Changes --- Sync v1 API with mesos.proto updates for NetworkInfo. 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 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.
--- 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.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39531/ --- (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) --- 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.
> 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.
--- 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: [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/ --- (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 --- 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) - 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.
> 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: Clarify NetworkInfo semantics for IP addresses and group policies.
> 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.
--- 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. 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 --- 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 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.
> 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 > >
Review Request 38943: Fixed misleading information in the Mesos framework writing guide.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38943/ --- Review request for mesos and Adam B. Repository: mesos Description --- The framework author's guide contained some misleading and incomplete information. Added description of the Mesos command executor. Fixed outdated information about how to use ExecutorInfo. Added a link to the Mesos fetcher user documentation. Diffs - docs/app-framework-development-guide.md c7aa08d Diff: https://reviews.apache.org/r/38943/diff/ Testing --- Visually verified the generated output in Chrome, using http://github.com/mesosphere/mesos-website-container. Thanks, Connor Doyle
Re: Review Request 38517: Make attributes.hpp public
> On Sept. 22, 2015, 12:40 a.m., Connor Doyle wrote: > > Ship It! > > Connor Doyle wrote: > Please re-run the post-reviews so the patch applies cleanly > (`src/slave/http.cpp` was concurrently modified). > > Felix Abecassis wrote: > Done. Please verify. Verified (patch applied, make, make check). - Connor --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38517/#review99883 --- On Sept. 22, 2015, 1:26 a.m., Felix Abecassis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38517/ > --- > > (Updated Sept. 22, 2015, 1:26 a.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bugs: MESOS-3366 > https://issues.apache.org/jira/browse/MESOS-3366 > > > Repository: mesos > > > Description > --- > > This is required in order to enable callback hooks that can modify attributes > of a slave during initialization. > > > Diffs > - > > include/mesos/attributes.hpp PRE-CREATION > src/Makefile.am e224060 > src/common/attributes.hpp 2a7efbd > src/common/attributes.cpp f713bb5 > src/common/http.hpp 058baa6 > src/common/http.cpp aaef10b > src/common/type_utils.cpp 22118b4 > src/master/http.cpp 3e44b06 > src/slave/http.cpp 12a4d39 > src/slave/slave.hpp 7a54fad > src/tests/attributes_tests.cpp ded6120 > src/tests/registrar_tests.cpp 5131b57 > > Diff: https://reviews.apache.org/r/38517/diff/ > > > Testing > --- > > > Thanks, > > Felix Abecassis > >
Re: Review Request 38517: Make attributes.hpp public
> On Sept. 22, 2015, 12:40 a.m., Connor Doyle wrote: > > Ship It! Please re-run the post-reviews so the patch applies cleanly (`src/slave/http.cpp` was concurrently modified). - Connor --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38517/#review99883 --- On Sept. 19, 2015, 12:59 a.m., Felix Abecassis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38517/ > --- > > (Updated Sept. 19, 2015, 12:59 a.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bugs: MESOS-3366 > https://issues.apache.org/jira/browse/MESOS-3366 > > > Repository: mesos > > > Description > --- > > This is required in order to enable callback hooks that can modify attributes > of a slave during initialization. > > > Diffs > - > > include/mesos/attributes.hpp PRE-CREATION > src/Makefile.am 834f69a > src/common/attributes.hpp 2a7efbd > src/common/attributes.cpp f713bb5 > src/common/http.hpp 058baa6 > src/common/http.cpp aaef10b > src/common/type_utils.cpp 22118b4 > src/master/http.cpp 8bde4c7 > src/slave/http.cpp 101aa06 > src/slave/slave.hpp 32e1830 > src/tests/attributes_tests.cpp ded6120 > src/tests/registrar_tests.cpp aa49c86 > > Diff: https://reviews.apache.org/r/38517/diff/ > > > Testing > --- > > > Thanks, > > Felix Abecassis > >
Re: Review Request 38517: Make attributes.hpp public
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38517/#review99883 --- Ship it! Ship It! - Connor Doyle On Sept. 19, 2015, 12:59 a.m., Felix Abecassis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38517/ > --- > > (Updated Sept. 19, 2015, 12:59 a.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bugs: MESOS-3366 > https://issues.apache.org/jira/browse/MESOS-3366 > > > Repository: mesos > > > Description > --- > > This is required in order to enable callback hooks that can modify attributes > of a slave during initialization. > > > Diffs > - > > include/mesos/attributes.hpp PRE-CREATION > src/Makefile.am 834f69a > src/common/attributes.hpp 2a7efbd > src/common/attributes.cpp f713bb5 > src/common/http.hpp 058baa6 > src/common/http.cpp aaef10b > src/common/type_utils.cpp 22118b4 > src/master/http.cpp 8bde4c7 > src/slave/http.cpp 101aa06 > src/slave/slave.hpp 32e1830 > src/tests/attributes_tests.cpp ded6120 > src/tests/registrar_tests.cpp aa49c86 > > Diff: https://reviews.apache.org/r/38517/diff/ > > > Testing > --- > > > Thanks, > > Felix Abecassis > >
Re: Review Request 38279: Enabled resources/attributes discovery
> On Sept. 14, 2015, 5:34 p.m., Connor Doyle wrote: > > src/hook/manager.cpp, line 261 > > <https://reviews.apache.org/r/38279/diff/1/?file=1067842#file1067842line261> > > > > Please add a comment describing how the order of hook execution is > > determined. e.g. order in which they appear in the modules json passed to > > the slave, etc. > > Felix Abecassis wrote: > Actually, it seems there is no predefined order right now since > src/hook/manager.cpp is using a hashmap, should I open another issue to > discuss how to fix this? Yes, please do. A HashMap might be overkill for that use case. Maybe the hook manager can get by with a simpler order-preserving data structure. As modules become more common, operators will probably want to control the invocation order. For now the flag value is the only control surface exposed. - Connor --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38279/#review98860 --- On Sept. 19, 2015, 12:51 a.m., Felix Abecassis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38279/ > --- > > (Updated Sept. 19, 2015, 12:51 a.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bugs: MESOS-3366 > https://issues.apache.org/jira/browse/MESOS-3366 > > > Repository: mesos > > > Description > --- > > First API draft for MESOS-3366. > > 1) Only supports resources for now, we can add another hook for attributes > with a very similar code. > 2) The callback currently receives the full SlaveInfo structure and construct > a new Resources object. > 3) If there is multiple callbacks, each callback will see the changes made by > previous callbacks and are free to override or merge the detected resources > as they see fit. > > > Diffs > - > > include/mesos/hook.hpp 2353602 > src/examples/test_hook_module.cpp 0dc74d6 > src/hook/manager.hpp a517c05 > src/hook/manager.cpp 691976e > src/slave/slave.cpp ad710d7 > src/tests/hook_tests.cpp b23a587 > > Diff: https://reviews.apache.org/r/38279/diff/ > > > Testing > --- > > make clean && make && make check > > > Thanks, > > Felix Abecassis > >
Re: Review Request 38279: Enabled resources/attributes discovery
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38279/#review98884 --- src/examples/test_hook_module.cpp (line 208) <https://reviews.apache.org/r/38279/#comment155545> Please add a test to verify that the modified resources are reflected in the resource offers formed by the master. There are some good examples to follow in `src/tests/hook_tests.cpp`. - Connor Doyle On Sept. 14, 2015, 5:39 p.m., Felix Abecassis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38279/ > --- > > (Updated Sept. 14, 2015, 5:39 p.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bugs: MESOS-3366 > https://issues.apache.org/jira/browse/MESOS-3366 > > > Repository: mesos > > > Description > --- > > First API draft for MESOS-3366. > > 1) Only supports resources for now, we can add another hook for attributes > with a very similar code. > 2) The callback currently receives the full SlaveInfo structure and construct > a new Resources object. > 3) If there is multiple callbacks, each callback will see the changes made by > previous callbacks and are free to override or merge the detected resources > as they see fit. > > > Diffs > - > > include/mesos/hook.hpp d90bacc > src/examples/test_hook_module.cpp bc13a8a > src/hook/manager.hpp 30d8321 > src/hook/manager.cpp 754c238 > src/slave/slave.cpp 5e5522e > > Diff: https://reviews.apache.org/r/38279/diff/ > > > Testing > --- > > make clean && make && make check > > > Thanks, > > Felix Abecassis > >
Re: Review Request 38279: Enabled resources/attributes discovery
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38279/#review98886 --- src/hook/manager.cpp (line 261) <https://reviews.apache.org/r/38279/#comment18> Please also add a test that verifies that the hooks are executed in the expected order. For example, have two hooks that both modify the `cpus` resource and verify that the second value is indeed retained. - Connor Doyle On Sept. 14, 2015, 5:39 p.m., Felix Abecassis wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38279/ > --- > > (Updated Sept. 14, 2015, 5:39 p.m.) > > > Review request for mesos, Connor Doyle and Niklas Nielsen. > > > Bugs: MESOS-3366 > https://issues.apache.org/jira/browse/MESOS-3366 > > > Repository: mesos > > > Description > --- > > First API draft for MESOS-3366. > > 1) Only supports resources for now, we can add another hook for attributes > with a very similar code. > 2) The callback currently receives the full SlaveInfo structure and construct > a new Resources object. > 3) If there is multiple callbacks, each callback will see the changes made by > previous callbacks and are free to override or merge the detected resources > as they see fit. > > > Diffs > - > > include/mesos/hook.hpp d90bacc > src/examples/test_hook_module.cpp bc13a8a > src/hook/manager.hpp 30d8321 > src/hook/manager.cpp 754c238 > src/slave/slave.cpp 5e5522e > > Diff: https://reviews.apache.org/r/38279/diff/ > > > Testing > --- > > make clean && make && make check > > > Thanks, > > Felix Abecassis > >
Re: Review Request 35961: Include protobuf classes in generated Javadoc.
On June 27, 2015, 6:59 p.m., Adam B wrote:
src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in, lines 27-36
https://reviews.apache.org/r/35961/diff/1/?file=993816#file993816line27
Did you verify that the curly-braces are properly handled here? I read
on http://stackoverflow.com/a/13512524/4056606 that the `{@code` tag will
interpret the first closing brace as its own ending brace. In fact, @code
may only be necessary for wrapping Generic `something` declarations. You
could additionally/instead use the old-style `code` tag to print
everything in code-font. I would hope this would be easier, but parsing
code within docs within code must be difficult. Don't even try to put `/*`
block comments inside your code example, or the compiler will get confused
too.
Yes, it does seem to work. Here is the output as rendered by my up-to-date
Google Chrome browser on OS X:
[](http://s15.postimg.org/p44aqh2e2/Screen_Shot_2015_06_29_at_07_47_24.jpg)
However, I personally find the `code/code` markup more readable than the
dangling closing curling brace. I will update this to use the plain HTML tag
style.
On June 27, 2015, 6:59 p.m., Adam B wrote:
src/java/mesos.pom.in, line 134
https://reviews.apache.org/r/35961/diff/1/?file=993817#file993817line134
Is `${project.basedir}/generated` any better/different than
`@abs_top_builddir@/src/java/generated`
Probably not, will update to use `@abs_top_builddir@` instead.
On June 27, 2015, 6:59 p.m., Adam B wrote:
src/java/src/org/apache/mesos/MesosExecutorDriver.java, line 31
https://reviews.apache.org/r/35961/diff/1/?file=993818#file993818line31
Oracle docs (and wikipedia) claim that javadoc only uses the opening
p tag, without the closing /p tag. Did you find that this rendered
incorrectly?
Good point. I ran the build with JAVA_HOME pointing to Java 8, which is much
more stringent regarding Javadoc errors. Without this change, the build
actually fails with Java 8. I will verify that this still works with Java 8.
- Connor
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
---
On June 27, 2015, 1:18 a.m., Connor Doyle wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/
---
(Updated June 27, 2015, 1:18 a.m.)
Review request for mesos, Adam B and Ben Whitehead.
Bugs: MESOS-1552
https://issues.apache.org/jira/browse/MESOS-1552
Repository: mesos-incubating
Description
---
Updates the Java POM to include the generated Java protobuf classes.
Diffs
-
src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
src/java/mesos.pom.in 138da55
src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
Diff: https://reviews.apache.org/r/35961/diff/
Testing
---
- configure
- make
- Manual verification of generated Javadoc HTML
Thanks,
Connor Doyle
Re: Review Request 35961: Include protobuf classes in generated Javadoc.
On June 27, 2015, 6:59 p.m., Adam B wrote: Thanks for doing this Connor! Just a couple of questions about javadoc parsing of html (I'm no expert, just googled a few things). Do we need to do a pass over our protos now to convert any `//` or `/*` block comments to `/**` comments? Worth a follow-up JIRA/patch? Most of mesos.proto looks fine, except TrafficControlStatistics and ResourceStatistics. And I'm not sure how public these need to be, but messages.proto is all `//` style, or completely missing comments. I think it makes sense to modify the comment style in a follow-up patch. As you mentioned the classes generated from mesos.proto already look pretty good. - Connor --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35961/#review89629 --- On June 29, 2015, 3:53 p.m., Connor Doyle wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35961/ --- (Updated June 29, 2015, 3:53 p.m.) Review request for mesos, Adam B and Ben Whitehead. Bugs: MESOS-1552 https://issues.apache.org/jira/browse/MESOS-1552 Repository: mesos-incubating Description --- Updates the Java POM to include the generated Java protobuf classes. Diffs - src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903 src/java/mesos.pom.in 138da55 src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60 Diff: https://reviews.apache.org/r/35961/diff/ Testing --- - configure - make - Manual verification of generated Javadoc HTML Thanks, Connor Doyle
Re: Review Request 35961: Include protobuf classes in generated Javadoc.
On June 27, 2015, 6:59 p.m., Adam B wrote:
src/java/mesos.pom.in, line 134
https://reviews.apache.org/r/35961/diff/1/?file=993817#file993817line134
Is `${project.basedir}/generated` any better/different than
`@abs_top_builddir@/src/java/generated`
Connor Doyle wrote:
Probably not, will update to use `@abs_top_builddir@` instead.
Updated and verified.
- Connor
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/#review89629
---
On June 29, 2015, 3:53 p.m., Connor Doyle wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35961/
---
(Updated June 29, 2015, 3:53 p.m.)
Review request for mesos, Adam B and Ben Whitehead.
Bugs: MESOS-1552
https://issues.apache.org/jira/browse/MESOS-1552
Repository: mesos-incubating
Description
---
Updates the Java POM to include the generated Java protobuf classes.
Diffs
-
src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903
src/java/mesos.pom.in 138da55
src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60
Diff: https://reviews.apache.org/r/35961/diff/
Testing
---
- configure
- make
- Manual verification of generated Javadoc HTML
Thanks,
Connor Doyle
Review Request 35961: Include protobuf classes in generated Javadoc.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35961/ --- Review request for mesos, Adam B and Ben Whitehead. Bugs: MESOS-1552 https://issues.apache.org/jira/browse/MESOS-1552 Repository: mesos-incubating Description --- Updates the Java POM to include the generated Java protobuf classes. Diffs - src/java/generated/org/apache/mesos/MesosNativeLibrary.java.in c54d903 src/java/mesos.pom.in 138da55 src/java/src/org/apache/mesos/MesosExecutorDriver.java 1b5ed60 Diff: https://reviews.apache.org/r/35961/diff/ Testing --- - configure - make - Manual verification of generated Javadoc HTML Thanks, Connor Doyle
