Re: Review Request 36501: MESOS-3023

2015-07-18 Thread Klaus Ma

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

(Updated July 18, 2015, 9:47 a.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Fix for MESOS-3023 (Factoring out the pattern for URL generation)


Diffs (updated)
-

  src/tests/fetcher_tests.cpp ae10c42 

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


Testing
---

1. Build successfully in Linux
2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest


Thanks,

Klaus Ma



Re: Review Request 36501: MESOS-3023

2015-07-18 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [36501]

All tests passed.

- Mesos ReviewBot


On July 18, 2015, 9:47 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36501/
> ---
> 
> (Updated July 18, 2015, 9:47 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-3023
> https://issues.apache.org/jira/browse/MESOS-3023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix for MESOS-3023 (Factoring out the pattern for URL generation)
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp ae10c42 
> 
> Diff: https://reviews.apache.org/r/36501/diff/
> 
> 
> Testing
> ---
> 
> 1. Build successfully in Linux
> 2. Test passed by src/mesos-tests --gtest_filter=FetcherTest.OSNetUriTest
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>



Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities

2015-07-18 Thread Aditi Dixit

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

(Updated July 18, 2015, 2:23 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This only updates the master, not the allocator. Added test too.


Diffs (updated)
-

  src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
  src/tests/fault_tolerance_tests.cpp 1070ccf17f98f6b3800684a5edd6517d90631c3e 

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


Testing
---

make check


Thanks,

Aditi Dixit



Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities

2015-07-18 Thread Aditi Dixit


> On July 16, 2015, 8:13 p.m., Vinod Kone wrote:
> > src/tests/fault_tolerance_tests.cpp, line 1831
> > 
> >
> > Nice test!

Thanks :)


> On July 16, 2015, 8:13 p.m., Vinod Kone wrote:
> > src/master/master.hpp, line 534
> > 
> >
> > s/capabilities_size()/capabilities.size()/

In mesos.pb.h, capabilities_size() is defined in line 1147 :)


- Aditi


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


On July 18, 2015, 2:23 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35797/
> ---
> 
> (Updated July 18, 2015, 2:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2880
> https://issues.apache.org/jira/browse/MESOS-2880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This only updates the master, not the allocator. Added test too.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
>   src/tests/fault_tolerance_tests.cpp 
> 1070ccf17f98f6b3800684a5edd6517d90631c3e 
> 
> Diff: https://reviews.apache.org/r/35797/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>



Re: Review Request 35797: Updated Frameworkinfo.capabilities on framework re-registration to support adding capabilities

2015-07-18 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [35797]

All tests passed.

- Mesos ReviewBot


On July 18, 2015, 2:23 p.m., Aditi Dixit wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35797/
> ---
> 
> (Updated July 18, 2015, 2:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2880
> https://issues.apache.org/jira/browse/MESOS-2880
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This only updates the master, not the allocator. Added test too.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
>   src/tests/fault_tolerance_tests.cpp 
> 1070ccf17f98f6b3800684a5edd6517d90631c3e 
> 
> Diff: https://reviews.apache.org/r/35797/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Aditi Dixit
> 
>



Re: Review Request 35755: Changed fetcher to handle leading whitespace in URLs.

2015-07-18 Thread Benjamin Hindman

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

Ship it!


I'll take care of these minor issues and commit, thanks.


src/launcher/fetcher.cpp (line 158)


Why not log this after trimming the string so that the log prints out what 
we're actually going to be fetching?



src/tests/fetcher_tests.cpp 


As commented earlier, we keep two newlines between top-level definitions 
(here and the rest of the file).



src/tests/fetcher_tests.cpp (line 278)


While funky, this is really just a method declaration/definition which we 
put after constructor declarations/definitions.



src/tests/fetcher_tests.cpp (line 282)


We've tried to keep the name of the endpoint the same as the name of the 
method (i.e, not 'uri_test' and 'index'). In this case, I recommend just 'test'.


- Benjamin Hindman


On July 10, 2015, 5:19 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35755/
> ---
> 
> (Updated July 10, 2015, 5:19 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2862
> https://issues.apache.org/jira/browse/MESOS-2862
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes MESOS-2862
> 
> 
> Diffs
> -
> 
>   src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd 
>   src/tests/fetcher_tests.cpp ae10c420f7dddb8650377c91b5343591e8560392 
> 
> Diff: https://reviews.apache.org/r/35755/diff/
> 
> 
> Testing
> ---
> 
> - make check 
> - added a test case for fetcher
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>



Re: Review Request 36574: Added helper testing functions for Labels.

2015-07-18 Thread Benjamin Hindman

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



src/tests/mesos.hpp (line 1382)


Can we make this functional instead? s/addLabel/createLabel/ and return the 
created Label. Then later we can do:

labels->add_labels()->CopyFrom(createLabel("foo", "bar"));



src/tests/slave_tests.cpp (line 2077)


labels->add_labels()->CopyFrom(createLabel("foo", "bar"));



src/tests/slave_tests.cpp (line 2126)


Once we have a 'createLabel' we can just do 
`JSON::Protobuf(createLabel("foo", "bar"))` here.


- Benjamin Hindman


On July 17, 2015, 7:55 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36574/
> ---
> 
> (Updated July 17, 2015, 7:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3076
> https://issues.apache.org/jira/browse/MESOS-3076
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - addLabel(lables, key, val) inserts a Label (key,val) into labels.
> - labelJSON(key, val) returns a JSON value with key-val pair.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 767c86cbde31eeb49d110d04bfb5a3eb5d469afc 
>   src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
>   src/tests/slave_tests.cpp 89cc7f68b33b037626ca6056647c360b5a6d5901 
> 
> Diff: https://reviews.apache.org/r/36574/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 36580: Added TaskStatus label decorator hook for Slave.

2015-07-18 Thread Kapil Arya

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

(Updated July 18, 2015, 8:38 p.m.)


Review request for mesos.


Changes
---

rebased


Repository: mesos


Description
---

This allows Slave modules to expose some information to the frameworks
as well as Mesos-DNS via state.json.


Diffs (updated)
-

  include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f 
  src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab 
  src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc 
  src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf 
  src/slave/slave.cpp 2119b5176aa7cfb7b0b551d4d4f65ee12818b9e4 
  src/tests/hook_tests.cpp 09205fb89925c22b1157294a756db87d911a63db 

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


Testing
---

make check with a new hook test.


Thanks,

Kapil Arya



Re: Review Request 36574: Added helper testing functions for Labels.

2015-07-18 Thread Kapil Arya

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

(Updated July 18, 2015, 8:38 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.


Changes
---

Rebased and addressed BenH's comments.


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


Repository: mesos


Description
---

- addLabel(lables, key, val) inserts a Label (key,val) into labels.
- labelJSON(key, val) returns a JSON value with key-val pair.


Diffs (updated)
-

  src/tests/master_tests.cpp 767c86cbde31eeb49d110d04bfb5a3eb5d469afc 
  src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 
  src/tests/slave_tests.cpp 89cc7f68b33b037626ca6056647c360b5a6d5901 

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


Testing
---

make check


Thanks,

Kapil Arya



Review Request 36596: Added jsonify() to stout.

2015-07-18 Thread Kapil Arya

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

Review request for mesos and Benjamin Hindman.


Repository: mesos


Description
---

The jsonify() function converts a Protobuf/string/double/bool to
JSON::Value. Added some tests as well.


Diffs
-

  3rdparty/libprocess/3rdparty/Makefile.am 
856c2b289451fd404b97285b825e72913feb2f04 
  3rdparty/libprocess/3rdparty/stout/Makefile.am 
89e7b1854bd7f449f4f0027d76c6430d259a24de 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
2394b95462182273464f0847f416ad83c3b64485 
  3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp PRE-CREATION 

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


Testing
---

make check with added tests.


Thanks,

Kapil Arya



Re: Review Request 36575: Added Labels to TaskStatus protobuf and expose them via state.json.

2015-07-18 Thread Kapil Arya

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

(Updated July 18, 2015, 8:38 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Niklas Nielsen.


Changes
---

rebased.


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


Repository: mesos


Description
---

The labels would allow executors and Slave modules to pass in some
meta-data about the task to the framework and Mesos-DNS (via state.json).


Diffs (updated)
-

  include/mesos/mesos.proto 1763129da535561503e89cbd8c4a371f8553d8d6 
  src/common/http.cpp 2bb1ba87a2755a4bd9b762280dea6fce81db1320 
  src/common/protobuf_utils.hpp afe5a85d3f58eaabb16807253c5fcc07cabcf8e8 
  src/common/protobuf_utils.cpp 9ac81c38efd70f92c64a5865fa79fe516e84dd92 
  src/tests/common/http_tests.cpp 38d062b2b4062e0a2fc912bad0cc2d73339eb66e 
  src/tests/master_tests.cpp 767c86cbde31eeb49d110d04bfb5a3eb5d469afc 
  src/tests/slave_tests.cpp 89cc7f68b33b037626ca6056647c360b5a6d5901 

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


Testing
---

make check with new tests to verify state.json output.


Thanks,

Kapil Arya



Re: Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-18 Thread Kapil Arya

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

(Updated July 18, 2015, 8:38 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy Chen.


Changes
---

rebased


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


Repository: mesos


Description (updated)
---

This would allow Mesos-DNS to lookup container information such as its
IP address.

NOTE: Apparently, the data size is about 15KB per `docker inspect` blob and 
that might addup for large clusters. One suggestion is to expose only the IP 
field(s) from:

"NetworkSettings": {
"Bridge": "",
"EndpointID": 
"c9d8bfb120503eda3dc88ffdcc5778cff4e7f05bb2e53f9b2fba82972ecfe3da",
"Gateway": "172.17.42.1",
"GlobalIPv6Address": "",
"GlobalIPv6PrefixLen": 0,
"HairpinMode": false,
"IPAddress": "172.17.0.5",
"IPPrefixLen": 16,
"IPv6Gateway": "",
"LinkLocalIPv6Address": "",
"LinkLocalIPv6PrefixLen": 0,
"MacAddress": "02:42:ac:11:00:05",
"NetworkID": 
"f36472f7725507d90ab10215d35d020bf3eaf5ebdc024cb5b3773967d9fc",
"PortMapping": null,
"Ports": {},
"SandboxKey": "/var/run/docker/netns/3c3d4d0f5e18",
"SecondaryIPAddresses": null,
"SecondaryIPv6Addresses": null
},

An alternate is to use a hook module to decorate TaskStatus with IP information 
extracted from the output of `docker inspect`.


Diffs (updated)
-

  src/docker/executor.cpp cdcd8ee7ad0b53748819bc1e565f708e30e99a5d 
  src/tests/docker_containerizer_tests.cpp 
6c6f4b7f30cec9b5bba77234b714e96289c82b43 

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


Testing
---

sudo make check


Thanks,

Kapil Arya



Re: Review Request 36596: Added jsonify() to stout.

2015-07-18 Thread Kapil Arya

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

(Updated July 18, 2015, 8:43 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
---

updated include files.


Repository: mesos


Description
---

The jsonify() function converts a Protobuf/string/double/bool to
JSON::Value. Added some tests as well.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
856c2b289451fd404b97285b825e72913feb2f04 
  3rdparty/libprocess/3rdparty/stout/Makefile.am 
89e7b1854bd7f449f4f0027d76c6430d259a24de 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
2394b95462182273464f0847f416ad83c3b64485 
  3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp PRE-CREATION 

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


Testing
---

make check with added tests.


Thanks,

Kapil Arya



Re: Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-18 Thread Mesos ReviewBot

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


Bad patch!

Reviews applied: [36596]

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

Error:
 2015-07-19 00:57:57 URL:https://reviews.apache.org/r/36596/diff/raw/ 
[8815/8815] -> "36596.patch" [1]
Successfully applied: Added jsonify() to stout.

The jsonify() function converts a Protobuf/string/double/bool to
JSON::Value. Added some tests as well.


Review: https://reviews.apache.org/r/36596
Checking 2 files using filter 
--filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/end_of_line,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/operators,+whitespace/semicolon,+whitespace/tab,+whitespace/todo
Total errors found: 0
ERROR: Commit spanning multiple projects.

Please use separate commits for mesos, libprocess and stout.

Paths grouped by project:
stout:
  3rdparty/libprocess/3rdparty/stout/Makefile.am
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am
  3rdparty/libprocess/3rdparty/stout/include/stout/jsonify.hpp
  3rdparty/libprocess/3rdparty/stout/tests/jsonify_tests.cpp
libprocess:
  3rdparty/libprocess/3rdparty/Makefile.am
Failed to commit patch

- Mesos ReviewBot


On July 19, 2015, 12:38 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36585/
> ---
> 
> (Updated July 19, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy Chen.
> 
> 
> Bugs: MESOS-3061
> https://issues.apache.org/jira/browse/MESOS-3061
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would allow Mesos-DNS to lookup container information such as its
> IP address.
> 
> NOTE: Apparently, the data size is about 15KB per `docker inspect` blob and 
> that might addup for large clusters. One suggestion is to expose only the IP 
> field(s) from:
> 
> "NetworkSettings": {
> "Bridge": "",
> "EndpointID": 
> "c9d8bfb120503eda3dc88ffdcc5778cff4e7f05bb2e53f9b2fba82972ecfe3da",
> "Gateway": "172.17.42.1",
> "GlobalIPv6Address": "",
> "GlobalIPv6PrefixLen": 0,
> "HairpinMode": false,
> "IPAddress": "172.17.0.5",
> "IPPrefixLen": 16,
> "IPv6Gateway": "",
> "LinkLocalIPv6Address": "",
> "LinkLocalIPv6PrefixLen": 0,
> "MacAddress": "02:42:ac:11:00:05",
> "NetworkID": 
> "f36472f7725507d90ab10215d35d020bf3eaf5ebdc024cb5b3773967d9fc",
> "PortMapping": null,
> "Ports": {},
> "SandboxKey": "/var/run/docker/netns/3c3d4d0f5e18",
> "SecondaryIPAddresses": null,
> "SecondaryIPv6Addresses": null
> },
> 
> An alternate is to use a hook module to decorate TaskStatus with IP 
> information extracted from the output of `docker inspect`.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp cdcd8ee7ad0b53748819bc1e565f708e30e99a5d 
>   src/tests/docker_containerizer_tests.cpp 
> 6c6f4b7f30cec9b5bba77234b714e96289c82b43 
> 
> Diff: https://reviews.apache.org/r/36585/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-18 Thread Timothy Chen

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



src/docker/executor.cpp (line 164)


So what's special about the label that we also need to include the docker 
inspect data in there? Why can't the framework just use the data field?



src/tests/docker_containerizer_tests.cpp (line 729)


Since you already check the size I don't think you need a for loop and a 
boolean as well.


- Timothy Chen


On July 19, 2015, 12:38 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36585/
> ---
> 
> (Updated July 19, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy Chen.
> 
> 
> Bugs: MESOS-3061
> https://issues.apache.org/jira/browse/MESOS-3061
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would allow Mesos-DNS to lookup container information such as its
> IP address.
> 
> NOTE: Apparently, the data size is about 15KB per `docker inspect` blob and 
> that might addup for large clusters. One suggestion is to expose only the IP 
> field(s) from:
> 
> "NetworkSettings": {
> "Bridge": "",
> "EndpointID": 
> "c9d8bfb120503eda3dc88ffdcc5778cff4e7f05bb2e53f9b2fba82972ecfe3da",
> "Gateway": "172.17.42.1",
> "GlobalIPv6Address": "",
> "GlobalIPv6PrefixLen": 0,
> "HairpinMode": false,
> "IPAddress": "172.17.0.5",
> "IPPrefixLen": 16,
> "IPv6Gateway": "",
> "LinkLocalIPv6Address": "",
> "LinkLocalIPv6PrefixLen": 0,
> "MacAddress": "02:42:ac:11:00:05",
> "NetworkID": 
> "f36472f7725507d90ab10215d35d020bf3eaf5ebdc024cb5b3773967d9fc",
> "PortMapping": null,
> "Ports": {},
> "SandboxKey": "/var/run/docker/netns/3c3d4d0f5e18",
> "SecondaryIPAddresses": null,
> "SecondaryIPv6Addresses": null
> },
> 
> An alternate is to use a hook module to decorate TaskStatus with IP 
> information extracted from the output of `docker inspect`.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp cdcd8ee7ad0b53748819bc1e565f708e30e99a5d 
>   src/tests/docker_containerizer_tests.cpp 
> 6c6f4b7f30cec9b5bba77234b714e96289c82b43 
> 
> Diff: https://reviews.apache.org/r/36585/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-18 Thread Kapil Arya


> On July 18, 2015, 9:28 p.m., Timothy Chen wrote:
> > src/docker/executor.cpp, line 164
> > 
> >
> > So what's special about the label that we also need to include the 
> > docker inspect data in there? Why can't the framework just use the data 
> > field?

We want to expose the data via state.json so that Mesos-DNS can consume it. 
However, the Master never stores TaskStatus::data and so it is not exposed via 
state.json.

Having said that, it looks like this RR would be a no-go. We would probably end 
up using a Mesos hook module to add a Label with just the container-IP which 
would then be exposed via state.json.


- Kapil


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


On July 18, 2015, 8:38 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36585/
> ---
> 
> (Updated July 18, 2015, 8:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy Chen.
> 
> 
> Bugs: MESOS-3061
> https://issues.apache.org/jira/browse/MESOS-3061
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would allow Mesos-DNS to lookup container information such as its
> IP address.
> 
> NOTE: Apparently, the data size is about 15KB per `docker inspect` blob and 
> that might addup for large clusters. One suggestion is to expose only the IP 
> field(s) from:
> 
> "NetworkSettings": {
> "Bridge": "",
> "EndpointID": 
> "c9d8bfb120503eda3dc88ffdcc5778cff4e7f05bb2e53f9b2fba82972ecfe3da",
> "Gateway": "172.17.42.1",
> "GlobalIPv6Address": "",
> "GlobalIPv6PrefixLen": 0,
> "HairpinMode": false,
> "IPAddress": "172.17.0.5",
> "IPPrefixLen": 16,
> "IPv6Gateway": "",
> "LinkLocalIPv6Address": "",
> "LinkLocalIPv6PrefixLen": 0,
> "MacAddress": "02:42:ac:11:00:05",
> "NetworkID": 
> "f36472f7725507d90ab10215d35d020bf3eaf5ebdc024cb5b3773967d9fc",
> "PortMapping": null,
> "Ports": {},
> "SandboxKey": "/var/run/docker/netns/3c3d4d0f5e18",
> "SecondaryIPAddresses": null,
> "SecondaryIPv6Addresses": null
> },
> 
> An alternate is to use a hook module to decorate TaskStatus with IP 
> information extracted from the output of `docker inspect`.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp cdcd8ee7ad0b53748819bc1e565f708e30e99a5d 
>   src/tests/docker_containerizer_tests.cpp 
> 6c6f4b7f30cec9b5bba77234b714e96289c82b43 
> 
> Diff: https://reviews.apache.org/r/36585/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 36585: Exposed `docker inspect` output via state.json

2015-07-18 Thread Timothy Chen


> On July 19, 2015, 1:28 a.m., Timothy Chen wrote:
> > src/docker/executor.cpp, line 164
> > 
> >
> > So what's special about the label that we also need to include the 
> > docker inspect data in there? Why can't the framework just use the data 
> > field?
> 
> Kapil Arya wrote:
> We want to expose the data via state.json so that Mesos-DNS can consume 
> it. However, the Master never stores TaskStatus::data and so it is not 
> exposed via state.json.
> 
> Having said that, it looks like this RR would be a no-go. We would 
> probably end up using a Mesos hook module to add a Label with just the 
> container-IP which would then be exposed via state.json.

I see, yes we can't store this for all masters.


- Timothy


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


On July 19, 2015, 12:38 a.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36585/
> ---
> 
> (Updated July 19, 2015, 12:38 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, and Timothy Chen.
> 
> 
> Bugs: MESOS-3061
> https://issues.apache.org/jira/browse/MESOS-3061
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would allow Mesos-DNS to lookup container information such as its
> IP address.
> 
> NOTE: Apparently, the data size is about 15KB per `docker inspect` blob and 
> that might addup for large clusters. One suggestion is to expose only the IP 
> field(s) from:
> 
> "NetworkSettings": {
> "Bridge": "",
> "EndpointID": 
> "c9d8bfb120503eda3dc88ffdcc5778cff4e7f05bb2e53f9b2fba82972ecfe3da",
> "Gateway": "172.17.42.1",
> "GlobalIPv6Address": "",
> "GlobalIPv6PrefixLen": 0,
> "HairpinMode": false,
> "IPAddress": "172.17.0.5",
> "IPPrefixLen": 16,
> "IPv6Gateway": "",
> "LinkLocalIPv6Address": "",
> "LinkLocalIPv6PrefixLen": 0,
> "MacAddress": "02:42:ac:11:00:05",
> "NetworkID": 
> "f36472f7725507d90ab10215d35d020bf3eaf5ebdc024cb5b3773967d9fc",
> "PortMapping": null,
> "Ports": {},
> "SandboxKey": "/var/run/docker/netns/3c3d4d0f5e18",
> "SecondaryIPAddresses": null,
> "SecondaryIPv6Addresses": null
> },
> 
> An alternate is to use a hook module to decorate TaskStatus with IP 
> information extracted from the output of `docker inspect`.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp cdcd8ee7ad0b53748819bc1e565f708e30e99a5d 
>   src/tests/docker_containerizer_tests.cpp 
> 6c6f4b7f30cec9b5bba77234b714e96289c82b43 
> 
> Diff: https://reviews.apache.org/r/36585/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>



Re: Review Request 36580: Added TaskStatus label decorator hook for Master/Slave.

2015-07-18 Thread Kapil Arya

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

(Updated July 19, 2015, 12:43 a.m.)


Review request for mesos.


Changes
---

added master hooks as well.


Summary (updated)
-

Added TaskStatus label decorator hook for Master/Slave.


Repository: mesos


Description (updated)
---

This allows Master/Slave modules to expose some information to the
frameworks as well as Mesos-DNS via state.json.


Diffs (updated)
-

  include/mesos/hook.hpp 0995c249e9f07c6c4a26d1c5c369d48bb8056f1f 
  src/examples/test_hook_module.cpp d61cd557d8e44e5089f324edf97b0335a4ededab 
  src/hook/manager.hpp 47e8eb7d54d55049d054cf9b1225e67333f22adc 
  src/hook/manager.cpp 0108534c1fc527a0c66d201d7a5232e80b9928bf 
  src/master/master.hpp 2343a684402972a8c336c0dcdde0bfaffabe7cec 
  src/master/master.cpp b877676afa6f3833eb7d2fb06beeaa288bd8bd5d 
  src/slave/slave.cpp 2119b5176aa7cfb7b0b551d4d4f65ee12818b9e4 
  src/tests/hook_tests.cpp 09205fb89925c22b1157294a756db87d911a63db 

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


Testing
---

make check with a new hook test.


Thanks,

Kapil Arya