You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Zhitao Li <zh...@gmail.com> on 2016/03/22 21:10:00 UTC
Review Request 45177: Prototype of setting DiscoveryInfo.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/
-----------------------------------------------------------
Review request for Aurora.
Repository: aurora
Description
-------
Prototype of setting DiscoveryInfo.
Disclaimer: this just shows how easy it is to pipe this information, not seeking to commit the code as is.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 20cbd419dcb988f2c7ba72c8acbe6fee90d02248
Diff: https://reviews.apache.org/r/45177/diff/
Testing
-------
Use the vagrant environment to start the example in http_example.aurora, and observe the following in master's state.json:
```
curl 192.168.33.7:5050/state | jq . | less
....
"tasks": [
{
"discovery": {
"environment": "test",
"name": "vagrant.test.http_example.1",
"ports": {
"ports": [
{
"name": "http",
"number": 31648
}
]
},
"visibility": "CLUSTER"
},
"executor_id": "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
"framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-0000",
"id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
"name": "vagrant/test/http_example",
"resources": {
"cpus": 0.4,
"disk": 64,
"mem": 32,
"ports": "[31648-31648]"
},
...
....
```
Thanks,
Zhitao Li
Re: Review Request 45177: Prototype of setting DiscoveryInfo.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/#review124885
-----------------------------------------------------------
Master (b5c9e1b) is red with this patch.
./build-support/jenkins/build.sh
:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74: Note: Wrote forwarder org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:20: error: 'org.apache.aurora.GuavaUtils' should be separated from previous imports.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:31: error: Using the '.*' form of import should be avoided - org.apache.aurora.scheduler.storage.entities.*.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:33: error: Using the '.*' form of import should be avoided - org.apache.mesos.Protos.*.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:37: error: Wrong order for 'javax.inject.Inject' import. Order should be: java, javax, scala, com, net, org. Each group should be separated by a single blank line.
[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:38: error: Wrong order for 'java.util.List' import. Order should be: java, javax, scala, com, net, org. Each group should be separated by a single blank line.
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
BUILD FAILED
Total time: 1 mins 44.437 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On March 22, 2016, 8:10 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 8:10 p.m.)
>
>
> Review request for Aurora.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prototype of setting DiscoveryInfo.
>
> Disclaimer: this just shows how easy it is to pipe this information, not seeking to commit the code as is.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 20cbd419dcb988f2c7ba72c8acbe6fee90d02248
>
> Diff: https://reviews.apache.org/r/45177/diff/
>
>
> Testing
> -------
>
> Use the vagrant environment to start the example in http_example.aurora, and observe the following in master's state.json:
>
> ```
> curl 192.168.33.7:5050/state | jq . | less
> ....
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-0000",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> ....
> ```
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 45177: Prototype of setting DiscoveryInfo.
Posted by Stephan Erb <se...@apache.org>.
> On March 25, 2016, 9:30 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 237
> > <https://reviews.apache.org/r/45177/diff/1/?file=1311046#file1311046line237>
> >
> > That can lead to non-DNS compatible names. I guess there is nothering that we can really do about it. It's just something consumers have to be aware of.
Looks like we don't have to worry: https://github.com/apache/aurora/blob/26efe5517fc0cb471101fdcb072e5dbf5d20bc56/api/src/main/thrift/org/apache/aurora/gen/api.thrift#L426
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/#review125456
-----------------------------------------------------------
On March 22, 2016, 9:10 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 9:10 p.m.)
>
>
> Review request for Aurora.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prototype of setting DiscoveryInfo.
>
> Disclaimer: this just shows how easy it is to pipe this information, not seeking to commit the code as is.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 20cbd419dcb988f2c7ba72c8acbe6fee90d02248
>
> Diff: https://reviews.apache.org/r/45177/diff/
>
>
> Testing
> -------
>
> Use the vagrant environment to start the example in http_example.aurora, and observe the following in master's state.json:
>
> ```
> curl 192.168.33.7:5050/state | jq . | less
> ....
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-0000",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> ....
> ```
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 45177: Prototype of setting DiscoveryInfo.
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/#review125456
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 217)
<https://reviews.apache.org/r/45177/#comment188242>
Sounds OK as hard coded value for now.
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 218)
<https://reviews.apache.org/r/45177/#comment188259>
That can lead to non-DNS compatible names. I guess there is nothering that we can really do about it. It's just something consumers have to be aware of.
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 220)
<https://reviews.apache.org/r/45177/#comment188247>
That sounds like a sane choice, but we could also leave it out for now until there is a proper usecase for it.
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 221)
<https://reviews.apache.org/r/45177/#comment188246>
I guess this refers to the API version of the task? The Mesos documentation is not clear here. I'd vote to keep it out for now.
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 224)
<https://reviews.apache.org/r/45177/#comment188254>
We could probably do this via the `Announcer` struct. Unfortunately, the documentation is rather unclear if we should pass L4 or L7 protocol names: https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1799
However, looks like Marathon is saying this should by only `TCP` or `UDP`: https://github.com/mesosphere/marathon/blob/master/src/main/scala/mesosphere/marathon/state/DiscoveryInfo.scala#L25 Hardcoding this value to `TCP` should therefore probably work for us for now, right?
- Stephan Erb
On March 22, 2016, 9:10 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> -----------------------------------------------------------
>
> (Updated March 22, 2016, 9:10 p.m.)
>
>
> Review request for Aurora.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Prototype of setting DiscoveryInfo.
>
> Disclaimer: this just shows how easy it is to pipe this information, not seeking to commit the code as is.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 20cbd419dcb988f2c7ba72c8acbe6fee90d02248
>
> Diff: https://reviews.apache.org/r/45177/diff/
>
>
> Testing
> -------
>
> Use the vagrant environment to start the example in http_example.aurora, and observe the following in master's state.json:
>
> ```
> curl 192.168.33.7:5050/state | jq . | less
> ....
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-0000",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> ....
> ```
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 45177: Setting DiscoveryInfo.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/
-----------------------------------------------------------
(Updated March 31, 2016, 7:51 p.m.)
Review request for Aurora.
Changes
-------
Unit test and command line flag switch.
Summary (updated)
-----------------
Setting DiscoveryInfo.
Repository: aurora
Description (updated)
-------
This allows alternative service discovery methodologies to find tasks from Aurora (e.g. mesos-dns), especially the dynamic port mapping.
Diffs (updated)
-----
RELEASE-NOTES.md 34f28a165aae4ae24fa95ef19b4972e088fd63a0
examples/vagrant/upstart/aurora-scheduler.conf 120b89a1dc10a259940cb9527eb2517f19d04471
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java 949c299bdbc54f976db994266fb97f3099256f13
src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java 7beea81a1e97f3f2fce215ff3e364919c3aebf6d
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 20cbd419dcb988f2c7ba72c8acbe6fee90d02248
src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java 16fa2d35cdf7ecdf82dd1ec7739e685ec1ff1aa2
src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 5103bd0f43d53079976b0f1596e299f2d91aa860
Diff: https://reviews.apache.org/r/45177/diff/
Testing (updated)
-------
1. Added unit test;
2. Use the vagrant environment to start the example in http_example.aurora, and observe the following in master's state.json:
```
curl 192.168.33.7:5050/state | jq . | less
....
"tasks": [
{
"discovery": {
"environment": "test",
"name": "vagrant.test.http_example.1",
"ports": {
"ports": [
{
"name": "http",
"number": 31648
}
]
},
"visibility": "CLUSTER"
},
"executor_id": "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
"framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-0000",
"id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
"name": "vagrant/test/http_example",
"resources": {
"cpus": 0.4,
"disk": 64,
"mem": 32,
"ports": "[31648-31648]"
},
...
....
```
Thanks,
Zhitao Li