Re: Review Request 36289: Custom executor support for Scheduler

[John Sirois]

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


This has been idle a good while and it appears wfarner has picked up work on 
https://issues.apache.org/jira/browse/AURORA-1376, is it safe to discard?

- John Sirois


On Aug. 18, 2015, 12:10 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated Aug. 18, 2015, 12:10 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for dynamically chosing an executor that's definied in a server 
> side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single 
> ExecutorSettings object.
> 
> Future:
> ===
> Create an offshoot of the current client that allows to send thrift calls 
> with different executor configs which will allow use of custom executors. 
> Some work on this has already been done and will be published ASAP for 
> testing.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 
> 8162323816aedc711a3af84cd499250b78718ab3 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 50e7fc91108993e547869df5b9e5c925fb89a225 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 37772d0b75d022f072af10d82d096981680e193f 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  608af1afe6fc27c8c597490e88fed75580076c95 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  b2327a47374d81b59886c1e4575ded8340322db7 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> 6a80503aeb2058e8f8065285adc151197d2d14d6 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java 
> a1ac922d471013779710e02c0c9ca9f84b506807 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
>  b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
>  66f20c6a63b331353c467cde5521f21e4df49e2d 
>   
> src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 
> 09380f95a7d9405f770513db35d2a45d23d89b61 
>   src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
> b07ff7babd217dac4153831a0d78325bcb72b306 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json
>  PRE-CREATION 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> Ran end to end, failed upon reaching kerberos tests.
> 
> 
> Thanks,

Re: Review Request 36289: Custom executor support for Scheduler

[Kevin Sweeney]

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


Partial review - stopped at the question of how to ensure the precondition 
check added in SchedulingFilterImpl always passes. I think this patch needs to 
add startup validation and a backfill strategy.


api/src/main/thrift/org/apache/aurora/gen/api.thrift (line 46)


Hmm - why does this magic string exist? Any idea what the implications of 
changing it are?



examples/vagrant/executors-config.json (lines 5 - 6)


Is there a difference between path and resources as far as Mesos is 
concerned?



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 
34)


Given that you use this type in a `Set` you will need to define `hashCode` 
and `equals` here (and for all contained types).



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 
37)


Make these `final` and add a constructor for testing?



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 
48)


Needs `hashCode` and `equals`



src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java (line 
85)


remove redundant use of `this`, here and below



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
(lines 40 - 44)


No JavaDoc for private methods - use a comment instead:

```java
private ExecutorSettingsLoader() {
  // Utility class.
}
```



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
60)


Take a `File` here instead of a `String`.

Return a narrower type, like `Set` (but probably 
`List`, see below.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
63)


Prefer `ImmutableSet.Builder`



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
66)


You leak a file descriptor here - consider just using 
`Files.asCharSource(configFile, StandardCharsets.UTF_8)` and opening it in a 
try-with-resources block:

```java
CharSource configFileSource = Files.asCharSource(configFile, 
StandardCharsets.UTF_8);
try (Reader reader = configFileSource.openBufferedReader()) {
  gson.read(...);
}
```



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
68)


nit: no space between `}` and `.`



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
75)


Mentioning the filename would make this easier to debug.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
(lines 76 - 77)


This is actually caused by the platform JVM not having support for an 
encoding (which is impossible for UTF8 as it's required by the standard). Use 
the `asCharSource` suggestion with the constant from `StandardCharsets` to 
avoid needing to handle this.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
79)


You'll want to concat the error message here as well as the second param 
will just print the stack trace. Also a bit more context would be useful. e.g.

```java
throw new ExecutorSettingsException("Error parsing executor settings JSON: 
" + e, e);
```



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
81)


Can you determine which parameter is missing? This would be a very 
frustrating error message to debug.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
84)


You read this as a `List`, any reason not to use `ImmutableList.copyOf` 
here instead?



src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java (line 81)


`Arg` and add a `@CanRead` annotation for more useful validation.



src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java (line 170)


This needs to re-throw, otherwise initialization w

Re: Review Request 36289: Custom executor support for Scheduler

[Bill Farner]

> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java, 
> > line 57
> > 
> >
> > Please model your configuration as an object, and let Gson do the heavy 
> > lifting for you.  As a starting point, it will probably be something like 
> > this (source trimmed for conciseness):
> > 
> > 
> > ```
> > class ExecutorConfig {
> >   String name;
> >   String path;
> >   String arguments;
> >   Set resources;
> >   ...
> > }
> > ```
> > 
> > Then have gson read `List`.
> > 
> > We'll probably want to have one field that is something like 
> > `JsonObject` so that the executor can have a snippet of custom schema.
> 
> Renan DelValle wrote:
> Aw shucks! Seems so obvious now that you mention it. I went ahead and did 
> that, made my life a lot easier. 
> 
> One thing to note is that thrift already generates an ExecutorConfig 
> class at org/apache/aurora/gen/ExecutorConfig.java 
> 
> Is it OK to have a second class in 
> org/apache/aurora/scheduler/app/ExecutorConfig.java with the same name?
> 
> Bill Farner wrote:
> :-)
> 
> I'd slightly perfer a separate name over fully-qualifying the name here - 
> so yeah, feel free to change it to whatever you see fit and we can iterate if 
> necessary.
> 
> Renan DelValle wrote:
> Bill, could you expand on this idea of having a custom schema as a 
> JsonObject? Could you provide me with an example?
> 
> Bill Farner wrote:
> Oh, all i mean is for common code to handle the required schema elements. 
>  Drawing from the POJO in my original comment, you could hav:
> 
> ```
> class ExecutorConfig {
>   String name;
>   String path;
>   String arguments;
>   Set resources;
>   JsonObject instructions;
> }
> ```
> 
> Gson will parse this, only requiring that the `instructions` field be a 
> valid JSON object.  For the purposes of _that_ parsing code, it is arbitrary 
> JSON.  You will want to create an interface for executors.  Not having 
> thought much about the types, it could be as simple as:
> 
> ```
> interface TaskBuilder {
>   TaskInfo buildFrom(ExecutorConfig config);
> }
> ```
> 
> As mentioned before, you would have multiple of these, keyed by the 
> executor name:
> 
> ```
> Map executorTypes;
> ```
> 
> This provides sufficient abstraction for the thermos `TaskBuilder` to 
> parse its more complex process graph from `ExecutorConfig.instructions`, 
> while the builder invoking the command executor can use a much simpler schema:
> 
> ```
> {
>   "name": "command",
>   "instructions": {
> "command": "echo hello"
>   }
> }
> ```
> 
> Meghdoot Bhattacharya wrote:
> Bill can you explain the motivation of this direction. Why would we want 
> scheduler to handle these? What is custom about building a Mesos task other 
> than the well know command vs other executor taskbuilder differences. The 
> client would pass the "custom data" for executor in the "data" field of the 
> executorconfig object in thrift and that maps to the mesos field. Only for 
> command executor we can make the assumption that "data" has the command 
> string. Server side configuration is pretty static. If at some point we 
> expose this entirely in client side to set then everything can become dynamic.
> In the example you provided above, there is no static instructions for 
> command executor, because command string will come in as data. I feel special 
> instructions to executor can be encoded in the data blob that executor can 
> find out after unmarshalling it. How will Mesos Task creation vary per custom 
> executor because the fields are pretty generic.
> 
> So, for thermos, what fields instructions object going to have. 
> thermosobserver, executoroverhead etc?. These are not fields required for 
> setting in Mesos task. In the end, if we need to introduce this field, this 
> should be strictly optional field and null should be a valid option. We have 
> examples of executors where we would not have any usage of this instructions 
> field. Other than just making config changes on the scheduler side, I dont 
> have to be forced to add plugins or code in the scheduler itself.
> 
> Let us know

Yes, my mistake - the `instructions` field may be unnecssary right now.  The 
intent is that if/when there are params that are specific to an executor, they 
should go into a generic field whose schema is defined by each executor 
launcher.


- Bill


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


On Aug. 18, 2015, 6:10 

Re: Review Request 36289: Custom executor support for Scheduler

[Aurora ReviewBot]

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


Master (22f9cbb) is red with this patch.
  ./build-support/jenkins/build.sh

 src.test.python.apache.aurora.client.cli.plugins   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.quota 
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.sla   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.supdate   
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.task  
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.update
 .   SUCCESS
 src.test.python.apache.aurora.client.cli.version   
 .   SUCCESS
 src.test.python.apache.aurora.client.config
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.client.hooks.non_hooked_api  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_aurora_job_key   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_cluster_option   
 .   SUCCESS
 src.test.python.apache.aurora.common.test_clusters 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_http_signaler
 .   SUCCESS
 src.test.python.apache.aurora.common.test_pex_version  
 .   SUCCESS
 src.test.python.apache.aurora.common.test_shellify 
 .   SUCCESS
 src.test.python.apache.aurora.common.test_transport
 .   SUCCESS
 src.test.python.apache.aurora.config.test_base 
 .   SUCCESS
 
src.test.python.apache.aurora.config.test_constraint_parsing
.   SUCCESS
 src.test.python.apache.aurora.config.test_loader   
 .   SUCCESS
 src.test.python.apache.aurora.config.test_thrift   
 .   SUCCESS
 
src.test.python.apache.aurora.executor.common.path_detector 
.   SUCCESS
 src.test.python.apache.aurora.executor.common.task_info
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_base   
 .   SUCCESS
 src.test.python.apache.aurora.executor.executor_vars   
 .   SUCCESS
 src.test.python.apache.aurora.executor.http_lifecycle  
 .   SUCCESS
 src.test.python.apache.aurora.executor.status_manager  
 .   SUCCESS
 src.test.python.apache.aurora.executor.thermos_task_runner 
 .   FAILURE
 src.test.python.apache.thermos.cli.commands.commands   
 .   SUCCESS
 src.test.python.apache.thermos.cli.common  
 .   SUCCESS
 src.test.python.apache.thermos.cli.main
 .   SUCCESS
 src.test.python.apache.thermos.common.test_pathspec
 .   SUCCESS
 
src.test.python.apache.thermos.core.test_runner_integration 
.   SUCCESS
 src.test.python.apache.thermos.monitoring.test_disk
 .   SUCCESS
 
FAILURE


   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 18, 2015, 6:10 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated Aug. 18, 2015, 6:10 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Reposito

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

> On Aug. 18, 2015, 3:06 a.m., Aurora ReviewBot wrote:
> > Master (22f9cbb) is red with this patch.
> >   ./build-support/jenkins/build.sh
> > 
> > 
> > :api:checkPython
> > :api:generateThriftEntitiesJava
> > :api:classesThriftEntities
> > :api:compileJava UP-TO-DATE
> > :api:generateThriftResources
> > :api:processResources UP-TO-DATE
> > :api:classes
> > :api:jar
> > :compileJavaNote: Writing 
> > file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2
> > 
> > :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/app/ExecutorConfiguration.java:23:8:
> >  Unused import - com.google.gson.JsonObject.
> >  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.xml
> > 
> > * 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 35.518 secs
> > 
> > 
> > I will refresh this build result if you post a review containing 
> > "@ReviewBot retry"

@ReviewBot retry


- Renan


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


On Aug. 18, 2015, 6:10 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated Aug. 18, 2015, 6:10 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for dynamically chosing an executor that's definied in a server 
> side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single 
> ExecutorSettings object.
> 
> Future:
> ===
> Create an offshoot of the current client that allows to send thrift calls 
> with different executor configs which will allow use of custom executors. 
> Some work on this has already been done and will be published ASAP for 
> testing.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 
> 8162323816aedc711a3af84cd499250b78718ab3 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 50e7fc91108993e547869df5b9e5c925fb89a225 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 37772d0b75d022f072af10d82d096981680e193f 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  608af1afe6fc27c8

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

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

