Repository: aurora Updated Branches: refs/heads/master e0c9e08d0 -> efffd85f6
Fixed starting cron jobs when using default_docker_parameters The code was previously attempting to re-sanitize the configuration read from storage rather than just using it as is. This causes issues if after sanitization the job no longer passes sanitization (which is the case here w/ default_docker_parameters). We've been running this in our branch forever. Bugs closed: AURORA-1684 Reviewed at https://reviews.apache.org/r/54754/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/efffd85f Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/efffd85f Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/efffd85f Branch: refs/heads/master Commit: efffd85f64234d7210c9fea5d1ae6472659174b0 Parents: e0c9e08 Author: Steve Niemitz <steveniem...@gmail.com> Authored: Tue Jan 31 18:18:42 2017 +0100 Committer: Stephan Erb <s...@apache.org> Committed: Tue Jan 31 18:18:42 2017 +0100 ---------------------------------------------------------------------- .../apache/aurora/scheduler/cron/quartz/AuroraCronJob.java | 9 +++------ .../apache/aurora/scheduler/cron/quartz/CronLifecycle.java | 9 +++------ .../aurora/scheduler/cron/quartz/AuroraCronJobTest.java | 2 -- 3 files changed, 6 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/efffd85f/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java index 5873983..d890cad 100644 --- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java +++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java @@ -38,7 +38,7 @@ import org.apache.aurora.scheduler.BatchWorker.NoResult; import org.apache.aurora.scheduler.base.JobKeys; import org.apache.aurora.scheduler.base.Query; import org.apache.aurora.scheduler.base.Tasks; -import org.apache.aurora.scheduler.configuration.ConfigurationManager; +import org.apache.aurora.scheduler.configuration.SanitizedConfiguration; import org.apache.aurora.scheduler.cron.CronException; import org.apache.aurora.scheduler.cron.SanitizedCronJob; import org.apache.aurora.scheduler.events.PubsubEvent.EventSubscriber; @@ -89,7 +89,6 @@ class AuroraCronJob implements Job, EventSubscriber { @VisibleForTesting static final Optional<String> KILL_AUDIT_MESSAGE = Optional.of("Killed by cronScheduler"); - private final ConfigurationManager configurationManager; private final StateManager stateManager; private final BackoffHelper delayedStartBackoff; private final BatchWorker<NoResult> batchWorker; @@ -121,12 +120,10 @@ class AuroraCronJob implements Job, EventSubscriber { @Inject AuroraCronJob( - ConfigurationManager configurationManager, Config config, StateManager stateManager, CronBatchWorker batchWorker) { - this.configurationManager = requireNonNull(configurationManager); this.stateManager = requireNonNull(stateManager); this.batchWorker = requireNonNull(batchWorker); this.delayedStartBackoff = requireNonNull(config.getDelayedStartBackoff()); @@ -171,8 +168,8 @@ class AuroraCronJob implements Job, EventSubscriber { SanitizedCronJob cronJob; try { - cronJob = SanitizedCronJob.fromUnsanitized(configurationManager, config.get()); - } catch (ConfigurationManager.TaskDescriptionException | CronException e) { + cronJob = SanitizedCronJob.from(new SanitizedConfiguration(config.get())); + } catch (CronException e) { LOG.warn("Invalid cron job for {} in storage - failed to parse", key, e); CRON_JOB_PARSE_FAILURES.incrementAndGet(); return BatchWorker.NO_RESULT; http://git-wip-us.apache.org/repos/asf/aurora/blob/efffd85f/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java index 650face..f18e1dc 100644 --- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java +++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java @@ -21,7 +21,7 @@ import javax.inject.Inject; import com.google.common.util.concurrent.AbstractIdleService; import org.apache.aurora.common.stats.Stats; -import org.apache.aurora.scheduler.configuration.ConfigurationManager; +import org.apache.aurora.scheduler.configuration.SanitizedConfiguration; import org.apache.aurora.scheduler.cron.CronException; import org.apache.aurora.scheduler.cron.SanitizedCronJob; import org.apache.aurora.scheduler.storage.Storage; @@ -43,19 +43,16 @@ class CronLifecycle extends AbstractIdleService { private static final AtomicInteger LOADED_FLAG = Stats.exportInt("cron_jobs_loaded"); private static final AtomicLong LAUNCH_FAILURES = Stats.exportLong("cron_job_launch_failures"); - private final ConfigurationManager configurationManager; private final Scheduler scheduler; private final CronJobManagerImpl cronJobManager; private final Storage storage; @Inject CronLifecycle( - ConfigurationManager configurationManager, Scheduler scheduler, CronJobManagerImpl cronJobManager, Storage storage) { - this.configurationManager = requireNonNull(configurationManager); this.scheduler = requireNonNull(scheduler); this.cronJobManager = requireNonNull(cronJobManager); this.storage = requireNonNull(storage); @@ -69,11 +66,11 @@ class CronLifecycle extends AbstractIdleService { for (IJobConfiguration job : Storage.Util.fetchCronJobs(storage)) { try { - SanitizedCronJob cronJob = SanitizedCronJob.fromUnsanitized(configurationManager, job); + SanitizedCronJob cronJob = SanitizedCronJob.from(new SanitizedConfiguration(job)); cronJobManager.scheduleJob( cronJob.getCrontabEntry(), cronJob.getSanitizedConfig().getJobConfig().getKey()); - } catch (CronException | ConfigurationManager.TaskDescriptionException e) { + } catch (CronException e) { logLaunchFailure(job, e); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/efffd85f/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java b/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java index fb06c28..2b81707 100644 --- a/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java +++ b/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java @@ -28,7 +28,6 @@ import org.apache.aurora.gen.ScheduledTask; import org.apache.aurora.scheduler.BatchWorker; import org.apache.aurora.scheduler.BatchWorker.RepeatableWork; import org.apache.aurora.scheduler.base.JobKeys; -import org.apache.aurora.scheduler.base.TaskTestUtil; import org.apache.aurora.scheduler.cron.quartz.AuroraCronJob.CronBatchWorker; import org.apache.aurora.scheduler.state.StateChangeResult; import org.apache.aurora.scheduler.state.StateManager; @@ -80,7 +79,6 @@ public class AuroraCronJobTest extends EasyMockTest { expectBatchExecute(batchWorker, storage, control).anyTimes(); auroraCronJob = new AuroraCronJob( - TaskTestUtil.CONFIGURATION_MANAGER, new AuroraCronJob.Config(backoffHelper), stateManager, batchWorker);
