You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Maxim Khutornenko <ma...@apache.org> on 2014/10/22 23:06:54 UTC
Re: Review Request 26762: Preparing for Identity struct deprecation
(scheduler).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26762/
-----------------------------------------------------------
(Updated Oct. 22, 2014, 9:06 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Changes
-------
Updating title and description.
Summary (updated)
-----------------
Preparing for Identity struct deprecation (scheduler).
Bugs: AURORA-84
https://issues.apache.org/jira/browse/AURORA-84
Repository: aurora
Description (updated)
-------
This patch replaces the Identity struct use within the scheduler and makes it ready for removal (i.e. when the JobStore is ready).
Sending it as a separate CR for easier reviewing.
Will have to be committed along with python changes (https://reviews.apache.org/r/26954/) to avoid breaking client diff functionality.
Summary of the changes:
* TaskConfig - dual write in StorageBackfill to populate new _key_ field. Incoming thrift objects are populated in ConfigurationManager during sanitizing.
* TaskQuery - _owner_ to _role_ switch is handled in Query.Builder. All internal searching is now handled via _role_.
* JobConfiguration - internal _owner_ refs redirected to _key.role_ 0.7.0.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c
src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 5c75cc8cae53edfa069c85c37ebad34774682081
src/main/java/org/apache/aurora/scheduler/TaskVars.java f1ab934541ad6d9ae74927f80a9c654a04922eb5
src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8
src/main/java/org/apache/aurora/scheduler/base/JobKeys.java a76c3fac71b35115064fba6644cff0066fd9e630
src/main/java/org/apache/aurora/scheduler/base/Query.java eded7a59eb394748b93d7fbc085a1bdf64b043cc
src/main/java/org/apache/aurora/scheduler/base/Tasks.java 6ad79270c35c4fccb01f29d34ef1c4bbd7c953c8
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2
src/main/java/org/apache/aurora/scheduler/http/Utilization.java a0cb7bf56aeb7edd92b25d8d69a739d87452777a
src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 5f08997f04ffa7d9610c2b41551943b563412626
src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 0f6731106c53420b92e60b9faf26c3614bd7ae00
src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 58b94c2f2f3bac00f0692579974e8bdf159b6e40
src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 37176237fac336413267f3c8bb4e1b9a6255150c
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 137f97d33decd14bf2f6dcdd9cd18c3db2b7c89c
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 6ec130f4a9a5075b34452efb27c8fd0f08f93a63
src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a
src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594
src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 371ae87f5954fa5f092db1f6d21e2291d7576173
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 606c4434b7158220ccf1403b6deac939021fee31
src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6
src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc
src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142
src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87
src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java d2d3e86bb5acf3402f55188b9ae440412ef14b5a
src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 33790b118e788d7c894f7635f896619a3266192a
src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 21640f7ec2172d4c1b1bc744a4d71a6fa0a29376
src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java cdd29ea2b6fc92b967571028d299260556e16d42
src/test/java/org/apache/aurora/scheduler/stats/ResourceCounterTest.java fc12933cefdbfc03de5918de04172c0744d34588
src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 80646a685ea918d80efafc5773e5805000a9c012
src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 3b61ff3c237de3ec5224a239d1756ce8d7093a52
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c3f0bbe3745e24519438be7e9e73a4698061cb52
src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java 5242a43b1c8a236b9420625a64ff24ad9ff75643
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f739e6d1b1af6eea4875e03d32bfe88cef87b3ff
Diff: https://reviews.apache.org/r/26762/diff/
Testing
-------
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 26762: Preparing for Identity struct deprecation
(scheduler).
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26762/#review57400
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
<https://reviews.apache.org/r/26762/#comment98769>
Is this safe to remove? I see red but no corresponding green.
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
<https://reviews.apache.org/r/26762/#comment98774>
Does it make sense to reorder this for more code reuse and predictable outcome?
- if key is not set, populate it
- validate key _and_ owner
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/26762/#comment98789>
For other reviewers - this is safe because `SanitizedConfiguration.fromUnsanitized` requires consistency between `key` and `owner` fields, so worst case is that they are inconsistent (and would fail sanitization anyway) and you auth against the wrong user.
src/main/thrift/org/apache/aurora/gen/api.thrift
<https://reviews.apache.org/r/26762/#comment98763>
How about s/key/job/? It's less ambiguous (since we could conceivably introduce a TaskConfigKey), and sufficiently desriptive.
- Bill Farner
On Oct. 22, 2014, 9:06 p.m., Maxim Khutornenko wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26762/
> -----------------------------------------------------------
>
> (Updated Oct. 22, 2014, 9:06 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Bill Farner.
>
>
> Bugs: AURORA-84
> https://issues.apache.org/jira/browse/AURORA-84
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This patch replaces the Identity struct use within the scheduler and makes it ready for removal (i.e. when the JobStore is ready).
>
> Sending it as a separate CR for easier reviewing.
>
> Will have to be committed along with python changes (https://reviews.apache.org/r/26954/) to avoid breaking client diff functionality.
>
> Summary of the changes:
> * TaskConfig - dual write in StorageBackfill to populate new _key_ field. Incoming thrift objects are populated in ConfigurationManager during sanitizing.
> * TaskQuery - _owner_ to _role_ switch is handled in Query.Builder. All internal searching is now handled via _role_.
> * JobConfiguration - internal _owner_ refs redirected to _key.role_ 0.7.0.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/MesosTaskFactory.java 83d0406a8bc7ccc1ae29804d2a4c8e8dfb90072c
> src/main/java/org/apache/aurora/scheduler/TaskIdGenerator.java 5c75cc8cae53edfa069c85c37ebad34774682081
> src/main/java/org/apache/aurora/scheduler/TaskVars.java f1ab934541ad6d9ae74927f80a9c654a04922eb5
> src/main/java/org/apache/aurora/scheduler/async/Preemptor.java e9f251508257cd7287ff00773e0073a3cd130df8
> src/main/java/org/apache/aurora/scheduler/base/JobKeys.java a76c3fac71b35115064fba6644cff0066fd9e630
> src/main/java/org/apache/aurora/scheduler/base/Query.java eded7a59eb394748b93d7fbc085a1bdf64b043cc
> src/main/java/org/apache/aurora/scheduler/base/Tasks.java 6ad79270c35c4fccb01f29d34ef1c4bbd7c953c8
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 865742171c11fbe5cf1469a69dd7258ec1be28c2
> src/main/java/org/apache/aurora/scheduler/http/Utilization.java a0cb7bf56aeb7edd92b25d8d69a739d87452777a
> src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 5f08997f04ffa7d9610c2b41551943b563412626
> src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 0f6731106c53420b92e60b9faf26c3614bd7ae00
> src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 58b94c2f2f3bac00f0692579974e8bdf159b6e40
> src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6
> src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 37176237fac336413267f3c8bb4e1b9a6255150c
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 137f97d33decd14bf2f6dcdd9cd18c3db2b7c89c
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 6ec130f4a9a5075b34452efb27c8fd0f08f93a63
> src/main/thrift/org/apache/aurora/gen/api.thrift 8794731f4b3f1033588bdfa33c292e4796319a2a
> src/test/java/org/apache/aurora/scheduler/MesosTaskFactoryImplTest.java e96974764844b5d1a3a05f6996075fccee209594
> src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 371ae87f5954fa5f092db1f6d21e2291d7576173
> src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 606c4434b7158220ccf1403b6deac939021fee31
> src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6
> src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc
> src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142
> src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87
> src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java d2d3e86bb5acf3402f55188b9ae440412ef14b5a
> src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 33790b118e788d7c894f7635f896619a3266192a
> src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 21640f7ec2172d4c1b1bc744a4d71a6fa0a29376
> src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java cdd29ea2b6fc92b967571028d299260556e16d42
> src/test/java/org/apache/aurora/scheduler/stats/ResourceCounterTest.java fc12933cefdbfc03de5918de04172c0744d34588
> src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 80646a685ea918d80efafc5773e5805000a9c012
> src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 3b61ff3c237de3ec5224a239d1756ce8d7093a52
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java c3f0bbe3745e24519438be7e9e73a4698061cb52
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java 5242a43b1c8a236b9420625a64ff24ad9ff75643
> src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f739e6d1b1af6eea4875e03d32bfe88cef87b3ff
>
> Diff: https://reviews.apache.org/r/26762/diff/
>
>
> Testing
> -------
>
> ./gradlew -Pq build
>
>
> Thanks,
>
> Maxim Khutornenko
>
>