(Updated Aug. 18, 2015, 6:10 a.m.)


Review request for Aurora and Bill Farner.


Changes
---

Commited unused import that I had forgotten to :B


Repository: aurora


Description
---

What was done:
==
Added support for dynamically chosing an executor that's definied in a server 
side config file.
Removed command line arguments that were moved over to the config file.
Updated existing code to reflect the use of a Map instead of a single 
ExecutorSettings object.

Future:
===
Create an offshoot of the current client that allows to send thrift calls with 
different executor configs which will allow use of custom executors. Some work 
on this has already been done and will be published ASAP for testing.


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f792be0ad393072b4a4ec525363e06cfd16b63d0 
  examples/vagrant/executors-config.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
744b4a35c61e749734e222b3d4cbd296927665aa 
  examples/vagrant/upstart/aurora-scheduler.conf 
789a3a0315e8530880999432aa9b1e7d0f57d1ff 
  src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
10389640c87b203386313ab79204ea936272d350 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
b3c913892248e4a9a8111412307463985f5ca97f 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
ff6eb980292c05e35dcf68104c870a7bef95629a 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 
8162323816aedc711a3af84cd499250b78718ab3 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
a0e71e1c74f67b8836e7da5418012f342977f661 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
50e7fc91108993e547869df5b9e5c925fb89a225 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
37772d0b75d022f072af10d82d096981680e193f 
  
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
 608af1afe6fc27c8c597490e88fed75580076c95 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
b2327a47374d81b59886c1e4575ded8340322db7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
6a80503aeb2058e8f8065285adc151197d2d14d6 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java 
a1ac922d471013779710e02c0c9ca9f84b506807 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
 b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 66f20c6a63b331353c467cde5521f21e4df49e2d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 
09380f95a7d9405f770513db35d2a45d23d89b61 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
b07ff7babd217dac4153831a0d78325bcb72b306 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json
 PRE-CREATION 

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


Testing
---

Ran jenkins build test, passed all tests, code style checks, findbugs check, 
and PMD.
Ran end to end, failed upon reaching kerberos tests.


Thanks,

Renan DelValle

Re: Review Request 36289: Custom executor support for Scheduler

[Aurora ReviewBot]

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


Master (22f9cbb) is red with this patch.
  ./build-support/jenkins/build.sh


:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2

: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/app/ExecutorConfiguration.java:23:8:
 Unused import - com.google.gson.JsonObject.
 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.xml

* 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 35.518 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Aug. 18, 2015, 12:33 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated Aug. 18, 2015, 12:33 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for dynamically chosing an executor that's definied in a server 
> side config file.
> Removed command line arguments that were moved over to the config file.
> Updated existing code to reflect the use of a Map instead of a single 
> ExecutorSettings object.
> 
> Future:
> ===
> Create an offshoot of the current client that allows to send thrift calls 
> with different executor configs which will allow use of custom executors. 
> Some work on this has already been done and will be published ASAP for 
> testing.
> 
> 
> Diffs
> -
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> f792be0ad393072b4a4ec525363e06cfd16b63d0 
>   examples/vagrant/executors-config.json PRE-CREATION 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 744b4a35c61e749734e222b3d4cbd296927665aa 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 789a3a0315e8530880999432aa9b1e7d0f57d1ff 
>   src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
> e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 10389640c87b203386313ab79204ea936272d350 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> b3c913892248e4a9a8111412307463985f5ca97f 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> ff6eb980292c05e35dcf68104c870a7bef95629a 
>   src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 
> 8162323816aedc711a3af84cd499250b78718ab3 
>   
> src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java
>  a0e71e1c74f67b8836e7da5418012f342977f661 
>   src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
> 50e7fc91108993e547869df5b9e5c925fb89a225 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java 
> PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 37772d0b75d022f072af10d82d096981680e193f 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  608af1afe6fc27c8c597490e88fed75580076c95 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  b2327a47374d81b59886c1e4575ded8340322db7 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> 02fe96445148d1e14d85dc7a6fa386d84

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

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

(Updated Aug. 18, 2015, 12:33 a.m.)


Review request for Aurora and Bill Farner.


Repository: aurora


Description (updated)
---

What was done:
==
Added support for dynamically chosing an executor that's definied in a server 
side config file.
Removed command line arguments that were moved over to the config file.
Updated existing code to reflect the use of a Map instead of a single 
ExecutorSettings object.

Future:
===
Create an offshoot of the current client that allows to send thrift calls with 
different executor configs which will allow use of custom executors. Some work 
on this has already been done and will be published ASAP for testing.


Diffs
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f792be0ad393072b4a4ec525363e06cfd16b63d0 
  examples/vagrant/executors-config.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
744b4a35c61e749734e222b3d4cbd296927665aa 
  examples/vagrant/upstart/aurora-scheduler.conf 
789a3a0315e8530880999432aa9b1e7d0f57d1ff 
  src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
10389640c87b203386313ab79204ea936272d350 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
b3c913892248e4a9a8111412307463985f5ca97f 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
ff6eb980292c05e35dcf68104c870a7bef95629a 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 
8162323816aedc711a3af84cd499250b78718ab3 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
a0e71e1c74f67b8836e7da5418012f342977f661 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
50e7fc91108993e547869df5b9e5c925fb89a225 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
37772d0b75d022f072af10d82d096981680e193f 
  
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
 608af1afe6fc27c8c597490e88fed75580076c95 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
b2327a47374d81b59886c1e4575ded8340322db7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
6a80503aeb2058e8f8065285adc151197d2d14d6 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java 
a1ac922d471013779710e02c0c9ca9f84b506807 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
 b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 66f20c6a63b331353c467cde5521f21e4df49e2d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 
