> On Jan. 22, 2015, 2:35 a.m., Kevin Sweeney wrote: > > docs/deploying-aurora-scheduler.md, line 163 > > <https://reviews.apache.org/r/28920/diff/18/?file=823201#file823201line163> > > > > Philosophical question: if there's already a hard requirement that the > > container have Python 2.7 why not require that the executor be baked in as > > well? Maybe it's worth calling out as a TODO, but you don't have to answer > > it now.
I think baking the executor into docker images is a recipe for disaster. Any time you upgraded aurora you'd need to then go update all containers with the new executor. Also, I don't like the idea of having to build specific aurora-isms into docker containers (I don't even really like requiring python, but that's unavoidable). > On Jan. 22, 2015, 2:35 a.m., Kevin Sweeney wrote: > > examples/vagrant/provision-dev-cluster.sh, line 17 > > <https://reviews.apache.org/r/28920/diff/18/?file=823204#file823204line17> > > > > nit: sh -c indirection is unnecessary here. Eh, this is just copied from the [docker install docs](https://docs.docker.com/installation/ubuntulinux/#ubuntu-precise-1204-lts-64-bit). > On Jan. 22, 2015, 2:35 a.m., Kevin Sweeney wrote: > > src/main/python/apache/thermos/core/runner.py, lines 625-632 > > <https://reviews.apache.org/r/28920/diff/18/?file=823218#file823218line625> > > > > Naive question: since we have this block here can we drop the preamble > > from the scheduler? We need both, they work together to get the logs/sandbox into the right place, and allow the observer to pick it up. > On Jan. 22, 2015, 2:35 a.m., Kevin Sweeney wrote: > > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, > > lines 147-158 > > <https://reviews.apache.org/r/28920/diff/18/?file=823211#file823211line147> > > > > I don't see a reason the executor can't do this itself, by reading the > > Container field in AssignedTask and environment variables. I'd prefer not > > to introduce a new channel to send configuration from the scheduler to the > > executor with this review. > > > > @wickman might be better able to answer whether this is feasible. Some of the things above can't be (easily) done here in the executor because it's already to late by the time code in the executor begins running. For example, logging is already initialized before any real app code runs. This has gone back and forth a few times now, and we landed on keeping anything docker specific out of the executor (which I like better anyways). There's been a good amount of conversation about this in reviews above. > On Jan. 22, 2015, 2:35 a.m., Kevin Sweeney wrote: > > src/main/python/apache/thermos/config/schema_base.py, lines 65-70 > > <https://reviews.apache.org/r/28920/diff/18/?file=823217#file823217line65> > > > > Can we match the union-like behavior the IDL defines here, > > > > something like: > > > > ```py > > class Container(Struct): > > docker = Docker > > > > class Docker(Struct): > > image = Required(String) > > > > ``` > > > > This will be more easily extensible to more container types without > > requiring backwards-incompatible changes. I like this idea. I'll make the changes. > On Jan. 22, 2015, 2:35 a.m., Kevin Sweeney wrote: > > docs/deploying-aurora-scheduler.md, line 158 > > <https://reviews.apache.org/r/28920/diff/18/?file=823201#file823201line158> > > > > Latest version doesn't require a wrapper script, update docs to reflect? The previous sentence here is "If using a wrapper script". - Steve ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review68845 ----------------------------------------------------------- On Jan. 16, 2015, 12:08 a.m., Steve Niemitz wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28920/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2015, 12:08 a.m.) > > > Review request for Aurora, Jay Buffington, Kevin Sweeney, and Bill Farner. > > > Bugs: AURORA-633 > https://issues.apache.org/jira/browse/AURORA-633 > > > Repository: aurora > > > Description > ------- > > This change adds support for launching docker containers through aurora. > These changes are based off of the discussion in > https://issues.apache.org/jira/browse/AURORA-633 > > As of now, a special thermos_executor.sh script is needed to launch the > executor inside docker containers. A sample aurora file is in > examples/jobs/docker. > > In addition, mesos-slave must be run with `--containerizers=docker,mesos`, > the example upstart config in examples/vagrant/upstart has been updated to > reflect this. > > More information is in subsequent review request comments. > > > Diffs > ----- > > Vagrantfile f8b7db8eebdc6a10989de3bc9a2c3e89ce17f5fc > api/src/main/thrift/org/apache/aurora/gen/api.thrift > 08ba1cdf88b712de22c26c04443079282db59ef9 > config/legacy_untested_classes.txt 33c1d6eb4ea02e01b7002c2c2bae5a3858c8b0e5 > docs/configuration-reference.md f3cb257206a194b82fd2045dc20456ee832dbcea > docs/deploying-aurora-scheduler.md 711ae7eda07c2c1735601c265c06a88c1862cce7 > examples/jobs/docker/hello_docker.aurora PRE-CREATION > examples/vagrant/aurorabuild.sh 1e31f21998d02fd69ce0db88e6adb3d32cff67fd > examples/vagrant/provision-dev-cluster.sh > 7af4b52a6876268a97630279221bb98d9b04efad > examples/vagrant/upstart/aurora-scheduler.conf > 788ec254270bca074dae91829c7f4fccdc8f8bb0 > examples/vagrant/upstart/mesos-slave.conf > 512ce7ecf34042ed68dda55efb2dd0415f8469db > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java > 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 > src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java > 5226e3d1b303b1773a057078f2911c5ec2aa97f5 > src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java > d885b224ec5a1d529347d84e03ba98ab6734a126 > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 01b03508afac37b5a8f0ec5c3da1460695e1ef59 > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > 5bf283062c9d119ff91ed45da8b236e36d0fc9aa > src/main/python/apache/aurora/config/thrift.py > ba94ac3c0cbaf3c91eb1a1d86a244ed6fa3b649c > src/main/python/apache/aurora/executor/aurora_executor.py > 636b23d30a897b557eb8c3f8733c90b23cb807ef > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py > f7d8977e42aa56188799400bf8e12a6886fb4a8f > src/main/python/apache/aurora/executor/common/sandbox.py > f47a32b3fefb4a89940b1ddc473b8316ac00df12 > src/main/python/apache/aurora/executor/thermos_task_runner.py > 5e4bd65537d186459003c0b9434f1b769e04f448 > src/main/python/apache/thermos/config/schema_base.py > f9143cc1b83143d6147f59d90c79435d055d0518 > src/main/python/apache/thermos/core/runner.py > 8aac6b50c66080abbb5308b367e9f74c487f42e3 > src/main/resources/scheduler/assets/configSummary.html > 28878908b0c9381e366b71a3135dfc28c542a890 > src/main/resources/scheduler/assets/js/services.js > b744f375411e09b7f776e4a05ee5961227143439 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 5e54364a49a208bd5f19b9649633dc8feca591e9 > src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java > 876e173ccbac04e4a06a245648c7c6af15eaaa92 > > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java > dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java > ddcb511d108220ab5e4efcf3496458f7ab4a20c2 > src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java > 7eafe074b686d55ad96018006cf4acfa823513c3 > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > ad9126c32893080e128d086ea3bfd7ad23d27b89 > src/test/python/apache/aurora/client/cli/test_status.py > e531fa06e508d9792af51c62e67120c21baa7a81 > src/test/python/apache/aurora/executor/test_thermos_executor.py > 503e62f4cac872b14f6985b5bccc3e4dfcf81789 > > Diff: https://reviews.apache.org/r/28920/diff/ > > > Testing > ------- > > > Thanks, > > Steve Niemitz > >
