Re: Review Request 28920: Add support for docker containers to aurora
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. Steve Niemitz wrote: 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). I'm with Steve here. Forcing this seems overly restrictive. However, i would like to support the mode of operation you describe, Kevin. - Bill --- 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
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69269 --- I think we should leave the yaml code but drop the PyYAML dependency from the client requirements. (And inject it into the test -- possibly one test with and one test without to make sure the try/except also functions correctly.) I feel that .yml is actually a much more natural way of defining the cluster list than json, because YAML supports basic templating in order to reduce redundancy, similar to pystachio. The way the code is currently structured, YAML will still work fine if it's available in the environmenet of your Aurora client, and gracefully fall back if not. Thoughts? - Brian Wickman On Jan. 22, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 9:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69268 --- Ship it! Ship It! - Zameer Manji On Jan. 22, 2015, 1:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 1:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
On Jan. 22, 2015, 9:30 p.m., Brian Wickman wrote: I think we should leave the yaml code but drop the PyYAML dependency from the client requirements. (And inject it into the test -- possibly one test with and one test without to make sure the try/except also functions correctly.) I feel that .yml is actually a much more natural way of defining the cluster list than json, because YAML supports basic templating in order to reduce redundancy, similar to pystachio. The way the code is currently structured, YAML will still work fine if it's available in the environmenet of your Aurora client, and gracefully fall back if not. Thoughts? Zameer Manji wrote: If we are going to drop the PyYAML dependency then I think we should drop all of the YAML related code. I agree that YAML is a much nicer format because of the templating functionality it has but as it stands I think the cost of supporting YAML (native code, extra compelxity in loading) is greater than the benefits of a simpler config file. Joshua Cohen wrote: I'm not sure the added complexity is worth the minor benefits provided by YAML. Is cluster configuration really so complex that it requires templating (and if so, could we rely on cluster administrators to work out their own mechanism for templatizing config, puppet/chef/etc.) I agree with Zameer and Joshua - we should avoid complexity that only benefits partially-supported features. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69269 --- On Jan. 22, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 9:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 29464: Add option to override local scheduler address published into ZooKeeper
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29464/#review69276 --- Ping - Kevin? - Bill Farner On Jan. 8, 2015, 5:25 p.m., Steve Niemitz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29464/ --- (Updated Jan. 8, 2015, 5:25 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- I've added a new flag for the aurora scheduler, -hostname which can override the scheduler server address published into ZK. This is useful for cases such as running the scheduler in EC2, where the autodetected local address is actual an interal IP and not the public address of the machine. Diffs - api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 2c449a491bc5a8ac858ea6487e4cef0591f36f66 src/main/java/org/apache/aurora/scheduler/app/AppModule.java 360e161b6c3f6fd412c7e8de7f1b9a3af109593c src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java cf173850635572c0df38bdd5cb14de8ce2016bf7 src/main/python/apache/aurora/executor/common/announcer.py 9e5bdc3885e76d8d03aa946caac9fdec7e1e9186 src/main/python/apache/aurora/executor/thermos_task_runner.py 5e4bd65537d186459003c0b9434f1b769e04f448 src/main/python/apache/thermos/bin/thermos_runner.py 647de2771f301b17de33d8b45198c211d2e84367 src/main/python/apache/thermos/core/runner.py 8aac6b50c66080abbb5308b367e9f74c487f42e3 src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 5e54364a49a208bd5f19b9649633dc8feca591e9 src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java fbc3da3ab239b67ce3012d5a14fccd3ccb20a241 src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java c3bf5ea4cbeaad03e187f84215b86531d55c25b3 src/test/python/apache/aurora/executor/common/test_announcer.py e329a90b8fba43611f5120e2a5ee82220dbe2a91 Diff: https://reviews.apache.org/r/29464/diff/ Testing --- Thanks, Steve Niemitz
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
On Jan. 22, 2015, 1:30 p.m., Brian Wickman wrote: I think we should leave the yaml code but drop the PyYAML dependency from the client requirements. (And inject it into the test -- possibly one test with and one test without to make sure the try/except also functions correctly.) I feel that .yml is actually a much more natural way of defining the cluster list than json, because YAML supports basic templating in order to reduce redundancy, similar to pystachio. The way the code is currently structured, YAML will still work fine if it's available in the environmenet of your Aurora client, and gracefully fall back if not. Thoughts? If we are going to drop the PyYAML dependency then I think we should drop all of the YAML related code. I agree that YAML is a much nicer format because of the templating functionality it has but as it stands I think the cost of supporting YAML (native code, extra compelxity in loading) is greater than the benefits of a simpler config file. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69269 --- On Jan. 22, 2015, 1:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 1:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 30187: Remove support for cluster metadata in YAML format.
On Jan. 22, 2015, 9:30 p.m., Brian Wickman wrote: I think we should leave the yaml code but drop the PyYAML dependency from the client requirements. (And inject it into the test -- possibly one test with and one test without to make sure the try/except also functions correctly.) I feel that .yml is actually a much more natural way of defining the cluster list than json, because YAML supports basic templating in order to reduce redundancy, similar to pystachio. The way the code is currently structured, YAML will still work fine if it's available in the environmenet of your Aurora client, and gracefully fall back if not. Thoughts? Zameer Manji wrote: If we are going to drop the PyYAML dependency then I think we should drop all of the YAML related code. I agree that YAML is a much nicer format because of the templating functionality it has but as it stands I think the cost of supporting YAML (native code, extra compelxity in loading) is greater than the benefits of a simpler config file. I'm not sure the added complexity is worth the minor benefits provided by YAML. Is cluster configuration really so complex that it requires templating (and if so, could we rely on cluster administrators to work out their own mechanism for templatizing config, puppet/chef/etc.) - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/#review69269 --- On Jan. 22, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30187/ --- (Updated Jan. 22, 2015, 9:09 p.m.) Review request for Aurora, Brian Wickman and Zameer Manji. Bugs: AURORA-1029 https://issues.apache.org/jira/browse/AURORA-1029 Repository: aurora Description --- Remove support for cluster metadata in YAML format. Diffs - src/main/python/apache/aurora/common/clusters.py e55aa774b4b868f696a7de51bb016f950871dd1e src/test/python/apache/aurora/common/BUILD 14165b96be99b8de418f4bb8def9f27eaf29e67d src/test/python/apache/aurora/common/test_clusters.py 45250e609cca1149dc296b2aaf645ff2f58f8288 Diff: https://reviews.apache.org/r/30187/diff/ Testing --- ./build-support/jenkins/build.sh test_end_to_end.sh is currently broken on master, i will address that and ensure it passes before committing this. Thanks, Bill Farner
Re: Review Request 28920: Add support for docker containers to aurora
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review69282 --- src/main/python/apache/thermos/core/runner.py https://reviews.apache.org/r/28920/#comment113921 this is an abstraction leak. grep the thermos codebase for 'aurora' and 'mesos'. thermos should remain aurora/mesos agnostic. thermos_runner.py includes a --sandbox flag and we should pass $MESOS_DIRECTORY to that, and not override here. the log_dir defaults to os.path.join(sandbox, '.logs'), so the log_dir part here is unnecessary. - Brian Wickman 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
Re: Review Request 28920: Add support for docker containers to aurora
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
Re: Review Request 28920: Add support for docker containers to aurora
On Jan. 22, 2015, 10:22 p.m., Brian Wickman wrote: src/main/python/apache/thermos/core/runner.py, lines 627-632 https://reviews.apache.org/r/28920/diff/18/?file=823218#file823218line627 this is an abstraction leak. grep the thermos codebase for 'aurora' and 'mesos'. thermos should remain aurora/mesos agnostic. thermos_runner.py includes a --sandbox flag and we should pass $MESOS_DIRECTORY to that, and not override here. the log_dir defaults to os.path.join(sandbox, '.logs'), so the log_dir part here is unnecessary. Heh, I'm pretty sure it was like that ~20 reviews ago. To be clear, your proposing passing the flag into the runner from thermos_task_runner.py? - Steve --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review69282 --- 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
Re: Review Request 28920: Add support for docker containers to aurora
On Jan. 22, 2015, 10:22 p.m., Brian Wickman wrote: src/main/python/apache/thermos/core/runner.py, lines 627-632 https://reviews.apache.org/r/28920/diff/18/?file=823218#file823218line627 this is an abstraction leak. grep the thermos codebase for 'aurora' and 'mesos'. thermos should remain aurora/mesos agnostic. thermos_runner.py includes a --sandbox flag and we should pass $MESOS_DIRECTORY to that, and not override here. the log_dir defaults to os.path.join(sandbox, '.logs'), so the log_dir part here is unnecessary. Steve Niemitz wrote: Heh, I'm pretty sure it was like that ~20 reviews ago. To be clear, your proposing passing the flag into the runner from thermos_task_runner.py? yep. - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review69282 --- 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
Review Request 30203: Fix impedance mismatch between offer matching and task launching.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30203/ --- Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1050 https://issues.apache.org/jira/browse/AURORA-1050 Repository: aurora Description --- See linked ticket for context on how this manifested. Please don't be overwhelmed by the large delta in this diff - a majority of it is reorganizing code to live in more appropriate places. This happened because the logic for offer matching and task launching were out of sync. For small tasks (smaller than `MIN_THERMOS_RESOURCES`), an additional amount (`MIN_TASK_RESOURCES`) would be unintentionally added when the task was launched, but this was not considered when comparing the task to offers. The test case `MesosTaskFactoryImplTest.testSmallTaskUpsizing` was added to reproduce this bug. This change does several things to make the situation more sane: - `ResourceSlot` no longer directly accesses command line arguments, to simplify testing - You may no longer create a `ResourceSlot` from its constitutent parts, to prevent accidental misuse - The algorithm used in `ResourceSlot` was simplified such that an epsilon is always used for the executor resources, and that is subtracted from the final resources required by the task. Diffs - src/main/java/org/apache/aurora/scheduler/ResourceSlot.java 0b15834ec67959d3be94f9a5240ed38f43ac4c5b src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 src/main/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterState.java e6bd1b517535cafce4976e585b377065dfd19796 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 024a689d788804e95de76570674b6d4aa77d7495 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 0204d14b19ae412236f19ca274d81decb4eba12d src/main/java/org/apache/aurora/scheduler/configuration/Resources.java 65c4b526c89a4d5607af4424ebe49bb48e296ae9 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java c2a342ce07bfb223193886038761f0da5230135d src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 1cb56f19c331508a1585077e9c4a98f52aac343b src/main/java/org/apache/aurora/scheduler/mesos/ExecutorSettings.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 5cc85f1f87f3b8355c89e8ecac19de1122a079e6 src/test/java/org/apache/aurora/scheduler/ResourceSlotTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 7ba946422577c21cbc3b3edf8d30ee313b0ef251 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 5e54364a49a208bd5f19b9649633dc8feca591e9 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 36dbcf73686c5a3ade01f7a10fda8ac4bdbcc7ad src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 0b41156f2a574d3d3c2cf840926f307dfb1e726e src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 265c38d20136210e7639ac8ea915d307a4b72949 src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java c7de6e111300b009e1f9f430624a56100328184e src/test/java/org/apache/aurora/scheduler/mesos/Offers.java PRE-CREATION Diff: https://reviews.apache.org/r/30203/diff/ Testing --- Thanks, Bill Farner
Review Request 30204: Upgrade to rbt=0.7.0.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30204/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Release notes: https://www.reviewboard.org/docs/releasenotes/rbtools/0.7/ Diffs - rbt 0742cf47924ee013758883878fa229b5b876be27 Diff: https://reviews.apache.org/r/30204/diff/ Testing --- Posted this review. Thanks, Bill Farner
Re: Review Request 30204: Upgrade to rbt=0.7.0.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30204/ --- (Updated Jan. 23, 2015, 2:11 a.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description (updated) --- Release notes: https://www.reviewboard.org/docs/releasenotes/rbtools/0.7/ Some potentially-interesting new features: ``` rbt land The new rbt land command is a quick and easy way to push a change that has been reviewed on Review Board to the upstream repository. The change may be in a local branch or stored as a patch on Review Board. When running against Review Board 2.0+, this command will start by checking the approval state of the review request (on older versions, it just checks for one or more “Ship It!”s). If the change is approved, the patch will be applied just like rbt patch. The change can optionally be pushed to the remote repository using the -p/--push command line option. This is currently only available when using Git repositories. rbt stamp The new rbt stamp command will amend a commit message with a “Reviewed at url” line. This feature is especially useful when using the close-on-submit hooks in Review Board 2.x to close out review requests when the changes are committed. This is currently only supported for Git repositories. Patch by Yanjia (Nicole) Xin. ``` Diffs - rbt 0742cf47924ee013758883878fa229b5b876be27 Diff: https://reviews.apache.org/r/30204/diff/ Testing --- Posted this review. Thanks, Bill Farner
Re: Review Request 28920: Add support for docker containers to aurora
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review69333 --- src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java https://reviews.apache.org/r/28920/#comment114008 Can you set force_pull_image to true here. I can't imagine why you would ever want that to be false. https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1028 - Jay Buffington 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/
Re: Review Request 28920: Add support for docker containers to aurora
On Jan. 23, 2015, 2:43 a.m., Jay Buffington wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 294 https://reviews.apache.org/r/28920/diff/18/?file=823211#file823211line294 Can you set force_pull_image to true here. I can't imagine why you would ever want that to be false. https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1028 I have plans for that along with privilaged mode, but both require mesos 0.21.0 :( Once this patch is in we can start the discussion of bumping the requirement up on mesos. - Steve --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review69333 --- 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
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/ --- (Updated Jan. 23, 2015, 2:55 vorm.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Implemented the changes Zameer mentioned Bugs: AURORA-184 https://issues.apache.org/jira/browse/AURORA-184 Repository: aurora Description --- [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints This is the first step for AURORA-184, that removes the default hostrack limit constraints. The second step that's still missing would be to add s.th. like --default-constraints as start parameter to the scheduler. AURORA-174 could probably be closed with this?(since the rack limit constraint can be configured in the .aurora file) I can't really estimate the effect of my changes in StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look at the changes I did there. Since this is also my first code submit, comments about codestyleother bad habbits are very appreciated. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 01b03508afac37b5a8f0ec5c3da1460695e1ef59 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 Diff: https://reviews.apache.org/r/30010/diff/ Testing --- Added test for ConfigurationManager.hasName Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present Tested on vagrant devcluster to see if constraints are also gone in real life Thanks, Florian Pfeiffer
Review Request 30207: Simplify AuroraCommandContext
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30207/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- The AuroraCommandContext class is used in multiple commands and contains common code for all of them. However some portions are only used by one command. This patch takes some of those portions and moves them to the command that requires that functionality. Diffs - src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/update.py PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py PRE-CREATION src/test/python/apache/aurora/client/cli/test_update.py 8b7d11202b35deb09a248cfe0a96458fb70c Diff: https://reviews.apache.org/r/30207/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/client:: Thanks, Zameer Manji
Re: Review Request 30010: [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/#review69346 --- Master (3fa004b) 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 :assemble :compileJmhJavawarning: Supported source version 'RELEASE_6' from annotation processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source '1.7' 1 warning :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain :compileTestJava :processTestResources :testClasses :checkstyleTest[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java:22: 'org.apache.aurora.gen.Constraint' should be separated from previous imports. FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':checkstyleTest'. Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/test.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 32.755 secs I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Jan. 23, 2015, 2:55 a.m., Florian Pfeiffer wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30010/ --- (Updated Jan. 23, 2015, 2:55 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-184 https://issues.apache.org/jira/browse/AURORA-184 Repository: aurora Description --- [AURORA-184] Remove hardcoded 'host' and 'rack' limit constraints This is the first step for AURORA-184, that removes the default hostrack limit constraints. The second step that's still missing would be to add s.th. like --default-constraints as start parameter to the scheduler. AURORA-174 could probably be closed with this?(since the rack limit constraint can be configured in the .aurora file) I can't really estimate the effect of my changes in StorageBackfillTestSchedulerThriftInterfaceTest, please have a closer look at the changes I did there. Since this is also my first code submit, comments about codestyleother bad habbits are very appreciated. Diffs - src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 01b03508afac37b5a8f0ec5c3da1460695e1ef59 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java dc2cb37adf32df0a6e4c7ee2ba776ba9f1f3c2f8 Diff: https://reviews.apache.org/r/30010/diff/ Testing --- Added test for ConfigurationManager.hasName Added test testNoHostAndRackConstraintsAdded, that checks if the constraints are present Tested on vagrant devcluster to see if constraints are also gone in real life Thanks, Florian Pfeiffer
Re: Review Request 28920: Add support for docker containers to aurora
On Jan. 22, 2015, 10:42 p.m., Brian Wickman wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, lines 153-158 https://reviews.apache.org/r/28920/diff/18/?file=823211#file823211line153 can't the stuff in DOCKER_COMMAND_PREFIX be accomplished with a DockerSandboxProvider in the thermos executor? similarly, DOCKER_COMMAND_SUFFIX should be unnecessary since this information is available in the TaskConfig. if both the above are true, then we don't need any of the changes to CommandUtil. No unfortunatly, once the executor is started it's too late to change some things, they need to be setup before the executor launches (see my previous comment w/ Kevin). The suffix could be moved into the executor, but again I'd like to keep docker specific logic out of there. - Steve --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review69288 --- 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
Re: Review Request 28920: Add support for docker containers to aurora
On Jan. 21, 2015, 6:35 p.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. Steve Niemitz wrote: Eh, this is just copied from the [docker install docs](https://docs.docker.com/installation/ubuntulinux/#ubuntu-precise-1204-lts-64-bit). The `sh -c` indirection is necessary there because of the use of `sudo`. In a script that's already running as root it's unnecesary. On Jan. 21, 2015, 6:35 p.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. Steve Niemitz wrote: 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). Bill Farner wrote: I'm with Steve here. Forcing this seems overly restrictive. However, i would like to support the mode of operation you describe, Kevin. Just seems weird to put partial requirements on the execution environment of the container we can run, which somewhat defeats the purpose of containers. Filed https://issues.apache.org/jira/browse/AURORA-1051 to explore removing this restriction. On Jan. 21, 2015, 6:35 p.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? Steve Niemitz wrote: The previous sentence here is If using a wrapper script. Gotcha, my bad. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review68845 --- On Jan. 15, 2015, 4:08 p.m., Steve Niemitz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/ --- (Updated Jan. 15, 2015, 4:08 p.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
Re: Review Request 28920: Add support for docker containers to aurora
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28920/#review69288 --- src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java https://reviews.apache.org/r/28920/#comment113931 can't the stuff in DOCKER_COMMAND_PREFIX be accomplished with a DockerSandboxProvider in the thermos executor? similarly, DOCKER_COMMAND_SUFFIX should be unnecessary since this information is available in the TaskConfig. if both the above are true, then we don't need any of the changes to CommandUtil. - Brian Wickman 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
Re: Review Request 29464: Add option to override local scheduler address published into ZooKeeper
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29464/#review69306 --- Ship it! Ship It! - Kevin Sweeney On Jan. 8, 2015, 9:25 a.m., Steve Niemitz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29464/ --- (Updated Jan. 8, 2015, 9:25 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- I've added a new flag for the aurora scheduler, -hostname which can override the scheduler server address published into ZK. This is useful for cases such as running the scheduler in EC2, where the autodetected local address is actual an interal IP and not the public address of the machine. Diffs - api/src/main/thrift/org/apache/thermos/thermos_internal.thrift 2c449a491bc5a8ac858ea6487e4cef0591f36f66 src/main/java/org/apache/aurora/scheduler/app/AppModule.java 360e161b6c3f6fd412c7e8de7f1b9a3af109593c src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 72c7545e7f16549f6a9ccb5fb74a06f154a7ea94 src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java cf173850635572c0df38bdd5cb14de8ce2016bf7 src/main/python/apache/aurora/executor/common/announcer.py 9e5bdc3885e76d8d03aa946caac9fdec7e1e9186 src/main/python/apache/aurora/executor/thermos_task_runner.py 5e4bd65537d186459003c0b9434f1b769e04f448 src/main/python/apache/thermos/bin/thermos_runner.py 647de2771f301b17de33d8b45198c211d2e84367 src/main/python/apache/thermos/core/runner.py 8aac6b50c66080abbb5308b367e9f74c487f42e3 src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 5e54364a49a208bd5f19b9649633dc8feca591e9 src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java fbc3da3ab239b67ce3012d5a14fccd3ccb20a241 src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java c3bf5ea4cbeaad03e187f84215b86531d55c25b3 src/test/python/apache/aurora/executor/common/test_announcer.py e329a90b8fba43611f5120e2a5ee82220dbe2a91 Diff: https://reviews.apache.org/r/29464/diff/ Testing --- Thanks, Steve Niemitz
Re: Review Request 28920: Add support for docker containers to aurora
On Jan. 22, 2015, 10:42 p.m., Brian Wickman wrote:
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java,
lines 153-158
https://reviews.apache.org/r/28920/diff/18/?file=823211#file823211line153
can't the stuff in DOCKER_COMMAND_PREFIX be accomplished with a
DockerSandboxProvider in the thermos executor?
similarly, DOCKER_COMMAND_SUFFIX should be unnecessary since this
information is available in the TaskConfig.
if both the above are true, then we don't need any of the changes to
CommandUtil.
Steve Niemitz wrote:
No unfortunatly, once the executor is started it's too late to change
some things, they need to be setup before the executor launches (see my
previous comment w/ Kevin). The suffix could be moved into the executor, but
again I'd like to keep docker specific logic out of there.
it's definitely not too late. and i disagree that we should keep docker
specific logic out of the executor. that's *exactly* where it should be.
afaict, the only thing affected is LogOptions.set_log_dir('.') in
thermos_executor_main. this can easily be changed to
LogOptions.set_log_dir(os.getenv('MESOS_DIRECTORY') or '.')
*ideally* in my opinion, the logical conclusion is creating a
thermos_docker_executor_main that wires things up in accordance to the docker
philosophy, then have a completely separate docker task factory within the
scheduler to construct these. but that might be a little more unsavory in the
short term than just having the thermos_executor that understands both
containerized and non-containerized modes. there is precedence for this
already (exactly why we have sandbox providers and task providers in the first
place -- to switch implementations entirely based upon incoming TaskConfigs.)
- Brian
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28920/#review69288
---
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
Re: Review Request 28920: Add support for docker containers to aurora
On Jan. 22, 2015, 10:42 p.m., Brian Wickman wrote:
src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java,
lines 153-158
https://reviews.apache.org/r/28920/diff/18/?file=823211#file823211line153
can't the stuff in DOCKER_COMMAND_PREFIX be accomplished with a
DockerSandboxProvider in the thermos executor?
similarly, DOCKER_COMMAND_SUFFIX should be unnecessary since this
information is available in the TaskConfig.
if both the above are true, then we don't need any of the changes to
CommandUtil.
Steve Niemitz wrote:
No unfortunatly, once the executor is started it's too late to change
some things, they need to be setup before the executor launches (see my
previous comment w/ Kevin). The suffix could be moved into the executor, but
again I'd like to keep docker specific logic out of there.
Brian Wickman wrote:
it's definitely not too late. and i disagree that we should keep docker
specific logic out of the executor. that's *exactly* where it should be.
afaict, the only thing affected is LogOptions.set_log_dir('.') in
thermos_executor_main. this can easily be changed to
LogOptions.set_log_dir(os.getenv('MESOS_DIRECTORY') or '.')
*ideally* in my opinion, the logical conclusion is creating a
thermos_docker_executor_main that wires things up in accordance to the docker
philosophy, then have a completely separate docker task factory within the
scheduler to construct these. but that might be a little more unsavory in
the short term than just having the thermos_executor that understands both
containerized and non-containerized modes. there is precedence for this
already (exactly why we have sandbox providers and task providers in the
first place -- to switch implementations entirely based upon incoming
TaskConfigs.)
Steve Niemitz wrote:
I can investiate moving it into the executor, but building an entirely
seperate executor for docker is more than I'm willing to take on at this
point.
I need to think about this more, using your suggestion of
LogOptions.set_log_dir(os.getenv('MESOS_DIRECTORY') or '.') fails because the
directory doesn't yet exist (since the symlink hasn't yet been created). Its
possible we could defer setting up logging until the executor gets a task and
set everything up in aurora_executor, but I dont know what else that might
affect.
- Steve
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28920/#review69288
---
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
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69236 --- Ship it! Master (116ee2d) 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 Jan. 22, 2015, 7:25 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 7:25 p.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
On Jan. 22, 2015, 7:43 p.m., Kevin Sweeney wrote: We've explored this in the past, but how about moving the authentication code to a decorator class and delegating the behavior-once-authenticated. Maxim Khutornenko wrote: We have a TODO tracking this: ``` // TODO(Sathya): Remove this after AOP-style session validation passes in a SessionContext. ``` I'd rather see this addressed separately as it will pollute this diff too much. I agree. I also think it would be a waste of effort with AURORA-809 on the horizon. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69232 --- On Jan. 22, 2015, 7:25 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 7:25 p.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Review Request 30178: Avoid performing RPC authentication while holding the write lock.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69232 --- We've explored this in the past, but how about moving the authentication code to a decorator class and delegating the behavior-once-authenticated. - Kevin Sweeney On Jan. 22, 2015, 11:25 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 11:25 a.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
On Jan. 22, 2015, 7:43 p.m., Kevin Sweeney wrote: We've explored this in the past, but how about moving the authentication code to a decorator class and delegating the behavior-once-authenticated. We have a TODO tracking this: ``` // TODO(Sathya): Remove this after AOP-style session validation passes in a SessionContext. ``` I'd rather see this addressed separately as it will pollute this diff too much. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69232 --- On Jan. 22, 2015, 7:25 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 7:25 p.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69243 --- Ship it! Ship It! - Kevin Sweeney On Jan. 22, 2015, 11:25 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 11:25 a.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 7:25 p.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69231 --- Ship it! src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java https://reviews.apache.org/r/30178/#comment113859 This is only used in one place, inline? - Maxim Khutornenko On Jan. 22, 2015, 7:25 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 7:25 p.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
On Jan. 22, 2015, 7:38 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 711 https://reviews.apache.org/r/30178/diff/1/?file=830099#file830099line711 This is only used in one place, inline? I pulled this out because a `final` variable was needed due to it being accessed from an anonymous inner class. I extracted the method as a compromise to avoid an awkward variable rename due to the reassignment. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69231 --- On Jan. 22, 2015, 7:25 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 7:25 p.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
On Jan. 22, 2015, 11:43 a.m., Kevin Sweeney wrote: We've explored this in the past, but how about moving the authentication code to a decorator class and delegating the behavior-once-authenticated. Maxim Khutornenko wrote: We have a TODO tracking this: ``` // TODO(Sathya): Remove this after AOP-style session validation passes in a SessionContext. ``` I'd rather see this addressed separately as it will pollute this diff too much. Bill Farner wrote: I agree. I also think it would be a waste of effort with AURORA-809 on the horizon. I think it would be complimentary to AURORA-809. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69232 --- On Jan. 22, 2015, 11:25 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 11:25 a.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30178: Avoid performing RPC authentication while holding the write lock.
On Jan. 22, 2015, 7:43 p.m., Kevin Sweeney wrote: We've explored this in the past, but how about moving the authentication code to a decorator class and delegating the behavior-once-authenticated. Maxim Khutornenko wrote: We have a TODO tracking this: ``` // TODO(Sathya): Remove this after AOP-style session validation passes in a SessionContext. ``` I'd rather see this addressed separately as it will pollute this diff too much. Bill Farner wrote: I agree. I also think it would be a waste of effort with AURORA-809 on the horizon. Kevin Sweeney wrote: I think it would be complimentary to AURORA-809. You're probably right - but i would really prefer to keep this diff tactical. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/#review69232 --- On Jan. 22, 2015, 7:25 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30178/ --- (Updated Jan. 22, 2015, 7:25 p.m.) Review request for Aurora, Kevin Sweeney and Steve Niemitz. Bugs: AURORA-1045 https://issues.apache.org/jira/browse/AURORA-1045 Repository: aurora Description --- The primary metric for success with this patch is to never interact with `sessionValidator` within a `storage.write` closure. This had two outcomes: 1. collapsing update-related RPC implementations for better DRY behavior 2. refactoring `killTasks` (2) has a behavioral change, though i think it's the correct behavior. For example, before this patch you could successfully kill all `PENDING` tasks, as long as you happened to own those tasks. The new behavior denies these requests for non-admin users regardless of the result of the query. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ad9126c32893080e128d086ea3bfd7ad23d27b89 Diff: https://reviews.apache.org/r/30178/diff/ Testing --- Thanks, Bill Farner