09380f95a7d9405f770513db35d2a45d23d89b61 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
b07ff7babd217dac4153831a0d78325bcb72b306 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json
 PRE-CREATION 

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


Testing (updated)
---

Ran jenkins build test, passed all tests, code style checks, findbugs check, 
and PMD.
Ran end to end, failed upon reaching kerberos tests.


Thanks,

Renan DelValle

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

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

(Updated Aug. 18, 2015, 12:28 a.m.)


Review request for Aurora and Bill Farner.


Changes
---

Added support for dynamically chosing an executor via the name provided in the 
thrift call using MapBinder.
Mesos-command executor now pulls command from data field inside of executor 
settings in the thrift call.
Removed executor name as an argument.
Changed code to allow GSON to do the heavy lifting.
Updated test classes to reflect the use of a Map instead of a single instance 
of ExecutorSettings.

Tested using different executors on one scheduler simultaneously using a 
modified version of the client.


Repository: aurora


Description
---

What was done:
==
Added support for custom executors in the Scheduler via a config file. 
Removed command line arguments that were moved over to the config file.

Future:
===
Extending the client to support custom executors and the mesos-executor.

Caveats:

This contains initial config file with support for thermos and limited support 
for the mesos commandline executor. Mesos-command line executor needs support 
from the client side in order to function at a better capacity. 

Currently, this uses the current client to launch both tasks, meaning as long 
as the client sends a thrift call, the scheduler will schedule a task, be it a 
mesos-command task with a preconfigured command temporarily set in the config 
file or a custom executor task. 

*Support for custom executors in the client must be added in order to fully 
utilize this feature.*


Diffs (updated)
-

  api/src/main/thrift/org/apache/aurora/gen/api.thrift 
f792be0ad393072b4a4ec525363e06cfd16b63d0 
  examples/vagrant/executors-config.json PRE-CREATION 
  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
744b4a35c61e749734e222b3d4cbd296927665aa 
  examples/vagrant/upstart/aurora-scheduler.conf 
789a3a0315e8530880999432aa9b1e7d0f57d1ff 
  src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 
e5953bbf02fc2b08fbdff5c25b5389c5a209dfca 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorConfiguration.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
e74b36bc5f85e5ae5fbb2e0b1e34961251739d9e 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
10389640c87b203386313ab79204ea936272d350 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
b3c913892248e4a9a8111412307463985f5ca97f 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
ff6eb980292c05e35dcf68104c870a7bef95629a 
  src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java 
8162323816aedc711a3af84cd499250b78718ab3 
  
src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java 
a0e71e1c74f67b8836e7da5418012f342977f661 
  src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java 
50e7fc91108993e547869df5b9e5c925fb89a225 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorConfigurationTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
37772d0b75d022f072af10d82d096981680e193f 
  
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
 608af1afe6fc27c8c597490e88fed75580076c95 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
b2327a47374d81b59886c1e4575ded8340322db7 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
02fe96445148d1e14d85dc7a6fa386d84a8a8c70 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
6a80503aeb2058e8f8065285adc151197d2d14d6 
  src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java 
a1ac922d471013779710e02c0c9ca9f84b506807 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java
 b9cb5bfe9f89a8bfdb96b6eeb1998ed105963484 
  
src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java
 66f20c6a63b331353c467cde5521f21e4df49e2d 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimTest.java 
09380f95a7d9405f770513db35d2a45d23d89b61 
  src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorImplTest.java 
b07ff7babd217dac4153831a0d78325bcb72b306 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-mesos-command-example.json
 PRE-CREATION 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-thermos-no-observer.json
 PRE-CREATION 

Diff: https://reviews.apache.org/r/3

Re: Review Request 36289: Custom executor support for Scheduler

[Meghdoot Bhattacharya]

> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java, 
> > line 57
> > 
> >
> > Please model your configuration as an object, and let Gson do the heavy 
> > lifting for you.  As a starting point, it will probably be something like 
> > this (source trimmed for conciseness):
> > 
> > 
> > ```
> > class ExecutorConfig {
> >   String name;
> >   String path;
> >   String arguments;
> >   Set resources;
> >   ...
> > }
> > ```
> > 
> > Then have gson read `List`.
> > 
> > We'll probably want to have one field that is something like 
> > `JsonObject` so that the executor can have a snippet of custom schema.
> 
> Renan DelValle wrote:
> Aw shucks! Seems so obvious now that you mention it. I went ahead and did 
> that, made my life a lot easier. 
> 
> One thing to note is that thrift already generates an ExecutorConfig 
> class at org/apache/aurora/gen/ExecutorConfig.java 
> 
> Is it OK to have a second class in 
> org/apache/aurora/scheduler/app/ExecutorConfig.java with the same name?
> 
> Bill Farner wrote:
> :-)
> 
> I'd slightly perfer a separate name over fully-qualifying the name here - 
> so yeah, feel free to change it to whatever you see fit and we can iterate if 
> necessary.
> 
> Renan DelValle wrote:
> Bill, could you expand on this idea of having a custom schema as a 
> JsonObject? Could you provide me with an example?
> 
> Bill Farner wrote:
> Oh, all i mean is for common code to handle the required schema elements. 
>  Drawing from the POJO in my original comment, you could hav:
> 
> ```
> class ExecutorConfig {
>   String name;
>   String path;
>   String arguments;
>   Set resources;
>   JsonObject instructions;
> }
> ```
> 
> Gson will parse this, only requiring that the `instructions` field be a 
> valid JSON object.  For the purposes of _that_ parsing code, it is arbitrary 
> JSON.  You will want to create an interface for executors.  Not having 
> thought much about the types, it could be as simple as:
> 
> ```
> interface TaskBuilder {
>   TaskInfo buildFrom(ExecutorConfig config);
> }
> ```
> 
> As mentioned before, you would have multiple of these, keyed by the 
> executor name:
> 
> ```
> Map executorTypes;
> ```
> 
> This provides sufficient abstraction for the thermos `TaskBuilder` to 
> parse its more complex process graph from `ExecutorConfig.instructions`, 
> while the builder invoking the command executor can use a much simpler schema:
> 
> ```
> {
>   "name": "command",
>   "instructions": {
> "command": "echo hello"
>   }
> }
> ```

