> On April 9, 2018, 3:33 p.m., Renan DelValle wrote: > > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > > Lines 277 (patched) > > <https://reviews.apache.org/r/66490/diff/4/?file=1993663#file1993663line281> > > > > I'm not 100% sure what happens if VolumeType is not set (esp since it's > > optional) here but it may crash the scheduler. There should be a check > > added to > > `src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java` > > > > That way even if someone uses a custom client to schedule a job on > > Aurora it'll be able to catch this before it's too late. > > > > It would also be great to add tests to > > `src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java` > > using this new feature. > > Justin Venus wrote: > I'll make some more tests and fix what shakes out. > > Justin Venus wrote: > I have handled the not set case, but I have not yet expanded test cases.
Went through the patch again, we're almost there! I just wanted to point out that the better place for that check that I mentioned before might be here: https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java#L300 - Renan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66490/#review200768 ----------------------------------------------------------- On April 13, 2018, 6:52 p.m., Justin Venus wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66490/ > ----------------------------------------------------------- > > (Updated April 13, 2018, 6:52 p.m.) > > > Review request for Aurora, Renan DelValle and Stephan Erb. > > > Bugs: AURORA-1983 > https://issues.apache.org/jira/browse/AURORA-1983 > > > Repository: aurora > > > Description > ------- > > This change implements the docker/volume isolator as described at > http://mesos.apache.org/documentation/latest/isolators/docker-volume/. > > * behind the scenes upgrade configuration to VolumeSource > * guard against unset enum value while transitioning to VolumeSource > * replace Iterables.transform w/ Stream interface in MesosTaskFactory > * Use string for hostPath instead of struct > * update config thrift tests to check for docker/volume isolator support > * update mesos task factory to support docker/volume isolator > * update documentation for docker/volume isolator support > > > Diffs > ----- > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > ef754e32172e7490a47a13e7b526f243ffa3efeb > docs/reference/configuration.md d4b869b938105ba301fc88d41019af2f1707f6f4 > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 4073229b74d0e0e7fd31552bd96894ceb8a0971a > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > bcb2bbf882f43d813dd26c746d806e78bae6bcf3 > src/main/python/apache/aurora/config/schema/base.py > a629bcd1261e5959da0a8458a55545d4e2c2a7a5 > src/main/python/apache/aurora/config/thrift.py > 6d2dde6e964daa68bf6f0e5bbbffecc5bd8c0431 > src/test/python/apache/aurora/config/test_thrift.py > 8e1d0e177959af12b97bdd1cd47845b72bc12fe1 > > > Diff: https://reviews.apache.org/r/66490/diff/10/ > > > Testing > ------- > > Tests pass locally > ```sh > ./gradlew test > ./pants test src/test/python:: > ``` > > I've deployed this code to a test cluster with rexray, dvdcli, and the agents > have `docker/volume` isolation enabled. I am able to exercise mounting an > EBS volume as I would expect into a container. > > > Thanks, > > Justin Venus > >
