> On March 30, 2017, 7:49 a.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java
> > Lines 89-93 (patched)
> > <https://reviews.apache.org/r/58066/diff/1/?file=1680699#file1680699line89>
> >
> >     It feeld weird to me that we are checkin for `image` when deciding if 
> > we should fill the (from a datastructure perspective) unrelated field 
> > `volumes`. 
> >     
> >     Would it be incorrect that we always populate the volumes?
> 
> Zameer Manji wrote:
>     It seems very wrong to specify volume mounts and not a filesystem image 
> from the Mesos perspective.
> 
> Stephan Erb wrote:
>     We are indeed using this in a form at the moment, even when runnign 
> without filesystem images. 
>     
>     We launch our agents with the following, so that each container gets a 
> private `/tmp` and `/var/tmp`. W
>     ```
>         --default_container_info='{
>                 "type": "MESOS",
>                 "volumes": [
>                     {"host_path": "system/tmp",     "container_path": "/tmp", 
>        "mode": "RW"},
>                     {"host_path": "system/vartmp",  "container_path": 
> "/var/tmp",    "mode": "RW"}
>                 ]
>             }'
>     ```
>     
>     When moving to images for some of our tasks, we will have to move those 
> mounts to the job configuration for the remaining non-image tasks. It is 
> therefore essential that we can this feature. On top, if we really wanted to 
> deny that feature, we would have to prevent it at scheduler admission time, 
> rather than on DB access of the DBTaskStore.

This is a very valid usecase and I think we should support it. However in 
`MesosTaskFactory` we only set volumes if there is an image. I think the best 
course of action from here is the following:

1. Leave the conservative behaviour as is in this patch.
2. File a ticket to track your feature request.
3. Fix the problem with a real patch that address storage, `MesosTaskFactory` 
and adds the appropriate tests.


- Zameer


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


On March 29, 2017, 8:26 p.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58066/
> -----------------------------------------------------------
> 
> (Updated March 29, 2017, 8:26 p.m.)
> 
> 
> Review request for Aurora and Stephan Erb.
> 
> 
> Bugs: AURORA-1914
>     https://issues.apache.org/jira/browse/AURORA-1914
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The original commit adding this feature added an artifical constraint to the 
> schema that prevented more than one task volume per task. This is because 
> there was a `UNIQUE` constraint between the volumes table and the task config 
> table, preventing a task config from being associated with more than one 
> volume.
> 
> This patch removes that constraint. As a result some of the MyBatis mappers 
> had to change and a new migration was added.
> 
> 
> Diffs
> -----
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/migration/V010_RemoveUniqueConstraint.java
>  PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/views/DbContainer.java 
> 6a36e276d85de7b463ff92e53f66e736c0823696 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbTaskConfig.java 
> cf65a28c4eb89e8de77734c93b26bc7234b4f927 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  97f6b80fa02ffb73671cf3c8b2aec32fe93a2030 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> ff1623acddf2ca2d59b5f987955005fe1e5d2bfd 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 80be33969d697ab7821eb0ef7cb5f7a7cb9ba61f 
>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 
> be43a099109d73b5dc34d8a66d2ca9782e025b0b 
> 
> 
> Diff: https://reviews.apache.org/r/58066/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Reply via email to