Bill can you explain the motivation of this direction. Why would we want 
scheduler to handle these? What is custom about building a Mesos task other 
than the well know command vs other executor taskbuilder differences. The 
client would pass the "custom data" for executor in the "data" field of the 
executorconfig object in thrift and that maps to the mesos field. Only for 
command executor we can make the assumption that "data" has the command string. 
Server side configuration is pretty static. If at some point we expose this 
entirely in client side to set then everything can become dynamic.
In the example you provided above, there is no static instructions for command 
executor, because command string will come in as data. I feel special 
instructions to executor can be encoded in the data blob that executor can find 
out after unmarshalling it. How will Mesos Task creation vary per custom 
executor because the fields are pretty generic.

So, for thermos, what fields instructions object going to have. 
thermosobserver, executoroverhead etc?. These are not fields required for 
setting in Mesos task. In the end, if we need to introduce this field, this 
should be strictly optional field and null should be a valid option. We have 
examples of executors where we would not have any usage of this instructions 
field. Other than just making config changes on the scheduler side, I dont have 
to be forced to add plugins or code in the scheduler itself.

Let us know


- Meghdoot


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


On July 22, 2015, 7:28 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 22, 2015, 7:28 p.m.)
> 
> 
> Review request for Aurora and Bill Far

Re: Review Request 36289: Custom executor support for Scheduler

[Bill Farner]

> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java, 
> > line 57
> > 
> >
> > Please model your configuration as an object, and let Gson do the heavy 
> > lifting for you.  As a starting point, it will probably be something like 
> > this (source trimmed for conciseness):
> > 
> > 
> > ```
> > class ExecutorConfig {
> >   String name;
> >   String path;
> >   String arguments;
> >   Set resources;
> >   ...
> > }
> > ```
> > 
> > Then have gson read `List`.
> > 
> > We'll probably want to have one field that is something like 
> > `JsonObject` so that the executor can have a snippet of custom schema.
> 
> Renan DelValle wrote:
> Aw shucks! Seems so obvious now that you mention it. I went ahead and did 
> that, made my life a lot easier. 
> 
> One thing to note is that thrift already generates an ExecutorConfig 
> class at org/apache/aurora/gen/ExecutorConfig.java 
> 
> Is it OK to have a second class in 
> org/apache/aurora/scheduler/app/ExecutorConfig.java with the same name?
> 
> Bill Farner wrote:
> :-)
> 
> I'd slightly perfer a separate name over fully-qualifying the name here - 
> so yeah, feel free to change it to whatever you see fit and we can iterate if 
> necessary.
> 
> Renan DelValle wrote:
> Bill, could you expand on this idea of having a custom schema as a 
> JsonObject? Could you provide me with an example?

Oh, all i mean is for common code to handle the required schema elements.  
Drawing from the POJO in my original comment, you could hav:

```
class ExecutorConfig {
  String name;
  String path;
  String arguments;
  Set resources;
  JsonObject instructions;
}
```

Gson will parse this, only requiring that the `instructions` field be a valid 
JSON object.  For the purposes of _that_ parsing code, it is arbitrary JSON.  
You will want to create an interface for executors.  Not having thought much 
about the types, it could be as simple as:

```
interface TaskBuilder {
  TaskInfo buildFrom(ExecutorConfig config);
}
```

As mentioned before, you would have multiple of these, keyed by the executor 
name:

```
Map executorTypes;
```

This provides sufficient abstraction for the thermos `TaskBuilder` to parse its 
more complex process graph from `ExecutorConfig.instructions`, while the 
builder invoking the command executor can use a much simpler schema:

```
{
  "name": "command",
  "instructions": {
"command": "echo hello"
  }
}
```


- Bill


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


On July 22, 2015, 7:28 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 22, 2015, 7:28 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/auror

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java, 
> > line 57
> > 
> >
> > Please model your configuration as an object, and let Gson do the heavy 
> > lifting for you.  As a starting point, it will probably be something like 
> > this (source trimmed for conciseness):
> > 
> > 
> > ```
> > class ExecutorConfig {
> >   String name;
> >   String path;
> >   String arguments;
> >   Set resources;
> >   ...
> > }
> > ```
> > 
> > Then have gson read `List`.
> > 
> > We'll probably want to have one field that is something like 
> > `JsonObject` so that the executor can have a snippet of custom schema.
> 
> Renan DelValle wrote:
> Aw shucks! Seems so obvious now that you mention it. I went ahead and did 
> that, made my life a lot easier. 
> 
> One thing to note is that thrift already generates an ExecutorConfig 
> class at org/apache/aurora/gen/ExecutorConfig.java 
> 
> Is it OK to have a second class in 
> org/apache/aurora/scheduler/app/ExecutorConfig.java with the same name?
> 
> Bill Farner wrote:
> :-)
> 
> I'd slightly perfer a separate name over fully-qualifying the name here - 
> so yeah, feel free to change it to whatever you see fit and we can iterate if 
> necessary.

Bill, could you expand on this idea of having a custom schema as a JsonObject? 
Could you provide me with an example?


- Renan


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


On July 22, 2015, 7:28 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 22, 2015, 7:28 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

