Repository: aurora Updated Branches: refs/heads/master 9ab3ede57 -> 2ef6a05e2
Remove "enable_legacy_constraints" flag. Remove the "enable_legacy_constraints" flag and associated behaviour. Testing Done: ./gradlew build -Pq Bugs closed: AURORA-1074 Reviewed at https://reviews.apache.org/r/35812/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/2ef6a05e Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/2ef6a05e Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/2ef6a05e Branch: refs/heads/master Commit: 2ef6a05e2ec31b4a4a66cf1f255733045c3160a6 Parents: 9ab3ede Author: Zameer Manji <[email protected]> Authored: Fri Jun 26 11:09:06 2015 -0700 Committer: [email protected] <[email protected]> Committed: Fri Jun 26 11:09:06 2015 -0700 ---------------------------------------------------------------------- NEWS | 3 + .../configuration/ConfigurationManager.java | 65 +------------------- .../local/simulator/ClusterSimulatorModule.java | 6 +- .../configuration/ConfigurationManagerTest.java | 29 --------- .../apache/aurora/scheduler/mesos/Offers.java | 4 +- .../thrift/SchedulerThriftInterfaceTest.java | 4 +- 6 files changed, 9 insertions(+), 102 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/2ef6a05e/NEWS ---------------------------------------------------------------------- diff --git a/NEWS b/NEWS index 1a0fb48..fd5b708 100644 --- a/NEWS +++ b/NEWS @@ -3,3 +3,6 @@ - Now requires JRE 8 or greater. - GC executor is fully replaced by the task state reconciliation (AURORA-1047). +- The scheduler command line argument 'enable_legacy_constraints' has been + removed, and the scheduler no longer automatically injects 'host' and 'rack' + constraints for production services. (AURORA-1074) http://git-wip-us.apache.org/repos/asf/aurora/blob/2ef6a05e/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java index b777777..a1ca93e 100644 --- a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java +++ b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java @@ -32,12 +32,10 @@ import com.google.common.collect.Sets; import com.twitter.common.args.Arg; import com.twitter.common.args.CmdLine; import com.twitter.common.base.Closure; -import com.twitter.common.base.MorePreconditions; import org.apache.aurora.gen.Constraint; import org.apache.aurora.gen.Container; import org.apache.aurora.gen.JobConfiguration; -import org.apache.aurora.gen.LimitConstraint; import org.apache.aurora.gen.MesosContainer; import org.apache.aurora.gen.TaskConfig; import org.apache.aurora.gen.TaskConfig._Fields; @@ -66,15 +64,8 @@ public final class ConfigurationManager { private static final Arg<List<Container._Fields>> ALLOWED_CONTAINER_TYPES = Arg.<List<Container._Fields>>create(ImmutableList.of(Container._Fields.MESOS)); - @CmdLine(name = "enable_legacy_constraints", - help = "Set a default host:limit:1 for all and rack:limit:1 constraint for production jobs") - private static final Arg<Boolean> ENABLE_LEGACY_CONSTRAINTS = Arg.create(true); - public static final String DEDICATED_ATTRIBUTE = "dedicated"; - @VisibleForTesting public static final String HOST_CONSTRAINT = "host"; - @VisibleForTesting public static final String RACK_CONSTRAINT = "rack"; - private static final Pattern GOOD_IDENTIFIER = Pattern.compile(GOOD_IDENTIFIER_PATTERN_JVM); private static final int MAX_IDENTIFIER_LENGTH = 255; @@ -371,70 +362,18 @@ public final class ConfigurationManager { * @return A filter that matches the constraint. */ public static Predicate<IConstraint> getConstraintByName(final String name) { - return new Predicate<IConstraint>() { - @Override - public boolean apply(IConstraint constraint) { - return constraint.getName().equals(name); - } - }; - } - - @VisibleForTesting - public static Constraint hostLimitConstraint(int limit) { - return new Constraint(HOST_CONSTRAINT, TaskConstraint.limit(new LimitConstraint(limit))); - } - - @VisibleForTesting - public static Constraint rackLimitConstraint(int limit) { - return new Constraint(RACK_CONSTRAINT, TaskConstraint.limit(new LimitConstraint(limit))); - } - - @VisibleForTesting - static Predicate<Constraint> hasName(final String name) { - MorePreconditions.checkNotBlank(name); - return new Predicate<Constraint>() { - @Override - public boolean apply(Constraint constraint) { - return name.equals(constraint.getName()); - } - }; - } - - /** - * Wrapper for applyDefaultsIfUnset that passes along the DISABLE_LEGACY_CONSTRAINTS. - * @param task Task to apply defaults to. - * @return A reference to the (modified) {@code task}. - */ - @VisibleForTesting - public static TaskConfig applyDefaultsIfUnset(TaskConfig task) { - return applyDefaultsIfUnset(task, ENABLE_LEGACY_CONSTRAINTS.get()); + return constraint -> constraint.getName().equals(name); } /** * Applies defaults to unset values in a task. * @param task Task to apply defaults to. - * @param useLegacyConstraints flag to decide if we should add legacy constraints or not. * @return A reference to the (modified) {@code task}. */ - @VisibleForTesting - static TaskConfig applyDefaultsIfUnset(TaskConfig task, boolean useLegacyConstraints) { + public static TaskConfig applyDefaultsIfUnset(TaskConfig task) { for (Closure<TaskConfig> populator : DEFAULT_FIELD_POPULATORS) { populator.execute(task); } - - // Adding a default Host&Rack constraint is legacy behaviour that can be disabled if needed. - if (useLegacyConstraints) { - if (!Iterables.any(task.getConstraints(), hasName(HOST_CONSTRAINT))) { - task.addToConstraints(hostLimitConstraint(1)); - } - if (!isDedicated(IConstraint.setFromBuilders(task.getConstraints())) - && task.isProduction() - && task.isIsService() - && !Iterables.any(task.getConstraints(), hasName(RACK_CONSTRAINT))) { - task.addToConstraints(rackLimitConstraint(1)); - } - } - return task; } http://git-wip-us.apache.org/repos/asf/aurora/blob/2ef6a05e/src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java b/src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java index 9ee4fe2..cd0f981 100644 --- a/src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java +++ b/src/test/java/org/apache/aurora/scheduler/app/local/simulator/ClusterSimulatorModule.java @@ -31,8 +31,6 @@ import org.apache.mesos.Protos.Offer; import static java.util.Objects.requireNonNull; import static org.apache.aurora.scheduler.configuration.ConfigurationManager.DEDICATED_ATTRIBUTE; -import static org.apache.aurora.scheduler.configuration.ConfigurationManager.HOST_CONSTRAINT; -import static org.apache.aurora.scheduler.configuration.ConfigurationManager.RACK_CONSTRAINT; import static org.apache.mesos.Protos.Value.Type.RANGES; import static org.apache.mesos.Protos.Value.Type.SCALAR; @@ -97,10 +95,10 @@ public class ClusterSimulatorModule extends AbstractModule { .addResources(Protos.Resource.newBuilder().setType(RANGES).setName(Resources.PORTS) .setRanges(portRanges)) .addAttributes(Protos.Attribute.newBuilder().setType(Protos.Value.Type.TEXT) - .setName(HOST_CONSTRAINT) + .setName("host") .setText(Protos.Value.Text.newBuilder().setValue(host))) .addAttributes(Protos.Attribute.newBuilder().setType(Protos.Value.Type.TEXT) - .setName(RACK_CONSTRAINT) + .setName("rack") .setText(Protos.Value.Text.newBuilder().setValue(rack))) .setSlaveId(Protos.SlaveID.newBuilder().setValue(slaveId)) .setHostname(host) http://git-wip-us.apache.org/repos/asf/aurora/blob/2ef6a05e/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java b/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java index abbd23d..92ba450 100644 --- a/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java +++ b/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java @@ -15,8 +15,6 @@ package org.apache.aurora.scheduler.configuration; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; -import com.google.common.collect.Sets; import org.apache.aurora.gen.Constraint; import org.apache.aurora.gen.Container; @@ -36,9 +34,6 @@ import org.junit.Test; import static org.apache.aurora.gen.test.testConstants.INVALID_IDENTIFIERS; import static org.apache.aurora.gen.test.testConstants.VALID_IDENTIFIERS; import static org.apache.aurora.scheduler.configuration.ConfigurationManager.DEDICATED_ATTRIBUTE; -import static org.apache.aurora.scheduler.configuration.ConfigurationManager.HOST_CONSTRAINT; -import static org.apache.aurora.scheduler.configuration.ConfigurationManager.RACK_CONSTRAINT; -import static org.apache.aurora.scheduler.configuration.ConfigurationManager.hasName; import static org.apache.aurora.scheduler.configuration.ConfigurationManager.isGoodIdentifier; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -122,28 +117,4 @@ public class ConfigurationManagerTest { ConfigurationManager.validateAndPopulate(ITaskConfig.build(taskConfig)); } - - @Test - public void testDisableLegacyConstraints() { - // By default, legacy constraints are applied to production jobs. - TaskConfig task = new TaskConfig() - .setIsService(true) - .setEnvironment("prod") - .setConstraints(Sets.<Constraint>newHashSet()) - .setProduction(true); - - TaskConfig copy = task.deepCopy(); - - task = ConfigurationManager.applyDefaultsIfUnset(task, false); - - // In this case we shouldn't have those fields added. - assertFalse(Iterables.any(task.getConstraints(), hasName(HOST_CONSTRAINT))); - assertFalse(Iterables.any(task.getConstraints(), hasName(RACK_CONSTRAINT))); - - copy = ConfigurationManager.applyDefaultsIfUnset(copy, true); - - // In this case we should have those fields added. - assertTrue(Iterables.any(copy.getConstraints(), hasName(HOST_CONSTRAINT))); - assertTrue(Iterables.any(copy.getConstraints(), hasName(RACK_CONSTRAINT))); - } } http://git-wip-us.apache.org/repos/asf/aurora/blob/2ef6a05e/src/test/java/org/apache/aurora/scheduler/mesos/Offers.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/mesos/Offers.java b/src/test/java/org/apache/aurora/scheduler/mesos/Offers.java index 83eec5d..c10d5b6 100644 --- a/src/test/java/org/apache/aurora/scheduler/mesos/Offers.java +++ b/src/test/java/org/apache/aurora/scheduler/mesos/Offers.java @@ -26,8 +26,6 @@ import org.apache.mesos.Protos.Value.Ranges; import org.apache.mesos.Protos.Value.Scalar; import org.apache.mesos.Protos.Value.Type; -import static org.apache.aurora.scheduler.configuration.ConfigurationManager.HOST_CONSTRAINT; - public final class Offers { private Offers() { // Utility class. @@ -54,7 +52,7 @@ public final class Offers { .addResources(Resource.newBuilder().setType(Type.RANGES).setName(Resources.PORTS) .setRanges(portRanges)) .addAttributes(Protos.Attribute.newBuilder().setType(Type.TEXT) - .setName(HOST_CONSTRAINT) + .setName("host") .setText(Protos.Value.Text.newBuilder().setValue("slavehost"))) .setSlaveId(SlaveID.newBuilder().setValue("SlaveId").build()) .setHostname("slavehost") http://git-wip-us.apache.org/repos/asf/aurora/blob/2ef6a05e/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java index 71b09b1..ee8f542 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -560,9 +560,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { .setProduction(true) .setRequestedPorts(ImmutableSet.<String>of()) .setTaskLinks(ImmutableMap.<String, String>of()) - .setConstraints(ImmutableSet.of( - ConfigurationManager.hostLimitConstraint(1), - ConfigurationManager.rackLimitConstraint(1))) + .setConstraints(ImmutableSet.of()) .setMaxTaskFailures(1) .setEnvironment("devel");