> On July 15, 2015, 6:08 p.m., Jay Buffington wrote:
> > > Support for custom executors in the client must be added in order to 
> > > fully utilize this feature.
> > 
> > For the first pass I would expose the interface in the server (thrift api) 
> > and default it to use thermos.  You can get that done and merged and people 
> > with custom clients could take advantage of it.  Then you can update the 
> > client to expose this feature for people using the python client.
> 
> Renan DelValle wrote:
> Sorry for the delayed response, as far as I know the thrift API is 
> currently exposed as the scheduler is open to receving thrift calls from any 
> client. (It just so happens to be that the only client for Aurora at this 
> point is Thermos.) 
> 
> In either case, it looks like the community has decided that it is best 
> to continue to thin out the client, thus the DSL will most likely not be 
> updated to include cusotm executor support, so using custom executors will 
> most likely require a generic Aurora client.

Apologies, apparently I should finish my coffee before doing things in the 
morning, I meant to say that the only Aurora client in existance right now is 
the one for which the DSL is written for. But any client can make thrift calls 
to the scheduler.


- Renan


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


On July 22, 2015, 7:28 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 22, 2015, 7:28 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

> On July 15, 2015, 6:08 p.m., Jay Buffington wrote:
> > > Support for custom executors in the client must be added in order to 
> > > fully utilize this feature.
> > 
> > For the first pass I would expose the interface in the server (thrift api) 
> > and default it to use thermos.  You can get that done and merged and people 
> > with custom clients could take advantage of it.  Then you can update the 
> > client to expose this feature for people using the python client.

Sorry for the delayed response, as far as I know the thrift API is currently 
exposed as the scheduler is open to receving thrift calls from any client. (It 
just so happens to be that the only client for Aurora at this point is 
Thermos.) 

In either case, it looks like the community has decided that it is best to 
continue to thin out the client, thus the DSL will most likely not be updated 
to include cusotm executor support, so using custom executors will most likely 
require a generic Aurora client.


- Renan


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


On July 22, 2015, 7:28 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 22, 2015, 7:28 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Bill Farner]

> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/dist/etc/executors.json, line 1
> > 
> >
> > Please move this file to `examples/vagrant`, as this configuration is 
> > not suitable for distribution to others.
> 
> Renan DelValle wrote:
> Done, however, in order to launch the service in the vagrant image 
> sucessfully, we need a configuration file. How should we address this?

The entire repository is mounted in the vagrant image, accessible at /vagrant.  
You can refer to it at that location.


> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java, 
> > line 57
> > 
> >
> > Please model your configuration as an object, and let Gson do the heavy 
> > lifting for you.  As a starting point, it will probably be something like 
> > this (source trimmed for conciseness):
> > 
> > 
> > ```
> > class ExecutorConfig {
> >   String name;
> >   String path;
> >   String arguments;
> >   Set resources;
> >   ...
> > }
> > ```
> > 
> > Then have gson read `List`.
> > 
> > We'll probably want to have one field that is something like 
> > `JsonObject` so that the executor can have a snippet of custom schema.
> 
> Renan DelValle wrote:
> Aw shucks! Seems so obvious now that you mention it. I went ahead and did 
> that, made my life a lot easier. 
> 
> One thing to note is that thrift already generates an ExecutorConfig 
> class at org/apache/aurora/gen/ExecutorConfig.java 
> 
> Is it OK to have a second class in 
> org/apache/aurora/scheduler/app/ExecutorConfig.java with the same name?

:-)

I'd slightly perfer a separate name over fully-qualifying the name here - so 
yeah, feel free to change it to whatever you see fit and we can iterate if 
necessary.


- Bill


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


On July 15, 2015, 1:13 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 15, 2015, 1:13 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java, 
> > line 57
> > 
> >
> > Please model your configuration as an object, and let Gson do the heavy 
> > lifting for you.  As a starting point, it will probably be something like 
> > this (source trimmed for conciseness):
> > 
> > 
> > ```
> > class ExecutorConfig {
> >   String name;
> >   String path;
> >   String arguments;
> >   Set resources;
> >   ...
> > }
> > ```
> > 
> > Then have gson read `List`.
> > 
> > We'll probably want to have one field that is something like 
> > `JsonObject` so that the executor can have a snippet of custom schema.

Aw shucks! Seems so obvious now that you mention it. I went ahead and did that, 
made my life a lot easier. 

One thing to note is that thrift already generates an ExecutorConfig class at 
org/apache/aurora/gen/ExecutorConfig.java 

Is it OK to have a second class in 
org/apache/aurora/scheduler/app/ExecutorConfig.java with the same name?


> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/dist/etc/executors.json, line 1
> > 
> >
> > Please move this file to `examples/vagrant`, as this configuration is 
> > not suitable for distribution to others.

Done, however, in order to launch the service in the vagrant image sucessfully, 
we need a configuration file. How should we address this?


- Renan


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


On July 15, 2015, 1:13 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 15, 2015, 1:13 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Bill Farner]

> On July 15, 2015, 7:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, lines 
> > 87-115
> > 
> >
> > In this change, we cannot remove these arguments as it breaks 
> > compatibility.  In this change, we'll need to synthesize an entry as though 
> > it were read from the json file, and log a big warning about the 
> > deprecation.
> 
> Kevin Sweeney wrote:
> Can you elaborate on the compatibility breakage? IMO changing 
> command-line args across releases doesn't rise to the level of a 
> compatibility break, as long as the change is isolated to a single component. 
> This doesn't change the way the scheduler behaves on the network from the 
> perspective of other components. The deprecation warning could just as easily 
> be an error, pointing to documentation for the new format.

I was on the fence here, and decided to err on the conservative side.  If this 
is the stance we want to take, i'm all for it.  Let's proceed with removing the 
redundant command line args.


- Bill


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


On July 15, 2015, 1:13 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 15, 2015, 1:13 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Bill Farner]

On Wed, Jul 15, 2015 at 12:13 PM, Kevin Sweeney  wrote:

>
>
> > On July 15, 2015, 12:08 p.m., Bill Farner wrote:
> > > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java,
> lines 87-115
> > > <
> https://reviews.apache.org/r/36289/diff/2/?file=1011920#file1011920line87>
> > >
> > > In this change, we cannot remove these arguments as it breaks
> compatibility.  In this change, we'll need to synthesize an entry as though
> it were read from the json file, and log a big warning about the
> deprecation.
>
> Can you elaborate on the compatibility breakage? IMO changing command-line
> args across releases doesn't rise to the level of a compatibility break, as
> long as the change is isolated to a single component. This doesn't change
> the way the scheduler behaves on the network from the perspective of other
> components. The deprecation warning could just as easily be an error,
> pointing to documentation for the new format.
>


I was on the fence here, and decided to err on the conservative side.  If
this is the stance we want to take, i'm all for it.  Updating my comment.

- Kevin
>
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/#review91793
> ---
>
>
> On July 14, 2015, 6:13 p.m., Renan DelValle wrote:
> >
> > ---
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/36289/
> > ---
> >
> > (Updated July 14, 2015, 6:13 p.m.)
> >
> >
> > Review request for Aurora.
> >
> >
> > Repository: aurora
> >
> >
> > Description
> > ---
> >
> > What was done:
> > ==
> > Added support for custom executors in the Scheduler via a config file.
> > Removed command line arguments that were moved over to the config file.
> >
> > Future:
> > ===
> > Extending the client to support custom executors and the mesos-executor.
> >
> > Caveats:
> > 
> > This contains initial config file with support for thermos and limited
> support for the mesos commandline executor. Mesos-command line executor
> needs support from the client side in order to function at a better
> capacity.
> >
> > Currently, this uses the current client to launch both tasks, meaning as
> long as the client sends a thrift call, the scheduler will schedule a task,
> be it a mesos-command task with a preconfigured command temporarily set in
> the config file or a custom executor task.
> >
> > *Support for custom executors in the client must be added in order to
> fully utilize this feature.*
> >
> >
> > Diffs
> > -
> >
> >   examples/vagrant/upstart/aurora-scheduler-kerberos.conf
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b
> >   examples/vagrant/upstart/aurora-scheduler.conf
> f261c8dcc760151d5a41a986d867585c3a544123
> >   src/dist/etc/executors.json PRE-CREATION
> >
>  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java
> PRE-CREATION
> >   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
> 554a380bdb4ef69561259cdbfbc361694041571e
> >   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java
> 325f55640648151ae19e0c18c6961aeff10bfac3
> >   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java
> c0d165ad34e46653dad95918e0058ebd3f2ee57f
> >
>  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java
> PRE-CREATION
> >   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
> 23c2693f1dfd589043c60ab22e302fb81e62335d
> >
>  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a
> >   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7
> >
>  
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
> PRE-CREATION
> >
> > Diff: https://reviews.apache.org/r/36289/diff/
> >
> >
> > Testing
> > ---
> >
> > Ran jenkins build test, passed all tests, code style checks, findbugs
> check, and PMD.
> >
> >
> > Thanks,
> >
> > Renan DelValle
> >
> >
>
>

Re: Review Request 36289: Custom executor support for Scheduler

[Kevin Sweeney]

> On July 15, 2015, 12:08 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java, lines 
> > 87-115
> > 
> >
> > In this change, we cannot remove these arguments as it breaks 
> > compatibility.  In this change, we'll need to synthesize an entry as though 
> > it were read from the json file, and log a big warning about the 
> > deprecation.

Can you elaborate on the compatibility breakage? IMO changing command-line args 
across releases doesn't rise to the level of a compatibility break, as long as 
the change is isolated to a single component. This doesn't change the way the 
scheduler behaves on the network from the perspective of other components. The 
deprecation warning could just as easily be an error, pointing to documentation 
for the new format.


- Kevin


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


On July 14, 2015, 6:13 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 14, 2015, 6:13 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Bill Farner]

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


Stopped at parsing code, since i belive it can be made quite a bit simpler.


src/dist/etc/executors.json (line 1)


Please move this file to `examples/vagrant`, as this configuration is not 
suitable for distribution to others.



src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java (line 
57)


Please model your configuration as an object, and let Gson do the heavy 
lifting for you.  As a starting point, it will probably be something like this 
(source trimmed for conciseness):

```
class ExecutorConfig {
  String name;
  String path;
  String arguments;
  Set resources;
  ...
}
```

Then have gson read `List`.

We'll probably want to have one field that is something like `JsonObject` 
so that the executor can have a snippet of custom schema.



src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java (lines 83 - 84)


In this change, we cannot remove these arguments as it breaks 
compatibility.  In this change, we'll need to synthesize an entry as though it 
were read from the json file, and log a big warning about the deprecation.


- Bill Farner


On July 15, 2015, 1:13 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 15, 2015, 1:13 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Jay Buffington]

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


> Support for custom executors in the client must be added in order to fully 
> utilize this feature.

For the first pass I would expose the interface in the server (thrift api) and 
default it to use thermos.  You can get that done and merged and people with 
custom clients could take advantage of it.  Then you can update the client to 
expose this feature for people using the python client.

- Jay Buffington


On July 14, 2015, 6:13 p.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 14, 2015, 6:13 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Aurora ReviewBot]

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

Ship it!


Master (d9dac92) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On July 15, 2015, 1:13 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 15, 2015, 1:13 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors and the mesos-executor.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
> 85052ac8e4dde85fbcd85ce839d0647f5632d74b 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> f261c8dcc760151d5a41a986d867585c3a544123 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
>   
> src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 23c2693f1dfd589043c60ab22e302fb81e62335d 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
> c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
>   src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
> ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
>   
> src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> Ran jenkins build test, passed all tests, code style checks, findbugs check, 
> and PMD.
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

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

(Updated July 15, 2015, 1:13 a.m.)


Review request for Aurora.


Changes
---

@ReviewBot retry


Repository: aurora


Description (updated)
---

What was done:
==
Added support for custom executors in the Scheduler via a config file. 
Removed command line arguments that were moved over to the config file.

Future:
===
Extending the client to support custom executors and the mesos-executor.

Caveats:

This contains initial config file with support for thermos and limited support 
for the mesos commandline executor. Mesos-command line executor needs support 
from the client side in order to function at a better capacity. 

Currently, this uses the current client to launch both tasks, meaning as long 
as the client sends a thrift call, the scheduler will schedule a task, be it a 
mesos-command task with a preconfigured command temporarily set in the config 
file or a custom executor task. 

*Support for custom executors in the client must be added in order to fully 
utilize this feature.*


Diffs
-

  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
85052ac8e4dde85fbcd85ce839d0647f5632d74b 
  examples/vagrant/upstart/aurora-scheduler.conf 
f261c8dcc760151d5a41a986d867585c3a544123 
  src/dist/etc/executors.json PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
554a380bdb4ef69561259cdbfbc361694041571e 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
325f55640648151ae19e0c18c6961aeff10bfac3 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
c0d165ad34e46653dad95918e0058ebd3f2ee57f 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
23c2693f1dfd589043c60ab22e302fb81e62335d 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
 PRE-CREATION 

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


Testing (updated)
---

Ran jenkins build test, passed all tests, code style checks, findbugs check, 
and PMD.


Thanks,

Renan DelValle

Re: Review Request 36289: Custom executor support for Scheduler

[Renan DelValle]

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

(Updated July 15, 2015, 1:10 a.m.)


Review request for Aurora.


Changes
---

Created simple test for ExecutorSettingsLoader.
Fixed broken tests as a result of code changes.
Fixed code style accoring to guidelines.


Repository: aurora


Description
---

What was done:
==
Added support for custom executors in the Scheduler via a config file. 
Removed command line arguments that were moved over to the config file.

Future:
===
Extending the client to support custom executors.

Caveats:

This contains initial config file with support for thermos and limited support 
for the mesos commandline executor. Mesos-command line executor needs support 
from the client side in order to function at a better capacity. 

Currently, this uses the current client to launch both tasks, meaning as long 
as the client sends a thrift call, the scheduler will schedule a task, be it a 
mesos-command task with a preconfigured command temporarily set in the config 
file or a custom executor task. 

*Support for custom executors in the client must be added in order to fully 
utilize this feature.*


Diffs (updated)
-

  examples/vagrant/upstart/aurora-scheduler-kerberos.conf 
85052ac8e4dde85fbcd85ce839d0647f5632d74b 
  examples/vagrant/upstart/aurora-scheduler.conf 
f261c8dcc760151d5a41a986d867585c3a544123 
  src/dist/etc/executors.json PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoader.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
554a380bdb4ef69561259cdbfbc361694041571e 
  src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
325f55640648151ae19e0c18c6961aeff10bfac3 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
c0d165ad34e46653dad95918e0058ebd3f2ee57f 
  src/test/java/org/apache/aurora/scheduler/app/ExecutorSettingsLoaderTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
23c2693f1dfd589043c60ab22e302fb81e62335d 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java 
c0cadfb34ade55bdb38ab2c0f89499bd6e8fa97a 
  src/test/java/org/apache/aurora/scheduler/mesos/TaskExecutors.java 
ebd81775c5c9f0ef5c309869df1d12dca3ddbdd7 
  
src/test/resources/org/apache/aurora/scheduler/app/executor-settings-example.json
 PRE-CREATION 

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


Testing
---

./gradlew test
895 tests completed, 44 failed, 3 skipped


Thanks,

Renan DelValle

Re: Review Request 36289: Custom executor support for Scheduler

[Aurora ReviewBot]

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


This patch does not apply cleanly on master (62a432a), do you need to rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On July 8, 2015, 2:06 a.m., Renan DelValle wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36289/
> ---
> 
> (Updated July 8, 2015, 2:06 a.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> What was done:
> ==
> Added support for custom executors in the Scheduler via a config file. 
> Removed command line arguments that were moved over to the config file.
> 
> Future:
> ===
> Extending the client to support custom executors.
> 
> Caveats:
> 
> This contains initial config file with support for thermos and limited 
> support for the mesos commandline executor. Mesos-command line executor needs 
> support from the client side in order to function at a better capacity. 
> 
> Currently, this uses the current client to launch both tasks, meaning as long 
> as the client sends a thrift call, the scheduler will schedule a task, be it 
> a mesos-command task with a preconfigured command temporarily set in the 
> config file or a custom executor task. 
> 
> *Support for custom executors in the client must be added in order to fully 
> utilize this feature.*
> 
> 
> Diffs
> -
> 
>   build.gradle 78159932388046883494dda97b4a86c06772d26d 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 1c2390f0248e91e65a548e67f6af1be8d2526b0a 
>   gradle/wrapper/gradle-wrapper.jar 085a1cdc27db1185342f15a00441734e74fe3735 
>   gradle/wrapper/gradle-wrapper.properties 
> e713793f7d63e65ea68e091b959ba6a68352b48f 
>   gradlew dd29ec5eae9a34d216475d6e5b5d8c810885aa76 
>   src/dist/etc/executors.json PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 554a380bdb4ef69561259cdbfbc361694041571e 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ExecutorSettingsLoader.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java 
> 325f55640648151ae19e0c18c6961aeff10bfac3 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> c0d165ad34e46653dad95918e0058ebd3f2ee57f 
> 
> Diff: https://reviews.apache.org/r/36289/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew test
> 895 tests completed, 44 failed, 3 skipped
> 
> 
> Thanks,
> 
> Renan DelValle
> 
>